Auto merge of #47203 - varkor:output-filename-conflicts-with-directory, r=estebank

Warn when rustc output conflicts with existing directories

When the compiled executable would conflict with a directory, display a
rustc error instead of a verbose and potentially-confusing linker
error. This is a usability improvement, and doesn’t actually change
behaviour with regards to compilation success. This addresses the
concern in #35887. Fixes #13098.

src/librustc/session/config.rs

@@ -548,25 +548,6 @@ impl OutputFilenames {
     pub fn filestem(&self) -> String {
         format!("{}{}", self.out_filestem, self.extra)
     }
-
-    pub fn contains_path(&self, input_path: &PathBuf) -> bool {
-        let input_path = input_path.canonicalize().ok();
-        if input_path.is_none() {
-            return false
-        }
-        match self.single_output_file {
-            Some(ref output_path) => output_path.canonicalize().ok() == input_path,
-            None => {
-                for k in self.outputs.keys() {
-                    let output_path = self.path(k.to_owned());
-                    if output_path.canonicalize().ok() == input_path {
-                        return true;
-                    }
-                }
-                false
-            }
-        }
-    }
 }
 
 pub fn host_triple() -> &'static str {

src/librustc_driver/driver.rs

@@ -121,23 +121,8 @@ pub fn compile_input(trans: Box<TransCrate>,
         };
 
         let outputs = build_output_filenames(input, outdir, output, &krate.attrs, sess);
-
-        // Ensure the source file isn't accidentally overwritten during compilation.
-        match *input_path {
-            Some(ref input_path) => {
-                if outputs.contains_path(input_path) && sess.opts.will_create_output_file() {
-                    sess.err(&format!(
-                        "the input file \"{}\" would be overwritten by the generated executable",
-                        input_path.display()));
-                    return Err(CompileIncomplete::Stopped);
-                }
-            },
-            None => {}
-        }
-
         let crate_name =
             ::rustc_trans_utils::link::find_crate_name(Some(sess), &krate.attrs, input);
-
         let ExpansionResult { expanded_crate, defs, analysis, resolutions, mut hir_forest } = {
             phase_2_configure_and_expand(
                 sess,
@@ -157,7 +142,29 @@ pub fn compile_input(trans: Box<TransCrate>,
             )?
         };
 
-        write_out_deps(sess, &outputs, &crate_name);
+        let output_paths = generated_output_paths(sess, &outputs, output.is_some(), &crate_name);
+
+        // Ensure the source file isn't accidentally overwritten during compilation.
+        if let Some(ref input_path) = *input_path {
+            if sess.opts.will_create_output_file() {
+                if output_contains_path(&output_paths, input_path) {
+                    sess.err(&format!(
+                        "the input file \"{}\" would be overwritten by the generated \
+                        executable",
+                        input_path.display()));
+                    return Err(CompileIncomplete::Stopped);
+                }
+                if let Some(dir_path) = output_conflicts_with_dir(&output_paths) {
+                    sess.err(&format!(
+                        "the generated executable for the input file \"{}\" conflicts with the \
+                        existing directory \"{}\"",
+                        input_path.display(), dir_path.display()));
+                    return Err(CompileIncomplete::Stopped);
+                }
+            }
+        }
+
+        write_out_deps(sess, &outputs, &output_paths);
         if sess.opts.output_types.contains_key(&OutputType::DepInfo) &&
             sess.opts.output_types.keys().count() == 1 {
             return Ok(())
@@ -1101,16 +1108,22 @@ fn escape_dep_filename(filename: &FileName) -> String {
     filename.to_string().replace(" ", "\\ ")
 }
 
-fn write_out_deps(sess: &Session, outputs: &OutputFilenames, crate_name: &str) {
+// Returns all the paths that correspond to generated files.
+fn generated_output_paths(sess: &Session,
+                          outputs: &OutputFilenames,
+                          exact_name: bool,
+                          crate_name: &str) -> Vec<PathBuf> {
     let mut out_filenames = Vec::new();
     for output_type in sess.opts.output_types.keys() {
         let file = outputs.path(*output_type);
         match *output_type {
-            OutputType::Exe => {
+            // If the filename has been overridden using `-o`, it will not be modified
-                for output in sess.crate_types.borrow().iter() {
+            // by appending `.rlib`, `.exe`, etc., so we can skip this transformation.
+            OutputType::Exe if !exact_name => {
+                for crate_type in sess.crate_types.borrow().iter() {
                     let p = ::rustc_trans_utils::link::filename_for_input(
                         sess,
-                        *output,
+                        *crate_type,
                         crate_name,
                         outputs
                     );
@@ -1125,7 +1138,46 @@ fn write_out_deps(sess: &Session, outputs: &OutputFilenames, crate_name: &str) {
             }
         }
     }
+    out_filenames
+}
 
+// Runs `f` on every output file path and returns the first non-None result, or None if `f`
+// returns None for every file path.
+fn check_output<F, T>(output_paths: &Vec<PathBuf>, f: F) -> Option<T>
+        where F: Fn(&PathBuf) -> Option<T> {
+            for output_path in output_paths {
+                if let Some(result) = f(output_path) {
+                    return Some(result);
+                }
+            }
+            None
+}
+
+pub fn output_contains_path(output_paths: &Vec<PathBuf>, input_path: &PathBuf) -> bool {
+    let input_path = input_path.canonicalize().ok();
+    if input_path.is_none() {
+        return false
+    }
+    let check = |output_path: &PathBuf| {
+        if output_path.canonicalize().ok() == input_path {
+            Some(())
+        } else { None }
+    };
+    check_output(output_paths, check).is_some()
+}
+
+pub fn output_conflicts_with_dir(output_paths: &Vec<PathBuf>) -> Option<PathBuf> {
+    let check = |output_path: &PathBuf| {
+        if output_path.is_dir() {
+            Some(output_path.clone())
+        } else { None }
+    };
+    check_output(output_paths, check)
+}
+
+fn write_out_deps(sess: &Session,
+                  outputs: &OutputFilenames,
+                  out_filenames: &Vec<PathBuf>) {
     // Write out dependency rules to the dep-info file if requested
     if !sess.opts.output_types.contains_key(&OutputType::DepInfo) {
         return;
@@ -1144,7 +1196,7 @@ fn write_out_deps(sess: &Session, outputs: &OutputFilenames, crate_name: &str) {
                                          .map(|fmap| escape_dep_filename(&fmap.name))
                                          .collect();
             let mut file = fs::File::create(&deps_filename)?;
-            for path in &out_filenames {
+            for path in out_filenames {
                 write!(file, "{}: {}\n\n", path.display(), files.join(" "))?;
             }
 
@@ -1327,7 +1379,10 @@ pub fn build_output_filenames(input: &Input,
                 Some(out_file.clone())
             };
             if *odir != None {
-                sess.warn("ignoring --out-dir flag due to -o flag.");
+                sess.warn("ignoring --out-dir flag due to -o flag");
+            }
+            if !sess.opts.cg.extra_filename.is_empty() {
+                sess.warn("ignoring -C extra-filename flag due to -o flag");
             }
 
             let cur_dir = Path::new("");

src/test/run-make/output-filename-conflicts-with-directory/Makefile

@@ -0,0 +1,7 @@
+-include ../tools.mk
+
+all:
+	cp foo.rs $(TMPDIR)/foo.rs
+	mkdir $(TMPDIR)/foo
+	$(RUSTC) $(TMPDIR)/foo.rs -o $(TMPDIR)/foo 2>&1 \
+		| $(CGREP) -e "the generated executable for the input file \".*foo\.rs\" conflicts with the existing directory \".*foo\""

src/test/run-make/output-filename-conflicts-with-directory/foo.rs

@@ -0,0 +1,11 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn main() {}

src/test/run-make/output-filename-overwrites-input/Makefile

@@ -2,8 +2,11 @@
 
 all:
 	cp foo.rs $(TMPDIR)/foo
-	$(RUSTC) $(TMPDIR)/foo 2>&1 \
+	$(RUSTC) $(TMPDIR)/foo -o $(TMPDIR)/foo 2>&1 \
 		| $(CGREP) -e "the input file \".*foo\" would be overwritten by the generated executable"
+	cp bar.rs $(TMPDIR)/bar.rlib
+	$(RUSTC) $(TMPDIR)/bar.rlib -o $(TMPDIR)/bar.rlib 2>&1 \
+		| $(CGREP) -e "the input file \".*bar.rlib\" would be overwritten by the generated executable"
 	$(RUSTC) foo.rs 2>&1 && $(RUSTC) -Z ls $(TMPDIR)/foo 2>&1
 	cp foo.rs $(TMPDIR)/foo.rs
 	$(RUSTC) $(TMPDIR)/foo.rs -o $(TMPDIR)/foo.rs 2>&1 \

src/test/run-make/output-filename-overwrites-input/bar.rs

@@ -0,0 +1,11 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![crate_type = "lib"]

src/test/run-make/output-filename-overwrites-input/foo.rs

@@ -1,4 +1,4 @@
-// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
 // file at the top-level directory of this distribution and at
 // http://rust-lang.org/COPYRIGHT.
 //