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

helix-term build: check git command's exit code #1674

Merged

Conversation

the-mikedavis
Copy link
Member

@the-mikedavis the-mikedavis commented Feb 16, 2022

Noticed this when building with a derivation very similar to the one in nixpkgs (https://github.com/the-mikedavis/dotfiles/blob/4767713774fb87b369725585719c75c334ca92a0/overlays/helix.nix): the git hash comes out to the empty string 🤔. The next line takes a [..8] slice of the string so it ends up panicing.

In this case I think it makes sense to leave the revision out of the version string.

@archseer
Copy link
Member

Weird, you'd think that git rev-parse would fail in that case.. There's no output at all?

@the-mikedavis
Copy link
Member Author

Yeah seems like it's the empty string, must be something weird about how nix is setting up the git repository.

Maybe it's empty string if it's a bare repository? Or maybe nix might be mucking with the HEAD in a weird way 🤔

@the-mikedavis
Copy link
Member Author

Oh actually since we're looking at the output of the Command it might be that the command is failing (I know this happens when the repository fails) and there's stuff on output.stderr. Maybe we should filter first on output.status.success()?

The unwrap (or '.ok()' rather) triggers for some errors but not
negative status codes. In the case where helix is being packaged
in an empty git repository, the existing mechanism will fail because

    git init
    git rev-parse HEAD

gives a negative exit code and prints to stderr

    stderr: "fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree....

with a stdout of "HEAD\n" (too short to slice with [..8]).
@the-mikedavis the-mikedavis force-pushed the md-term-git-hash-length-check branch from e87ad6d to 703ba28 Compare February 18, 2022 03:55
@the-mikedavis the-mikedavis changed the title helix-term build: check git hash length helix-term build: check git command's exit code Feb 18, 2022
@the-mikedavis
Copy link
Member Author

Seems like that was it ☝️

$ git init
$ git rev-parse HEAD
HEAD
fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

Where HEAD\n endns up on stdout and the rest on stderr

@archseer archseer merged commit a8cf0c6 into helix-editor:master Feb 18, 2022
@the-mikedavis the-mikedavis deleted the md-term-git-hash-length-check branch February 18, 2022 04:07
dead10ck pushed a commit to dead10ck/helix that referenced this pull request Feb 20, 2022
The unwrap (or '.ok()' rather) triggers for some errors but not
negative status codes. In the case where helix is being packaged
in an empty git repository, the existing mechanism will fail because

    git init
    git rev-parse HEAD

gives a negative exit code and prints to stderr

    stderr: "fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree....

with a stdout of "HEAD\n" (too short to slice with [..8]).
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

Successfully merging this pull request may close these issues.

2 participants