flock icon indicating copy to clipboard operation
flock copied to clipboard

The -c option behaves differently to Linux

Open marcomorain opened this issue 6 years ago • 10 comments

First of all, thanks for this project. It's great! ⭐️

On Linux, flock(1) will only accept a single string argument as the -c argument. This does not work with discoteq/flock:

Linux:

$ touch test.txt
$ flock test.txt -c 'echo 1'
1
$ flock test.txt -c echo 1
flock: -c requires exactly one command argument

macOS:

$ touch test.txt
$ ./flock -x test.txt -c "echo 1"
flock: failed to execute command: echo 1: No such file or directory

Interestingly, BusyBox also fails:

$ flock -x test.txt -c 'echo 1'; echo $?
255

Version:

commit 49c73617a18be069bf9f922b07d466511d1d3f5f
Merge: c02fc6b ec6f09e
Author: Joseph Anthony Pasquale Holsten <[email protected]>
Date:   Thu Dec 1 15:25:23 2016 -0800

marcomorain avatar Jul 23 '17 20:07 marcomorain

Great, I'll see how they're executing. I was a little afraid of entering n levels of encoding hell, so I just pass arguments along to execvp, see https://github.com/discoteq/flock/blob/master/src/flock.c#L304

I'm not sure I agree that linux-utils behaviour is what is most reasonable, but compatibility trumps sanity, right?

josephholsten avatar Jul 23 '17 23:07 josephholsten

Ah, they're being "smart" and passing along to a subshell: https://github.com/karelzak/util-linux/blob/master/sys-utils/flock.c#L239

Can you find one other person to agree this is the appropriate behaviour? I'm torn.

josephholsten avatar Jul 23 '17 23:07 josephholsten

Looks like it was added back in https://github.com/karelzak/util-linux/commit/baf39af15b1fe8570e6430788ec7cd4959fbc5d9#diff-1219b3b65dcec8078370f245769671b1R193

And the only explanation in its NEWS is:

  • flock: replaced with flock-2.0.2 by H. Peter Anvin

josephholsten avatar Jul 23 '17 23:07 josephholsten

maybe if it's a single string, pass to a subshell; if it's multiple, exec directly? I feel like something has similar behaviour but I can't remember what.

josephholsten avatar Jul 23 '17 23:07 josephholsten

Hi @josephholsten

Thanks for looking into this.

I think the -c option is to allow the caller have some symbols that would otherwise be interpreted by the shell.

flock -x file.lock cat id_rsa.pub >> authorized_keys

vs

flock -x file.lock -c 'cat id_rsa.pub >> authorized_keys'

In the first case above, the output of flock is appended to authorized_keys, outside of the lock; in the second case, the contents of id_rsa.pub are appended to authorized_keys inside the lock.

marcomorain avatar Jul 24 '17 23:07 marcomorain

ah, well that use case convinces me. I'd usually this pattern for that use case, but that's obnoxious for one-offs.

ok, I'll look into implementing this.

josephholsten avatar Jul 25 '17 15:07 josephholsten

+1

wbaelen avatar Sep 17 '17 02:09 wbaelen

so sorry, I dropped this right on the floor. I know I can't get to this till at least this evening, so here are my notes:

At the moment, -c is just grabbing the command from getopt_long():

cmd_argv = &argv[optind + 1];

and tossing it straight into execvp()

			if (0 != execvp(cmd_argv[0], cmd_argv)) {

so I really just want to build a new cmd_argv with cmd_argv[0] = getenv("SHELL") || "/bin/sh", cmd_argv[1] = "-c", then push the rest in.

Needs doc, especially to say that you can SHELL=/bin/wtfsh to specify your own.

josephholsten avatar Sep 17 '17 19:09 josephholsten

Yes, please implement this, it prevents it from being a 'drop in replacement' with some scripts i have :(

hongkongkiwi avatar Mar 15 '18 13:03 hongkongkiwi

If this is interesting to you, please review the implementation in #23

josephholsten avatar Aug 01 '19 15:08 josephholsten