Skip to content

Add command and args to execvp error message#7511

Merged
straight-shoota merged 1 commit intocrystal-lang:masterfrom
straight-shoota:fix/execvp-error-message
Mar 12, 2019
Merged

Add command and args to execvp error message#7511
straight-shoota merged 1 commit intocrystal-lang:masterfrom
straight-shoota:fix/execvp-error-message

Conversation

@straight-shoota
Copy link
Member

When LibC.execvp fails, it would previously raise an Errno with message execvp followed by the reason of failure (for example No such file or directory). But that's not a great help for debugging, unless you know which command was to be executed.

This PR adds the command and it's arguments to the error message. I'm not sure if the arguments are really necessary.

Related to #7508

# FIXME: Oddly doubled error message
expect_raises_errno(Errno::ENOENT, "execvp: No such file or directory: No such file or directory") do
Process.new("foobarbaz")
expect_raises_errno(Errno::ENOENT, %(execvp (foobarbaz "foo"): No such file or directory: No such file or directory)) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the printed command need a length restriction? If it has many arguments, then there could be some issues with the usability.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restricting the length could also have usability implications. The best solution would be to change the order and print the failed command after the libc error message. Errno doesn't support that though.

@straight-shoota straight-shoota merged commit 1396af8 into crystal-lang:master Mar 12, 2019
@straight-shoota straight-shoota deleted the fix/execvp-error-message branch March 12, 2019 22:30
@straight-shoota straight-shoota added this to the 0.28.0 milestone Mar 12, 2019
urde-graven pushed a commit to urde-graven/crystal that referenced this pull request Mar 20, 2019
* 'master' of github.com:crystal-lang/crystal:
  Change the font-weight used in Playground (crystal-lang#7552)
  Fix formatting absolute paths (crystal-lang#7560)
  Refactor IO::Syscall as IO::Evented (crystal-lang#7505)
  YAML: fix test checking serialization of slices for libyaml 0.2.2 (crystal-lang#7555)
  Let Array#sort only use `<=>`, and let `<=>` return `nil` for partial comparability (crystal-lang#6611)
  Update `to_s` and `inspect` to have similar signatures accross the stdlib (crystal-lang#7528)
  Fix restriction of valid type vs free vars (crystal-lang#7536)
  Rewrite macro spec without executing a shell command (crystal-lang#6962)
  Suggest `next` when trying to break from captured block  (crystal-lang#7406)
  Fix GenericClassType vs GenericClassInstanceType restrictions (crystal-lang#7537)
  Add human readable formatting for numbers (crystal-lang#6314)
  Add command and args to execvp error message (crystal-lang#7511)
  Implement Set#add? method (crystal-lang#7495)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants