Page MenuHomeFreeBSD

os-release: draft of os-release port
ClosedPublic

Authored by tcberner on Apr 9 2019, 6:31 PM.

Details

Summary

Add port sysutils/etc_os-release which installs ${LOCALBASE}/etc/os-release [1].
The patch also includes a patch to devel/qt5-core to read that file for operating system
version information instead of the default /etc/os-release as used on Linux.

Similar patches can then also be applied to sysutils/plasma5-kinfocenter to essentially remove
sysutils/plasma5-kinfocenter/files/patch-Modules_about-distro_src_OSRelease.cpp.

[1] https://www.freedesktop.org/software/systemd/man/os-release.html

Test Plan
#include <QString>
#include <QSysInfo>
#include <QDebug>
int main()
{
        qDebug() << QSysInfo::buildCpuArchitecture();
        qDebug() << QSysInfo::currentCpuArchitecture();
        qDebug() << QSysInfo::kernelType();
        qDebug() << QSysInfo::kernelVersion();
        qDebug() << QSysInfo::machineHostName();
        qDebug() << QSysInfo::prettyProductName();
        qDebug() << QSysInfo::productType();
        qDebug() << QSysInfo::productVersion();
}

should now return

./a.out                                                                     
"x86_64"
"x86_64"
"freebsd"
"13.0-CURRENT"
"angua.firefly"
"FreeBSD 13.0-CURRENT"
"freebsd"
"13.0"

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

arrowd added inline comments.
devel/qt5-core/Makefile
55 ↗(On Diff #56020)

Why isn't it in post-patch?

sysutils/etc_os-release/Makefile
27 ↗(On Diff #56020)

Is explicit dependency on apply_slist really required here?

28 ↗(On Diff #56020)

Can you please split this into two commands like we normally do?

sysutils/etc_os-release/files/os-release.in
9 ↗(On Diff #56020)

Actually, given the simplicity of this file, you could have simply echoed its contents directly, without any SUB_LIST dances.

sysutils/etc_os-release/pkg-descr
1 ↗(On Diff #56020)

Please stick to plain ASCII when writing port descriptions. Also, this first line contain bogus capitalization and is actually useless since it merely repeats COMMENT. Something like this would be fine:

This package installs /usr/local/etc/os-release file which contains version and other information about installed operating system.

WWW: ...

Is it better to add /etc/os-version to base instead?
I occasionally see people with finger memory from Debian/RHEL, etc. try to check this file on FreeBSD.

Is it better to add /etc/os-version to base instead?

No, even /usr/local/etc/os-version kind of sucks, but I guess it's the least of evil.

I occasionally see people with finger memory from Debian/RHEL, etc. try to check this file on FreeBSD.

That's because GNU/Linux systems are distributions of mixed software, so uname(1) does not work for them very well, as it displays only kernel version. FreeBSD is a normal, complete POSIX operating system, so people should learn to use standard tools to obtain its version information, that is, uname(1) on the command line/scripts, and uname(3) in the C code.

  • do not use SUB_FILES
  • update pkg-descr
mikael added inline comments.
sysutils/etc_os-release/pkg-descr
1 ↗(On Diff #56178)

/usr/local is hardcoded, is it allowed in pkg-descr?

sysutils/etc_os-release/pkg-descr
1 ↗(On Diff #56178)

How about ${PREFIX}/etc/os-release.

mat added inline comments.
sysutils/etc_os-release/Makefile
10–12 ↗(On Diff #56178)

Wrong place in the Makefile. See Chapter 15. Order of Variables in Port Makefiles.

  • os-release: update pkg-descr
  • os-release: Fix makefile variable order
sysutils/etc_os-release/Makefile
20 ↗(On Diff #56377)

Can this file contain comments? Would it make sense to include top $FreeBSD$ indent string, if possible?

Hmm, then again, it might not be easy because of the stupid Git we're forced to comply against. :-(

32 ↗(On Diff #56377)

Why the .for loop instead of simple one-liner @${ECHO_CMD} ${CONTENTS} | ${XARGS} -n 1 > ...? You might also want to mute ${MKDIR}.

sysutils/etc_os-release/pkg-descr
1 ↗(On Diff #56178)

It'd say it's fine to hard-code /usr/local in a port description (we seldom do this, if ever), but I won't firmly insist.

sysutils/etc_os-release/pkg-descr
1 ↗(On Diff #56178)

Grrr, I mean, we seldom use ${PREFIX} (or any other variables) in a pkg-descr files, sorry. Why can't I edit inline comment I wonder...

sysutils/etc_os-release/Makefile
20 ↗(On Diff #56377)

Yes, it understands comments, beginning with #. Lines beginning with "#" shall be ignored as comments. Blank lines are permitted and ignored.

Though, I'm not sure how to get $FreeBSD$ working in a file that is not being committed, but generated on the fly when building.

  • os-release: simplify file creation using xargs
  • os-release: use post-patch target
devel/qt5-core/Makefile
55 ↗(On Diff #56020)

No good reason :) -- I'll change it.

  • os-release: fix quoting with xargs
This revision is now accepted and ready to land.Apr 25 2019, 8:48 PM

LGTM, works with gnome-control-center.

This revision was automatically updated to reflect the committed changes.