Details

Reviewers

mat
adamw
rakuco
tcberner

Group Reviewers

portmgr
kde
O5: Ports Framework	(Owns No Changed Paths)

Commits

rP473503: Replace bsd.qt.mk by Uses/qt.mk and Uses/qt-dist.mk

Summary

bsd.qt.mk did two things:

it provides USE_Qt4 and USE_QT5 copmonents to be used by ports depending on Qt
it handles the business of building the Qt4 and Qt5 ports

Further there is qmake.mk which is weirdly interconnected with it.

This diff does the following:

replace the dependency parts of bsd.qt.mk by Uses/qt.mk
replace the Qt-building parts of bsd.qt.mk by Uses/qt-dist.mk

As opposed to the last try (see D8540) this time, keep qmake.mk around.

The changes to the ports tree are the following

Ports depending on Qt4:

USE_QT4=    foo bar

becomes

USES=       qt:4
USE_QT=     foo bar

Ports depending on Qt5:

USE_QT5=    foo bar

becomes

USES=       qt:5
USE_QT=     foo bar

For USES=qmake ports

qmake.mk now accepts a Qt version, so ports only depending on qmake, and on no Qt-component can specify

USES=       qmake:4

USES=       qmake:5

respectively.

For ports (misusing) QT_NONSTANDARD:

QT_NONSTANDARD has been deprecated, and is replaced by the new qmake args

no_env: Don't add CONFIGURE_ARGS and CONFIGURE_ENV
no_configure: Don't add the qmake configure target

For the Qt-dist ports:

Instead of setting
For qt:4

QT_DIST=    yes

and for qt:5

QT_DIST=   foo

now

USES=qt-dist:yes

USES=qt-dist:4,yes

respectively

USES=qt-dist:foo

USES=qt-dist:5,foo

There is a fallback to uses yes (for qt:4) and ${PORTNAME} (for qt:5).

All in all, the ports tree, and how ports use Qt is only affected minimally -- far less than the previous attempt :)

Test Plan

Exp-Run

Diff Detail

Repository

rP FreeBSD ports repository

Lint

No Lint Coverage

Unit

No Test Coverage

Build Status

Buildable 16973
Build 16842: arc lint + arc unit

Event Timeline

tcberner created this revision.May 23 2018, 9:04 PM

Herald added a reviewer: portmgr. · View Herald TranscriptMay 23 2018, 9:05 PM

Herald added subscribers: Contributor Reviewers (ports), mat. · View Herald Transcript

tcberner added reviewers: kde, mat, adamw, rakuco, adridg.May 23 2018, 9:06 PM

Mmmm, phabricator does not seem to want to show the files here 😑
Could you maybe only add the Mk changes to the review, as all the ports changes seem to be pretty mechanical.

Just the good bits.

Owners added a reviewer: O5: Ports Framework.May 24 2018, 7:58 AM

Harbormaster completed remote builds in B16805: Diff 42918.May 24 2018, 7:58 AM

I may have missed it, but USE_QT4 and USE_QT5 should be added to the unsupported list in bsd.sanity.mk.

Mk/Uses/qmake.mk
119	I may have missed it, but I did not see where there was something making sure qmake was included before qt.

Append to bsd.sanity.mk

Harbormaster completed remote builds in B16917: Diff 43154.May 30 2018, 5:56 PM

Mk/Uses/qmake.mk
119	Would a match ala .if empty(USES:Mqt.*qmake) IGNORE=qmake must appear before qt in USES .endif work?

mat added inline comments.May 30 2018, 8:18 PM

Mk/Uses/qmake.mk
50–53	That should not be needed, qmake_ARGS should always be defined, unless this is because we can come here from USES=qt-dist.
119	No, it has to work whatever the order they are added. You'll have to include qt.mk after ading to USE_QT. I see that qt.mk is already included early here, so adding to USE_QT doesn't, does it? Or maybe it is handled in POST so it does work.
Mk/Uses/qt-dist.mk
63	At this point, `PORTSDIR` should always be defined.
107–113	You know tar_ARGS= xz .include "${USESDIR}/tar.mk" could be written as: EXTRACT_SUFX?= .tar.xz And it would probably be faster, one less file to open and parse.
Mk/bsd.sanity.mk
257–258	You could probably write `USE_QT=${USE_QT4}` / `USE_QT=${USE_QT5}` to help a bit more.

tcberner added inline comments.Jun 1 2018, 5:41 AM

Mk/Uses/qmake.mk
119	I think I added the POST stuff because of this, but I have to check whether it works in any order already.

mat added inline comments.Jun 1 2018, 10:14 AM

Mk/Uses/qmake.mk
119	Please do test, it works whatever the order USES is set. (There is a sort police that likes to have it sorted, but the order is really undefined.)

Address some of the concerns.

Mk/Uses/qmake.mk
50–53	Exactly, this is needed for ports like qmake with: USES= qt-dist:4
Mk/Uses/qt-dist.mk
63	I don't quite understand what you mean here, sorry :)
63	? :)

Harbormaster completed remote builds in B16972: Diff 43264.Jun 2 2018, 7:19 AM

tcberner added inline comments.Jun 2 2018, 7:37 AM

Mk/Uses/qmake.mk
119	# make -VUSES -VUSE_QT -Cdevel/qbs compiler:c++11-lib python:run qt:5 qmake:outsource shebangfix buildtools_build core concurrent gui network script testlib widgets xml qmake_build seems to work \o/

Add QT_NONSTANDARD to bsd.sanity.mk

Harbormaster completed remote builds in B16973: Diff 43265.Jun 2 2018, 7:52 AM

mat added inline comments.Jun 2 2018, 9:10 AM

Mk/Uses/qmake.mk
50–53	I think qt-dist should do a `qt_ARGS?=` so that qt is not special like that in any way
Mk/Uses/qt-dist.mk
63	The only reason we use `.CURDIR:H:H` in ports to reference `PORTSDIR` is because bsd.port.mk has not yet been included, and `PORTSDIR` is not yet defined. Here, we are in a USES, and `PORTSDIR` is always defined, it should be used instead of the obscure `.CURDIR:H:H`.
Mk/bsd.sanity.mk
21	This should probably be in the SANITY_UNSUPORTED, not here. The ALL_* variables are for user facing variables, stuff you put in make.conf, for example. I don't think QT_NONSTANDARD is in that category.

QT_NONSTANDARD in sanity.m
qt_ARGS and qmake_ARGS in qt-dist.mk
PORTSDIR instead of CURDIR:H:H

Harbormaster completed remote builds in B16975: Diff 43267.Jun 2 2018, 10:35 AM

tcberner added inline comments.Jun 2 2018, 10:37 AM

Mk/bsd.sanity.mk
22	woops, stray empty line...

@mat should I ask for an exp-run now?

Yes, I think it is ok now, at least I do not have any more ideas on how to improve it :-)

exp-run

Committed in rP473503

Commandeering it so the review can be subsequently abandoned / closed -- since it was committed some time ago.

.. was committed r473503

Diff 43265

View Options

Mk/Scripts/qa.sh

Show First 20 Lines • Show All 215 Lines • ▼ Show 20 Lines	procfile(const char *fn)
for (c = 0; c == 0 \|\| !(lflag \|\| qflag); ) {		for (c = 0; c == 0 \|\| !(lflag \|\| qflag); ) {
ln.off += ln.len + 1;		ln.off += ln.len + 1;
if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL \|\| ln.len == 0) {		if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL \|\| ln.len == 0) {
if (ln.line_no == 0 && matchall)		if (ln.line_no == 0 && matchall)
exit(0);		exit(0);
else		else
break;		break;
}		}
if (ln.len > 0 && ln.dat[ln.len - 1] == '\n')		if (ln.len > 0 && ln.dat[ln.len - 1] == fileeol)
--ln.len;		--ln.len;
ln.line_no++;		ln.line_no++;

/* Return if we need to skip a binary file */		/* Return if we need to skip a binary file */
if (f->binary && binbehave == BINFILE_SKIP) {		if (f->binary && binbehave == BINFILE_SKIP) {
grep_close(f);		grep_close(f);
free(ln.file);		free(ln.file);
free(f);		free(f);
▲ Show 20 Lines • Show All 254 Lines • ▼ Show 20 Lines	if ((oflag \|\| color) && m > 0) {
}		}
if (!oflag) {		if (!oflag) {
if (line->len - a > 0)		if (line->len - a > 0)
fwrite(line->dat + a, line->len - a, 1, stdout);		fwrite(line->dat + a, line->len - a, 1, stdout);
putchar('\n');		putchar('\n');
}		}
} else {		} else {
fwrite(line->dat, line->len, 1, stdout);		fwrite(line->dat, line->len, 1, stdout);
putchar('\n');		putchar(fileeol);
		cemUnsubmitted Not Done Inline Actions Hm, should the nul input separater really be used for outputting matched lines? And if so, shouldn't it also be used with `oflag` above? cem: Hm, should the nul input separater really be used for outputting matched lines? And if so…
		kevansAuthorUnsubmitted Not Done Inline Actions So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful to the input file -- i.e., this line, and actually line 491 that I missed (`!oflag`, but `color`). Everything else is printing excerpts and should use the standard `grep` newline separator. kevans: So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful…
}		}
}		}

View Options

Mk/Uses/qmake.mk

Show First 20 Lines • Show All 215 Lines • ▼ Show 20 Lines	procfile(const char *fn)
for (c = 0; c == 0 \|\| !(lflag \|\| qflag); ) {		for (c = 0; c == 0 \|\| !(lflag \|\| qflag); ) {
ln.off += ln.len + 1;		ln.off += ln.len + 1;
if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL \|\| ln.len == 0) {		if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL \|\| ln.len == 0) {
if (ln.line_no == 0 && matchall)		if (ln.line_no == 0 && matchall)
exit(0);		exit(0);
else		else
break;		break;
}		}
if (ln.len > 0 && ln.dat[ln.len - 1] == '\n')		if (ln.len > 0 && ln.dat[ln.len - 1] == fileeol)
--ln.len;		--ln.len;
ln.line_no++;		ln.line_no++;

/* Return if we need to skip a binary file */		/* Return if we need to skip a binary file */
if (f->binary && binbehave == BINFILE_SKIP) {		if (f->binary && binbehave == BINFILE_SKIP) {
grep_close(f);		grep_close(f);
free(ln.file);		free(ln.file);
free(f);		free(f);
▲ Show 20 Lines • Show All 254 Lines • ▼ Show 20 Lines	if ((oflag \|\| color) && m > 0) {
}		}
if (!oflag) {		if (!oflag) {
if (line->len - a > 0)		if (line->len - a > 0)
fwrite(line->dat + a, line->len - a, 1, stdout);		fwrite(line->dat + a, line->len - a, 1, stdout);
putchar('\n');		putchar('\n');
}		}
} else {		} else {
fwrite(line->dat, line->len, 1, stdout);		fwrite(line->dat, line->len, 1, stdout);
putchar('\n');		putchar(fileeol);
		cemUnsubmitted Not Done Inline Actions Hm, should the nul input separater really be used for outputting matched lines? And if so, shouldn't it also be used with `oflag` above? cem: Hm, should the nul input separater really be used for outputting matched lines? And if so…
		kevansAuthorUnsubmitted Not Done Inline Actions So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful to the input file -- i.e., this line, and actually line 491 that I missed (`!oflag`, but `color`). Everything else is printing excerpts and should use the standard `grep` newline separator. kevans: So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful…
}		}
}		}

View Options

Mk/Uses/qt-dist.mk

Show First 20 Lines • Show All 215 Lines • ▼ Show 20 Lines	procfile(const char *fn)
for (c = 0; c == 0 \|\| !(lflag \|\| qflag); ) {		for (c = 0; c == 0 \|\| !(lflag \|\| qflag); ) {
ln.off += ln.len + 1;		ln.off += ln.len + 1;
if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL \|\| ln.len == 0) {		if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL \|\| ln.len == 0) {
if (ln.line_no == 0 && matchall)		if (ln.line_no == 0 && matchall)
exit(0);		exit(0);
else		else
break;		break;
}		}
if (ln.len > 0 && ln.dat[ln.len - 1] == '\n')		if (ln.len > 0 && ln.dat[ln.len - 1] == fileeol)
--ln.len;		--ln.len;
ln.line_no++;		ln.line_no++;

/* Return if we need to skip a binary file */		/* Return if we need to skip a binary file */
if (f->binary && binbehave == BINFILE_SKIP) {		if (f->binary && binbehave == BINFILE_SKIP) {
grep_close(f);		grep_close(f);
free(ln.file);		free(ln.file);
free(f);		free(f);
▲ Show 20 Lines • Show All 254 Lines • ▼ Show 20 Lines	if ((oflag \|\| color) && m > 0) {
}		}
if (!oflag) {		if (!oflag) {
if (line->len - a > 0)		if (line->len - a > 0)
fwrite(line->dat + a, line->len - a, 1, stdout);		fwrite(line->dat + a, line->len - a, 1, stdout);
putchar('\n');		putchar('\n');
}		}
} else {		} else {
fwrite(line->dat, line->len, 1, stdout);		fwrite(line->dat, line->len, 1, stdout);
putchar('\n');		putchar(fileeol);
		cemUnsubmitted Not Done Inline Actions Hm, should the nul input separater really be used for outputting matched lines? And if so, shouldn't it also be used with `oflag` above? cem: Hm, should the nul input separater really be used for outputting matched lines? And if so…
		kevansAuthorUnsubmitted Not Done Inline Actions So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful to the input file -- i.e., this line, and actually line 491 that I missed (`!oflag`, but `color`). Everything else is printing excerpts and should use the standard `grep` newline separator. kevans: So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful…
}		}
}		}

View Options

Mk/Uses/qt.mk

Show First 20 Lines • Show All 215 Lines • ▼ Show 20 Lines	procfile(const char *fn)
for (c = 0; c == 0 \|\| !(lflag \|\| qflag); ) {		for (c = 0; c == 0 \|\| !(lflag \|\| qflag); ) {
ln.off += ln.len + 1;		ln.off += ln.len + 1;
if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL \|\| ln.len == 0) {		if ((ln.dat = grep_fgetln(f, &ln.len)) == NULL \|\| ln.len == 0) {
if (ln.line_no == 0 && matchall)		if (ln.line_no == 0 && matchall)
exit(0);		exit(0);
else		else
break;		break;
}		}
if (ln.len > 0 && ln.dat[ln.len - 1] == '\n')		if (ln.len > 0 && ln.dat[ln.len - 1] == fileeol)
--ln.len;		--ln.len;
ln.line_no++;		ln.line_no++;

/* Return if we need to skip a binary file */		/* Return if we need to skip a binary file */
if (f->binary && binbehave == BINFILE_SKIP) {		if (f->binary && binbehave == BINFILE_SKIP) {
grep_close(f);		grep_close(f);
free(ln.file);		free(ln.file);
free(f);		free(f);
▲ Show 20 Lines • Show All 254 Lines • ▼ Show 20 Lines	if ((oflag \|\| color) && m > 0) {
}		}
if (!oflag) {		if (!oflag) {
if (line->len - a > 0)		if (line->len - a > 0)
fwrite(line->dat + a, line->len - a, 1, stdout);		fwrite(line->dat + a, line->len - a, 1, stdout);
putchar('\n');		putchar('\n');
}		}
} else {		} else {
fwrite(line->dat, line->len, 1, stdout);		fwrite(line->dat, line->len, 1, stdout);
putchar('\n');		putchar(fileeol);
		cemUnsubmitted Not Done Inline Actions Hm, should the nul input separater really be used for outputting matched lines? And if so, shouldn't it also be used with `oflag` above? cem: Hm, should the nul input separater really be used for outputting matched lines? And if so…
		kevansAuthorUnsubmitted Not Done Inline Actions So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful to the input file -- i.e., this line, and actually line 491 that I missed (`!oflag`, but `color`). Everything else is printing excerpts and should use the standard `grep` newline separator. kevans: So, it depends. If we're supposed to be printing the matched lines, we need to do so faithful…
}		}
}		}

View Options

Mk/bsd.port.mk

View Options

Mk/bsd.qt.mk

View Options

Replace bsd.qt.mk by qt.mk and qt-dist.mk
AbandonedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 43265

Mk/Scripts/qa.sh

Mk/Uses/qmake.mk

Mk/Uses/qt-dist.mk

Mk/Uses/qt.mk

Mk/bsd.port.mk

Mk/bsd.qt.mk

Mk/bsd.sanity.mk

Replace bsd.qt.mk by qt.mk and qt-dist.mkAbandonedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 43265

Mk/Scripts/qa.sh

Mk/Uses/qmake.mk

Mk/Uses/qt-dist.mk

Mk/Uses/qt.mk

Mk/bsd.port.mk

Mk/bsd.qt.mk

Mk/bsd.sanity.mk

Replace bsd.qt.mk by qt.mk and qt-dist.mk
AbandonedPublic
Actions

Revision Contents
Changeset List