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

With submodules=true, using ./x.py fmt as a git hook makes bootstrap delete all submodules #125954

Closed
RalfJung opened this issue Jun 4, 2024 · 3 comments · Fixed by #126255
Closed
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@RalfJung
Copy link
Member

RalfJung commented Jun 4, 2024

To reproduce, I am in a worktree without .git (i.e., not the "main" worktree). I have a pre-commit hook set up as follows

#!/bin/sh
./x.py fmt || true # ignore failures

Now I change something trivial (e.g. remove a trailing . in some comments) and commit. That does the following

$ git commit -am test
Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.06s
Updating submodule src/tools/cargo
Saved working directory and index state WIP on from-ref: 64e7337be59 more explicitly state the basic rules of working with the obtained raw pointers

After a second or two I get suspicious and hit Ctrl-C. Now it looks like bootstrap has done something with the (previously clean) cargo submodule

$ git status
On branch from-ref
Your branch is up to date with 'ralf/from-ref'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
  (commit or discard the untracked or modified content in submodules)
    modified:   library/core/src/ptr/mod.rs
    modified:   src/tools/cargo (modified content, untracked content)

So let's cd src/tools/cargo and see what's up

$ git status
HEAD detached at 7a6fad098
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
    modified:   .github/ISSUE_TEMPLATE/config.yml
    modified:   .gitignore
    modified:   CONTRIBUTING.md
    modified:   Cargo.lock
    modified:   Cargo.toml
    modified:   LICENSE-APACHE
    modified:   README.md
    modified:   triagebot.toml

Untracked files:
  (use "git add <file>..." to include in what will be committed)
    .editorconfig
    .git-blame-ignore-revs
    .gitattributes
    .github/ISSUE_TEMPLATE/blank_issue.md
    .github/ISSUE_TEMPLATE/bug_report.md
    .github/ISSUE_TEMPLATE/diagnostics.yaml
    .github/ISSUE_TEMPLATE/documentation.yaml
    .github/ISSUE_TEMPLATE/ice.md
    .github/ISSUE_TEMPLATE/ice.yaml
    .github/ISSUE_TEMPLATE/library_tracking_issue.md
    .github/ISSUE_TEMPLATE/regression.md
    .github/ISSUE_TEMPLATE/tracking_issue.md
    .github/pull_request_template.md
    .github/workflows/ci.yml
    .github/workflows/dependencies.yml
    .gitmodules
    .mailmap
    .reuse/
    COPYRIGHT
    INSTALL.md
    LICENSES/
    RELEASES.md
    compiler/
    config.example.toml
    configure
    library/
    rust-bors.toml
    rustfmt.toml
    src/README.md
    src/bootstrap/
    src/ci/
    src/doc/complement-design-faq.md
    src/doc/complement-lang-faq.md
    src/doc/complement-project-faq.md
    src/doc/favicon.inc
    src/doc/footer.inc
    src/doc/full-toc.inc
    src/doc/grammar.md
    src/doc/guide-crates.md
    src/doc/guide-error-handling.md
    src/doc/guide-ffi.md
    src/doc/guide-macros.md
    src/doc/guide-ownership.md
    src/doc/guide-plugins.md
    src/doc/guide-pointers.md
    src/doc/guide-strings.md
    src/doc/guide-tasks.md
    src/doc/guide-testing.md
    src/doc/guide-unsafe.md
    src/doc/guide.md
    src/doc/index.md
    src/doc/intro.md
    src/doc/man/rustc.1
    src/doc/man/rustdoc.1
    src/doc/not_found.md
    src/doc/redirect.inc
    src/doc/reference.md
    src/doc/robots.txt
    src/doc/rust.css
    src/doc/rust.md
    src/doc/rustc/
    src/doc/rustdoc.md
    src/doc/rustdoc/
    src/doc/style-guide/
    src/doc/tutorial.md
    src/doc/unstable-book/
    src/doc/version_info.html.template
    src/etc/CONFIGS.md
    src/etc/cat-and-grep.sh
    src/etc/completions/
    src/etc/cpu-usage-over-time-plot.sh
    src/etc/ctags.rust
    src/etc/dec2flt_table.py
    src/etc/gdb_load_rust_pretty_printers.py
    src/etc/gdb_lookup.py
    src/etc/gdb_providers.py
    src/etc/generate-deriving-span-tests.py
    src/etc/generate-keyword-tests.py
    src/etc/htmldocck.py
    src/etc/indenter
    src/etc/installer/
    src/etc/lldb_batchmode.py
    src/etc/lldb_commands
    src/etc/lldb_lookup.py
    src/etc/lldb_providers.py
    src/etc/natvis/
    src/etc/pre-push.sh
    src/etc/rust-gdb
    src/etc/rust-gdbgui
    src/etc/rust-lldb
    src/etc/rust-windbg.cmd
    src/etc/rust_analyzer_settings.json
    src/etc/rust_types.py
    src/etc/test-float-parse/
    src/etc/third-party/
    src/librustdoc/
    src/rustdoc-json-types/
    src/stage0
    src/tools/
    src/version
    tests/COMPILER_TESTS.md
    tests/assembly/
    tests/auxiliary/
    tests/codegen-units/
    tests/codegen/
    tests/coverage-run-rustdoc/
    tests/coverage/
    tests/crashes/
    tests/debuginfo/
    tests/incremental/
    tests/mir-opt/
    tests/pretty/
    tests/run-make-fulldeps/
    tests/run-make/
    tests/run-pass-valgrind/
    tests/rustdoc-gui/
    tests/rustdoc-js-std/
    tests/rustdoc-js/
    tests/rustdoc-json/
    tests/rustdoc-ui/
    tests/rustdoc/
    tests/ui-fulldeps/
    tests/ui/
    x
    x.ps1
    x.py

no changes added to commit (use "git add" and/or "git commit -a")

Eh... so the added files are rustc files? Why are those being added in src/tools/cargo?
Something is going seriously wrong here.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 4, 2024
@jieyouxu jieyouxu added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) C-bug Category: This is a bug. A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jun 4, 2024
@onur-ozkan
Copy link
Member

I think this only happens with worktrees. I attempted to reproduce it about 25 times without success.

@RalfJung
Copy link
Member Author

Yes indeed, this does not happen in my rustc folder (the main checkout, where .git lives), but it happens in worktree folders.

@RalfJung
Copy link
Member Author

Adding some debug logging, it seems like some truly strange things are happening...

output from cd "/home/r/src/rust/rustc.2/src/tools/cargo" && "git" "rev-parse" "HEAD" is "d402830c8a356332de93761d6996faf5a2ca29ca\n"

This is the HEAD commit of the outer repository!

I think what's happening is that some environment variables are set inside the git hook which overwrite the --git-dir. x.py relies on just setting the working directory to determine which git dir to use, but e.g. GIT_DIR takes precedence over that. I tried setting --git-dir instead of using current_dir and that does seem to help.

onur-ozkan added a commit to onur-ozkan/rust that referenced this issue Jun 12, 2024
Due to rust-lang#125954, we had to modify git invocations with
certain flags in rust-lang#126255. However, because there are so many
instances of `Command::new("git")` in bootstrap, it is difficult to apply these solutions to all of
them.

This PR creates a helper function that unifies the git usage in bootstrap. Meaning, whenever special flags
or hacks are needed, we can apply them to this single function which makes things much simpler for the bootstrap team.

Signed-off-by: onur-ozkan <[email protected]>
onur-ozkan added a commit to onur-ozkan/rust that referenced this issue Jun 12, 2024
Due to rust-lang#125954, we had to modify git invocations with
certain flags in rust-lang#126255. However, because there are so many
instances of `Command::new("git")` in bootstrap, it is difficult to apply these solutions to all of
them.

This PR creates a helper function that unifies the git usage in bootstrap. Meaning, whenever special flags
or hacks are needed, we can apply them to this single function which makes things much simpler for the bootstrap team.

Signed-off-by: onur-ozkan <[email protected]>
onur-ozkan added a commit to onur-ozkan/rust that referenced this issue Jun 12, 2024
Due to rust-lang#125954, we had to modify git invocations with
certain flags in rust-lang#126255. However, because there are so many
instances of `Command::new("git")` in bootstrap, it is difficult to apply these solutions to all of
them.

This PR creates a helper function that unifies the git usage in bootstrap. Meaning, whenever special flags
or hacks are needed, we can apply them to this single function which makes things much simpler for the bootstrap team.

Signed-off-by: onur-ozkan <[email protected]>
jhpratt added a commit to jhpratt/rust that referenced this issue Jun 16, 2024
…r=Kobzol

unify git command preperation

Due to rust-lang#125954, we had to modify git invocations with certain flags in rust-lang#126255. However, because there are so many instances of `Command::new("git")` in bootstrap, it is difficult to apply these solutions to all of them.

This PR creates a helper function that unifies the git usage in bootstrap. Meaning, whenever special flags or hacks are needed, we can apply them to this single function which makes things much simpler for the bootstrap team.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 16, 2024
Rollup merge of rust-lang#126309 - onur-ozkan:unify-git-utilization, r=Kobzol

unify git command preperation

Due to rust-lang#125954, we had to modify git invocations with certain flags in rust-lang#126255. However, because there are so many instances of `Command::new("git")` in bootstrap, it is difficult to apply these solutions to all of them.

This PR creates a helper function that unifies the git usage in bootstrap. Meaning, whenever special flags or hacks are needed, we can apply them to this single function which makes things much simpler for the bootstrap team.
@bors bors closed this as completed in dcda375 Jun 17, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 17, 2024
Rollup merge of rust-lang#126255 - RalfJung:bootstrap-submodules, r=onur-ozkan

fix checking git submodules during a commit hook

Fixes rust-lang#125954
The one thing that still puzzles me is why this only is a problem with worktrees.

r? `@onur`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants