Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
CEP: support for ABI3 packages #86
base: main
Are you sure you want to change the base?
CEP: support for ABI3 packages #86
Changes from 2 commits
d17a103
7c3e459
7e85388
b696d37
2b83f16
b4f5649
49b3cda
bd4f67c
f13370c
2ed5000
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
For reference, rattler follows the same behavior as micromamba.
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'm interested what your suggestion is for this mismatch in behavior, should conda be adapted to follow mamba and rattler in this case?
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.
My suggestion is to keep the existing mismatched behaviour in
package_metadata_version = 1
and make this uniform inpackage_metadata_version = 2
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.
Is it planned that the conda-forge tooling would check for this option and then set up Windows/Mac/Linux builds even though
noarch: python
is specified where it would otherwise only build on one arch?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 can do 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.
I think for V1 recipes we would use:
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.
Is
version_independent
better thanabi3
? It seems possibly misleading to me because a package can still set a minimum Python version and so be Python version-dependent.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.
abi3
packages withpackage_metadata_version = 2
should not be that different from any other package. It just has more things ininfo/link.json
.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 think something like
leaves open enough space to support future expansion of other abis that may require special behavior
for example
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.
@mariusvniekerk, both those options need the exact same thing. (B1-B4)
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.
Here's what a conda-build PR would look like conda/conda-build#5456
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.
Doesnt this imply that older python versions available on for instance conda-forge will never have the ability to use abi3 packages? If we are modifying the installer we could just as well move the Python files to the python specific site-packages directory.
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.
Sure, but we can also have the
site.py
in a separate conda package as well. I'm trying to reduce all these special things that a installer has to do. With the files inlib/python/site-packages
, the only thing that a installer has to do for an abi3 package is to compile the pyc files which is an optional thing anyway.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.
But relying on a special package might also be problematic if you are not using conda-forge though.
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'm hoping we can add support in defaults too.
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 would not prefer this because it means the proper functioning of the installer is dependent on a specific package. I'd rather encode the behavior in the installer itself.
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 agree that the installer should do less special case stuff. But I also don't think that it should depend on specific packages to function properly.
Even with this proposal the installer does already have special case behavior. However, without the dependency on a package that introduces a
site.py
file it will leave the package in a broken state. To me that adds complexity for the package maintainer.However, one could argue that the same holds for depending on
python
itself..It would be ideal if a recipe author doesn't have to care about these details.
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.
We could make the build tool add this package, then the installer does not have to care about it.
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.
Do we do this for other packages? Because then we tie the build-tool to specific package names which might also not be ideal.
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.
We take the C compiler name from
c_compiler
variable in the config. We could do the same.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.
Oh I forgot aboutrun_exports: noarch
. We can extend that in the python recipe.