Skip to content
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.

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

TOML: use default profile values by default when another profile is active #4657

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

palinatolmach
Copy link
Contributor

@palinatolmach palinatolmach commented Oct 8, 2024

Follow up to runtimeverification/kontrol#825.

In Foundry, when parsing TOML config files, values set in a default profile (e.g., [prove.default] in kontrol.toml) are inherited by other profiles unless they are explicitly overridden in those profiles, which helps avoid redefining shared configuration settings between profiles (Foundry docs). We'd like to support the same behavior in kontrol.toml.

Similarly to a change made in runtimeverification/kontrol#825 to the foundry.toml parsing, this PR enforces reading the values set in both active and default profiles and using the default ones for values that are not set in the active profile. This PR also adds a test_prove_legacy_profiles test that ensures that both default and active profile's values are used.

I do realise, however, that this behavior (other profiles inheriting from default) is non-standard, so I'd appreciate any feedback on this PR. Maybe it'd be better if I added another parameter in get_profile that would enable this behavior only if the function is called by Kontrol?

@rv-jenkins rv-jenkins changed the base branch from master to develop October 8, 2024 11:06
@palinatolmach palinatolmach force-pushed the always-use-default-toml-profile branch from ef3f58f to 1dab690 Compare October 8, 2024 11:45
@palinatolmach palinatolmach force-pushed the always-use-default-toml-profile branch from 1dab690 to 0b0189c Compare October 8, 2024 11:47
@palinatolmach palinatolmach reopened this Oct 8, 2024
@palinatolmach palinatolmach marked this pull request as ready for review October 8, 2024 12:35
@palinatolmach palinatolmach self-assigned this Oct 9, 2024
Copy link
Contributor

@anvacaru anvacaru left a comment

Choose a reason for hiding this comment

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

This looks good to me. It shouldn't introduce any backward compatibility issues.

@palinatolmach palinatolmach merged commit 5ced750 into develop Oct 10, 2024
18 checks passed
@palinatolmach palinatolmach deleted the always-use-default-toml-profile branch October 10, 2024 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants