Page MenuHomeFreeBSD

Create /etc/os-release file.
AcceptedPublic

Authored by imp on Thu, Nov 7, 5:10 AM.

Details

Summary

Each boot, regenerate /var/run/os-release based on the currently running
system. Create a /etc/os-release symlink pointing to this file (so that this
doesn't create a new reason /etc can not be mounted read-only).

This is compatible with what other systems do and is what the sysutil/os-release
port attempted to do, but in an incomplete way. Linux, Solaris and DragonFly all
implement this natively as well. The complete standard can be found at
https://www.freedesktop.org/software/systemd/man/os-release.html

Moving this to the base solves both the non-standard location problem with the
port, as well as the lack of update of this file on system update.

PR: 238953

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 27403
Build 25650: arc lint + arc unit

Event Timeline

imp created this revision.Thu, Nov 7, 5:10 AM
imp edited the summary of this revision. (Show Details)Thu, Nov 7, 6:10 PM
zeising accepted this revision.Thu, Nov 7, 6:38 PM
This revision is now accepted and ready to land.Thu, Nov 7, 6:38 PM
brooks added a subscriber: brooks.Thu, Nov 7, 6:42 PM
brooks added inline comments.
libexec/rc/rc.d/os-release
24

I'd use

VERSION_ID=${VERSION%%[^0-9.]*}
35

I'd tend to save a subshell and a bunch of quotes with a heardoc.

cem accepted this revision.Thu, Nov 7, 6:54 PM
cem added a subscriber: cem.

Any reason to avoid generating to a temporary file first (possibly in tmpfs) and installing with install -C (i.e., don't re-write or bump mtime if the contents did not change)? Slightly more friendly to low-endurance media /var devices.

I support the general idea and have no objection to the implementation.

libexec/rc/rc.d/os-release
34

Suggest adding a canonical trailing "/" to BUG_REPORT_URL.

(Used to be "/bugzilla/" was required, but it seems we have a working redirect now, so "/" is fine.)

imp updated this revision to Diff 64062.Thu, Nov 7, 9:25 PM

update per cem and brooks

This revision now requires review to proceed.Thu, Nov 7, 9:25 PM
cem accepted this revision.Thu, Nov 7, 9:34 PM
This revision is now accepted and ready to land.Thu, Nov 7, 9:34 PM
rpokala added a subscriber: rpokala.Thu, Nov 7, 9:36 PM

and is what the
sysutil/os-release port.

E_GRAMMAR

tcberner accepted this revision.Thu, Nov 7, 9:50 PM

Thank you.

brooks accepted this revision.Thu, Nov 7, 10:20 PM
cy accepted this revision.Fri, Nov 8, 6:05 AM
se added a subscriber: se.Fri, Nov 8, 6:15 AM
se added inline comments.
libexec/rc/rc.d/os-release
14

The value of $desc could take a user-defined setting of $osrelease_file into account if moved down 4 lines (or if setting default values for osrelease_file and os_release_perms was moved up to above name=...)

imp marked 3 inline comments as done.Fri, Nov 8, 6:40 AM
imp added inline comments.
libexec/rc/rc.d/os-release
14

I'll include that in the next update. Thanks

bapt accepted this revision.Fri, Nov 8, 8:40 AM
zeising accepted this revision.Fri, Nov 8, 8:44 AM
bapt added a comment.Fri, Nov 8, 9:55 AM

To be clear, I am validating the content of the file and the principle of a rc.d. Now I am wondering it is won't be better served by a port/package as it has a niche usage anyway (for now at least), and that will make it cover all supported version of FreeBSD straight away.

It will still be possible to move it to base later if we have a real need for it in base in the futur

manu added a subscriber: manu.Fri, Nov 8, 10:04 AM

Does that needs a exp-run too ?
I guess that some ports currently test if this file exists at configure step and don't depend on the current port so this might affect them.
I'd still like to know what ports currently need this too (at least the main one).

In D22271#487097, @manu wrote:

Does that needs a exp-run too ?
I guess that some ports currently test if this file exists at configure step and don't depend on the current port so this might affect them.
I'd still like to know what ports currently need this too (at least the main one).

At the moment three ports explicitely use it:

  • gnome uses it in sysutils/gnome-control-center
  • every Qt5 application can use its information as it's available through devel/qt5-core and further in devel/kf5-kcoreaddons [2], [3], so for example kinfocenter uses the information provided.

I'm not aware of it being checked at configure stage by anything -- if this were the case however, it would be even more important for it to be in base :)

[1] https://github.com/GNOME/gnome-control-center/blob/1752cf3bb573d300e990ea10006dea70a322d8df/panels/common/cc-os-release.c#L45
[2] https://github.com/qt/qtbase/blob/dev/src/corelib/global/qglobal.cpp#L2270
[3] https://github.com/KDE/kcoreaddons/blob/master/src/lib/util/kosrelease.cpp#L87

manu added a comment.Fri, Nov 8, 4:13 PM
In D22271#487097, @manu wrote:

Does that needs a exp-run too ?
I guess that some ports currently test if this file exists at configure step and don't depend on the current port so this might affect them.
I'd still like to know what ports currently need this too (at least the main one).

At the moment three ports explicitely use it:

  • gnome uses it in sysutils/gnome-control-center
  • every Qt5 application can use its information as it's available through devel/qt5-core and further in devel/kf5-kcoreaddons [2], [3], so for example kinfocenter uses the information provided.

I'm not aware of it being checked at configure stage by anything -- if this were the case however, it would be even more important for it to be in base :)
[1] https://github.com/GNOME/gnome-control-center/blob/1752cf3bb573d300e990ea10006dea70a322d8df/panels/common/cc-os-release.c#L45
[2] https://github.com/qt/qtbase/blob/dev/src/corelib/global/qglobal.cpp#L2270
[3] https://github.com/KDE/kcoreaddons/blob/master/src/lib/util/kosrelease.cpp#L87

Thanks. So I guess that my next question is : how does this files affects those program ?
It is just cosmetic ? (like they will display FreeBSD instead of the default "Linux" string) or something else ?

emaste added a subscriber: emaste.Fri, Nov 8, 4:27 PM
imp added a comment.Fri, Nov 8, 5:13 PM
I'm not aware of it being checked at configure stage by anything -- if this were the case however, it would be even more important for it to be in base :)

clang uses it for various things (like directory paths if I'm reading the code correctly), though only for the Linux drivers...

imp added a comment.Fri, Nov 8, 5:16 PM
In D22271#487094, @bapt wrote:

To be clear, I am validating the content of the file and the principle of a rc.d. Now I am wondering it is won't be better served by a port/package as it has a niche usage anyway (for now at least), and that will make it cover all supported version of FreeBSD straight away.
It will still be possible to move it to base later if we have a real need for it in base in the future

It is already a port. The problem is that it's not updated with the base is updated so can be out of date in the information it supplies. The other problem is that the port installs it in a non-standard place and we patch all the programs in ports (that we catch) to use the non-standard path. So, between these two issues, I thought that it was time to migrate it into base, especially in light of DragonflyBSD creating and installing one as part of their installworld. Since we can't hack files in /etc, per long standing tradition, this was the next best way to conform to the standard.

imp added a comment.Fri, Nov 8, 5:21 PM
In D22271#487162, @manu wrote:

Thanks. So I guess that my next question is : how does this files affects those program ?
It is just cosmetic ? (like they will display FreeBSD instead of the default "Linux" string) or something else ?

It looks like it will display Linux, at the very least. This is bad for our branding efforts (FreeBSD isn't just a Linux distro). It's unclear if there are downstream users that use this value for other things. It will also not display version information. It makes the FreeBSD versions of programs look less professional and the integration hap-hazard and incomplete.

imp added a comment.EditedFri, Nov 8, 5:31 PM

Also standard in Solaris, per bcran:

Also, it's enough of a standard that it's supported in Solaris 11.4 - https://docs.oracle.com/cd/E88353_01/html/E37852/os-release-5.html

Solaris 11.4 was released in October 2018.

imp edited the summary of this revision. (Show Details)Fri, Nov 8, 5:40 PM
imp updated this revision to Diff 64082.Fri, Nov 8, 5:46 PM

Update per review comments
Update etc/defaults/rc.conf

This revision now requires review to proceed.Fri, Nov 8, 5:46 PM
imp added a comment.Fri, Nov 8, 5:47 PM
In D22271#487097, @manu wrote:

Does that needs a exp-run too ?
I guess that some ports currently test if this file exists at configure step and don't depend on the current port so this might affect them.

It wouldn't hurt, though I'd omit it if we're running short on exp-run resources.

trasz added a subscriber: trasz.Fri, Nov 8, 7:41 PM
trasz added inline comments.
libexec/rc/rc.conf
681

Uppercase the 'Default'?

libexec/rc/rc.d/os-release
35

Should we perhaps follow the usual way of spelling our URL as FreeBSD.org?

jhibbits accepted this revision.Fri, Nov 8, 7:47 PM
This revision is now accepted and ready to land.Fri, Nov 8, 7:47 PM
vangyzen accepted this revision.Fri, Nov 8, 7:53 PM
vangyzen added a subscriber: vangyzen.

It would be nice to include some documentation. The systemd man page seems most useful and canonical, but it's LGPL. Maybe include a link to it in a comment in the generated file?

libexec/rc/rc.d/os-release
25

s/motd/os-release/ ?

imp updated this revision to Diff 64089.Fri, Nov 8, 7:56 PM

More review commentary addressed.

This revision now requires review to proceed.Fri, Nov 8, 7:56 PM
trasz accepted this revision.Fri, Nov 8, 8:00 PM
This revision is now accepted and ready to land.Fri, Nov 8, 8:00 PM
imp updated this revision to Diff 64090.Fri, Nov 8, 9:33 PM

Now with freshly-written man page.

This revision now requires review to proceed.Fri, Nov 8, 9:33 PM
imp added a comment.Fri, Nov 8, 9:37 PM

It would be nice to include some documentation. The systemd man page seems most useful and canonical, but it's LGPL. Maybe include a link to it in a comment in the generated file?

I wrote a man page instead :) It summarizes the standard.

zeising accepted this revision.Fri, Nov 8, 9:45 PM
zeising added a subscriber: bcr.

Perhaps @bcr can have a quick look at the man page?

This revision is now accepted and ready to land.Fri, Nov 8, 9:45 PM

At the moment three ports explicitely use it:

  • gnome uses it in sysutils/gnome-control-center

GLib will have a new API called g_get_os_info to read /etc/os-release in the next stable release: https://gitlab.gnome.org/GNOME/glib/merge_requests/1063. gnome-control-center is likely switch to it because the API was proposed and implemented by a gnome-control-center maintainer. It is expected to see more and more GNOME applications using it because nearly all GNOME applications use GLib. I wrote a uname fallback for the function: https://gitlab.gnome.org/GNOME/glib/merge_requests/1165, so the situation isn't too bad on FreeBSD. At least it already reports correct name and version of FreeBSD kernel. It is nice to know that FreeBSD will have /etc/os-release and the fallback will not be needed.

bcr accepted this revision.Sat, Nov 9, 10:33 AM

Man page looks good to me.

vangyzen accepted this revision.Sat, Nov 9, 8:52 PM

Thanks for the man page!

sef accepted this revision.Sun, Nov 10, 6:16 AM
sef added a subscriber: sef.

My only concern involves the /etc symlink possibly pointing at a non-existent file. This is a small concern not worth preventing this.

rpokala accepted this revision.Sun, Nov 10, 8:06 AM
0mp added a subscriber: 0mp.Mon, Nov 11, 3:42 PM
0mp added inline comments.
libexec/rc/rc.d/os-release
22

It is just a minor thing, but shouldn't we use local variables here like local _version to match other rc scripts?

share/man/man5/os-release.5
123

Wouldn't it be nice if we use the .Lk macros for links?