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

Improve command not found errors #2498

Closed
wants to merge 4 commits into from

Conversation

LnL7
Copy link
Member

@LnL7 LnL7 commented Oct 30, 2018

Improves error message when git/mercurial are not available. eg.

error: program 'git' failed with exit code 127
The program 'git' is currently not installed. It is required for builtins.fetchGit
You can install it by running the following:
nix-env -f '<nixpkgs>' -iA git

std::ostringstream out;
out << e.what() << std::endl;
out << "The program 'git' is currently not installed. It is required by builtins.fetchGit" << std::endl;
out << "You can install it by typing one of the following:" << std::endl;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
out << "You can install it by typing one of the following:" << std::endl;
out << "You can install it by running the following:" << std::endl;

std::ostringstream out;
out << e.what() << std::endl;
out << "The program 'hg' is currently not installed. It is required by builtins.fetchMercurial" << std::endl;
out << "You can install it by typing one of the following:" << std::endl;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
out << "You can install it by typing one of the following:" << std::endl;
out << "You can install it by running the following:" << std::endl;

@grahamc
Copy link
Member

grahamc commented Oct 30, 2018

This is awesome!

@LnL7 LnL7 force-pushed the improve-command-not-found-errors branch from 218ef8b to 4980230 Compare October 30, 2018 16:48
if (state.allowedPaths)
state.allowedPaths->insert(gitInfo.storePath);
} catch (ExecError & e) {
if (e.status / 256 == 127) {
Copy link
Member

Choose a reason for hiding this comment

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

Should be WEXITSTATUS(e.status) == 127.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, didn't realise where to look for that.

out << "The program 'git' is currently not installed. It is required for builtins.fetchGit" << std::endl;
out << "You can install it by running the following:" << std::endl;
out << "nix-env -f '<nixpkgs>' -iA git";
throw ExecError(e.status, out.str());
Copy link
Member

Choose a reason for hiding this comment

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

This should use format strings:

throw ExecError("%s\nThe program 'git' is currently not installed. ...", e.what());

@LnL7
Copy link
Member Author

LnL7 commented Nov 25, 2018

ping

Bash uses the same exit code to indictate a binary was not found in
PATH.
@LnL7 LnL7 force-pushed the improve-command-not-found-errors branch from a79f726 to b022d8e Compare July 1, 2019 22:18
@LnL7 LnL7 force-pushed the improve-command-not-found-errors branch from b022d8e to 75c6c42 Compare July 2, 2019 17:34
@grahamc
Copy link
Member

grahamc commented Jul 2, 2019

Just built the rebased version, and checked out the code at https://github.com/NixOS/nix/pull/2498/files?w=1

Looks good to me I think


if (state.allowedPaths)
state.allowedPaths->insert(state.store->toRealPath(gitInfo.storePath));
} catch (ExecError & e) {
Copy link
Member

Choose a reason for hiding this comment

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

It's better to put this check in exportGit because (on the flakes branch) it's used in other places as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, should I do the same for mercurial?

@grahamc
Copy link
Member

grahamc commented Jul 2, 2019 via email

@LnL7 LnL7 force-pushed the improve-command-not-found-errors branch from 419e211 to cfe3cbb Compare July 3, 2019 21:16
@domenkozar domenkozar added the error-messages Confusing messages and better diagnostics label May 20, 2020
@roberth
Copy link
Member

roberth commented Oct 1, 2020

Still relevant but needs a rebase. Current output with flakes is:

error: error: --- SysError -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- nix-build
executing 'git': No such file or directory
error: --- Error ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- nix-build
program 'git' failed with exit code 1

@LnL7
Copy link
Member Author

LnL7 commented Oct 18, 2020

I'm happy to rebase this, but it's been open for quite some time without getting attention.

@stale
Copy link

stale bot commented Jun 2, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Jun 2, 2021
@stale
Copy link

stale bot commented Jun 19, 2022

I closed this issue due to inactivity. → More info

@stale stale bot closed this Jun 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error-messages Confusing messages and better diagnostics stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants