Skip to content

Allow for requirements files to differ per platform - #531

Merged
mattem merged 2 commits into
bazel-contrib:mainfrom
alexeagle:platform_requirements
Apr 22, 2022
Merged

Allow for requirements files to differ per platform#531
mattem merged 2 commits into
bazel-contrib:mainfrom
alexeagle:platform_requirements

Conversation

@alexeagle

@alexeaglealexeagle commented Sep 8, 2021

Copy link
Copy Markdown
Contributor

As a common example, we need a compiled requirements file for linux that differs from mac os

PR Checklist

Please check if your PR fulfills the following requirements:

  • Does not include precompiled binaries, eg. .par files. See CONTRIBUTING.md for info
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@alexeagle
alexeagleforce-pushed the platform_requirements branch 2 times, most recently from 82ebb17 to 956e1bfCompareSeptember 8, 2021 19:20
@UebelAndreUebelAndre mentioned this pull request Oct 22, 2021
Closed
@alexeagle
alexeagleforce-pushed the platform_requirements branch from 956e1bf to be0f661CompareJanuary 5, 2022 15:56
@alexeagle
alexeagleforce-pushed the platform_requirements branch 2 times, most recently from 232a6f3 to 8bf2bf4CompareApril 22, 2022 12:33
@alexeagle
alexeagle marked this pull request as ready for review April 22, 2022 12:36
@alexeagle
alexeagle requested review from hrfuller and removed request for brandjon, lberki and thundergolferApril 22, 2022 12:36
Alex Eagle@alexeagle
Allow for requirements files to differ per platform
f480ebe
As a common example, we need a compiled requirements file for linux that differs from mac os
@alexeagle
alexeagleforce-pushed the platform_requirements branch 3 times, most recently from 539cd04 to f6c0e46CompareApril 22, 2022 13:02
@alexeagle
alexeagle requested a review from mattemApril 22, 2022 13:10
@alexeagle
Add pip_repository documentation
0c811e0
The macros are leaky and otherwise you have to read sources to find out about the kwargs
Fixesbazel-contrib#384
@alexeagle
alexeagleforce-pushed the platform_requirements branch from f6c0e46 to 0c811e0CompareApril 22, 2022 13:11
@alexeaglealexeagle mentioned this pull request Apr 22, 2022
Closed
12 tasks
@mattem
mattem merged commit ae7a267 into bazel-contrib:mainApr 22, 2022
@alexeagle
alexeagle deleted the platform_requirements branch April 22, 2022 14:10
phst added a commit to phst/rules_elisp that referenced this pull request Apr 22, 2022
@phst
Use per-platform pip requirements.
52cda39
These are now natively supported by rules_python,
cf. bazel-contrib/rules_python#531. This means we can now
remove our hand-rolled support for per-platform requirements.
@phstphst mentioned this pull request Apr 22, 2022
Merged
@UebelAndre

UebelAndre commented Apr 22, 2022

Copy link
Copy Markdown
Contributor

I think this is slightly incorrect. Instead of selecting the requirements based on the host, a dependency graph should be created for each target platform, so if I build on a macos host and target a linux platform (maybe for a docker container), I get the right set of requirements. Right now it'd be wrong unless my dependency graph happens to be the same.

phst added a commit to phst/rules_elisp that referenced this pull request Apr 22, 2022
@phst
Use per-platform pip requirements.
8a2a7c4
These are now natively supported by rules_python,
cf. bazel-contrib/rules_python#531. This means we can now
remove our hand-rolled support for per-platform requirements.
@alexeagle

alexeagle commented Apr 23, 2022

Copy link
Copy Markdown
ContributorAuthor

@UebelAndre the actual install has to happen on the target platform so in a docker container you'd copy in the Linux requirements and use pip_install. There's no Mac outputs of pip_parse that can be useful there.

@alexeagle

alexeagle commented Aug 16, 2022

Copy link
Copy Markdown
ContributorAuthor

Note to self: this PR only updated the pip_parse/incremental=True codepath to use the platform-specific lockfile. pip_install usage still ends up with the unlocked requirements, and isn't platform-aware.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hrfullerhrfullerAwaiting requested review from hrfuller
1 more reviewer
@mattemmattemmattem approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alexeagle@UebelAndre@mattem