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 libsecp256k1 & libsecp256k1-py-bindings & coincurve #23741

Merged
merged 99 commits into from
Jan 12, 2024

Conversation

MementoRC
Copy link
Contributor

@MementoRC MementoRC commented Aug 21, 2023

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@conda-forge-webservices
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/coincurve) and found it was in an excellent condition.

@MementoRC MementoRC marked this pull request as ready for review August 21, 2023 23:34
@MementoRC
Copy link
Contributor Author

@conda-forge/help-python-c, I think I need help, the windows build fails on python-3.8, I don't understand the dependency-links.txt error message. It worked yesterday on a previous submission, but I was to ashamed of the 300+ commits I went through to submit it. This is my first recipe and the learning curve is brutal for this old of a person.
Thanks for the help

@MementoRC
Copy link
Contributor Author

@carterbox @ofek I think we finally got it cleaned-up

Copy link
Member

@carterbox carterbox left a comment

Choose a reason for hiding this comment

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

All the builds seem to be linking correctly, have the correct dependency relationships, and contain the appropriate artifacts, so we're just waiting on a tagged release for the latest coincurve version.

recipes/libsecp256k1-py-bindings/meta.yaml Show resolved Hide resolved
recipes/coincurve/meta.yaml Outdated Show resolved Hide resolved
@ofek
Copy link
Contributor

ofek commented Jan 11, 2024

@MementoRC
Copy link
Contributor Author

@carterbox It should be ready for your review

@ofek
Copy link
Contributor

ofek commented Jan 11, 2024

Did something break?

@MementoRC
Copy link
Contributor Author

@ofek, well, yes, but it does not seem related to your release. The 3 are failing for different reasons too, I think it is a recipe issue, I got these before in other receipes, but I forgot how to resolve them :/

@carterbox
Copy link
Member

['  ERROR (libsecp256k1,lib/libsecp256k1.so): Needed DSO x86_64-conda-linux-gnu/sysroot/lib64/libc.so.6 not found in any CDT/compiler package, nor the whitelist?!']

This is not an error from your package. It is a problem with our compiler packages. Please update this branch to see if the error resolves.

@carterbox
Copy link
Member

@conda-forge/core I believe there is an error with the link checker in conda-build version 3.28.3. When building the libsecp256k1 recipe from this pull request, we get a missing dependency error as follows:

['  ERROR (libsecp256k1,lib/libsecp256k1.so): Needed DSO x86_64-conda-linux-gnu/sysroot/lib64/libc.so.6 not found in any CDT/compiler package, nor the whitelist?!']

It seems that the link checker cannot find libc.so.6 for the libsecp256k1 output even the compiler jinja template is included in the build requirements section for that output.

The same requirements are used for the libsecp256k1-2 output, but the link checker can find libc there.

I was able to recproduce the issue locally, but only with conda-build 3.28.3.

conda build -m .ci_support/linux64.yaml recipes/libsecp256k1 --error-overlinking

Earlier versions of conda-build including 3.26, 3.27 did not check the links of libsecp256k1.so (because it is a symlink?), and so do not raise the error. 3.28.2 checks the links but is able to find libc.

@carterbox
Copy link
Member

@MementoRC, using a different version of conda-build seems to allows the builds to succeed. Please allow me more time to now review the log output and discuss what to do about the broken conda-build version.

@MementoRC
Copy link
Contributor Author

@carterbox Cool. Note that when I install secp256k1 from source, it creates a so.2.1.2, Let me know how it goes. Thank you

Copy link
Member

@carterbox carterbox left a comment

Choose a reason for hiding this comment

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

Build logs LGTM.

Note that when I install secp256k1 from source, it creates a so.2.1.2, Let me know how it goes. Thank you

The build logs here clearly show that the so version is 2.1.1. Are you building locally with the release config? Maybe they automatically bump the patch number when they detect it's not a release build.

This version of conda-build was not able to correctly
locate libc for satisfying links in as multioutput
recipe.

conda/conda-build#5136
@carterbox
Copy link
Member

I am going to update my commit message that prevents conda-build 3.28.3 from being used, then merge this PR. You will probably need to patch the feedstock to prevent conda-build 3.28.3 from being used in .scripts/build_steps.sh

@carterbox carterbox merged commit b3a240d into conda-forge:main Jan 12, 2024
1 of 5 checks passed
@MementoRC
Copy link
Contributor Author

@carterbox Thank you. This was a long journey

@MementoRC MementoRC deleted the coincurve branch January 12, 2024 21:29
@jakirkham
Copy link
Member

It might be possible to pin conda-build using remote_ci_setup in conda-forge.yml

Note that conda-forge-ci-setup will need to be in that YAML list as well otherwise there will be build errors

Also note would re-render after making these changes

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

Successfully merging this pull request may close these issues.

4 participants