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

Duplicate recipe and variable override order in imports is backwards #2540

Open
casey opened this issue Dec 22, 2024 · 2 comments
Open

Duplicate recipe and variable override order in imports is backwards #2540

casey opened this issue Dec 22, 2024 · 2 comments

Comments

@casey
Copy link
Owner

casey commented Dec 22, 2024

When duplicate recipes are in the same module, the later one overrides the earlier one:

set allow-duplicate-recipes

foo:

foo:
  echo winner

When they are in imports, because just uses a stack to process imports, the duplicate in the earlier module will win, because it will be processed last:

# a.just
foo:
  echo winner

# b.just
foo:

# justfile
import 'a.just'
import 'b.just'

This was discovered by @redsun82 in #2523.

This is not great behavior, and pretty clearly a bug. I'm not sure, but I don't think we ever considered this case when designing either imports or allow-duplicate-recipes. (@neunenak do you have any recollection of this? I'm thinking that we just didn't notice it.)

just has a very strong backwards compatibility guarantee, but this is definitely a bug and should probably be fixed.

However, I feel like there are probably a bunch of existing justfiles that rely on this behavior, and they're likely especially complicated, so such a bug fix could be disruptive.

The most non-disruptive way to fix it would be:

  1. Add a setting which opts into the new behavior
  2. Print a warning if a justfile doesn't opt in to the new behavior and doing so would change which recipes are defined
  3. Elevate the warning in 2 to an error
  4. After some suitably long period of time, change the default to the new behavior, and make the setting a no-op

We could also add a setting which opts into the new behavior, and then wait and see if it's worth actually changing the default behavior.

@casey
Copy link
Owner Author

casey commented Dec 22, 2024

No tests break when we use the stack like a queue:

while !stack.is_empty() {
  let ast = stack.remove(0);}

So we probably just didn't consider this.

@neunenak
Copy link
Contributor

This is not great behavior, and pretty clearly a bug. I'm not sure, but I don't think we ever considered this case when designing either imports or allow-duplicate-recipes. (@neunenak do you have any recollection of this? I'm thinking that we just didn't notice it.)

Back when this feature was called include and was experimental, I remember proposing that the semantics of a justfile with includes should always be identical to that of a justfile that had the literal contents of the included justfile copy-pasted in place of the include line. That principle would imply that this should definitely be considered a just bug, although yeah this is probably something that just slipped in as a result of using a stack for import processing that no one thought to explicitly test for.

I wonder if there are other weird semantics that result from using a stack for import processing in this way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants