Bug 204127
| Summary: | [MAINTAINER] biology/seqan split apps out into biology/seqan-apps | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Hannes Hauswedell <h2+fbsdports> | ||||||||||||||||||||
| Component: | Individual Port(s) | Assignee: | Jason Unovitch <junovitch> | ||||||||||||||||||||
| Status: | Closed FIXED | ||||||||||||||||||||||
| Severity: | Affects Only Me | CC: | junovitch, lme | ||||||||||||||||||||
| Priority: | --- | Keywords: | needs-qa, patch | ||||||||||||||||||||
| Version: | Latest | ||||||||||||||||||||||
| Hardware: | Any | ||||||||||||||||||||||
| OS: | Any | ||||||||||||||||||||||
| Bug Depends on: | |||||||||||||||||||||||
| Bug Blocks: | 196851 | ||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||
|
Description
Hannes Hauswedell
2015-10-29 15:22:22 UTC
Unfortunately I cannot add attachments, because I get 503 :( Created attachment 162571 [details]
update to new version
This patch fails to build the port with patch stage errors, can you test it, fix the issues and provide a new patch? http://pd.valinor.palantiri.org/data/93amd64-default/2015-11-08_17h19m53s/logs/errors/seqan-2.0.1.log Created attachment 164374 [details]
QA'd Updated patch
The patch QA requires:
svn rm files/patch-cmake-CMakeLists.txt
svn rm files/patch-cmake-apps-CMakeLists.txt
This patch also fixes bad tabs in the OPTIONS_DEFINE and DOCS_DESC line. The patches were safe to remove since this doesn't compile anything anymore (NO_BUILD).
However, can you provide feedback on the fact that this is now a NO_BUILD port and removes all the bin/* files? This feels like a POLA violation for anyone with a certain expectation of what this port is supposed to provide. I have reservations about committing anything without further confirmation on the intent of the changes and perhaps a recommendation of text to add to UPDATING regarding the bin/* removals.
> However, can you provide feedback on the fact that this is now a NO_BUILD port and removes all the bin/* files?
My plan was to provide an additional port for the binaries... anyways, I now also received feedback that users need the old port (for the seqan1 API), so I would propose:
* make a new biology/seqan2 port that includes only the library (similar to the currently proposed update)
* make a new biology/seqan-apps port that includes the corresponding apps (these will not be versioned since that doesnt make sense)
* fix the current biology/seqan port to work with newer compilers (this will probably imply deactivating some targets, but users will just have to move on)
What do you think?
Created attachment 165571 [details]
remove apps from port
This patch removes only the binaries from the port, it does not upgrade to a new version.
Created attachment 165572 [details]
new port for the apps
This shar contains a new port for the apps. The apps port is already based on version 2.0.1.
[Since SeqAn is a header-only library and the apps-package comes with the library source the two ports are completely independent]
The last two replies contain 2/3 of the solution. Please review and commit as you see fit. They should also resolve #196851 as biology/seqan is now independent of compilers and biology/seqan-apps works with newer versions of gcc also. The last third (a new library port for seqan2) will be submitted beginning of February following a new upstream release (2.1.0). I don't know how UPDATING changes are handled, but if you feel that these changes needs a message to UPDATING, it could be: The biology/seqan port has been split into biology/seqan (just the library, version 1.3.1) and biology/seqan-apps for the programs based on SeqAn. The latter already tracks version 2 of SeqAn and there will soon also be biology/seqan2 with a new version of the library. The apps port will always track the latest version, but for compatibility reasons there will be multiple library ports as long as multiple versions are in use. (In reply to Hannes Hauswedell from comment #5) Excellent. Thank you for the feedback. What would most end users be using? For example, the latest Ansible update split of a sysutils/ansible1 for the legacy release to deal with incompatible changes. Most users should be using the current release. https://svnweb.FreeBSD.org/changeset/ports/406016 Another example of the same port but now there is a 1.x and 2.x series with incompatible changes where both will be maintained by the upstream. https://svnweb.FreeBSD.org/changeset/ports/401977 So the answer is going to be, "it depends", on what most end users would use. We have two options. 1: If the Seqan 1 API is very limited use: - `svn cp seqan seqan1' to provide the legacy API, then update biology/seqan and add a biology/seqan-apps for the applications. 2: If upstream will maintain both and the Seqan 1 API will be around for a while then we should `svn cp seqan seqan2' and update the new biology/seqan2 with the patch attached. (In reply to Hannes Hauswedell from comment #9) Yes, that would be good as an UPDATING entry. It sounds like your preferences is option 2 but I'm not familiar with Seqan so I'm relying on your expertise to determine what most end users will use and expect to be in biology/seqan. Let me know and we'll press on with that route. > Excellent. Thank you for the feedback. What would most end users be using?
Hopefully version 2. There are no updates for version 1 anymore and upstream would really like everyone to switch.
So altogether your first solution seems more adequate. However that would mean we have to wait for the seqan2 upgrade before we split out seqan1. I want to wait for upstream's 2.1 release (due in about two weeks) before upgrading the port, because it contains FreeBSD-related fixes and also introduces incompatibilities to 2.0 -- so best to get no-one used to 2.0.
Do you agree? If not and you think it is more important to get the fixes in now then we should go with your second solution, i.e. please commit the "split-update" to biology/seqan (version1) and the new port of biology/seqan-apps.
in afterthought, please merge the discussed changes now. Copying the updated biology/seqan to biology/seqan1 and updating biology/seqan to 2.1.0 can be done in a month independently. I will open a new ticket for that. Created attachment 169191 [details]
patch for biology/seqan
Created attachment 169192 [details]
patch for biology/seqan1
Created attachment 169193 [details]
new port biology/seqan-apps
I have just created three new attachments (please deprecate "QA'd Updated patch"). Please proceed with them as follows: 0. cd biology 1. svn rm -r seqan/files 2. svn cp seqan seqan1 3. patch < seqan.patch # brings library port to version 2.1.1 4. patch < seqan1.patch # removes binares from old library port (now renamed to seqan1) 5. sh seqan-apps.shar # create the new port for the apps (based on 2.1.1) Please add the following to UPDATING The biology/seqan port has been split into biology/seqan (only the library) and biology/seqan-apps for the programs based on SeqAn. Both ports are based on version 2.1.1 of the SeqAn repository. There is a new biology/seqan1 port with version 1.3 of SeqAn for backwards compatibility, but this port will likely be deprecated in the next year so please update your software to SeqAn2. I tried it, but the patches do not apply cleanly. Created attachment 169671 [details]
patch for biology/seqan
Created attachment 169674 [details]
patch for biology/seqan1
Hi Lars, thanks for checking! I have recreated the patch for seqan1 and also uploaded a new patch for seqan, because I had the CONFLICTS lines wrong. Does it work for you now? Regards, Hannes (In reply to Hannes Hauswedell from comment #20) I've applied all the patches now and just fired off a build run overnight for QA. The only thing of note is seqan1 will be PORTNAME=seqan and PKGNAMESUFFIX=1 so the distinfo is still correct. I've fixed that. If I come across anything else I will handle it or mention it here. More to come when the builds finish overnight and I get some free time to get back at it. Thanks! These are the only questions I had. It's all in biology/seqan-apps ------------------------ Is this going to be an issue? Does this need Python as a BUILD_DEPENDS? It showed a bunch of this during the build. -- Could NOT find PythonInterp (missing: PYTHON_EXECUTABLE) ------------------------ There was some missing files that I added to the pkg-plist: ===> Checking for items in pkg-plist which are not in STAGEDIR Error: Missing: bin/bisar Error: Missing: bin/casbar Error: Missing: bin/compute_gain Error: Missing: bin/fiona Error: Missing: bin/four2three Error: Missing: bin/snp_store Error: Missing: share/doc/bs_tools/LICENSE Error: Missing: share/doc/bs_tools/README Error: Missing: share/doc/fiona/LICENSE Error: Missing: share/doc/fiona/README Error: Missing: share/doc/fiona/example/reads.fa Error: Missing: share/doc/snp_store/LICENSE Error: Missing: share/doc/snp_store/README Error: Missing: share/doc/snp_store/example/exampleGenome.fa Error: Missing: share/doc/snp_store/example/exampleReads.gff ------------------------ Is the first line here supposed to be commented? # once #199603 is resolved, do the following instead of USE_GCC USES= cmake:outsource compiler:openmp,c++14-lang USES= cmake:outsource ------------------------ (In reply to Jason Unovitch from comment #22) Thanks for testing and having a thorough look! > Does this need Python as a BUILD_DEPENDS? No, Python is only required for unit tests and some other unrelated things. I agree the buildsystem shouldn't be spamming out the messages, but it is not a problem from a technical point of view. > There was some missing files that I added to the pkg-plist: Ah, indeed, some of the apps require boost. I could make it optional in the future, but for now, could you just include boost-libs as a build dependency? [only the boost header library is used so it is not a run-depends, I checked with ldd on all affected binaries] > Is the first line here supposed to be commented? YES, totally :) Thank you for taking the time. (In reply to Hannes Hauswedell from comment #23) I'm seeing some build issues with the boost addition in place and the future USES line commented. The logs are located at https://people.FreeBSD.org/~junovitch/poudriere/PR204127/seqan-apps/ and the shar is at https://people.FreeBSD.org/~junovitch/poudriere/PR204127/seqan-apps/seqan-apps.shar Can you let me know what is missing here to get this to build properly? I'll be ready to commit the whole lot when we can get this sorted out. (In reply to Jason Unovitch from comment #24) ## Concerning the plist issues on freebsd >= 10 For some reason the problematic files appear twice in your shar file, that's why they are still listed. In my original upload they appeared only once. Did you maybe add them a second time after adding the boost dependency? ## Concerning the build error on freebsd9 FreeBSD9's math.h still lacks the C99 math functions logl and expl. A possible solution would be to just replace them with c++ equivalents in std namespace, e.g. replace "logl(" with "std::log(" and "expl(" with "std::exp(" in apps/snp_store/snp_store.cpp and apps/snp_store/snp_store.h Note that the c++ equivalents for long double have no l suffix: http://en.cppreference.com/w/cpp/numeric/math/log http://en.cppreference.com/w/c/numeric/math/log This way they might pick up the code from gcc49's stdlib. If this still doesn't work, one could accept loss in precision and just replace "logl" with "log" and "expl" with "exp" for FreeBSD9. I unfortunately don't have a FreeBSD9 box to test :( But it should only be one or two lines of sed... Thank you! (In reply to Hannes Hauswedell from comment #25) > Did you maybe add them a second time after adding the boost dependency? Yes, indeed. My mistake. The shar has been fixed. > I unfortunately don't have a FreeBSD9 box to test :( But it should only be one or two lines of sed... Have you tested in Poudriere at any point? It's fairly easy to do with a 9.3 jail on any newer version host. (In reply to Jason Unovitch from comment #26) > Have you tested in Poudriere at any point? It's fairly easy to do with a 9.3 jail on any newer version host. I have set up poudriere, but it took a while to build everything and to debug 9.3. The situation with math functions there is really sub-optimal. The additions to the Makefile at the end of the post fix the builds on my setup. Can you confirm this and if yes, merge? Thanks! .include <bsd.port.pre.mk> .if ${OSVERSION} < 1000000 post-patch: # missing C99 functions in FreeBSD's math.h, use C++ instead @${REINPLACE_CMD} -e 's|expl(|std::exp(|g' ${WRKSRC}/apps/snp_store/snp_store.h @${REINPLACE_CMD} -e 's|logl(|std::log(|g' ${WRKSRC}/apps/snp_store/snp_store.h # the other way around (use math.h instead of cmath) @${REINPLACE_CMD} -e 's|std::round|round|g' ${WRKSRC}/apps/yara/mapper_writer.h @${REINPLACE_CMD} -e 's|std::erfc|erfc|g' ${WRKSRC}/apps/yara/bits_matches.h .endif .include <bsd.port.post.mk> *polite ping* Anything from my side I can do to help? These issues have be haunting me for so long, I would love to finally close them :) A commit references this bug: Author: junovitch Date: Tue May 10 00:52:05 UTC 2016 New revision: 414897 URL: https://svnweb.freebsd.org/changeset/ports/414897 Log: New port: biology/seqan-apps SeqAn is an open source C++ library of efficient algorithms and data structures for the analysis of sequences with the focus on biological data. This port contains applications built on SeqAn and developed within the SeqAn project. Among them are famous read mappers like RazerS and Yara, as well as many other tools. Some applications are packaged seperately and the library can be found at biology/seqan. WWW: http://www.seqan.de/ PR: 204127 Submitted by: Hannes Hauswedell <h2+fbsdports@fsfe.org> Changes: head/biology/Makefile head/biology/seqan-apps/ head/biology/seqan-apps/Makefile head/biology/seqan-apps/distinfo head/biology/seqan-apps/pkg-descr head/biology/seqan-apps/pkg-plist A commit references this bug: Author: junovitch Date: Tue May 10 01:05:23 UTC 2016 New revision: 414898 URL: https://svnweb.freebsd.org/changeset/ports/414898 Log: biology/seqan: update 1.3.1 -> 2.1.1 biology/seqan1: create port from current SeqAn 1.3.1 for legacy usage UPDATING: document SeqAn updates and seqan1 port for legacy usage PR: 204127 Submitted by: Hannes Hauswedell <h2+fbsdports@fsfe.org> Changes: head/UPDATING head/biology/Makefile head/biology/seqan/Makefile head/biology/seqan/distinfo head/biology/seqan/files/ head/biology/seqan/pkg-descr head/biology/seqan/pkg-plist head/biology/seqan1/ head/biology/seqan1/Makefile head/biology/seqan1/files/ head/biology/seqan1/pkg-descr head/biology/seqan1/pkg-plist (In reply to Hannes Hauswedell from comment #28) No. The math.h -> C++ was sufficient on 9.x to build properly. I also added conditional checks for the binaries that don't build on i386 with a message like the following... -- Not building fiona on 32bit architectures. I believe that covers everything but if you see something I may have missed please let me know. Thank you very much +1 |
