Commit 60912200 authored by Kyle Edwards's avatar Kyle Edwards Committed by Kitware Robot
Browse files

Merge topic 'cmake-presets-invalid-macro' into release-3.19

638557cb

 CMakePresets.json: Properly report macro expansion errors
Acked-by: Kitware Robot's avatarKitware Robot <kwrobot@kitware.com>
Merge-request: !5397
parents 6af1185b 638557cb
Pipeline #196125 failed with stages
in 40 minutes and 6 seconds
......@@ -157,8 +157,8 @@ void QCMake::setPreset(const QString& name, bool setBinary)
if (!name.isNull()) {
std::string presetName(name.toLocal8Bit());
auto const& preset = this->CMakePresetsFile.Presets[presetName];
auto expandedPreset = this->CMakePresetsFile.ExpandMacros(preset);
auto const& expandedPreset =
this->CMakePresetsFile.Presets[presetName].Expanded;
if (expandedPreset) {
if (setBinary) {
QString binaryDir =
......@@ -420,8 +420,7 @@ QCMakePropertyList QCMake::properties() const
if (!this->PresetName.isNull()) {
std::string presetName(this->PresetName.toLocal8Bit());
auto p = this->CMakePresetsFile.ExpandMacros(
this->CMakePresetsFile.Presets.at(presetName));
auto const& p = this->CMakePresetsFile.Presets.at(presetName).Expanded;
if (p) {
for (auto const& v : p->CacheVariables) {
if (!v.second) {
......@@ -537,7 +536,8 @@ void QCMake::loadPresets()
QVector<QCMakePreset> presets;
for (auto const& name : this->CMakePresetsFile.PresetOrder) {
auto const& p = this->CMakePresetsFile.Presets[name];
auto const& it = this->CMakePresetsFile.Presets[name];
auto const& p = it.Unexpanded;
if (p.Hidden) {
continue;
}
......@@ -554,12 +554,12 @@ void QCMake::loadPresets()
preset.toolset = std::move(QString::fromLocal8Bit(p.Toolset.data()));
preset.setGenConfig = !p.GeneratorConfig ||
p.GeneratorConfig == cmCMakePresetsFile::CMakeGeneratorConfig::Default;
preset.enabled = std::find_if(this->AvailableGenerators.begin(),
this->AvailableGenerators.end(),
[&p](const cmake::GeneratorInfo& g) {
return g.name == p.Generator;
}) != this->AvailableGenerators.end() &&
this->CMakePresetsFile.ExpandMacros(p);
preset.enabled = it.Expanded &&
std::find_if(this->AvailableGenerators.begin(),
this->AvailableGenerators.end(),
[&p](const cmake::GeneratorInfo& g) {
return g.name == p.Generator;
}) != this->AvailableGenerators.end();
presets.push_back(preset);
}
emit this->presetsChanged(presets);
......
......@@ -29,6 +29,7 @@ enum class CycleStatus
using ReadFileResult = cmCMakePresetsFile::ReadFileResult;
using CacheVariable = cmCMakePresetsFile::CacheVariable;
using UnexpandedPreset = cmCMakePresetsFile::UnexpandedPreset;
using ExpandedPreset = cmCMakePresetsFile::ExpandedPreset;
using CMakeGeneratorConfig = cmCMakePresetsFile::CMakeGeneratorConfig;
constexpr int MIN_VERSION = 1;
......@@ -312,9 +313,9 @@ void InheritOptionalBool(cm::optional<bool>& child,
* that each preset has the required fields, either directly or through
* inheritance.
*/
ReadFileResult VisitPreset(std::map<std::string, UnexpandedPreset>& presets,
UnexpandedPreset& preset,
std::map<std::string, CycleStatus> cycleStatus)
ReadFileResult VisitPreset(
std::map<std::string, cmCMakePresetsFile::PresetPair>& presets,
UnexpandedPreset& preset, std::map<std::string, CycleStatus> cycleStatus)
{
switch (cycleStatus[preset.Name]) {
case CycleStatus::InProgress:
......@@ -340,35 +341,38 @@ ReadFileResult VisitPreset(std::map<std::string, UnexpandedPreset>& presets,
return ReadFileResult::INVALID_PRESET;
}
if (!preset.User && parent->second.User) {
if (!preset.User && parent->second.Unexpanded.User) {
return ReadFileResult::USER_PRESET_INHERITANCE;
}
auto result = VisitPreset(presets, parent->second, cycleStatus);
auto result = VisitPreset(presets, parent->second.Unexpanded, cycleStatus);
if (result != ReadFileResult::READ_OK) {
return result;
}
InheritString(preset.Generator, parent->second.Generator);
InheritString(preset.Architecture, parent->second.Architecture);
InheritString(preset.Toolset, parent->second.Toolset);
InheritString(preset.Generator, parent->second.Unexpanded.Generator);
InheritString(preset.Architecture, parent->second.Unexpanded.Architecture);
InheritString(preset.Toolset, parent->second.Unexpanded.Toolset);
if (!preset.GeneratorConfig) {
preset.GeneratorConfig = parent->second.GeneratorConfig;
preset.GeneratorConfig = parent->second.Unexpanded.GeneratorConfig;
}
InheritString(preset.BinaryDir, parent->second.BinaryDir);
InheritOptionalBool(preset.WarnDev, parent->second.WarnDev);
InheritOptionalBool(preset.ErrorDev, parent->second.ErrorDev);
InheritOptionalBool(preset.WarnDeprecated, parent->second.WarnDeprecated);
InheritString(preset.BinaryDir, parent->second.Unexpanded.BinaryDir);
InheritOptionalBool(preset.WarnDev, parent->second.Unexpanded.WarnDev);
InheritOptionalBool(preset.ErrorDev, parent->second.Unexpanded.ErrorDev);
InheritOptionalBool(preset.WarnDeprecated,
parent->second.Unexpanded.WarnDeprecated);
InheritOptionalBool(preset.ErrorDeprecated,
parent->second.ErrorDeprecated);
parent->second.Unexpanded.ErrorDeprecated);
InheritOptionalBool(preset.WarnUninitialized,
parent->second.WarnUninitialized);
InheritOptionalBool(preset.WarnUnusedCli, parent->second.WarnUnusedCli);
InheritOptionalBool(preset.WarnSystemVars, parent->second.WarnSystemVars);
for (auto const& v : parent->second.CacheVariables) {
parent->second.Unexpanded.WarnUninitialized);
InheritOptionalBool(preset.WarnUnusedCli,
parent->second.Unexpanded.WarnUnusedCli);
InheritOptionalBool(preset.WarnSystemVars,
parent->second.Unexpanded.WarnSystemVars);
for (auto const& v : parent->second.Unexpanded.CacheVariables) {
preset.CacheVariables.insert(v);
}
for (auto const& v : parent->second.Environment) {
for (auto const& v : parent->second.Unexpanded.Environment) {
preset.Environment.insert(v);
}
}
......@@ -393,7 +397,7 @@ ReadFileResult VisitPreset(std::map<std::string, UnexpandedPreset>& presets,
}
ReadFileResult ComputePresetInheritance(
std::map<std::string, UnexpandedPreset>& presets)
std::map<std::string, cmCMakePresetsFile::PresetPair>& presets)
{
std::map<std::string, CycleStatus> cycleStatus;
for (auto const& it : presets) {
......@@ -401,7 +405,7 @@ ReadFileResult ComputePresetInheritance(
}
for (auto& it : presets) {
auto result = VisitPreset(presets, it.second, cycleStatus);
auto result = VisitPreset(presets, it.second.Unexpanded, cycleStatus);
if (result != ReadFileResult::READ_OK) {
return result;
}
......@@ -437,44 +441,111 @@ bool IsValidMacroNamespace(const std::string& str)
return false;
}
bool VisitEnv(const cmCMakePresetsFile& file,
cmCMakePresetsFile::ExpandedPreset& preset,
std::map<std::string, CycleStatus>& envCycles,
std::string& value, CycleStatus& status);
enum class ExpandMacroResult
{
Ok,
Ignore,
Error,
};
ExpandMacroResult VisitEnv(const cmCMakePresetsFile& file,
cmCMakePresetsFile::ExpandedPreset& preset,
std::map<std::string, CycleStatus>& envCycles,
std::string& value, CycleStatus& status);
ExpandMacroResult ExpandMacros(const cmCMakePresetsFile& file,
cmCMakePresetsFile::ExpandedPreset& preset,
std::map<std::string, CycleStatus>& envCycles,
std::string& out);
ExpandMacroResult ExpandMacro(const cmCMakePresetsFile& file,
cmCMakePresetsFile::ExpandedPreset& preset,
std::map<std::string, CycleStatus>& envCycles,
std::string& out,
const std::string& macroNamespace,
const std::string& macroName);
bool ExpandMacros(const cmCMakePresetsFile& file,
cmCMakePresetsFile::ExpandedPreset& preset,
std::map<std::string, CycleStatus>& envCycles,
std::string& out);
bool ExpandMacro(const cmCMakePresetsFile& file,
cmCMakePresetsFile::ExpandedPreset& preset,
std::map<std::string, CycleStatus>& envCycles,
std::string& out, const std::string& macroNamespace,
const std::string& macroName);
bool VisitEnv(const cmCMakePresetsFile& file,
cmCMakePresetsFile::ExpandedPreset& preset,
std::map<std::string, CycleStatus>& envCycles,
std::string& value, CycleStatus& status)
const UnexpandedPreset& preset,
cm::optional<ExpandedPreset>& out)
{
out = preset;
std::map<std::string, CycleStatus> envCycles;
for (auto const& v : out->Environment) {
envCycles[v.first] = CycleStatus::Unvisited;
}
for (auto& v : out->Environment) {
if (v.second) {
switch (VisitEnv(file, *out, envCycles, *v.second, envCycles[v.first])) {
case ExpandMacroResult::Error:
return false;
case ExpandMacroResult::Ignore:
out.reset();
return true;
case ExpandMacroResult::Ok:
break;
}
}
}
std::string binaryDir = preset.BinaryDir;
switch (ExpandMacros(file, *out, envCycles, binaryDir)) {
case ExpandMacroResult::Error:
return false;
case ExpandMacroResult::Ignore:
out.reset();
return true;
case ExpandMacroResult::Ok:
break;
}
if (!cmSystemTools::FileIsFullPath(binaryDir)) {
binaryDir = cmStrCat(file.SourceDir, '/', binaryDir);
}
out->BinaryDir = cmSystemTools::CollapseFullPath(binaryDir);
cmSystemTools::ConvertToUnixSlashes(out->BinaryDir);
for (auto& variable : out->CacheVariables) {
if (variable.second) {
switch (ExpandMacros(file, *out, envCycles, variable.second->Value)) {
case ExpandMacroResult::Error:
return false;
case ExpandMacroResult::Ignore:
out.reset();
return true;
case ExpandMacroResult::Ok:
break;
}
}
}
return true;
}
ExpandMacroResult VisitEnv(const cmCMakePresetsFile& file,
cmCMakePresetsFile::ExpandedPreset& preset,
std::map<std::string, CycleStatus>& envCycles,
std::string& value, CycleStatus& status)
{
if (status == CycleStatus::Verified) {
return true;
return ExpandMacroResult::Ok;
}
if (status == CycleStatus::InProgress) {
return false;
return ExpandMacroResult::Error;
}
status = CycleStatus::InProgress;
if (!ExpandMacros(file, preset, envCycles, value)) {
return false;
auto e = ExpandMacros(file, preset, envCycles, value);
if (e != ExpandMacroResult::Ok) {
return e;
}
status = CycleStatus::Verified;
return true;
return ExpandMacroResult::Ok;
}
bool ExpandMacros(const cmCMakePresetsFile& file,
cmCMakePresetsFile::ExpandedPreset& preset,
std::map<std::string, CycleStatus>& envCycles,
std::string& out)
ExpandMacroResult ExpandMacros(const cmCMakePresetsFile& file,
cmCMakePresetsFile::ExpandedPreset& preset,
std::map<std::string, CycleStatus>& envCycles,
std::string& out)
{
std::string result;
std::string macroNamespace;
......@@ -521,9 +592,10 @@ bool ExpandMacros(const cmCMakePresetsFile& file,
case State::MacroName:
if (c == '}') {
if (!ExpandMacro(file, preset, envCycles, result, macroNamespace,
macroName)) {
return false;
auto e = ExpandMacro(file, preset, envCycles, result, macroNamespace,
macroName);
if (e != ExpandMacroResult::Ok) {
return e;
}
macroNamespace.clear();
macroName.clear();
......@@ -543,67 +615,72 @@ bool ExpandMacros(const cmCMakePresetsFile& file,
result += macroNamespace;
break;
case State::MacroName:
return false;
return ExpandMacroResult::Error;
}
out = std::move(result);
return true;
return ExpandMacroResult::Ok;
}
bool ExpandMacro(const cmCMakePresetsFile& file,
cmCMakePresetsFile::ExpandedPreset& preset,
std::map<std::string, CycleStatus>& envCycles,
std::string& out, const std::string& macroNamespace,
const std::string& macroName)
ExpandMacroResult ExpandMacro(const cmCMakePresetsFile& file,
cmCMakePresetsFile::ExpandedPreset& preset,
std::map<std::string, CycleStatus>& envCycles,
std::string& out,
const std::string& macroNamespace,
const std::string& macroName)
{
if (macroNamespace.empty()) {
if (macroName == "sourceDir") {
out += file.SourceDir;
return true;
return ExpandMacroResult::Ok;
}
if (macroName == "sourceParentDir") {
out += cmSystemTools::GetParentDirectory(file.SourceDir);
return true;
return ExpandMacroResult::Ok;
}
if (macroName == "presetName") {
out += preset.Name;
return true;
return ExpandMacroResult::Ok;
}
if (macroName == "generator") {
out += preset.Generator;
return true;
return ExpandMacroResult::Ok;
}
if (macroName == "dollar") {
out += '$';
return true;
return ExpandMacroResult::Ok;
}
}
if (macroNamespace == "env" && !macroName.empty()) {
auto v = preset.Environment.find(macroName);
if (v != preset.Environment.end() && v->second) {
if (!VisitEnv(file, preset, envCycles, *v->second,
envCycles[macroName])) {
return false;
auto e =
VisitEnv(file, preset, envCycles, *v->second, envCycles[macroName]);
if (e != ExpandMacroResult::Ok) {
return e;
}
out += *v->second;
return true;
return ExpandMacroResult::Ok;
}
}
if (macroNamespace == "env" || macroNamespace == "penv") {
if (macroName.empty()) {
return false;
return ExpandMacroResult::Error;
}
const char* value = std::getenv(macroName.c_str());
if (value) {
out += value;
}
return true;
return ExpandMacroResult::Ok;
}
// "vendor" falls through to here
return false;
if (macroNamespace == "vendor") {
return ExpandMacroResult::Ignore;
}
return ExpandMacroResult::Error;
}
}
......@@ -626,7 +703,7 @@ cmCMakePresetsFile::ReadFileResult cmCMakePresetsFile::ReadProjectPresets(
this->PresetOrder.clear();
std::vector<std::string> presetOrder;
std::map<std::string, UnexpandedPreset> presetMap;
std::map<std::string, PresetPair> presetMap;
std::string filename = GetUserFilename(this->SourceDir);
if (cmSystemTools::FileExists(filename)) {
......@@ -656,6 +733,12 @@ cmCMakePresetsFile::ReadFileResult cmCMakePresetsFile::ReadProjectPresets(
return result;
}
for (auto& it : presetMap) {
if (!ExpandMacros(*this, it.second.Unexpanded, it.second.Expanded)) {
return ReadFileResult::INVALID_MACRO_EXPANSION;
}
}
this->PresetOrder = std::move(presetOrder);
this->Presets = std::move(presetMap);
return ReadFileResult::READ_OK;
......@@ -694,51 +777,16 @@ const char* cmCMakePresetsFile::ResultToString(ReadFileResult result)
return "Cyclic preset inheritance";
case ReadFileResult::USER_PRESET_INHERITANCE:
return "Project preset inherits from user preset";
default:
return "Unknown error";
}
}
cm::optional<cmCMakePresetsFile::ExpandedPreset>
cmCMakePresetsFile::ExpandMacros(const UnexpandedPreset& preset) const
{
ExpandedPreset retval{ preset };
std::map<std::string, CycleStatus> envCycles;
for (auto const& v : retval.Environment) {
envCycles[v.first] = CycleStatus::Unvisited;
}
for (auto& v : retval.Environment) {
if (v.second &&
!VisitEnv(*this, retval, envCycles, *v.second, envCycles[v.first])) {
return cm::nullopt;
}
}
std::string binaryDir = preset.BinaryDir;
if (!::ExpandMacros(*this, retval, envCycles, binaryDir)) {
return cm::nullopt;
}
if (!cmSystemTools::FileIsFullPath(binaryDir)) {
binaryDir = cmStrCat(this->SourceDir, '/', binaryDir);
}
retval.BinaryDir = cmSystemTools::CollapseFullPath(binaryDir);
cmSystemTools::ConvertToUnixSlashes(retval.BinaryDir);
for (auto& variable : retval.CacheVariables) {
if (variable.second &&
!::ExpandMacros(*this, retval, envCycles, variable.second->Value)) {
return cm::nullopt;
}
case ReadFileResult::INVALID_MACRO_EXPANSION:
return "Invalid macro expansion";
}
return cm::make_optional(retval);
return "Unknown error";
}
cmCMakePresetsFile::ReadFileResult cmCMakePresetsFile::ReadJSONFile(
const std::string& filename, std::vector<std::string>& presetOrder,
std::map<std::string, UnexpandedPreset>& presetMap, bool user)
std::map<std::string, PresetPair>& presetMap, bool user)
{
cmsys::ifstream fin(filename.c_str());
if (!fin) {
......@@ -785,7 +833,7 @@ cmCMakePresetsFile::ReadFileResult cmCMakePresetsFile::ReadJSONFile(
if (preset.Name.empty()) {
return ReadFileResult::INVALID_PRESET;
}
if (!presetMap.insert({ preset.Name, preset }).second) {
if (!presetMap.insert({ preset.Name, { preset, cm::nullopt } }).second) {
return ReadFileResult::DUPLICATE_PRESETS;
}
presetOrder.push_back(preset.Name);
......
......@@ -102,8 +102,15 @@ public:
}
};
class PresetPair
{
public:
UnexpandedPreset Unexpanded;
cm::optional<ExpandedPreset> Expanded;
};
std::string SourceDir;
std::map<std::string, UnexpandedPreset> Presets;
std::map<std::string, PresetPair> Presets;
std::vector<std::string> PresetOrder;
enum class ReadFileResult
......@@ -123,6 +130,7 @@ public:
DUPLICATE_PRESETS,
CYCLIC_PRESET_INHERITANCE,
USER_PRESET_INHERITANCE,
INVALID_MACRO_EXPANSION,
};
static std::string GetFilename(const std::string& sourceDir);
......@@ -131,11 +139,9 @@ public:
bool allowNoFiles = false);
static const char* ResultToString(ReadFileResult result);
cm::optional<ExpandedPreset> ExpandMacros(
const UnexpandedPreset& preset) const;
private:
ReadFileResult ReadJSONFile(
const std::string& filename, std::vector<std::string>& presetOrder,
std::map<std::string, UnexpandedPreset>& presetMap, bool user);
ReadFileResult ReadJSONFile(const std::string& filename,
std::vector<std::string>& presetOrder,
std::map<std::string, PresetPair>& presetMap,
bool user);
};
......@@ -1042,17 +1042,17 @@ void cmake::SetArgs(const std::vector<std::string>& args)
this->PrintPresetList(settingsFile);
return;
}
if (preset->second.Hidden) {
if (preset->second.Unexpanded.Hidden) {
cmSystemTools::Error(cmStrCat("Cannot use hidden preset in ",
this->GetHomeDirectory(), ": \"",
presetName, '"'));
this->PrintPresetList(settingsFile);
return;
}
auto expandedPreset = settingsFile.ExpandMacros(preset->second);
auto const& expandedPreset = preset->second.Expanded;
if (!expandedPreset) {
cmSystemTools::Error(cmStrCat("Could not evaluate preset \"",
preset->second.Name,
preset->second.Unexpanded.Name,
"\": Invalid macro expansion"));
return;
}
......@@ -1464,13 +1464,12 @@ void cmake::PrintPresetList(const cmCMakePresetsFile& file) const
std::vector<cmCMakePresetsFile::UnexpandedPreset> presets;
for (auto const& p : file.PresetOrder) {
auto const& preset = file.Presets.at(p);
if (!preset.Hidden &&
if (!preset.Unexpanded.Hidden && preset.Expanded &&
std::find_if(generators.begin(), generators.end(),
[&preset](const GeneratorInfo& info) {
return info.name == preset.Generator;
}) != generators.end() &&
file.ExpandMacros(preset)) {
presets.push_back(preset);
return info.name == preset.Unexpanded.Generator;
}) != generators.end()) {
presets.push_back(preset.Unexpanded);
}
}
......
......@@ -321,16 +321,6 @@
"generator": "@RunCMake_GENERATOR@",
"binaryDir": "${sourceDir}/build"
},
{
"name": "UnclosedMacro",
"generator": "@RunCMake_GENERATOR@",
"binaryDir": "${sourceDir"
},
{
"name": "NoSuchMacro",
"generator": "@RunCMake_GENERATOR@",
"binaryDir": "${noexist}"
},
{
"name": "VendorMacro",
"generator": "@RunCMake_GENERATOR@",
......@@ -341,31 +331,6 @@
"generator": "Invalid Generator",
"binaryDir": "${sourceDir}/build"
},
{
"name": "EnvCycle",
"generator": "@RunCMake_GENERATOR@",
"binaryDir": "${sourceDir}/build",
"environment": {
"ENV_1": "$env{ENV_2}",
"ENV_2": "$env{ENV_1}"
}
},
{
"name": "EmptyEnv",
"generator": "@RunCMake_GENERATOR@",
"binaryDir": "${sourceDir}/build",
"cacheVariables": {
"MY_VAR": "$env{}"
}
},
{
"name": "EmptyPenv",
"generator": "@RunCMake_GENERATOR@",