Page MenuHomeFreeBSD

freebsd-version(1): present -v option, to print verbose output
Needs ReviewPublic

Authored by egypcio on Nov 22 2020, 4:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 28, 5:05 AM
Unknown Object (File)
Tue, Nov 26, 2:52 PM
Unknown Object (File)
Sun, Nov 24, 1:13 PM
Unknown Object (File)
Fri, Nov 22, 10:30 AM
Unknown Object (File)
Thu, Nov 21, 9:26 PM
Unknown Object (File)
Mon, Nov 18, 4:22 AM
Unknown Object (File)
Nov 18 2024, 12:35 AM
Unknown Object (File)
Nov 17 2024, 8:44 PM

Details

Summary

by applying this patch, we would be able to extend freebsd-version's features to:

  • print verbose information of the base system (using existing userland_version() function);
  • print verbose information of the running kernel (using existing running_version() function);
  • print verbose information from a slightly parsed output of what -qs (using existing kernel_version() function);
  • update its manpage, mentioning __FreeBSD_version and the references to it.

this was tested against the following branches:

  • releng/12.3;
  • stable/12;
  • releng/13.1;
  • stable/13;
  • main
Test Plan
  • default output, kept as is
% freebsd-version
14.0-CURRENT
  • output using verbose information
% freebsd-version -v
14.0-CURRENT 1400063
  • output using verbose information (running_version)
% freebsd-version -rv
14.0-CURRENT 1400063
  • userland and running_version, kept as is
% freebsd-version -ru
14.0-CURRENT
14.0-CURRENT
  • userland and running_version with verbose information
% freebsd-version -ruv
14.0-CURRENT 1400063
14.0-CURRENT 1400063
  • print out kernel version
% freebsd-version -k
14.0-CURRENT
  • print out verbose kernel information
% freebsd-version -kv
14.0-CURRENT main-n256697-b691e485bda5

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

extend verbose output also into the kernel information, using what @kevans recommended for the running_version() too.

egypcio@tak:~ % freebsd-version 
14.0-CURRENT
egypcio@tak:~ % freebsd-version -ruv
14.0-CURRENT 1400063
14.0-CURRENT 1400063

print out verbose information, but not a long line (with build date) like it was before.

egypcio@tak:~ % freebsd-version -k
14.0-CURRENT
egypcio@tak:~ % freebsd-version -kv
FreeBSD 14.0-CURRENT #0 main-n256697-b691e485bda5

so, the above output is based on what's output from:

egypcio@tak:~ % what -qs /boot/kernel/kernel
FreeBSD 14.0-CURRENT #0 main-n256697-b691e485bda5: Fri Jul 15 09:59:10 UTC 2022

same as latest change above, removing "FreeBSD" from the output for verbose kernel's version output:

egypcio@tak:~ % freebsd-version -k
14.0-CURRENT
egypcio@tak:~ % freebsd-version -kv
14.0-CURRENT #0 main-n256697-b691e485bda5

Getting closer.

bin/freebsd-version/freebsd-version.1
26

s/documented //

Also,

... in the
%B "Porter's Handbook" .

I'm not 100% sure about . after inline %B, so it may need adjusting.

40–41

Accidental text deletion between these lines? Sections SEE ALSO and HISTORY, and some of AUTHORS, all seem missing.

And I'd add the Porter's Handbook to SEE ALSO:

.Rs
.%B "Porter's Handbook"
.%Q "FreeBSD Documentation Project"
.%U https://docs.freebsd.org/en/books/fdp-primer/
.Re

This revision now requires changes to proceed.Aug 10 2022, 8:58 PM
egypcio changed the visibility from "All Users" to "Public (No Login Required)".
egypcio marked 2 inline comments as done.
egypcio edited the test plan for this revision. (Show Details)

See also PR#265789, it would help if

freebsd-version -rku and -kru did not print the same regardless of the sequence of CLI arguments.

So if some patch for that would be added to this review, that would be very helpful.

In D27318#821022, @pi wrote:

See also PR#265789, it would help if

freebsd-version -rku and -kru did not print the same regardless of the sequence of CLI arguments.

So if some patch for that would be added to this review, that would be very helpful.

thanks for sharing the though!

I shared the link to the Phabricator on that Bugzilla ticket really to get more inputs here, and see what others think about it.

IMHO I do not believe that changing the order will be much of a big help here - but that's just me.

In D27318#821022, @pi wrote:

See also PR#265789, it would help if

freebsd-version -rku and -kru did not print the same regardless of the sequence of CLI arguments.

So if some patch for that would be added to this review, that would be very helpful.

I'd keep them separate, personally.

This revision is now accepted and ready to land.Aug 12 2022, 1:13 PM

thanks for accepting this revision @pauamma
we shall just get an OK from @gbe and/or @kevans and this is good to be committed. right?

thanks for accepting this revision @pauamma
we shall just get an OK from @gbe and/or @kevans and this is good to be committed. right?

Yep, forgot to mention that.

Thanks!

Approved by: kevans (src)

Thanks!

Approved by: kevans (src)

I would like to get back to these changes in a very few and propose a slightly minor change.

what would you folks think about the following output?

% freebsd-version -urkv
14.0-CURRENT main-n256697-b691e485bda5 kernel_version
14.0-CURRENT 1400063 running_version
14.0-CURRENT 1400063 userland_version

ONLY the verbose output would be impacted by that.

thoughts? I would be happy to change it all to fit this kind of format (which I believe can benefit scripting the proposed changes to get verbose information).

remove the number of times kernel was compiled from the verbose output of freebsd-version -kv;

this gives this output only 2 columns, instead of 3.

by doing so we have a consistency on all outputs which would consist of:

  • column 1 represents the version;
  • column 2 represents the verbose information.

final presented diff produces us the following:

egypcio@tak:~ % freebsd-version -kruv
14.0-CURRENT main-n257229-e9a2e4d1d28b
14.0-CURRENT 1400065
14.0-CURRENT 1400065
This revision now requires review to proceed.Aug 13 2022, 5:49 AM

I would need an approval from src once again (@des @gbe @kevans );
considering an implicit approval from doc - as the final diff presented here didn't touch any manpage since their last approval.

LGTM for the man page parts. I can't give permission for the src changes, that must be done by an src committer.

@des @kevans @imp would you grant permission for @egypcio ?

bin/freebsd-version/freebsd-version.1
63–74

This places option v alphabetically after u, followed by j.

I think, better to:

  1. preserve the sequence of k r u j – that is, strictly, the order of printing if two or more of these four options are specified ("… finally the userland version of the specified jails, …")
  2. then describe v (after j).
65

More concisely:

Verbose output.

adding a more detailed diff, based on git diff -U999;
getting consistent about the term "verbose output" on updated manpage.

egypcio retitled this revision from freebsd-version: present -v option, to print verbose information to freebsd-version(1): present -v option, to print verbose output.Aug 14 2022, 10:41 AM
egypcio edited the test plan for this revision. (Show Details)
egypcio added inline comments.
bin/freebsd-version/freebsd-version.1
63–74

IMHO, we already have a pretty solid order defined for this manpage (since its first implementation):

  1. options that do not take arguments (alph. sorted);
  2. options that take arguments (alph. sorted).

I do not see us all in a need to address this suggestion of yours into freebsd-version(1).

bin/freebsd-version/freebsd-version.1
83–87

several of the above options is true for k r u j.

If option v is added above (not below) this paragraph, then this sentence might benefit from a rewrite.

egypcio marked an inline comment as done.
egypcio marked an inline comment as done.

update kernel_version() to support printing verbose output for different FreeBSD releases and snapshots.

while working to patch previous releases of FreeBSD against this revision, I applied a minor fix to get a formatted output not only for CURRENT, but also for STABLE and RELENG.

these are the default outputs of what -qs "$kernel_file" for all tested releases/snapshots:

FreeBSD 12.3-RELEASE r371126 GENERIC
FreeBSD 12.3-STABLE r372404 GENERIC
FreeBSD 13.1-RELEASE releng/13.1-n250148-fc952ac2212 GENERIC
FreeBSD 13.1-STABLE #0 stable/13-n252107-eb2a9b78586: Fri Aug 12 02:23:37 UTC 2022
FreeBSD 14.0-CURRENT #0 main-n257314-6a70a0c8bfa: Fri Aug 12 08:33:35 UTC 2022

the applies changes would print the proper verbose output for all mentioned versions now. taking a few examples we have:

% freebsd-version -kv
12.3-STABLE r372404
% freebsd-version -kv
13.1-RELEASE releng/13.1-n250148-fc952ac2212
% freebsd-version -kv
13.1-STABLE stable/13-n252107-eb2a9b78586

I'm sorry, but... why? freebsd-version(1) was never intended for human consumption, it was intended for use by configure scripts, installers, etc. Adding -j was bad enough, I'm sorry I didn't catch it in time (I suppose it's too late to remove it). If you want the kind of information -v would give you, use uname instead.

In D27318#821873, @des wrote:

I'm sorry, but... why? freebsd-version(1) was never intended for human consumption, it was intended for use by configure scripts, installers, etc. Adding -j was bad enough, I'm sorry I didn't catch it in time (I suppose it's too late to remove it). If you want the kind of information -v would give you, use uname instead.

hello there! thanks for the inputs, much appreciated.

the "never intention" to make freebsd-update(1) be consumed by humans got pretty lost these days, if I may flag that. really.
it's used by folks troubleshooting issues, and share FreeBSD's version information (and many other use cases: eg.: from a simple Nagios monitoring script, to a more complex update routine or a status report script).

recently the following Bugzilla ticket brought more attention to this differential review (and we are trying to fix at least a few of the raised issues):

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=265594

regarding the mention about uname(1): folks tend to use it without adding the -U and/or -K options to it; they probably only get the "usual" uname -a output, which would not report the OSVERSION/OSRELDATE.

we are also providing a clean output right away -- different from what (e.g.) uname -abiUK would produce. so: this changes here into freebsd-version(1) would also make easier to parse/use/read.

we are using uname(1) with a proper argument to add this proposed improvement into freebsd-version(1). this will pretty much, IMHO, solve folks' problems to get a more detailed information about FreeBSD's version on their boxes.

that's my input to your "why?" question.

bin/freebsd-version/freebsd-version.1
5–24

Is this addition at @des' request?

pauamma_gundo.com added inline comments.
bin/freebsd-version/freebsd-version.1
5–24

Never mind. Reverting an earlier deletion.

This revision is now accepted and ready to land.Aug 16 2022, 1:08 AM

ping @kevans (or anyone else from src)?
mind to have a look again, to maybe approve it after all changes got applied; it should be ready-to-go.

ping @kevans (or anyone else from src)?
mind to have a look again, to maybe approve it after all changes got applied; it should be ready-to-go.

ping? anyone.

bin/freebsd-version/freebsd-version.sh.in
73

^12?

The ^12 thing is concerning...
Otherwise I think it looks good.

bin/freebsd-version/freebsd-version.sh.in
73

that's indeed necessary; as we might take care about output's compatibility here.

probably due to a switch to a different VCS ( from svn to git)?

the outputs for what -qs /boot/kernel/kernel are documented here on a previous comment;

FreeBSD 12.3-RELEASE r371126 GENERIC
FreeBSD 12.3-STABLE r372404 GENERIC
FreeBSD 13.1-RELEASE releng/13.1-n250148-fc952ac2212 GENERIC
FreeBSD 13.1-STABLE #0 stable/13-n252107-eb2a9b78586: Fri Aug 12 02:23:37 UTC 2022
FreeBSD 14.0-CURRENT #0 main-n257314-6a70a0c8bfa: Fri Aug 12 08:33:35 UTC 2022

bin/freebsd-version/freebsd-version.sh.in
73

^12 is wrong though.
What about FreeBSD 11?

But more importantly, the problem here isn't stable vs release. It's reproducible build vs not reproducible build. You should be testing more directly for that in the output rather than using this heuristic that just tests for environments where reproducible build was yes vs no.

bin/freebsd-version/freebsd-version.sh.in
73

hmm. tho the current outcome is reproducible, now I am confused; "two" (and a half) questions here:

  • is 11 even considered to be supported?
  • what about MFC? -- is that even reaching out stable/11?
bin/freebsd-version/freebsd-version.sh.in
73

I would like to flag one little thing still; should we've being talking about WITH_REPRODUCIBLE_BUILD.

newvers.sh do produces a kern.version that is used as base for us here to format our verbose output.

https://cgit.freebsd.org/src/tree/sys/conf/newvers.sh#n292

taking the example of a recent FreeBSD CURRENT we have the following:

egypcio@gaz:~ % sysctl -n kern.version
FreeBSD 14.0-CURRENT #11 main-n257521-97be6fced7d: Sat Aug 20 11:07:35 UTC 2022
    root@gaz.localhost.localdomain:/usr/obj/usr/src/amd64.amd64/sys/GENERIC-NODEBUG

egypcio@gaz:~ % sysctl -n kern.version | head -1
FreeBSD 14.0-CURRENT #11 main-n257521-97be6fced7d: Sat Aug 20 11:07:35 UTC 2022

egypcio@gaz:~ % what -qs /boot/kernel/kernel
FreeBSD 14.0-CURRENT #11 main-n257521-97be6fced7d: Sat Aug 20 11:07:35 UTC 2022

based on what the newvers.sh does, sysctl -n kern.version | head -1 is pretty much what one gets by default, should reproducible builds are enabled; and that's exact the same (reproducible) output as we do get from what -qs

any new inputs about it? shall we finally land the patch?

CC: @kevans

bin/freebsd-version/freebsd-version.sh.in
73

I guess my point is that this should be data driven based on the number of fields, not looking for a specific tag like RELEASE which likely is going to be unreliable.

bin/freebsd-version/freebsd-version.sh.in
73

thanks for clarifying that out. much appreciated! I shall take care of it and update the diff ASAP.
if I indeed got it clear, the proposed change would be changing the following conditional check:

-if ( echo "$out" | grep -Eiq '^12|RELEASE' ) ; then
+if [ $(echo "$out" | wc -w) -lt 4 ] ; then

changes since last update:

  1. rebased on top of freebsd/main 671f55828d03;
  2. applied sugestion from @imp, to make output checks more reproducible
This revision now requires review to proceed.Nov 27 2022, 10:30 AM
egypcio marked 2 inline comments as done.
egypcio marked an inline comment as done.
egypcio marked an inline comment as done.
bcr added a subscriber: bcr.

OK from manpages for these latest changes. Thanks for your patience working on this patch!

In D27318#855240, @bcr wrote:

OK from manpages for these latest changes. Thanks for your patience working on this patch!

thank you Benedict. much appreciated!

hello folks,

I will ask this with all due respect:

  • am I touching some sort of "prestige object" here on the FreeBSD base, that a very reserved (closed?) group of people is allowed to handle?
  • should that be the case, please take the crown or lights out of me, "steal" the diff and allow the other developers commit it;

maybe the same kind of situation is targeting https://reviews.freebsd.org/D36241 ?

it's a huge frustration surprise to see these two revisions to improve FreeBSD base tools being "blocked" for more than 2 and a half years (total);
considering that they are simple and small changes/patches, with proper documentation and detailed information added to the src and manpages.

both updates motivated by https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=265594

CC: @rwatson

You seem to want to see some sort of conspiracy in this situation but have you considered the possibility that nobody wants to approve this review simply because _it's bad_?

a) I'm not convinced the change is needed
b) I'm not even sure I understand what it's supposed to do because the description you added to the manual page does not match what the code does
c) The code does not work as intended

also,

d) I'm sorely tempted to remove the -j option which flies in the face of the intent of this script, should never have been added, and would not have been approved if I had been consulted.

finally 2 cents of review here after a long time hiatus! thank you @des

In D27318#910362, @des wrote:

You seem to want to see some sort of conspiracy in this situation but have you considered the possibility that nobody wants to approve this review simply because _it's bad_?

I seek or want to see no conspiracy here; on top of that, the review (and its patched code+documentation) was approved before -- and improved after suggestions/recommendations from those involved.

should you consider it bad like that, bringing the points into discussion is much appreciated;
shall all agree that's just bad, reject it. happy to not reproduce what happened with those "wireguard implementations" that happened in the past :3

if not, we could merge it. others that shared input and approved the review before do see the benefits of this change.

a) I'm not convinced the change is needed

mind to share a little more input here? did you understand the main motivation, at least?

I believe that the tl;dr; here would be: "instead of using uname -KU it will be possible to simply run freebsd-version -v and give new users a clear view of the exact version they are running"

the verbose version information can be used to compare changes tracked by https://docs.freebsd.org/en/books/porters-handbook/versions/

b) I'm not even sure I understand what it's supposed to do because the description you added to the manual page does not match what the code does

how is that not clear enough?
the title of this review is not the same (detailed) information added to the man page, and that text was reviewed and approved already

c) The code does not work as intended

mind to explain how did you test it and how others can reproduce that?

also,

d) I'm sorely tempted to remove the -j option which flies in the face of the intent of this script, should never have been added, and would not have been approved if I had been consulted.

that's a different subject; we are not touching that piece of code here on this review.