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

depexts: add more precise depext testing family #5453

Merged
merged 3 commits into from
Mar 15, 2023

Conversation

rjbou
Copy link
Collaborator

@rjbou rjbou commented Feb 24, 2023

** Updated to new version**

Syntax is dummy-<success|failure>[:<*|0|pkgslist>:*|0|pkgslist>]
Examples:

  • dummy-success: install succeed, default: no package installed, all packages available
  • dummy-success:depext-ok:: install succeed, only depext-ok installed, all packages available
  • dummy-failure::*: install fails, no package installed, all packages available
  • dummy-success:depext-ok:depext-avail: install succeed, only depext-ok installed, only depext-avail available
  • dunny-failure::0: install fails, no package installed, no package available

** old version **

Syntax is dummy-<success|failure>[#installed@<all|none|pkgslist>][#avail@<all|none|pkgslist>]
Examples:

  • dummy-success
  • dummy-success#installed@depext-ok
  • dummy-failure#avail@all
  • dummy-success#installed@depext-ok#avail@depext-avail

@rjbou rjbou added the PR: WIP Not for merge at this stage label Feb 24, 2023
@rjbou rjbou added this to the 2.2.0~alpha milestone Feb 24, 2023
@rjbou rjbou added this to PR in Out of release : doc, test, install, etc. via automation Feb 24, 2023
@rjbou rjbou added this to PR in progress in Opam 2.2.0 via automation Feb 24, 2023
@rjbou rjbou moved this from PR in progress to PR to review for 2.2.0~alpha in Opam 2.2.0 Feb 28, 2023
Copy link
Member

@dra27 dra27 left a comment

Choose a reason for hiding this comment

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

At a great risk of bike-shedding, can I propose a slightly simpler syntax?

dummy-<success|failure>[:<available-pkgs>:<installed-pkgs>]

Where <pkgs> is:

  • * - all packages are available/installed
  • 0 - no packages are available/installed
  • pkg_1,...,pkg_n

The principal advantage is that's trivial to parse (it can be done with split_on_char). At the moment, there are corner cases which come up which just seem worth avoiding - e.g. dummy-success#installed@all#installed@none.

The examples in the PR where this is used are then just:

  • dummy-success#avail@nonedummy-success:0:0
  • dummy-success#avail@os1-depextdummy-success:os1-depext:0
  • dummy-success#avail@os2-depext#installed@os1-depextdummy-success:os2-depext:os1-depext

src/state/opamSysInteract.ml Show resolved Hide resolved
@kit-ty-kate kit-ty-kate self-assigned this Mar 14, 2023
@rjbou rjbou added PR: NEEDS UPDATE and removed PR: WIP Not for merge at this stage labels Mar 14, 2023
@rjbou rjbou moved this from PR to review for 2.2.0~alpha to PR to update for 2.2.0~alpha in Opam 2.2.0 Mar 15, 2023
Syntax is `dummy-<success|failure>[:<*|0|pkgslist>:*|0|pkgslist>]`
Examples:
- `dummy-success`: install succeed, default: no package installed, all packages available
- `dummy-success:depext-ok:`: install succeed, only `depext-ok` installed, all packages available
- `dummy-failure::*`: install fails, no package installed, all packages available
- `dummy-success:depext-ok:depext-avail`: install succeed, only `depext-ok` installed, only `depext-avail` available
- `dunny-failure::0`: install fails, no package installed, no package available
@rjbou rjbou moved this from PR to update for 2.2.0~alpha to PR to review for 2.2.0~alpha in Opam 2.2.0 Mar 15, 2023
@kit-ty-kate kit-ty-kate moved this from PR to review for 2.2.0~alpha to PR finalised (merge with CI) in Opam 2.2.0 Mar 15, 2023
@kit-ty-kate
Copy link
Member

Thanks!

@kit-ty-kate kit-ty-kate merged commit b28362f into ocaml:master Mar 15, 2023
Opam 2.2.0 automation moved this from PR finalised (merge with CI) to Done Mar 15, 2023
Out of release : doc, test, install, etc. automation moved this from PR to done Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants