search

cloudfoundry / docs-buildpacks

http://docs.cloudfoundry.org/buildpacks/
Apache License 2.0
21 stars 118 forks source link

Fix pip vendor command to match the explanation #289

Closed vanschelven closed 1 year ago

vanschelven commented 2 years ago

The explanation is

To ensure proper installation of dependencies, Cloud Foundry recommends non-binary vendored dependencies. However, as per the documentation of pip

Do not use binary packages. [..] Accepts either “:all:” to disable all binary packages, “:none:” to empty the set (notice the colons)

i.e. to disable binary packages (wheels) and get non-binary (source) packages the command must be adapted with :all:. (This assumes the explanation is correct and the code was wrong)

cf-gitbot commented 2 years ago

We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.

The labels on this github issue will be updated when the story is started.

vanschelven commented 2 years ago

On second thought, this is likely a case of "code correct, comment incorrect". If the reviewers agree, they should close this PR and merge #290 instead.

vanschelven commented 2 years ago

The most recent change in the vicinity was cbf6cdca which indeed indicates that at the time it was believed :none: was more correct (but did not update the documentation). I cannot see the underlying issue (presumably in pivotal-tracker) so I'll let @cshollingsworth weigh in on the matter.

anita-flegg commented 1 year ago

@vanschelven , if I understand your comments, this commit should not be merged. Please clarify or close this PR. Thanks.

vanschelven commented 1 year ago

The other PR was merged and @cshollingsworth never weighed in on the matter so I'm presuming #290 was indeed the correct solution

anita-flegg commented 1 year ago

@cshollingsworth was away for an extended time, so I don't think you can read anything into her silence.