From 700abe7bca0486a2d67383b2e8e364db241566b5 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Tue, 13 Jul 2021 00:51:59 +0300 Subject: [PATCH 01/10] Refactor: Drop useless assignments of `retval` before return --- Source/CPack/cmCPackDebGenerator.cxx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Source/CPack/cmCPackDebGenerator.cxx b/Source/CPack/cmCPackDebGenerator.cxx index db30a0d663..14f3f50cbd 100644 --- a/Source/CPack/cmCPackDebGenerator.cxx +++ b/Source/CPack/cmCPackDebGenerator.cxx @@ -508,8 +508,7 @@ int cmCPackDebGenerator::PackageOnePack(std::string const& initialTopLevel, if (!this->ReadListFile("Internal/CPack/CPackDeb.cmake")) { cmCPackLogger(cmCPackLog::LOG_ERROR, "Error while execution CPackDeb.cmake" << std::endl); - retval = 0; - return retval; + return 0; } { // Isolate globbing of binaries vs. dbgsyms @@ -660,8 +659,7 @@ int cmCPackDebGenerator::PackageComponentsAllInOne( if (!this->ReadListFile("Internal/CPack/CPackDeb.cmake")) { cmCPackLogger(cmCPackLog::LOG_ERROR, "Error while execution CPackDeb.cmake" << std::endl); - retval = 0; - return retval; + return 0; } cmsys::Glob gl; From 45a6fa0c336d4fcecde8017aafa626c7eceec074 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Tue, 13 Jul 2021 00:56:10 +0300 Subject: [PATCH 02/10] Refactor: Drop unnecessary `if` before `return` --- Source/CPack/cmCPackDebGenerator.cxx | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/Source/CPack/cmCPackDebGenerator.cxx b/Source/CPack/cmCPackDebGenerator.cxx index 14f3f50cbd..43d732eef8 100644 --- a/Source/CPack/cmCPackDebGenerator.cxx +++ b/Source/CPack/cmCPackDebGenerator.cxx @@ -834,10 +834,7 @@ int cmCPackDebGenerator::createDeb() this->IsSet("GEN_CPACK_DEBIAN_PACKAGE_CONTROL_STRICT_PERMISSION"), this->packageFiles); - if (!gen.generate()) { - return 0; - } - return 1; + return gen.generate(); } int cmCPackDebGenerator::createDbgsymDDeb() @@ -890,10 +887,7 @@ int cmCPackDebGenerator::createDbgsymDDeb() this->IsSet("GEN_CPACK_DEBIAN_PACKAGE_CONTROL_STRICT_PERMISSION"), this->packageFiles); - if (!gen.generate()) { - return 0; - } - return 1; + return gen.generate(); } bool cmCPackDebGenerator::SupportsComponentInstallation() const From 008321595afed33352bfa575894251e9ac3ed1bd Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Tue, 13 Jul 2021 01:10:33 +0300 Subject: [PATCH 03/10] Refactor: Change return value to bool for private members The `cmCPackDebGenerator::createDeb()` and `cmCPackDebGenerator::createDbgsymDDeb()` in fact have boolean return value. --- Source/CPack/cmCPackDebGenerator.cxx | 25 +++++++------------------ Source/CPack/cmCPackDebGenerator.h | 4 ++-- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/Source/CPack/cmCPackDebGenerator.cxx b/Source/CPack/cmCPackDebGenerator.cxx index 43d732eef8..0216a4ac51 100644 --- a/Source/CPack/cmCPackDebGenerator.cxx +++ b/Source/CPack/cmCPackDebGenerator.cxx @@ -481,7 +481,6 @@ int cmCPackDebGenerator::InitializeInternal() int cmCPackDebGenerator::PackageOnePack(std::string const& initialTopLevel, std::string const& packageName) { - int retval = 1; // Begin the archive for this pack std::string localToplevel(initialTopLevel); std::string packageFileName( @@ -529,10 +528,7 @@ int cmCPackDebGenerator::PackageOnePack(std::string const& initialTopLevel, std::sort(this->packageFiles.begin(), this->packageFiles.end()); } - int res = this->createDeb(); - if (res != 1) { - retval = 0; - } + bool retval = this->createDeb(); // add the generated package to package file names list packageFileName = cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', this->GetOption("GEN_CPACK_OUTPUT_FILE_NAME")); @@ -556,10 +552,7 @@ int cmCPackDebGenerator::PackageOnePack(std::string const& initialTopLevel, // Sort files so that they have a reproducible order std::sort(this->packageFiles.begin(), this->packageFiles.end()); - res = this->createDbgsymDDeb(); - if (res != 1) { - retval = 0; - } + retval = this->createDbgsymDDeb() || retval; // add the generated package to package file names list packageFileName = cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', @@ -567,7 +560,7 @@ int cmCPackDebGenerator::PackageOnePack(std::string const& initialTopLevel, this->packageFileNames.push_back(std::move(packageFileName)); } - return retval; + return int(retval); } int cmCPackDebGenerator::PackageComponents(bool ignoreGroup) @@ -616,7 +609,6 @@ int cmCPackDebGenerator::PackageComponents(bool ignoreGroup) int cmCPackDebGenerator::PackageComponentsAllInOne( const std::string& compInstDirName) { - int retval = 1; /* Reset package file name list it will be populated during the * component packaging run*/ this->packageFileNames.clear(); @@ -678,15 +670,12 @@ int cmCPackDebGenerator::PackageComponentsAllInOne( // Sort files so that they have a reproducible order std::sort(this->packageFiles.begin(), this->packageFiles.end()); - int res = this->createDeb(); - if (res != 1) { - retval = 0; - } + bool retval = this->createDeb(); // add the generated package to package file names list packageFileName = cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', this->GetOption("GEN_CPACK_OUTPUT_FILE_NAME")); this->packageFileNames.push_back(std::move(packageFileName)); - return retval; + return int(retval); } int cmCPackDebGenerator::PackageFiles() @@ -710,7 +699,7 @@ int cmCPackDebGenerator::PackageFiles() return this->PackageComponentsAllInOne(""); } -int cmCPackDebGenerator::createDeb() +bool cmCPackDebGenerator::createDeb() { std::map controlValues; @@ -837,7 +826,7 @@ int cmCPackDebGenerator::createDeb() return gen.generate(); } -int cmCPackDebGenerator::createDbgsymDDeb() +bool cmCPackDebGenerator::createDbgsymDDeb() { // Packages containing debug symbols follow the same structure as .debs // but have different metadata and content. diff --git a/Source/CPack/cmCPackDebGenerator.h b/Source/CPack/cmCPackDebGenerator.h index ee8f39af6e..502746cf5e 100644 --- a/Source/CPack/cmCPackDebGenerator.h +++ b/Source/CPack/cmCPackDebGenerator.h @@ -63,8 +63,8 @@ protected: const std::string& componentName) override; private: - int createDeb(); - int createDbgsymDDeb(); + bool createDeb(); + bool createDbgsymDDeb(); std::vector packageFiles; }; From 7fd3134ea16d2d1fbf08fa5a5d28059d62067ad6 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Tue, 13 Jul 2021 01:48:38 +0300 Subject: [PATCH 04/10] Refactor: cmCPackDebGenerator::PackageComponents handle `else` first Also, return early to reduce nesting level of the function body. --- Source/CPack/cmCPackDebGenerator.cxx | 52 ++++++++++++++-------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/Source/CPack/cmCPackDebGenerator.cxx b/Source/CPack/cmCPackDebGenerator.cxx index 0216a4ac51..fef8bde853 100644 --- a/Source/CPack/cmCPackDebGenerator.cxx +++ b/Source/CPack/cmCPackDebGenerator.cxx @@ -565,40 +565,40 @@ int cmCPackDebGenerator::PackageOnePack(std::string const& initialTopLevel, int cmCPackDebGenerator::PackageComponents(bool ignoreGroup) { - int retval = 1; - /* Reset package file name list it will be populated during the - * component packaging run*/ + // Reset package file name list it will be populated during the + // component packaging run this->packageFileNames.clear(); std::string initialTopLevel(this->GetOption("CPACK_TEMPORARY_DIRECTORY")); + int retval = 1; // The default behavior is to have one package by component group // unless CPACK_COMPONENTS_IGNORE_GROUP is specified. - if (!ignoreGroup) { - for (auto const& compG : this->ComponentGroups) { - cmCPackLogger(cmCPackLog::LOG_VERBOSE, - "Packaging component group: " << compG.first << std::endl); - // Begin the archive for this group - retval &= this->PackageOnePack(initialTopLevel, compG.first); - } - // Handle Orphan components (components not belonging to any groups) + if (ignoreGroup) { + // CPACK_COMPONENTS_IGNORE_GROUPS is set + // We build 1 package per component for (auto const& comp : this->Components) { - // Does the component belong to a group? - if (comp.second.Group == nullptr) { - cmCPackLogger( - cmCPackLog::LOG_VERBOSE, - "Component <" - << comp.second.Name - << "> does not belong to any group, package it separately." - << std::endl); - // Begin the archive for this orphan component - retval &= this->PackageOnePack(initialTopLevel, comp.first); - } + retval &= this->PackageOnePack(initialTopLevel, comp.first); } + return retval; } - // CPACK_COMPONENTS_IGNORE_GROUPS is set - // We build 1 package per component - else { - for (auto const& comp : this->Components) { + + for (auto const& compG : this->ComponentGroups) { + cmCPackLogger(cmCPackLog::LOG_VERBOSE, + "Packaging component group: " << compG.first << std::endl); + // Begin the archive for this group + retval &= this->PackageOnePack(initialTopLevel, compG.first); + } + // Handle Orphan components (components not belonging to any groups) + for (auto const& comp : this->Components) { + // Does the component belong to a group? + if (comp.second.Group == nullptr) { + cmCPackLogger( + cmCPackLog::LOG_VERBOSE, + "Component <" + << comp.second.Name + << "> does not belong to any group, package it separately." + << std::endl); + // Begin the archive for this orphan component retval &= this->PackageOnePack(initialTopLevel, comp.first); } } From c8f298ae0808675ddd70341f7c83348bc092fb3f Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Tue, 13 Jul 2021 02:29:44 +0300 Subject: [PATCH 05/10] Refactor: Extract packaged files finder into a function --- Source/CPack/cmCPackDebGenerator.cxx | 79 ++++++++++++---------------- 1 file changed, 34 insertions(+), 45 deletions(-) diff --git a/Source/CPack/cmCPackDebGenerator.cxx b/Source/CPack/cmCPackDebGenerator.cxx index fef8bde853..70670af0b8 100644 --- a/Source/CPack/cmCPackDebGenerator.cxx +++ b/Source/CPack/cmCPackDebGenerator.cxx @@ -8,6 +8,7 @@ #include #include #include +#include #include #include "cmsys/Glob.hxx" @@ -463,6 +464,23 @@ bool DebGenerator::generateDeb() const return true; } +std::vector findFilesIn(const std::string& path) +{ + cmsys::Glob gl; + std::string findExpr = path + "/*"; + gl.RecurseOn(); + gl.SetRecurseListDirs(true); + gl.SetRecurseThroughSymlinks(false); + if (!gl.FindFiles(findExpr)) { + throw std::runtime_error( + "Cannot find any files in the installed directory"); + } + std::vector files{ gl.GetFiles() }; + // Sort files so that they have a reproducible order + std::sort(files.begin(), files.end()); + return files; +} + } // end anonymous namespace cmCPackDebGenerator::cmCPackDebGenerator() = default; @@ -510,54 +528,34 @@ int cmCPackDebGenerator::PackageOnePack(std::string const& initialTopLevel, return 0; } - { // Isolate globbing of binaries vs. dbgsyms - cmsys::Glob gl; - std::string findExpr(this->GetOption("GEN_WDIR")); - findExpr += "/*"; - gl.RecurseOn(); - gl.SetRecurseListDirs(true); - gl.SetRecurseThroughSymlinks(false); - if (!gl.FindFiles(findExpr)) { - cmCPackLogger(cmCPackLog::LOG_ERROR, - "Cannot find any files in the installed directory" - << std::endl); - return 0; - } - this->packageFiles = gl.GetFiles(); - // Sort files so that they have a reproducible order - std::sort(this->packageFiles.begin(), this->packageFiles.end()); + try { + this->packageFiles = findFilesIn(this->GetOption("GEN_WDIR")); + } catch (const std::runtime_error& ex) { + cmCPackLogger(cmCPackLog::LOG_ERROR, ex.what() << std::endl); + return 0; } bool retval = this->createDeb(); // add the generated package to package file names list packageFileName = cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', this->GetOption("GEN_CPACK_OUTPUT_FILE_NAME")); - this->packageFileNames.push_back(std::move(packageFileName)); + this->packageFileNames.emplace_back(std::move(packageFileName)); if (this->IsOn("GEN_CPACK_DEBIAN_DEBUGINFO_PACKAGE") && this->GetOption("GEN_DBGSYMDIR")) { - cmsys::Glob gl; - std::string findExpr(this->GetOption("GEN_DBGSYMDIR")); - findExpr += "/*"; - gl.RecurseOn(); - gl.SetRecurseListDirs(true); - gl.SetRecurseThroughSymlinks(false); - if (!gl.FindFiles(findExpr)) { - cmCPackLogger(cmCPackLog::LOG_ERROR, - "Cannot find any files in the installed directory" - << std::endl); + try { + this->packageFiles = findFilesIn(this->GetOption("GEN_DBGSYMDIR")); + } catch (const std::runtime_error& ex) { + cmCPackLogger(cmCPackLog::LOG_ERROR, ex.what() << std::endl); return 0; } - this->packageFiles = gl.GetFiles(); - // Sort files so that they have a reproducible order - std::sort(this->packageFiles.begin(), this->packageFiles.end()); retval = this->createDbgsymDDeb() || retval; // add the generated package to package file names list packageFileName = cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', this->GetOption("GEN_CPACK_DBGSYM_OUTPUT_FILE_NAME")); - this->packageFileNames.push_back(std::move(packageFileName)); + this->packageFileNames.emplace_back(std::move(packageFileName)); } return int(retval); @@ -654,27 +652,18 @@ int cmCPackDebGenerator::PackageComponentsAllInOne( return 0; } - cmsys::Glob gl; - std::string findExpr(this->GetOption("GEN_WDIR")); - findExpr += "/*"; - gl.RecurseOn(); - gl.SetRecurseListDirs(true); - gl.SetRecurseThroughSymlinks(false); - if (!gl.FindFiles(findExpr)) { - cmCPackLogger(cmCPackLog::LOG_ERROR, - "Cannot find any files in the installed directory" - << std::endl); + try { + this->packageFiles = findFilesIn(this->GetOption("GEN_WDIR")); + } catch (const std::runtime_error& ex) { + cmCPackLogger(cmCPackLog::LOG_ERROR, ex.what() << std::endl); return 0; } - this->packageFiles = gl.GetFiles(); - // Sort files so that they have a reproducible order - std::sort(this->packageFiles.begin(), this->packageFiles.end()); bool retval = this->createDeb(); // add the generated package to package file names list packageFileName = cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', this->GetOption("GEN_CPACK_OUTPUT_FILE_NAME")); - this->packageFileNames.push_back(std::move(packageFileName)); + this->packageFileNames.emplace_back(std::move(packageFileName)); return int(retval); } From 593ff734b08779bc2ac3f7d5975c2d12dfa5614b Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Tue, 13 Jul 2021 02:58:17 +0300 Subject: [PATCH 06/10] CPack/DEB: dbgsym package not generated for non-component packaging Fix: #19735 --- Source/CPack/cmCPackDebGenerator.cxx | 81 +++++++++---------- Source/CPack/cmCPackDebGenerator.h | 1 + Tests/RunCMake/CPack/RunCMakeTest.cmake | 1 + .../CPack/tests/DEBUGINFO/ExpectedFiles.cmake | 68 ++++++++++------ .../RunCMake/CPack/tests/DEBUGINFO/test.cmake | 2 + 5 files changed, 82 insertions(+), 71 deletions(-) diff --git a/Source/CPack/cmCPackDebGenerator.cxx b/Source/CPack/cmCPackDebGenerator.cxx index 70670af0b8..cbdcf49c38 100644 --- a/Source/CPack/cmCPackDebGenerator.cxx +++ b/Source/CPack/cmCPackDebGenerator.cxx @@ -528,37 +528,7 @@ int cmCPackDebGenerator::PackageOnePack(std::string const& initialTopLevel, return 0; } - try { - this->packageFiles = findFilesIn(this->GetOption("GEN_WDIR")); - } catch (const std::runtime_error& ex) { - cmCPackLogger(cmCPackLog::LOG_ERROR, ex.what() << std::endl); - return 0; - } - - bool retval = this->createDeb(); - // add the generated package to package file names list - packageFileName = cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', - this->GetOption("GEN_CPACK_OUTPUT_FILE_NAME")); - this->packageFileNames.emplace_back(std::move(packageFileName)); - - if (this->IsOn("GEN_CPACK_DEBIAN_DEBUGINFO_PACKAGE") && - this->GetOption("GEN_DBGSYMDIR")) { - try { - this->packageFiles = findFilesIn(this->GetOption("GEN_DBGSYMDIR")); - } catch (const std::runtime_error& ex) { - cmCPackLogger(cmCPackLog::LOG_ERROR, ex.what() << std::endl); - return 0; - } - - retval = this->createDbgsymDDeb() || retval; - // add the generated package to package file names list - packageFileName = - cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', - this->GetOption("GEN_CPACK_DBGSYM_OUTPUT_FILE_NAME")); - this->packageFileNames.emplace_back(std::move(packageFileName)); - } - - return int(retval); + return this->createDebPackages(); } int cmCPackDebGenerator::PackageComponents(bool ignoreGroup) @@ -652,19 +622,7 @@ int cmCPackDebGenerator::PackageComponentsAllInOne( return 0; } - try { - this->packageFiles = findFilesIn(this->GetOption("GEN_WDIR")); - } catch (const std::runtime_error& ex) { - cmCPackLogger(cmCPackLog::LOG_ERROR, ex.what() << std::endl); - return 0; - } - - bool retval = this->createDeb(); - // add the generated package to package file names list - packageFileName = cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', - this->GetOption("GEN_CPACK_OUTPUT_FILE_NAME")); - this->packageFileNames.emplace_back(std::move(packageFileName)); - return int(retval); + return this->createDebPackages(); } int cmCPackDebGenerator::PackageFiles() @@ -688,6 +646,40 @@ int cmCPackDebGenerator::PackageFiles() return this->PackageComponentsAllInOne(""); } +bool cmCPackDebGenerator::createDebPackages() +{ + try { + this->packageFiles = findFilesIn(this->GetOption("GEN_WDIR")); + } catch (const std::runtime_error& ex) { + cmCPackLogger(cmCPackLog::LOG_ERROR, ex.what() << std::endl); + return 0; + } + + bool retval = this->createDeb(); + // add the generated package to package file names list + this->packageFileNames.emplace_back( + cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', + this->GetOption("GEN_CPACK_OUTPUT_FILE_NAME"))); + + if (this->IsOn("GEN_CPACK_DEBIAN_DEBUGINFO_PACKAGE") && + this->GetOption("GEN_DBGSYMDIR")) { + try { + this->packageFiles = findFilesIn(this->GetOption("GEN_DBGSYMDIR")); + } catch (const std::runtime_error& ex) { + cmCPackLogger(cmCPackLog::LOG_ERROR, ex.what() << std::endl); + return 0; + } + + retval = this->createDbgsymDDeb() || retval; + // add the generated package to package file names list + this->packageFileNames.emplace_back( + cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', + this->GetOption("GEN_CPACK_DBGSYM_OUTPUT_FILE_NAME"))); + } + + return int(retval); +} + bool cmCPackDebGenerator::createDeb() { std::map controlValues; @@ -855,7 +847,6 @@ bool cmCPackDebGenerator::createDbgsymDDeb() DebGenerator gen( this->Logger, this->GetOption("GEN_CPACK_DBGSYM_OUTPUT_FILE_NAME"), this->GetOption("GEN_DBGSYMDIR"), - this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), this->GetOption("CPACK_TEMPORARY_DIRECTORY"), this->GetOption("GEN_CPACK_DEBIAN_COMPRESSION_TYPE"), diff --git a/Source/CPack/cmCPackDebGenerator.h b/Source/CPack/cmCPackDebGenerator.h index 502746cf5e..61a6616d3f 100644 --- a/Source/CPack/cmCPackDebGenerator.h +++ b/Source/CPack/cmCPackDebGenerator.h @@ -63,6 +63,7 @@ protected: const std::string& componentName) override; private: + bool createDebPackages(); bool createDeb(); bool createDbgsymDDeb(); diff --git a/Tests/RunCMake/CPack/RunCMakeTest.cmake b/Tests/RunCMake/CPack/RunCMakeTest.cmake index 746ff8b1c0..7997c785f0 100644 --- a/Tests/RunCMake/CPack/RunCMakeTest.cmake +++ b/Tests/RunCMake/CPack/RunCMakeTest.cmake @@ -7,6 +7,7 @@ include("${RunCMake_SOURCE_DIR}/CPackTestHelpers.cmake") run_cpack_test(CUSTOM_BINARY_SPEC_FILE "RPM.CUSTOM_BINARY_SPEC_FILE" false "MONOLITHIC;COMPONENT") run_cpack_test(CUSTOM_NAMES "RPM.CUSTOM_NAMES;DEB.CUSTOM_NAMES;TGZ;DragNDrop" true "COMPONENT") run_cpack_test(DEBUGINFO "RPM.DEBUGINFO;DEB.DEBUGINFO" true "COMPONENT") +run_cpack_test(DEBUGINFO "DEB.DEBUGINFO" true "MONOLITHIC") run_cpack_test_subtests(DEFAULT_PERMISSIONS "CMAKE_var_set;CPACK_var_set;both_set;invalid_CMAKE_var;invalid_CPACK_var" "RPM.DEFAULT_PERMISSIONS;DEB.DEFAULT_PERMISSIONS" false "MONOLITHIC;COMPONENT") run_cpack_test(DEPENDENCIES "RPM.DEPENDENCIES;DEB.DEPENDENCIES" true "COMPONENT") run_cpack_test(DIST "RPM.DIST" false "MONOLITHIC") diff --git a/Tests/RunCMake/CPack/tests/DEBUGINFO/ExpectedFiles.cmake b/Tests/RunCMake/CPack/tests/DEBUGINFO/ExpectedFiles.cmake index cf4aa510ac..b3e64857fb 100644 --- a/Tests/RunCMake/CPack/tests/DEBUGINFO/ExpectedFiles.cmake +++ b/Tests/RunCMake/CPack/tests/DEBUGINFO/ExpectedFiles.cmake @@ -1,8 +1,5 @@ set(whitespaces_ "[\t\n\r ]*") -set(EXPECTED_FILES_COUNT "6") -set(EXPECTED_FILES_NAME_GENERATOR_SPECIFIC_FORMAT TRUE) - if(GENERATOR_TYPE STREQUAL "RPM") set(NAME "Debuginfo") set(DEBUG_SUFFIX "debuginfo") @@ -15,30 +12,49 @@ elseif(GENERATOR_TYPE STREQUAL "DEB") set(DEBUG_PKG "ddeb") endif() -set(EXPECTED_FILE_1_NAME "${NAME}") -set(EXPECTED_FILE_1_COMPONENT "applications") -set(EXPECTED_FILE_CONTENT_1_LIST "/foo;/foo/test_prog") +set(EXPECTED_FILES_NAME_GENERATOR_SPECIFIC_FORMAT TRUE) -set(EXPECTED_FILE_2 "TestDinfo-pkg*-headers.${PKG}") -set(EXPECTED_FILE_CONTENT_2_LIST "/bar;/bar/CMakeLists.txt") +if(PACKAGING_TYPE STREQUAL "COMPONENT") + set(EXPECTED_FILES_COUNT "6") -set(EXPECTED_FILE_3 "TestDinfo-pkg*-libs.${PKG}") -set(EXPECTED_FILE_CONTENT_3_LIST "/bas;/bas/libtest_lib.so") + set(EXPECTED_FILE_1_NAME "${NAME}") + set(EXPECTED_FILE_1_COMPONENT "applications") + set(EXPECTED_FILE_CONTENT_1_LIST "/foo;/foo/test_prog") + + set(EXPECTED_FILE_2 "TestDinfo-pkg*-headers.${PKG}") + set(EXPECTED_FILE_CONTENT_2_LIST "/bar;/bar/CMakeLists.txt") + + set(EXPECTED_FILE_3 "TestDinfo-pkg*-libs.${PKG}") + set(EXPECTED_FILE_CONTENT_3_LIST "/bas;/bas/libtest_lib.so") + + set(EXPECTED_FILE_4 "${NAME}-applications-${DEBUG_SUFFIX}*.${DEBUG_PKG}") + if(GENERATOR_TYPE STREQUAL "RPM") + set(EXPECTED_FILE_CONTENT_4 ".*/src${whitespaces_}/src/src_1${whitespaces_}/src/src_1/main.cpp.*\.debug.*") + elseif(GENERATOR_TYPE STREQUAL "DEB") + set(EXPECTED_FILE_CONTENT_4 ".*/usr/lib/debug/.build-id/.*\.debug.*") + endif() + + if(GENERATOR_TYPE STREQUAL "RPM") + set(EXPECTED_FILE_5 "libs-DebugInfoPackage.rpm") + set(EXPECTED_FILE_CONTENT_5 ".*/src${whitespaces_}/src/src_1${whitespaces_}/src/src_1/test_lib.cpp.*\.debug.*") + elseif(GENERATOR_TYPE STREQUAL "DEB") + set(EXPECTED_FILE_5 "TestDinfo-pkg-libs-dbgsym.ddeb") + set(EXPECTED_FILE_CONTENT_5 ".*/usr/lib/debug/.build-id/.*\.debug.*") + endif() + + set(EXPECTED_FILE_6 "TestDinfo-pkg*-appheaders.${PKG}") + set(EXPECTED_FILE_CONTENT_6_LIST "/include;/include/test_lib.hpp") + +elseif(PACKAGING_TYPE STREQUAL "MONOLITHIC" AND GENERATOR_TYPE STREQUAL "DEB") + set(EXPECTED_FILES_COUNT "2") + + set(EXPECTED_FILE_1 "TestDinfo-pkg.deb") + set( + EXPECTED_FILE_CONTENT_1_LIST + "/bar;/bar/CMakeLists.txt;/bas;/bas/libtest_lib.so;/foo;/foo/test_prog;/include;/include/test_lib.hpp" + ) + + set(EXPECTED_FILE_2 "TestDinfo-pkg-dbgsym.ddeb") + set(EXPECTED_FILE_CONTENT_2 ".*/usr/lib/debug/.build-id/.*\.debug.*") -set(EXPECTED_FILE_4 "${NAME}-applications-${DEBUG_SUFFIX}*.${DEBUG_PKG}") -if(GENERATOR_TYPE STREQUAL "RPM") - set(EXPECTED_FILE_CONTENT_4 ".*/src${whitespaces_}/src/src_1${whitespaces_}/src/src_1/main.cpp.*\.debug.*") -elseif(GENERATOR_TYPE STREQUAL "DEB") - set(EXPECTED_FILE_CONTENT_4 ".*/usr/lib/debug/.build-id/.*\.debug.*") endif() - -if(GENERATOR_TYPE STREQUAL "RPM") - set(EXPECTED_FILE_5 "libs-DebugInfoPackage.rpm") - set(EXPECTED_FILE_CONTENT_5 ".*/src${whitespaces_}/src/src_1${whitespaces_}/src/src_1/test_lib.cpp.*\.debug.*") -elseif(GENERATOR_TYPE STREQUAL "DEB") - set(EXPECTED_FILE_5 "TestDinfo-pkg-libs-dbgsym.ddeb") - set(EXPECTED_FILE_CONTENT_5 ".*/usr/lib/debug/.build-id/.*\.debug.*") -endif() - -set(EXPECTED_FILE_6 "TestDinfo-pkg*-appheaders.${PKG}") -set(EXPECTED_FILE_CONTENT_6_LIST "/include;/include/test_lib.hpp") diff --git a/Tests/RunCMake/CPack/tests/DEBUGINFO/test.cmake b/Tests/RunCMake/CPack/tests/DEBUGINFO/test.cmake index 9ff1f8a049..e9cebbf8d6 100644 --- a/Tests/RunCMake/CPack/tests/DEBUGINFO/test.cmake +++ b/Tests/RunCMake/CPack/tests/DEBUGINFO/test.cmake @@ -28,6 +28,8 @@ install(TARGETS test_prog DESTINATION foo COMPONENT applications) install(FILES CMakeLists.txt DESTINATION bar COMPONENT headers) install(TARGETS test_lib DESTINATION bas COMPONENT libs) +set(CPACK_DEBIAN_DEBUGINFO_PACKAGE ON) + set(CPACK_RPM_APPLICATIONS_FILE_NAME "RPM-DEFAULT") set(CPACK_RPM_APPLICATIONS_DEBUGINFO_PACKAGE ON) set(CPACK_DEBIAN_APPLICATIONS_FILE_NAME "DEB-DEFAULT") From 7add10f288e956d45781726784867847c268efa4 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Tue, 13 Jul 2021 04:50:32 +0300 Subject: [PATCH 07/10] Refactor: Deduplicate code of `createDebPackages()` Also, fix incorrect `retval` accumulation. --- Source/CPack/cmCPackDebGenerator.cxx | 41 +++++++++++++--------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/Source/CPack/cmCPackDebGenerator.cxx b/Source/CPack/cmCPackDebGenerator.cxx index cbdcf49c38..210ef6424f 100644 --- a/Source/CPack/cmCPackDebGenerator.cxx +++ b/Source/CPack/cmCPackDebGenerator.cxx @@ -648,35 +648,32 @@ int cmCPackDebGenerator::PackageFiles() bool cmCPackDebGenerator::createDebPackages() { - try { - this->packageFiles = findFilesIn(this->GetOption("GEN_WDIR")); - } catch (const std::runtime_error& ex) { - cmCPackLogger(cmCPackLog::LOG_ERROR, ex.what() << std::endl); - return 0; - } - - bool retval = this->createDeb(); - // add the generated package to package file names list - this->packageFileNames.emplace_back( - cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', - this->GetOption("GEN_CPACK_OUTPUT_FILE_NAME"))); - - if (this->IsOn("GEN_CPACK_DEBIAN_DEBUGINFO_PACKAGE") && - this->GetOption("GEN_DBGSYMDIR")) { + auto make_package = [this](const char* const path, const char* const output, + bool (cmCPackDebGenerator::*creator)()) -> bool { try { - this->packageFiles = findFilesIn(this->GetOption("GEN_DBGSYMDIR")); + this->packageFiles = findFilesIn(this->GetOption(path)); } catch (const std::runtime_error& ex) { cmCPackLogger(cmCPackLog::LOG_ERROR, ex.what() << std::endl); return 0; } - retval = this->createDbgsymDDeb() || retval; - // add the generated package to package file names list - this->packageFileNames.emplace_back( - cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', - this->GetOption("GEN_CPACK_DBGSYM_OUTPUT_FILE_NAME"))); + if ((this->*creator)()) { + // add the generated package to package file names list + this->packageFileNames.emplace_back( + cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', + this->GetOption(output))); + return true; + } + return false; + }; + bool retval = make_package("GEN_WDIR", "GEN_CPACK_OUTPUT_FILE_NAME", + &cmCPackDebGenerator::createDeb); + if (this->IsOn("GEN_CPACK_DEBIAN_DEBUGINFO_PACKAGE") && + this->GetOption("GEN_DBGSYMDIR")) { + retval = make_package("GEN_DBGSYMDIR", "GEN_CPACK_DBGSYM_OUTPUT_FILE_NAME", + &cmCPackDebGenerator::createDbgsymDDeb) && + retval; } - return int(retval); } From 13549674cc8f2b72380aa4a367fef3bce26a5131 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Tue, 13 Jul 2021 05:03:54 +0300 Subject: [PATCH 08/10] Refactor: Avoid duplicate calls to `GetOption("GEN_DBGSYMDIR")` --- Source/CPack/cmCPackDebGenerator.cxx | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/Source/CPack/cmCPackDebGenerator.cxx b/Source/CPack/cmCPackDebGenerator.cxx index 210ef6424f..675f6f8cd4 100644 --- a/Source/CPack/cmCPackDebGenerator.cxx +++ b/Source/CPack/cmCPackDebGenerator.cxx @@ -648,29 +648,31 @@ int cmCPackDebGenerator::PackageFiles() bool cmCPackDebGenerator::createDebPackages() { - auto make_package = [this](const char* const path, const char* const output, + auto make_package = [this](const char* const path, + const char* const output_var, bool (cmCPackDebGenerator::*creator)()) -> bool { try { - this->packageFiles = findFilesIn(this->GetOption(path)); + this->packageFiles = findFilesIn(path); } catch (const std::runtime_error& ex) { cmCPackLogger(cmCPackLog::LOG_ERROR, ex.what() << std::endl); - return 0; + return false; } if ((this->*creator)()) { // add the generated package to package file names list this->packageFileNames.emplace_back( cmStrCat(this->GetOption("CPACK_TOPLEVEL_DIRECTORY"), '/', - this->GetOption(output))); + this->GetOption(output_var))); return true; } return false; }; - bool retval = make_package("GEN_WDIR", "GEN_CPACK_OUTPUT_FILE_NAME", - &cmCPackDebGenerator::createDeb); - if (this->IsOn("GEN_CPACK_DEBIAN_DEBUGINFO_PACKAGE") && - this->GetOption("GEN_DBGSYMDIR")) { - retval = make_package("GEN_DBGSYMDIR", "GEN_CPACK_DBGSYM_OUTPUT_FILE_NAME", + bool retval = + make_package(this->GetOption("GEN_WDIR"), "GEN_CPACK_OUTPUT_FILE_NAME", + &cmCPackDebGenerator::createDeb); + const char* const dbgsymdir_path = this->GetOption("GEN_DBGSYMDIR"); + if (this->IsOn("GEN_CPACK_DEBIAN_DEBUGINFO_PACKAGE") && dbgsymdir_path) { + retval = make_package(dbgsymdir_path, "GEN_CPACK_DBGSYM_OUTPUT_FILE_NAME", &cmCPackDebGenerator::createDbgsymDDeb) && retval; } From afcc5449e877c122bff03cffa1f70c70196817ad Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Tue, 13 Jul 2021 05:49:22 +0300 Subject: [PATCH 09/10] Refactor: Use `cmStrToLong` instead of `std::strtol` --- Source/CPack/cmCPackDebGenerator.cxx | 35 ++++++++++++++-------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/Source/CPack/cmCPackDebGenerator.cxx b/Source/CPack/cmCPackDebGenerator.cxx index 675f6f8cd4..a93cd98e06 100644 --- a/Source/CPack/cmCPackDebGenerator.cxx +++ b/Source/CPack/cmCPackDebGenerator.cxx @@ -3,7 +3,6 @@ #include "cmCPackDebGenerator.h" #include -#include #include #include #include @@ -56,7 +55,7 @@ private: const std::string TopLevelDir; const std::string TemporaryDir; const char* DebianArchiveType; - int NumThreads; + long NumThreads; const std::map ControlValues; const bool GenShLibs; const std::string ShLibsFilename; @@ -100,19 +99,19 @@ DebGenerator::DebGenerator( debianCompressionType = "gzip"; } - if (!strcmp(debianCompressionType, "lzma")) { + if (!std::strcmp(debianCompressionType, "lzma")) { this->CompressionSuffix = ".lzma"; this->TarCompressionType = cmArchiveWrite::CompressLZMA; - } else if (!strcmp(debianCompressionType, "xz")) { + } else if (!std::strcmp(debianCompressionType, "xz")) { this->CompressionSuffix = ".xz"; this->TarCompressionType = cmArchiveWrite::CompressXZ; - } else if (!strcmp(debianCompressionType, "bzip2")) { + } else if (!std::strcmp(debianCompressionType, "bzip2")) { this->CompressionSuffix = ".bz2"; this->TarCompressionType = cmArchiveWrite::CompressBZip2; - } else if (!strcmp(debianCompressionType, "gzip")) { + } else if (!std::strcmp(debianCompressionType, "gzip")) { this->CompressionSuffix = ".gz"; this->TarCompressionType = cmArchiveWrite::CompressGZip; - } else if (!strcmp(debianCompressionType, "none")) { + } else if (!std::strcmp(debianCompressionType, "none")) { this->CompressionSuffix.clear(); this->TarCompressionType = cmArchiveWrite::CompressNone; } else { @@ -121,16 +120,15 @@ DebGenerator::DebGenerator( << debianCompressionType << std::endl); } - if (numThreads == nullptr) { - numThreads = "1"; - } - - char* endptr; - this->NumThreads = static_cast(strtol(numThreads, &endptr, 10)); - if (numThreads != endptr && *endptr != '\0') { - cmCPackLogger(cmCPackLog::LOG_ERROR, - "Unrecognized number of threads: " << numThreads - << std::endl); + if (numThreads != nullptr) { + if (!cmStrToLong(numThreads, &this->NumThreads)) { + this->NumThreads = 1; + cmCPackLogger(cmCPackLog::LOG_ERROR, + "Unrecognized number of threads: " << numThreads + << std::endl); + } + } else { + this->NumThreads = 1; } } @@ -189,7 +187,8 @@ bool DebGenerator::generateDataTar() const return false; } cmArchiveWrite data_tar(fileStream_data_tar, this->TarCompressionType, - this->DebianArchiveType, 0, this->NumThreads); + this->DebianArchiveType, 0, + static_cast(this->NumThreads)); data_tar.Open(); // uid/gid should be the one of the root user, and this root user has From 9dc007e17c80e354e51e68fa94f1fa7e2636f0e4 Mon Sep 17 00:00:00 2001 From: Alex Turbov Date: Tue, 13 Jul 2021 16:37:54 +0300 Subject: [PATCH 10/10] Refactor: Drop redundand `std::endl` calls in the middle of output --- Source/CPack/cmCPackDebGenerator.cxx | 88 ++++++++++++++++------------ 1 file changed, 52 insertions(+), 36 deletions(-) diff --git a/Source/CPack/cmCPackDebGenerator.cxx b/Source/CPack/cmCPackDebGenerator.cxx index a93cd98e06..5b031857ee 100644 --- a/Source/CPack/cmCPackDebGenerator.cxx +++ b/Source/CPack/cmCPackDebGenerator.cxx @@ -249,11 +249,15 @@ bool DebGenerator::generateDataTar() const // do not recurse because the loop will do it if (!data_tar.Add(file, topLevelLength, ".", false)) { cmCPackLogger(cmCPackLog::LOG_ERROR, - "Problem adding file to tar:" - << std::endl - << "#top level directory: " << this->WorkDir << std::endl - << "#file: " << file << std::endl - << "#error:" << data_tar.GetError() << std::endl); + "Problem adding file to tar:\n" + "#top level directory: " + << this->WorkDir + << "\n" + "#file: " + << file + << "\n" + "#error:" + << data_tar.GetError() << std::endl); return false; } } @@ -335,11 +339,13 @@ bool DebGenerator::generateControlTar(std::string const& md5Filename) const !control_tar.Add(this->WorkDir + "/control", this->WorkDir.length(), ".")) { cmCPackLogger(cmCPackLog::LOG_ERROR, - "Error adding file to tar:" - << std::endl - << "#top level directory: " << this->WorkDir << std::endl - << "#file: \"control\" or \"md5sums\"" << std::endl - << "#error:" << control_tar.GetError() << std::endl); + "Error adding file to tar:\n" + "#top level directory: " + << this->WorkDir + << "\n" + "#file: \"control\" or \"md5sums\"\n" + "#error:" + << control_tar.GetError() << std::endl); return false; } @@ -347,11 +353,13 @@ bool DebGenerator::generateControlTar(std::string const& md5Filename) const if (this->GenShLibs) { if (!control_tar.Add(this->ShLibsFilename, this->WorkDir.length(), ".")) { cmCPackLogger(cmCPackLog::LOG_ERROR, - "Error adding file to tar:" - << std::endl - << "#top level directory: " << this->WorkDir << std::endl - << "#file: \"shlibs\"" << std::endl - << "#error:" << control_tar.GetError() << std::endl); + "Error adding file to tar:\n" + "#top level directory: " + << this->WorkDir + << "\n" + "#file: \"shlibs\"\n" + "#error:" + << control_tar.GetError() << std::endl); return false; } } @@ -361,11 +369,13 @@ bool DebGenerator::generateControlTar(std::string const& md5Filename) const control_tar.SetPermissions(permission755); if (!control_tar.Add(this->PostInst, this->WorkDir.length(), ".")) { cmCPackLogger(cmCPackLog::LOG_ERROR, - "Error adding file to tar:" - << std::endl - << "#top level directory: " << this->WorkDir << std::endl - << "#file: \"postinst\"" << std::endl - << "#error:" << control_tar.GetError() << std::endl); + "Error adding file to tar:\n" + "#top level directory: " + << this->WorkDir + << "\n" + "#file: \"postinst\"\n" + "#error:" + << control_tar.GetError() << std::endl); return false; } control_tar.SetPermissions(permission644); @@ -375,11 +385,13 @@ bool DebGenerator::generateControlTar(std::string const& md5Filename) const control_tar.SetPermissions(permission755); if (!control_tar.Add(this->PostRm, this->WorkDir.length(), ".")) { cmCPackLogger(cmCPackLog::LOG_ERROR, - "Error adding file to tar:" - << std::endl - << "#top level directory: " << this->WorkDir << std::endl - << "#file: \"postinst\"" << std::endl - << "#error:" << control_tar.GetError() << std::endl); + "Error adding file to tar:\n" + "#top level directory: " + << this->WorkDir + << "\n" + "#file: \"postinst\"\n" + "#error:" + << control_tar.GetError() << std::endl); return false; } control_tar.SetPermissions(permission644); @@ -413,11 +425,12 @@ bool DebGenerator::generateControlTar(std::string const& md5Filename) const // if we can copy the file, it means it does exist, let's add it: if (!cmsys::SystemTools::FileExists(i)) { cmCPackLogger(cmCPackLog::LOG_WARNING, - "Adding file to tar:" << std::endl - << "#top level directory: " - << this->WorkDir << std::endl - << "#missing file: " << i - << std::endl); + "Adding file to tar:\n" + "#top level directory: " + << this->WorkDir + << "\n" + "#missing file: " + << i << std::endl); } if (cmsys::SystemTools::CopyFileIfDifferent(i, localcopy)) { @@ -452,12 +465,15 @@ bool DebGenerator::generateDeb() const !deb.Add(tlDir + "control.tar.gz", tlDir.length()) || !deb.Add(tlDir + "data.tar" + this->CompressionSuffix, tlDir.length())) { cmCPackLogger(cmCPackLog::LOG_ERROR, - "Error creating debian package:" - << std::endl - << "#top level directory: " << this->TopLevelDir - << std::endl - << "#file: " << this->OutputName << std::endl - << "#error:" << deb.GetError() << std::endl); + "Error creating debian package:\n" + "#top level directory: " + << this->TopLevelDir + << "\n" + "#file: " + << this->OutputName + << "\n" + "#error:" + << deb.GetError() << std::endl); return false; } return true;