just icon indicating copy to clipboard operation
just copied to clipboard

Duplicate recipe and variable override order in imports is backwards

Open casey opened this issue 1 year ago • 2 comments

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 avatar Dec 22 '24 02:12 casey

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.

casey avatar Dec 22 '24 02:12 casey

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.

neunenak avatar Dec 22 '24 02:12 neunenak