Page MenuHomeFreeBSD

Get rid of the QT_FOODIR plist subs
ClosedPublic

Authored by tcberner on Oct 28 2016, 7:32 PM.

Details

Summary

Very few non-Qt-dist ports actually needed to use the plist subs for
QT_BINDIR, QT_LIBDIR and so on. However, as they were exported by bsd.qt.mk,
make makeplist freely replaced every occurence of bin or lib with
said subs.

There is however no straight-forward way to decide whether a sub is sensible or
not without manually looking at the created plist and the projects buildscripts,
followed by thinking about it (does it actually install to `where the Qt
binaries go`, or just bin/).

Therefore simply get rid of all the QT_FOODIR subs.

Note1: Maybe this is too aggressive, and we could only rid ourselves of
QT_BINDIR and QT_LIBDIR as they are the most egregiously misused ones.
But, I think ripping them all out at once is the cleanest way to handle this.

Note2: As there are no changes apart from plist rewrites, I don't think any version
needs to be bumped.

Note3: It could be, that some ports (or slaveports) that have QT4&QT5 options
and used %%QT_FOO%% are broken by this -- they need to define the PLIST_SUB
now manually, like multimedia/gstreamer1-qt[45] and math/vtk6.

[diff created using 'svn diff --diff-cmd diff -x -U99999', as arc choked on it]

Test Plan
  • This of course will require an exp-run :)
  • I test built all affected ports -- I did however not chase options.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 6088
Build 6350: arc lint + arc unit

Event Timeline

tcberner updated this revision to Diff 21781.Oct 28 2016, 7:32 PM
tcberner retitled this revision from to Get rid of the QT_FOODIR plist subs.
tcberner updated this object.
tcberner edited the test plan for this revision. (Show Details)
tcberner added reviewers: kde, mat, rakuco.
tcberner set the repository for this revision to rP FreeBSD ports repository.
tcberner added a subscriber: adridg.

As a "lighter" version of this diff, we could keep exporting the QT_FOODIR subs whenever QT_DIST is defined. That way the qt[45]-ports would not be affected by this, and the diff would be more managable.

mat edited edge metadata.Oct 28 2016, 11:05 PM

Mmmm, I was only talking about removing QT_BINDIR and QT_LIBDIR, because both are way too short. The other are fine.

Misunderstandings are fun, aren't they :D

In that case, the whole thing is moot.

With qt-5.7 I plan to move the qt5 libraries to lib/qt5. QT_LIBDIR therefore becomes lib/qt5 and QT_BINDIR for qt4 and qt5 will become ${QT_LIBDIR}/bin [or maybe bin/${_QT_RELMAME}].

So they won't get plist-subbed in anymore then if not actually needed anyway.

mat added a comment.Oct 29 2016, 9:12 AM

I don't quite like having QT_BINDIR not being in PREFIX/bin, it means its content will not be in PATH, and not readily accessible. (unless you add symlinks to PREFIX/bin, in which case I don't really see the point of them being some place else anyway.)

misc/qtchooser will be used which installs to ${LOCALBASE}/bin and selects the proper qmake /uic/moc/foo depending on the environment.

mat added a comment.Oct 30 2016, 9:54 AM

So, this means people working on QT apps, when they want to build with qt4 or qt5, they'll have to sudo and run some software ?

misc/qtchooser installs wrapper binaries that select the correct non-wrapped binary depending on the value of QT_SELECT (which has to be set to an installed qt-profile [1]).

% env QT_SELECT=qt4 qmake -v
QMake version 2.01a
Using Qt version 4.8.7 in /usr/local/lib/qt4

% env QT_SELECT=qt5 qmake -v
QMake version 3.0
Using Qt version 5.7.1 in /usr/local/lib/qt5

This means with that change, users could set their default QT_SELECT in their shell-profile, and qmake would simply give them the correct one. Unlike now, where you have to decide whether you want /usr/local/bin/qmake-qt4 or /usr/local/lib/qt5/bin/qmake.

For the ports, bsd.qt.mk/qt.mk will just choose the correct profile.

This also allows people that work on Qt itself to have multiple version of Qt installed simultaneously and build against the proper one by providing a new qt-foo profile [1].

[1] The profiles currently installed are

% cat /usr/local/etc/xdg/qtchooser/qt4.conf
/usr/local/lib/qt4/bin
/usr/local/lib/qt4
% cat /usr/local/etc/xdg/qtchooser/qt5.conf
/usr/local/lib/qt5/bin
/usr/local/lib/qt5

A person working on Qt could simply add a profile ala

% cat qt58.conf 
/home/Tobias/qt-devel/qt-5.8/bin
/home/Tobias/qt-devel/qt-5.8/lib

and use it via env QT_SELECT=qt58 qmake.

tcberner updated this revision to Diff 22068.Nov 8 2016, 1:13 AM
tcberner edited the test plan for this revision. (Show Details)
tcberner edited edge metadata.

Let's do this completely minimal...

Only export QT_BINDIR and QT_LIBDIR if
QT_DIST is set.

Thereby all the Qt-ports themselves are unaffected by
this change, and non Qt-ports will no longer pick them
up by mistake.

rakuco accepted this revision.Nov 26 2016, 1:54 PM
rakuco edited edge metadata.

Looks fine to me, but I'd drop the whitespace changes in bsd.qt.mk. They make the patch bigger, the history more difficult to navigate through and moving to Uses/qt.mk will make these changes short-lived.

mat added a comment.Nov 28 2016, 3:36 PM

Yes, drop the whitespace change, you will be able to use Tools/scripts/indent_make_if.pl when moving to USES=qt.

tcberner updated this revision to Diff 22579.Nov 28 2016, 5:08 PM
tcberner edited edge metadata.

Get rid of the changes made by Tools/scripts/indent_make_if.pl .

Owners edited edge metadata.Nov 28 2016, 5:08 PM
rakuco accepted this revision.Nov 28 2016, 8:32 PM
rakuco edited edge metadata.

lgtm!

tcberner updated this revision to Diff 22589.Nov 28 2016, 9:26 PM
tcberner edited edge metadata.

An other grep for QT_BINDIR and QT_LIBDIR brought some new wrong uses.
They are fixed now too.

Owners edited edge metadata.Nov 28 2016, 9:26 PM
mat accepted this revision.Nov 28 2016, 9:56 PM
mat edited edge metadata.

Macro shipit:

This revision is now accepted and ready to land.Nov 28 2016, 9:56 PM
This revision was automatically updated to reflect the committed changes.