Skip to content
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

[tree] Fix GetTreeFullPaths in case of protocol:// #10217

Merged
merged 1 commit into from
Mar 24, 2022

Conversation

eguiraud
Copy link
Member

GetTreeFullPaths assumed that the first occurrence of ":/" was
the separator between filename and tree name in strings such as
"file.root:/dir/tree". However, the separator is the last
occurrence of ":/" -- e.g. if the file is read via a remote
protocol, its name starts with "protocol://".

This logic is of course still broken in case the name of the tree
or the one of the directory that contains it contains ":/", we
do not support that case.

This fixes #10216.

@eguiraud eguiraud requested a review from pcanal March 23, 2022 19:28
@eguiraud eguiraud self-assigned this Mar 23, 2022
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1015/python3, mac11/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-ubuntu16/nortcxxmod.
Running on sft-ubuntu-1604-2.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-performance-centos8-multicore/default.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

Failing tests:

GetTreeFullPaths assumed that the first occurrence of ":/" was
the separator between filename and tree name in strings such as
"file.root:/dir/tree". However, the separator is the _last_
occurrence of ":/" -- e.g. if the file is read via a remote
protocol, its name starts with "protocol://".

This logic is of course still broken in case the name of the tree
or the one of the directory that contains it contains ":/", we
do not support that case.

This fixes root-project#10216.
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1015/python3, mac11/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-ubuntu2004/soversion.
See console output.

@eguiraud eguiraud merged commit 6bae05e into root-project:master Mar 24, 2022
@eguiraud eguiraud deleted the fix-10216 branch March 24, 2022 08:08
@calcuttj
Copy link

calcuttj commented Jan 3, 2023

Hi @eguiraud I see similar behavior to this issue when using TTreeProcessorMT on xrootd-streamed files when using ROOT 6.22/08. Just wanted to check that this fix will also work for my issue before we try integrating >= 6.28/00 into our workflow.

Edit: sorry for bumping such an old thread btw

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DF] EnableImplicitMT() prevents reading TTree in sub-directory from XrootD file
4 participants