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

Don't default to submodules = true unless the rust repo has a .git directory #87380

Merged
merged 1 commit into from
Jul 23, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jul 22, 2021

Should hopefully fix #82653 (comment) - @semarie can you confirm?

r? @Mark-Simulacrum

@jyn514 jyn514 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jul 22, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 22, 2021
@semarie
Copy link
Contributor

semarie commented Jul 22, 2021

@jyn514 yes, with the patch applied on top of latest tarball, it is building.

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 23, 2021

📌 Commit 6194cc8 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 23, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 23, 2021
Rollup of 14 pull requests

Successful merges:

 - rust-lang#86410 (VecMap::get_value_matching should return just one element)
 - rust-lang#86790 (Document iteration order of `retain` functions)
 - rust-lang#87171 (Remove Option from BufWriter)
 - rust-lang#87175 (Stabilize `into_parts()` and `into_error()`)
 - rust-lang#87185 (Fix panics on Windows when the build was cancelled)
 - rust-lang#87191 (Package LLVM libs for the target rather than the build host)
 - rust-lang#87255 (better support for running libcore tests with Miri)
 - rust-lang#87266 (Add testcase for 87076)
 - rust-lang#87283 (Add `--codegen-backends=foo,bar` configure flag)
 - rust-lang#87322 (fix: clarify suggestion that `&T` must refer to `T: Sync` for `&T: Send`)
 - rust-lang#87358 (Fix `--dry-run` when download-ci-llvm is set)
 - rust-lang#87380 (Don't default to `submodules = true` unless the rust repo has a .git directory)
 - rust-lang#87398 (Add test for fonts used for module items)
 - rust-lang#87412 (Add missing article)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f709999 into rust-lang:master Jul 23, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jul 23, 2021
@jyn514 jyn514 deleted the smarter-submodule-defaults branch July 23, 2021 23:37
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jul 28, 2021
Don't treat git repos as non-existent when `ignore_git` is set

The new submodule handling depends on `is_git()` to be accurate to
decide whether it should handle submodules at all or not. Unfortunately,
`is_git()` treated "this directory does not have a git repository" and
"this repository should not be used for SHA/version/commit date info"
the same. This changes it to distinguish the two.

To clarify: ignore_get is set by default whenever channel == "dev", which it is by default whenever you're compiling locally. So basically everyone would hit this, not just people who had explicitly configured ignore_git.

Here's an example of an error this fixes:

```
$ x build
Updating only changed submodules
Submodules updated in 0.01 seconds
    Finished dev [unoptimized + debuginfo] target(s) in 0.17s
warning: x.py has made several changes recently you may want to look at
help: consider looking at the changes in `src/bootstrap/CHANGELOG.md`
note: to silence this warning, add `changelog-seen = 2` at the top of `config.toml`
Building stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.16s
Copying stage0 std from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Building LLVM for x86_64-unknown-linux-gnu
detected home dir change, cleaning out entire build directory
running: "cmake" "/home/joshua/rustc3/src/llvm-project/llvm" "-G" "Ninja" "-DLLVM_ENABLE_ASSERTIONS=OFF" "-DLLVM_TARGETS_TO_BUILD=AArch64;ARM;BPF;Hexagon;MSP430;Mips;NVPTX;PowerPC;RISCV;Sparc;SystemZ;WebAssembly;X86" "-DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=AVR" "-DLLVM_INCLUDE_EXAMPLES=OFF" "-DLLVM_INCLUDE_DOCS=OFF" "-DLLVM_INCLUDE_BENCHMARKS=OFF" "-DLLVM_ENABLE_TERMINFO=OFF" "-DLLVM_ENABLE_LIBEDIT=OFF" "-DLLVM_ENABLE_BINDINGS=OFF" "-DLLVM_ENABLE_Z3_SOLVER=OFF" "-DLLVM_PARALLEL_COMPILE_JOBS=48" "-DLLVM_TARGET_ARCH=x86_64" "-DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-unknown-linux-gnu" "-DLLVM_ENABLE_ZLIB=ON" "-DLLVM_ENABLE_LIBXML2=OFF" "-DLLVM_VERSION_SUFFIX=-rust-dev" "-DCMAKE_INSTALL_MESSAGE=LAZY" "-DCMAKE_C_COMPILER=gcc" "-DCMAKE_CXX_COMPILER=g++" "-DCMAKE_ASM_COMPILER=gcc" "-DCMAKE_C_FLAGS=-ffunction-sections -fdata-sections -fPIC -m64" "-DCMAKE_CXX_FLAGS=-ffunction-sections -fdata-sections -fPIC -m64" "-DCMAKE_INSTALL_PREFIX=/home/joshua/rustc3/build/x86_64-unknown-linux-gnu/llvm" "-DCMAKE_ASM_FLAGS= -ffunction-sections -fdata-sections -fPIC -m64" "-DCMAKE_BUILD_TYPE=Release"
CMake Error: The source directory "/home/joshua/rustc3/src/llvm-project/llvm" does not exist.
Specify --help for usage, or press the help button on the CMake GUI.
thread 'main' panicked at '
command did not execute successfully, got: exit status: 1

build script failed, must exit now', /home/joshua/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/cmake-0.1.44/src/lib.rs:885:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
	finished in 0.783 seconds
Build completed unsuccessfully in 0:00:01
```

I *believe* this regression was only introduced in rust-lang#87380, not rust-lang#82653. `@petrochenkov` can you check that this fixes the issue you encountered in rust-lang#82653 (comment) ?

r? `@Mark-Simulacrum`
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jul 28, 2021
Don't treat git repos as non-existent when `ignore_git` is set

The new submodule handling depends on `is_git()` to be accurate to
decide whether it should handle submodules at all or not. Unfortunately,
`is_git()` treated "this directory does not have a git repository" and
"this repository should not be used for SHA/version/commit date info"
the same. This changes it to distinguish the two.

To clarify: ignore_get is set by default whenever channel == "dev", which it is by default whenever you're compiling locally. So basically everyone would hit this, not just people who had explicitly configured ignore_git.

Here's an example of an error this fixes:

```
$ x build
Updating only changed submodules
Submodules updated in 0.01 seconds
    Finished dev [unoptimized + debuginfo] target(s) in 0.17s
warning: x.py has made several changes recently you may want to look at
help: consider looking at the changes in `src/bootstrap/CHANGELOG.md`
note: to silence this warning, add `changelog-seen = 2` at the top of `config.toml`
Building stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.16s
Copying stage0 std from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Building LLVM for x86_64-unknown-linux-gnu
detected home dir change, cleaning out entire build directory
running: "cmake" "/home/joshua/rustc3/src/llvm-project/llvm" "-G" "Ninja" "-DLLVM_ENABLE_ASSERTIONS=OFF" "-DLLVM_TARGETS_TO_BUILD=AArch64;ARM;BPF;Hexagon;MSP430;Mips;NVPTX;PowerPC;RISCV;Sparc;SystemZ;WebAssembly;X86" "-DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=AVR" "-DLLVM_INCLUDE_EXAMPLES=OFF" "-DLLVM_INCLUDE_DOCS=OFF" "-DLLVM_INCLUDE_BENCHMARKS=OFF" "-DLLVM_ENABLE_TERMINFO=OFF" "-DLLVM_ENABLE_LIBEDIT=OFF" "-DLLVM_ENABLE_BINDINGS=OFF" "-DLLVM_ENABLE_Z3_SOLVER=OFF" "-DLLVM_PARALLEL_COMPILE_JOBS=48" "-DLLVM_TARGET_ARCH=x86_64" "-DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-unknown-linux-gnu" "-DLLVM_ENABLE_ZLIB=ON" "-DLLVM_ENABLE_LIBXML2=OFF" "-DLLVM_VERSION_SUFFIX=-rust-dev" "-DCMAKE_INSTALL_MESSAGE=LAZY" "-DCMAKE_C_COMPILER=gcc" "-DCMAKE_CXX_COMPILER=g++" "-DCMAKE_ASM_COMPILER=gcc" "-DCMAKE_C_FLAGS=-ffunction-sections -fdata-sections -fPIC -m64" "-DCMAKE_CXX_FLAGS=-ffunction-sections -fdata-sections -fPIC -m64" "-DCMAKE_INSTALL_PREFIX=/home/joshua/rustc3/build/x86_64-unknown-linux-gnu/llvm" "-DCMAKE_ASM_FLAGS= -ffunction-sections -fdata-sections -fPIC -m64" "-DCMAKE_BUILD_TYPE=Release"
CMake Error: The source directory "/home/joshua/rustc3/src/llvm-project/llvm" does not exist.
Specify --help for usage, or press the help button on the CMake GUI.
thread 'main' panicked at '
command did not execute successfully, got: exit status: 1

build script failed, must exit now', /home/joshua/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/cmake-0.1.44/src/lib.rs:885:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
	finished in 0.783 seconds
Build completed unsuccessfully in 0:00:01
```

I *believe* this regression was only introduced in rust-lang#87380, not rust-lang#82653. ``@petrochenkov`` can you check that this fixes the issue you encountered in rust-lang#82653 (comment) ?

r? ``@Mark-Simulacrum``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants