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

Prevent --repo_env from triggering unnecessary fetches #13126

Closed

Conversation

philsc
Copy link
Contributor

@philsc philsc commented Feb 28, 2021

It looks like --repo_env entries get added to every repository
rule's marker data. That is regardless of whether the repository rule
declared that it uses the environment variable. This causes
unnecessary fetches of the external repositories.

This patch aims to fix that by making repository rules only depend on
--repo_env values that they specify in their environ attributes.

I also took this opportunity to fix up all instances of bazel-genfiles
in starlark_repository_test.sh to bazel-bin.

@philsc philsc requested a review from lberki as a code owner February 28, 2021 04:24
@google-cla google-cla bot added the cla: yes label Feb 28, 2021
@lberki lberki requested review from philwo and removed request for lberki March 1, 2021 07:29
@jin jin added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website labels Mar 1, 2021
@philsc
Copy link
Contributor Author

philsc commented Mar 3, 2021

@philwo , any thoughts on this?

Copy link
Member

@Wyverald Wyverald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll import it now.

It looks like `--repo_env` entries get added to every repository
rule's marker data. That is regardless of whether the repository rule
declared that it uses the environment variable. This causes
unnecessary fetches of the external repositories.

This patch aims to fix that by making repository rules only depend on
`--repo_env` values that they specify in their `environ` attributes.
@philsc philsc force-pushed the unreviewed/repo_env_fix-publish branch from 55bed66 to 7f23478 Compare March 9, 2021 15:30
@bazel-io bazel-io closed this in 321fe3b Mar 9, 2021
@philsc philsc deleted the unreviewed/repo_env_fix-publish branch March 9, 2021 18:28
philwo pushed a commit that referenced this pull request Mar 15, 2021
It looks like `--repo_env` entries get added to every repository
rule's marker data. That is regardless of whether the repository rule
declared that it uses the environment variable. This causes
unnecessary fetches of the external repositories.

This patch aims to fix that by making repository rules only depend on
`--repo_env` values that they specify in their `environ` attributes.

I also took this opportunity to fix up all instances of `bazel-genfiles`
in `starlark_repository_test.sh` to `bazel-bin`.

Closes #13126.

PiperOrigin-RevId: 361813865
philwo pushed a commit that referenced this pull request Mar 15, 2021
It looks like `--repo_env` entries get added to every repository
rule's marker data. That is regardless of whether the repository rule
declared that it uses the environment variable. This causes
unnecessary fetches of the external repositories.

This patch aims to fix that by making repository rules only depend on
`--repo_env` values that they specify in their `environ` attributes.

I also took this opportunity to fix up all instances of `bazel-genfiles`
in `starlark_repository_test.sh` to `bazel-bin`.

Closes #13126.

PiperOrigin-RevId: 361813865
@philwo philwo removed the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants