Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustc: rename -Z emit-directives to -Z emit-artifact-notifications and simplify the output. #60464

Merged
merged 5 commits into from
May 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1462,8 +1462,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
the same values as the target option of the same name"),
allow_features: Option<Vec<String>> = (None, parse_opt_comma_list, [TRACKED],
"only allow the listed language features to be enabled in code (space separated)"),
emit_directives: bool = (false, parse_bool, [UNTRACKED],
"emit build directives if producing JSON output"),
emit_artifact_notifications: bool = (false, parse_bool, [UNTRACKED],
"emit notifications after each artifact has been output (only in the JSON format)"),
}

pub fn default_lib_output() -> CrateType {
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_codegen_ssa/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>(sess: &'a Session,
);
}
}
if sess.opts.debugging_opts.emit_artifact_notifications {
sess.parse_sess.span_diagnostic.emit_artifact_notification(&out_filename);
}
}

if sess.opts.cg.save_temps {
Expand Down
8 changes: 5 additions & 3 deletions src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use std::borrow::Cow;
use std::io::prelude::*;
use std::io;
use std::cmp::{min, Reverse};
use std::path::Path;
use termcolor::{StandardStream, ColorChoice, ColorSpec, BufferWriter, Ansi};
use termcolor::{WriteColor, Color, Buffer};

Expand Down Expand Up @@ -52,9 +53,10 @@ pub trait Emitter {
/// Emit a structured diagnostic.
fn emit_diagnostic(&mut self, db: &DiagnosticBuilder<'_>);

/// Emit a JSON directive. The default is to do nothing; this should only
/// be emitted with --error-format=json.
fn maybe_emit_json_directive(&mut self, _directive: String) {}
/// Emit a notification that an artifact has been output.
/// This is currently only supported for the JSON format,
/// other formats can, and will, simply ignore it.
fn emit_artifact_notification(&mut self, _path: &Path) {}

/// Checks if should show explanations about "rustc --explain"
fn should_show_explain(&self) -> bool {
Expand Down
18 changes: 6 additions & 12 deletions src/librustc_errors/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use std::borrow::Cow;
use std::cell::Cell;
use std::{error, fmt};
use std::panic;
use std::path::Path;

use termcolor::{ColorSpec, Color};

Expand Down Expand Up @@ -294,16 +295,9 @@ impl error::Error for ExplicitBug {
pub use diagnostic::{Diagnostic, SubDiagnostic, DiagnosticStyledString, DiagnosticId};
pub use diagnostic_builder::DiagnosticBuilder;

/// A handler deals with two kinds of compiler output.
/// - Errors: certain errors (fatal, bug, unimpl) may cause immediate exit,
/// others log errors for later reporting.
/// - Directives: with --error-format=json, the compiler produces directives
/// that indicate when certain actions have completed, which are useful for
/// Cargo. They may change at any time and should not be considered a public
/// API.
///
/// This crate's name (rustc_errors) doesn't encompass the directives, because
/// directives were added much later.
/// A handler deals with errors and other compiler output.
/// Certain errors (fatal, bug, unimpl) may cause immediate exit,
/// others log errors for later reporting.
pub struct Handler {
pub flags: HandlerFlags,

Expand Down Expand Up @@ -775,8 +769,8 @@ impl Handler {
}
}

pub fn maybe_emit_json_directive(&self, directive: String) {
self.emitter.borrow_mut().maybe_emit_json_directive(directive);
pub fn emit_artifact_notification(&self, path: &Path) {
self.emitter.borrow_mut().emit_artifact_notification(path);
}
}

Expand Down
13 changes: 5 additions & 8 deletions src/librustc_interface/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1048,14 +1048,11 @@ fn encode_and_write_metadata<'tcx>(
tcx.sess.fatal(&format!("couldn't create a temp dir: {}", err))
});
let metadata_filename = emit_metadata(tcx.sess, &metadata, &metadata_tmpdir);
match std::fs::rename(&metadata_filename, &out_filename) {
Ok(_) => {
if tcx.sess.opts.debugging_opts.emit_directives {
tcx.sess.parse_sess.span_diagnostic.maybe_emit_json_directive(
format!("metadata file written: {}", out_filename.display()));
}
}
Err(e) => tcx.sess.fatal(&format!("failed to write {}: {}", out_filename.display(), e)),
if let Err(e) = fs::rename(&metadata_filename, &out_filename) {
tcx.sess.fatal(&format!("failed to write {}: {}", out_filename.display(), e));
}
if tcx.sess.opts.debugging_opts.emit_artifact_notifications {
tcx.sess.parse_sess.span_diagnostic.emit_artifact_notification(&out_filename);
}
}

Expand Down
8 changes: 7 additions & 1 deletion src/libserialize/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -764,12 +764,18 @@ macro_rules! tuple {

tuple! { T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, }

impl Encodable for path::PathBuf {
impl Encodable for path::Path {
fn encode<S: Encoder>(&self, e: &mut S) -> Result<(), S::Error> {
self.to_str().unwrap().encode(e)
}
}

impl Encodable for path::PathBuf {
fn encode<S: Encoder>(&self, e: &mut S) -> Result<(), S::Error> {
path::Path::encode(self, e)
}
}

impl Decodable for path::PathBuf {
fn decode<D: Decoder>(d: &mut D) -> Result<path::PathBuf, D::Error> {
let bytes: String = Decodable::decode(d)?;
Expand Down
13 changes: 7 additions & 6 deletions src/libsyntax/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use errors::emitter::{Emitter, HumanReadableErrorType};
use syntax_pos::{MacroBacktrace, Span, SpanLabel, MultiSpan};
use rustc_data_structures::sync::{self, Lrc};
use std::io::{self, Write};
use std::path::Path;
use std::vec;
use std::sync::{Arc, Mutex};

Expand Down Expand Up @@ -91,15 +92,15 @@ impl Emitter for JsonEmitter {
}
}

fn maybe_emit_json_directive(&mut self, directive: String) {
let data = Directive { directive };
fn emit_artifact_notification(&mut self, path: &Path) {
let data = ArtifactNotification { artifact: path };
let result = if self.pretty {
writeln!(&mut self.dst, "{}", as_pretty_json(&data))
} else {
writeln!(&mut self.dst, "{}", as_json(&data))
};
if let Err(e) = result {
panic!("failed to print message: {:?}", e);
panic!("failed to print notification: {:?}", e);
}
}
}
Expand Down Expand Up @@ -181,9 +182,9 @@ struct DiagnosticCode {
}

#[derive(RustcEncodable)]
struct Directive {
/// The directive itself.
directive: String,
struct ArtifactNotification<'a> {
/// The path of the artifact.
artifact: &'a Path,
}

impl Diagnostic {
Expand Down
2 changes: 0 additions & 2 deletions src/test/ui/consts/const-eval/unused-broken-const.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
warning: due to multiple output types requested, the explicitly specified output file name will be adapted for each output type

error: any use of this value will cause an error
--> $DIR/unused-broken-const.rs:5:18
|
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/emit-artifact-notifications.nll.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"artifact":"$TEST_BUILD_DIR/emit-artifact-notifications.nll/libemit_artifact_notifications.rmeta"}
6 changes: 6 additions & 0 deletions src/test/ui/emit-artifact-notifications.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// compile-flags:--emit=metadata --error-format=json -Z emit-artifact-notifications
// compile-pass

// A very basic test for the emission of artifact notifications in JSON output.

fn main() {}
1 change: 1 addition & 0 deletions src/test/ui/emit-artifact-notifications.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"artifact":"$TEST_BUILD_DIR/emit-artifact-notifications/libemit_artifact_notifications.rmeta"}
12 changes: 0 additions & 12 deletions src/test/ui/emit-directives.rs

This file was deleted.

1 change: 0 additions & 1 deletion src/test/ui/emit-directives.stderr

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// compile-pass

// FIXME(eddyb) shorten the name so windows doesn't choke on it.
#![crate_name = "trait_test"]

// Regression test related to #56288. Check that a supertrait projection (of
// `Output`) that references `Self` is ok if there is another occurence of
// the same supertrait that specifies the projection explicitly, even if
Expand Down
10 changes: 5 additions & 5 deletions src/tools/compiletest/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use crate::errors::{Error, ErrorKind};
use crate::runtest::ProcRes;
use serde_json;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::str::FromStr;

#[derive(Deserialize)]
Expand All @@ -18,9 +18,9 @@ struct Diagnostic {
}

#[derive(Deserialize)]
struct Directive {
struct ArtifactNotification {
#[allow(dead_code)]
directive: String,
artifact: PathBuf,
}

#[derive(Deserialize, Clone)]
Expand Down Expand Up @@ -75,8 +75,8 @@ pub fn extract_rendered(output: &str) -> String {
if line.starts_with('{') {
if let Ok(diagnostic) = serde_json::from_str::<Diagnostic>(line) {
diagnostic.rendered
} else if let Ok(_directive) = serde_json::from_str::<Directive>(line) {
// Swallow the directive.
} else if let Ok(_) = serde_json::from_str::<ArtifactNotification>(line) {
// Ignore the notification.
None
} else {
print!(
Expand Down
58 changes: 36 additions & 22 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1422,10 +1422,21 @@ impl<'test> TestCx<'test> {
}

fn compile_test(&self) -> ProcRes {
let mut rustc = self.make_compile_args(
&self.testpaths.file,
TargetLocation::ThisFile(self.make_exe_name()),
);
// Only use `make_exe_name` when the test ends up being executed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did these changes come from and/or how are they related to the PR's title?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the PR description - they remove the need for normalization, by fixing the compiletest bugs that affected #60006. IMO, the resulting tests are more clearly correct.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok, sorry for missing that!

let will_execute = match self.config.mode {
RunPass | Ui => self.should_run_successfully(),
Incremental => self.revision.unwrap().starts_with("r"),
RunFail | RunPassValgrind | MirOpt |
DebugInfoBoth | DebugInfoGdb | DebugInfoLldb => true,
_ => false,
};
let output_file = if will_execute {
TargetLocation::ThisFile(self.make_exe_name())
} else {
TargetLocation::ThisDirectory(self.output_base_dir())
};

let mut rustc = self.make_compile_args(&self.testpaths.file, output_file);

rustc.arg("-L").arg(&self.aux_output_dir_name());

Expand Down Expand Up @@ -1882,7 +1893,12 @@ impl<'test> TestCx<'test> {
rustc.arg("-o").arg(path);
}
TargetLocation::ThisDirectory(path) => {
rustc.arg("--out-dir").arg(path);
if is_rustdoc {
// `rustdoc` uses `-o` for the output directory.
rustc.arg("-o").arg(path);
} else {
rustc.arg("--out-dir").arg(path);
}
}
}

Expand Down Expand Up @@ -3138,42 +3154,40 @@ impl<'test> TestCx<'test> {
}

fn normalize_output(&self, output: &str, custom_rules: &[(String, String)]) -> String {
let parent_dir = self.testpaths.file.parent().unwrap();
let cflags = self.props.compile_flags.join(" ");
let json = cflags.contains("--error-format json")
|| cflags.contains("--error-format pretty-json")
|| cflags.contains("--error-format=json")
|| cflags.contains("--error-format=pretty-json");
let parent_dir_str = if json {
parent_dir.display().to_string().replace("\\", "\\\\")
} else {
parent_dir.display().to_string()

let mut normalized = output.to_string();

let mut normalize_path = |from: &Path, to: &str| {
let mut from = from.display().to_string();
if json {
from = from.replace("\\", "\\\\");
}
normalized = normalized.replace(&from, to);
};

let mut normalized = output.replace(&parent_dir_str, "$DIR");
let parent_dir = self.testpaths.file.parent().unwrap();
normalize_path(parent_dir, "$DIR");

// Paths into the libstd/libcore
let src_dir = self.config.src_base.parent().unwrap().parent().unwrap();
let src_dir_str = if json {
src_dir.display().to_string().replace("\\", "\\\\")
} else {
src_dir.display().to_string()
};
normalized = normalized.replace(&src_dir_str, "$SRC_DIR");
normalize_path(src_dir, "$SRC_DIR");

// Paths into the build directory
let test_build_dir = &self.config.build_base;
let parent_build_dir = test_build_dir.parent().unwrap().parent().unwrap().parent().unwrap();

// eg. /home/user/rust/build/x86_64-unknown-linux-gnu/test/ui
normalized = normalized.replace(test_build_dir.to_str().unwrap(), "$TEST_BUILD_DIR");
normalize_path(test_build_dir, "$TEST_BUILD_DIR");
// eg. /home/user/rust/build
normalized = normalized.replace(&parent_build_dir.to_str().unwrap(), "$BUILD_DIR");
normalize_path(parent_build_dir, "$BUILD_DIR");

// Paths into lib directory.
let mut lib_dir = parent_build_dir.parent().unwrap().to_path_buf();
lib_dir.push("lib");
normalized = normalized.replace(&lib_dir.to_str().unwrap(), "$LIB_DIR");
normalize_path(&parent_build_dir.parent().unwrap().join("lib"), "$LIB_DIR");

if json {
// escaped newlines in json strings should be readable
Expand Down