Crashes on packages with broken symbolic links

[jyn514]

I had a link in the top level directory that was broken, took me forever to figure out what the error actually was.

The error is coming from somewhere near here: https://github.com/rust-lang/rustwide/blob/master/src/crates/local.rs#L51

...
2019/11/04 04:49:50 [INFO] cratesfyi::docbuilder::rustwide_builder: building package rcc 0.1.0
2019/11/04 04:49:50 [INFO] rustwide::crates::local: copying local crate from /build to /home/cratesfyi/rustwide/builds/rcc-0.1.0/source
thread 'main' panicked at 'failed to build documentation: Os { code: 2, kind: NotFound, message: "No such file or directory" }

stack backtrace:
   0:     0x5618e503acbd - backtrace::backtrace::trace::h8a6a3924ab912084
   1:     0x5618e50396d2 - backtrace::capture::Backtrace::new_unresolved::h32baafbff7da6381
   2:     0x5618e5039087 - failure::backtrace::internal::InternalBacktrace::new::he7ea4be726cbc04f
   3:     0x5618e5039241 - <failure::backtrace::Backtrace as core::default::Default>::default::hb3a5647f7da2650d
   4:     0x5618e4985c72 - <rustwide::crates::local::Local as rustwide::crates::CrateTrait>::copy_source_to::h38d3150a66853397
   5:     0x5618e49c41ca - rustwide::crates::Crate::copy_source_to::hb8883f6638576a50
   6:     0x5618e49a4ce7 - rustwide::prepare::Prepare::prepare::h68b81601bb428e21
   7:     0x5618e47fa38f - rustwide::build::BuildBuilder::run::hc1083f614508117a
   8:     0x5618e489c6fe - cratesfyi::docbuilder::rustwide_builder::RustwideBuilder::build_package::h7a26bcb3a0f58c30
   9:     0x5618e489bf7c - cratesfyi::docbuilder::rustwide_builder::RustwideBuilder::build_local_package::h481bc5961bfef4c6
  10:     0x5618e47a5d78 - cratesfyi::main::h081055f3d1e49a0b
  11:     0x5618e4799182 - std::rt::lang_start::{{closure}}::h38c15e7f2a27e7a1
  12:     0x5618e505ec62 - std::rt::lang_start_internal::{{closure}}::h4e93c1949c7a1955
                        at src/libstd/rt.rs:49
                         - std::panicking::try::do_call::h9440ccd4dc467eaa
                        at src/libstd/panicking.rs:296
  13:     0x5618e5069669 - __rust_maybe_catch_panic
                        at src/libpanic_unwind/lib.rs:80
  14:     0x5618e505f82c - std::panicking::try::hc046e7ee42ee744f
                        at src/libstd/panicking.rs:275
                         - std::panic::catch_unwind::h27dfc457c200aee0
                        at src/libstd/panic.rs:394
                         - std::rt::lang_start_internal::hea1b49a567afe309
                        at src/libstd/rt.rs:48
  15:     0x5618e47a7641 - main
  16:     0x7f3090f08b96 - __libc_start_main
  17:     0x5618e4794179 - _start
  18:                0x0 - <unknown>', src/bin/cratesfyi.rs:175:17
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.34/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.34/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:47
   3: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:36
   4: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:200
   5: std::panicking::default_hook
             at src/libstd/panicking.rs:214
   6: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:477
   7: std::panicking::continue_panic_fmt
             at src/libstd/panicking.rs:384
   8: std::panicking::begin_panic_fmt
             at src/libstd/panicking.rs:339
   9: cratesfyi::main
  10: std::rt::lang_start::{{closure}}
  11: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:49
  12: std::panicking::try::do_call
             at src/libstd/panicking.rs:296
  13: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:80
  14: std::panicking::try
             at src/libstd/panicking.rs:275
  15: std::panic::catch_unwind
             at src/libstd/panic.rs:394
  16: std::rt::lang_start_internal
             at src/libstd/rt.rs:48
  17: main
  18: __libc_start_main
  19: _start

[jyn514]

Strangely the documentation for WalkDir says that this shouldn't be an error:

Follow symbolic links. By default, this is disabled. When yes is true, symbolic links are followed as if they were normal directories and files. If a symbolic link is broken or is involved in a loop, an error is yielded.

https://docs.rs/walkdir/2.2.9/walkdir/struct.WalkDir.html#method.follow_links

This might be a bug in WalkDir, not Rustwide.

[pietroalbini]

My guess is that, since WalkDir is not configured to follow symlinks, copy_dir tries to call std::fs::copy on the broken symlink:

https://github.com/rust-lang/rustwide/blob/afb04e81d6626d347148282a5804db8a513bfde3/src/crates/local.rs#L60-L64

I think the best fix is to instead configure WalkDir to follow them, so we don't have to deal with the issue ourselves.

[jyn514]

If WalkDir followed links, we would still get the same error, just a little sooner:

If a symbolic link is broken or is involved in a loop, an error is yielded.

Maybe we could configure std::fs::copy not to follow symbolic links? I don't see a way to do that in the documentation though ... https://doc.rust-lang.org/std/fs/fn.copy.html

[jyn514]

WalkDir has an API to check whether or not it's a symbolic link at least, maybe we could do something like this:

    for entry in WalkDir::new(&src).into_iter()
        .filter_entry(|entry| !(entry.path_is_symlink() && /* check that entry isn't a broken link */) {
        // ...
    }

[pietroalbini]

I'd consider a link being broken an error though.

[jyn514]

I feel like that should be up to the developer ... But in any case we should give a better error message if we're going to leave this in place, 'No such file or directory' sounds very strange for a copy.

[pietroalbini]

Yeah, adding context to the errors mentioning the file name would probably solve it.