Fix Nix build header: F* and krml versions empty#394
Fix Nix build header: F* and krml versions empty#394cmovcc wants to merge 5 commits intoFStarLang:masterfrom
Conversation
What problem are you trying to solve? Have you seen hash collisions?
@mtzguido @tahina-pro is this output expected to be stable?
unless you can show me a situation where an exception will be raised, no |
No, I haven't, and it remains very unlikely, even the Linux kernel keeps revision of length 12 without any ambiguity issue, afaik. I think that the header purpose is to provide one with information about the toolchain that has been used to produce the corresponding file, possibly long after the extraction. As I don't think the header would be less readable/practical with full git revisions, why shouldn't it be changed to use full git revisions? By the way,
I guess it depends on the previous question (thanks for cc-ing). |
While I don't foresee any changes, I wouldn't want to close the door on adding/removing/reordering some lines in it. Could this code just try to read each line and find the one that starts with |
|
Good suggestion, Guido -- thanks. Regarding the commit hash: for the sake of keeping things readable, let's hardcode 12 as the length (what is good for Linux is good for us). Do not use a global variable, just leave 12 in the code, and let's not bikeshed on this -- this is entirely opinion-driven, unrelated to the goal of this PR, and does not solve any problem encountered in the wild. |
|
@cmovcc: KString.starts_with is the helper you want here, something like
|
8b7f6df to
f198096
Compare
use fstar.exe --version instead of git rev-parse HEAD in $FSTAR_HOME; bump git revision length from 8 to 12
f198096 to
de6337b
Compare
aff02d3 to
cee62a0
Compare
use Version.version instead of git rev-parse HEAD in $KRML_HOME
cee62a0 to
96f97c9
Compare
|
Thanks for the hint! |
Use
fstar.exe --versioninstead ofgit rev-parse HEADin$FSTAR_HOMEdirectory. Indeed, when building the F*+KaRaMeL toolchain in a Nix environment,$FSTAR_HOMEdoes not point to a git repository.I foresee a few discussions.
List.nth (...) 5), I was unsure about the portability ofgrepusage, for example when using MacOS. As a consequence, we rely on the current output pattern offstar.exe --version, should this be documented somewhere ? Does this choice seem reasonable?git_rev_lengthvariable to control the length of F*/KaRaMeL revisions that are present in the KaRaMeL default header and raise it from 8 to 10. Perhaps this could/should be raised further, e.g. to 40, to be future-proof without any need to bump it?