Skip to content

Conversation

@Wyverald
Copy link
Member

@Wyverald Wyverald commented Jan 9, 2026

The current invalidation logic didn't take --incompatible_repo_env_ignores_action_env and --experimental_strict_repo_env into account, which resulted in incorrect invalidation of repo rules and module extensions.

This is addressed by a larger refactoring that consolidates the logic that computes the effective environment for repository rules and module extensions in CommandEnvironment. This environment is then read and sliced by a new RepoEnvironmentFunction that operates analogously to ClientEnvironmentFunction.

Along the way, the variety of terminology in CommandEnvironment and various SkyFunctions is cleaned up to consistently use:

  • repoEnv to refer to the environment seen by repo rules and module extensions, which may or may not see the full client env based on the value of --experimental_strict_repo_env and
  • nonstrictRepoEnv, which refers to same environment with, conceptually, --experimental_strict_repo_env forced to false. This is used for certain non-hermetic operations such as downloader and credential helper logic.

Closes #28168.

PiperOrigin-RevId: 854228202
Change-Id: I900f260dd5d0c6e20dcc32eaee0821567e60d5d1

@Wyverald Wyverald requested a review from a team as a code owner January 9, 2026 17:49
@github-actions github-actions bot added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. awaiting-review PR is awaiting review from an assigned reviewer labels Jan 9, 2026
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request is a significant and valuable refactoring of repository environment handling. It successfully consolidates the logic for computing the effective environment for repository rules and module extensions into CommandEnvironment, which greatly improves clarity and maintainability. The introduction of RepoEnvironmentFunction and the consistent use of repoEnv and nonstrictRepoEnv terminology are excellent changes. The replacement of ClientEnvironmentValue with the more general EnvironmentVariableValue record is also a good modernization. The changes appear to fix the described invalidation bugs correctly. Overall, this is a high-quality change that improves the codebase. I have no specific comments as the implementation looks solid.

The current invalidation logic didn't take `--incompatible_repo_env_ignores_action_env` and `--experimental_strict_repo_env` into account, which resulted in incorrect invalidation of repo rules and module extensions.

This is addressed by a larger refactoring that consolidates the logic that computes the effective environment for repository rules and module extensions in `CommandEnvironment`. This environment is then read and sliced by a new `RepoEnvironmentFunction` that operates analogously to `ClientEnvironmentFunction`.

Along the way, the variety of terminology in `CommandEnvironment` and various `SkyFunction`s is cleaned up to consistently use:
* `repoEnv` to refer to the environment seen by repo rules and module extensions, which may or may not see the full client env based on the value of `--experimental_strict_repo_env` and
* `nonstrictRepoEnv`, which refers to same environment with, conceptually, `--experimental_strict_repo_env` forced to `false`. This is used for certain non-hermetic operations such as downloader and credential helper logic.

Closes #28168.

PiperOrigin-RevId: 854228202
Change-Id: I900f260dd5d0c6e20dcc32eaee0821567e60d5d1
@iancha1992 iancha1992 added this pull request to the merge queue Jan 9, 2026
Merged via the queue into release-9.0.0 with commit 535586a Jan 9, 2026
48 checks passed
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Jan 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants