-
Notifications
You must be signed in to change notification settings - Fork 121
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
Support variable substitution with toml lists in the same way "${@}" is supported #202
Comments
so just to be clear, you are looking to replace 1 arg with multiple args, for example [tasks.something]
command = "somecmd"
args = ["${OPTIONS}"] and if OPTIONS is [tasks.something]
command = "somecmd"
args = ["a", "b", "c"] if so, ya it would be very powerful, but also a bit risky and flaky. risky, because unlike ${@} the user expects it to be a single value, but with expansion something else happens which kinda reminds me of sql injection. which is why command arguments in most runtimes (for example: rust, java, node.js, python) do not automatically expand (i'm actually not sure what runtime does automatically expand command arguments when provided as a list and not a single string). flaky because cargo-make is not bash and doesn't try to be and putting too much bash like behavior makes it do unexpected things. maybe instead (thinking out loud), apart of the env block which defines env vars, i'll have some vars block which defines vars which only cargo-make knows. [vars]
options = ["a", "b", "c"] and use those in command arguments (not scripts) as follows: [tasks.something]
command = "somecmd"
args = ["%{options}"] Than i'll auto expand it if the options var is a list. what do you think? |
I have no problem with that. As long as there's some kind of list variable that I can define in one place and then expand into multiple arguments in several other places (including composite variables), it'll meet my needs. Though, now that |
decided to go a different path a bit. so functions will have a @@FUNCTION_NAME(ARG1,ARG2,ARG3,...) syntax. [env]
MULTIPLE_VALUES="1|2|3|4"
[tasks.split]
command = "echo"
args = ["@@split(MULTIPLE_VALUES,|)"] in runtime it would look like the following: [cargo-make] INFO - Running Task: split
[cargo-make] INFO - Execute Command: "echo" "1" "2" "3" "4"
1 2 3 4 so env vars with all their capabilities in support will remain and functions will only come to expand on that to workaround their limitations. you can already test that in the 0.17.0 development branch. |
With this and #201, you've been busy. :) I'll try to test both either today or tomorrow. |
I've started poking at it and I just ran across another problem that I hadn't anticipated. I'm porting the justfile from rust-cli-boilerplate as a test, since it makes use of this feature, and I hit a brick wall: Practically every cargo subcommand I run in the justfile begins its args with either (ie. The README doesn't seem to mention any (And I think you can see why it would be... sub-optimal for me to work around that by making every single task a |
you mean this |
Never mind. No clue why, but That said, I did find a legitimate problem. Attempting to use space as a separator (
Supporting space is important because people will expect to be able to use variables like this, as they do in shell scripts and GNU make:
|
Yeah. I thought you called it "channel" because of EDIT: OK. I'm using |
To clarify, the reason supporting space as a separator is important:
|
Will support it. |
fixed |
|
as for your use case, try:
Try to filter such remarks. |
Good idea. Clearly, the late hour is blinding me to obvious directions to search for solutions.
I certainly don't want cargo-make to turn into a full-blown scripting language. The only reason I wrote all that is that I'd just realized right then and there why my intuition kept calling for an array type, and I was "getting that realization onto paper before I forgot".
I meant no malice. It's just an observation. It's a simple fact that it is an inherent shortcoming/weakness/flaw in any design which readily discards and then re-infers tokenization information from strings... like Bourne shell script.
Agreed. My point was that, in hindsight, I should have advocated more vigorously for more support for arrays.
No, I'd just like a nice way, with minimal duplication, to allow users to feed custom arguments to the various cargo invocations when some of them have different requirements for which subsets of the arguments can be used. Any "I want bash scripting"-isms are just an artifact of my still getting used to cargo-make as I work to port my old justfile to it. For example, if there were an environment variable Cargo would accept in place of
No argument there.
I have no idea what you mean here. The only thing I "hate" about cargo-make is that, at times, the verbosity involved in describing complex tasks makes it feel a bit like WiX and I worry that, to feel comfortable, I might have to write a pre-processor for generating
Again, agreed. My only issue with the predefined tasks is that they embody a workflow very different from mine. (And, as a UI/UX guy, I have strong opinions on task naming.) That's why I want things like |
I just realized why Note the escaped quotes In the original
That is, if It's not a deal-breaker, but it does mean I'll be duplicating that bit of boilerplate across practically every task block I define. (Another reason for me to consider whipping up a quick little PyTOML script to act as a preprocessor for that, as well as converting blocks of shellscript-like markup into groups of |
ok, so workaround is args = [ "--features=\"${features}\"", "@@split(build_flags, )"] meaning split both of them to 2 args instead of 1 arg? |
The workaround is [tasks.bloat]
# ...
args = ["bloat", "--features=${features}", "@@split(build_flags, )", "${@}"]
[tasks.build]
# ...
args = ["build", "--features=${features}", "@@split(build_flags, )", "${@}"]
[tasks.check]
# ...
args = ["check", "--features=${features}", "@@split(build_flags, )", "${@}"]
[tasks.doc]
# ...
script = ['''
cargo doc --document-private-items --features=${features} @@split(_build_flags, ) ${@}
cargo deadlinks --dir target/${CARGO_BUILD_TARGET}/doc/${CARGO_MAKE_CRATE_NAME}
''']
# etc. etc. etc. (NOTE: I know the quoting on that |
in that case you can have a template task that first arg is some env var. |
Good idea. When you think about it, it's kind of funny that I'm the one that's trying to move away from shell script-esque build automation, yet you're the one who keeps realizing when tricks that feel very shell script-inspired (eg. using a variable for the subcommand name rather than its arguments) are viable solutions. |
i always thought of how to make tasks as reusable functions. i do something similar with do-on-members. |
I slept terribly today and my schedule was a mess, so I'm going to have to finish porting and testing my justfile tasks tomorrow instead. Sorry about that. |
no problem |
OK. I haven't finished testing yet, but here are the things I've tripped over so far:
I'll keep porting and testing and let you know what else I find. |
In light of what I discovered for how At the moment, I just maintain two private tasks that are identical except for the [tasks._simple_cargo_command]
category = "Development"
command = "cargo"
args = ["${cargo_subcommand}", "--features=${features}", "@@split(build_flags, )", "${@}"]
private = true
# Like `_simple_cargo_command` but it includes "--" so you don't need to
# type "makers foo -- -- ...` to feed in flags.
[tasks._simple_cargo_command_with_inner]
category = "Development"
command = "cargo"
args = ["${cargo_subcommand}", "--features=${features}", "@@split(build_flags, )", "--", "${@}"]
private = true Also, that Am I running an outdated build or is that still unimplemented in the release candidate? Finally, in case you were unaware, rustfmt's handling of [tasks.fmt-check]
description = "Alias for `cargo +nightly fmt -- --check [args]` which un-bloats TODO/FIXME warnings"
category = "Development"
script_runner = "@shell"
script = ['''
cargo +nightly fmt -- --check --color always ${@} 2>&1 | egrep -v '[0-9]*[ ]*\|'
'''] Still not done porting and testing. |
I don't know if you have a more idiomatic way to collapse away duplication with commands that expect their output to be piped, but I was drawing a blank. # TODO: Regression test that features and build flags got passed in properly
[tasks.dist-supplemental]
description = "Build the shell completions and a manpage, and put them in `dist/`"
category = "Release Builds"
script_runner = "sh"
script = ['''
mkdir -p dist
# Generate completions and store them in dist/
for shell in bash zsh fish elvish powershell; do
cargo run --features="$features" $build_flags -- --dump-completions $shell > dist/$CARGO_MAKE_CRATE_NAME.$shell
done
# Generate manpage and store it gzipped in dist/
# (This comes last so the earlier calls to `cargo run` will get the compiler warnings out)
help2man -N 'cargo +$RUSTUP_TOOLCHAIN run -q --features="$features" $build_flags --' | gzip -9 > dist/$CARGO_MAKE_CRATE_NAME.1.gz
'''] Also, I was reminded of a bug I seem to have discovered in This task, or its justfile equivalent, causes # TODO: Check what of this cargo-make will install automatically
[tasks.install-rustup-deps]
description = "Install (don't update) nightly and `channel` toolchains, plus `CARGO_BUILD_TARGET`, clippy, and rustfmt"
category = "Dependencies"
script_runner = "sh"
script = ['''
# Prevent this from gleefully doing an unwanted "rustup update"
rustup toolchain list | grep -q "$RUSTUP_TOOLCHAIN" || rustup toolchain install "$RUSTUP_TOOLCHAIN" || true
rustup toolchain list | grep -q nightly || rustup toolchain install nightly || true
rustup target list | grep -q "$CARGO_BUILD_TARGET (" || rustup target add "$CARGO_BUILD_TARGET" || true
rustup component list | grep -q 'clippy-\S* (' || rustup component add clippy || true
rustup component list --toolchain nightly | grep -q 'rustfmt-\S* (' || rustup component add rustfmt --toolchain nightly || true
echo "DONE"
'''] That's the end of the hand-testing but I won't consider it truly ready for me to move over from (Speaking of which, tests for the the various dependency-installation tasks will be interesting. I may need to add Vagrant into my test setup so I can start each test with a VM that's missing the cargo subcommands and then roll it back after the test completes.) |
Makes sense but its expected from using 'extend'. another behavior would be confusing.
verbose would allow you to see the scripts content before invocation. it prints out a LOT of stuff, but since its once every now and than, its not a big deal.
I explained about it a lot in the docs. think about having task A and task B depend on C.
Ya that is because old executables are there. i'm open for suggestions how to clean those beforehand.
Can you print that out? I don't have such messages so i'm not 100% sure what you mean.
that would never work on windows unless you have bash available.
you sure you need this one? i do it for you in the clippy task Generally speaking, you have some really complex script based tasks there. |
It's clear that I'm misunderstanding something. Every time I tried to use Would you mind sharing an example?
Huh. I thought I tried Still, I'd much prefer something that leaves the temporary file intact so I can easily open it in Vim to see what my Syntastic-integrated copy of (It'd be even better if cargo make included
You misunderstand. My intuition interpreted Here are the problematic tasks. [tasks._clean_kcov]
private = true
command = "cargo"
env = { "CARGO_TARGET_DIR" = "target/kcov" }
args = ['clean', "-v"]
# BUG: This breaks because _clean_kcov's CARGO_TARGET_DIR gets applied to it
[tasks.clean]
description = "Superset of `cargo clean -v` which deletes other stuff this makefile builds"
category = "Development"
script_runner = "@shell"
script = ['''
cargo clean -v "${@}"
rm -rf target/doc/
rm -rf dist/
''']
dependencies = ["_clean_kcov"]
I just woke up, so I'll reply to this once I'm more alert.
Sure... after I've had breakfast.
Squashing down each report of a TODO or FIXME in the code as close to rustfmt's old single-line format for them of them as possible. That's basically a literal port of the justfile, so, once I've got feature parity with the cargo-make version, I'll probably use
Note the comment.
A lot of these are direct ports of the justfile tasks that still need to be reworked to take advantage of cargo-make's features and predefined tasks. The important goal was reaching feature parity with the justfile first (and the justfile doesn't work on Windows) and only then refactoring it further to make things idiomatic and portable in places where
Some of them will have to be gated to Linux/Mac because they depend on tools like Valgrind and KCachegrind which don't run on Windows. Given that you don't have any kind of "lint for staying within what |
The approach I use is adapted from these two sources and, while I do also set a custom kcov_path="$CARGO_TARGET_DIR/html"
for file in "$CARGO_TARGET_DIR"/"$CARGO_BUILD_TARGET"/debug/$CARGO_MAKE_CRATE_NAME-*; do
rm -f "$file"
done
cargo test --no-run || exit $?
rm -rf "$kcov_path" (With |
In fact, here's the whole script in case I've stumbled upon edge cases that need to be handled which you missed: #!/bin/sh
# Adapted from:
# - http://sunjay.ca/2016/07/25/rust-code-coverage
# - https://users.rust-lang.org/t/tutorial-how-to-collect-test-coverages-for-rust-project/650
#
# As of July 2, 2016, there is no option to make rustdoc generate a runnable
# test executable. That means that documentation tests will not show in your
# coverage data. If you discover a way to run the doctest executable with kcov,
# please open an Issue and we will add that to these instructions.
# -- https://github.com/codecov/example-rust
# Ensure that kcov can see totally unused functions without clobbering regular builds
# Adapted from:
# - http://stackoverflow.com/a/38371687/435253
# - https://gist.github.com/dikaiosune/07177baf5cea76c27783efa55e99da89
export CARGO_TARGET_DIR="target/kcov"
export RUSTFLAGS='-C link-dead-code'
kcov_path="$CARGO_TARGET_DIR/html"
for file in "$CARGO_TARGET_DIR"/"$CARGO_BUILD_TARGET"/debug/$CARGO_MAKE_CRATE_NAME-*; do
rm -f "$file"
done
if [ "$#" -gt 0 ]; then shift; fi # workaround for "can\'t shift that many" being fatal in dash
cargo test --no-run || exit $?
rm -rf "$kcov_path"
for file in "$CARGO_TARGET_DIR"/"$CARGO_BUILD_TARGET"/debug/$CARGO_MAKE_CRATE_NAME-*; do
if [ -x "$file" ]; then
outpath="$kcov_path/$(basename "$file")"
mkdir -p "$outpath"
kcov --exclude-pattern=/.cargo,/usr/lib --verify "$outpath" "$file" "$@"
elif echo "$file" | grep -F -e '-*'; then
echo "No build files found for coverage!"
exit 1
fi
done (The escaped apostrophe in the comment is just to work around a bug in gVim's TOML highlighter.) |
Bah. Now I can't reproduce it anymore. With this args line...
...I just get this more reasonable-to-expect output:
(That said, |
implemented in 0.17.0 |
Features Description
One Makefile-ism that I like to use is variables which contain the common options shared between multiple different invocations of a command, to ensure they remain consistent.
(eg. So I can retrofit a
CARGOFLAGS
onto Cargo to complementRUSTFLAGS
.)However, cargo-make lacking the shell's loose relationship with list-like behaviour rules out the old standby of a space-delimited string and cargo-make panics if you attempt to put something like
options = ["--foo", "--bar"]
in the[env]
section.(And the
@shell
runner seems to be attempting some kind of weird, broken quoting on my experiments in feeding it shell-quoted strings.)It'd be helpful to have an alternative to crafting tasks like this, which bring in a whole separate language just to split a shell-quoted string for want of list-type variables:
In fact, there's actually a case in the default Makefile.toml which doesn't need this, but which would have benefitted from the mindset that produced it. There are quite a few repetitions of
--all-features
that would be more useful if replaced with something like "${features_opt}" so it'd be easy to replace with nothing or--features="whatever I want"
.(That's actually one of the things contributing to my decision that it's easier to
skip_core_tasks
. I don't want--all-features
and I want users of my template to be able to change the set of enabled features for a single run by invoking with--env
.)Describe the solution you'd like
Support some kind of list variable which can be used the same way
${@}
is.(It'd also be nice to support temporarily overriding them using
--env
but I haven't had time to think about the pros and cons of various approaches to implementing that yet.)The text was updated successfully, but these errors were encountered: