casey / just

🤖 Just a command runner
https://just.systems
Creative Commons Zero v1.0 Universal
21.42k stars 476 forks source link

Make assignment evaluation lazy #953

Open tomprince opened 3 years ago

tomprince commented 3 years ago

I'd like to be able to define variables with backtick substitutions that are only run if the variable is referenced (I'd be fine if all the variables used in any commands are run before any of the commands run).

The use case I have is having a justfile that runs in multiple contexts, and some of the backtick substitutions can only be run in one of the contexts:

nix_volume_path := `podman volume inspect --format '{{.Mountpoint}}' nix`

build:
    podman exec nix just /src/_build

_build
   nix build ...

where the podman volume inspect does not succeed in the inner context.

casey commented 3 years ago

That sounds reasonable. I think I've actually run into this myself, where I have top-level backticks that are really only applicable to certain recipes, but have preconditions that aren't always satisfied.

I think it would be fine to evaluate backticks before running any recipe, or track which backticks a recipe depends on, and evaluate them before running the recipe.

hartror commented 2 years ago

👋 @casey, have been using just at work a fair bit so I thought I'd contribute and this issue looks fun.

What did you have in mind here?

Making all backticks evaluate as required seems tricky as it would impact anything that depends on a Expression with a backtick?

Alternatively making assignments lazy seems doable perhaps by modifying Scope.value to evaluate and cache assignment values. This however means Scope.value could become recursive an undesirable attribute of something driven by user input. The dependencies runs Evaluator -> Scope right now which is also an issue.

Tracking dependencies of recipes seems like it could cause issues for use-cases such as the one tomprince has laid out? The only way to do it is collect for all code branches which could lead to evaluation of backticks in branches intended for a different environment if `uname -a` ....

Finally this feature means the evaluation order of assignments becomes unpredictable and users may rely on side effects?

casey commented 2 years ago

Sorry for the slow reply here!

I think I haven't really thought this through very careful, so it might not work, but my thought would be to figure out which assignments are going to be used by the recipes that are being run, and then evaluate only those assignments before running any recipes. So they wouldn't be lazy, in the sense of being evaluated when needed, but instead would be all evaluated up front, but only if they were going to be used later. I don't know how annoying figuring out which assignments are used for which recipes would be though.

Finally this feature means the evaluation order of assignments becomes unpredictable and users may rely on side effects?

I think that evaluation order wouldn't be unreliable, per se. Assignments would be evaluated before any recipe ran, some of them would just be skipped.

Users might be relying on side effects of backticks in unused expressions. That's a little weird, but I would hate to break users who are relying on this behavior.

hartror commented 2 years ago

No worries!

I'll take a look at how to collect recipe assignment dependencies.

With side effects I think there needs to be a setting as I don't think this feature should be enabled when set export is true. So a setting can ensure that a user isn't relying on both features at the same time and that generally just's behaviour stays the same for side effects.

Exporting an assignment should always evaluate.

casey commented 2 years ago

Exporting an assignment should always evaluate.

Nice catch, I forgot about this! Yeah, definitely, exporting an assignment should always evaluated.

This is the kind of thing that although unlikely to be disruptive, is technically a breaking change.

I've been thinking about how to handle this, now that just is at version 1.0 and has strong stability guarantees. This is a small change, and you could say that when backticks are evaluated is undefined, so they can't be relied on for side-effects.

Another option is to hide it behind a setting, like:

set lazy-eval := true

So that users have to opt into it.

One thing I've been thinking about is that breaking changes are added behind individual settings that the user has to turn on, and then every once in a while, we announce a new "edition" of just, that users can opt-into, that flips a bunch of settings from opt-in to opt-out. This is entirely stolen from Rust's approach to making backwards-compatible changes to the language.

So the way it would work would be:

  1. Setting foo is added to just as a backwards compatible, opt-in change
  2. Setting bar is added to just as a backwards compatible, opt-in change
  3. From experience, we find out that foo and bar should be true by default
  4. We announce a new edition of just, that users can opt-into with set edition := 2, and when they do that, foo and bar are opt-out instead of opt-in

This has the downside that people have to put set edition := LATEST_EDITION in their justfiles, but has the advantage that we never break even edge-case justfiles, and we don't have to think too hard about cases like this which are probably good and don't break things, but which we can't be 100% sure about.

casey commented 2 years ago

More thoughts on editions here: #1201

hartror commented 2 years ago

Sounds good I'll keep an eye on that.

casey commented 2 years ago

Good point from #1233, that you might want things other than backticks to be lazy. This makes me think that maybe it would be best for whole assignments to be lazy, which I think is basically what we were talking about, but the issue title doesn't necessarily make clear.

zx8 commented 11 months ago

This functionality is the very last piece that's preventing us going all-in and replacing our Makefiles with Justfiles. In a large number of our repositories we have "expensive" operations (e.g. retrieving remote credentials) that are only executed for specific targets, and we wouldn't want this API call to be made on every target invocation, regardless of if they're actually needed or not.

As it stands, it adds ~1 second to every Justfile target invocation, to the point we had to reluctantly go back our old Makefiles. Using Makefiles, we achieve this using the ?= lazy assignment operator instead of := operator, which defers evaluation until the point the variable is referenced for the first time.