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

tcberner created this revision.Apr 9 2019, 6:31 PM
arrowd added a subscriber: arrowd.Apr 9 2019, 7:12 PM
arrowd added inline comments.
devel/qt5-core/Makefile
55 ↗(On Diff #56020)

Why isn't it in post-patch?

danfe added inline comments.Apr 10 2019, 8:42 AM
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.

danfe added a comment.EditedApr 11 2019, 5:14 PM

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.

tcberner updated this revision to Diff 56178.Apr 13 2019, 5:29 PM
  • do not use SUB_FILES
  • update pkg-descr
mikael.urankar_gmail.com added inline comments.
sysutils/etc_os-release/pkg-descr
1 ↗(On Diff #56178)

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

tcberner added inline comments.Apr 13 2019, 5:52 PM
sysutils/etc_os-release/pkg-descr
1 ↗(On Diff #56178)

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

mat added a subscriber: mat.Apr 18 2019, 12:46 PM
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.

tcberner updated this revision to Diff 56377.Apr 19 2019, 7:39 AM
  • os-release: update pkg-descr
  • os-release: Fix makefile variable order
danfe added inline comments.Apr 19 2019, 4:48 PM
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.

danfe added inline comments.Apr 19 2019, 4:50 PM
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...

tcberner added inline comments.Apr 19 2019, 5:26 PM
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.

tcberner updated this revision to Diff 56393.Apr 19 2019, 5:30 PM
  • os-release: simplify file creation using xargs
  • os-release: use post-patch target
tcberner added inline comments.Apr 19 2019, 5:30 PM
devel/qt5-core/Makefile
55 ↗(On Diff #56020)

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

tcberner updated this revision to Diff 56396.Apr 19 2019, 6:02 PM
  • os-release: fix quoting with xargs
adridg accepted this revision.Apr 25 2019, 8:48 PM
This revision is now accepted and ready to land.Apr 25 2019, 8:48 PM
kwm accepted this revision.Apr 28 2019, 6:22 PM

LGTM, works with gnome-control-center.

This revision was automatically updated to reflect the committed changes.