Auto merge of #5333 - dwijnand:warn-install-2018, r=matklad

Warn/error when cargo installing the cwd in 2015/2018

Fixes #5327

submitted for early review. feedback very welcome, I'm happy to iterate (and learn).

Author: bors

src/bin/commands/install.rs

@@ -79,6 +79,8 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
         .unwrap_or_default()
         .collect::<Vec<_>>();
 
+    let mut from_cwd = false;
+
     let source = if let Some(url) = args.value_of("git") {
         let url = url.to_url()?;
         let gitref = if let Some(branch) = args.value_of("branch") {
@@ -94,6 +96,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     } else if let Some(path) = args.value_of_path("path", config) {
         SourceId::for_path(&path)?
     } else if krates.is_empty() {
+        from_cwd = true;
         SourceId::for_path(config.cwd())?
     } else {
         SourceId::crates_io(config)?
@@ -109,6 +112,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
             root,
             krates,
             &source,
+            from_cwd,
             version,
             &compile_opts,
             args.is_present("force"),

src/cargo/core/source/source_id.rs

@@ -36,7 +36,7 @@ struct SourceIdInner {
     name: Option<String>,
 }
 
-/// The possible kinds of code source. Along with a URL, this fully defines the
+/// The possible kinds of code source. Along with SourceIdInner this fully defines the
 /// source
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 enum Kind {

src/cargo/ops/cargo_install.rs

@@ -10,7 +10,7 @@ use semver::{Version, VersionReq};
 use tempfile::Builder as TempFileBuilder;
 use toml;
 
-use core::{Dependency, Package, PackageIdSpec, Source, SourceId};
+use core::{Dependency, Edition, Package, PackageIdSpec, Source, SourceId};
 use core::{PackageId, Workspace};
 use ops::{self, CompileFilter, DefaultExecutor};
 use sources::{GitSource, PathSource, SourceConfigMap};
@@ -57,6 +57,7 @@ pub fn install(
     root: Option<&str>,
     krates: Vec<&str>,
     source_id: &SourceId,
+    from_cwd: bool,
     vers: Option<&str>,
     opts: &ops::CompileOptions,
     force: bool,
@@ -70,6 +71,7 @@ pub fn install(
             &map,
             krates.into_iter().next(),
             source_id,
+            from_cwd,
             vers,
             opts,
             force,
@@ -88,6 +90,7 @@ pub fn install(
                 &map,
                 Some(krate),
                 source_id,
+                from_cwd,
                 vers,
                 opts,
                 force,
@@ -149,6 +152,7 @@ fn install_one(
     map: &SourceConfigMap,
     krate: Option<&str>,
     source_id: &SourceId,
+    from_cwd: bool,
     vers: Option<&str>,
     opts: &ops::CompileOptions,
     force: bool,
@@ -229,6 +233,23 @@ fn install_one(
     };
     let pkg = ws.current()?;
 
+    if from_cwd {
+        match pkg.manifest().edition() {
+            Edition::Edition2015 =>
+                config.shell().warn("To build the current package use `cargo build`, to install the current package run `cargo install --path .`")?
+            ,
+            Edition::Edition2018 =>
+                bail!(
+                    "To build the current package use `cargo build`, \
+                     to install the current package run `cargo install --path .`, \
+                     otherwise specify a crate to install from \
+                     crates.io, or use --path or --git to \
+                     specify alternate source"
+                )
+            ,
+        }
+    };
+
     config.shell().status("Installing", pkg)?;
 
     // Preflight checks to check up front whether we'll overwrite something.

tests/testsuite/install.rs

@@ -3,6 +3,7 @@ use std::fs::{self, File, OpenOptions};
 use std::io::prelude::*;
 
 use cargo::util::ProcessBuilder;
+use cargotest::ChannelChanger;
 use cargotest::install::{cargo_home, has_installed_exe};
 use cargotest::support::git;
 use cargotest::support::paths;
@@ -1008,11 +1009,50 @@ fn installs_from_cwd_by_default() {
 
     assert_that(
         cargo_process("install").cwd(p.root()),
-        execs().with_status(0),
+        execs().with_status(0).with_stderr_contains(
+                "\
+warning: To build the current package use `cargo build`, to install the current package run `cargo install --path .`
+",
+        ),
     );
     assert_that(cargo_home(), has_installed_exe("foo"));
 }
 
+#[test]
+fn installs_from_cwd_with_2018_warnings() {
+    if !cargotest::is_nightly() {
+        // Stable rust won't have the edition option.  Remove this once it
+        // is stabilized.
+        return;
+    }
+    let p = project("foo")
+        .file(
+            "Cargo.toml",
+            r#"
+            cargo-features = ["edition"]
+
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+            rust = "2018"
+        "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    assert_that(
+        cargo_process("install").cwd(p.root()).masquerade_as_nightly_cargo(),
+        execs().with_status(101).with_stderr_contains(
+            "error: To build the current package use `cargo build`, \
+             to install the current package run `cargo install --path .`, \
+             otherwise specify a crate to install from crates.io, \
+             or use --path or --git to specify alternate source\
+        "),
+    );
+    assert_that(cargo_home(), is_not(has_installed_exe("foo")));
+}
+
 #[test]
 fn do_not_rebuilds_on_local_install() {
     let p = project("foo")

tests/testsuite/required_features.rs

@@ -742,12 +742,12 @@ fn install_default_features() {
         .file("examples/foo.rs", "fn main() {}")
         .build();
 
-    assert_that(p.cargo("install"), execs().with_status(0));
+    assert_that(p.cargo("install --path ."), execs().with_status(0));
     assert_that(cargo_home(), has_installed_exe("foo"));
     assert_that(p.cargo("uninstall").arg("foo"), execs().with_status(0));
 
     assert_that(
-        p.cargo("install").arg("--no-default-features"),
+        p.cargo("install --path .").arg("--no-default-features"),
         execs().with_status(101).with_stderr(format!(
             "\
 [INSTALLING] foo v0.0.1 ([..])
@@ -758,12 +758,12 @@ fn install_default_features() {
     );
     assert_that(cargo_home(), is_not(has_installed_exe("foo")));
 
-    assert_that(p.cargo("install").arg("--bin=foo"), execs().with_status(0));
+    assert_that(p.cargo("install --path .").arg("--bin=foo"), execs().with_status(0));
     assert_that(cargo_home(), has_installed_exe("foo"));
     assert_that(p.cargo("uninstall").arg("foo"), execs().with_status(0));
 
     assert_that(
-        p.cargo("install")
+        p.cargo("install --path .")
             .arg("--bin=foo")
             .arg("--no-default-features"),
         execs().with_status(101).with_stderr(format!(
@@ -781,14 +781,14 @@ Consider enabling them by passing e.g. `--features=\"a\"`
     assert_that(cargo_home(), is_not(has_installed_exe("foo")));
 
     assert_that(
-        p.cargo("install").arg("--example=foo"),
+        p.cargo("install --path .").arg("--example=foo"),
         execs().with_status(0),
     );
     assert_that(cargo_home(), has_installed_exe("foo"));
     assert_that(p.cargo("uninstall").arg("foo"), execs().with_status(0));
 
     assert_that(
-        p.cargo("install")
+        p.cargo("install --path .")
             .arg("--example=foo")
             .arg("--no-default-features"),
         execs().with_status(101).with_stderr(format!(
@@ -868,21 +868,21 @@ fn install_multiple_required_features() {
         .file("src/foo_2.rs", "fn main() {}")
         .build();
 
-    assert_that(p.cargo("install"), execs().with_status(0));
+    assert_that(p.cargo("install --path ."), execs().with_status(0));
     assert_that(cargo_home(), is_not(has_installed_exe("foo_1")));
     assert_that(cargo_home(), has_installed_exe("foo_2"));
     assert_that(p.cargo("uninstall").arg("foo"), execs().with_status(0));
 
     assert_that(
-        p.cargo("install").arg("--features").arg("c"),
+        p.cargo("install --path .").arg("--features").arg("c"),
         execs().with_status(0),
     );
     assert_that(cargo_home(), has_installed_exe("foo_1"));
     assert_that(cargo_home(), has_installed_exe("foo_2"));
     assert_that(p.cargo("uninstall").arg("foo"), execs().with_status(0));
 
     assert_that(
-        p.cargo("install").arg("--no-default-features"),
+        p.cargo("install --path .").arg("--no-default-features"),
         execs().with_status(101).with_stderr(
             "\
 [INSTALLING] foo v0.0.1 ([..])
@@ -1166,7 +1166,7 @@ Consider enabling them by passing e.g. `--features=\"bar/a\"`
 
     // install
     assert_that(
-        p.cargo("install"),
+        p.cargo("install --path ."),
         execs().with_status(101).with_stderr(format!(
             "\
 [INSTALLING] foo v0.0.1 ([..])