Skip to content

test: aliases field of user rules generates multiple actions #11557

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 23, 2025
Merged

test: aliases field of user rules generates multiple actions #11557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4c767a
test: aliases field of user rules generates multiple actions
Alizter Mar 22, 2025
767faf0
fix: user rules with multiple aliases generate single action
Alizter Mar 22, 2025
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
14 changes: 10 additions & 4 deletions src/dune_rules/simple_rules.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,16 @@ let user_rule sctx ?extra_bindings ~dir ~expander (rule : Rule_conf.t) =
Some targets
| Aliases_only aliases ->
let+ () =
let action = interpret_and_add_locks ~expander rule.locks action.build in
Memo.parallel_iter aliases ~f:(fun alias ->
let alias = Alias.make ~dir alias in
Alias_rules.add sctx ~alias ~loc:rule.loc action)
match List.map ~f:(Alias.make ~dir) aliases with
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we have the same issue above with Aliases_with_targets ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There we are using Rules.Produce.Alias.add_deps which correctly registers the deps without introducing any anonymous actions. Here we are using Super_context.add_alias_action which is creating an anonymous action. This is a wrapper around Rules.Produce.Alias.add_action which for our purposes is probably not what we want.

I've gone ahead and replaced this with Rules.Produce.Alias.add_deps here as well and added a test to make sure the with_targets version is treated the same.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can confirm that with_targets did not have the issue I am fixing here.

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 you look at my latest push, there is a test for both with targets and without and as I mentioned the one with targets does not run twice. The bug I am fixing here only appears to be an issue for rules with no targets.

| [] -> Code_error.raise "empty list of aliases" []
| alias :: extra_aliases ->
let loc = rule.loc in
interpret_and_add_locks ~expander rule.locks action.build
|> Alias_rules.add sctx ~alias ~loc
>>> Memo.parallel_iter extra_aliases ~f:(fun extra_alias ->
Dep.alias alias
|> Action_builder.dep
|> Rules.Produce.Alias.add_deps ~loc extra_alias)
in
None)
;;
Expand Down
27 changes: 26 additions & 1 deletion test/blackbox-tests/test-cases/alias-multiple.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ Updating the dune-project file to use dune 3.5 allows the build to succeed:
$ dune build @a
I have run
$ dune build @b
I have run

Also note having both the alias and aliases fields in the same rule stanza is
not allowed
Expand Down Expand Up @@ -101,3 +100,29 @@ Even if the aliases list is empty
4 | (action (echo "I have run")))
Error: fields "alias" and "aliases" are mutually exclusive.
[1]

Building both aliases at the same time should only run the action once
$ cat > dune << EOF
> (rule
> (aliases a b)
> (action (echo "I have run\n")))
> EOF

$ dune clean
$ dune build @a @b
I have run

A similar test with a rule that produces a target
$ cat > dune << EOF
> (rule
> (targets a)
> (aliases b c)
> (action
> (progn
> (echo "I have run\n")
> (with-stdout-to a (echo "foo")))))
> EOF

$ dune clean
$ dune build @b @c
I have run
Loading