Skip to content

Fix use of deleted assignment operator for gDirectory under ROOT 6.24 in StFwdTrackMaker #757

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.

Sign up for GitHub

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

Jump to bottom
Merged
plexoos merged 4 commits into from
Oct 9, 2025
Merged

Fix use of deleted assignment operator for gDirectory under ROOT 6.24 in StFwdTrackMaker #757

plexoos merged 4 commits into from
Oct 9, 2025

Conversation

@plexoos
Copy link
Member

@plexoos plexoos commented Jul 25, 2025

Replaced gDirectory = prevDir with prevDir->cd() to avoid compilation error with ROOT 6.24

.sl73_x8664_gcc485/OBJ/StRoot/StFwdTrackMaker/StFwdQAMaker.cxx: In member function 'virtual int StFwdQAMaker::Finish()':
.sl73_x8664_gcc485/OBJ/StRoot/StFwdTrackMaker/StFwdQAMaker.cxx:196:20: error: use of deleted function 'ROOT::Internal::TDirectoryAtomicAdapter& ROOT::Internal::TDirectoryAtomicAdapter::operator=(const ROOT::Internal::TDirectoryAtomicAdapter&)'
         gDirectory = prevDir;
                    ^
In file included from /cvmfs/star.sdcc.bnl.gov/star-spack/spack/opt/spack/linux-rhel7-x86_64/gcc-4.8.5/root-6.24.06-ojgeqx4c6cazamrg2mltgfm3wpivwzyf/include/TDirectoryFile.h:25:0,
                 from /cvmfs/star.sdcc.bnl.gov/star-spack/spack/opt/spack/linux-rhel7-x86_64/gcc-4.8.5/root-6.24.06-ojgeqx4c6cazamrg2mltgfm3wpivwzyf/include/TFile.h:28,
                 from StRoot/StChain/StMaker.h:20,
                 from StRoot/StFwdTrackMaker/StFwdQAMaker.h:11,
                 from .sl73_x8664_gcc485/OBJ/StRoot/StFwdTrackMaker/StFwdQAMaker.cxx:1:
/cvmfs/star.sdcc.bnl.gov/star-spack/spack/opt/spack/linux-rhel7-x86_64/gcc-4.8.5/root-6.24.06-ojgeqx4c6cazamrg2mltgfm3wpivwzyf/include/TDirectory.h:247:11: note: 'ROOT::Internal::TDirectoryAtomicAdapter& ROOT::Internal::TDirectoryAtomicAdapter::operator=(const ROOT::Internal::TDirectoryAtomicAdapter&)' is implicitly deleted because the default definition would be ill-formed:
    struct TDirectoryAtomicAdapter {
           ^
/cvmfs/star.sdcc.bnl.gov/star-spack/spack/opt/spack/linux-rhel7-x86_64/gcc-4.8.5/root-6.24.06-ojgeqx4c6cazamrg2mltgfm3wpivwzyf/include/TDirectory.h:247:11: error: non-static reference member 'std::atomic<TDirectory*>& ROOT::Internal::TDirectoryAtomicAdapter::fValue', can't use default assignment operator
cons: *** [.sl73_x8664_gcc485/OBJ/StRoot/StFwdTrackMaker/StFwdQAMaker.o] Error 1
cons: errors constructing .sl73_x8664_gcc485/OBJ/StRoot/StFwdTrackMaker/StFwdQAMaker.o

@plexoos
Fix use of deleted assignment operator for gDirectory under ROOT 6.24
a01846f 
Replaced `gDirectory = prevDir` with `prevDir->cd()` to avoid compilation error with ROOT 6.24

```
.sl73_x8664_gcc485/OBJ/StRoot/StFwdTrackMaker/StFwdQAMaker.cxx: In member function 'virtual int StFwdQAMaker::Finish()':
.sl73_x8664_gcc485/OBJ/StRoot/StFwdTrackMaker/StFwdQAMaker.cxx:196:20: error: use of deleted function 'ROOT::Internal::TDirectoryAtomicAdapter& ROOT::Internal::TDirectoryAtomicAdapter::operator=(const ROOT::Internal::TDirectoryAtomicAdapter&)'
         gDirectory = prevDir;
                    ^
In file included from /cvmfs/star.sdcc.bnl.gov/star-spack/spack/opt/spack/linux-rhel7-x86_64/gcc-4.8.5/root-6.24.06-ojgeqx4c6cazamrg2mltgfm3wpivwzyf/include/TDirectoryFile.h:25:0,
                 from /cvmfs/star.sdcc.bnl.gov/star-spack/spack/opt/spack/linux-rhel7-x86_64/gcc-4.8.5/root-6.24.06-ojgeqx4c6cazamrg2mltgfm3wpivwzyf/include/TFile.h:28,
                 from StRoot/StChain/StMaker.h:20,
                 from StRoot/StFwdTrackMaker/StFwdQAMaker.h:11,
                 from .sl73_x8664_gcc485/OBJ/StRoot/StFwdTrackMaker/StFwdQAMaker.cxx:1:
/cvmfs/star.sdcc.bnl.gov/star-spack/spack/opt/spack/linux-rhel7-x86_64/gcc-4.8.5/root-6.24.06-ojgeqx4c6cazamrg2mltgfm3wpivwzyf/include/TDirectory.h:247:11: note: 'ROOT::Internal::TDirectoryAtomicAdapter& ROOT::Internal::TDirectoryAtomicAdapter::operator=(const ROOT::Internal::TDirectoryAtomicAdapter&)' is implicitly deleted because the default definition would be ill-formed:
    struct TDirectoryAtomicAdapter {
           ^
/cvmfs/star.sdcc.bnl.gov/star-spack/spack/opt/spack/linux-rhel7-x86_64/gcc-4.8.5/root-6.24.06-ojgeqx4c6cazamrg2mltgfm3wpivwzyf/include/TDirectory.h:247:11: error: non-static reference member 'std::atomic<TDirectory*>& ROOT::Internal::TDirectoryAtomicAdapter::fValue', can't use default assignment operator
cons: *** [.sl73_x8664_gcc485/OBJ/StRoot/StFwdTrackMaker/StFwdQAMaker.o] Error 1
cons: errors constructing .sl73_x8664_gcc485/OBJ/StRoot/StFwdTrackMaker/StFwdQAMaker.o
```
@plexoos plexoos requested a review from jdbrice as a code owner July 25, 2025 18:23
@plexoos plexoos added the ROOT6 Issues and changes related to transition from ROOT5 to ROOT6 label Jul 25, 2025
genevb
genevb reviewed Jul 25, 2025
View reviewed changes
Copy link
Contributor

@genevb genevb left a comment

Choose a reason for hiding this comment

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

I have no objection to the proposed changes. But perhaps best also to replace the two lines...

auto prevDir = gDirectory;

...at StFwdQAMaker.cxx line 185, and StFwdAnalysisMaker.cxx line 31 with this thread-safe code...

auto prevDir = TDirectory::CurrentDirectory();

-Gene

@plexoos
Copy link
Member Author

plexoos commented Jul 25, 2025

Sure, why not. Functionally it looks like the same thing.

@genevb
Copy link
Contributor

genevb commented Jul 30, 2025

@plexoos 's commits look good to me. @jdbrice ?

klendathu2k
klendathu2k approved these changes Aug 5, 2025
View reviewed changes
Copy link
Contributor

@klendathu2k klendathu2k left a comment

Choose a reason for hiding this comment

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

LGTM.

Copilot AI review requested due to automatic review settings August 21, 2025 23:21
Copilot AI reviewed Aug 21, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a compilation error in ROOT 6.24 where the assignment operator for gDirectory has been deleted. The fix replaces direct assignment to gDirectory with appropriate ROOT API calls to change directories.

Key changes:

  • Replace gDirectory assignment with TDirectory::CurrentDirectory() and cd() method calls
  • Ensure compatibility with ROOT 6.24 while maintaining the same directory management functionality

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
StRoot/StFwdUtils/StFwdAnalysisMaker.cxx Updated directory handling to use ROOT 6.24 compatible API calls
StRoot/StFwdTrackMaker/StFwdQAMaker.cxx Updated directory handling to use ROOT 6.24 compatible API calls

@genevb
Copy link
Contributor

genevb commented Aug 25, 2025

@jdbrice : ping

@genevb genevb added the FWD Forward Upgrade label Aug 28, 2025
@plexoos
Copy link
Member Author

plexoos commented Sep 15, 2025

@jdbrice

@fgeurts
Copy link
Member

fgeurts commented Sep 16, 2025

Any more updates/follow-ups? Can we close & merge?

@plexoos
Copy link
Member Author

plexoos commented Sep 16, 2025

Any more updates/follow-ups? Can we close & merge?

Nothing from my side. We can merge if the primary maintainer remains unresponsive

@genevb
Copy link
Contributor

genevb commented Sep 17, 2025

Any more updates/follow-ups? Can we close & merge?

Nothing from my side. We can merge if the primary maintainer remains unresponsive

Same from my side. Last time I spoke with @jdbrice , I told him this was pretty straightforward for him to approve.

@genevb
Copy link
Contributor

genevb commented Oct 9, 2025

Hi, @fgeurts and @plexoos . We have not heard from @jdbrice on this one. I would like to see this included in both SL25b and SL25x (which was going to be built from SL24y_3 that @plexoos tagged yesterday). Can this please move forward?

@genevb
Copy link
Contributor

genevb commented Oct 9, 2025

Hi, @fgeurts and @plexoos . We have not heard from @jdbrice on this one. I would like to see this included in both SL25b and SL25x (which was going to be built from SL24y_3 that @plexoos tagged yesterday). Can this please move forward?

Just correcting my typo to get it correctly documented here: we agreed yesterday that "SL25y" is the tag we will use for the descendent of SL24y, and which @plexoos yesterday tagged as SL24y_3. Sorry for the noise.

@plexoos
Copy link
Member Author

plexoos commented Oct 9, 2025

Yes, let’s merge it, and I’ll tag it however you’d like.

@plexoos plexoos merged commit 5ece687 into main Oct 9, 2025
148 checks passed
@plexoos plexoos deleted the fix-StFwdTrackMaker branch October 9, 2025 19:58
@plexoos
Copy link
Member Author

plexoos commented Oct 9, 2025

Created the following refs: SL25y, SL25b, and SL25b_0 (= SL25b). Please use them when cloning or updating your git repository.

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

Reviewers

@genevb genevb genevb left review comments

Copilot code review Copilot Copilot left review comments

@klendathu2k klendathu2k klendathu2k approved these changes

@jdbrice jdbrice Awaiting requested review from jdbrice jdbrice is a code owner

@fgeurts fgeurts Awaiting requested review from fgeurts

Assignees

No one assigned

Labels

FWD Forward Upgrade ROOT6 Issues and changes related to transition from ROOT5 to ROOT6

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@plexoos @genevb @fgeurts @klendathu2k