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

cqfd remove quotes in commands and additionnal commands #99

Open
eroussy opened this issue Mar 15, 2023 · 6 comments
Open

cqfd remove quotes in commands and additionnal commands #99

eroussy opened this issue Mar 15, 2023 · 6 comments

Comments

@eroussy
Copy link
Contributor

eroussy commented Mar 15, 2023

When inside a git repository, running git commit -m "some message" will create a new commit, but running cqfd run git commit -m "some message" send an error : error: pathspec 'message' did not match any file(s) known to git.
This error is the same when running git commit -m some message (without the double quotes) inside a terminal without cqfd.

cqfd seems to remove the quotes when running a custom command. It does it also with additional commands when running run -c.

.cqfdrc

[project]
org='default-org'
name='default-name'

[build]
command=''

.cqfd/docker/Dockerfile

FROM ubuntu:22.04

ENV DEBIAN_FRONTEND noninteractive

RUN apt-get update && apt-get install -y --no-install-recommends \
  git

Run a git init inside the current directory to have git support

@eroussy
Copy link
Contributor Author

eroussy commented Mar 21, 2023

I work a bit on this and find a fix :
0001-cqfd-parse-quotes-in-additionnal-commands.patch.txt

With this fix, the command cqfd run git commit -m "some message" works correctly.
However, a simple command such as cqfd run "echo test" doesn't work anymore.

Fact is that with the above patch, cqfd take all the characters of the argument line, including all the quotes and pass it to the docker.
In the first case, the command is then git commit -m "some message" wich is ok. But in the second case, the command is "echo test" which cannot be interpreted correctly because of the quotes.

In order to fix it properly, a solution would be to take the quotes except if it is the first character of the argument line.
I can't think of a simple solution to do that for now.

@gportay
Copy link
Contributor

gportay commented Apr 6, 2023

These are some notes, I have not yet the time to order all my ideas and write something that is well prepared. I will do some editing later.

TL;DR; To be honest, I really like the goal of your change; I have always considered cqdc as a wrapper on top of docker run that should be a shell frontend; your change sounds like I want to run these command through docker-run.

  • I would introduce a new command name shell to be able to run cqfd shell git message -m "Initial commmit" as you requested. This is the ssh cli style.
  • I would fix the command run to take a single argument cqfd run 'git message -m \"Initial commit\"'. This is the sh -c-ish cli style, but without the extra arguments, at least for now.

Both CLI are valids, and they have to be handled by different commands (the -c in the sh cli).

Note: I would also remove the -c STRING that concatenate the STRING to $command. Or at least rework to allow to prepend commands as well as they are similar. I must admit that this feature, as is at least, does not make sense to my eye; I think that command+=" foo bar" in a flavor may work.

This is where the long story starts...

All is a matter of the definition of the arguments... and the executable that are used underneath (docker-run, sh, sudo and su to name a few).

Unfortunately, the cqfd command-line interface (CLI) is unclear:

cqfd/cqfd

Line 33 in 0e4ffb6

Usage: $PROGNAME [OPTIONS] [COMMAND] [COMMAND OPTIONS] [ARGUMENTS]

cqfd/cqfd

Line 47 in 0e4ffb6

run Run argument(s) inside build container

The argument(s?) given to cqfd run is(are):

  • a COMMAND_STRING: a single string given as is(*); the string is split into arguments by the program that received the string (i.e. /bin/sh, or system(const char *) in C, given to /bin/sh -c)
  • or [COMMAND_FILE [ARGUMENT...]]: multiple strings; the arguments are split already and given as is(*) to the main() entry point of the program that received the strings (execv(const char *, char * const[]) in C, /bin/sh -c but not totally true because of second argument that is argv0)

*: The strings are subject to be expanded; see bash(1), in particular sections QUOTING and EXPANSION.

Important: Something really important to take into account is the COMMAMD_STRING is a single string that is interpreted by a program in the container. cqfd ends with a shell in the container via docker-run and su/sudo. Therefore, the COMMAND_STRING is a string following the shell grammar, it may contains control sequences, multiple commands..., where as the COMMAND_FILE [ARGUMENTS...] is a single command.

This misleading is due to the instruction below that forwards the remaining arguments to docker run:

cqfd/cqfd

Line 472 in 0e4ffb6

build_cmd_alt="$@"

Note: The proper instruction to save the arguments to a string variable is build_cmd_alt="$*"; "$@" expands the arguments to a list of strings; "$*" expands the arguments to a single string.

But, the arguments are forwarded to the function that wraps the docker-run as a single argument:

cqfd/cqfd

Line 497 in 0e4ffb6

docker_run "$build_cmd"

But (once again), the arguments (actually, the single argument) are given as multiple arguments to entry-point of docker-run:

cqfd/cqfd

Line 218 in 0e4ffb6

$docker_img_name cqfd_launch "$@" 2>&1

Finally, the arguments are forwarded to su in the container, via the its option -c:

cqfd/cqfd

Line 352 in 0e4ffb6

su $cqfd_user -p -c "\$@"

-c, --command=COMMAND
    pass a single COMMAND to the shell with -c

Or, to sudo if installed in the container, via the shell sh -c:

cqfd/cqfd

Line 349 in 0e4ffb6

sudo -E -u $cqfd_user sh -c "\$@"

sh -c [-abCefhimnuvx] [-o option]... [+abCefhimnuvx] [+o option]... command_string [command_name [argument...]]

-c        Read commands from the command_string operand. Set the value of special parameter 0 (see Section 2.5.2, Special Parameters) from the value of the  com‐
          mand_name  operand  and  the positional parameters ($1, $2, and so on) in sequence from the remaining argument operands. No commands shall be read from
          the standard input.

Additionnaly, looking at the cqfd run examples and tests, some of them:

  • quote the command to make a COMMAND_STRING, in order to add somme shell grammar in it to run multiple commands in the docker
  • use directly COMMAND [ARGUMENTS]

So, it looks the arguments are COMMAND [ARGUMENTS] but they are converted to a single string to be run in /bin/sh -c COMMAND_STRING.

Long time ago...

The cqfd run command was a short option -b BUILD_CMD before the commit 5f5b511 was merged.

See the option -b BUILD_CMD:

cqfd/common/sfl/cqfd

Lines 150 to 152 in 823c14e

b)
BUILD_CMD="$OPTARG"
;;

How the command= in the .cqfdrc is set to BUILD_CMD:

cqfd/common/sfl/cqfd

Lines 132 to 133 in 823c14e

cfg.section.build
BUILD_CMD="$command"

And how the BUILD_CMD is given to docker run here:

docker_run "$BUILD_CMD"

And there:

cqfd/common/sfl/cqfd

Lines 100 to 107 in 823c14e

docker run --privileged -v "$PROJECT_PATH":/home/builder/src \
-v ~/.ssh:/home/builder/.ssh \
$opt \
-it "$DOCKER_IMG_NAME" \
/bin/bash -c "groupadd -og $GROUPS -f builders && \
useradd -s /bin/bash -u $UID -g $GROUPS builder && \
chown $UID:$GROUPS /home/builder && \
su - builder -c \"cd src/ && $1\""

The instructions BUILD_CMD="$OPTARG", BUILD_CMD="$command" and su - builder -c \"cd src/ && $1\"" make me thing that cqfd was using the argument as a COMMAND_STRING. And my opinion is it was doing it right.

But what about the extra argument?

Conclusion

cqfd must define what are the arguments given to the command run.

It should answer the following questions:

What does cqfd run 'echo "$#"' one two three should echo?

  • 0
  • 3
  • 2

What does cqfd run 'echo \("$@"\)' one two three should echo?

  • () one two three
  • (one two three)
  • (two three)

@gportay
Copy link
Contributor

gportay commented Apr 6, 2023

I work a bit on this and find a fix : 0001-cqfd-parse-quotes-in-additionnal-commands.patch.txt

@eroussy thanks for let me know the existence of that variable substitution. I use to use bash arrays to preserve a list of argument.

In order to fix it properly, a solution would be to take the quotes except if it is the first character of the argument line. I can't think of a simple solution to do that for now.

I think the run CLI is kind of broken and you should have a second verb to run your command.

@gportay
Copy link
Contributor

gportay commented Apr 12, 2023

@joufella any special thoughs on this?

@gportay
Copy link
Contributor

gportay commented Jun 1, 2023

ping?

gportay added a commit to gportay/cqfd that referenced this issue Nov 29, 2023
TL;DR; The command run takes arguments as if they are given to system()
(i.e. the command_string is intepreted by sh -c). The command shell
takes arguments as if they are given to execlp($SHELL, ...), and the new
command exec takes arguments as they are given to execlp() (i.e. for the
two last, the command is not interpreted by sh; except for the arguments
that are not simple-quoted and that are subject to get interpreted by
the current shell). In short:
 - cqfd run -> sh -c
 - cqfd shell -> sh
 - cqfd exec -> ssh

This adds the exec command to run a command from within the container.

The command consists of the exec-program and its arguments (if any), as
it is given to the C function execlp(). The exec-program could be either
a binary or a script.

	$ cqfd exec COMMAND [ARGUMENTS...]

It is a distinct CLI to the commands run and shell that both takes a
command_string as it is given to the C function system() (i.e. given to
sh -c):
 - cqfd shell has three different forms:
   cqfd shell [command_file [argument...]]
   cqfd shell -c command_string [command_name [argument...]]
   cqfd shell -s [argument...] (requires CQFD_EXTRA_RUN_ARGS=-i for now)
 - cqfd run [-c] [command_string] takes a command_string as specified by
   sh(1) or the command= attribute from the file .cqfdrc.
   Important: the option -c of run is for concatenate and it is
   different to the option -c of sh(1).

Note: The command exec (as the command shell) does not run the command=
set in the file .cqfdrc.

According to sh(1):

	argument

	The positional parameters ($1, $2, and so on) shall be set to
	arguments, if any.

	command_file

	The pathname of a file containing commands. If the pathname
	contains one or more <slash> characters, the implementation
	attempts to read that file; the file need not be executable. If
	the pathname does not contain a <slash> character:
	* The implementation shall attempt to read that file from the
	  current working directory; the file need not be executable.
	* If the file is not in the current working directory, the
	  implementation may perform a search for an executable file
	  using the value of PATH, as described in Section 2.9.1.1,
	  Command Search and Execution.

	Special parameter 0 (see Section 2.5.2, Special Parameters)
	shall be set to the value of command_file. If sh is called using
	a synopsis form that omits command_file, special parameter 0
	shall be set to the value of the first argument passed to sh
	from its parent (for example, argv[0] for a C program), which is
	normally a pathname used to execute the sh utility.

	command_name

	A string assigned to special parameter 0 when executing the
	commands in command_string. If command_name is not specified,
	special parameter 0 shall be set to the value of the first
	argument passed to sh from its parent (for example, argv[0] for
	a C program), which is normally a pathname used to execute the
	sh utility.

	command_string

	A string that shall be interpreted by the shell as one or more
	commands, as if the string were the argument to the system()
	function defined in the System Interfaces volume of
	POSIX.1‐2017. If the command_string operand is an empty string,
	sh shall exit with a zero exit status.

Fixes: savoirfairelinux#99
@gportay
Copy link
Contributor

gportay commented Nov 29, 2023

@eroussy, do you think the cqfd exec git commit -m "my commit" help with your situation?

gportay added a commit to gportay/cqfd that referenced this issue Dec 1, 2023
TL;DR; The command run takes arguments as if they are given to system()
(i.e. the command_string is intepreted by sh -c). The command shell
takes arguments as if they are given to execlp($SHELL, ...), and the new
command exec takes arguments as they are given to execlp() (i.e. for the
two last, the command is not interpreted by sh; except for the arguments
that are not simple-quoted and that are subject to get interpreted by
the current shell). In short:
 - cqfd run -> sh -c "$*"
 - cqfd shell -> sh "$@"
 - cqfd exec -> ssh "$@"

This adds the exec command to run a command from within the container.

The command consists of the exec-program and its arguments (if any), as
it is given to the C function execlp(). The exec-program could be either
a binary or a script.

	$ cqfd exec COMMAND [ARGUMENTS...]

It is a distinct CLI to the commands run and shell that both takes a
command_string as it is given to the C function system() (i.e. given to
sh -c):
 - cqfd shell has three different forms:
   cqfd shell [command_file [argument...]]
   cqfd shell -c command_string [command_name [argument...]]
   cqfd shell -s [argument...] (requires CQFD_EXTRA_RUN_ARGS=-i for now)
 - cqfd run [-c] [command_string] takes a command_string as specified by
   sh(1) or the command= attribute from the file .cqfdrc.
   Important: the option -c of run is for concatenate and it is
   different to the option -c of sh(1).

Note: The command exec (as the command shell) does not run the command=
set in the file .cqfdrc.

According to sh(1):

	argument

	The positional parameters ($1, $2, and so on) shall be set to
	arguments, if any.

	command_file

	The pathname of a file containing commands. If the pathname
	contains one or more <slash> characters, the implementation
	attempts to read that file; the file need not be executable. If
	the pathname does not contain a <slash> character:
	* The implementation shall attempt to read that file from the
	  current working directory; the file need not be executable.
	* If the file is not in the current working directory, the
	  implementation may perform a search for an executable file
	  using the value of PATH, as described in Section 2.9.1.1,
	  Command Search and Execution.

	Special parameter 0 (see Section 2.5.2, Special Parameters)
	shall be set to the value of command_file. If sh is called using
	a synopsis form that omits command_file, special parameter 0
	shall be set to the value of the first argument passed to sh
	from its parent (for example, argv[0] for a C program), which is
	normally a pathname used to execute the sh utility.

	command_name

	A string assigned to special parameter 0 when executing the
	commands in command_string. If command_name is not specified,
	special parameter 0 shall be set to the value of the first
	argument passed to sh from its parent (for example, argv[0] for
	a C program), which is normally a pathname used to execute the
	sh utility.

	command_string

	A string that shall be interpreted by the shell as one or more
	commands, as if the string were the argument to the system()
	function defined in the System Interfaces volume of
	POSIX.1‐2017. If the command_string operand is an empty string,
	sh shall exit with a zero exit status.

Fixes: savoirfairelinux#99
gportay added a commit to gportay/cqfd that referenced this issue Jan 14, 2024
TL;DR; The command run takes arguments as if they are given to system()
(i.e. the command_string is intepreted by sh -c). The command shell
takes arguments as if they are given to execlp($SHELL, ...), and the new
command exec takes arguments as they are given to execlp() (i.e. for the
two last, the command is not interpreted by sh; except for the arguments
that are not simple-quoted and that are subject to get interpreted by
the current shell). In short:
 - cqfd run -> sh -c "$*"
 - cqfd shell -> sh "$@"
 - cqfd exec -> ssh "$@"

This adds the exec command to run a command from within the container.

The command consists of the exec-program and its arguments (if any), as
it is given to the C function execlp(). The exec-program could be either
a binary or a script.

	$ cqfd exec COMMAND [ARGUMENTS...]

It is a distinct CLI to the commands run and shell that both takes a
command_string as it is given to the C function system() (i.e. given to
sh -c):
 - cqfd shell has three different forms:
   cqfd shell [command_file [argument...]]
   cqfd shell -c command_string [command_name [argument...]]
   cqfd shell -s [argument...] (requires CQFD_EXTRA_RUN_ARGS=-i for now)
 - cqfd run [-c] [command_string] takes a command_string as specified by
   sh(1) or the command= attribute from the file .cqfdrc.
   Important: the option -c of run is for concatenate and it is
   different to the option -c of sh(1).

Note: The command exec (as the command shell) does not run the command=
set in the file .cqfdrc.

According to sh(1):

	argument

	The positional parameters ($1, $2, and so on) shall be set to
	arguments, if any.

	command_file

	The pathname of a file containing commands. If the pathname
	contains one or more <slash> characters, the implementation
	attempts to read that file; the file need not be executable. If
	the pathname does not contain a <slash> character:
	* The implementation shall attempt to read that file from the
	  current working directory; the file need not be executable.
	* If the file is not in the current working directory, the
	  implementation may perform a search for an executable file
	  using the value of PATH, as described in Section 2.9.1.1,
	  Command Search and Execution.

	Special parameter 0 (see Section 2.5.2, Special Parameters)
	shall be set to the value of command_file. If sh is called using
	a synopsis form that omits command_file, special parameter 0
	shall be set to the value of the first argument passed to sh
	from its parent (for example, argv[0] for a C program), which is
	normally a pathname used to execute the sh utility.

	command_name

	A string assigned to special parameter 0 when executing the
	commands in command_string. If command_name is not specified,
	special parameter 0 shall be set to the value of the first
	argument passed to sh from its parent (for example, argv[0] for
	a C program), which is normally a pathname used to execute the
	sh utility.

	command_string

	A string that shall be interpreted by the shell as one or more
	commands, as if the string were the argument to the system()
	function defined in the System Interfaces volume of
	POSIX.1‐2017. If the command_string operand is an empty string,
	sh shall exit with a zero exit status.

Fixes: savoirfairelinux#99
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

No branches or pull requests

2 participants