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 passing arguments to dependencies #555

Merged
merged 13 commits into from
Dec 7, 2019
Merged

Allow passing arguments to dependencies #555

merged 13 commits into from
Dec 7, 2019

Conversation

casey
Copy link
Owner

@casey casey commented Dec 3, 2019

I've started working on passing arguments to dependencies, see #289, a longstanding feature request.

Open questions to resolve:

Syntax

    1. Lisp style:
bar name options:
  ./build {{name}} {{options}}

foo: (bar "arg" "baz")
    1. Fortran style:
bar name:
  ./build {{name}}

foo: bar("arg", "baz")

Lisp can be pretty incomprehensible, for example:

foo: (bar 'a' + 'b' 'c' + 'd')

As opposed to:

foo: bar('a' + 'b', 'c' + 'd')

However, I think in some sense lisp style is more natural, since recipe invocation on the command line doesn't require parenthesis or commas. At the moment I'm leaning towards lisp style.

Repeat dependency invocations

Just, like Make, will only run a recipe once per invocation, even if it appears multiple times in a dependency tree. For example, given the following, just bob will run bar only once, even though foo and baz both depend on it:

bar:

foo: bar

baz: bar

bob: foo baz

However, the situation is complicated by arguments. Consider:

build target:
  ./build {{target}}

release: (build "foo") (build "bar")
  ./release

Clear, the user intends to run build twice, once for "foo" and once for "bar".

I think a reasonable approach would be to run recipes once per recipe and combination of arguments. So given the following justfile, just release would run build "foo", build "bar" and build "baz":

build target:
  ./build {{target}}

build-foo: (build "foo") (build "bar")

build-baz: (build "baz") (build "bar")

release: build-foo build-baz

I can't immediately think of any counter-examples where this behavior wouldn't be desirable, so barring any counter examples, this is what I'll implement.

To Do

  • error on undefined variables in arguments
  • allow recipes with parameters in dependencies
  • error if wrong number of arguments are passed to dependency
  • test assignments can be used in arguments
  • test parameters can be used in arguments
  • test functions can be used in arguments
  • test backticks can be used in arguments
  • dependency memoization is correct (recipe with same args run once, different args run multiple times)
  • update readme

@clarfonthey
Copy link

I also think the lisp style is the way to go.

@runeimp
Copy link

runeimp commented Dec 5, 2019

I'm good with either but I'd say the "lowest common denominator" might be the Fortran style. Most languages I've dealt with handle functions in the same manner that you have Fortran listed. Were as the LISP style is similar to calling functions in a subshell. So if your coming from anything other than LISP (and Scheme?) development or shell scripting the Fortran style will be more natural.

@casey casey mentioned this pull request Dec 7, 2019
@casey casey marked this pull request as ready for review December 7, 2019 11:10
@casey casey merged commit 0931fa8 into master Dec 7, 2019
@casey casey deleted the dependency-arguments branch December 7, 2019 12:03
@casey
Copy link
Owner Author

casey commented Dec 7, 2019

@clarfon and @runeimp, thanks for the feedback! I decided to go with the lisp-style, just because it felt more natural, and to make it visually distinct from function invocations.

As for duplicate recipe invocations, Just will run every combination of recipe and arguments at most once.

For example in:

foo: (bar "A") (bar "A")
bar x:

just foo will run bar once with the argument "A".

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

Successfully merging this pull request may close these issues.

3 participants