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

Allow dependencies that take parameters #289

Closed
fuine opened this issue Dec 15, 2017 · 18 comments
Closed

Allow dependencies that take parameters #289

fuine opened this issue Dec 15, 2017 · 18 comments

Comments

@fuine
Copy link

fuine commented Dec 15, 2017

I'm not sure if this is possible, but consider the following case:

run stuff:
    cd {{stuff}} && python main.py

visualize stuff:
    python src/visualize.py {{stuff}}

run_visualize stuff:
    just run {{stuffl}}
    just visualize {{stuff}}

Is there a way to create run_visualize recipe without subsequent executions of just from the main just process?

@casey
Copy link
Owner

casey commented Dec 15, 2017

Is there a good default value you could give to stuff? If so, you could turn it into a variable:

stuff = "foo"

run:
    cd {{stuff}} && python main.py

visualize:
    python src/visualize.py {{stuff}}

run_visualize: run visualize

You could then override it on the command line like so:

just stuff=bar run

@fuine
Copy link
Author

fuine commented Dec 15, 2017

I'm afraid I can't, that's the whole problem, I thought about using the default value but I want to make sure that this particular value is always passed by user

@casey
Copy link
Owner

casey commented Dec 16, 2017

Hmm, darn, this is a tricky one.

The only solution I can think of is one which would be pretty weird. Basically to allow dependencies that take the exact same number of arguments, and pass the arguments of the dependee through, which would allow:

run stuff:
    cd {{stuff}} && python main.py

visualize stuff:
    python src/visualize.py {{stuff}}

run_visualize stuff: run visualize

This would be preeeeetty magical though. An additional constraint could be that they must have the same name, which would mean that the above would still work, but the following wouldn't:

run bar:
    cd {{bar}} && python main.py

visualize foo:
    python src/visualize.py {{foo}}

run_visualize stuff: run visualize

Since foo != stuff and bar != stuff.

I can't decide whether this is super weird or not.

What do you think? Also, anyone else, would you use this or has this been an issue before?

@casey casey changed the title Invoke recipes from other recipes Allow dependencies that take parameters Dec 16, 2017
@fuine
Copy link
Author

fuine commented Dec 16, 2017

I'm not really sure this is a good idea, because that would break backward compatibility, consider

foo arg: bar baz

what if bar and baz don't take any arguments? I think a better option would be to allow the user to provide the arguments somehow so maybe something like

foo arg1 arg2: (bar arg1 arg2) (baz arg2)

What do you think?

@fuine
Copy link
Author

fuine commented Dec 16, 2017

When I think about it now, maybe it's a better idea to introduce something like that within the body of the recipe? I'd think that a lot of the times you want to run the recipes somewhere in the middle of different operations like so:

foo arg1 arg2:
    # some bash commands here
    bar arg1 arg2  # run recipe bar here
    # more bash commands
    baz arg2 # run another recipe

That way you don't need to change the dependencies syntax, and if user wants dependencies to take arguments then he'd need to call them by hand from within the body of the recipe. But now there should be a standardized way to mark a line in the body that it starts a recipe, maybe similar to how @ sygil works?

@casey
Copy link
Owner

casey commented Dec 17, 2017

If bar and baz took no arguments, then they just wouldn't get passed any, so it would be okay for backwards compatibility. foo arg1: (bar arg1) would definitely be better for readability, and would be less magical.

Going the sigil route, I think it would be better to have a sigil that meant the opposite, that this line wasn't a normal recipe line, and was a call to a dependency or set a variable.

Still not sure about either route, since recursively calling just is a viable solution.

@fuine
Copy link
Author

fuine commented Dec 17, 2017

Oh I meant the sygil to be working in the way you described it - something to describe a non-normal line of recipe

@casey
Copy link
Owner

casey commented Dec 19, 2017

Thinking about this a bit; I think that introducing special lines within recipes would be a big step*, so the solution I like right now is your version of parameter passing, using parenthesis for grouping:

foo arg1 arg2: (bar arg1 arg2) (baz arg2)

I'm not sure when I'll be able to get to it though, I'm working on a parser-generator, which is taking all my attention at the moment.

* Never say never though! This would also allow intra-recipe assignments, which have been requested before.

@casey
Copy link
Owner

casey commented Feb 7, 2018

I started to implement this, but I ran into an issue. Check out the following:

foo a: (baz a)

bar a: (baz a)

baz a:
  echo {{a}}

bob a b: (foo a) (bar b)

If we run just bob x y, then baz will be invoked with both x and y as arguments from foo and bar recipes. Hmmm, I suppose this could be statically detected though.

@fuine
Copy link
Author

fuine commented Feb 7, 2018

Is this bad? It looks like that's exactly what should happen...

@casey
Copy link
Owner

casey commented Feb 7, 2018

I'm not sure if it should be allowed. Up until now, any recipe would only ever run once in any particular run.

There are two options here, either treat those two instances of baz that are called with two different arguments as different recipes and run baz twice, or treat them as two conflicting calls to the same recipe, and disallow it.

My initial reaction was that I should disallow it, mostly because that's my usual strategy when extending just. It's usually better to disallow too much and then extend what's allowed, as opposed to allow too much but then disallow what was once allowed. Both to avoid adding weird misfeatures, but also since the former kind of changes are backwards compatible, whereas the latter aren't.

However, it might be reasonable to treat them as separate recipe invocations, but only run them once if the arguments match. Can you think of a case where this would be useful? I'll try to come up with one tomorrow, but it's bedtime 😪

@casey casey added this to the soon milestone Apr 16, 2019
@clarfonthey
Copy link

I also ran into this issue-- one other consideration for this is how variadic parameters are sent to dependencies as well. My vote: test +version: build +version means that test 1.0 1.1 1.2 would have dependency build 1.0 1.1 1.2 whereas test +version: build version would have dependencies build 1.0, build 1.1, and build 1.2.

@casey casey modified the milestones: soon, eventually May 27, 2019
@casey casey modified the milestones: eventually, 1.0 Jun 22, 2019
@tpoliaw
Copy link

tpoliaw commented Sep 13, 2019

I have just run into a similar issue where a dependency requires the argument. I think @fuine's suggestion of having a way of running recipes from within other recipes would be a suitable alternative. In my case I need something like

# several recipes require the same preparation
prepare xyz:
    # do something with xyz

build target:
    # do stuff
    prepare {{target}}_output
    # continue doing stuff

The current restriction on dependencies not having arguments could stay as it is which would prevent recipes being run twice with different arguments (unless explicitly called).

Could maybe introduce a new syntax to prevent recipes overriding existing commands. Something like & that is never valid bash. eg

build target:
    &prepare {{target}}_output

@casey
Copy link
Owner

casey commented Sep 14, 2019

@tpoliaw One possibility to work around this would be to invoke just recursively, i.e.:

build target:
    # do stuff
    just prepare {{target}}_output
    # continue doing stuff

It feels a little unsatisfying, but it should work.

I'd like to keep just shell agnostic, so I'm hesitant to add syntax which might conflict with commands in non-bash shell languages. One way to do it, although it's very weird looking, would be to put the marker for an intra-recipe invocation somewhere in the leading whitespace for the line, like so:

build target:
    # do stuff
:   prepare {{target}}_output
    # continue doing stuff

Since a : can't start a regular justfile line, and it's not part of the line text, the parser can figure out what's going on, and it can't conflict with any shell's syntax. I used : since it's the character that introduces recipe dependencies, so it feels appropriate.

We'd have to figure how/if these intra recipe calls would participate in dependency resolution. I.e. are transitive dependencies of intra-recipe invocations re-run every time they're invoked?

@tpoliaw
Copy link

tpoliaw commented Sep 16, 2019

That's what I've been doing so far but it falls over if the justfile is not in the current directory tree. eg running just --justfile ~/.justfile deploy outside of your home directory will fail to find the correct recipes when just is run recursively.

I like the idea of an opening : although it'd break any syntax highlighting that relies on it being similar to make. Would an opening & be more likely to clash with a posix shell than the @ used for silent commands?

@casey
Copy link
Owner

casey commented Sep 17, 2019

@tpoliaw, ahh, gotcha that's unfortunate.

It's not so much that & is more likely to clash with @, it's that I think it was a mistake for me to support @ in the first place. I'd like it to be possible to use any kind of shell or programming language with Just, and any kind of syntax (like @ or &) that is part of the recipe lines can potentially conflict with the syntax of the recipe language.

@casey casey modified the milestones: 1.0, eventually Nov 7, 2019
@casey
Copy link
Owner

casey commented Dec 3, 2019

I've started working on this feature in #555. I have a couple of open questions around syntax and symantics, so do check the draft PR out and and let me know how you feel either way!

@casey casey modified the milestones: eventually, soon Dec 3, 2019
@casey
Copy link
Owner

casey commented Dec 7, 2019

This has been implemented and included in v0.5.2, which I just published to crates.io. Releases should be available soon here. (@clarfon I opened #558 to track expanding dependencies w/variadic arguments, which I quite like, although it'll be tricky to implement.)

And thank you for your patience with this now two-year old feature request :)

@casey casey closed this as completed Dec 7, 2019
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