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

bootstrap/clean.rs cannot handle symlinks #40860

Closed
nodakai opened this issue Mar 27, 2017 · 1 comment
Closed

bootstrap/clean.rs cannot handle symlinks #40860

nodakai opened this issue Mar 27, 2017 · 1 comment
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@nodakai
Copy link
Contributor

nodakai commented Mar 27, 2017

The implementation can only handle regular files and directories:

$ RUST_BACKTRACE=1 make clean
extracting /home/nodakai/src/rust-HEAD/build/cache/2017-02-01/rust-std-beta-x86_64-unknown-linux-gnu.tar.gz
extracting /home/nodakai/src/rust-HEAD/build/cache/2017-02-01/rustc-beta-x86_64-unknown-linux-gnu.tar.gz
extracting /home/nodakai/src/rust-HEAD/build/cache/407edef22e894266eb562618cba5ca9757051946/cargo-nightly-x86_64-unknown-linux-gnu.tar.gz
    Finished dev [unoptimized] target(s) in 0.0 secs
thread 'main' panicked at 'failed to remove dir /home/nodakai/src/rust-HEAD/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/doc: Not a directory (os error 20)', src/bootstrap/clean.rs:84
stack backtrace:
   1:     0x55fb07f52d7c - std::sys::imp::backtrace::tracing::imp::write::h23bcdb89e70c5bbf
                        at /buildslave/rust-buildbot/slave/beta-dist-rustc-linux/build/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:42
   2:     0x55fb07f5b0be - std::panicking::default_hook::{{closure}}::he7b82439fd2d2bb6
                        at /buildslave/rust-buildbot/slave/beta-dist-rustc-linux/build/src/libstd/panicking.rs:351
   3:     0x55fb07f5acc4 - std::panicking::default_hook::he1cd4269c1558f23
                        at /buildslave/rust-buildbot/slave/beta-dist-rustc-linux/build/src/libstd/panicking.rs:367
   4:     0x55fb07f5b4fb - std::panicking::rust_panic_with_hook::h006b37e36b7c8982
                        at /buildslave/rust-buildbot/slave/beta-dist-rustc-linux/build/src/libstd/panicking.rs:555
   5:     0x55fb07f5b344 - std::panicking::begin_panic::h043cddfdd3933cc4
                        at /buildslave/rust-buildbot/slave/beta-dist-rustc-linux/build/src/libstd/panicking.rs:517
   6:     0x55fb07f5b2b9 - std::panicking::begin_panic_fmt::h34e588bba6b8a2c2
                        at /buildslave/rust-buildbot/slave/beta-dist-rustc-linux/build/src/libstd/panicking.rs:501
   7:     0x55fb07ddf1ab - bootstrap::clean::do_op::hdae2989b2a396018
   8:     0x55fb07ddd187 - bootstrap::clean::rm_rf::h8404e5c7dea5380d
   9:     0x55fb07ddd08c - bootstrap::clean::rm_rf::h8404e5c7dea5380d
  10:     0x55fb07ddd08c - bootstrap::clean::rm_rf::h8404e5c7dea5380d
  11:     0x55fb07ddc4b1 - bootstrap::clean::clean::h735ac6b031a4468a
  12:     0x55fb07e4a82f - bootstrap::Build::build::hec419daa0bd76b14
  13:     0x55fb07cbe3be - bootstrap::main::h8b662183e4af9c72
  14:     0x55fb07f6250a - __rust_maybe_catch_panic
                        at /buildslave/rust-buildbot/slave/beta-dist-rustc-linux/build/src/libpanic_unwind/lib.rs:98
  15:     0x55fb07f5bc66 - std::rt::lang_start::h1ef940195e3c010e
                        at /buildslave/rust-buildbot/slave/beta-dist-rustc-linux/build/src/libstd/panicking.rs:436
                        at /buildslave/rust-buildbot/slave/beta-dist-rustc-linux/build/src/libstd/panic.rs:361
                        at /buildslave/rust-buildbot/slave/beta-dist-rustc-linux/build/src/libstd/rt.rs:57
  16:     0x55fb07cbe539 - main
  17:     0x7f27382bf2b0 - __libc_start_main
  18:     0x55fb07cb66a9 - _start
  19:                0x0 - <unknown>
Build completed unsuccessfully in 0:00:02
Makefile:37: recipe for target 'clean' failed
make: *** [clean] Error 101
$ LANG=C ll /home/nodakai/src/rust-HEAD/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/doc
lrwxrwxrwx 1 nodakai nodakai 69 Mar 12 16:11 /home/nodakai/src/rust-HEAD/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/doc -> /home/nodakai/src/rust-HEAD/build/x86_64-unknown-linux-gnu/crate-docs/

Apparently the symlink was generated by

    let my_out = build.crate_doc_out(target);
    build.clear_if_dirty(&my_out, &rustdoc);
    t!(symlink_dir_force(&my_out, &out_dir));

in bootstrap/doc.rs

@alexcrichton alexcrichton added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Mar 27, 2017
@CleanCut
Copy link
Contributor

CleanCut commented Apr 2, 2017

I was able to reproduce this on my setup with:

export MYARCH=x86_64-apple-darwin && mkdir -p build/$MYARCH/subdir && touch build/$MYARCH/subdir/file && ln -s build/$MYARCH/subdir/file build/$MYARCH/subdir/symlink

frewsxcv added a commit to frewsxcv/rust that referenced this issue Apr 6, 2017
Handle symlinks in src/bootstrap/clean.rs (mostly) -- resolves rust-lang#40860.

In response to rust-lang#40860

The broken condition can be replicated with:

```shell
export MYARCH=x86_64-apple-darwin && mkdir -p build/$MYARCH/subdir &&
touch build/$MYARCH/subdir/file && ln -s build/$MYARCH/subdir/file
build/$MYARCH/subdir/symlink
```

`src/bootstrap/clean.rs` has a custom implementation of removing a tree
`fn rm_rf` that used `std::path::Path::{is_file, is_dir, exists}` while
recursively deleting directories and files.  Unfortunately, `Path`'s
implementation of `is_file()` and `is_dir()` and `exists()` always
unconditionally follow symlinks, which is the exact opposite of standard
implementations of deleting file trees.

It appears that this custom implementation is being used to workaround a
behavior in Windows where the files often get marked as read-only, which
prevents us from simply using something nice and simple like
`std::fs::remove_dir_all`, which properly deletes links instead of
following them.

So it looks like the fix is to use `.symlink_metadata()` to figure out
whether tree items are files/symlinks/directories.  The one corner case
this won't cover is if there is a broken symlink in the "root"
`build/$MYARCH` directory, because those initial entries are run through
`Path::canonicalize()`, which panics with broken symlinks.  So lets just
never use symlinks in that one directory. :-)
frewsxcv added a commit to frewsxcv/rust that referenced this issue Apr 6, 2017
Handle symlinks in src/bootstrap/clean.rs (mostly) -- resolves rust-lang#40860.

In response to rust-lang#40860

The broken condition can be replicated with:

```shell
export MYARCH=x86_64-apple-darwin && mkdir -p build/$MYARCH/subdir &&
touch build/$MYARCH/subdir/file && ln -s build/$MYARCH/subdir/file
build/$MYARCH/subdir/symlink
```

`src/bootstrap/clean.rs` has a custom implementation of removing a tree
`fn rm_rf` that used `std::path::Path::{is_file, is_dir, exists}` while
recursively deleting directories and files.  Unfortunately, `Path`'s
implementation of `is_file()` and `is_dir()` and `exists()` always
unconditionally follow symlinks, which is the exact opposite of standard
implementations of deleting file trees.

It appears that this custom implementation is being used to workaround a
behavior in Windows where the files often get marked as read-only, which
prevents us from simply using something nice and simple like
`std::fs::remove_dir_all`, which properly deletes links instead of
following them.

So it looks like the fix is to use `.symlink_metadata()` to figure out
whether tree items are files/symlinks/directories.  The one corner case
this won't cover is if there is a broken symlink in the "root"
`build/$MYARCH` directory, because those initial entries are run through
`Path::canonicalize()`, which panics with broken symlinks.  So lets just
never use symlinks in that one directory. :-)
@bors bors closed this as completed in efd6eab Apr 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

No branches or pull requests

3 participants