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
Added qh3 as a package #25193
base: main
Are you sure you want to change the base?
Added qh3 as a package #25193
Conversation
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 ( |
@conda-forge/help-c-cpp @help-python-c It seems the ls-qpack is included in vendor directory in the source but somehow it's still complaining it can't find the header file? Tried adding the conda pylsqpack to no avail... |
recipes/qh3/meta.yaml
Outdated
- {{ compiler('c') }} | ||
|
||
# Adding this here as otherwise it relies on vendor lsqpack | ||
- pylsqpack |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be in host
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let me try that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The compiler needs to stay in build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it I'll move it back
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to https://conda-metadata-app.streamlit.app/?q=conda-forge%2Flinux-64%2Fpylsqpack-0.3.18-py39hf860d4a_0.conda the package doesn't contain the file in question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xhochy does this mean the C library would need to be brought onto conda first?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we will need that first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xhochy, I've never worked on a pure C library for Conda before, is here a good example recipe you think I can reference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends on what the build tool is. Is it using autotools?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked upstream, seems like CMake?
@xhochy no dice still
|
Hi @xhochy I'm pivoting back to this now, it's asking for |
recipes/qh3/conda_build_config.yaml
Outdated
@@ -0,0 +1,2 @@ | |||
c_stdlib_version: # [osx and x86] | |||
- '10.12' # [osx and x86] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- '10.12' # [osx and x86] | |
- '10.13' # [osx and x86] |
10.13 is now the new default/minimum
recipes/qh3/meta.yaml
Outdated
- {{ compiler('c') }} | ||
- {{ compiler('rust') }} | ||
- cmake | ||
- libclang # [linux] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- libclang # [linux] |
This is something on PyPI only to reduce the size of binary artifacts (mostly?). In all recipes I have seen it, I just patched it out as a requirement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try Clang instead then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or maybe I need to use Clang as the C compiler and not gcc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xhochy It seems that it's during the rust/cargo build phase it needs libclang, but not sure why it's not detecting it
@xhochy New OSX errors
Also linux build still not working, same issue, can't find libclang. I might try switching from gcc to clang |
host: | ||
- python | ||
- maturin >=1.2,<2.0 | ||
- pip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- pip | |
- pip | |
- clangdev |
This is probably what you need
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it, let me try
I think you need openssl in host and set OPENSSL_DIR=$PREFIX |
@xhochy maybe it's confused now because there is both gcc and clang? Or is it asking for a c++ compiler
|
Yes, it needs a C++ compiler |
@xhochy it still has trouble finding a compiler, I wonder if I need to specify or change some env variables for Clang?
|
No. This should be buildable with gcc. Switching to clang might even bring more hassle in this case. |
It really is this |
I read a bit through its code. It seems like adding cmake to build and setting AWS_LC_SYS_CMAKE_BUILDER=1 may lead to better errors. |
@xhochy I already have cmake in build but let me try that env var |
You need make in build |
recipes/qh3/meta.yaml
Outdated
- {{ compiler('c') }} | ||
- {{ compiler('cxx') }} | ||
- {{ compiler('rust') }}\ | ||
- clangdev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should only be in host
recipes/qh3/meta.yaml
Outdated
- {{ stdlib('c') }} | ||
- {{ compiler('c') }} | ||
- {{ compiler('cxx') }} | ||
- {{ compiler('rust') }}\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- {{ compiler('rust') }}\ | |
- {{ compiler('rust') }} |
@xhochy Thanks so much! Linux build is fixed. Now trying to figure out the OSX build, I found this possible related situation but not sure if it is addressable here: openssl/openssl#16458 |
Checklist
url
) rather than a repo (e.g.git_url
) is used in your recipe (see here for more details).