Page MenuHomeFreeBSD

Check for qt depends in LOCALBASE, not PREFIX
ClosedPublic

Authored by adamw on Oct 13 2017, 11:31 PM.

Details

Summary

PREFIX is for where this port gets installed. LOCALBASE is where existing dependencies are checked.

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

adamw created this revision.Oct 13 2017, 11:31 PM
adamw updated this revision to Diff 33976.Oct 14 2017, 6:25 AM

Only change the resulting paths, not the underlying destination dirs.

You might want to compare with D10010 .

adamw added a comment.Oct 14 2017, 5:01 PM

You might want to compare with D10010 .

I think that one has the same original problem where instead of using PREFIX incorrectly, now it uses LOCALBASE incorrectly (installs the current stuff into LOCALBASE).

adamw updated this revision to Diff 33985.Oct 14 2017, 5:08 PM

Use the correct path for everything, rather than subbing it at the end (which was fragile for any later groupings).

I'm a bit confused by this. If we install too $FOO/bin but check for presence in $BAR/bin, how is that not broken anyway when ever $FOO != $BAR ?

How about adding

.if empty(${QT_DIST})
QT_PREFIX=${LOCALBASE}
.else
QT_PREFIX=${PREFIX}
.endif

and using ${QT_PREFIX} for the foo_PATH=. This feels slightly more correct to me. But I might be off :)

adamw added a comment.Oct 14 2017, 7:17 PM

I'm a bit confused by this. If we install too $FOO/bin but check for presence in $BAR/bin, how is that not broken anyway when ever $FOO != $BAR ?

PREFIX (where this port is being installed into) can't be assumed to equal LOCALBASE (where previously installed ports exist). Currently, when PREFIX is changed, QT-based ports break.

adamw added a comment.Oct 14 2017, 7:18 PM

How about adding

.if empty(${QT_DIST})
QT_PREFIX=${LOCALBASE}
.else
QT_PREFIX=${PREFIX}
.endif

and using ${QT_PREFIX} for the foo_PATH=. This feels slightly more correct to me. But I might be off :)

I believe ${QT_DIST} is only for ports that are actually QT modules. This patch deals with any port that uses QT modules.

How about adding

.if empty(${QT_DIST})
QT_PREFIX=${LOCALBASE}
.else
QT_PREFIX=${PREFIX}
.endif

and using ${QT_PREFIX} for the foo_PATH=. This feels slightly more correct to me. But I might be off :)

I believe ${QT_DIST} is only for ports that are actually QT modules. This patch deals with any port that uses QT modules.

Exactly, but QT_DIST ports also use Qt modules, and as they are basically one big split up thing, they should look in PREFIX and not LOCALBASE for the stuff. The component paths you modify are not only used by other ports.

A port may expect to find Qt in $LOCALBASE, but a part of Qt (i.e. a $QT_BASE setting port) needs to look for it in $PREFIX as Qt-parts cannot be installed into different paths than other parts.

adamw added a comment.Oct 14 2017, 8:50 PM

Exactly, but QT_DIST ports also use Qt modules, and as they are basically one big split up thing, they should look in PREFIX and not LOCALBASE for the stuff. The component paths you modify are not only used by other ports.
A port may expect to find Qt in $LOCALBASE, but a part of Qt (i.e. a $QT_BASE setting port) needs to look for it in $PREFIX as Qt-parts cannot be installed into different paths than other parts.

When you make install PREFIX=/foo, the ONLY thing that should be in /foo is the current port. Separating $PREFIX and $LOCALBASE isn't about coming up with a usable system. It's about installing this port to a specific location. Maybe it's for testing. Maybe's it's separate packaging. Either way, ports always install into $PREFIX, and always look for dependencies in $LOCALBASE. Dependencies are NEVER supposed to be checked in $PREFIX.

tcberner accepted this revision.Oct 14 2017, 9:11 PM

Ok, this seems to be proper by principle :) So please have it exp-run.

This revision is now accepted and ready to land.Oct 14 2017, 9:11 PM
This revision was automatically updated to reflect the committed changes.