Skip to content

Add argument --ignore-available-on to ignore availibility of specific packages - #6836

Merged
kit-ty-kate merged 2 commits into
ocaml:masterfrom
WardBrian:feature-5283-force-available
Jul 16, 2026
Merged

Add argument --ignore-available-on to ignore availibility of specific packages#6836
kit-ty-kate merged 2 commits into
ocaml:masterfrom
WardBrian:feature-5283-force-available

Conversation

@WardBrian

@WardBrian WardBrian commented Dec 15, 2025 •
edited by kit-ty-kate
Loading

Copy link
Copy Markdown
Contributor

This is a proposed new argument to resolve the feature request in #5283.

I've decided to make it a separate argument from the --ignore-constraints-on argument mentioned there, but it's an easy modification if we'd prefer that argument to do double duty.

As a minimum demonstration, here is me trying to install the osx-plutil opam package, which is marked as only available on macos, on my ubuntu machine:

$ ./opam install osx-plutil 
[ERROR] osx-plutil: unmet availability conditions: 'os = "macos"'
$ ./opam install --force-available osx-plutil osx-plutil 
The following actions will be performed:
=== install 3 packages
  ∗ osx-plutil 0.5.0
  ∗ process    0.2.1 [required by osx-plutil]
  ∗ result     1.5   [required by osx-plutil]

Proceed with ∗ 3 installations? [Y/n] n

I have thus far made exactly the set of modifications that seemed necessary to get this working and no more.
I suspect that there is at least a couple more places this information would need to be propagated to be 'feature complete', but I decided to pause here to kindly ask for feedback on the general approach before continuing.

Fixes #5283

@WardBrian WardBrian mentioned this pull request Dec 15, 2025
Closed
@kit-ty-kate kit-ty-kate added this to the 2.6.0~alpha1 milestone Dec 15, 2025
kit-ty-kate
kit-ty-kate reviewed Dec 15, 2025
View reviewed changes

@kit-ty-kate kit-ty-kate 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 looks globally really good, thanks a lot. Modulo this single command i don't see anything wrong on first glance.

Do you have extra time to add some tests for this feature? If not we'll do it whenever we get around to properly review this.

If you do, the documentation can be found here: https://github.com/ocaml/opam/tree/master/tests/reftests#readme and all the commands that use build_options should probably be tested for completeness (e.g. install, reinstall, remove, upgrade)

Comment thread src/client/opamArg.ml Outdated
@WardBrian

WardBrian commented Dec 15, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@kit-ty-kate tests added! This also forced me to flesh out the other places that it is needed, and move the logic into a single val force_available: 'a switch_state -> OpamPackage.Name.Set.t -> 'a switch_state function, which means that I no longer needed to add ?force_available arguments to all the functions, I just give them a switch state with the already-good-to-go availabilities.

I think this is better, and it might even make sense to roll into the OpamSwitchState.with_ function, but I didn't want to go touching too many existing systems.

Edit: I moved the logic into OpamSwitchState.load, as this seems like a central place that already reads from OpamStateConfig

@WardBrian
WardBrian marked this pull request as ready for review December 15, 2025 19:52
kit-ty-kate
kit-ty-kate reviewed Dec 15, 2025
View reviewed changes

@kit-ty-kate kit-ty-kate 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.

a couple of minor comments to remove now unused additions

Comment thread src/state/opamStateConfig.ml Outdated
Comment thread src/client/opamArg.ml Outdated
Comment thread src/state/opamStateConfig.ml Outdated
Comment thread src/state/opamStateConfig.ml Outdated
Comment thread master_changes.md Outdated
Comment thread src/state/opamStateConfig.mli Outdated
@kit-ty-kate
kit-ty-kate requested a review from rjbou December 15, 2025 20:57
@kit-ty-kate

kit-ty-kate commented Dec 15, 2025

Copy link
Copy Markdown
Member

Thinking a little bit more, i think the option might be better named --ignore-available-on to make it more clear that the option is expecting an argument, to make it easier to search for people coming from --ignore-constraints-on and to avoid clashes with a potential future --force option.

@rjbou what do you think?

@WardBrian

WardBrian commented Dec 15, 2025

Copy link
Copy Markdown
Contributor Author

I do not believe that the code you've flagged above is unused -- deleting it and chasing down the subsequent compiler errors ends up essentially reverting the PR.

i think the option might be better named --ignore-available-on

Happy to go with whatever name is preferred

@kit-ty-kate

kit-ty-kate commented Dec 15, 2025

Copy link
Copy Markdown
Member

I do not believe that the code you've flagged above is unused -- deleting it and chasing down the subsequent compiler errors ends up essentially reverting the PR.

mmh, that doesn't sound right. These 4 lines are all related to the parsing of the OPAMFORCEAVAILABLE environment variable that was removed. My only mistake was to suggest the removal of ?force_available:(...) instead of ?force_available:None. With this it compiles and works just fine.

@WardBrian

WardBrian commented Dec 15, 2025

Copy link
Copy Markdown
Contributor Author

?force_available:(...) instead of ?force_available:None. With this it compiles and works just fine.

Indeed, that was the piece that was breaking everything if I let the compiler guide me (The ml/mli disagreement errors you get when dealing with both continuations-style functions and module includes are not something with which I am all that fluent).

@WardBrian WardBrian changed the title Add argument --force-available to ignore availibility of specific packages Add argument --ignore-available-on to ignore availibility of specific packages Dec 15, 2025
kit-ty-kate
kit-ty-kate approved these changes Dec 15, 2025
View reviewed changes

@kit-ty-kate kit-ty-kate 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 modulo some really minor stylistic choice (we generally have a 80 columns rule for new lines of code) and commit reorganization, that we can do ourselves just before merging by force-pushing.

I'll let @rjbou do a second review, but overall i believe this should be good to go in 2.6 whose first alpha is planned around February

@WardBrian

WardBrian commented Dec 15, 2025

Copy link
Copy Markdown
Contributor Author

Thanks for the pointers!

@WardBrian

WardBrian commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

I've just run into another case (ocaml/opam-repository#30147) where this argument would have been very useful

@kit-ty-kate
kit-ty-kate force-pushed the feature-5283-force-available branch 2 times, most recently from 3e3cedd to 1aece0c Compare July 14, 2026 22:11
@kit-ty-kate

kit-ty-kate commented Jul 14, 2026

Copy link
Copy Markdown
Member

I squashed and renamed the commits to make them ready to merge (first force-push). Do they sound good to you?
I also brought back the IGNOREAVAILABLE environment variable as i suspect @rjbou will prefer to have it given the structure of the code.

The second force-push is here to simply rebase on master and update the tests result.

@WardBrian

WardBrian commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

They do sound good to me. Thank you!

@kit-ty-kate
kit-ty-kate force-pushed the feature-5283-force-available branch from 1aece0c to a530b68 Compare July 15, 2026 22:21
@rjbou

rjbou commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

The second force-push is here to simply rebase on master and update the tests result.

github no longer show the first force push when there are done within a short period of time i guess

@kit-ty-kate kit-ty-kate force-pushed the feature-5283-force-available branch 2 times, most recently from 3e3cedd to 1aece0c yesterday

rjbou
rjbou approved these changes Jul 16, 2026
View reviewed changes

@rjbou rjbou 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!

Comment thread tests/reftests/ignore-available-on.test Outdated
@kit-ty-kate
kit-ty-kate force-pushed the feature-5283-force-available branch from a530b68 to d904e10 Compare July 16, 2026 10:29
@kit-ty-kate

kit-ty-kate commented Jul 16, 2026

Copy link
Copy Markdown
Member

github no longer show the first force push when there are done within a short period of time i guess

sigh... does it still show up in your email inbox at least? (not ideal so i'll have to think about a better solution for the future, but good to know for now)

@WardBrian

WardBrian commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Yes, there are still separate emails in the thread

@kit-ty-kate

kit-ty-kate commented Jul 16, 2026

Copy link
Copy Markdown
Member

Thanks a lot!

@kit-ty-kate
kit-ty-kate merged commit 80a9dcf into ocaml:master Jul 16, 2026
41 checks passed
@WardBrian
WardBrian deleted the feature-5283-force-available branch July 23, 2026 20:42
@kit-ty-kate kit-ty-kate mentioned this pull request Sep 1, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kit-ty-kate kit-ty-kate kit-ty-kate approved these changes
@rjbou rjbou rjbou approved these changes

Assignees

No one assigned

Labels

AREA: UI KIND: FEATURE WISH

Projects

None yet

Milestone

2.6.0~alpha1

Development

Successfully merging this pull request may close these issues.

--ignore-constraints-on doesn't ignore package available constraints

3 participants

@WardBrian @kit-ty-kate @rjbou