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

fd does not expose exit status from --exec result #333

Closed
allenbenz opened this issue Oct 2, 2018 · 9 comments · Fixed by #477
Closed

fd does not expose exit status from --exec result #333

allenbenz opened this issue Oct 2, 2018 · 9 comments · Fixed by #477

Comments

@allenbenz
Copy link

allenbenz commented Oct 2, 2018

It would be helpful for error handling to be able to opt into fd returning a non-zero status when a utility invoked with --exec returns a non-zero status.

find does this when the arguments of --exec contain {} +

@sharkdp
Copy link
Owner

sharkdp commented Oct 3, 2018

Thank you for your feedback.

Sounds like a reasonable request to me, but we currently don't have the {} + variant of --exec in fd (related: #274). Also partially related: #303.

@allenbenz
Copy link
Author

allenbenz commented Oct 3, 2018

If you have any design suggestions I could put together a PR.

I also took another look at what I was asking for and what I wanted.
I misread the --ok documentation, it does not work in the way I described and have removed it from post.
Also {} + is two things, non-zero status code on exec failure and aggregating arguments.

Would it be better to ignore how posix find defines --exec and split those two concepts? It seems like it was designed that way to work with find's expression model.

@sharkdp
Copy link
Owner

sharkdp commented Oct 5, 2018

Ok, so there are several things that we need to clarify/decide here:

  • Do we want to support the {} + variant of finds -exec?

    Yes, I think so. I kind of like the --exec-once idea in Multiple instances of subl get launched #274.

  • What should the exit code be in the case where we use --exec?

    So find only uses a non-zero exit code in case of problems while traversing the directory tree (like in a normal execution of fd/find).

    Additionally using non-zero exit codes in the case where one of the child processes fails would somehow overload that error path.

    However, there could be a chance to do things better than find here (see https://apple.stackexchange.com/questions/49042/how-do-i-make-find-fail-if-exec-fails). Maybe we could use different exit codes for the two failure modes (1 in case of directory traversal error, 2 in case of --exec failures).

  • What should be the exit-code for uses of --exec-once?

    I don't really see a reason why this should be handled differently from the --exec code. Maybe someone can find a reason why find seems to do this?

@allenbenz
Copy link
Author

{} + can still execute multiple times when there are too many characters in the line (8192 for cmd, 32767 for powershell, ARG_MAX otherwise). Maybe it should be something else, like --exec-batch?

I could not find an explicit reasoning behind the exit status change in the rational for adding {} +

It might have been because {} + was meant to workaround a problem with piping weird filenames to xargs and xargs raises the status code.

@sharkdp
Copy link
Owner

sharkdp commented Oct 9, 2018

{} + can still execute multiple times when there are too many characters in the line (8192 for cmd, 32767 for powershell, ARG_MAX otherwise). Maybe it should be something else, like --exec-batch?

Exactly - see my comment in #274. As for the name, I kind of like --open-with.

@mcandre
Copy link

mcandre commented Nov 15, 2018

Meaningful exit status is the biggest fail in the typical UNIX find utility. Not to mention the limitations of -exec, the limitations of xargs, and the exit status of the overall find ... | xargs composite command. I would consider replacing find in my scripts with another tool, but only if this were addressed by that tool. As much as I dislike find, it is still easier to communicate to my fellow developers a dependency on (GNU) findutils and the known ball of awful that comes with it, than to add a new layer to the stack that doesn't even address find's basic problems.

Ultimately, the fragility of find workflows is one reason I am moving away from make and shell scripts in general, and instead using library calls to accomplish my tasks. It's just more reliable to use mage, rake, invoke, tinyrick, and so on compared to constantly fixing corner cases.

@sharkdp
Copy link
Owner

sharkdp commented Nov 18, 2018

@mcandre So what exactly are the big "fails" that you see in finds exit status handling? What could we learn from these mistakes and how exactly do you think fd should behave?

(@ALL note that we have recently added --exec-batch to fd)

@sharkdp
Copy link
Owner

sharkdp commented Sep 13, 2019

@allenbenz @mcandre This has been implemented in #477.

@sharkdp
Copy link
Owner

sharkdp commented Sep 15, 2019

Released in v7.4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants