Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add recipes for Chemfiles #1571

Merged
merged 1 commit into from
Oct 18, 2016
Merged

Add recipes for Chemfiles #1571

merged 1 commit into from
Oct 18, 2016

Conversation

Luthaf
Copy link
Contributor

@Luthaf Luthaf commented Sep 14, 2016

A C++ library for reading/writing theoretical chemistry trajectories, with Python bindings. See http://chemfiles.github.io/ for more informations.

I added three recipes: C++ library, Python binding and meta-package. If you prefer having everything in one recipe, I can to it.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/chemfiles, recipes/chemfiles-lib, recipes/chemfiles-python) and found some lint.

Here's what I've got...

For recipes/chemfiles:

  • The top level meta keys are in an unexpected order. Expecting ['package', 'requirements', 'about', 'extra'].
  • The recipe must have some tests.
  • The recipe must have a build/number section.

For recipes/chemfiles-lib:

  • The top level meta keys are in an unexpected order. Expecting ['package', 'source', 'build', 'requirements', 'about', 'extra'].
  • The recipe must have some tests.
  • The recipe must have a build/number section.

For recipes/chemfiles-python:

  • Failed to even lint the recipe (might be a conda-smithy bug) 😢

sha256: {{ sha256 }}

requirements:
build:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add toolchain. This configures the compiler to use and how to use it.

@jakirkham
Copy link
Member

Thanks for proposing these, @Luthaf. Glad to see you over here. 😄

Tried to add some addition comments in addition to what the linter included. Once we get these cleaned up a bit more, we can take a closer look.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/chemfiles, recipes/chemfiles-lib, recipes/chemfiles-python) and found some lint.

Here's what I've got...

For recipes/chemfiles:

  • The recipe must have some tests.

For recipes/chemfiles-lib:

  • The recipe must have some tests.

For recipes/chemfiles-python:

  • The top level meta keys are in an unexpected order. Expecting ['package', 'source', 'build', 'requirements', 'test', 'about', 'extra'].
  • The recipe must have a build/number section.

@Luthaf
Copy link
Contributor Author

Luthaf commented Sep 14, 2016

I addressed most of the comments; still have to enable tests for chemfiles-lib and check for what is going on with export MACOSX_DEPLOYMENT_TARGET="" # [osx].

@jakirkham
Copy link
Member

I know the linter is mentioning tests. There are a couple types of tests we like to see.

Running the test suite (mainly for compiled code) so make check, ctest or similar. Normally that happens in the build.

For Python libraries import tests in the test section are nice.

Some checks to make sure files (libraries, headers, configuration files, etc.) were installed where they should be.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/chemfiles, recipes/chemfiles-lib, recipes/chemfiles-python) and found it was in an excellent condition.

@Luthaf
Copy link
Contributor Author

Luthaf commented Sep 14, 2016

I actually need to export MACOSX_DEPLOYMENT_TARGET="", because without it I get a Bus Error (segfault) in one of my tests. I will try to debug this using the binary built by conda, but I cannot reproduce with a fresh build.

@Luthaf
Copy link
Contributor Author

Luthaf commented Sep 14, 2016

The Circle-CI failure is strange: I got an error about netcdf.h file not found, even though libnetcdf is part of the tests.require entry. Any idea on the origin of this failure?

@Luthaf
Copy link
Contributor Author

Luthaf commented Sep 16, 2016

So after investigations (see the linked issue), the bus error is very weird. I am in favor of just unsetting MACOSX_DEPLOYMENT_TARGET in the build, since that make the bug disappear.

This is fixed by updating Boost, and I release the 0.6.2 version with this fix on Chemfiles.

@jakirkham, I do not get the origin of the failure on travis/circle-ci, any idea? The Appveyor issue looks strange and related to installing libnetcdf with MSVC.

@Luthaf
Copy link
Contributor Author

Luthaf commented Sep 30, 2016

Appveyor failure seems legitimate, the build log is

conda.resolve.Unsatisfiable: The following specifications were found to be in conflict:
  - cmake
  - libnetcdf 4.4.* -> feature:vc10
  - libnetcdf 4.4.* -> feature:vc14
  - libnetcdf 4.4.* -> feature:vc9
Use "conda info <package>" to see the dependencies for each package.
Command exited with code 1

I do not know what I can do here. It may be related to #1678, but I need VS14 to build the code here.

EDIT: maybe I can just disable the appveyor/windows version for now?

@Luthaf
Copy link
Contributor Author

Luthaf commented Oct 7, 2016

Ping @jakirkham? I disabled the Windows version, it can be added later.

@jakirkham
Copy link
Member

So I'm in favor of merging.

Though I'd like to save us some time restarting CIs. Could we please put the non-meta recipes (looks like the first 2 commits in a separate PR)? Once done please ping me and I will merge ASAP.

Then we can cut this down the metapackage, which we can merge after the first two are built and deployed.

Sound good?

@Luthaf
Copy link
Contributor Author

Luthaf commented Oct 13, 2016

Sound good, I've separated the non-meta recipes in #1764

@jakirkham
Copy link
Member

Thanks. Have gone ahead and merged that. It might be a little bit as conversion to feedstocks occurs on Travis CI ATM and they are a bit backed up as we speak.

@jakirkham
Copy link
Member

So we ran into some issues with chemfiles-python. Have added PR ( conda-forge/chemfiles-python-feedstock#1 ) to fix them. Please take a look. Once that is sorted we should be in good shape to proceed with this.

@jakirkham
Copy link
Member

Toggling for CI.

@jakirkham jakirkham closed this Oct 16, 2016
@jakirkham jakirkham reopened this Oct 16, 2016
@Luthaf
Copy link
Contributor Author

Luthaf commented Oct 16, 2016

Should I rebase and only include the meta recipe here?

@jakirkham
Copy link
Member

That sounds like a good idea. Sure let's do that.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/chemfiles) and found it was in an excellent condition.

version: {{ version }}

build:
number: 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add skip: true # [win]. That should fix the CI failure. Also feel free to add [skip appveyor] in your commit message to avoid having to run that CI as it is not used.

@Luthaf
Copy link
Contributor Author

Luthaf commented Oct 18, 2016

Travis failure looks unrelated (502 gateway error from conda.anaconda.org), could you restart the build?

@jakirkham
Copy link
Member

Yep. Restarted.

@jakirkham jakirkham merged commit 30540f2 into conda-forge:master Oct 18, 2016
@jakirkham
Copy link
Member

Thanks @Luthaf .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants