Loading executor.cpp +30 −1 Changes for executor.cpp: 30 added lines, 1 removed line. Original line number Diff line number Diff line Loading @@ -512,6 +512,27 @@ std::map<std::string, std::string> Executor::compute_target_scope( for (const auto& entry : mf_.target_vars) { if (entry.first != target && !pattern_key_matches(entry.first, target)) continue; for (const auto& b : entry.second) { // GNU make: a variable whose current value came from the command // line (or a parent's -e environment override) can't be touched // by an ordinary '=' / '+=' assignment -- target-specific ones // included -- unless the binding used 'override'. Without this // check, a recipe pattern like zstd's SET_CACHE_DIRECTORY -- // which re-invokes $(MAKE) passing CPPFLAGS/CFLAGS/LDFLAGS // explicitly on the recursive command line (making them // command-line-origin in the child) -- causes the child's own // identical target-specific '+=' bindings to append the same // flags a second time on top of the value it was just handed, // doubling e.g. "-fPIC -fvisibility=hidden". Once a name is // already present in `scope` it got there via a (permitted) // target-specific binding rather than the command line, so // further target-specific bindings for it remain free to modify // it further. bool already_scoped = scope.count(b.name) != 0; if (!already_scoped && !b.is_override) { auto git = mf_.variables.find(b.name); if (git != mf_.variables.end() && git->second.from_cmdline) continue; } std::string val = resolve(b.value); if (b.append) { std::string base; Loading @@ -521,8 +542,16 @@ std::map<std::string, std::string> Executor::compute_target_scope( } else { auto git = mf_.variables.find(b.name); if (git != mf_.variables.end()) // Scope-aware, like `resolve` above: a recursively- // expanded base value may itself reference a variable // that the current target scope overrides (e.g. // zstd's "CFLAGS += $(DEBUGFLAGS) ..." where a // '%-release' pattern binds DEBUGFLAGS := "" for this // target). Expanding against the un-scoped mf_ would // resolve $(DEBUGFLAGS) to its global default instead // of the target-specific override in effect here. base = (git->second.flavor == VarFlavor::Recursive) ? parser_.expand(git->second.value, mf_) : git->second.value; ? resolve(git->second.value) : git->second.value; } scope[b.name] = base.empty() ? val : base + " " + val; } else { Loading parser.cpp +13 −2 Changes for parser.cpp: 13 added lines, 2 removed lines. Original line number Diff line number Diff line Loading @@ -906,12 +906,19 @@ bool Parser::try_parse_rule(const std::string& line, Makefile& mf, Rule*& curren // additionally marks the variable for the recipe environment. std::string tv_rhs = rhs_raw; bool tv_export = false; bool tv_override = false; for (;;) { std::string t = tv_rhs; size_t sp = t.find_first_of(" \t"); if (sp == std::string::npos) break; std::string kw = t.substr(0, sp); if (kw == "private" || kw == "override") { if (kw == "private") { tv_rhs = trim(t.substr(sp + 1)); } else if (kw == "override") { // Like a plain 'override VAR = ...' assignment, this lets the // binding take effect even if VAR's current value came from // the command line (see compute_target_scope's lock check). tv_override = true; tv_rhs = trim(t.substr(sp + 1)); } else if (kw == "export") { tv_export = true; Loading @@ -938,13 +945,17 @@ bool Parser::try_parse_rule(const std::string& line, Makefile& mf, Rule*& curren pr.value = val; pr.append = false; } // Sticky: once any binding for this (target, name) // pair used 'override', treat the merged binding as // overriding too. pr.is_override = pr.is_override || tv_override; found = true; break; } } // A first-time '+=' must append to whatever value the variable // has in the surrounding (global) scope when the recipe runs. if (!found) vec.push_back({tv_name, val, tv_op == "+="}); if (!found) vec.push_back({tv_name, val, tv_op == "+=", tv_override}); } current_rule = nullptr; return true; Loading parser.h +7 −0 Changes for parser.h: 7 added lines, 0 removed lines. Original line number Diff line number Diff line Loading @@ -45,6 +45,13 @@ struct Makefile { std::string name; std::string value; bool append = false; // Whether this binding used the 'override' keyword. Like a plain // 'override VAR = ...' assignment, an overriding target-specific // binding is allowed to modify a variable whose current value came // from the command line (or environment via -e); without it, GNU // make silently leaves a command-line-origin variable untouched // (see compute_target_scope). bool is_override = false; }; std::map<std::string, std::vector<TargetVar>> target_vars; // Variables marked for export into the recipe / sub-make environment. Loading Loading
executor.cpp +30 −1 Changes for executor.cpp: 30 added lines, 1 removed line. Original line number Diff line number Diff line Loading @@ -512,6 +512,27 @@ std::map<std::string, std::string> Executor::compute_target_scope( for (const auto& entry : mf_.target_vars) { if (entry.first != target && !pattern_key_matches(entry.first, target)) continue; for (const auto& b : entry.second) { // GNU make: a variable whose current value came from the command // line (or a parent's -e environment override) can't be touched // by an ordinary '=' / '+=' assignment -- target-specific ones // included -- unless the binding used 'override'. Without this // check, a recipe pattern like zstd's SET_CACHE_DIRECTORY -- // which re-invokes $(MAKE) passing CPPFLAGS/CFLAGS/LDFLAGS // explicitly on the recursive command line (making them // command-line-origin in the child) -- causes the child's own // identical target-specific '+=' bindings to append the same // flags a second time on top of the value it was just handed, // doubling e.g. "-fPIC -fvisibility=hidden". Once a name is // already present in `scope` it got there via a (permitted) // target-specific binding rather than the command line, so // further target-specific bindings for it remain free to modify // it further. bool already_scoped = scope.count(b.name) != 0; if (!already_scoped && !b.is_override) { auto git = mf_.variables.find(b.name); if (git != mf_.variables.end() && git->second.from_cmdline) continue; } std::string val = resolve(b.value); if (b.append) { std::string base; Loading @@ -521,8 +542,16 @@ std::map<std::string, std::string> Executor::compute_target_scope( } else { auto git = mf_.variables.find(b.name); if (git != mf_.variables.end()) // Scope-aware, like `resolve` above: a recursively- // expanded base value may itself reference a variable // that the current target scope overrides (e.g. // zstd's "CFLAGS += $(DEBUGFLAGS) ..." where a // '%-release' pattern binds DEBUGFLAGS := "" for this // target). Expanding against the un-scoped mf_ would // resolve $(DEBUGFLAGS) to its global default instead // of the target-specific override in effect here. base = (git->second.flavor == VarFlavor::Recursive) ? parser_.expand(git->second.value, mf_) : git->second.value; ? resolve(git->second.value) : git->second.value; } scope[b.name] = base.empty() ? val : base + " " + val; } else { Loading
parser.cpp +13 −2 Changes for parser.cpp: 13 added lines, 2 removed lines. Original line number Diff line number Diff line Loading @@ -906,12 +906,19 @@ bool Parser::try_parse_rule(const std::string& line, Makefile& mf, Rule*& curren // additionally marks the variable for the recipe environment. std::string tv_rhs = rhs_raw; bool tv_export = false; bool tv_override = false; for (;;) { std::string t = tv_rhs; size_t sp = t.find_first_of(" \t"); if (sp == std::string::npos) break; std::string kw = t.substr(0, sp); if (kw == "private" || kw == "override") { if (kw == "private") { tv_rhs = trim(t.substr(sp + 1)); } else if (kw == "override") { // Like a plain 'override VAR = ...' assignment, this lets the // binding take effect even if VAR's current value came from // the command line (see compute_target_scope's lock check). tv_override = true; tv_rhs = trim(t.substr(sp + 1)); } else if (kw == "export") { tv_export = true; Loading @@ -938,13 +945,17 @@ bool Parser::try_parse_rule(const std::string& line, Makefile& mf, Rule*& curren pr.value = val; pr.append = false; } // Sticky: once any binding for this (target, name) // pair used 'override', treat the merged binding as // overriding too. pr.is_override = pr.is_override || tv_override; found = true; break; } } // A first-time '+=' must append to whatever value the variable // has in the surrounding (global) scope when the recipe runs. if (!found) vec.push_back({tv_name, val, tv_op == "+="}); if (!found) vec.push_back({tv_name, val, tv_op == "+=", tv_override}); } current_rule = nullptr; return true; Loading
parser.h +7 −0 Changes for parser.h: 7 added lines, 0 removed lines. Original line number Diff line number Diff line Loading @@ -45,6 +45,13 @@ struct Makefile { std::string name; std::string value; bool append = false; // Whether this binding used the 'override' keyword. Like a plain // 'override VAR = ...' assignment, an overriding target-specific // binding is allowed to modify a variable whose current value came // from the command line (or environment via -e); without it, GNU // make silently leaves a command-line-origin variable untouched // (see compute_target_scope). bool is_override = false; }; std::map<std::string, std::vector<TargetVar>> target_vars; // Variables marked for export into the recipe / sub-make environment. Loading