Add warning if run command has multiple arguments

[gportay]

Dear Maintainers,

I think the command run command_string is ambiguous, and cqfd should tell the user to switch to cqfd exec PROGRAM [ARGUMENTS...] if the command_string is a single command or to cqfd shell -c "command_string" if the command_string is a list of command.

More information in 344969c.

What do you think about this?

[florentsfl]

this warning depends on if we really want to make cqfd run disappear totally or keep it for retrocompatibility

[gportay]

Yes kind of. but there is a world before removing the command run and just deprecating it using a warning telling to switch to another command.

So up to you.

[florentsfl]

Yes but the question is: do we really want to make people forget the "run" keyword ? If we don’t want that, then it is not necessary to put a warning.

run is used everywhere so it will be cumbersome for people to change, especially since there is no real issue with it at the moment, it works sufficiently well for what we need.

[gportay]

The run command does not work properly since cqfd run "touch foo" and cqfd run touch foo expect the same result. Since the command cannot forward the parameters property to the container, I suggest to move to another command that does it correctly.

This would prevent people to file an issue because it cannot deal with parameters:

cqfd run bash -c 'echo $0 $# $*' zero one two three
# no output because it runs bash -c echo $0 $# $* zero two three. the command string is word split.
#  sh -c [-abCefhimnuvx] [-o option]... [+abCefhimnuvx] [+o option]... command_string [command_name [argument...]]
# with command_string is echo instead of echo "$0 $# $*"
# arguments are: $0, $#, $*, zero, one, two, three instead of zero, one, two, three

The example below shows how the run command it broken.

So my opinion is to tell the user the command is broken and it should move to another one, as it does not work for every use case.

And we cannot fix it because the people using it as cqfd run "touch foo" would file an issue because it does not work anymore, and that example was part of the README.md.

So deprecating it is the only solution.

[florentsfl]

My point of view is that we can make cqfd run behave like cqfd exec without explicitly asking people to change their habits

[gportay]

My point of view is that we can make cqfd run behave like cqfd exec without explicitly asking people to change their habits

As said... "And we cannot fix it because the people using it as cqfd run "touch foo" would file an issue because it does not work anymore, and that example was part of the README.md."

We can leave this alone, if you wish and leave this it its own state. It is broken for years now.

EDIT; The sudoization of cqfd is a good way of deprecating the commands, since it simplifies the CLI by removing a word in their command. cqfd run ... -> cqfd .... And we can still make some a glue for the transition by shifting it, but this is not truly correct if run exists in the container.

[florentsfl]

With sudoization, what would cqfd "touch foo" do ?

[gportay]

With sudoization, what would cqfd "touch foo" do ?

It will fail because there is no "touch foo" binary.

The correct command would be cqfd sh -c "touch foo".

Currently it is cqfd shell -c "touch foo".

[florentsfl]

okay okay it is getting more and more clear in my mind, but I’m almost sure we will never get rid of cqfd run for backward compatibility.

For example when you reuse a very old project which has an old cqfdrc and old pipeline jobs, it is always comfortable if it still runs.

But I’m ok to display warning for the cqfd run "touch foo" even if it will never be deprecated

And I would combine that with a sudoization not breaking backward compatibility, i.e. we keep run but we ditch exec and shell.

[gportay]

But I’m ok to display warning for the cqfd run "touch foo" even if it will never be deprecated

Yes.

Furthermore, it is just a warning that does not mention about possible deprecation.