Closed izahn closed 2 years ago
conda-forge-linter
commented
2 years ago 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.
I do have some suggestions for making it better though...
For recipe:
Documentation on acceptable licenses can be found here.
izahn
commented
2 years ago @conda-forge-admin please rerender
github-actions[bot]
commented
2 years ago Hi! This is the friendly automated conda-forge-webservice. I tried to rerender for you but ran into some issues, please ping conda-forge/core for further assistance. You can also try re-rendering locally.
beckermr
commented
2 years ago Do we need this patch on the cos7 sysroots as well?
izahn
commented
2 years ago Do we need this patch on the cos7 sysroots as well?
Yes, I think so. The glibc in Centos7 sill has libnsl, while the one distributed with Centos8 does not.
isuruf
commented
2 years ago We should delete the header files as well.
izahn
commented
2 years ago We should delete the header files as well.
That seems like a more delicate surgery, I'm a little scared. libnsl has
├── include
│ └── rpcsvc
│ ├── ypclnt.h
│ ├── yp.h
│ ├── yppasswd.h
│ ├── yppasswd.x
│ ├── yp_prot.h
│ ├── ypupd.h
│ └── yp.xshould we remove all of those?
beckermr
commented
2 years ago Please send a pr targeting the cos7 branch of this feedstock with the same changes to the files once we settle on them here.
izahn
commented
2 years ago Please send a pr targeting the cos7 branch of this feedstock with the same changes to the files once we settle on them here.
I think master is the cos7 branch, and v2.12 is the cos6 branch. Or do I not understand that correctly? Either way, I will prepare a PR for the other branch as well.
beckermr
commented
2 years ago Ah woops! Yeah that sounds more correct and thank you!
izahn
commented
2 years ago I've looked into this some more (e.g., in https://github.com/bminor/glibc/search?q=libnsl&type=commits) and I'm out of my depth on the header file issue. Help appreciated, or we can merge this as a first step and look into header files later if needed.
isuruf
commented
2 years ago should we remove all of those?
Yes, let's remove those
izahn
commented
2 years ago It looks like there are some issues with the recipe/conda_build_config.yaml that prevent re-rendering. Can someone help resolve those?
mbargull
commented
2 years ago @conda-forge-admin, please rerender
izahn
commented
2 years ago Thanks to @mbargull for catching my typo and helping cleanup the file removal code and to @isuruf for guidance. This PR now removes shared and static libnsl library files as well as the header files that upstream split out into the separate libnsl project. I believe this is in decently good shape. I'm going to wait and see if there is any more feedback here before submitting a PR to the v2.12 branch
isuruf
commented
2 years ago We should delete all references to libnsl.*, so yes.
Checklist
0(if the version changed)conda-smithy(Use the phrase code>@<space/conda-forge-admin, please rerender in a comment in this PR for automated rerendering)Older glibc versions included
libnslbut this was removed at some point. Some Conda-forge packages built with our older glibc will link tolibnsl, causing them to break on more recent Linux distributions. This has been reported and discussed in several places, includingWe have now included
libnslin conda-forge, and builtpythonandxerces-cagainst it. However, there may be other affected packages, or new packages may be added in the future that also run into this issue. This PR seeks to prevent that from happening by removinglibnslfrom this package, thereby preventing packages from automatically and mistakenly building against it.