Skip to content

Allow passing env vars to processes without them entering the cache key #23645

Description

@jasonwbarnett

Is your feature request related to a problem? Please describe.

Environment variable values are part of a process's cache key, which is correct by default — an env var can change what a process produces. But some values unavoidably vary per run and provably cannot affect a process's output, and today there is no way to pass one to a process without destroying its cache entry.

The case that pushed us here is test result reporting. Buildkite's buildkite-test-collector is a pytest plugin that uploads results to Buildkite Test Engine. It auto-loads via a [pytest11] entry point and reads os.environ directly for its upload token and for build attribution:

  • BUILDKITE_ANALYTICS_TOKEN — gates the upload
  • BUILDKITE_BUILD_ID — gates CI detection; without it the collector falls back to a uuid4() run key
  • BUILDKITE_BUILD_URL, BUILDKITE_BRANCH, BUILDKITE_COMMIT, BUILDKITE_BUILD_NUMBER, BUILDKITE_JOB_ID, BUILDKITE_MESSAGE — attribution fields

The only way to get those into the sandbox is [test].extra_env_vars. Doing so puts roughly 25 per-job values into every test process's key — job id, build id, commit, agent id, instance id, step id. Every test process in CI then computes a key no other job can reproduce, so pants test never hits the local or remote cache across builds. With remote_cache_write = true, each run also writes results under keys that can never be read back, so the cache fills with entries that are dead on arrival.

The values themselves cannot change whether a test passes. They label a report. But Pants has no way to express that.

This generalises past our case: a build URL injected for a coverage uploader, a correlation id read by a reporting plugin, anything a tool wants for provenance rather than behaviour. The current answer is to choose between the tool working and the cache working.

Concretely, in process_execution/src/lib.rs, make_execute_request writes every req.env name/value into the remexec Command proto, which is hashed into command_digestaction_digest → the CacheKey used by both process_execution/src/cache.rs and the remote cache. One changed byte is a guaranteed miss.

Describe the solution you'd like

A Process field for environment variables that are excluded from the Command proto and merged into the environment after the cache has been consulted — plus a test-goal surface for it, since that is where the need is sharpest.

There is already precedent for exactly this mechanism in the codebase. bounded::CommandRunner inserts execution_slot_variable into process.env at run time, and context.rs builds the runner stack so bounded::CommandRunner is wrapped by both remote_cache::CommandRunner and cache::CommandRunner. So that insertion already happens below the key. The code even flags it:

// TODO: Both of these templating cases should be implemented at the lowest possible level:
// they might currently be applied above a cache.

So the shape is established; what is missing is a way for callers to opt into it.

Two limitations are inherent rather than implementation gaps, and I think they belong in the docs rather than being designed around:

  1. On a cache hit the process does not run, so these values are not observed at all. This cannot be used to make something happen on every run. For the reporting case that is a real behaviour change: a cached test contributes no upload. Arguably that is the more honest signal — the test did not run — but it is a change, not a no-op.
  2. Under remote execution the Command proto is the only channel to the worker, so a value excluded from it cannot be delivered. It has to be dropped, with a warning.

The obvious risk is misuse: someone puts a behaviour-affecting variable in the new field and gets silently wrong cached results. I think that is acceptable if the option help and the field docs are blunt about the contract, in the same spirit as --no-process-cleanup or cache_scope. Happy to be talked out of it, or into a narrower form.

Describe alternatives you've considered

  • Collect out of band instead. Set [pytest].junit_xml_dir so the XML lands in the process's output_files, which are restored on a cache hit, then upload after the Pants run where per-job values touch no key. This is strictly better for the reporting case and is what we are also pursuing. It does not help the general case, because it only works for tools whose input is an artifact Pants already captures — it does nothing for a plugin that wants a correlation id at run time.
  • append_only_caches as a side channel. Write the token into a named cache and have a conftest read it. It keeps the value out of the key, but it is indirection through the filesystem for something that is conceptually an env var, and it still cannot be observed on a cache hit.
  • ProcessCacheScope.PER_SESSION for affected tests. Correct but backwards: it fixes the key-churn by disabling caching outright, which is the thing we are trying to recover.
  • Scope the variables per-target. What we do today, and worth doing regardless, but it only shrinks the blast radius. Any target that genuinely needs a per-run value is still uncacheable.

Additional context

I have an implementation working locally and will open a PR against this issue shortly, so there is something concrete to react to. Sketch:

  • Process.uncached_env (Python FrozenDict, Rust BTreeMap), marked #[derivative(PartialEq = "ignore", Hash = "ignore")] so it does not split graph nodes either.
  • Merged into env in bounded::CommandRunner next to the existing slot-variable injection. env wins a collision, since an explicitly cache-keyed value is the more specific request.
  • Excluded from the Command proto for free, since it is a separate field from env.
  • A warning in remote::CommandRunner::run when it is non-empty, because those values cannot be delivered.
  • [test].uncached_env_vars plus an uncached_env_vars field on test targets, threaded through PexProcess/VenvPexProcess and pytest_runner.

Tests cover the two invariants that matter: that Process identity and hash ignore the field while the same values in env still separate them, and that adding or changing an uncached_env value leaves both the command and action digests untouched while the same values in env change them.

Happy to adjust naming (uncached_env vs something like cache_transparent_env), or to gate it behind an allowlist if you would rather the footgun be harder to reach.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions