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

Hydra executables should provide the commit at which they were built #849

Closed
abailly-iohk opened this issue May 5, 2023 · 10 comments · Fixed by #873
Closed

Hydra executables should provide the commit at which they were built #849

abailly-iohk opened this issue May 5, 2023 · 10 comments · Fixed by #873
Assignees
Labels
bug 🐛 Something isn't working
Milestone

Comments

@abailly-iohk
Copy link
Contributor

Running recent static binary, I got the following:

$ ./hydra-node  --version
0.10.0

which is not very informative and does not help in checking whether or not I am running the expected version of the node, and at which commit this version was built.

I would expect something like

$ ./hydra-node  --version
0.10.0-342137bcd
@abailly-iohk abailly-iohk added the bug 🐛 Something isn't working label May 5, 2023
@ch1bo
Copy link
Collaborator

ch1bo commented May 8, 2023

There was a related red bin item with the observation this is coming from the nix build which does not use git describe output:

$ cabal build hydra-node
$ cabal exec hydra-node -- --version
0.8.0-175-g4cf4155cf
$ nix build -f release.nix hydra-node
$ result/bin/hydra-node --version    
0.9.0

@abailly-iohk
Copy link
Contributor Author

cardano-node is patching the executable's binary to add the version, apparently: https://github.com/input-output-hk/cardano-node/blob/master/nix/haskell.nix#L297-L324

@ch1bo
Copy link
Collaborator

ch1bo commented May 10, 2023

May I suggest that we also try to use the git describe output in the nix build? Would your expectation be met with something like

$ ./hydra-node --version
0.9.0-g123-342137bcd

Which includes the git revision 342137bcd, but also additional information that the last release was 0.9.0 and this is some 123 commits ahead.

@abailly-iohk
Copy link
Contributor Author

The cardano-node uses a complexe machinery to patch the binaries, which I find somewhat annoying. I have started a branch that instead tries to read the needed revision (or description) from an environment variable that can be set for the underlying builder to use. The problem is that I don't know how to pass that environment variable to the nix build...
Any idea @ch1bo?

Note: I am aware this makes nix build non hermetic. I can resort to the node's solution (patching binaries) if need be.

@abailly-iohk
Copy link
Contributor Author

@pgrange Suggested we could also patch the source code before the build which is a sed oneliner and friendlier to nix perhaps?

@pgrange
Copy link
Contributor

pgrange commented May 10, 2023

Not proud of it but relying on this trick when releasing another project:

@abailly-iohk
Copy link
Contributor Author

It would be great if we could use the version field of the cabal package description but of course, cabal does not allow that: https://cabal.readthedocs.io/en/3.4/cabal-package.html#pkg-field-version
@pgrange The problem with modifying sources is that nix flake is sensitive to uncommitted changes I think. I think the least ugly solution is to patch the binary (note that nix already does this for setting dynamic libraries path to /nix/store)

@ch1bo
Copy link
Collaborator

ch1bo commented May 15, 2023

It would be great if we could use the version field of the cabal package

IIRC we do use the version from the .cabal file as a fallback. It's the 0.10.0 from your original post where we did not (yet) git tag 0.10.0.

@abailly-iohk
Copy link
Contributor Author

@ch1bo Yes, but my point was to suggest we could use the version field to add the SHA information on the fly when building (eg. patching source)

@abailly-iohk
Copy link
Contributor Author

I think I will go for a simple solution: Generate a file containing the data and use file-embed to inject the content of this file, if it exists, in the executable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants