Skip to content

Comments

[hadd] Fix file equivalence test in case of remote files#20878

Merged
silverweed merged 1 commit intoroot-project:masterfrom
silverweed:hadd_checkequiv_remote
Jan 15, 2026
Merged

[hadd] Fix file equivalence test in case of remote files#20878
silverweed merged 1 commit intoroot-project:masterfrom
silverweed:hadd_checkequiv_remote

Conversation

@silverweed
Copy link
Contributor

@silverweed silverweed commented Jan 14, 2026

hadd is trying to use std::filesystem::equivalent indiscriminately for any source and target files, but the function only makes sense for local files.

This PR adds a function that checks if the files are remote and, if so, if simply compares their URL.

NOTE: this PR should be backported to 6.38.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes #20872

Copy link
Member

@dpiparo dpiparo left a comment

Choose a reason for hiding this comment

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

Thanks.

@silverweed silverweed force-pushed the hadd_checkequiv_remote branch from 6284930 to 4c1cc17 Compare January 14, 2026 10:55
@github-actions
Copy link

Test Results

    23 files      23 suites   3d 19h 13m 14s ⏱️
 3 814 tests  3 814 ✅ 0 💤 0 ❌
80 445 runs  80 445 ✅ 0 💤 0 ❌

Results for commit 4c1cc17.

@silverweed silverweed merged commit b13c0f3 into root-project:master Jan 15, 2026
31 checks passed
if (sourceIsLocal != targetIsLocal)
return false;

// We cannot use std::filesystem functions for remote files.
Copy link
Member

@pcanal pcanal Jan 15, 2026

Choose a reason for hiding this comment

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

Suggested change
// We cannot use std::filesystem functions for remote files.
// We cannot use std::filesystem functions for remote files nor local files
// expressed with a protocol.

eg auto f = TFile::Open("file://t1.root", "RECREATE") creates a local file.


static bool FilesAreEquivalent(std::string_view source, std::string_view target)
{
const bool sourceIsLocal = source.find_first_of("://") == std::string_view::npos;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const bool sourceIsLocal = source.find_first_of("://") == std::string_view::npos;
const bool sourceHasProtocol = source.find_first_of("://") == std::string_view::npos;

See https://github.com/root-project/root/pull/20878/changes#r2694519784

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hadd crashes when already existing output file is used with remote file input paths

3 participants