Add recipe for ckzg
Checklist
- [x] Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
- [x] License file is packaged (see here for an example).
- [x] Source is from official source.
- [x] 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).
- [x] If static libraries are linked in, the license of the static library is packaged.
- [x] Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
- [x] Build number is 0.
- [x] A tarball (
url) rather than a repo (e.g.git_url) is used in your recipe (see here for more details). - [x] GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
- [x] When in trouble, please check our knowledge base documentation before pinging a team.
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/ckzg) and found it was in an excellent condition.
Note that upstream has in the meantime added a patch for the Windows issue at https://github.com/ethereum/c-kzg-4844/pull/419. You can turn that into a patch here, or skip Windows until a new release with the patch comes out.
Note that upstream has in the meantime added a patch for the Windows issue at ethereum/c-kzg-4844#419. You can turn that into a patch here, or skip Windows until a new release with the patch comes out.
I saw this, because as you can see I was in that discussion with upstream, but there is no indication that that patch solves this particular windows issue. I had wanted to test it, but as I wrote had/have trouble getting a system able to test it.
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/ckzg) and found it was in an excellent condition.
I do have some suggestions for making it better though...
For recipes/ckzg:
- This recipe is using a compiler, which now requires adding a build dependence on
{{ stdlib("c") }}as well. For further details, please see https://github.com/conda-forge/conda-forge.github.io/issues/2102.
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/ckzg) and found it was in an excellent condition.
@conda-forge/core please review/approve with the original build script or tell me how I can force/replace the build script to use only clang. The way @xhochy suggested did not work, thus I reverted the commits to a working build.
what if you removed clang alltogether?
what if you removed clang alltogether?
I assume it would still clash with this Makefile https://github.com/ethereum/c-kzg-4844/blob/main/src/Makefile which has
# Cross-platform compilation settings.
ifeq ($(PLATFORM),Windows)
CC = gcc
CFLAGS += -D_CRT_SECURE_NO_WARNINGS
else
CC = clang
CFLAGS += -fPIC -Werror
endif
Windows also probably does not really use this file, as in setup.py there is
class CustomBuild(build_ext):
def run(self):
if system() == "Windows":
try:
check_call(["blst\\build.bat"])
except Exception:
pass
check_call(["make", "-C", "src", "blst"])
super().run()
I'm not saying these are great, but I assume I would have to patch them only use either clang or gcc. (and I do not know if there was a reason to only use either)
you'll have to patch setup.py to use something like
check_call(["make", "-C", "src", "blst", f"CC={os.environ['CC']}"])
This unfortunately fails on Win, which still uses cl.exe. Any ideas? (I also patched out the use of the windows bat file, as it didn't use either gcc or clang and I'm not sure what kind of magic it is doing. https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=961378&view=logs&jobId=240f1fee-52bc-5498-a14a-8361bde76ba0&j=240f1fee-52bc-5498-a14a-8361bde76ba0&t=7c0f8eae-6d6f-51bf-636f-73a1a7fb1bc4
Maybe disable Windows and then we can review/merge?
The Windows portion could be revisited in the feedstock
Maybe disable Windows and then we can review/merge?
The Windows portion could be revisited in the feedstock
Ok good idea. Windows build is skipped now.
This is ready for review, @conda-forge/help-python
Hey, @jakirkham without any patch, we are now again at the juncture of this Makefile I referred to here: https://github.com/conda-forge/staged-recipes/pull/26068#issuecomment-2181023138 This forces clang for linux and osx. On OSX that works, because it is installed by default. The main question is - how to force it to use only one compiler? In the pursuit of first merging, then fixing the feedstock, should I add clang back, so that only clang is used?
@step21 After a few days looking into it, I proposed this: https://github.com/conda-forge/staged-recipes/pull/27412, which should build Windows using gcc 5, separate blst as its own package (with the py-binding being worked in a separate recipe that still has a few snags on linux)
Note that it is still work in progress as I need to fine-tune windows/osx by going through CI.
Awesome, thanks for your work. Looks great from what I can tell. I don't know yet if I can take a look at the remaining build failures.
Cool. I am still working on it. I want to enforce linking to the versioned dynamic library and it's a bit trickier. It feels more reliable, but maybe overkill. As for windows, it's my Achilles heel, I had it working but must have changed something during the many refactoring. I'll ping you when I get it to pass
Similar functionality with separated blst/ckzg build to address the upstream package assumptions on compiler: https://github.com/conda-forge/staged-recipes/pull/27490
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/ckzg/meta.yaml) and found it was in an excellent condition.
I do have some suggestions for making it better though...
For recipes/ckzg/meta.yaml:
- No valid build backend found for Python recipe for package
ckzgusingpip. Python recipes usingpipneed to explicitly specify a build backend in thehostsection. If your recipe has built with onlypipin thehostsection in the past, you likely should addsetuptoolsto thehostsection of your recipe.
thanks for your comments. TBH I don't really care which PR we continue. Also added you as maintainer here. Though your has a lot more changes, I only patched out the build.bat so far, so I assume I would also have to carry those over.
builds! all pkgs are using clang. Only thing is that there is a deprecation warning for setup.py, but I would ignore it for now.
Looks good to me!
Thanks @jakirkham for the suggestions, should all be applied now.
Are we waiting for one more review now?
Right there were a bunch of folks looking at this. So was waiting to see if anyone else still had more thoughts
Are we still waiting?
thanks!