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

Include itk-elastix in tomviz packaging #19

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

psavery
Copy link
Contributor

@psavery psavery commented Jan 5, 2022

We want to include itk-elastix as a dependency to tomviz, but it is
not available on conda-forge.

However, it is available on pip, it is small, and it was designed to
work in conda environments (similar to ITK). Thus, we can pip install
it and include it in our tomviz package.

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

We want to include `itk-elastix` as a dependency to tomviz, but it is
not available on conda-forge.

However, it is available on pip, it is small, and it was designed to
work in conda environments (similar to ITK). Thus, we can `pip install`
it and include it in our tomviz package.

Signed-off-by: Patrick Avery <[email protected]>
@psavery psavery requested review from cjh1 and a team as code owners January 5, 2022 18:31
@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 (recipe) and found it was in an excellent condition.

@cjh1
Copy link
Contributor

cjh1 commented Jan 5, 2022

@conda-forge-admin, please rerender

conda-forge-webservices[bot] and others added 3 commits January 5, 2022 18:34
Signed-off-by: Patrick Avery <[email protected]>
This is to try to fix some errors, such as the following:

```
warning: libGLdispatch.so.0, needed by /usr/lib64/libGLX.so, not found
/usr/lib64/libGLX.so: undefined reference to `__glDispatchLoseCurrent'
```

Signed-off-by: Patrick Avery <[email protected]>
@cjh1
Copy link
Contributor

cjh1 commented Jan 5, 2022

@conda-forge-admin, please rerender

@mrakitin
Copy link
Member

mrakitin commented Jan 5, 2022

Thanks for the contribution, @psavery! I am not sure about conda-forge's policy on installing packages from PyPI, but we could ask. How hard would it be to package the needed code in conda-forge? Does it require any compilation?

@cjh1
Copy link
Contributor

cjh1 commented Jan 5, 2022

@mrakitin We are following a similar approach that is used in the itk conda forage feedstock, so figure it would be ok? We don't fancy takin on creating a conda package for itk-elastix. At the moment we are getting error unrelated to these changes. Any ideas?

@psavery
Copy link
Contributor Author

psavery commented Jan 5, 2022

Thanks for the contribution, @psavery! I am not sure about conda-forge's policy on installing packages from PyPI, but we could ask. How hard would it be to package the needed code in conda-forge? Does it require any compilation?

ITK compiles the binaries in their wheels in a way so that they are compatible in conda environments. As @cjh1 mentioned, the ITK feedstock actually just installs their pypi wheels in their feedstock. You can see that here.

@mrakitin
Copy link
Member

mrakitin commented Jan 6, 2022

OK, thanks for the details, @cjh1, @psavery, I haven't encountered such an approach in the conda packaging world, but that sounds reasonable to me as long as the packages are compatible.

@mrakitin
Copy link
Member

mrakitin commented Jan 6, 2022

At the moment we are getting error unrelated to these changes. Any ideas?

From the failed building log for Linux it looks like conda can't find the package:

-   - nothing provides requested libglvnd-devel-cos6-x86_64

Do you want https://anaconda.org/conda-forge/libglvnd-glx-cos7-x86_64 instead? I think it's related to the recent CF switch to the CentOS 7 (see conda-forge/conda-forge.github.io#1436).

@psavery
Copy link
Contributor Author

psavery commented Jan 6, 2022

At the moment we are getting error unrelated to these changes. Any ideas?

From the failed building log for Linux it looks like conda can't find the package:

-   - nothing provides requested libglvnd-devel-cos6-x86_64

Do you want https://anaconda.org/conda-forge/libglvnd-glx-cos7-x86_64 instead? I think it's related to the recent CF switch to the CentOS 7 (see conda-forge/conda-forge.github.io#1436).

Hi Maksim,

I think that sounds good to us! I don't think we are intentionally using CentOS 6. Could you help us use 7 instead?

We added libglvnd as a dependency because we were getting some errors we hadn't seen before like:

warning: libGLdispatch.so.0, needed by /usr/lib64/libGLX.so, not found
/usr/lib64/libGLX.so: undefined reference to `__glDispatchLoseCurrent'

We searched for libGLdispatch.so.0 and it looked like it came from libglvnd, so we tried adding that as a dependency. And then we started encountering the error that you see.

Thank you!
Patrick

@mrakitin
Copy link
Member

mrakitin commented Jan 6, 2022

Actually, the CentOS7 is already used here after rerendering, see https://github.com/conda-forge/tomviz-feedstock/pull/19/files#diff-6608a67cc3923d5132134d7371ec0fb28a30decb656682b28c86c49acab8371bR16. You may try to roll back to CentOS6 (the previous value in that diff, done via the recipe/conda_build_config.yaml file).

Otherwise, I suspect the metapackage {{ cdt('libglvnd-devel') }} may be broken for CentOS7. Cc-ing @conda-forge/core to see if it's a known problem. Thanks!

Copy link
Member

@beckermr beckermr left a comment

Choose a reason for hiding this comment

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

This package has to be submitted to staged-recipes. It cannot be installed via pip in this PR.

@beckermr
Copy link
Member

beckermr commented Jan 6, 2022

We are following a similar approach that is used in the itk conda forage feedstock, so figure it would be ok? We don't fancy takin on creating a conda package for itk-elastix. At the moment we are getting error unrelated to these changes. Any ideas?

This feedstock needs to be fixed as well.

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

Successfully merging this pull request may close these issues.

5 participants