Skip to content

feat: only link roots into default view#208

Open
wdconinc wants to merge 3 commits intomasterfrom
view-default-link-roots
Open

feat: only link roots into default view#208
wdconinc wants to merge 3 commits intomasterfrom
view-default-link-roots

Conversation

@wdconinc
Copy link
Copy Markdown
Contributor

Briefly, what does this PR introduce?

We keep hearing about many files from OSG folks. We have a symlink for all packages, even if we don't actually need to expose some of this functionality beyond explicitly included packages. This restricts what is linked into the view to only the roots.

I think we've tried this before. I forget why it failed. We'll see...

Copilot AI review requested due to automatic review settings March 23, 2026 18:39
Copy link
Copy Markdown
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

Updates the shared Spack environment view configuration to reduce the number of files exposed in the default /opt/local view by linking only environment root specs, aligning with the goal of minimizing unnecessary symlinks for downstream users (e.g., OSG).

Changes:

  • Configure the default Spack view to link: roots (previously implicitly linking more than roots).
  • Keep link_type: symlink and existing exclude/select behavior unchanged.

Copilot AI review requested due to automatic review settings March 26, 2026 21:50
@wdconinc
Copy link
Copy Markdown
Contributor Author

The problem here is that ROOT now cannot find headers like vdt/vdtMath.h since vdt is now not exposed in /opt/local anymore...

Copy link
Copy Markdown
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

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

Comment on lines 3 to 8
root: /opt/local
exclude:
- epic
- py-pip@23.0
- py-urllib3@1
link: roots
link_type: symlink
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

link: roots is a significant behavioral change: only top-level env specs will be present in the view, and common dependency-provided CLIs/files that previously appeared under /opt/local will no longer be discoverable via the view. If any workflows relied on dependency tools being in the view, consider either (a) adding those packages as explicit roots, or (b) switching to a less restrictive linking mode (e.g., linking runtimes) for the default view.

Copilot uses AI. Check for mistakes.
Comment on lines 4 to +7
exclude:
- epic
- py-pip@23.0
- py-urllib3@1
link: roots
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

With link: roots, excludes that target packages which are only ever dependencies won’t have any practical effect (they aren’t linked anyway). If py-urllib3 is not intended to be a root, consider removing this exclude to avoid confusion; if it is sometimes a root, add a brief comment explaining why it must be excluded even under root-only linking. Also consider removing the trailing whitespace after exclude: to avoid churn/lint noise.

Copilot uses AI. Check for mistakes.
- py-toml
- py-uproot
- py-vector
- root
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

Since the default view now links only roots, adding root here will make it appear in the default view. If the goal is only to reduce view bloat, consider whether root belongs specifically in the TF environment or should be added to the environment(s) that actually need it to be exposed via /opt/local.

Suggested change
- root

Copilot uses AI. Check for mistakes.
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.

2 participants