-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[IO] Fix rootcp --replace flag not working with recursive copy #20344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[IO] Fix rootcp --replace flag not working with recursive copy #20344
Conversation
|
Thank you very much for your PR! Could you fix the Python syntax errors? Thanks. |
674d1cb to
4c59740
Compare
|
The CI failures appear to be unrelated to my changes. The build is failing with: Error: roofit/hs3/src/RooJSONFactoryWSTool.cxx:260:1: error: '{anonymous}::Var::Var' defined but not used [-Werror=unused-function] This error is in C++ RooFit code, while my changes only modify main/python/cmdLineUtils.py. The Python linting checks (ruff) pass successfully. Could a maintainer please advise if this is a known issue or if I should rebase on the latest master? |
Test Results 22 files 22 suites 3d 13h 57m 20s ⏱️ For more details on these failures, see this check. Results for commit 1a74ecf. ♻️ This comment has been updated with latest results. |
|
Hi @bellenot, I've fixed the Python linting errors as requested. The However, the CI is failing with an error in RooFit code that appears unrelated to my changes: Could you advise if:
Thank you for your guidance! |
|
Hi @ahmedbilal9 I see failures like: And not the one with RooFit |
Hi @bellenot, Thank you for clarifying! I see now - the failures are in the roottest-main-SimpleRootmv tests, not the RooFit code. I'll investigate these test failures:
Could you point me to where I can find the detailed CI logs for these tests, or should I look at the full CI output? I want to understand what specific behavior is failing so I can fix it properly. Thank you! |
|
|
Summary
Local checks
Scope
|
|
Hi @guitargeek, I hope you’re doing well. I wanted to follow up on my pull request #20344 ([IO] Fix rootcp --replace flag not working with recursive copy) which I opened recently. Summary of the change:
Status:
Could you please let me know if there is anything else you need from me to move this forward (additional tests, rebase, or further adjustments)? I’d be happy to add a minimal roottest if that would help. Thanks for your time and review, |
When copying non-tree objects recursively, the --replace flag was checking for 'setName' instead of the actual object name that would be written. This caused objects to be written with new cycles instead of replacing existing ones. The fix determines the actual name (setName if provided, otherwise objectName) and uses that for the replacement check. Fixes root-project#7052
7854805 to
8e23b75
Compare
Fixes #7052
Changes or fixes:
The --replace flag was not being checked when copying regular objects (non-tree, non-collection) in recursive mode. This caused objects to be written with new cycles instead of replacing existing ones.
Added replaceOption check in the else block of copyRootObjectRecursive() to delete existing objects before writing replacements.
Checklist: