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

Fallback justfile stabilization tracking issue #1148

Closed
casey opened this issue Mar 30, 2022 · 23 comments
Closed

Fallback justfile stabilization tracking issue #1148

casey opened this issue Mar 30, 2022 · 23 comments

Comments

@casey
Copy link
Owner

casey commented Mar 30, 2022

This is a tracking issue for stabilizing the currently unstable feature which allows just to check justfiles in parent directories if a recipe is missing from the first justfile it finds.

The feature is currently unstable. Please comment with any feedback, suggestions, or if you like the feature and think it's ready for stabilization.

Update 2022-10-19

Looks like this is getting stabilized, with a change to behavior. The change is that fallback through a justfile in which a recipe is not found will only occur if the justfile contains set fallback or set fallback := true.

Stabilization timeline:

  • 2022-10-19 - The unstable version with the new behavior is released in just 1.6.0
  • 2022-10-26 - Released just 1.7.0 with the setting defaulting to true
  • 2022-11-25 - Release just 1.9.0 with the setting defaulting to false
  • 2023-1-1 - Stabilize the new feature, barring any issues

Please try out the new setting-based fallback in just 1.6.0 and let me know if you have any issues. Also, regardless of issues, let me know if it works or doesn't work for your use case.

@casey casey changed the title Run ascendant tracking issue Fallback justfile stabilization tracking issue Mar 31, 2022
@heavelock
Copy link
Contributor

Hey!

I just started exploring that feature and it's great, thanks for adding it!

Are you planning to include also fallback of variables between parent/child justfiles?

In the current setup, I have some 'global variables' defined in the main justfile that I am using in a command. Now, if I want to overwrite that command via child justfile, I also need to redefine all of the variables, there is no fallback for them.
The same issue is with using just settings such as set shell that require to be defined both in the parent and child justfiles.

I guess that might not be exactly what this issue is about. Probably a better implementation would not be rather an explicit import to force just to get all settings and vars from the parent.

@casey
Copy link
Owner Author

casey commented Apr 27, 2022

I just started exploring that feature and it's great, thanks for adding it!

You bet, glad you like it!

Are you planning to include also fallback of variables between parent/child justfiles?

I don't think so. This feature is a simple stopgap to make it easier to work with multi-justfile repositories. For something where recipes and variables from multiple justfiles in the same invocation, have a look at #929. There's no active work on that at the moment, but some day!

@nk9
Copy link
Contributor

nk9 commented Jun 30, 2022

I wanted this and then discovered that it was already available! I'm planning to use it to have a justfile in ~ with general shell shortcuts.

The question I have about the current implementation is about tab completion. This seems like it would become even more important with an increased number of recipes. Presently, it looks like tab completion only works with recipes in the first justfile found. I wonder what the right solution is:

  1. Keep the current behavior.
  2. Tab complete shows recipes from all found justfiles.
  3. Tab complete only shows recipes from from the justfile in which it found the first match.

I can't decide if 2 would be too much. Possibly it could get confusing, since two different files could contain multiple receipes with the same name. The issue with 3 is that the tab completions could appear to change as you changed the initial string.

I'm not really loving the first behavior, though. Now that I've turned on tab completion, I find myself relying on it for nearly every invocation. If I can't use it for the upper-level justfiles, that would make me less likely to use them.

@runeimp
Copy link

runeimp commented Jul 2, 2022

I think #2 would be fine in the majority of cases but if someone with god like completion building skills could potentially list completions that aren't for the active Justfile under those that are. Possibly with some sort of delimiter. I'm pretty sure you can do anything you could possibly want with Zsh. It's incredibly sophisticated and harder to learn than C++. Bash completions might be able to pull something like that off. And I suspect other newer shells like Fish, Nu or PowerShell can likely handle it as well.

But I think that ultimately if submodules become a reality that at the very least completions should take submodules into consideration.

@nk9
Copy link
Contributor

nk9 commented Jul 4, 2022

Interesting situation brought up in the Discord channel while might affect this feature:

I'm trying to write my justfiles so basic commands like build, run etc. automatically work on Windows and Linux, even when those commands do different things on each OS.

I'm currently doing this - it works but is this an idiomatic way to do it?

build:
    just build-{{ os_family() }}

build-unix:
    # unix build command goes here

build-windows:
    # windows build command goes here

When this feature is implemented, that call to just in the build recipe might not find the expected justfile. It could lead to some pretty tricksy bugs if a recipe in the current justfile was being shadowed by the recipe in a file deeper in the hierarchy.

I think a solution to this would be to make calls to just itself from within a justfile always act as if the call was actually just -f {{ justfile() }}.

@runeimp
Copy link

runeimp commented Jul 7, 2022

I think it would be best that if there are conflicts Just exists with an error explaining the shadowing issue. Then the user would be aware of the issue and could fix it as they see fit. Maybe Just could provide suggestions for common situations as time goes on as well.

@vjustov
Copy link

vjustov commented Aug 1, 2022

Hello Everyone, i just started looking into just because we want it to replace a custom build tool.
right now just --unstable -l just return the tasks in the nearest justfile. it would be nice if it would merge the tasks of al the justfile in the path, showing an error/warning mentioning the shadowing like @runeimp says.

@casey
Copy link
Owner Author

casey commented Sep 3, 2022

This has been unstable for a while, and no complaints so far. @52f73db mentioned that he was using it in #1148, and it seems like it might be time to stabilize it. One concern I have is that this is a change in behavior. If a user is relying on getting an error in a sub-justfile, they might be surprised when the recipe runs in the parent.

@rberger
Copy link

rberger commented Oct 12, 2022

Could make it a setting to enable it. We're adopting Just now and using this feature as fundamental. Had to create an alias just so everyone doesn't have to add --unstable

@casey
Copy link
Owner Author

casey commented Oct 13, 2022

Could make it a setting to enable it. We're adopting Just now and using this feature as fundamental. Had to create an alias just so everyone doesn't have to add --unstable

Great idea! I think that's better than stabilizing it as-is, since falling back to parent justfiles could be undesirable if you aren't expecting it. Also, if you plan on using fallback, you can omit the setting in the root justfile in your repository, and just will stop there instead of ascending to /.

I implemented it in this PR, have a look: #1368

This would make things worse for your use-case temporarily, since you would have to add set fallback to your justfiles, in addition to using --unstable, since I'd like to give it a month or so with the setting to get feedback. But, assuming that feedback is good, this could be stabilized the release after that:

  • day N: release version which requires set fallback and --unstable
  • day N + 30: stabilize

How does that sound?

@casey
Copy link
Owner Author

casey commented Oct 13, 2022

Forgot to @ @rberger

@heavelock
Copy link
Contributor

heavelock commented Oct 13, 2022

That's awesome idea in my opinion. We have a structure of monorepo where we have per-project justfile as well as a main, repository wide justfile and falling back even further up than the monorepo root is not desirable. Having it as a setting in each justfile solves that issue completely.

The stabilization path, although more annoying at start, is a thing that we just need to live with.

@casey
Copy link
Owner Author

casey commented Oct 15, 2022

@soenkehahn What do you think of this? You would have to put set fallback in justfiles where you wanted fallback, but it would feel much safer to stabilize, because the concern about getting unexpected behavior is ameliorated.

@casey
Copy link
Owner Author

casey commented Oct 20, 2022

Nobody complained, so I just released version 1.6.0, which has the new setting-controlled fallback behavior. It's still unstable, and barring any issues, I'll make it unstable in the next release in a month or so. Binaries should be available soon on the release page.

Try it out and let me know what you think!

@heavelock
Copy link
Contributor

heavelock commented Oct 20, 2022

Hey! Thanks for implementing that! One problem I stumbled on while implementing the switch.

❯ just --unstable build
error: Unknown setting `fallback`
  |
6 | set fallback := true
  |     ^^^^^^^^

The reason is obvious, I forgot to upgrade version of just on my machine. This leads to some complications however, I need to force everyone to upgrade their versions basically before I merge that commit. Otherwise, I will break workflow for literally everyone as well as for CI. The latter is obviously easier to deal with.

Is throwing an error an optimal solution for dealing with unknown settings in face of a super stability of just? It means that adoption of backward incompatible changes in justfiles is going to be very hard.

Wouldn't a warning and ignoring of unknown setting work out better? Obviously, it could lead to some unexpected behaviours but it wouldn't break stuff that heavily.

Edit:
Yeah, obviously the previous behaviour was unstable so I should not be surprise that this is a breaking change 🤦 I think, however, that introduction of a new setting should not be backward incompatible change.

@casey
Copy link
Owner Author

casey commented Oct 20, 2022

That's definitely annoying! However I don't think just can ignore a setting that it doesn't understand. A setting may do anything to the behavior of just, so running a justfile without some setting that the user intended it to be used with might do something undesirable, so I think seeing an unknown setting has to be a hard error.

In this case though, there is something that could be done to make this transaction less painful:

  • Release a version of just where fallaback defaults to true when --unstable is passed
  • Wait a while
  • Change the default to false, and have --unstable still be required
  • Wait a while
  • Stabilize the feature so that --unstable isn't required

This would allow you to wait for the everyone to update to the new version where true was the default, then add the setting, and then wait for the default to change, and for it to be stabilized. You would see a change in behavior when the default changed to false, but that would be minor.

However, this would be at the expense of having to wait a longer time for the feature to be stabilized, since I wouldn't want to stabilize it at the same time that the default changes.

How hard is it to ensure that everyone has upgraded to the new version of just? Also if anyone else is having this problem, or would have this problem, definitely chime in.

As you said, this is an unstable feature, so breaking changes are to be expected. However, there's no reason not to try to minimize how annoying they are.

@heavelock
Copy link
Contributor

I think it won't be that hard to ensure everyone to use just. The only prerequisite is to be sure that the new version is available via brew so people have a familiar way of obtaining it.

I think your proposed solution will work, definitely it will ease the transition. It would be great if you could walk this release path.

And regarding ignoring unknown settings - how about introducing a switch that would allow for unknown settings to be present in a justfile? Kind of a double edged sword but could help with making transition smoother.

@casey
Copy link
Owner Author

casey commented Oct 27, 2022

I think your proposed solution will work, definitely it will ease the transition. It would be great if you could walk this release path.

Done! I just released 1.7.0, which changes the new fallback setting to default to true. Binaries should be on the releases page soon.

Check the updated timeline above, and let me know if it's too fast or too slow.

@heavelock
Copy link
Contributor

Oh wow, that was fast! I think a month for each phase is plenty of time. Thank you!

@casey
Copy link
Owner Author

casey commented Nov 25, 2022

I just released 1.9.0, which changes the fallback setting default to false. This should be the last step towards stabilizing this feature, i.e. making it work without the --unstable flag. Give it a shot! Barring any issues, I'll release a new version that stabilizes the feature around January 1st, 2023.

@nyonson
Copy link

nyonson commented Jan 2, 2023

@casey did this stabilize in to 1.10.0 by any chance? Just gave it a shot and I think it still has 1.9.0 behavior.

@casey
Copy link
Owner Author

casey commented Jan 4, 2023

@nyonson Nope, I didn't wasn't able to get it in, but just has no official release schedule, so I'll release 1.11 within a few days that stabilizes fallback.

@casey
Copy link
Owner Author

casey commented Jan 4, 2023

It is done! I just published 1.11.0, with fallback stabilized. Prebuilt binaries should be up on the releases page soon. Thanks for waiting and providing feedback!

@casey casey closed this as completed Jan 4, 2023
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

7 participants