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

Keep python pinning in hashing if there's a space #3895

Merged
merged 2 commits into from
Mar 5, 2020

Conversation

isuruf
Copy link
Contributor

@isuruf isuruf commented Mar 4, 2020

No description provided.

@cla-bot cla-bot bot added the cla-signed [bot] added once the contributor has signed the CLA label Mar 4, 2020
@mingwandroid
Copy link
Contributor

@msarahan, could you give this a review? It's part of the multiple python interpreter support drive.

@msarahan
Copy link
Contributor

msarahan commented Mar 4, 2020

It seems cleaner to me to just include all python pinning in the hashing, rather than restricting it only to cases with a space. We didn't do that because of the legacy standard of putting python in the build string directly. Python is also so much more commonly a point of interest that keeping it in the build string kept it easy for people to visually inspect packages. With .conda files, pulling out the hash contents should now be fast enough that it might be feasible to get rid of python and the other special-cased junk in the build string and have only the hash.

That's definitely not a conversation I'd wade into recklessly, though. People will feel strongly about it.

If you keep only this change, how does this handle the rest of the build string? What do you envision a final package filename looking like?

@isuruf isuruf changed the title Keep python pinning in hashing if there's a space [WIP] Keep python pinning in hashing if there's a space Mar 4, 2020
@isuruf
Copy link
Contributor Author

isuruf commented Mar 4, 2020

For pypy packages it will have py36 in the build string, but there'll also be a hash.

I wanted this to be backward compatible so that people relying on --skip-existing will not get affected.

@msarahan
Copy link
Contributor

msarahan commented Mar 4, 2020

seems reasonable to me. I'd say give it a try.

@isuruf
Copy link
Contributor Author

isuruf commented Mar 4, 2020

I've marked this WIP, till I try it out. I'll let you know if this works.

@isuruf isuruf changed the title [WIP] Keep python pinning in hashing if there's a space Keep python pinning in hashing if there's a space Mar 5, 2020
@isuruf
Copy link
Contributor Author

isuruf commented Mar 5, 2020

This works as expected. I've removed WIP

@mingwandroid mingwandroid merged commit 9a9cc78 into conda:master Mar 5, 2020
@mingwandroid
Copy link
Contributor

Thank you!

@isuruf isuruf deleted the python branch March 5, 2020 15:33
@isuruf
Copy link
Contributor Author

isuruf commented Mar 5, 2020

I don't suppose there's a new release that's going to happen soon, right? 😃

isuruf added a commit to isuruf/conda-build-feedstock that referenced this pull request Mar 5, 2020
@mingwandroid
Copy link
Contributor

Yes. ASAP. But probably not today. I must make patchelf the default patcher first.

ocefpaf added a commit to conda-forge/conda-build-feedstock that referenced this pull request Mar 6, 2020
@github-actions
Copy link

Hi there, thank you for your contribution!

This pull request has been automatically locked because it has not had recent activity after being closed.

Please open a new issue or pull request if needed.

Thanks!

@github-actions github-actions bot added the locked [bot] locked due to inactivity label Mar 11, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants