It doesn't provide much value, and contributes noise in diff when comparing the output to src.conf.5 for downstream users using a VCS that does not expand the tags.
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
I find this very useful as it tells you what file and revision each section was generated from.
Comment Actions
I won't pursue this if it's actually useful to folks.
However, the file is redundant, it's already listed as the .It Va
.It Va WITHOUT_ASSERT_DEBUG .\" from FreeBSD: head/tools/build/options/WITHOUT_ASSERT_DEBUG 162215 2006-09-11 13:55:27Z ru
and the version of each WITH_/WITHOUT_ is going to be the current one on the branch when src.conf.5 is regenerated, and svn can give you the modification date and revision if you need it:
% svn info -r 200000 WITHOUT_ASSERT_DEBUG ... Last Changed Author: ru Last Changed Rev: 162215 Last Changed Date: 2006-09-11 13:55:27 +0000 (Mon, 11 Sep 2006)
Comment Actions
Yes, but it's quite a special case. It doesn't normally happen. I almost left out the tag changes too.
Comment Actions
By the way I do understand the reasoning for this, I just like having them. I only have an SVN checkout for head for makeman handling.
Comment Actions
I only have an SVN checkout for head for makeman handling.
In that case this change would (nearly) allow you to do away with that SVN checkout and use your normal development repository for makeman. (Nearly, because it still inserts the makeman version itself that was used to generate the src.conf.5.)
Comment Actions
For reference, the generated output with this change looks like:
.\" DO NOT EDIT-- this file is automatically generated. .\" $FreeBSD$ .Dd March 16, 2017 .Dt SRC.CONF 5 ... .Pp This list provides a name and short description for variables that can be used for source builds. .Bl -tag -width indent .It Va WITHOUT_ACCT Set to not build process accounting tools such as .Xr accton 8 and .Xr sa 8 . .It Va WITHOUT_ACPI Set to not build .Xr acpiconf 8 , .Xr acpidump 8 and related programs. ...
| tools/build/options/makeman | ||
|---|---|---|
| 5 ↗ | (On Diff #26333) | How about adding a comment here that describes what this byzantine script actually is meant to do? (As opposed to what it actually does, not necessarily the same thing.) See the echo on line 127 for a start, but something more would be useful. |
| tools/build/options/makeman | ||
|---|---|---|
| 5 ↗ | (On Diff #26333) | I agree we should, but in a separate commit. |
Comment Actions
As someone who adds options in a git controlled tree, the version entries are really unhelpful and cause conflicts. I'd like to see this go in.
Comment Actions
I'm OK not having the IDs used to generate things... They don't provide that much info in this case, and the ID for the file will get us traceability.
On a side note, both svn and cvs had methods to exclude keyword strings from diffs that were generated. A feature that git doesn't really have, hence the friction point. But the keywords aren't all bad. They provide, or have in the past, a useful when other projects imported code, there'd be a nice, standardized version string that people almost universally used in preference to gin'ing up their own. It would be nice if there was a simple, canonical form in git-land.
| tools/build/options/makeman | ||
|---|---|---|
| 134 ↗ | (On Diff #26333) | In a perfect world (where we all used svn/generated src.conf(5) from svn), this could be replaced with ident | awk... but that needs to be cleaned up first (an error message is showing up in makeman >_>..): $ git diff tools/build/options/makeman diff --git a/tools/build/options/makeman b/tools/build/options/makeman index db9cb4d02faa..8a30c5c1eeed 100755 --- a/tools/build/options/makeman +++ b/tools/build/options/makeman @@ -5,7 +5,7 @@ set -o errexit export LC_ALL=C -ident='$FreeBSD$' +ident='$FreeBSD: ngieBSD $' t=$(mktemp -d -t makeman) trap 'test -d $t && rm -rf $t' exit $ ident -q tools/build/options/makeman tools/build/options/makeman: $FreeBSD: ngieBSD $ $target: ignoring duplicate option $ Sadly, it doesn't look like there's a good way to do this with git since $FreeBSD$ isn't expanded :(. Oh well.. back to the drawing board. |
| 244 ↗ | (On Diff #26333) | Could you please add a quick note about this? It took me a second to parse what was being done here, but for folks who don't know how sed regexp's work, saying "Delete RCS keywords from WITH* files". Also, I think this regular expression can be simplified to: sed -e '/$FreeBSD.*$/d' This is preferred because:
|
Comment Actions
The options are already documented in the WITH* files. I'm not sure what you're gaining with knowing the file, especially when future commits stated what generated the manpage in the AUTHORS section ;)..
| tools/build/options/makeman | ||
|---|---|---|
| 244 ↗ | (On Diff #26333) | With your simplified expression I think it's self-explanatory and no comment needed. |
Comment Actions
Ok, thanks for the feedback all. I accept that there's some desire to keep these, but I think on balance there's more benefit in removing them and will commit shortly.
Comment Actions
Well, you could also argue that by not expanding keyword strings, git automatically excludes them from generated diffs :-)
But the keywords aren't all bad. They provide, or have in the past, a useful when other projects imported code, there'd be a nice, standardized version string that people almost universally used in preference to gin'ing up their own.
Yes, this is a fair point, and is certainly relevant when drivers, libc code, utilities etc. get picked up by a 3rd party and subsequently made available again as open source, without the history being available. This is kind of a special case, but it does happen. This is why I won't advocate for doing away with $FreeBSD$ altogether. Of course, that doesn't really apply in cases where we commit generated files to the source tree.