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

feat: introduce the @pkg-install alias #11046

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/dune_rules/alias0.ml
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@ let check = standard "check"
let install = standard "install"
let ocaml_index = standard "ocaml-index"
let runtest = standard "runtest"
let pkg_install = standard "pkg-install"
Copy link
Member

Choose a reason for hiding this comment

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

It's incorrect to mark this alias as "standard" as it's not defined in every single directory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If I don't mark it as standard, it will complain that it is not attached to any folder in the source. What is the best way to attached it to the source if I don't declare it as standard?

let all = standard "all"
1 change: 1 addition & 0 deletions src/dune_rules/alias0.mli
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ val check : Name.t
val ocaml_index : Name.t
val install : Name.t
val runtest : Name.t
val pkg_install : Name.t
val all : Name.t
val is_standard : Name.t -> bool
val register_as_standard : Name.t -> unit
14 changes: 14 additions & 0 deletions src/dune_rules/gen_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,19 @@ let gen_rules_group_part_or_root sctx dir_contents cctxs ~source_dir ~dir
contexts
;;

let gen_project_rule_for_package_alias sctx project =
let ctx = Super_context.context sctx in
let ctx_name = Context.name ctx in
let* is_lock_dir_active = Lock_dir.lock_dir_active ctx_name in
if is_lock_dir_active
then (
let dir =
Path.Build.append_source (Context.build_dir ctx) @@ Dune_project.root project
in
Pkg_rules.gen_rule_alias_from_package_universe ~dir ctx_name)
else Memo.return ()
;;

(* Warn whenever [(name <name>)]) is missing from the [dune-project] file *)
let missing_project_name =
Warning.make
Expand All @@ -355,6 +368,7 @@ let gen_project_rules =
and+ () = Odoc.gen_project_rules sctx project
and+ () = Odoc_new.gen_project_rules sctx project
and+ () = Ocaml_index.project_rule sctx project
and+ () = gen_project_rule_for_package_alias sctx project
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be done only once per context rather than for every project

and+ () =
let version = 2, 8 in
match Dune_project.allow_approximate_merlin project with
Expand Down
19 changes: 19 additions & 0 deletions src/dune_rules/pkg_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1937,6 +1937,25 @@ let build_rule context_name ~source_deps (pkg : Pkg.t) =
~directory_targets:[ pkg.write_paths.target_dir ]
;;

let gen_rule_alias_from_package_universe ~dir ctx_name =
let target_path_of_pkg pkg =
let pkg_name = Dune_lang.Package_name.to_string pkg in
Path.Build.L.relative
Private_context.t.build_dir
[ Context_name.to_string ctx_name; ".pkg"; pkg_name; "target" ]
Copy link
Member

Choose a reason for hiding this comment

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

You should go through the paths module to get this path

|> Path.build
in
let* packages =
Package_universe.lock_dir (Project_dependencies ctx_name)
>>| (fun lock_dir -> lock_dir.packages)
>>| Dune_lang.Package_name.Map.keys
in
let alias = Alias.make Alias0.pkg_install ~dir in
List.map ~f:target_path_of_pkg packages
|> Action_builder.paths
|> Rules.Produce.Alias.add_deps alias
;;

let gen_rules context_name (pkg : Pkg.t) =
let* source_deps, copy_rules = source_rules pkg in
let* () = copy_rules
Expand Down
6 changes: 5 additions & 1 deletion src/dune_rules/pkg_rules.mli
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ open Import
- A rule for fetching the source to produce .pkg/$package/source

- A rule to build the package and produce the artifacts in
.pkg/$package/target *)
.pkg/$package/target.

It setups an alias rules to trigger the fetch and build of the
package universe. *)

val setup_rules
: components:string list
Expand All @@ -22,3 +25,4 @@ val which : Context_name.t -> (Filename.t -> Path.t option Memo.t) Staged.t
val exported_env : Context_name.t -> Env.t Memo.t
val ocamlpath : Context_name.t -> Path.t list Memo.t
val find_package : Context_name.t -> Package.Name.t -> unit Action_builder.t option Memo.t
val gen_rule_alias_from_package_universe : dir:Path.Build.t -> Context_name.t -> unit Memo.t
74 changes: 74 additions & 0 deletions test/blackbox-tests/test-cases/pkg/build-pkg-deps.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
This test verifies the @pkg-deps alias fetch and build the project dependencies
without building the project itself.

$ . ./helpers.sh

Create a fake library to install in the target project as a dependency:
Copy link
Member

Choose a reason for hiding this comment

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

I didn't really get why you need a fake library and executable in this test. I see the following as sufficient:

  1. Write a lock directory
  2. Build your new alias
  3. Verify that the sources exist

Everything else seems redundant.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As we want to build the library and ensure it is installed, you need the fake library to make sure dune build correctly. Otherwise, it will fail when you try to use it as a dependency.

$ mkdir foo
$ cd foo
$ cat > dune-project <<EOF
> (lang dune 3.16)
> (package (name foo))
> EOF
$ cat > foo.ml <<EOF
> let foo = "Hello, World!"
> EOF
$ cat > dune <<EOF
> (library
> (public_name foo))
> EOF
$ cd ..
$ tar cf foo.tar foo
$ rm -rf foo

Make an opam package for the library:
$ mkpkg foo <<EOF
> build: [
> [
> "dune"
> "build"
> "-p"
> name
> "@install"
> ]
> ]
> url {
> src: "$PWD/foo.tar"
> }
> EOF

Create a project using the fake library as a dependency:
$ cat > dune-project << EOF
> (lang dune 3.16)
> (package
> (name bar)
> (allow_empty)
> (depends foo))
> EOF
$ add_mock_repo_if_needed
$ dune pkg lock
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need to involve the solver in this test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was the easiest way to write the test. However, it should be possible to write it without using the lock command. I'll rework it.

Solution for dune.lock:
- foo.0.0.1

Create an executable which uses the fake dependency. It ensures we build the
dependency package but not the package:
$ cat > dune << EOF
> (executable
> (name bar)
> (libraries foo))
> EOF
$ cat > bar.ml << EOF
> let () = print_endline "source"
> EOF

The alias call builds the `foo` dependency but not the project itself. We
verify we have the `foo` package but not the `bar.exe` in the build directory:
$ dune build @pkg-deps
$ ls _build/_private/default/.pkg/
foo
$ ls _build/default/
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was thinking about the way to detect that the build is happening is to maybe enable the console output and have the opam file echo "foo is building".

Copy link
Member

Choose a reason for hiding this comment

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

Why should there be a build happening? This alias is supposed to only download the deps.


Verify that building the `bar.exe` file is working:
$ dune build ./bar.exe
$ ls _build/default/bar.exe
_build/default/bar.exe
Copy link
Collaborator

Choose a reason for hiding this comment

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

And this could then just be replaced by dune exec ./bar.exe?

This has the slight downside that bar.exe could've been built before so maybe building bar.exe should also echo something to stdout?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It would mean to add a rule that depends on the bar.exe no?

Copy link
Collaborator

Choose a reason for hiding this comment

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

dune exec ./bar.exe automatically adds a dependency. Unless I misunderstand your question.

Loading