Skip to content

[core] Disable local level of gEnv - #22775

Draft
jblomer wants to merge 8 commits into
root-project:masterfrom
jblomer:restrict-rclogon-v2
Draft

jblomer wants to merge 8 commits into
root-project:masterfrom
jblomer:restrict-rclogon-v2

Conversation

@jblomer

@jblomer jblomer commented Jul 8, 2026 •

Copy link
Copy Markdown
Contributor

Add a new option for constructing TEnv that allows to disable the local
level. If disabled, configuration cannot be read anymore into the local
level, the local level cannot be saved and values cannot be set in the
local level. Effectively, the local level is always empty in this mode.

By default, new instances of TEnv have the local level enabled. Only
gEnv has it disabled to prevent environment poisoning.

The old behavior can be restored by setting the ROOTENV_USE_LOCAL
environment variable to a non-empty string.

Note: first two commits submitted as separate PR #22783

@jblomer
jblomer requested review from hageboeck and silverweed July 8, 2026 12:43
@jblomer jblomer self-assigned this Jul 8, 2026
@jblomer
jblomer requested review from dpiparo and pcanal as code owners July 8, 2026 12:43
@jblomer
jblomer force-pushed the restrict-rclogon-v2 branch from 6950a4d to b8076f5 Compare July 8, 2026 13:55
@github-actions

github-actions Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit a68bfda.

♻️ This comment has been updated with latest results.

Comment thread core/base/src/TEnv.cxx Outdated
Comment thread core/base/inc/TEnv.h
Comment thread core/base/inc/TEnv.h Outdated
Comment thread core/base/src/TEnv.cxx Outdated
@jblomer
jblomer force-pushed the restrict-rclogon-v2 branch from b8076f5 to b02753b Compare July 10, 2026 08:04

@hageboeck hageboeck left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change probably warrants some info in the release notes.
Also, if it's too much work/noise to flip the logic, leave it.

Comment thread core/base/inc/TEnv.h
Comment thread core/base/inc/TEnv.h Outdated
Comment thread core/base/src/TEnv.cxx Outdated
@jblomer
jblomer force-pushed the restrict-rclogon-v2 branch 2 times, most recently from ef4d149 to 524479e Compare October 10, 2026 22:45
@jblomer
jblomer force-pushed the restrict-rclogon-v2 branch from 524479e to d714644 Compare October 14, 2026 13:27
@jblomer jblomer added the clean build Ask CI to do non-incremental build on PR label Oct 14, 2026
@jblomer
jblomer force-pushed the restrict-rclogon-v2 branch 2 times, most recently from 717b645 to d41aa49 Compare October 14, 2026 21:27
Add a new option for constructing TEnv that allows to disable the local
level. If disabled, configuration cannot be read anymore into the local
level, the local level cannot be saved and values cannot be set in the
local level. Effectively, the local level is always empty in this mode.

By default, new instances of TEnv have the local level _enabled_. Only
gEnv has it disabled to prevent environment poisoning.

The old behavior can be restored by setting the ROOTENV_USE_LOCAL
environment variable to a non-empty string.
@jblomer jblomer closed this Oct 15, 2026
@jblomer jblomer reopened this Oct 15, 2026
Comment thread core/base/inc/TEnv.h
THashList *fTable = nullptr; // hash table containing env records
TString fRcName; // resource file base name
Bool_t fIgnoreDup = false; // ignore duplicates, don't issue warning
Bool_t fIsLocalLevelEnabled = true; //! By default, gEnv does not allow use of the local level

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

SInce it is transient (as it should) but the TEnv can be stored, do we need to take care of setting up with the current value when reading it back?

@jblomer jblomer Oct 16, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am not sure. I think you can argue either way. One argument for leaving it the way it currently is may be that if you deliberately create a TEnv that should disable the local level, then read "contents" from disk and then save it or re-read things, your behavioral trait of disabling the local level should stay as it was in the beginning. But this is quite hypothetical and you can probably also argue either way.

Overall, I don't see a compelling reason to do anything special.

@pcanal

pcanal commented Oct 15, 2026

Copy link
Copy Markdown
Member

This change would have benefited from being part of master for a while before a release so that it can be tested a bit in the wild (at least through some of the LHC experiments' build)

@jblomer

jblomer commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

This change would have benefited from being part of master for a while before a release so that it can be tested a bit in the wild (at least through some of the LHC experiments' build)

I do not intend to rush this into the release if we don't feel confident about the change.

The custom settings are in the .rootrc file are no longer needed since
v6.40.
@jblomer

jblomer commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

After discussion in the I/O meeting, the consensus was that this change requires further discussion.

@jblomer
jblomer marked this pull request as draft October 16, 2026 14:56

This branch has not been deployed

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

Labels

clean build Ask CI to do non-incremental build on PR in:Core Libraries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants