Skip to content

workspace concretize: omit injected-if-missing packages and compilers by default - #1810

Merged
douglasjacobsen merged 1 commit into
Ramble-Project:developfrom
chinglung880218:omit-injected-packages-concretize
Oct 2, 2026
Merged

douglasjacobsen merged 1 commit into
Ramble-Project:developfrom
chinglung880218:omit-injected-packages-concretize

Conversation

@chinglung880218

Copy link
Copy Markdown
Contributor

Currently, when workspace concretize is executed, packages and compilers with inject_if_missing=True (from modifiers or systems) are written into ramble.yaml. Since Ramble injects these automatically at runtime anyway, persisting them clutters the configuration and can trigger false unused compiler warnings.

This PR omits inject_if_missing packages and compilers from ramble.yaml by default, and adds an --include-injected-packages flag to workspace concretize if users want to explicitly include them.

It also improves compiler tracking to avoid false "Unused compiler" warnings for omitted compilers, while ensuring compilers from overridden injected specs are not incorrectly marked as used. Tests are included or adjusted correspondingly.

@rfbgo

rfbgo commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

/gcbrun

@ramble-project-pr-bot

ramble-project-pr-bot Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Ramble Performance Test Metrics

Results produced with commit: 6e6b810

Test Name Outcome Duration (s) Most Recent Run (s) Last 5 Avg (s)
test_analyze_large_file passed 1.0438 1.0725 (dff2e37) 1.0519
test_large_template_expansion passed 1.1538 1.2142 (dff2e37) 1.2073
test_many_experiments passed 28.3967 29.8921 (dff2e37) 30.4717
test_many_objects_defaults passed 15.0762 16.2349 (dff2e37) 16.3540
test_matrix_filter_perf passed 1.0074 1.0832 (dff2e37) 1.0949

@codecov

codecov Bot commented Sep 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@chinglung880218
chinglung880218 force-pushed the omit-injected-packages-concretize branch from a63612a to b0bff29 Compare September 28, 2026 16:31
@rfbgo

rfbgo commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

FYI I don't think your test failure here is caused by your code. See #1813 for details if curious, but I will try fix on develop and re-run

@rfbgo

rfbgo commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

/gcbrun

@chinglung880218
chinglung880218 force-pushed the omit-injected-packages-concretize branch from b0bff29 to 24d6924 Compare September 28, 2026 18:45
@chinglung880218

Copy link
Copy Markdown
Contributor Author

Thanks for the fix!

@rfbgo

rfbgo commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

/gcbrun

Signed-off-by: Ching-Lung Hsu <chinglunghsu@google.com>
@chinglung880218
chinglung880218 force-pushed the omit-injected-packages-concretize branch from 24d6924 to 6e6b810 Compare September 28, 2026 21:29
@rfbgo

rfbgo commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

/gcbrun

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

Thanks, this is really helpful!

@douglasjacobsen
douglasjacobsen merged commit f00fe58 into Ramble-Project:develop Oct 2, 2026
24 checks passed
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.

3 participants