Details

Reviewers

danfe
kwm
bapt
adridg

Group Reviewers

kde
portmgr

Commits

rP500364: devel/qt5-core: depend on newly added sysutils/etc_os-release
rP500363: New port: sysutils/etc_os-release

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

Lint

No Lint Coverage

Unit

No Test Coverage

Build Status

Buildable 23770
Build 22719: arc lint + arc unit

Event Timeline

tcberner created this revision.Apr 9 2019, 6:31 PM

Harbormaster completed remote builds in B23602: Diff 56020.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
59	Why isn't it in `post-patch`?

danfe added inline comments.Apr 10 2019, 8:42 AM

sysutils/etc_os-release/Makefile
28	Is explicit dependency on `apply_slist` really required here?
29	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
2	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.

In D19866#426988, @lifanov wrote:

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

Harbormaster completed remote builds in B23660: Diff 56178.Apr 13 2019, 5:29 PM

mikael added a subscriber: mikael.Apr 13 2019, 5:46 PM

mikael added inline comments.

sysutils/etc_os-release/pkg-descr
2	/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
2	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
11–13	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

Harbormaster completed remote builds in B23762: Diff 56377.Apr 19 2019, 7:39 AM

danfe added inline comments.Apr 19 2019, 4:48 PM

sysutils/etc_os-release/Makefile
21	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. :-(
33	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
2	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
2	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
21	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

Harbormaster completed remote builds in B23770: Diff 56393.Apr 19 2019, 5:30 PM

tcberner added inline comments.Apr 19 2019, 5:30 PM