Skip to content
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
7eb1f72
[core] circumvent call to rmkdep
ferdymercury Sep 25, 2026
9495138
!fixup missed var
ferdymercury Sep 25, 2026
17ad775
typo
ferdymercury Sep 25, 2026
72d74be
min standard cxx flag
ferdymercury Sep 25, 2026
4d9a281
silence unused vars warnings
ferdymercury Sep 25, 2026
307c67c
[core] silence deprecation warning for header extension mismatch
ferdymercury Sep 29, 2026
5958676
[nfc] formatting
ferdymercury Sep 29, 2026
c4e39f8
[core] attempt fallback implementation for Windows
ferdymercury Sep 29, 2026
56e746c
[win] try catch stdout and strip extra messages
ferdymercury Sep 29, 2026
e602185
[win32] missing header
ferdymercury Sep 29, 2026
9e680ac
[core] use same compiler as the one used to compile ROOT
ferdymercury Sep 30, 2026
7de514e
pass standard option variable
ferdymercury Sep 30, 2026
5b61a6c
Update core/base/CMakeLists.txt
ferdymercury Sep 30, 2026
4c887c3
try escape quote via build interface
ferdymercury Sep 30, 2026
a248727
try with targetcompiledef
ferdymercury Sep 30, 2026
5f6b8ad
try more fixes for win32
ferdymercury Sep 30, 2026
29d1cf7
fix -I include syntax and EP for optimization on windows
ferdymercury Oct 1, 2026
347d5df
[DEBUG] win32 failures
ferdymercury Oct 1, 2026
78d64e2
windows fixes
ferdymercury Oct 1, 2026
3b8d5af
win32 try extra quotes
ferdymercury Oct 2, 2026
93cf0d6
try more fixes
ferdymercury Oct 2, 2026
f2a8fe0
Apply suggestions from code review
ferdymercury Oct 12, 2026
e3e4e3d
[core] use basename of the compiler instead of full path
ferdymercury Oct 29, 2026
28fd0e9
[core] consistent basename path
ferdymercury Oct 29, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion core/base/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ if(MSVC AND MSVC_VERSION GREATER_EQUAL 1925 AND MSVC_VERSION LESS 1929)
# Try to re-enable /O2 after the upgrade of llvm & clang, or when upgrading Visual Studio
string(REPLACE "-O2" "-O1 -Oi" CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE}")
string(REPLACE "-O2" "-O1 -Oi" CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO}")
endif()
endif()
Comment thread
ferdymercury marked this conversation as resolved.
Resolved

set(BASE_HEADERS
ROOT/RCryptoRandom.hxx
Expand Down Expand Up @@ -228,6 +228,11 @@ if(ROOT_NEED_STDCXXFS)
target_link_libraries(Core PRIVATE stdc++fs)
endif()

# https://gitlab.kitware.com/cmake/cmake/-/work_items/18910
set_property(SOURCE src/TSystem.cxx
APPEND PROPERTY COMPILE_DEFINITIONS "ROOT_CXX_STANDARD_OPTION=\"${CMAKE_CXX${CMAKE_CXX_STANDARD}_STANDARD_COMPILE_OPTION}\""
)

# This code about the LIB_CORE_NAME define is important for TROOT::GetSharedLibDir()
# On Linux, dl_iterate_phdr reports loaded libraries by their SONAME
# (e.g. libCore.so.6.38), not their full filename (libCore.so.6.38.02).
Expand Down
211 changes: 105 additions & 106 deletions core/base/src/TSystem.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ allows a simple partial implementation for new OS'es.
#include <set>

#ifdef WIN32
#include <algorithm>
#include <io.h>
#include "Windows4Root.h"
#endif
Expand Down Expand Up @@ -2587,107 +2588,99 @@ static void R__AddPath(TString &target, const TString &path) {
}
#endif

static void R__WriteDependencyFile(const TString & build_loc, const TString &depfilename, const TString &filename, const TString &library, const TString &libname,
const TString &extension, const char *version_var_prefix, const TString &includes, const TString &defines, const TString &incPath)
static bool R__GenerateCompilerDependencies(const TString &depfilename, const TString &filename, const TString &targetname,
const TString &includes, const TString &defines)
{
// Generate the dependency via standard output, not searching the
// standard include directories,
TString compiler = COMPILER;

bool needToUnlinkTempFile = false;
TString devnullfile = "/dev/null";
TString cmd = compiler;
cmd += " " CXXOPT;
cmd += " " ROOT_CXX_STANDARD_OPTION;
#ifdef WIN32
// Determine the null device based on the shell in use.
// COMSPEC unset or pointing to cmd.exe -> NUL
// COMSPEC pointing to powershell -> $null
// Anything else (e.g. bash/sh on Windows) -> `depfilename`.stderr.tmp
const char *comspec = gSystem->Getenv("COMSPEC");
if (!comspec || !comspec[0]) {
devnullfile = "NUL";
} else {
TString comspecStr(comspec);
comspecStr.ToLower();
if (comspecStr.EndsWith("cmd.exe")) {
devnullfile = "NUL";
} else if (comspecStr.Contains("powershell.exe")) {
devnullfile = "$null";
} else {
needToUnlinkTempFile = true;
devnullfile = depfilename + ".stderr.tmp";
gSystem->PrependPathName(build_loc, devnullfile);
}
}
cmd += " /nologo /E /showIncludes";
cmd += " \"/I";
#else
cmd += " -MM";
cmd += " -MF \"" + depfilename + "\"";
cmd += " -MT \"";
R__AddPath(cmd, targetname);
cmd += "\" ";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The design of ACLiC here was to delegate this whole command line build (for the most part) to the build system and insert it in compiledata.h

For example we have:

#define MAKESHAREDLIB  "cd $BuildDir ; c++ -fPIC -c $Opt -Wc++11-narrowing -Wsign-compare -Wsometimes-uninitialized -Wconditional-uninitialized -Wheader-guard -Warray-bounds -Wcomment -Wtautological-compare -Wstrncat-size -Wloop-analysis -Wbool-conversion -m64 -pipe -W -Woverloaded-virtual -fsigned-char -fsized-deallocation -fno-common -Qunused-arguments -pthread -stdlib=libc++ -std=c++17 $IncludePath $SourceFiles ; c++ $Opt $ObjectFiles -dynamiclib -Wl,-headerpad_max_install_names -m64 -Wl,-dead_strip_dylibs -Wl,-install_name,@rpath/$LibName.so  $RPath $LinkedLibs -o $SharedLib "

So I would imagine we would add a new line in compiledata.h with all the compiler/OS specific part filed in and the run-time part easily inserted via some $variables

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this PR is a bit complicated, now that the CI is green, I'd prefer doing this optimization in a subsequent one. On the other hand, only TSystem would be using this, so I do not see that it simplifies things. The ifdefs in the Cxx would be just moved to the CMake code, so similar if not larger number of lines and more abstraction. If there would be more places being used, then I would see that more clearly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it sortof can wait for a further PR. Yes, the complexity is essentially the same.

There is 2 major distinctions

One is to have the same pattern for all the cases rather than 2 different ways.

Additionally, going through the indirection of having one command line pattern allows the user to manipulate and customizing as they need.

cmd += " \"-I";
#endif
TString bakdepfilename = depfilename + ".bak";

struct Defer {
bool fNeedToUnlinkTempFile;
const TString &fDevNullFile, &fBakdepfilename;
~Defer()
{
if (fNeedToUnlinkTempFile) {
// Remove the temporary stderr file if it was created.
gSystem->Unlink(fDevNullFile);
}
gSystem->Unlink(fBakdepfilename);
}
} deferGuard{needToUnlinkTempFile, devnullfile, bakdepfilename};

TString builddep = "rmkdepend";
gSystem->PrependPathName(TROOT::GetBinDir(), builddep);
builddep += " \"-f";
builddep += depfilename;
builddep += "\" -o_" + extension + "." + gSystem->GetSoExt() + " ";
if (build_loc.BeginsWith(gSystem->WorkingDirectory())) {
Int_t len = strlen(gSystem->WorkingDirectory());
if ( build_loc.Length() > (len+1) ) {
builddep += " \"-p";
if (build_loc[len] == '/' || build_loc[len+1] != '\\' ) {
// Since the path is now ran through TSystem::ExpandPathName the single \ is also possible.
R__AddPath(builddep, build_loc.Data() + len + 1 );
} else {
// Case of dir\\name
R__AddPath(builddep, build_loc.Data() + len + 2 );
}
builddep += "/\" ";
}
} else {
builddep += " \"-p";
R__AddPath(builddep, build_loc);
builddep += "/\" ";
}
builddep += " -Y -- ";
TString rootsysInclude = TROOT::GetIncludeDir();
builddep += " \"-I"+rootsysInclude+"\" "; // cflags
builddep += includes;
builddep += defines;
builddep += " -- \"";
builddep += filename;
builddep += "\" ";
TString targetname;
if (library.BeginsWith(gSystem->WorkingDirectory())) {
Int_t len = strlen(gSystem->WorkingDirectory());
if ( library.Length() > (len+1) ) {
if (library[len] == '/' || library[len+1] != '\\' ) {
targetname = library.Data() + len + 1;
} else {
targetname = library.Data() + len + 2;
R__AddPath(cmd, rootsysInclude);
cmd += "\" ";

cmd += includes;
cmd += defines;

#ifdef WIN32
cmd += " /TP"; // avoid warning when header is .h but is C++ code
#else
cmd += " -x c++-header"; // avoid warning when header is .h but is C++ code
#endif
cmd += " \"";
R__AddPath(cmd, filename);
cmd += "\"";

if (gDebug > 4)
::Info("ACLiC", "%s", cmd.Data());

#ifdef WIN32
TString wrapperCmd = "cmd.exe /c \"" + cmd + "\"";
FILE *pipe = gSystem->OpenPipe(wrapperCmd, "r");
if (!pipe) {
::Warning("ACLiC", "Failed to open pipe dependencies for %s", filename.Data());
return false;
}
std::ofstream depFile(depfilename, std::ios::out | std::ios::trunc);
Comment thread
ferdymercury marked this conversation as resolved.
Resolved
if (depFile) {
depFile << targetname << ": \\\n";
char buffer[4096];
const char *prefix = "Note: including file:";
size_t prefixLen = strlen(prefix);

while (fgets(buffer, sizeof(buffer), pipe) != nullptr) {
std::string line(buffer);
// Find and strip "Note: including file:"
Comment thread
ferdymercury marked this conversation as resolved.
Resolved
size_t pos = line.find(prefix);
if (pos != std::string::npos) {
std::string path = line.substr(pos + prefixLen);
// Trim leading/trailing spaces or newlines
path.erase(0, path.find_first_not_of(" \t"));
path.erase(path.find_last_not_of(" \r\n\t") + 1);
// Convert Windows backslashes to forward slashes for Makefile compatibility
std::replace(path.begin(), path.end(), '\\', '/');
if (!path.empty()) {
depFile << " \"" << path << "\" \\\n";
}
}
} else {
targetname = library;
}
} else {
targetname = library;
depFile << "\n"; // End the dependency rule cleanly
depFile.close();
}
int retVal = gSystem->ClosePipe(pipe);
if (retVal != 0) {
Comment thread
ferdymercury marked this conversation as resolved.
Resolved
::Warning("ACLiC", "Failed to close pipe dependencies for %s", filename.Data());
return false;
}
#else
if (gSystem->Exec(cmd)) {
::Warning("ACLiC", "Failed to generate dependencies for %s", filename.Data());
return false;
}
builddep += " \"";
builddep += "-t";
R__AddPath(builddep, targetname);
builddep += "\" > ";
builddep += devnullfile;
builddep += " 2>&1 ";
#endif

return true;
}

static TString R__GetRootDictionaryDependencies(const TString &targetname, const TString &incPath)
{
TString adddictdep;
R__AddPath(adddictdep,targetname);
adddictdep += ": ";
TString rootsysInclude = TROOT::GetIncludeDir();

#if defined(R__HAS_CLING_DICTVERSION)
{
char *clingdictversion = gSystem->Which(incPath,"clingdictversion.h");
Expand Down Expand Up @@ -2734,27 +2727,33 @@ static void R__WriteDependencyFile(const TString & build_loc, const TString &dep
delete [] rootCling;
}
}
return adddictdep;
}

{
std::ofstream depFile(depfilename, std::ios::out | std::ios::trunc);
if (!depFile) {
::Warning("ACLiC", "Failed to open dependency file %s for %s", depfilename.Data(), library.Data());
return;
static void R__WriteDependencyFile(const TString & build_loc, const TString &depfilename, const TString &filename, const TString &library, const TString &libname,
const TString &extension, const char *version_var_prefix, const TString &includes, const TString &defines, const TString &incPath)
{
(void)build_loc;
(void)extension;
TString targetname;
if (library.BeginsWith(gSystem->WorkingDirectory())) {
Int_t len = strlen(gSystem->WorkingDirectory());
if ( library.Length() > (len+1) ) {
if (library[len] == '/' || library[len+1] != '\\' ) {
targetname = library.Data() + len + 1;
} else {
targetname = library.Data() + len + 2;
}
} else {
targetname = library;
}
#ifdef WIN32
depFile << "#\n";
#endif
}

if (gDebug > 4) {
::Info("ACLiC", "%s", builddep.Data());
::Info("ACLiC", "%s", adddictdep.Data());
} else {
targetname = library;
}
bool depbuiltOk = !gSystem->Exec(builddep);
if (!depbuiltOk) {
::Warning("ACLiC", "Failed to run rmkdepend for %s", library.Data());
if (!R__GenerateCompilerDependencies(depfilename, filename, targetname, includes, defines))
return;
}

TString adddictdep = R__GetRootDictionaryDependencies(targetname, incPath);

std::ofstream depFile(depfilename, std::ios::out | std::ios::app);
if (!depFile) {
Expand Down
Loading