Avoid hard-coding the value of SRCTOP in generated files. Use /usr/src
as the canonical srcdir.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Was off for a couple of weeks so didn't have a chance to review prior to commit -- apologies. LGTM.
That said, I believe these are only in comments, and maybe we could just unconditionally strip the prefix entirely? E.g. instead of switching
* These definitions were generated by ./MKkey_defs.sh /home/emaste/src/freebsd-git/wipbsd/contrib/ncurses/include/Caps /home/emaste/src/freebsd-git/wipbsd/contrib/ncurses/include/Caps-ncurses
to
* These definitions were generated by ./MKkey_defs.sh /usr/src/contrib/ncurses/include/Caps /usr/src/contrib/ncurses/include/Caps-ncurses
we could just make it
* These definitions were generated by ./MKkey_defs.sh contrib/ncurses/include/Caps contrib/ncurses/include/Caps-ncurses
Comment Actions
That said, I believe these are only in comments,
Right.
and maybe we could just unconditionally strip the prefix entirely?
We could. In this and the other linked commits, I've consistently used /usr/src and /usr/obj as the canonical path prefixes for src and obj. We could use the empty string for src instead, but I suspect we want to provide a non-empty prefix for obj, and in that case it makes a bit more sense to me to provide a non-empty prefix for both src and obj.
Using an empty prefix here also doesn't really simplify anything AFAICS.
Given that, do you think it's worth making the change?
Comment Actions
Using an empty prefix here also doesn't really simplify anything AFAICS.
I like the empty prefix because it is both reproducible and nominally correct (a relative path with an unspecified prefix rather than an absolute path with a standard but not actually correct in this instance path).
I suspect we want to provide a non-empty prefix for obj.
Yeah, my argument doesn't hold for obj.
Given that, do you think it's worth making the change?
In one file I don't think it makes sense to have absolute obj and relative src paths, so /usr/src and /usr/obj for that case. If there's a possibility we could eliminate obj references from all files I'd argue for using relative, unspecified prefix src paths everywhere. So overall, 🤷
Comment Actions
Looking at the full stack now, I think we have three cases:
- comments (this and D50952)
- DWARF info
- newvers metadata
I have a slight preference for just using unspecified-prefix relative paths in comments, and not making it conditional on MK_REPRODUCIBLE_BUILD.
Comment Actions
Sorry for the slow response. My main reasons for using a full path and only canonicalizing when necessary are 1) to be consistent everywhere, so when non-reproducible paths are found going forward we have a simple policy to handle them, and 2) including the full path makes it easier to identify the origin of the file if one has multiple source trees on the same machine and reproducible builds aren't in use (which I expect will remain the default going forward?).
These are fairly weak reasons. If you still prefer to unconditionally provide relative paths, I'll change the review.
Comment Actions
IMO to the extent possible reproducible builds should be an intrinsic property of our tree, not something that needs special modes or workarounds. Having extra metadata in the kernel is definitely useful for certain workflows but I would probably have that and debuginfo be the only consumers of the reproducible build knob.
Anyway, the reproducible mode is on for releases which is where it's important and it's not too much work to carry a few small .ifs around so I don't insist on any changes here.