Changes telemetry config name, adds telemetry version to log files, a…

…dds more telemetry tests.

src/rustup-cli/common.rs

@@ -1,12 +1,11 @@
 //! Just a dumping ground for cli stuff
 
 use rustup::{Cfg, Result, Notification, Toolchain, Error, UpdateStatus};
-use rustup_utils::{self, utils};
+use rustup_utils::utils;
 use rustup_utils::notify::NotificationLevel;
 use self_update;
-use std::ffi::OsStr;
 use std::io::{Write, Read, BufRead};
-use std::process::{self, Command};
+use std::process::Command;
 use std::{cmp, iter};
 use std::str::FromStr;
 use std;
@@ -122,30 +121,6 @@ pub fn set_globals(verbose: bool) -> Result<Cfg> {
 
 }
 
-pub fn run_inner<S: AsRef<OsStr>>(mut command: Command,
-                                  args: &[S]) -> Result<()> {
-    command.args(&args[1..]);
-    // FIXME rust-lang/rust#32254. It's not clear to me
-    // when and why this is needed.
-    command.stdin(process::Stdio::inherit());
-
-    let status = command.status();
-
-    match status {
-        Ok(status) => {
-            // Ensure correct exit code is returned
-            let code = status.code().unwrap_or(1);
-            process::exit(code);
-        }
-        Err(e) => {
-            Err(rustup_utils::Error::RunningCommand {
-                name: args[0].as_ref().to_owned(),
-                error: rustup_utils::raw::CommandError::Io(e),
-            }.into())
-        }
-    }
-}
-
 pub fn show_channel_update(cfg: &Cfg, name: &str,
                            updated: Result<UpdateStatus>) -> Result<()> {
     show_channel_updates(cfg, vec![(name.to_string(), updated)])

src/rustup-cli/main.rs

@@ -1,8 +1,4 @@
-<<<<<<< a5128e8b36acb156594bca2ef502bb66999cff7a:src/rustup-cli/main.rs
 extern crate rustup_dist;
-=======
-extern crate multirust_dist;
->>>>>>> Improving modularization, telemetry checks:src/multirust-cli/main.rs
 #[macro_use]
 extern crate rustup_utils;
 

src/rustup-cli/proxy_mode.rs

@@ -1,4 +1,4 @@
-use common::{run_inner, set_globals};
+use common::set_globals;
 use rustup::{Cfg, Result, Error};
 use rustup_utils::utils;
 use rustup::command::run_command_for_dir;

src/rustup-cli/rustup_mode.rs

@@ -1,6 +1,6 @@
 use clap::{App, Arg, AppSettings, SubCommand, ArgMatches};
 use common;
-use rustup::{Result, Cfg, Error, Toolchain};
+use rustup::{Result, Cfg, Error, Toolchain, command};
 use rustup_dist::manifest::Component;
 use rustup_dist::dist::TargetTriple;
 use rustup_utils::utils;
@@ -58,6 +58,7 @@ pub fn main() -> Result<()> {
                 (_ ,_) => unreachable!(),
             }
         }
+        ("telemetry", Some(m)) => try!(telemetry(&cfg, m)),
         (_, _) => unreachable!(),
     }
 
@@ -160,6 +161,14 @@ pub fn cli() -> App<'static, 'static> {
                      .short("y")))
             .subcommand(SubCommand::with_name("upgrade-data")
                 .about("Upgrade the internal data format.")))
+        .subcommand(SubCommand::with_name("telemetry")
+                .about("Enable or disable rust telemetry")
+                .arg(Arg::with_name("enabled")
+                        .hidden(true)
+                        .takes_value(true)
+                        .value_name("telemetry")
+                        .help("Set telemetry 'on' or 'off'")
+                        .possible_values(&["on", "off"])))
 }
 
 fn maybe_upgrade_data(cfg: &Cfg, m: &ArgMatches) -> Result<bool> {
@@ -228,7 +237,7 @@ fn run(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
     let args: Vec<_> = args.collect();
     let cmd = try!(cfg.create_command_for_toolchain(toolchain, args[0]));
 
-    common::run_inner(cmd, &args)
+    command::run_command_for_dir(cmd, &args, &cfg)
 }
 
 fn which(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
@@ -374,8 +383,6 @@ fn self_uninstall(m: &ArgMatches) -> Result<()> {
 
     self_update::uninstall(no_prompt)
 }
-<<<<<<< a5128e8b36acb156594bca2ef502bb66999cff7a:src/rustup-cli/rustup_mode.rs
-=======
 
 fn telemetry(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
     let telemetry_string = m.value_of("enabled").unwrap();
@@ -385,4 +392,3 @@ fn telemetry(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
         _ => Err(Error::Custom { id: "Telemetry".to_string(), desc: "Incorrect telemetry setting".to_string() }),
     }
 }
->>>>>>> Improving modularization, telemetry checks:src/multirust-cli/rustup_mode.rs

src/multirust/command.rs → src/rustup/command.rs

@@ -9,7 +9,7 @@ use regex::Regex;
 
 use Cfg;
 use errors::*;
-use multirust_utils;
+use rustup_utils;
 use telemetry::{Telemetry, TelemetryEvent};
 
 
@@ -97,9 +97,9 @@ fn telemetry_rustc<S: AsRef<OsStr>>(mut cmd: Command, args: &[S], cfg: &Cfg) ->
                                                 exit_code: exit_code,
                                                 errors: None };
             t.log_telemetry(te);
-            Err(multirust_utils::Error::RunningCommand {    
+            Err(rustup_utils::Error::RunningCommand {    
                 name: args[0].as_ref().to_owned(),
-                error: multirust_utils::raw::CommandError::Io(e),
+                error: rustup_utils::raw::CommandError::Io(e),
             }.into())
         },
     }
@@ -119,9 +119,9 @@ fn run_command_for_dir_without_telemetry<S: AsRef<OsStr>>(mut cmd: Command, args
             process::exit(code);
         }
         Err(e) => {
-            Err(multirust_utils::Error::RunningCommand {
+            Err(rustup_utils::Error::RunningCommand {
                 name: args[0].as_ref().to_owned(),
-                error: multirust_utils::raw::CommandError::Io(e),
+                error: rustup_utils::raw::CommandError::Io(e),
             }.into())
         }
     }    

src/rustup/config.rs

@@ -425,15 +425,15 @@ impl Cfg {
 
         try!(utils::write_file("temp", &work_file, ""));
 
-        try!(utils::rename_file("telemetry", &*work_file, &self.multirust_dir.join("telemetry_on")));
+        try!(utils::rename_file("telemetry", &*work_file, &self.multirust_dir.join("telemetry-on")));
 
         self.notify_handler.call(Notification::SetTelemetry("on"));
 
         Ok(())
     }
 
     fn disable_telemetry(&self) -> Result<()> {
-        let _ = utils::remove_file("telemetry_on", &self.multirust_dir.join("telemetry_on"));
+        let _ = utils::remove_file("telemetry-on", &self.multirust_dir.join("telemetry-on"));
 
         self.notify_handler.call(Notification::SetTelemetry("off"));
 
@@ -449,7 +449,7 @@ impl Cfg {
 
     fn find_telemetry(multirust_dir: &PathBuf) -> TelemetryMode {
         // default telemetry should be off - if no telemetry file is found, it's off
-        let telemetry_file = multirust_dir.join("telemetry_on");
+        let telemetry_file = multirust_dir.join("telemetry-on");
 
         if utils::is_file(telemetry_file) {
             return TelemetryMode::On;

src/multirust/telemetry.rs → src/rustup/telemetry.rs

@@ -1,5 +1,5 @@
 use time;
-use multirust_utils::utils;
+use rustup_utils::utils;
 use rustc_serialize::json;
 
 use std::collections::HashSet;
@@ -22,26 +22,31 @@ pub enum TelemetryEvent {
 struct LogMessage {
     log_time_s: i64,
     event: TelemetryEvent,
+    version: i32,
 }
 
 #[derive(Debug)]
 pub struct Telemetry {
     telemetry_dir: PathBuf
 }
 
+const LOG_FILE_VERSION: i32 = 1;
+
 impl Telemetry {
     pub fn new(telemetry_dir: PathBuf) -> Telemetry {
         Telemetry { telemetry_dir: telemetry_dir }
-    }   
+    }
 
     pub fn log_telemetry(&self, event: TelemetryEvent) {
         let current_time = time::now_utc();
-        let ln = LogMessage { log_time_s: current_time.to_timespec().sec, event: event };
+        let ln = LogMessage { log_time_s: current_time.to_timespec().sec,
+                              event: event,
+                              version: LOG_FILE_VERSION };
 
         let json = json::encode(&ln).unwrap();
 
         let filename = format!("log-{}-{:02}-{:02}.json", current_time.tm_year + 1900, current_time.tm_mon + 1, current_time.tm_mday);
 
         let _ = utils::append_file("telemetry", &self.telemetry_dir.join(&filename), &json);
     }
-}
+}

src/rustup/toolchain.rs

@@ -146,9 +146,9 @@ impl<'a> Toolchain<'a> {
     }
 
     pub fn install_from_dist(&self) -> Result<UpdateStatus> {
-        // if self.cfg.telemetry_enabled() {
-        //     return self.install_from_dist_with_telemetry();
-        // }
+        if self.cfg.telemetry_enabled() {
+            return self.install_from_dist_with_telemetry();
+        }
         self.install_from_dist_inner()
     }
 

tests/cli-exact.rs

@@ -118,15 +118,15 @@ fn list_overrides() {
     setup(&|config| {
         let cwd = std::fs::canonicalize(env::current_dir().unwrap()).unwrap();
         let mut cwd_formatted = format!("{}", cwd.display()).to_string();
-        
+
         if cfg!(windows) {
             cwd_formatted = cwd_formatted[4..].to_owned();
         }
-        
+
         let trip = this_host_triple();
         expect_ok(config, &["rustup", "override", "add", "nightly"]);
-        expect_ok_ex(config, &["rustup", "override", "list"], 
-                     &format!("{:<40}\t{:<20}\n", cwd_formatted, &format!("nightly-{}", trip)), r""); 
+        expect_ok_ex(config, &["rustup", "override", "list"],
+                     &format!("{:<40}\t{:<20}\n", cwd_formatted, &format!("nightly-{}", trip)), r"");
     });
 }
 
@@ -200,7 +200,7 @@ info: installing component 'rust-std' for '{0}'
 #[test]
 fn enable_telemetry() {
     setup(&|config| {
-        expect_ok_ex(config, 
+        expect_ok_ex(config,
                      &["rustup", "telemetry", "on"],
                      r"",
                      &format!("info: telemetry set to 'on'\n"));
@@ -210,9 +210,9 @@ fn enable_telemetry() {
 #[test]
 fn disable_telemetry() {
     setup(&|config| {
-        expect_ok_ex(config, 
+        expect_ok_ex(config,
                      &["rustup", "telemetry", "off"],
                      r"",
                      &format!("info: telemetry set to 'off'\n"));
     });
-}
+}

tests/cli-misc.rs

@@ -9,6 +9,7 @@ use rustup_mock::clitools::{self, Config, Scenario,
                                expect_stdout_ok, expect_stderr_ok,
                                expect_ok, expect_err, run,
                                this_host_triple};
+use rustup_utils::utils;
 
 macro_rules! for_host { ($s: expr) => (&format!($s, this_host_triple())) }
 
@@ -283,3 +284,21 @@ fn proxies_pass_empty_args() {
         expect_ok(config, &["rustup", "run", "nightly", "rustc", "--empty-arg-test", ""]);
     });
 }
+
+#[test]
+fn enabling_telemetry_and_compiling_creates_log() {
+    setup(&|config| {
+        expect_ok(config, &["rustup", "default", "stable"]);
+        expect_ok(config, &["rustup", "telemetry", "on"]);
+        expect_ok(config, &["rustc", "--version"]);
+
+        let telemetry_dir = config.rustupdir.join("telemetry");
+        let _ = utils::assert_is_directory(telemetry_dir.as_path());
+
+        let out = telemetry_dir.read_dir();
+        assert!(out.is_ok());
+
+        let contents = out.unwrap();
+        assert!(contents.count() > 0);
+    });
+}