Skip to content

Enforce dry run not changing opam root - #6565

Draft
rjbou wants to merge 18 commits into
ocaml:masterfrom
rjbou:dry-run
Draft

Enforce dry run not changing opam root#6565
rjbou wants to merge 18 commits into
ocaml:masterfrom
rjbou:dry-run

Conversation

@rjbou

@rjbou rjbou commented Jun 20, 2025

Copy link
Copy Markdown
Collaborator

No description provided.

@rjbou rjbou added the PR: WIP Not for merge at this stage label Jun 20, 2025
@rjbou rjbou changed the title Enforce dry run not changin opam root Enforce dry run not changing opam root Jun 20, 2025
@rjbou

rjbou commented Sep 4, 2025

Copy link
Copy Markdown
Collaborator Author

I added several tests in the PR, all are fixed. I'm looking for other tests to add, even if they are not fixed in this PR.
Also i'm wondering if the PR needs a split, there is a lot of little fixes, but they can be grouped by topic (switch, pin, etc.). In all cases, the tests addition needs to contain all the cases.

Comment thread src/state/opamUpdate.ml
match OpamStateConfig.(!r.dryrun), result with
| true, Result () ->
let base f = OpamFilename.Base.of_string (OpamFilename.Dir.to_string f) in
(match OpamRepositoryBackend.get_diff (OpamFilename.Dir.of_string "/") (base srcdir) (base srcdir0) with

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

temporary hack, get_diff needs to change arguments and take absolute path, or have 2 functions.

let srcdir = OpamPath.Switch.pinned_package root st.switch nv.name in
let srcdir =
if OpamStateConfig.(!r.dryrun) then
OpamFilename.mk_tmp_dir ()

@rjbou rjbou Sep 4, 2025

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

i didn't check if all these temp dir created are well cleaned

Comment thread tests/reftests/dryrun.test Outdated
@rjbou rjbou added the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Sep 11, 2025
@kit-ty-kate kit-ty-kate linked an issue Sep 12, 2025 that may be closed by this pull request
@kit-ty-kate

Copy link
Copy Markdown
Member

Does this also fix #6492 ?

@kit-ty-kate

Copy link
Copy Markdown
Member

Per 2025/09/08 dev meeting: we don't need to split this PR up. The remaining thing to do is to move the API change to another PR, to add some more tests and figure out what needs to be done with OpamRepositoryBackend.get_diff.

@kit-ty-kate kit-ty-kate added this to the 2.6.0~alpha1 milestone Oct 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: QUEUED Pending pull request, waiting for other work to be merged or closed PR: WIP Not for merge at this stage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

switch create --dry-run actually creates folder

2 participants