lczech / gappa

A toolkit for analyzing and visualizing phylogenetic (placement) data
GNU General Public License v3.0
56 stars 7 forks source link

GAPPA conda package build error #4

Closed gavinmdouglas closed 5 years ago

gavinmdouglas commented 5 years ago

Hi there,

This error occurs during the CircleCI build of the GAPPA conda package I'm working on:

In file included from /opt/conda/conda-bld/gappa_1546532283897/work/src/options/matrix_output.hpp:32:0,

                 from /opt/conda/conda-bld/gappa_1546532283897/work/src/options/matrix_output.cpp:24:

/opt/conda/conda-bld/gappa_1546532283897/work/libs/genesis/lib/genesis/utils/containers/matrix/writer.hpp: In constructor ‘genesis::utils::MatrixWriter<T>::MatrixWriter() [with T = double]’:

/opt/conda/conda-bld/gappa_1546532283897/work/libs/genesis/lib/genesis/utils/containers/matrix/writer.hpp:72:5: error: conversion from ‘const char [2]’ to non-scalar type ‘std::string {aka std::basic_string<char>}’ requested

     MatrixWriter() = default;

     ^

/opt/conda/conda-bld/gappa_1546532283897/work/src/options/matrix_output.cpp: In member function ‘void MatrixOutputOptions::write_matrix(const genesis::utils::Matrix<double>&, const std::vector<std::basic_string<char> >&, const std::vector<std::basic_string<char> >&, const string&) const’:

/opt/conda/conda-bld/gappa_1546532283897/work/src/options/matrix_output.cpp:99:40: note: synthesized method ‘genesis::utils::MatrixWriter<T>::MatrixWriter() [with T = double]’ first required here 

     auto writer = MatrixWriter<double>();

                                        ^

You can see the bioconda pull request here.

And the full CircleCI report here.

Thanks,

Gavin

lczech commented 5 years ago

Hi @gavinmdouglas,

thanks for reporting this issue, and sorry for the delay. This is a weird bug that probably is due to the old compiler you use (gcc 4.8.2). I applied a fix for the issue (you need to pull the latest master branch), but as this only seems to be an issue with old (unsupported) compilers, I cannot check whether this works now for you.

Note that genesis (and hence, gappa as well) generally expects gcc >= 4.9, see here: http://doc.genesis-lib.org/supplement_build_process.html - see Section "Supported Compilers". So, you can try to compile with gcc 4.8.2, but as stated in the link, this might lead to issues. I'd recommend using a more recent compiler.

Let me know if the fix works for you, and I'd be curious to hear whether your gcc version actually is able to compile and run the program.

Lucas

gavinmdouglas commented 5 years ago

Hi @lczech,

Thanks for the fix! Typical users wouldn't need to use the older gcc version, but I believe it's used in certain bioconda checks just to be safe, so I appreciate it.

After troubleshooting a few other issues the circleci checks have now passed, so it seems that lower gcc version was sufficient to compile (and run gappa -h).

Thanks,

Gavin

lczech commented 5 years ago

Hi Gavin,

I just had another look at why gcc < 4.9 might not work. It does not yet fully implement C++11 regex functions. Which means, it compiles (as you proofed), but it might crash when using regexes. Would you mind trying the following:

gappa prepare chunkify --fasta-path <some_directory>

where <some_directory> points to a dir with some *.fasta files in it. It has to be a directory, not a single file - because then a regex is used that looks for files with the correct extension is triggered.

Please let me know if this works. Thanks a lot in advance! Lucas

gavinmdouglas commented 5 years ago

Ok great, I'll test that out and get back to you.

By the way - apparently bioconda doesn't typically host development software (see comments at bottom of: https://github.com/bioconda/bioconda-recipes/pull/12897). Are you planning on making an official release in the near future? If not would you be opposed to me bumping the version number of my fork to 0.0.1? Note that I made a fork simply to make a stable download link for the repository.

gavinmdouglas commented 5 years ago

Hi again,

I realized that I had specified a newer GCC version in the conda recipe (GNU 7.3.0) after troubleshooting other issues with the build. I'm still not familiar with all of the checks that bioconda runs in circleci, but this appears to be the only GCC version tested now so no need to worry about incompatibilities with earlier versions. However, I did run the test you suggested as a sanity check and it did pass.

Also regarding the release status - a version tag is required for all packages on conda. Would you mind making a tag for the current version? No problem of course if you don't think the package is ready for that.

Best,

Gavin

lczech commented 5 years ago

Hi Gavin,

thanks for testing thing! So, do I understand correctly that the bioconda build uses GNU 7.3.0 then? In that case, everything should compile and work properly.

As for release: Yes, it's probably a good idea, as I am also about to publish two papers that use gappa, so it makes sense to refer to a specific version. Hence, I just created a release, which you can also use for bioconda. Be aware however that gappa is still in active development, and more features are planned to be added in the near future. How would you like to handle this? A bioconda update for every new release maybe? Do I need to notify you about this, or do you have some way of tracking progress?

Thanks a lot, all the best Lucas

gavinmdouglas commented 5 years ago

Yes that's right - GNU 7.3.0 is used now.

Great! An automatic PR should be made when you make a new release (and any changed dependencies can then be troubleshooted manually). For example, here is a recent automatic PR generated for the latest EPA-NG release: https://github.com/bioconda/bioconda-recipes/pull/13020. I'll watch the repo and make sure this happens the next time you make a release - no need to notify me.

Best,

Gavin

lczech commented 5 years ago

Perfect! Thank you very much for adding EPA-ng and gappa to bionconda!

gavinmdouglas commented 5 years ago

No worries! It can now be installed with: conda install -c biocore gappa