-
Notifications
You must be signed in to change notification settings - Fork 454
fix: switching to custom lock dir #12712
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -177,11 +177,23 @@ let get_source_path_for_context ctx_name = | |
| | false -> None | ||
| | true -> Some ctx) | ||
| with | ||
| | None | Some (Default { lock_dir = None; _ }) -> Memo.return (Some default_source_path) | ||
| | Some (Default { lock_dir = Some lock_dir_selection; _ }) -> | ||
| let+ source_lock_dir = select_lock_dir lock_dir_selection in | ||
| Some source_lock_dir | ||
| | Some (Opam _) -> Memo.return None | ||
| | None | Some (Default { lock_dir = None; _ }) -> | ||
| (* CR-someday Alizter: The logic here is hard to follow. Improve the lock | ||
| directory selection logic so that it is harder to make a mistake. This | ||
| can be done by enforcing invariants about contexts and their lock | ||
| directories slightly earlier. *) | ||
| Memo.return | ||
| @@ | ||
| (* Context doesn't specify a lock_dir, check if there's a top-level one *) | ||
| (match workspace.lock_dirs with | ||
| | [ lock_dir ] -> Some lock_dir.path | ||
| | [] | _ :: _ :: _ -> | ||
| (* No clear choice, fall back to default *) | ||
| Some default_source_path) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But what if It might be correct; my question is more a behavioral: if the workspace specifies lock dirs, do they exist in addition to the default lock dir or replacing the default lock dir? Personally looking from a user perspective I would assume the latter, but I think it is worth being aware of the choice. |
||
| ;; | ||
|
|
||
| let get_path ctx_name = | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| This test checks what happens when you lock with default directory then change | ||
| workspace to use a custom lock directory. | ||
|
|
||
| $ . ./helpers.sh | ||
| $ mkrepo | ||
| $ mkpkg foo | ||
| $ add_mock_repo_if_needed | ||
|
|
||
| $ cat > dune-project <<EOF | ||
| > (lang dune 3.21) | ||
| > (package | ||
| > (name myproject) | ||
| > (depends foo)) | ||
| > EOF | ||
|
|
||
| Lock with default directory name: | ||
|
|
||
| $ dune pkg lock | ||
| Solution for dune.lock | ||
|
|
||
| Dependencies common to all supported platforms: | ||
| - foo.0.0.1 | ||
|
|
||
| Now change the workspace to use a custom lock directory: | ||
|
|
||
| $ cat > dune-workspace <<EOF | ||
| > (lang dune 3.21) | ||
| > (lock_dir | ||
| > (path custom.lock) | ||
| > (repositories mock)) | ||
| > (repository | ||
| > (name mock) | ||
| > (url "file://$PWD/mock-opam-repository")) | ||
| > EOF | ||
|
|
||
| $ cat > dune <<EOF | ||
| > (executable | ||
| > (public_name bar)) | ||
| > EOF | ||
|
|
||
| $ cat > bar.ml <<EOF | ||
| > let () = print_endline "hello from bar" ;; | ||
| > EOF | ||
|
|
||
| $ dune exec -- bar | ||
| hello from bar | ||
|
|
||
| Now try with a context stanza that references the custom lock directory with a | ||
| context stanza: | ||
|
|
||
| $ cat > dune-workspace <<EOF | ||
| > (lang dune 3.21) | ||
| > (context | ||
| > (default | ||
| > (lock_dir custom.lock))) | ||
| > (lock_dir | ||
| > (path custom.lock) | ||
| > (repositories mock)) | ||
| > (repository | ||
| > (name mock) | ||
| > (url "file://$PWD/mock-opam-repository")) | ||
| > EOF | ||
|
|
||
| This is working as intended. The error is not relevant here. | ||
|
|
||
| $ dune exec -- bar | ||
| hello from bar |
Uh oh!
There was an error while loading. Please reload this page.