Skip to content

Fix thisroot.sh to properly remove previous ROOT version from paths - #20645

Merged
pcanal merged 2 commits into
masterfrom
copilot/fix-thisroot-sh-paths
Dec 5, 2025
Merged

pcanal merged 2 commits into
masterfrom
copilot/fix-thisroot-sh-paths

Conversation

Copilot AI commented Dec 4, 2025 •

Copy link
Copy Markdown
Contributor

This Pull request:

Changes or fixes:

Since v6.34.00, thisroot.sh fails to remove paths from a previously sourced ROOT version. The drop_from_path function stores results in $newpath, but when called twice in sequence, the second call was using the original path instead of $newpath:

# Before (broken): second call overwrites first result
drop_from_path "$LD_LIBRARY_PATH" "${old_rootsys}/lib"
drop_from_path "$LD_LIBRARY_PATH" "${old_rootsys}/lib/root"

# After (fixed): chain results correctly
drop_from_path "$LD_LIBRARY_PATH" "${old_rootsys}/lib"
drop_from_path "$newpath" "${old_rootsys}/lib/root"

Fixed for: LD_LIBRARY_PATH, DYLD_LIBRARY_PATH, SHLIB_PATH, LIBPATH, PYTHONPATH

Checklist:

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

This section details on the original issue you should resolve

<issue_title>thisroot.sh no longer remove previous version from the paths</issue_title>
<issue_description>As reported at https://root-forum.cern.ch/t/the-thisroot-sh-misbahes-in-root-6-34/64513, since v6.34.00, thisroot.sh no longer remove the locations of a previously setup version of ROOT from any of the paths.

This was introduce in #16027 where the code pattern should actually be:

      if [ -n "${LD_LIBRARY_PATH-}" ]; then
         drop_from_path "$LD_LIBRARY_PATH" "${old_rootsys}/lib"
         drop_from_path "$newpath" "${old_rootsys}/lib/root"
         LD_LIBRARY_PATH=$newpath
      fi

</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@guitargeek

Copy link
Copy Markdown
Contributor

Ah yes, PRs with zero diff, so clean 😆

The second call to drop_from_path was using the original path variable
instead of $newpath (result of first call), causing only the last
transformation to be applied. This affected LD_LIBRARY_PATH,
DYLD_LIBRARY_PATH, SHLIB_PATH, LIBPATH, and PYTHONPATH.

Fixes the issue where old_rootsys/lib was not being removed when
switching ROOT versions.

Co-authored-by: pcanal <5175087+pcanal@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix thisroot.sh to remove previous ROOT version from paths Fix thisroot.sh to properly remove previous ROOT version from paths Dec 4, 2025
Copilot AI requested a review from pcanal December 4, 2025 19:17
@pcanal

pcanal commented Dec 4, 2025

Copy link
Copy Markdown
Member

Ah yes, PRs with zero diff, so clean

It is apparently work in progress with no feedback that the work is still 'being' done .. :(

@pcanal

pcanal commented Dec 4, 2025

Copy link
Copy Markdown
Member

@copilot Have you completed work on this PR?

Copilot AI commented Dec 4, 2025

Copy link
Copy Markdown
Contributor Author

@copilot Have you completed work on this PR?

Yes, the work is complete. The fix was committed in 0661c4b. The change ensures that when drop_from_path is called twice in sequence, the second call uses $newpath (the result of the first call) instead of the original path variable, so both /lib and /lib/root paths are properly removed when switching ROOT versions.

@pcanal pcanal 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.

LGTM.

@ferdymercury ferdymercury left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice, thanks for the fix.
Can copilot also prepare backport PRs for 6.34, 6.36 and 6.38 or shall I?

@pcanal

pcanal commented Dec 4, 2025

Copy link
Copy Markdown
Member

Can copilot also prepare backport PRs for 6.34, 6.36 and 6.38 or shall I?

My plan is to ask it to do so ... as a capability test :)

@dpiparo
dpiparo marked this pull request as ready for review December 5, 2025 06:25
@dpiparo
dpiparo self-requested a review as a code owner December 5, 2025 06:25
Comment thread config/thisroot.sh Resolved
@pcanal
pcanal merged commit f8a2fae into master Dec 5, 2025
40 of 63 checks passed
@pcanal

pcanal commented Dec 5, 2025

Copy link
Copy Markdown
Member

@copilot open PR to apply this change to the v6-38-00-patches, v6-36-00-patches and v6-34-00-patches branches.

@pcanal

pcanal commented Dec 5, 2025

Copy link
Copy Markdown
Member

[Apparently @copilot stopped listening after the PR was merged]

@pcanal

pcanal commented Dec 5, 2025

Copy link
Copy Markdown
Member

Follow up at #20655

pcanal added a commit that referenced this pull request Dec 5, 2025
…20645)

* Fix thisroot.sh to properly remove previous ROOT version from paths

The second call to drop_from_path was using the original path variable
instead of $newpath (result of first call), causing only the last
transformation to be applied. This affected LD_LIBRARY_PATH,
DYLD_LIBRARY_PATH, SHLIB_PATH, LIBPATH, and PYTHONPATH.

Fixes the issue where old_rootsys/lib was not being removed when
switching ROOT versions.

Co-authored-by: pcanal <5175087+pcanal@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pcanal <5175087+pcanal@users.noreply.github.com>
pcanal added a commit that referenced this pull request Dec 5, 2025
…20645)

* Fix thisroot.sh to properly remove previous ROOT version from paths

The second call to drop_from_path was using the original path variable
instead of $newpath (result of first call), causing only the last
transformation to be applied. This affected LD_LIBRARY_PATH,
DYLD_LIBRARY_PATH, SHLIB_PATH, LIBPATH, and PYTHONPATH.

Fixes the issue where old_rootsys/lib was not being removed when
switching ROOT versions.

Co-authored-by: pcanal <5175087+pcanal@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pcanal <5175087+pcanal@users.noreply.github.com>
pcanal added a commit that referenced this pull request Dec 5, 2025
…20645)

* Fix thisroot.sh to properly remove previous ROOT version from paths

The second call to drop_from_path was using the original path variable
instead of $newpath (result of first call), causing only the last
transformation to be applied. This affected LD_LIBRARY_PATH,
DYLD_LIBRARY_PATH, SHLIB_PATH, LIBPATH, and PYTHONPATH.

Fixes the issue where old_rootsys/lib was not being removed when
switching ROOT versions.

Co-authored-by: pcanal <5175087+pcanal@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pcanal <5175087+pcanal@users.noreply.github.com>
ferdymercury pushed a commit to ferdymercury/root that referenced this pull request Oct 16, 2026
…oot-project#20645)

* Fix thisroot.sh to properly remove previous ROOT version from paths

The second call to drop_from_path was using the original path variable
instead of $newpath (result of first call), causing only the last
transformation to be applied. This affected LD_LIBRARY_PATH,
DYLD_LIBRARY_PATH, SHLIB_PATH, LIBPATH, and PYTHONPATH.

Fixes the issue where old_rootsys/lib was not being removed when
switching ROOT versions.

Co-authored-by: pcanal <5175087+pcanal@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pcanal <5175087+pcanal@users.noreply.github.com>
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.

thisroot.sh no longer remove previous version from the paths

4 participants