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

bdist_wheel should support custom platform tags #144

Closed
agronholm opened this issue Jun 10, 2015 · 6 comments
Closed

bdist_wheel should support custom platform tags #144

agronholm opened this issue Jun 10, 2015 · 6 comments

Comments

@agronholm
Copy link
Contributor

Originally reported by: Timothy Allen (Bitbucket: OptiverTimAll, GitHub: OptiverTimAll)


Now that Pip automatically caches built wheels, and since the cache may be in a shared location, it would be useful to allow people to invent custom platform-tags so that wheels compiled for different scenarios can be kept apart.

This has been discussed in pip's issue tracker and in a distutils-sig thread, with positive reception in both. Some of the concrete use-cases listed include:

  • Users may want pip/wheel to distinguish wheels built on platforms where pip cannot reliably determine a platform tag on its own, like linux_x86_64
  • Users may want pip/wheel to distinguish wheels built with security-hardening flags, so non-hardened wheels are not deployed into production
  • Users may want pip/wheel to distinguish wheels built with compiler-flags that enable the use of CPU instructions that are not yet widely available.

bdist_wheel currently supports a --plat-name option, but it has some unfortunate behavour: so far as I can tell, if you give it a custom platform name and build a pure-Python package, the option is silently ignored. On the other hand, if you build a package that includes a C module, the process dies with an assertion error.

Perhaps --plat-name and its behaviour could be deprecated, and a new --platform-tag option (named for consistency with --python-tag and the terminology of PEP0425) that accepts any tag that meets the character set requirements (alphanumerics and underscores, if I read the PEP correctly).


@agronholm
Copy link
Contributor Author

Original comment by Andrés Díaz (Bitbucket: ajdiaz, GitHub: ajdiaz):


I created the PR #60 to allow --plat-tag and also "force" platform tag even when pure python packages.

Feedback is welcome!

@agronholm
Copy link
Contributor Author

Original comment by Daniel Holth (Bitbucket: dholth, GitHub: dholth):


I like the override feature.

I notice you changed the command line argument name to tag, but changed
a lot of tag variables to name in the source.

It is probably not OK to remove the old command line argument.

@agronholm
Copy link
Contributor Author

Original comment by Andrés Díaz (Bitbucket: ajdiaz, GitHub: ajdiaz):


Yep, I agree. I changed the argument name because the Timothy's comment in the issue recommended to change the argument name to be more close to the PEP notation and also because plat_name attribute is also used by distutil.cmd so to keep the variable will require more work. But in general, I agree that we should keep the original argumet name.

I could try to fix that and update the PR if there are more comments about that.

Thanks for the feedback!!

@agronholm
Copy link
Contributor Author

Original comment by Nate Coraor (Bitbucket: natefoo, GitHub: natefoo):


Thanks @ajdiaz, I merged your pull request #60 and have some additional changes and tests in e3c7215947c0

@agronholm
Copy link
Contributor Author

Original comment by Nate Coraor (Bitbucket: natefoo, GitHub: natefoo):


I found a bug in my changes that broke pure python wheels (they were no longer getting the any tag). I've fixed this in development and wrote a test to catch it in the future, but as a result, I've changed my mind on the use of --plat-tag deprecating --plat-name. --plat-name is a standard option through most of the commands and wheel uses set_undefined_options() to have bdist set it. I think we need to follow suit here.

@agronholm
Copy link
Contributor Author

Original comment by Nate Coraor (Bitbucket: natefoo, GitHub: natefoo):


Changed as per my comment in e3747d1e06d2.

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

No branches or pull requests

1 participant