Add cmd output type

[pditommaso]

Implements the support for cmd output type to collect task tool version and metadata as described here #4386 (comment)

POC usage:

process foo {
  output:
  cmd 'git --version', emit: gitVersion
  
  '''
  echo Ciao
  '''
}

workflow {
  foo()
  foo.out.gitVersion | view
}

[netlify]

✅ Deploy Preview for nextflow-docs-staging canceled.

Name	Link
🔨 Latest commit	c4c6182
🔍 Latest deploy log	https://app.netlify.com/sites/nextflow-docs-staging/deploys/65c7dbc1e12437000807d870

[bentsherman]

I kinda like the idea of overloading stdout for this purpose, since it can only be the stdout of the command. Otherwise we should use the full word command

[pditommaso]

I wanted to keep simple, at least as POC

[pditommaso]

Next steps:

• Handle command failure (how?)
  • Ben: command fail should trigger process fail, unless marked as optional (still print warning)
  • if implementation too complex, worth it to just document a || true syntax
• Handle multiline command output (likely currently breaks)
• Only works for Bash based tasks
  • will be a constraint of the implementation
• Support for tuple composition
• Add + fix tests

[bentsherman]

Is this ready to test with rnaseq? Even if it only supports single-line bash commands, should be enough.

[pditommaso]

it should

[bentsherman]

It doesn't seem to work with tuples, gonna need that for something like:

tuple val('gunzip'), cmd("gunzip --version")

[bentsherman]

I will add it

[marcodelapierre]

I kinda like the idea of overloading stdout for this purpose, since it can only be the stdout of the command. Otherwise we should use the full word command

@pditommaso sounds tempting to me, too, on this same ground by Ben.

[marcodelapierre]

I think this comment is interesting: #4386 (comment)
Also, it relates to your point above Paolo: Only works for Bash based tasks

If multi-line is allowed, then we might leverage the shebang to allow non-bash executions, including other shells, Groovy, popular scripting langs such as Python .. a bit like the current scoping for the script block , really.

Thoughts on this point?

[marcodelapierre]

Handle command failure (how?)

I would say, if the command fail, the task should fail. I.e. consistent with the behaviour as for the main script block.

[pditommaso]

If multi-line is allowed, then we might leverage the shebang to allow non-bash executions

not sure it's possible

[bentsherman]

I would say, if the command fail, the task should fail. I.e. consistent with the behaviour as for the main script block.

On this point, I think I suggested that the user should be able to ignore the failure by specifying optional: true, although it would be helpful to still log a warning in that case

If multi-line is allowed, then we might leverage the shebang to allow non-bash executions

Due to the compact nature of process inputs and outputs, it would be unwieldy to specify a multi-line script here. Instead, it should already be possible to reference a script from the bin directory in the cmd output. That would be useful anyway because, in rnaseq for example, many processes use the same tools and so the same version commands are duplicated.

Regarding the shebang, we could add a shell option to the command output and still support one-liners:

output:
cmd("print('Hello!')", shell: 'python')

[bentsherman]

I kinda like the idea of overloading stdout for this purpose, since it can only be the stdout of the command. Otherwise we should use the full word command

@pditommaso sounds tempting to me, too, on this same ground by Ben.

I'm starting to lean back towards a separate command or cmd type... , this command output feels different enough from stdout that it should have a different name, and it wouldn't make sense to extend stdin in the same way

[marcodelapierre]

I kinda like the idea of overloading stdout for this purpose, since it can only be the stdout of the command. Otherwise we should use the full word command

@pditommaso sounds tempting to me, too, on this same ground by Ben.

I'm starting to lean back towards a separate command or cmd type... , this command output feels different enough from stdout that it should have a different name, and it wouldn't make sense to extend stdin in the same way

I agree on the leaning back, based on the optional and script attributes you mentioned above, #4493 (comment), both of which make sense to me.

[bentsherman]

Handle multiline command output (likely currently breaks)

So the multi-line output does not fail, but the newlines are converted to spaces:

$ echo nxf_out_cmd_1=$(bash --version) > .command.env
$ cat .command.env
nxf_out_cmd_1=GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu) Copyright (C) 2020 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software; you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law.

The command itself is easily fixed with some quotes:

$ echo nxf_out_cmd_1="$(bash --version)" > .command.env
$ cat .command.env 
nxf_out_cmd_1=GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu)
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

But that would break the .command.env file which expects a key-value pair on each line.

The same limitation already exists for env outputs, e.g. if I do FOO=$(bash --version) in the process script then the process output will be squeezed onto a single line. For this reason, and the fact that cmd outputs are intended for small things like metadata, while larger outputs can always be written to a file, I am fine with leaving the current behavior as it is.

[pditommaso]

A single line is a too weak assumption. If you notice, nf-core uses a multi-lines YAML snippet!

https://github.com/nf-core/rnaseq/blob/a10f41afa204538d5dcc89a5910c299d68f94f41/modules/nf-core/salmon/index/main.nf#L42-L45

[bentsherman]

That is only because they are eagerly formatting it to YAML. Instead in my proof-of-concept PR I use tuples and separate cmd outputs:

https://github.com/nf-core/rnaseq/pull/1115/files#diff-d81c6cc6d9c866f8162450ce077b2413b28ecd47881a70a5c24a1ec1f1b70205

[marcodelapierre]

@pditommaso any blocker for this to be merged? Conversation around the review seems pretty much settled

[pditommaso]

Ok, I've cleaned up and refactored a but this. The main change is output: cmd become output: eval. The main reason is that command is too generic term and it was overlapping with the task command. I think eval express better the overall idea of a small computation that should be evaluated to capture the output value.

[ewels]

Nice! I like eval, good call 👍🏻