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

Make assignment evaluation lazy #953

Open
tomprince opened this issue Aug 27, 2021 · 9 comments
Open

Make assignment evaluation lazy #953

tomprince opened this issue Aug 27, 2021 · 9 comments

Comments

@tomprince
Copy link

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
Copy link
Owner

casey commented Aug 27, 2021

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
Copy link

hartror commented May 22, 2022

👋 @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
Copy link
Owner

casey commented May 28, 2022

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
Copy link

hartror commented May 28, 2022

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
Copy link
Owner

casey commented May 28, 2022

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
Copy link
Owner

casey commented May 28, 2022

More thoughts on editions here: #1201

@hartror
Copy link

hartror commented May 29, 2022

Sounds good I'll keep an eye on that.

@casey casey mentioned this issue Jun 15, 2022
@casey
Copy link
Owner

casey commented Jun 20, 2022

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.

@casey casey changed the title Allow backticks in variable substitutions to be lazy. Make assignment evaluation lazy Jun 20, 2022
@zx8
Copy link

zx8 commented Dec 12, 2023

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.

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

No branches or pull requests

4 participants