Skip to content

boring-sys: clear repository-local git env vars in build script - #540

Open
RaphaelFakhri wants to merge 1 commit into
cloudflare:masterfrom
RaphaelFakhri:fix-git-env-vars
Open

RaphaelFakhri wants to merge 1 commit into
cloudflare:masterfrom
RaphaelFakhri:fix-git-env-vars

Conversation

@RaphaelFakhri

Copy link
Copy Markdown

Fixes #527.

When cargo build runs from a git hook, git has already exported GIT_DIR and related variables. The build script inherits them, so git init and git apply in OUT_DIR run against the caller's repository instead of the copied BoringSSL sources. When the hook runs in a worktree, git init writes core.bare = true into the main checkout's config, and git commands in the main checkout fail with fatal: this operation must be run in a work tree.

Changes

  • Add a git_command() helper in boring-sys/build/main.rs that removes the variables listed by git rev-parse --local-env-vars.
  • Use it for all three git invocations (submodule update, init, apply).

Testing

The build script has no test harness, so this was checked with a script that simulates a hook in a consumer worktree:

git init consumer/main && git -C consumer/main commit --allow-empty -m init
git -C consumer/main worktree add ../wt
GIT_DIR=$(git -C consumer/wt rev-parse --absolute-git-dir) cargo build -p boring-sys
git -C consumer/main config --get core.bare
git -C consumer/main status
  • Before: core.bare is true and git status in the main checkout fails.
  • After: core.bare is false, git status works, and the patches are applied in OUT_DIR/boringssl, which now gets its own .git.

cargo fmt --all -- --check, cargo clippy -p boring-sys --all-targets and cargo test -p boring (300 + 9 + 39 passed) are clean.

A build started from a git hook inherits GIT_DIR and related variables,
so the build script's git init and git apply ran against the caller's
repository instead of the copied BoringSSL sources. From a worktree this
writes core.bare = true into the main checkout's config.

Fixes cloudflare#527
@clanky-the-clanker

Copy link
Copy Markdown

This pull request was not reviewed automatically because its author does not have write access to this repository. A maintainer with write access can start a review by commenting clanky review (or clanker review or @clanky-the-clanker review), optionally followed by instructions (e.g. clanky review focus on the retry logic).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cargo builds can corrupt consumer git repos due to env vars

1 participant