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

RFE: Allow to --set-installer for custom INSTALLER value #8156

Open
hroncok opened this issue Apr 28, 2020 · 9 comments
Open

RFE: Allow to --set-installer for custom INSTALLER value #8156

hroncok opened this issue Apr 28, 2020 · 9 comments
Labels
type: feature request Request for a new feature

Comments

@hroncok
Copy link
Contributor

hroncok commented Apr 28, 2020

What's the problem this feature will solve?

In Fedora, we use pip to install wheels during the build of RPM packages.
We then replace pip with rpm in the INSTALLER file to indicate: This package has been actually installed trough rpm on the user systems (despite it being installed by pip in the build system).

The hash of the INSTALLER file is recorded in RECORD. When we change the value, we should also recalculate the hash and adjust RECORD.

This is getting fragile. I'd rather tell pip to put arbitrary value to INSTALLER than sed it afterwards.

Describe the solution you'd like

We'd like to have --set-installer option (or --installer etc.) for pip install that we would use like this:

pip install --set-installer=rpm ...

It would put the value (rpm in the example) to INSTALLER instead of pip and would put the correct hash to RECORD.

Alternative Solutions

The alternate solution is what we use now + sed-ing the hash:

pip install ...
sed -i s/pip/rpm/ ...dist-info/INSTALLER
sed -iE 's|(\.dist-info/INSTALLER,sha256=)[^,]+,|\1'$(sha256sum ...dist-info/INSTALLER | cut -f1 -d" ")',|' ...dist-info/RECORD

Additional context

https://discuss.python.org/t/pep-610-usage-guidelines-for-linux-distributions/4012/17

I'm happy to submit a PR for this if agreed upon.

@triage-new-issues triage-new-issues bot added the S: needs triage Issues/PRs that need to be triaged label Apr 28, 2020
@dstufft
Copy link
Member

dstufft commented Apr 28, 2020

I wonder if a better overall solution would be to write a simple, but lower level wheel installer.

My thinking here is basically instead of trying to shoe horn pip into this, it might be better to just make something lower level designed to be part of a build process like this. It could be laser focused on just installing wheels, so no need for network code or resolvers. If it was done as a python api with a cli interface to that api, then pip and fedora could both use it.

I dunno. My main concern here is adding a proliferation of options and then still ending up with a subpar workflow for fedoras use case. Particularly since this is not our primary use case, there is likely a fair amount of risk of new features regularly needing to be worked around ala direct url support.

Beyond that, I dont personally have a problem with this.

@pradyunsg pradyunsg added the type: feature request Request for a new feature label Apr 28, 2020
@triage-new-issues triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label Apr 28, 2020
@pradyunsg
Copy link
Member

pradyunsg commented Apr 28, 2020

I wonder if a better overall solution would be to write a simple, but lower level wheel installer.

That's what I imagine https://github.com/pradyunsg/installer would become eventually.

See https://github.com/pradyunsg/installer/issues/1 for context.

@hroncok
Copy link
Contributor Author

hroncok commented Apr 28, 2020

I dunno. My main concern here is adding a proliferation of options and then still ending up with a subpar workflow for fedoras use case. Particularly since this is not our primary use case, there is likely a fair amount of risk of new features regularly needing to be worked around ala direct url support.

I get that and I think you are right.

💯 to @pradyunsg for the installer project :)

@EpicWink
Copy link
Contributor

What's the purpose of INSTALLER? If it's to identify what constructed the current package layout, then should it stay as pip? If it's to let other programs know who's in control of managing the package, then it would become rpm

Interestingly, PEP 376 recommended to have an --installer option in distutils for exactly this reason

@hroncok
Copy link
Contributor Author

hroncok commented Apr 28, 2020

We use it to let other programs know who's in control of managing the package. Especially for things like #5346 (all other uses are strictly hypothetical).

@dstufft
Copy link
Member

dstufft commented Apr 29, 2020

I get that and I think you are right.

Do you think we should close this, or do you still think you would like this feature in the absence of the installer project being functional?

@hroncok
Copy link
Contributor Author

hroncok commented Apr 29, 2020

I'd still like this feature, but I have realized our RECORD file is a mess anyway, so I'll dig deeper into what needs to be done on our side to prevent that.

  • packagers tent to remove files or add them for compatibility reasons
  • we automatically mangle some shebangs
  • we ship different levels of bytecode cache
  • possibly more

I plan to look closer and we might very well just need to recalculate all hashes, drop all hashes or make RECORD optional for packages managed by external tools. When we go that way, setting installer via sed will be sufficient for us.

Can we keep this open for couple weeks and I'll get back with more info?

@dstufft
Copy link
Member

dstufft commented Apr 29, 2020

Sure

@hroncok
Copy link
Contributor Author

hroncok commented May 7, 2020

I plan to look closer and we might very well just need to recalculate all hashes, drop all hashes or make RECORD optional for packages managed by external tools.

https://discuss.python.org/t/pep-376-vs-rpm-packaging-is-record-necessary/4126

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request Request for a new feature
Projects
None yet
Development

No branches or pull requests

4 participants