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

pkg: copy files from opam repository to lock dir #8648

Conversation

Alizter
Copy link
Collaborator

@Alizter Alizter commented Sep 14, 2023

The opam solver now copies any files in the files directory inside a package directory in an opam repository.

@@ -482,5 +482,40 @@ let solve_lock_dir solver_env version_preference (repo, repo_id) ~local_packages
| Ok pkgs_by_name ->
Lock_dir.create_latest_version pkgs_by_name ~ocaml:None ~repo_id
in
summary, lock_dir)
let files =
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rgrinberg Is there a better way to do this?

src/dune_pkg/opam_solver.ml Outdated Show resolved Hide resolved
src/dune_pkg/opam_solver.ml Outdated Show resolved Hide resolved
src/dune_pkg/lock_dir.mli Outdated Show resolved Hide resolved
src/dune_pkg/lock_dir.ml Outdated Show resolved Hide resolved
@Alizter Alizter force-pushed the ps/branch/pkg__copy_files_from_opam_repository_to_lock_dir branch from 2293d33 to 991dbe3 Compare September 14, 2023 12:34

The error message should have a location for the opam repository.

This does not currently seem to be the case.
Copy link
Collaborator Author

@Alizter Alizter Sep 14, 2023

Choose a reason for hiding this comment

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

This should be fixed once this todo is resolved:

dune/bin/pkg.ml

Lines 368 to 369 in b982acf

(* TODO figure out the loc situation *)
let repo_id = Option.map ~f:(fun repo_id -> Loc.none, repo_id) repo_id in

cc @Leonidas-from-XIV

@@ -13,4 +13,6 @@ val solve_lock_dir
-> Version_preference.t
-> Opam_repo.t * (Loc.t * Repository_id.t) option
-> local_packages:OpamFile.OPAM.t OpamTypes.name_map
-> (Summary.t * Lock_dir.t, [ `Diagnostic_message of _ Pp.t ]) result
-> ( Summary.t * Lock_dir.t * Lock_dir.Write_disk.Files_entry.t Package_name.Map.Multi.t
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Shall I make this a record?

Copy link
Member

Choose a reason for hiding this comment

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

Not in this PR

@Alizter Alizter force-pushed the ps/branch/pkg__copy_files_from_opam_repository_to_lock_dir branch from 991dbe3 to e9e7772 Compare September 14, 2023 13:32
Signed-off-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter Alizter force-pushed the ps/branch/pkg__copy_files_from_opam_repository_to_lock_dir branch from e9e7772 to eb3e3bb Compare September 14, 2023 13:35
_
Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
@rgrinberg rgrinberg merged commit 7d720b0 into ocaml:main Sep 14, 2023
19 of 23 checks passed
@Alizter Alizter deleted the ps/branch/pkg__copy_files_from_opam_repository_to_lock_dir branch September 14, 2023 16:13
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.

None yet

2 participants