Thanks for the feedback.
I have prepared an other approach which uses checksum suffixes
instead of nested directory generation.
This was tested on my Debian/amd64/Makefile setup.
Interesting; I like it. Might be worthwhile to reuse it for the .o files
in the future as well (since these can conflict, but are much less
likely to do so).
Please review.
Could you run clang-format over the code? There are a number of style
issues that it will fix (IIRC, it needs at least Clang 3.8; I'm unsure
what version of the clang that comes with Xcode is sufficient).
I didn't knew clang-format. That's a powerful tool!
I have applied clang-format-3.8 which is available in Debian/Stretch.
+ rccOutputPath += cmQtAutoGeneratorUtil::BuildFileBase( qrcSourceName,
+ makefile->GetCurrentSourceDirectory(),
+ makefile->GetCurrentBinaryDirectory(),
+ makefile->GetHomeDirectory(),
+ makefile->GetHomeOutputDirectory() );
Is there a reason we can't pass just the `makefile` down and have it
query all the parameters rather than having 5 arguments?
In fact there is.
cmQtAutoGeneratorUtil::BuildFileBase is shared between
cmQtAutoGeneratorInitializer and cmQtAutoGenerators.
Both hold the directory names in different variables
which is why they must be passed manually.
+ struct TPair {
+ const std::string * dir;
+ const char * seed;
+ };
+ TPair pDirs[4] = {
+ { &parenDirCSource, "CurrentSource" },
+ { &parenDirCBinary, "CurrentBinary" },
+ { &parenDirPSource, "ProjectSource" },
+ { &parenDirPBinary, "ProjectBinary" }
+ };
+ TPair * itc = &pDirs[0];
+ TPair * ite = &pDirs[0] + ( sizeof ( pDirs ) / sizeof ( TPair ) );
It might be better to just use a NULL sentinel at the end of the array.
If not, the extra spaces here should go away.
I have changed that to use a NULL sentinel.
std::array would be the best solution IMO but it is not allowed, is it?
+ // Calculate the file ( seed + relative path + name ) checksum
+ std::string checksumBase64;
+ {
+ ::std::vector<unsigned char>hashBytes;
CMake just uses std::, not ::std::.
Changed.
+ {
+ // Acquire hex value hash string
+ std::string hexHash = cmCryptoHash::New("SHA256")->HashString(
+ (sourceRelSeed+sourceRelPath+sourceFileName).c_str());
+ // Convert hex value string to bytes
+ hashBytes.resize ( hexHash.size()/2 );
+ for ( unsigned int ii=0; ii != hashBytes.size(); ++ii ) {
+ unsigned char byte ( 0 );
+ for ( unsigned int jj=0; jj != 2; ++jj ) {
+ unsigned char cval ( 0 );
+ switch ( hexHash[ii*2+jj] ) {
+ case '0': cval=0; break; case '1': cval=1; break;
+ case '2': cval=2; break; case '3': cval=3; break;
+ case '4': cval=4; break; case '5': cval=5; break;
+ case '6': cval=6; break; case '7': cval=7; break;
+ case '8': cval=8; break; case '9': cval=9; break;
+ case 'a': cval=10; break; case 'b': cval=11; break;
+ case 'c': cval=12; break; case 'd': cval=13; break;
+ case 'e': cval=14; break; case 'f': cval=15; break;
+ default: break;
I feel like this is better as:
int nibble = hexHash[ii * 2 + jj];
if ('0' <= nibble && nibble <= '9') {
cval = nibble - '0';
} else if ('a' <= nibble && nibble <= 'f') {
cval = nibble - 'a' + 10;
} else {
// internal error about an unexpected hexchar
}
Alternatively, cmUuid::IntFromHexDigit() (and possibly other methods)
could be refactored to allow this to share an implementation.
I've changed it to the nibble version.
There also was a bit shift error (1 vs 4).
Doing so I found that Base64 allows '+' and '/' as characters which is
bad for directory names obviously.
For now these characters get replaced with 'A' and 'B'.
If this technique gets wider use using Base58 might be a better choice.
But it is not available in CMake, yet.
-Sebastian
>From 21c8b1f478c598c244cebab8f6f60956ecc51de0 Mon Sep 17 00:00:00 2001
From: Sebastian Holtermann <sebh...@xwmw.org>
Date: Tue, 26 Jul 2016 16:39:12 +0200
Subject: [PATCH] QtAutogen fix for too deep nested directory generation.
Instead of generating moc_* and qrc_* files in subdirectories
that reflect their source's location in the source tree
the files get generated solely in the TARGET_NAME_automoc.dir/
but get a Base64 encoded checksum suffix that was generated
from their source path and a few more seed strings.
---
Source/cmQtAutoGeneratorInitializer.cxx | 79 +++---------
Source/cmQtAutoGenerators.cxx | 211 +++++++++++++++++++++++---------
Source/cmQtAutoGenerators.h | 21 +++-
3 files changed, 188 insertions(+), 123 deletions(-)
diff --git a/Source/cmQtAutoGeneratorInitializer.cxx b/Source/cmQtAutoGeneratorInitializer.cxx
index dd19760..6bbe29c 100644
--- a/Source/cmQtAutoGeneratorInitializer.cxx
+++ b/Source/cmQtAutoGeneratorInitializer.cxx
@@ -12,6 +12,7 @@
============================================================================*/
#include "cmQtAutoGeneratorInitializer.h"
+#include "cmQtAutoGenerators.h"
#include "cmLocalGenerator.h"
#include "cmMakefile.h"
@@ -53,49 +54,24 @@ static std::string GetAutogenTargetBuildDir(cmGeneratorTarget const* target)
return targetDir;
}
-static std::string GetSourceRelativePath(cmGeneratorTarget const* target,
- const std::string& fileName)
+static std::string GetQrcBuildPath(cmGeneratorTarget const* target,
+ const std::string& qrcSourceName)
{
- std::string pathRel;
- // Test if the file is child to any of the known directories
+ std::string rccOutputPath = GetAutogenTargetBuildDir(target);
+ // Create output directory
+ cmSystemTools::MakeDirectory(rccOutputPath.c_str());
+
+ rccOutputPath += "qrc_";
{
- const std::string fileNameReal = cmsys::SystemTools::GetRealPath(fileName);
- std::string parentDirectory;
- bool match(false);
- {
- std::string testDirs[4];
- {
- cmMakefile* makefile = target->Target->GetMakefile();
- testDirs[0] = makefile->GetCurrentSourceDirectory();
- testDirs[1] = makefile->GetCurrentBinaryDirectory();
- testDirs[2] = makefile->GetHomeDirectory();
- testDirs[3] = makefile->GetHomeOutputDirectory();
- }
- for (int ii = 0; ii != sizeof(testDirs) / sizeof(std::string); ++ii) {
- const ::std::string testDir =
- cmsys::SystemTools::GetRealPath(testDirs[ii]);
- if (!testDir.empty() &&
- cmsys::SystemTools::IsSubDirectory(fileNameReal, testDir)) {
- parentDirectory = testDir;
- match = true;
- break;
- }
- }
- }
- // Use root as fallback parent directory
- if (!match) {
- cmsys::SystemTools::SplitPathRootComponent(fileNameReal,
- &parentDirectory);
- }
- pathRel = cmsys::SystemTools::RelativePath(
- parentDirectory, cmsys::SystemTools::GetParentDirectory(fileNameReal));
+ cmMakefile* makefile = target->Target->GetMakefile();
+ rccOutputPath += cmQtAutoGeneratorUtil::BuildFileBase(
+ qrcSourceName, makefile->GetCurrentSourceDirectory(),
+ makefile->GetCurrentBinaryDirectory(), makefile->GetHomeDirectory(),
+ makefile->GetHomeOutputDirectory());
}
- // Sanitize relative path
- if (!pathRel.empty()) {
- pathRel += '/';
- cmSystemTools::ReplaceString(pathRel, "..", "__");
- }
- return pathRel;
+ rccOutputPath += ".cpp";
+
+ return rccOutputPath;
}
static void SetupSourceFiles(cmGeneratorTarget const* target,
@@ -129,15 +105,7 @@ static void SetupSourceFiles(cmGeneratorTarget const* target,
if (ext == "qrc" &&
!cmSystemTools::IsOn(sf->GetPropertyForUser("SKIP_AUTORCC"))) {
- std::string rcc_output_dir = GetAutogenTargetBuildDir(target);
- rcc_output_dir += GetSourceRelativePath(target, absFile);
- cmSystemTools::MakeDirectory(rcc_output_dir.c_str());
-
- std::string basename =
- cmsys::SystemTools::GetFilenameWithoutLastExtension(absFile);
-
- std::string rcc_output_file = rcc_output_dir;
- rcc_output_file += "qrc_" + basename + ".cpp";
+ const std::string rcc_output_file = GetQrcBuildPath(target, absFile);
makefile->AppendProperty("ADDITIONAL_MAKE_CLEAN_FILES",
rcc_output_file.c_str(), false);
makefile->GetOrCreateSource(rcc_output_file, true);
@@ -804,17 +772,8 @@ void cmQtAutoGeneratorInitializer::InitializeAutogenTarget(
if (ext == "qrc" &&
!cmSystemTools::IsOn(sf->GetPropertyForUser("SKIP_AUTORCC"))) {
- {
- std::string rcc_output_dir = GetAutogenTargetBuildDir(target);
- rcc_output_dir += GetSourceRelativePath(target, absFile);
- cmSystemTools::MakeDirectory(rcc_output_dir.c_str());
-
- std::string basename =
- cmsys::SystemTools::GetFilenameWithoutLastExtension(absFile);
- std::string rcc_output_file = rcc_output_dir;
- rcc_output_file += "qrc_" + basename + ".cpp";
- rcc_output.push_back(rcc_output_file);
- }
+ const std::string rcc_output_file = GetQrcBuildPath(target, absFile);
+ rcc_output.push_back(rcc_output_file);
if (!cmSystemTools::IsOn(sf->GetPropertyForUser("GENERATED"))) {
if (qtMajorVersion == "5") {
diff --git a/Source/cmQtAutoGenerators.cxx b/Source/cmQtAutoGenerators.cxx
index ac64397..70cf7ca 100644
--- a/Source/cmQtAutoGenerators.cxx
+++ b/Source/cmQtAutoGenerators.cxx
@@ -20,6 +20,9 @@
#include "cmState.h"
#include "cmSystemTools.h"
+#include "cmCryptoHash.h"
+#include "cmsys/Base64.h"
+
#include <sys/stat.h>
#include <assert.h>
@@ -31,6 +34,103 @@
#include <unistd.h>
#endif
+std::string cmQtAutoGeneratorUtil::BuildFileBase(
+ const std::string& filePath, const std::string& parenDirCSource,
+ const std::string& parenDirCBinary, const std::string& parenDirPSource,
+ const std::string& parenDirPBinary)
+{
+ std::string baseName;
+
+ std::string sourceFileName = cmsys::SystemTools::GetFilenameName(filePath);
+ std::string sourceBaseName =
+ cmsys::SystemTools::GetFilenameWithoutLastExtension(sourceFileName);
+
+ // Determine the relative path to a known parent directory
+ std::string sourceRelPath;
+ std::string sourceRelSeed;
+ {
+ std::string sourceNameReal = cmsys::SystemTools::GetRealPath(filePath);
+ std::string parentDirectory;
+ bool parentFound(false);
+ {
+ struct TPair
+ {
+ const std::string* dir;
+ const char* seed;
+ };
+ TPair pDirs[] = { { &parenDirCSource, "CurrentSource" },
+ { &parenDirCBinary, "CurrentBinary" },
+ { &parenDirPSource, "ProjectSource" },
+ { &parenDirPBinary, "ProjectBinary" },
+ { 0, 0 } };
+ for (TPair* itc = &pDirs[0]; itc->dir != 0; ++itc) {
+ const std::string pDir = cmsys::SystemTools::GetRealPath(*(itc->dir));
+ if (cmsys::SystemTools::IsSubDirectory(sourceNameReal, pDir)) {
+ sourceRelSeed = itc->seed;
+ parentDirectory = pDir;
+ parentFound = true;
+ break;
+ }
+ }
+ }
+ // Use root as fallback parent directory
+ if (!parentFound) {
+ sourceRelSeed = "FileSystemRoot";
+ cmsys::SystemTools::SplitPathRootComponent(sourceNameReal,
+ &parentDirectory);
+ }
+ sourceRelPath = cmsys::SystemTools::RelativePath(
+ parentDirectory, cmsys::SystemTools::GetParentDirectory(sourceNameReal));
+ }
+
+ // Calculate the file ( seed + relative path + name ) checksum
+ std::string checksumBase64;
+ {
+ std::vector<unsigned char> hashBytes;
+ {
+ // Acquire hex value hash string
+ std::string hexHash = cmCryptoHash::New("SHA256")->HashString(
+ (sourceRelSeed + sourceRelPath + sourceFileName).c_str());
+ // Convert hex value string to bytes
+ hashBytes.resize(hexHash.size() / 2);
+ for (unsigned int ii = 0; ii != hashBytes.size(); ++ii) {
+ unsigned char hbyte[2] = { 0, 0 };
+ for (unsigned int jj = 0; jj != 2; ++jj) {
+ unsigned char nibble = hexHash[ii * 2 + jj];
+ if ('0' <= nibble && nibble <= '9') {
+ hbyte[jj] = nibble - '0';
+ } else if ('a' <= nibble && nibble <= 'f') {
+ hbyte[jj] = nibble - 'a' + 10;
+ } else {
+ // unexpected hexchar
+ }
+ }
+ hashBytes[ii] = hbyte[0] | (hbyte[1] << 4);
+ }
+ }
+ // Convert hash bytes to Base64 text string
+ {
+ std::vector<unsigned char> base64Bytes(hashBytes.size() * 2, 0);
+ cmsysBase64_Encode(&hashBytes[0], hashBytes.size(), &base64Bytes[0], 0);
+ checksumBase64 = reinterpret_cast<const char*>(&base64Bytes[0]);
+ // Base64 allows '+' and '/' characters. Replace these.
+ std::replace(checksumBase64.begin(), checksumBase64.end(), '+', 'A');
+ std::replace(checksumBase64.begin(), checksumBase64.end(), '/', 'B');
+ }
+ }
+
+ // Compose the file name
+ {
+ const size_t nameLength(14);
+ const size_t checkSumLength(14);
+ baseName.append(sourceBaseName.substr(0, nameLength));
+ baseName.append("_");
+ baseName.append(checksumBase64.substr(0, checkSumLength));
+ }
+
+ return baseName;
+}
+
static bool requiresMocing(const std::string& text, std::string& macroName)
{
// this simple check is much much faster than the regexp
@@ -87,6 +187,23 @@ static std::string extractSubDir(const std::string& absPath,
return subDir;
}
+static bool FileNameIsUnique(const std::string& filePath,
+ const std::map<std::string, std::string>& fileMap)
+{
+ size_t count(0);
+ const std::string fileName = cmsys::SystemTools::GetFilenameName(filePath);
+ for (std::map<std::string, std::string>::const_iterator si = fileMap.begin();
+ si != fileMap.end(); ++si) {
+ if (cmsys::SystemTools::GetFilenameName(si->first) == fileName) {
+ ++count;
+ if (count > 1) {
+ return false;
+ }
+ }
+ }
+ return true;
+}
+
cmQtAutoGenerators::cmQtAutoGenerators()
: Verbose(cmsys::SystemTools::GetEnv("VERBOSE") != 0)
, ColorOutput(true)
@@ -950,12 +1067,8 @@ void cmQtAutoGenerators::ParseHeaders(
std::string macroName;
if (requiresMocing(contents, macroName)) {
- const std::string parentDir =
- this->TargetBuildSubDir + this->SourceRelativePath(headerName);
- const std::string basename =
- cmsys::SystemTools::GetFilenameWithoutLastExtension(headerName);
- const std::string currentMoc = parentDir + "moc_" + basename + ".cpp";
- notIncludedMocs[headerName] = currentMoc;
+ notIncludedMocs[headerName] =
+ this->TargetBuildSubDir + BuildFileName(headerName, "moc_", ".cpp");
}
}
this->ParseForUic(headerName, contents, includedUis);
@@ -1022,7 +1135,9 @@ bool cmQtAutoGenerators::GenerateMocFiles(
for (std::map<std::string, std::string>::const_iterator it =
notIncludedMocs.begin();
it != notIncludedMocs.end(); ++it) {
- outStream << "#include \"" << it->second << "\"\n";
+ outStream << "#include \""
+ << cmsys::SystemTools::ConvertToOutputPath(it->second)
+ << "\"\n";
}
}
outStream.flush();
@@ -1186,7 +1301,7 @@ bool cmQtAutoGenerators::GenerateUi(const std::string& realName,
cmsys::SystemTools::MakeDirectory(this->Builddir.c_str());
}
- const ::std::string uiBuildFile = this->Builddir + uiOutputFile;
+ const std::string uiBuildFile = this->Builddir + uiOutputFile;
int sourceNewerThanUi = 0;
bool success = cmsys::SystemTools::FileTimeCompare(uiInputFile, uiBuildFile,
@@ -1262,13 +1377,8 @@ bool cmQtAutoGenerators::GenerateQrcFiles()
si != this->RccSources.end(); ++si) {
const std::string ext = cmsys::SystemTools::GetFilenameLastExtension(*si);
if (ext == ".qrc") {
- std::string basename =
- cmsys::SystemTools::GetFilenameWithoutLastExtension(*si);
- std::string qrcOutputFile = this->TargetBuildSubDir +
- this->SourceRelativePath(*si) + "qrc_" + basename + ".cpp";
- // std::string qrcOutputFile = "CMakeFiles/" + this->OriginTargetName
- // + ".dir/qrc_" + basename + ".cpp";
- qrcGenMap[*si] = qrcOutputFile;
+ qrcGenMap[*si] =
+ (this->TargetBuildSubDir + this->BuildFileName(*si, "qrc_", ".cpp"));
}
}
@@ -1290,7 +1400,8 @@ bool cmQtAutoGenerators::GenerateQrcFiles()
for (std::map<std::string, std::string>::const_iterator si =
qrcGenMap.begin();
si != qrcGenMap.end(); ++si) {
- if (!this->GenerateQrc(si->first, si->second)) {
+ if (!this->GenerateQrc(si->first, si->second,
+ FileNameIsUnique(si->first, qrcGenMap))) {
if (this->RunRccFailed) {
return false;
}
@@ -1300,13 +1411,21 @@ bool cmQtAutoGenerators::GenerateQrcFiles()
}
bool cmQtAutoGenerators::GenerateQrc(const std::string& qrcInputFile,
- const std::string& qrcOutputFile)
+ const std::string& qrcOutputFile,
+ bool unique_n)
{
- std::string relName = this->SourceRelativePath(qrcInputFile);
- std::replace(relName.begin(), relName.end(), '/', '_');
- relName += cmsys::SystemTools::GetFilenameWithoutLastExtension(qrcInputFile);
+ std::string symbolName;
+ if (unique_n) {
+ symbolName =
+ cmsys::SystemTools::GetFilenameWithoutLastExtension(qrcInputFile);
+ } else {
+ symbolName =
+ cmsys::SystemTools::GetFilenameWithoutLastExtension(qrcOutputFile);
+ // Remove "qrc_" at string begin
+ symbolName.erase(0, 4);
+ }
- const ::std::string qrcBuildFile = this->Builddir + qrcOutputFile;
+ const std::string qrcBuildFile = this->Builddir + qrcOutputFile;
int sourceNewerThanQrc = 0;
bool generateQrc = !cmsys::SystemTools::FileTimeCompare(
@@ -1332,7 +1451,7 @@ bool cmQtAutoGenerators::GenerateQrc(const std::string& qrcInputFile,
}
command.push_back("-name");
- command.push_back(relName);
+ command.push_back(symbolName);
command.push_back("-o");
command.push_back(qrcBuildFile);
command.push_back(qrcInputFile);
@@ -1357,47 +1476,17 @@ bool cmQtAutoGenerators::GenerateQrc(const std::string& qrcInputFile,
return true;
}
-std::string cmQtAutoGenerators::SourceRelativePath(const std::string& filename)
+std::string cmQtAutoGenerators::BuildFileName(const std::string& filename,
+ const std::string& prefix,
+ const std::string& suffix)
{
- std::string pathRel;
+ std::string buildName(prefix);
+ buildName += cmQtAutoGeneratorUtil::BuildFileBase(
+ filename, this->Srcdir, this->Builddir, this->ProjectSourceDir,
+ this->ProjectBinaryDir);
+ buildName += suffix;
- // Test if the file is child to any of the known directories
- {
- std::string fileNameReal = cmsys::SystemTools::GetRealPath(filename);
- std::string parentDirectory;
- bool match(false);
- {
- const ::std::string* testDirs[4];
- testDirs[0] = &(this->Srcdir);
- testDirs[1] = &(this->Builddir);
- testDirs[2] = &(this->ProjectSourceDir);
- testDirs[3] = &(this->ProjectBinaryDir);
- for (int ii = 0; ii != sizeof(testDirs) / sizeof(const ::std::string*);
- ++ii) {
- const ::std::string testDir =
- cmsys::SystemTools::GetRealPath(*(testDirs[ii]));
- if (cmsys::SystemTools::IsSubDirectory(fileNameReal, testDir)) {
- parentDirectory = testDir;
- match = true;
- break;
- }
- }
- }
- // Use root as fallback parent directory
- if (!match) {
- cmsys::SystemTools::SplitPathRootComponent(fileNameReal,
- &parentDirectory);
- }
- pathRel = cmsys::SystemTools::RelativePath(
- parentDirectory, cmsys::SystemTools::GetParentDirectory(fileNameReal));
- }
-
- // Sanitize relative path
- if (!pathRel.empty()) {
- pathRel += '/';
- cmSystemTools::ReplaceString(pathRel, "..", "__");
- }
- return pathRel;
+ return buildName;
}
/**
diff --git a/Source/cmQtAutoGenerators.h b/Source/cmQtAutoGenerators.h
index 5e7fab5..2d1d304 100644
--- a/Source/cmQtAutoGenerators.h
+++ b/Source/cmQtAutoGenerators.h
@@ -23,6 +23,21 @@
class cmMakefile;
+/// Class with static utility functions
+///
+class cmQtAutoGeneratorUtil
+{
+public:
+ // Returns a file name (without directory) that consists in part
+ // of the source file name and in part of a Base64 encoded checksum
+ // of the source file path
+ static std::string BuildFileBase(const std::string& filePath,
+ const std::string& parenDirCSource,
+ const std::string& parenDirCBinary,
+ const std::string& parenDirPSource,
+ const std::string& parenDirPBinary);
+};
+
class cmQtAutoGenerators
{
public:
@@ -51,7 +66,7 @@ private:
const std::string& uiOutputFile);
bool GenerateQrcFiles();
bool GenerateQrc(const std::string& qrcInputFile,
- const std::string& qrcOutputFile);
+ const std::string& qrcOutputFile, bool unique_n);
void ParseCppFile(
const std::string& absFilename,
const std::vector<std::string>& headerExtensions,
@@ -83,7 +98,9 @@ private:
void Init();
- std::string SourceRelativePath(const std::string& filename);
+ std::string BuildFileName(const std::string& filename,
+ const std::string& prefix,
+ const std::string& suffix);
bool NameCollisionTest(const std::map<std::string, std::string>& genFiles,
std::multimap<std::string, std::string>& collisions);
--
2.8.1
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
