Rollup merge of rust-lang#147898 - Zalathar:aux-props, r=jieyouxu

compiletest: Move `AuxProps` out of `EarlyProps`

The primary purpose of `EarlyProps` is to discover revisions, so that we can create a separate test structure for each revision.

Revisions can (and do) have different auxiliaries, and up-to-date checking is already done per-revision, so it makes more sense to perform up-to-date checks based on the current revisions's auxiliaries only.

r? jieyouxu

Author: Zalathar

src/tools/compiletest/src/directives.rs

@@ -8,7 +8,8 @@ use tracing::*;
 
 use crate::common::{CodegenBackend, Config, Debugger, FailMode, PassMode, RunFailMode, TestMode};
 use crate::debuggers::{extract_cdb_version, extract_gdb_version};
-use crate::directives::auxiliary::{AuxProps, parse_and_update_aux};
+pub(crate) use crate::directives::auxiliary::AuxProps;
+use crate::directives::auxiliary::parse_and_update_aux;
 use crate::directives::directive_names::{
     KNOWN_DIRECTIVE_NAMES, KNOWN_HTMLDOCCK_DIRECTIVE_NAMES, KNOWN_JSONDOCCK_DIRECTIVE_NAMES,
 };
@@ -21,7 +22,7 @@ use crate::executor::{CollectedTestDesc, ShouldPanic};
 use crate::util::static_regex;
 use crate::{fatal, help};
 
-pub(crate) mod auxiliary;
+mod auxiliary;
 mod cfg;
 mod directive_names;
 mod file;
@@ -44,10 +45,6 @@ impl DirectivesCache {
 /// the test.
 #[derive(Default)]
 pub(crate) struct EarlyProps {
-    /// Auxiliary crates that should be built and made available to this test.
-    /// Included in [`EarlyProps`] so that the indicated files can participate
-    /// in up-to-date checking. Building happens via [`TestProps::aux`] instead.
-    pub(crate) aux: AuxProps,
     pub(crate) revisions: Vec<String>,
 }
 
@@ -66,7 +63,6 @@ impl EarlyProps {
             file_directives,
             // (dummy comment to force args into vertical layout)
             &mut |ln: &DirectiveLine<'_>| {
-                parse_and_update_aux(config, ln, &mut props.aux);
                 config.parse_and_update_revisions(ln, &mut props.revisions);
             },
         );
@@ -1310,6 +1306,7 @@ pub(crate) fn make_test_description(
     file_directives: &FileDirectives<'_>,
     test_revision: Option<&str>,
     poisoned: &mut bool,
+    aux_props: &mut AuxProps,
 ) -> CollectedTestDesc {
     let mut ignore = false;
     let mut ignore_message = None;
@@ -1327,6 +1324,9 @@ pub(crate) fn make_test_description(
                 return;
             }
 
+            // Parse `aux-*` directives, for use by up-to-date checks.
+            parse_and_update_aux(config, ln, aux_props);
+
             macro_rules! decision {
                 ($e:expr) => {
                     match $e {

src/tools/compiletest/src/directives/tests.rs

@@ -3,8 +3,9 @@ use semver::Version;
 
 use crate::common::{Config, Debugger, TestMode};
 use crate::directives::{
-    DirectivesCache, EarlyProps, Edition, EditionRange, FileDirectives, extract_llvm_version,
-    extract_version_range, iter_directives, line_directive, parse_edition, parse_normalize_rule,
+    AuxProps, DirectivesCache, EarlyProps, Edition, EditionRange, FileDirectives,
+    extract_llvm_version, extract_version_range, iter_directives, line_directive, parse_edition,
+    parse_normalize_rule,
 };
 use crate::executor::{CollectedTestDesc, ShouldPanic};
 
@@ -20,6 +21,7 @@ fn make_test_description(
     let mut poisoned = false;
     let file_directives = FileDirectives::from_file_contents(path, file_contents);
 
+    let mut aux_props = AuxProps::default();
     let test = crate::directives::make_test_description(
         config,
         &cache,
@@ -29,6 +31,7 @@ fn make_test_description(
         &file_directives,
         revision,
         &mut poisoned,
+        &mut aux_props,
     );
     if poisoned {
         panic!("poisoned!");
@@ -225,7 +228,7 @@ fn cfg() -> ConfigBuilder {
     ConfigBuilder::default()
 }
 
-fn parse_rs(config: &Config, contents: &str) -> EarlyProps {
+fn parse_early_props(config: &Config, contents: &str) -> EarlyProps {
     let file_directives = FileDirectives::from_file_contents(Utf8Path::new("a.rs"), contents);
     EarlyProps::from_file_directives(config, &file_directives)
 }
@@ -253,25 +256,7 @@ fn should_fail() {
 fn revisions() {
     let config: Config = cfg().build();
 
-    assert_eq!(parse_rs(&config, "//@ revisions: a b c").revisions, vec!["a", "b", "c"],);
-}
-
-#[test]
-fn aux_build() {
-    let config: Config = cfg().build();
-
-    assert_eq!(
-        parse_rs(
-            &config,
-            r"
-        //@ aux-build: a.rs
-        //@ aux-build: b.rs
-        "
-        )
-        .aux
-        .builds,
-        vec!["a.rs", "b.rs"],
-    );
+    assert_eq!(parse_early_props(&config, "//@ revisions: a b c").revisions, vec!["a", "b", "c"],);
 }
 
 #[test]
@@ -550,7 +535,7 @@ fn test_extract_version_range() {
 #[should_panic(expected = "duplicate revision: `rpass1` in line ` rpass1 rpass1`")]
 fn test_duplicate_revisions() {
     let config: Config = cfg().build();
-    parse_rs(&config, "//@ revisions: rpass1 rpass1");
+    parse_early_props(&config, "//@ revisions: rpass1 rpass1");
 }
 
 #[test]
@@ -559,14 +544,14 @@ fn test_duplicate_revisions() {
 )]
 fn test_assembly_mode_forbidden_revisions() {
     let config = cfg().mode("assembly").build();
-    parse_rs(&config, "//@ revisions: CHECK");
+    parse_early_props(&config, "//@ revisions: CHECK");
 }
 
 #[test]
 #[should_panic(expected = "revision name `true` is not permitted")]
 fn test_forbidden_revisions() {
     let config = cfg().mode("ui").build();
-    parse_rs(&config, "//@ revisions: true");
+    parse_early_props(&config, "//@ revisions: true");
 }
 
 #[test]
@@ -575,7 +560,7 @@ fn test_forbidden_revisions() {
 )]
 fn test_codegen_mode_forbidden_revisions() {
     let config = cfg().mode("codegen").build();
-    parse_rs(&config, "//@ revisions: CHECK");
+    parse_early_props(&config, "//@ revisions: CHECK");
 }
 
 #[test]
@@ -584,7 +569,7 @@ fn test_codegen_mode_forbidden_revisions() {
 )]
 fn test_miropt_mode_forbidden_revisions() {
     let config = cfg().mode("mir-opt").build();
-    parse_rs(&config, "//@ revisions: CHECK");
+    parse_early_props(&config, "//@ revisions: CHECK");
 }
 
 #[test]
@@ -608,7 +593,7 @@ fn test_forbidden_revisions_allowed_in_non_filecheck_dir() {
         let content = format!("//@ revisions: {rev}");
         for mode in modes {
             let config = cfg().mode(mode).build();
-            parse_rs(&config, &content);
+            parse_early_props(&config, &content);
         }
     }
 }

src/tools/compiletest/src/lib.rs

@@ -41,7 +41,7 @@ use crate::common::{
     CodegenBackend, CompareMode, Config, Debugger, PassMode, TestMode, TestPaths, UI_EXTENSIONS,
     expected_output_path, output_base_dir, output_relative_path,
 };
-use crate::directives::{DirectivesCache, FileDirectives};
+use crate::directives::{AuxProps, DirectivesCache, FileDirectives};
 use crate::edition::parse_edition;
 use crate::executor::{CollectedTest, ColorConfig};
 
@@ -891,6 +891,11 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te
         // Create a test name and description to hand over to the executor.
         let (test_name, filterable_path) =
             make_test_name_and_filterable_path(&cx.config, testpaths, revision);
+
+        // While scanning for ignore/only/needs directives, also collect aux
+        // paths for up-to-date checking.
+        let mut aux_props = AuxProps::default();
+
         // Create a description struct for the test/revision.
         // This is where `ignore-*`/`only-*`/`needs-*` directives are handled,
         // because they historically needed to set the libtest ignored flag.
@@ -903,11 +908,15 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te
             &file_directives,
             revision,
             &mut collector.poisoned,
+            &mut aux_props,
         );
 
         // If a test's inputs haven't changed since the last time it ran,
         // mark it as ignored so that the executor will skip it.
-        if !cx.config.force_rerun && is_up_to_date(cx, testpaths, &early_props, revision) {
+        if !desc.ignore
+            && !cx.config.force_rerun
+            && is_up_to_date(cx, testpaths, &aux_props, revision)
+        {
             desc.ignore = true;
             // Keep this in sync with the "up-to-date" message detected by bootstrap.
             // FIXME(Zalathar): Now that we are no longer tied to libtest, we could
@@ -936,7 +945,7 @@ fn stamp_file_path(config: &Config, testpaths: &TestPaths, revision: Option<&str
 fn files_related_to_test(
     config: &Config,
     testpaths: &TestPaths,
-    props: &EarlyProps,
+    aux_props: &AuxProps,
     revision: Option<&str>,
 ) -> Vec<Utf8PathBuf> {
     let mut related = vec![];
@@ -953,8 +962,11 @@ fn files_related_to_test(
         related.push(testpaths.file.clone());
     }
 
-    for aux in props.aux.all_aux_path_strings() {
+    for aux in aux_props.all_aux_path_strings() {
         // FIXME(Zalathar): Perform all `auxiliary` path resolution in one place.
+        // FIXME(Zalathar): This only finds auxiliary files used _directly_ by
+        // the test file; if a transitive auxiliary is modified, the test might
+        // be treated as "up-to-date" even though it should run.
         let path = testpaths.file.parent().unwrap().join("auxiliary").join(aux);
         related.push(path);
     }
@@ -979,7 +991,7 @@ fn files_related_to_test(
 fn is_up_to_date(
     cx: &TestCollectorCx,
     testpaths: &TestPaths,
-    props: &EarlyProps,
+    aux_props: &AuxProps,
     revision: Option<&str>,
 ) -> bool {
     let stamp_file_path = stamp_file_path(&cx.config, testpaths, revision);
@@ -1000,7 +1012,7 @@ fn is_up_to_date(
     // Check the timestamp of the stamp file against the last modified time
     // of all files known to be relevant to the test.
     let mut inputs_stamp = cx.common_inputs_stamp.clone();
-    for path in files_related_to_test(&cx.config, testpaths, props, revision) {
+    for path in files_related_to_test(&cx.config, testpaths, aux_props, revision) {
         inputs_stamp.add_path(&path);
     }