From 51b438499a646dbe8bf0080eb8c1f45c323c7ab8 Mon Sep 17 00:00:00 2001 From: Jeremiah Peschka Date: Mon, 18 Apr 2016 09:01:04 -0400 Subject: [PATCH] Changes telemetry config name, adds telemetry version to log files, adds more telemetry tests. --- Cargo.lock | 7 +------ src/rustup-cli/common.rs | 29 ++------------------------ src/rustup-cli/main.rs | 4 ---- src/rustup-cli/proxy_mode.rs | 2 +- src/rustup-cli/rustup_mode.rs | 16 +++++++++----- src/{multirust => rustup}/command.rs | 10 ++++----- src/rustup/config.rs | 6 +++--- src/{multirust => rustup}/telemetry.rs | 13 ++++++++---- src/rustup/toolchain.rs | 6 +++--- tests/cli-exact.rs | 14 ++++++------- tests/cli-misc.rs | 19 +++++++++++++++++ 11 files changed, 61 insertions(+), 65 deletions(-) rename src/{multirust => rustup}/command.rs (93%) rename src/{multirust => rustup}/telemetry.rs (86%) diff --git a/Cargo.lock b/Cargo.lock index 828e461a98..9118d35890 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10,16 +10,11 @@ dependencies = [ "libc 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)", "openssl 0.7.10 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", - "regex 0.1.65 (registry+https://github.com/rust-lang/crates.io-index)", - "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", -<<<<<<< a5128e8b36acb156594bca2ef502bb66999cff7a - "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", "regex 0.1.66 (registry+https://github.com/rust-lang/crates.io-index)", + "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", "rustup-dist 0.1.7", "rustup-mock 0.1.7", "rustup-utils 0.1.7", -======= ->>>>>>> Improving modularization, telemetry checks "scopeguard 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "tempdir 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)", "term 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/src/rustup-cli/common.rs b/src/rustup-cli/common.rs index f32780458d..3560872dc8 100644 --- a/src/rustup-cli/common.rs +++ b/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 { } -pub fn run_inner>(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) -> Result<()> { show_channel_updates(cfg, vec![(name.to_string(), updated)]) diff --git a/src/rustup-cli/main.rs b/src/rustup-cli/main.rs index 1d9503a4b4..5f8a7f0c6d 100644 --- a/src/rustup-cli/main.rs +++ b/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; diff --git a/src/rustup-cli/proxy_mode.rs b/src/rustup-cli/proxy_mode.rs index fa6169ecb1..6420a45fee 100644 --- a/src/rustup-cli/proxy_mode.rs +++ b/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; diff --git a/src/rustup-cli/rustup_mode.rs b/src/rustup-cli/rustup_mode.rs index 555d2d4b2c..2cbd191d4d 100644 --- a/src/rustup-cli/rustup_mode.rs +++ b/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 { @@ -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 diff --git a/src/multirust/command.rs b/src/rustup/command.rs similarity index 93% rename from src/multirust/command.rs rename to src/rustup/command.rs index 8d113a33b0..17be04f26a 100644 --- a/src/multirust/command.rs +++ b/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>(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>(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()) } } diff --git a/src/rustup/config.rs b/src/rustup/config.rs index 4ed27b3d5a..bb6148d2a4 100644 --- a/src/rustup/config.rs +++ b/src/rustup/config.rs @@ -425,7 +425,7 @@ 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")); @@ -433,7 +433,7 @@ impl Cfg { } 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; diff --git a/src/multirust/telemetry.rs b/src/rustup/telemetry.rs similarity index 86% rename from src/multirust/telemetry.rs rename to src/rustup/telemetry.rs index 9915a1701a..e2ab1d070b 100644 --- a/src/multirust/telemetry.rs +++ b/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,6 +22,7 @@ pub enum TelemetryEvent { struct LogMessage { log_time_s: i64, event: TelemetryEvent, + version: i32, } #[derive(Debug)] @@ -29,14 +30,18 @@ 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(); @@ -44,4 +49,4 @@ impl Telemetry { let _ = utils::append_file("telemetry", &self.telemetry_dir.join(&filename), &json); } -} \ No newline at end of file +} diff --git a/src/rustup/toolchain.rs b/src/rustup/toolchain.rs index f5d79d2657..b733b566c5 100644 --- a/src/rustup/toolchain.rs +++ b/src/rustup/toolchain.rs @@ -146,9 +146,9 @@ impl<'a> Toolchain<'a> { } pub fn install_from_dist(&self) -> Result { - // 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() } diff --git a/tests/cli-exact.rs b/tests/cli-exact.rs index 2be1483c1c..b4293b357c 100644 --- a/tests/cli-exact.rs +++ b/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")); }); -} \ No newline at end of file +} diff --git a/tests/cli-misc.rs b/tests/cli-misc.rs index c057c42260..83f89bd374 100644 --- a/tests/cli-misc.rs +++ b/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); + }); +}