Unfortunately I cannot add attachments, because I get 503 :(

Created attachment 162571 [details]
update to new version

Comment 3 Bartek Rutkowski 2015-11-08 16:21:59 UTC

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

Comment 4 Jason Unovitch 2015-12-19 01:37:41 UTC

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.

Comment 10 Jason Unovitch 2016-01-18 17:39:41 UTC

(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.

Comment 17 Lars Engels 2016-04-23 11:48:11 UTC

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

Comment 21 Jason Unovitch 2016-04-28 00:56:59 UTC

(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!

Comment 22 Jason Unovitch 2016-04-29 01:43:31 UTC

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.

Comment 24 Jason Unovitch 2016-05-02 23:11:57 UTC

(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!

Comment 26 Jason Unovitch 2016-05-03 14:11:16 UTC

(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 :)

Comment 29 commit-hook 2016-05-10 00:52:12 UTC

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

Comment 30 commit-hook 2016-05-10 01:06:15 UTC

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

Comment 31 Jason Unovitch 2016-05-10 01:10:53 UTC

(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