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

The -c option behaves differently to Linux #10

Open
marcomorain opened this issue Jul 23, 2017 · 10 comments
Open

The -c option behaves differently to Linux #10

marcomorain opened this issue Jul 23, 2017 · 10 comments

Comments

@marcomorain
Copy link
Contributor

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 <joseph@josephholsten.com>
Date:   Thu Dec 1 15:25:23 2016 -0800
@josephholsten
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

Looks like it was added back in util-linux/util-linux@baf39af#diff-1219b3b65dcec8078370f245769671b1R193

And the only explanation in its NEWS is:

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

@josephholsten
Copy link
Contributor

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.

@marcomorain
Copy link
Contributor Author

marcomorain commented Jul 24, 2017

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.

@josephholsten
Copy link
Contributor

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.

@wbaelen
Copy link

wbaelen commented Sep 17, 2017

+1

@josephholsten
Copy link
Contributor

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.

@hongkongkiwi
Copy link

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

@josephholsten
Copy link
Contributor

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

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

4 participants