From eacf1f879b0933509efbd4fb4d6d72ce99412aa7 Mon Sep 17 00:00:00 2001 From: Robert Maynard Date: Thu, 20 Jan 2022 11:04:57 -0500 Subject: cmake: Warn about unnecessary paths on command line We can't make it an error as that would break existing behavior. Fixes: #23110 --- Source/cmake.cxx | 37 +++++++++++++++++++--- Source/cmake.h | 2 +- .../RunCMake/CommandLine/B-S-extra-path-stderr.txt | 4 +++ Tests/RunCMake/CommandLine/RunCMakeTest.cmake | 6 ++++ .../RunCMake/CommandLine/S-B-extra-path-stderr.txt | 4 +++ 5 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 Tests/RunCMake/CommandLine/B-S-extra-path-stderr.txt create mode 100644 Tests/RunCMake/CommandLine/S-B-extra-path-stderr.txt diff --git a/Source/cmake.cxx b/Source/cmake.cxx index 5632c38..72b26e1 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -790,6 +790,7 @@ void cmake::SetArgs(const std::vector& args) bool haveBArg = false; bool scriptMode = false; std::string possibleUnknownArg; + std::string extraProvidedPath; #if !defined(CMAKE_BOOTSTRAP) std::string profilingFormat; std::string profilingOutput; @@ -1168,10 +1169,18 @@ void cmake::SetArgs(const std::vector& args) } else if (!matched && cmHasLiteralPrefix(arg, "-")) { possibleUnknownArg = arg; } else if (!matched) { - this->SetDirectoriesFromFile(arg); + bool parsedDirectory = this->SetDirectoriesFromFile(arg); + if (!parsedDirectory) { + extraProvidedPath = arg; + } } } + if (!extraProvidedPath.empty() && !scriptMode) { + this->IssueMessage(MessageType::WARNING, + cmStrCat("Ignoring extra path from command line:\n ", + extraProvidedPath)); + } if (!possibleUnknownArg.empty() && !scriptMode) { cmSystemTools::Error(cmStrCat("Unknown argument ", possibleUnknownArg)); cmSystemTools::Error("Run 'cmake --help' for all supported options."); @@ -1450,22 +1459,27 @@ void cmake::PrintTraceFormatVersion() } } -void cmake::SetDirectoriesFromFile(const std::string& arg) +bool cmake::SetDirectoriesFromFile(const std::string& arg) { // Check if the argument refers to a CMakeCache.txt or // CMakeLists.txt file. std::string listPath; std::string cachePath; + bool is_empty_directory = false; if (cmSystemTools::FileIsDirectory(arg)) { std::string path = cmSystemTools::CollapseFullPath(arg); cmSystemTools::ConvertToUnixSlashes(path); std::string cacheFile = cmStrCat(path, "/CMakeCache.txt"); std::string listFile = cmStrCat(path, "/CMakeLists.txt"); + + is_empty_directory = true; if (cmSystemTools::FileExists(cacheFile)) { cachePath = path; + is_empty_directory = false; } if (cmSystemTools::FileExists(listFile)) { listPath = path; + is_empty_directory = false; } } else if (cmSystemTools::FileExists(arg)) { std::string fullPath = cmSystemTools::CollapseFullPath(arg); @@ -1497,7 +1511,7 @@ void cmake::SetDirectoriesFromFile(const std::string& arg) if (existingValue) { this->SetHomeOutputDirectory(cachePath); this->SetHomeDirectory(*existingValue); - return; + return true; } } } @@ -1505,9 +1519,15 @@ void cmake::SetDirectoriesFromFile(const std::string& arg) bool no_source_tree = this->GetHomeDirectory().empty(); bool no_build_tree = this->GetHomeOutputDirectory().empty(); + // When invoked with a path that points to an existing CMakeCache + // This function is called multiple times with the same path + const bool passed_same_path = (listPath == this->GetHomeDirectory()) || + (listPath == this->GetHomeOutputDirectory()); + bool used_provided_path = + (passed_same_path || no_source_tree || no_build_tree); + // If there is a CMakeLists.txt file, use it as the source tree. if (!listPath.empty()) { - // When invoked with a path that points to an existing CMakeCache // This function is called multiple times with the same path if (no_source_tree && no_build_tree) { @@ -1527,13 +1547,20 @@ void cmake::SetDirectoriesFromFile(const std::string& arg) std::string full = cmSystemTools::CollapseFullPath(arg); this->SetHomeDirectory(full); } - if (no_build_tree) { + if (no_build_tree && !no_source_tree && is_empty_directory) { + // passed `-S when build_dir is an empty directory + std::string full = cmSystemTools::CollapseFullPath(arg); + this->SetHomeOutputDirectory(full); + } else if (no_build_tree) { // We didn't find a CMakeCache.txt and it wasn't specified // with -B. Assume the current working directory as the build tree. std::string cwd = cmSystemTools::GetCurrentWorkingDirectory(); this->SetHomeOutputDirectory(cwd); + used_provided_path = false; } } + + return used_provided_path; } // at the end of this CMAKE_ROOT and CMAKE_COMMAND should be added to the diff --git a/Source/cmake.h b/Source/cmake.h index b13cc96..aee9c04 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -648,7 +648,7 @@ protected: */ int CheckBuildSystem(); - void SetDirectoriesFromFile(const std::string& arg); + bool SetDirectoriesFromFile(const std::string& arg); //! Make sure all commands are what they say they are and there is no /// macros. diff --git a/Tests/RunCMake/CommandLine/B-S-extra-path-stderr.txt b/Tests/RunCMake/CommandLine/B-S-extra-path-stderr.txt new file mode 100644 index 0000000..8794f74 --- /dev/null +++ b/Tests/RunCMake/CommandLine/B-S-extra-path-stderr.txt @@ -0,0 +1,4 @@ +^CMake Warning: + Ignoring extra path from command line: + + /extra/path/$ diff --git a/Tests/RunCMake/CommandLine/RunCMakeTest.cmake b/Tests/RunCMake/CommandLine/RunCMakeTest.cmake index 313b579..033fbe6 100644 --- a/Tests/RunCMake/CommandLine/RunCMakeTest.cmake +++ b/Tests/RunCMake/CommandLine/RunCMakeTest.cmake @@ -143,6 +143,10 @@ endif() run_cmake_with_options(S-arg-build-dir-not-created -S ${source_dir} build/) run_cmake_with_options(S-arg-reverse-build-dir-not-created build/ -S${source_dir} ) + file(REMOVE_RECURSE "${source_dir}/build") + file(MAKE_DIRECTORY "${source_dir}/build") + run_cmake_with_options(S-arg-build-dir-empty -S ${source_dir} build/) + set(source_dir ${RunCMake_SOURCE_DIR}/ExplicitDirs) set(binary_dir ${RunCMake_BINARY_DIR}/ExplicitDirs-build) @@ -155,6 +159,7 @@ endif() run_cmake_with_options(S-no-arg -S ) run_cmake_with_options(S-no-arg2 -S -T) run_cmake_with_options(S-B -S ${source_dir} -B ${binary_dir}) + run_cmake_with_options(S-B-extra-path -S ${source_dir} -B ${binary_dir} /extra/path/) # make sure that -B can explicitly construct build directories file(REMOVE_RECURSE "${binary_dir}") @@ -165,6 +170,7 @@ endif() run_cmake_with_options(B-no-arg2 -B -T) file(REMOVE_RECURSE "${binary_dir}") run_cmake_with_options(B-S -B${binary_dir} -S${source_dir}) + run_cmake_with_options(B-S-extra-path -B${binary_dir} -S${source_dir} /extra/path/) message("copied to ${RunCMake_TEST_BINARY_DIR}/initial-cache.txt") file(COPY ${RunCMake_SOURCE_DIR}/C_buildsrcdir/initial-cache.txt DESTINATION ${RunCMake_TEST_BINARY_DIR}) diff --git a/Tests/RunCMake/CommandLine/S-B-extra-path-stderr.txt b/Tests/RunCMake/CommandLine/S-B-extra-path-stderr.txt new file mode 100644 index 0000000..8794f74 --- /dev/null +++ b/Tests/RunCMake/CommandLine/S-B-extra-path-stderr.txt @@ -0,0 +1,4 @@ +^CMake Warning: + Ignoring extra path from command line: + + /extra/path/$ -- cgit v0.12