casey / just

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

Deal with list substitution #208

Open casey opened 7 years ago

casey commented 7 years ago

Currently, a + argument is a string consisting of the original arguments separated by spaces.

This presents difficulties, since the shell will re-split the text. If the argument contains intra-argument spaces, bad things can happen.

For example, with the following justfile:

compile flags+:
  cc {{flags}}

Invoking just with:

just compile -DVAR='hello, world'

Will run the command:

cc -DVAR=hello, world

Since the single quotes were evaluated in the calling shell, the command line will be parsed by the subshell invoked by just as cc, -DVAR=hello,, and world, which is clearly not what was intended.

So, given this problem, here's what I'm thinking of for a solution:

I'll add functions, with the first one being a quote() function. Quote may be applied to any variable, like so: quote(variable). In a recipe, one would use it like this:

compile flags+:
  cc {{quote(flags)}}

It will surround each element of a list with quotes, escaping existing quotes and backslashes as necessary.

I haven't decided on whether to use quotes or double quotes. I think it mostly depends on which has the most consistent behavior between dash and bash.

This complicates the type system a little bit, since we now must distinguish between lists and strings. I propose that all variables are now lists of strings, and so existing variables are now single-element lists. So, the quote function can be applied to all variables.

casey commented 7 years ago

@ssokolow, we discussed this a bit in #114

What do you think?

ssokolow commented 7 years ago

My main concern is that it feels like it might be a case of getting the wrong result from all of the right parts... possibly for unavoidable reasons.

  1. The need to opt into quoting is in line with bourne-style shell semantics.
  2. Functions like quote() are in line with Django/Jinja/Twig/etc.-style templating syntax. (though, typically as filters like {{flags|quote}})
  3. There's a limit to how far you want to drift from Bourne semantics for familiarity's sake.
  4. You do want to retain backwards compatibility with existing Justfiles.

Despite all that, I have this nagging feeling that it's Bourne shell's biggest footgun, writ more verbose.

(In my dash and bash scripts, I end up writing constructs like cd "$(dirname "$(readlink -f "$0")")" because splitting should be opt in, rather than opt out. That's actually one of the biggest reasons I mostly use Python for my command scripting these days and never write Bourne without Syntastic and shellcheck installed.)

If it weren't for the backwards compatibility issue, I'd say to make variables like auto-quoted and use the function to opt into unquoted behaviour.

casey commented 7 years ago

I agree that it's an ugly solution, but with the Bourne shell's terrible splitting semantics, it feels like it's the best worst option.

Auto-quoting variables is interesting, but requiring users to opt-out of quoting feels weird. I guess it would be an unquote() function? quote() feels clearer, since you take a list of strings as input and you get a list of strings out, but with the quotes added. unquote() would be more of a type system mechanism thing, where you take a list of strings-that-are-destined-to-be-quoted in and get a list of strings-that-are-not-destined-to-be-quoted out, which seems harder to explain and understand.

Alternatively, we can build a time machine, go back to 1989 and pick up Tom Duff – right after he wrote the rc shell, which has much better splitting and quoting behavior – and then head to 1977 so he can have a chat with Stephen Bourne and we can avoid this whole mess.

ssokolow commented 7 years ago

I'd think of it more in terms of split() (ie. all variables are lists which get serialized to strings using quotes but most have length 1) with split() splitting up each list member using Bourne quoting/escaping semantics.

Then, you'd get this intuitive-for-anything-but-bash behaviour: my_command {{split(args)}} {{infile_containing_spaces}} {{outfile_containing_spaces}}

(You'd want to split using something like the shlex crate since some commands have options which take a quoted argv string that they'll parse or shell() themselves.)

ssokolow commented 7 years ago

...and I'd argue that, since {{variable_name}} already isn't Bourne syntax, it'd be reasonable to give it different quoting behaviour. It's not as if you have to redefine how quoting works for $variable_name.

Heck, I actually found it confusing and UN-intuitive when something which was obviously a pre-processing step separate from the Bourne shell evaluation with syntax inspired by an HTML templating engine which auto-escapes didn't auto-escape in this context.

valscion commented 5 years ago

Hi there :relaxed: I'm wondering is there any way to currently work around this lack of functionality in just?

We have more than 50 just tasks currently for various unrelated tasks, so we have split them up to multiple directories to simulate "namespaces". We've got a Justfile in the root of our application that looks something like this:

help:
  @just -l

admin +args='help':
  @just --working-directory `pwd` --justfile tasks/admin/Justfile {{ args }}

aws +args='help':
  @just --working-directory `pwd` --justfile tasks/aws/Justfile {{ args }}

chores +args='help':
  @just --working-directory `pwd` --justfile tasks/chores/Justfile {{ args }}

So we end up going back to just with the given variadic arguments.

This all works nicely up until we have some script that wants arguments to be passed like --something="foo bar". We've yet to find a nice way to keep the spaces in the --something in place up until the final execution, as the quotes for --something are "lost" at the first step of this process.

casey commented 5 years ago

Hi Vesa! What you're doing is really cool, it would be nice to support subcommands natively. Unfortunately I don't think there's a good way to work around this right now. I don't know of a solution which would work consistently for different use cases and shells. I'm definitely open to suggestions, but I don't have a lot of bandwidth to work on just right now, so it's been sadly languishing.

valscion commented 5 years ago

Thanks for getting back to me so soon 😊. just is already pretty amazing and it works splendidly for 98% of our use cases.

Issues with quoting are definitely one of the biggest annoyances with shells and difficult to solve. I didn't expect you to have any solution but hey, it was worth a try 😃

Thanks for this tool, it has been really helpful in automating our mundane admin and chore tasks ☺️

valscion commented 5 years ago

Ok, now I think I'm in a happy place. I can write commands like this:

just admin fix-venue-location staging --where-venue-like="'Food Camp / %'" --lat=60.1817 --lon=24.522

We'll merely have to use both single and double quotes for arguments containing spaces. That's completely reasonable, as there's (as far as I understand) no way for just to understand the importance of these quotes.

I only need to use this kind of syntax in various places when I'm going deeper in to other tasks. Yes, it's additional glue code and I wish I didn't need to write that, but it will suffice.

# in root Justfile
admin +args='help':
  #!/usr/bin/env bash
  set -eo pipefail
  args_as_is=$(cat <<EOF;
  {{args}}
  EOF)
  just --working-directory `pwd` --justfile tasks/admin/Justfile $args_as_is

# in tasks/admin/Justfile
fix-venue-location env='staging' +ARGS='--usage':
  #!/usr/bin/env bash
  set -eo pipefail
  args_as_is=$(cat <<EOF;
  {{ARGS}}
  EOF)
  just admin _run-ruby-script-using-confirmation {{env}} fix_venue_location $args_as_is

_run-ruby-script-using-confirmation env script_name +ARGS:
  #!/usr/bin/env bash
  set -eo pipefail
  args_as_is=$(cat <<EOF;
  {{ARGS}}
  EOF)
  # ...and so on
casey commented 5 years ago

It's definitely not ideal, but I'm glad that you were able to find a workaround.

I just opened #383, which I hope eventually leads to just natively supporting submodules and subcommands, which I think would be really cool

kwshi commented 3 years ago

My two cents:

  1. I like the quote() approach the most (speaking of which, it can/should also be applicable to plain string arguments).

    • I don't think quoting-by-default/requiring opt-out of quoting is a particularly great idea, because I think that behavior is too "magical"/implicit and can cause even more unexpected issues depending on where it is used (e.g., what if they were trying to get the $* behavior and already trying to substitute it into quotes? Then automatic quoting would unexpectedly introduce additional quotes canceling the existing ones, fudging up the syntax).

      I believe that keeping the default behavior the explicit, straightforward, and dumb is in line with the philosophy of avoiding magic-induced, Make-esque idiosyncrasies.

  2. In the absence of this feature, there is a partial workaround, I think: set positional-arguments would allow a single list to be passed to the recipe w/o escaping issues (since they are passed directly as $1, $2, etc.).

casey commented 3 years ago

@kwshi I agree with your thoughts on quote(). One thing that's stopped me from adding quote() is would ideally be a function from List<String> → List<String>, and Just currently only has values of type string. (Variadic arguments are just joined with spaces into a single string when being evaluated and inserted into a recipe's scope.)

Ideally, Just would have a static type system, so quote() could distinguish between items of type String and List<String>. But, a static type system is quite a large undertaking, and I've been too lazy to actually do it.

Also, @valscion, there's a new feature which you might find useful for passing arguments that would otherwise need to be quoted. (@kwshi, thanks for reminding me!)

If you add set positional-arguments to a justfile, recipe arguments will be made available as positional arguments.

For example:

set positional-arguments

variadic +arg:
  echo $@ # values of `arg`

single arg:
  echo $1 # value of `arg`

The shell won't interpret quotes, spaces, or other special characters inside of positional argument values, and will pass them to sub-commands unmmolested.

Another thing you can do is add a $ in front of an argument, which will cause it to be made available as an environment variable, with much the same effect:

single $arg:
  echo $arg # value of `arg`

I think that this might help with your use cases.

valscion commented 3 years ago

Cool, thanks for letting me know of these new features! I'll keep them in mind the next time I'm figuring out our Justfile usages ☺️