Repository navigation
Conversation
When Root.PluginPath (or ROOT_PLUGIN_PATH) contains multiple colon-separated directories, TPluginManager::LoadHandlersFromPluginDirs loops over each directory to load handler macros for the requested base class. In commit e60640c (root-project#18859), ConcatFileName was replaced by PrependPathName. Unlike ConcatFileName which returned a new heap-allocated string, PrependPathName modifies its second parameter in-place. Because `sbase` was passed directly: const char *p = gSystem->PrependPathName(d, sbase); the variable `sbase` was mutated in-place across iterations (e.g., from "TArchiveFile" to "/dir1/TArchiveFile", then to "/dir2//dir1/TArchiveFile"). Consequently, subsequent search directories received a mangled path and ROOT could not find its own built-in plugins whenever a custom plugin directory was prepended. Fix this by copying `sbase` to a temporary `TString temp = sbase;` on each iteration, matching the pattern used in the rest of TPluginManager.cxx. Fixes root-project#23536. Added regression test in core/base/test/TPluginManagerTests.cxx.
Use three temporary plugin directories and a fresh plugin manager so cached built-in handlers cannot hide path accumulation. Select the path separator for the platform and restore global state and remove temporary files in fixture teardown, including after fatal assertions. Validation: clang-format --dry-run --Werror and git diff --check passed. Runtime validation remains pending because VS Code CMake build and test tools failed and unit-test discovery found no tests. Assisted-by: Copilot SDK:GPT 6 astra
Avoid prefixing the dependency output path twice. Advance the parent component while reconstructing dictionary header paths, and stop at the filesystem root. Assisted-by: Copilot SDK:GPT 6 astra
Exercise the ACLiC dependency generation path with a cmd.exe alias so that the nonstandard COMSPEC branch is covered without changing the actual shell behavior. Assisted-by: Copilot SDK:GPT 6 astra
Exercise TCling_GenerateDictionary with an unknown class declared in a nested ROOT header path to cover directory traversal while generating a dictionary. Assisted-by: Copilot SDK:GPT 6 astra
Test Results 23 files 23 suites 3d 7h 40m 47s ⏱️ For more details on these failures, see this check. Results for commit 101b2a8. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As prompted by the PR #23796, review usage of
TSystem::PrependPathNamewhich is (intentionally) updating its argument rather than just returning a new value.In addition to the case addressed in the original PR, we also address two related issues in TSystem.cxx and TCling.cxx.
The original PR test has been significantly extended (It was failing on windows failure due to joining directories with
:, whereas the implementation uses;on Windows. The cleanup was oncomplete cleanup due fatal assertions bypassing cleanup. An originally empty or absent Root.PluginPath is never restored, leaving it pointing at deleted directories.Cache-dependent coverage: built-in handlers already loaded into the global plugin manager can mask the bug. Prefer controlled temporary plugin directories, unique base names, and isolated plugin-manager state.)
Regression test for the two other issues has been added.