diff --git a/make/src/lib.rs b/make/src/lib.rs index 74abb027c..5ea37219b 100644 --- a/make/src/lib.rs +++ b/make/src/lib.rs @@ -39,7 +39,12 @@ const DEFAULT_SHELL: &str = "/bin/sh"; /// The only way to create a Make is from a Makefile and a Config. pub struct Make { macros: Vec, + /// Target rules (non-special, non-inference). + /// Invariant: inference rules are never stored here, so `first_target()` + /// always returns a valid default target per POSIX. rules: Vec, + /// Inference rules (e.g. `.c.o:`, `.txt.out:`). + inference_rules: Vec, default_rule: Option, // .DEFAULT pub config: Config, } @@ -58,10 +63,52 @@ impl Make { } pub fn first_target(&self) -> Result<&Target, ErrorCode> { - let rule = self.rules.first().ok_or(NoTarget { target: None })?; + // Per POSIX: "the first target that make encounters that is not a special + // target or an inference rule shall be used." + // If there are no non-special, non-inference targets, fall back to the + // first inference rule (which will scan CWD for matching files). + let rule = self + .rules + .first() + .or_else(|| self.inference_rules.first()) + .ok_or(NoTarget { target: None })?; rule.targets().next().ok_or(NoTarget { target: None }) } + /// Finds a matching inference rule for the given target name. + /// + /// Per POSIX: the suffix of the target (.s1) is compared to .SUFFIXES. + /// If found, inference rules are searched for the first .s2.s1 rule whose + /// prerequisite file ($*.s2) exists. + fn find_inference_rule(&self, name: &str) -> Option<&Rule> { + let suffixes = self.config.rules.get(".SUFFIXES")?; + + // Find the target's suffix (.s1) + let target_suffix = suffixes + .iter() + .filter(|s| name.ends_with(s.as_str())) + .max_by_key(|s| s.len())?; + + let stem = &name[..name.len() - target_suffix.len()]; + + // Search inference rules for .s2.s1 where $*.s2 exists + for rule in &self.inference_rules { + let Some(rule_target) = rule.targets().next() else { + continue; + }; + if let Target::Inference { from, to, .. } = rule_target { + let expected_suffix = format!(".{}", to); + if expected_suffix == *target_suffix { + let prereq_path = format!("{}.{}", stem, from); + if std::path::Path::new(&prereq_path).exists() { + return Some(rule); + } + } + } + } + None + } + /// Builds the target with the given name. /// /// # Returns @@ -69,14 +116,31 @@ impl Make { /// - Ok(false) if the target was already up to date. /// - Err(_) if any errors occur. pub fn build_target(&self, name: impl AsRef) -> Result { + // Search both regular rules and inference rules let rule = match self.rule_by_target_name(&name) { Some(rule) => rule, - None => match &self.default_rule { + None => match self + .inference_rules + .iter() + .find(|rule| rule.targets().any(|t| t.as_ref() == name.as_ref())) + { Some(rule) => rule, None => { - return Err(NoTarget { - target: Some(name.as_ref().to_string()), - }) + // Per POSIX: "If a target exists and there is neither a target rule + // nor an inference rule for the target, the target shall be considered + // up-to-date." + if get_modified_time(&name).is_some() { + return Ok(false); + } + // No rule and file doesn't exist - try .DEFAULT or fail + match &self.default_rule { + Some(rule) => rule, + None => { + return Err(NoTarget { + target: Some(name.as_ref().to_string()), + }) + } + } } }, }; @@ -111,6 +175,18 @@ impl Make { for prerequisite in &newer_prerequisites { self.build_target(prerequisite)?; } + + // Per POSIX: "When no target rule with commands is found to update a + // target, the inference rules shall be checked." If the matched target + // rule has no recipes, look for a matching inference rule and run it + // for this specific target instead. + if rule.recipes().count() == 0 { + if let Some(inference_rule) = self.find_inference_rule(target.as_ref()) { + inference_rule.run_for_target(&self.config, &self.macros, target, up_to_date)?; + return Ok(true); + } + } + rule.run(&self.config, &self.macros, target, up_to_date)?; Ok(true) @@ -184,32 +260,60 @@ impl TryFrom<(Makefile, Config)> for Make { type Error = ErrorCode; fn try_from((makefile, config): (Makefile, Config)) -> Result { - let mut rules = vec![]; - let mut special_rules = vec![]; - let mut inference_rules = vec![]; - - for rule in makefile.rules() { - let rule = Rule::from(rule); + // Two-pass classification: .SUFFIXES must be processed before inference + // rule classification so that user-defined suffixes (especially with -r) + // are available when determining whether a rule like `.txt.out:` is an + // inference rule. + + let mut suffixes_rules = vec![]; + let mut remaining_parsed_rules = vec![]; + + // Pass 1: Separate .SUFFIXES rules from everything else and process + // them immediately so config.rules[".SUFFIXES"] is populated. + for parsed_rule in makefile.rules() { + let rule = Rule::from(parsed_rule); let Some(target) = rule.targets().next() else { return Err(NoTarget { target: None }); }; - - if SpecialTarget::try_from(target.clone()).is_ok() { - special_rules.push(rule); - } else if InferenceTarget::try_from((target.clone(), config.clone())).is_ok() { - inference_rules.push(rule); + if let Ok(SpecialTarget::Suffixes) = SpecialTarget::try_from(target.clone()) { + suffixes_rules.push(rule); } else { - rules.push(rule); + remaining_parsed_rules.push(rule); } } + // Build the Make struct early so we can process .SUFFIXES via the + // normal special_target::process path (which writes to make.config). let mut make = Self { - rules, + rules: vec![], + inference_rules: vec![], macros: makefile.variable_definitions().collect(), default_rule: None, config, }; + for rule in suffixes_rules { + special_target::process(rule, &mut make)?; + } + + // Pass 2: Classify remaining rules. Now make.config.rules[".SUFFIXES"] + // contains both built-in (unless -r) and user-defined suffixes. + let mut special_rules = vec![]; + + for rule in remaining_parsed_rules { + let Some(target) = rule.targets().next() else { + return Err(NoTarget { target: None }); + }; + + if SpecialTarget::try_from(target.clone()).is_ok() { + special_rules.push(rule); + } else if InferenceTarget::try_from((target.clone(), make.config.clone())).is_ok() { + make.inference_rules.push(rule); + } else { + make.rules.push(rule); + } + } + for rule in special_rules { special_target::process(rule, &mut make)?; } diff --git a/make/src/rule.rs b/make/src/rule.rs index fe25cb045..6dd69b8fd 100644 --- a/make/src/rule.rs +++ b/make/src/rule.rs @@ -66,6 +66,37 @@ impl Rule { self.recipes.iter() } + /// Runs an inference rule for a specific target (not a CWD scan). + /// + /// This is used when POSIX requires applying an inference rule to a specific + /// target that has no commands of its own. The internal macros ($<, $*, etc.) + /// are substituted based on the target name and the inference rule's suffixes. + pub fn run_for_target( + &self, + global_config: &GlobalConfig, + macros: &[VariableDefinition], + target: &Target, + up_to_date: bool, + ) -> Result<(), ErrorCode> { + // For an inference rule applied to a specific target, compute the + // input/output pair from the target name and the rule's suffixes. + let files = if let Some(Target::Inference { from, to, .. }) = self.targets().next() { + let target_name = target.as_ref(); + let expected_suffix = format!(".{}", to); + if let Some(stem) = target_name.strip_suffix(&expected_suffix) { + let input = PathBuf::from(format!("{}.{}", stem, from)); + let output = PathBuf::from(target_name); + vec![(input, output)] + } else { + vec![(PathBuf::from(""), PathBuf::from(""))] + } + } else { + vec![(PathBuf::from(""), PathBuf::from(""))] + }; + + self.run_with_files(global_config, macros, target, up_to_date, files) + } + /// Runs the rule with the global config and macros passed in. /// /// Returns `Ok` on success and `Err` on any errors while running the rule. @@ -75,6 +106,32 @@ impl Rule { macros: &[VariableDefinition], target: &Target, up_to_date: bool, + ) -> Result<(), ErrorCode> { + let files = match target { + Target::Inference { from, to, .. } => find_files_with_extension(from)? + .into_iter() + .map(|input| { + let mut output = input.clone(); + output.set_extension(to); + (input, output) + }) + .collect::>(), + _ => { + vec![(PathBuf::from(""), PathBuf::from(""))] + } + }; + + self.run_with_files(global_config, macros, target, up_to_date, files) + } + + /// Internal helper: runs the rule's recipes for the given input/output file pairs. + fn run_with_files( + &self, + global_config: &GlobalConfig, + macros: &[VariableDefinition], + target: &Target, + up_to_date: bool, + files: Vec<(PathBuf, PathBuf)>, ) -> Result<(), ErrorCode> { let GlobalConfig { ignore: global_ignore, @@ -97,20 +154,6 @@ impl Rule { phony: _, } = self.config; - let files = match target { - Target::Inference { from, to, .. } => find_files_with_extension(from)? - .into_iter() - .map(|input| { - let mut output = input.clone(); - output.set_extension(to); - (input, output) - }) - .collect::>(), - _ => { - vec![(PathBuf::from(""), PathBuf::from(""))] - } - }; - for inout in files { for recipe in self.recipes() { let RecipeConfig { diff --git a/make/src/special_target.rs b/make/src/special_target.rs index ed4a4bfbd..cec4a0bf0 100644 --- a/make/src/special_target.rs +++ b/make/src/special_target.rs @@ -200,6 +200,7 @@ impl Processor<'_> { self.make .rules .iter_mut() + .chain(self.make.inference_rules.iter_mut()) .filter(|r| r.targets().any(|t| t.as_ref() == prerequisite.as_ref())) .for_each(f.clone()); } @@ -209,7 +210,11 @@ impl Processor<'_> { /// specified. fn global(&mut self, f: impl FnMut(&mut Rule) + Clone) { if self.rule.prerequisites().count() == 0 { - self.make.rules.iter_mut().for_each(f); + self.make + .rules + .iter_mut() + .chain(self.make.inference_rules.iter_mut()) + .for_each(f); } } } diff --git a/make/tests/integration.rs b/make/tests/integration.rs index 23e9d5183..b44e319c1 100644 --- a/make/tests/integration.rs +++ b/make/tests/integration.rs @@ -7,7 +7,6 @@ // SPDX-License-Identifier: MIT // -use std::env; use std::fs::{remove_file, File}; use std::io::Write; use std::process::{Child, Command, Stdio}; @@ -160,17 +159,6 @@ mod arguments { 0, ) } - - #[test] - fn dash_r() { - run_test_helper( - &["-r"], - "", - "make: no makefile\n", - ErrorCode::NoMakefile.into(), - ) - } - #[test] fn dash_r_with_file() { run_test_helper_with_setup_and_destruct( @@ -182,12 +170,22 @@ mod arguments { File::create("testfile.txt").expect("failed to create file"); }, || { - remove_file("testfile.txt").expect("failed to remove file"); - remove_file("testfile.out").expect("failed to remove file"); + let _ = remove_file("testfile.txt"); + let _ = remove_file("testfile.out"); }, ); } + #[test] + fn dash_r() { + run_test_helper( + &["-r"], + "", + "make: no makefile\n", + ErrorCode::NoMakefile.into(), + ) + } + #[test] fn dash_i() { run_test_helper( @@ -449,6 +447,9 @@ mod target_behavior { #[test] fn async_events() { + // Clean up any leftover files from previous test runs + let _ = remove_file("text.txt"); + let args = [ "-f", "tests/makefiles/target_behavior/async_events/signal.mk", @@ -472,6 +473,10 @@ mod target_behavior { assert_eq!(stderr, "make: Interrupt\nmake: Deleting file 'text.txt'\n"); assert_eq!(output.status.code(), Some(130)); + + // The makefile creates text.txt and the signal handler should delete it, + // but clean up anyway in case test fails + let _ = remove_file("text.txt"); } } @@ -624,6 +629,11 @@ mod special_targets { #[test] fn precious() { + // Clean up any leftover files from previous test runs + let _ = remove_file("precious_text.txt"); + let _ = remove_file("preciousdir/some.txt"); + let _ = remove_dir("preciousdir"); + let args = [ "-f", "tests/makefiles/special_targets/precious/basic_precious.mk", @@ -653,6 +663,7 @@ mod special_targets { assert_eq!(output.status.code(), Some(130)); + let _ = remove_file("precious_text.txt"); remove_file("preciousdir/some.txt").unwrap(); remove_dir("preciousdir").unwrap(); } @@ -664,33 +675,23 @@ mod special_targets { "-f", "tests/makefiles/special_targets/suffixes/suffixes_basic.mk", ], - "Converting copied.txt to copied.out\n", + "Converting suffixes_test.sfx to suffixes_test.xfo\n", "", 0, - create_txt, + create_file, remove_files, ); - fn create_txt() { - let dir = env::current_dir().unwrap(); - for file in dir.read_dir().unwrap().map(Result::unwrap) { - if !file.file_type().map(|x| x.is_file()).unwrap_or(false) { - continue; - } - if file.path().extension().map(|x| x == "txt").unwrap_or(false) { - remove_file(file.path()).unwrap(); - } - } - - File::create("copied.txt") + fn create_file() { + File::create("suffixes_test.sfx") .unwrap() .write_all(b"some content") .unwrap(); } fn remove_files() { - remove_file("copied.txt").unwrap(); - remove_file("copied.out").unwrap(); + let _ = remove_file("suffixes_test.sfx"); + let _ = remove_file("suffixes_test.xfo"); } } @@ -785,3 +786,66 @@ mod special_targets { } } } + +mod inference_rules { + use super::*; + + #[test] + fn dash_r_suffix_classification() { + // Test that -r flag + user-defined .SUFFIXES + inference rule works correctly. + // This directly tests the original bug: inference rules must be classified + // correctly even when -r clears built-in suffixes. + run_test_helper_with_setup_and_destruct( + &["-rf", "tests/makefiles/inference_rules/dash_r_suffix.mk"], + "Converting dash_r_test.txt to dash_r_test.out\n", + "", + 0, + || { + File::create("dash_r_test.txt").expect("failed to create file"); + }, + || { + let _ = remove_file("dash_r_test.txt"); + let _ = remove_file("dash_r_test.out"); + }, + ); + } + + #[test] + fn target_no_commands_uses_inference() { + // Test that a target rule with no commands triggers inference rule lookup. + // Per POSIX: "When no target rule with commands is found to update a + // target, the inference rules shall be checked." + run_test_helper_with_setup_and_destruct( + &[ + "-sf", + "tests/makefiles/inference_rules/target_no_commands.mk", + ], + "Inference applied: inference_test.src -> inference_test.dst\n", + "", + 0, + || { + File::create("inference_test.src").expect("failed to create file"); + }, + || { + let _ = remove_file("inference_test.src"); + let _ = remove_file("inference_test.dst"); + }, + ); + } + + #[test] + fn inference_rule_not_default_target() { + // Test that inference rules are never selected as the default target. + // Per POSIX: "the first target that make encounters that is not a + // special target or an inference rule shall be used." + run_test_helper( + &[ + "-sf", + "tests/makefiles/inference_rules/not_default_target.mk", + ], + "CORRECT: real target ran\n", + "", + 0, + ); + } +} diff --git a/make/tests/makefiles/inference_rules/dash_r_suffix.mk b/make/tests/makefiles/inference_rules/dash_r_suffix.mk new file mode 100644 index 000000000..d5f7e5247 --- /dev/null +++ b/make/tests/makefiles/inference_rules/dash_r_suffix.mk @@ -0,0 +1,7 @@ +.SUFFIXES: .txt .out + +.txt.out: + @echo "Converting $< to $@" + @cp $< $@ + +dash_r_test.out: dash_r_test.txt diff --git a/make/tests/makefiles/inference_rules/not_default_target.mk b/make/tests/makefiles/inference_rules/not_default_target.mk new file mode 100644 index 000000000..78e9f7aa4 --- /dev/null +++ b/make/tests/makefiles/inference_rules/not_default_target.mk @@ -0,0 +1,7 @@ +.SUFFIXES: .a .b + +.a.b: + @echo "WRONG: inference rule ran as default" + +real_target: + @echo "CORRECT: real target ran" diff --git a/make/tests/makefiles/inference_rules/target_no_commands.mk b/make/tests/makefiles/inference_rules/target_no_commands.mk new file mode 100644 index 000000000..b4c824ab8 --- /dev/null +++ b/make/tests/makefiles/inference_rules/target_no_commands.mk @@ -0,0 +1,7 @@ +.SUFFIXES: .src .dst + +.src.dst: + @echo "Inference applied: $< -> $@" + @cp $< $@ + +inference_test.dst: inference_test.src diff --git a/make/tests/makefiles/special_targets/precious/basic_precious.mk b/make/tests/makefiles/special_targets/precious/basic_precious.mk index 39a7da28b..7a8b5b0b5 100644 --- a/make/tests/makefiles/special_targets/precious/basic_precious.mk +++ b/make/tests/makefiles/special_targets/precious/basic_precious.mk @@ -1,5 +1,5 @@ -.PRECIOUS: -text.txt: +.PRECIOUS: +precious_text.txt: echo hello mkdir preciousdir touch preciousdir/some.txt diff --git a/make/tests/makefiles/special_targets/suffixes/suffixes_basic.mk b/make/tests/makefiles/special_targets/suffixes/suffixes_basic.mk index ca34c1450..ebe359de5 100644 --- a/make/tests/makefiles/special_targets/suffixes/suffixes_basic.mk +++ b/make/tests/makefiles/special_targets/suffixes/suffixes_basic.mk @@ -1,7 +1,7 @@ -.SUFFIXES: .txt .out +.SUFFIXES: .sfx .xfo -.txt.out: - @echo "Converting copied.txt to copied.out" - @cp copied.txt copied.out +.sfx.xfo: + @echo "Converting suffixes_test.sfx to suffixes_test.xfo" + @cp suffixes_test.sfx suffixes_test.xfo