bsd.port.mk: Switch to USES=gl

[zeising]

Summary

Switch to USES=gl instead of USE_GL=

This is designed such that you need USES=gl, and then specify components as needed with USE_GL= as before. For backwards compatibility, USE_GL= automatically sets USES=gl.

See also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=230692

Test Plan

exp-run

[matthew]

Mk/Uses/gl.mk

6 ↗

(On Diff #46847)

Spelling: 'component'

[matthew]

Mk/Uses/gl.mk

6 ↗

(On Diff #46847)

further more this comment contradicts the statement later on the USES=gl takes no arguments.

[mat]

Mk/Uses/gl.mk

6 ↗

(On Diff #46847)

Could you maybe expand component and argument a bit, as just afterwards yo say the script does not take any.

28 ↗

(On Diff #46847)

Remove quotes around yes.

28–30 ↗

(On Diff #46847)

USE_GL=yes should generate a DEV_WARNING too, as this is now only an optional argument to the USES.

Mk/bsd.port.mk

366–369 ↗

(On Diff #46847)

This whole bit should be moved to Mk/Uses/gl.mk.

1427–1429 ↗

(On Diff #46847)

You should add a check testing that USES does not already contains gl.

Also, add a DEV_WARNING saying that using USE_GL without USES=gl is wrong.

[zeising]

I've updated the patch to take into account the comments in the review.

The comment at the top of Uses/gl.mk was wrong, hence the confusion. It was a leftover from a previous attempt at implementing this.
The USE_GL= must remain, it's how to select which components to use. It's modeled on USES=gnome.

[mat]

Mk/bsd.port.mk

1422–1424 ↗

(On Diff #47005)

This should only be a compat shim, so it should only add gl if it is not already in USES. It should also add a DEV_WARNING saying that it is invalid to use USE_GL without USES=gl.

[zeising]

Mk/bsd.port.mk

1422–1424 ↗

(On Diff #47005)

Ok, however, USE_GL= is not a compat shim, it's still needed to specify which GL components to bring in. But perhaps I'm misunderstanding what you mean?

[mat]

Mk/bsd.port.mk

1422–1424 ↗

(On Diff #47005)

Thank you for your input, I worded it wrong.

It should only add if USES does not include gl. Then it needs to have a warning like this:

DEV_WARNINGS+=   "Using USE_GL alone is deprecated, please add USES=gl."

[zeising]

Updated patch with a DEV_WARN if USE_GL is set without USES=gl.

[zeising]

Mk/bsd.port.mk

1422–1424 ↗

(On Diff #47005)

Thank you very much for the clarification.

[antoine]

Mk/bsd.port.mk

1424 ↗

(On Diff #47303)

This breaks previous behavior (yes used to imply glu, not gl)

[mat]

Mk/bsd.port.mk

1423 ↗

(On Diff #47303)

The variable is called DEV_WARNING.

[zeising]

Updated with latest round of comments.

Thank you very much for the review!

[mat]

Mk/Uses/gl.mk

10–11 ↗

(On Diff #47454)

s/gl/glu/

34 ↗

(On Diff #47454)

This should be glu too.

[zeising]

Mk/Uses/gl.mk

10–11 ↗

(On Diff #47454)

Doh. Will fix.

34 ↗

(On Diff #47454)

No, this is the case when USES=gl but USE_GL is not defined at all, then we should use gl. The case when USE_GL is yes is handled below, and defaults to glu, just like before.

[mat]

Mk/Uses/gl.mk

34 ↗

(On Diff #47454)

Having

1. USE_GL=yes -> USE_GL=glu
2. USE_GL is not defined -> USE_GL=gl

is really really a bad idea. yes and not defined should mean the same thing.

[zeising]

Mk/Uses/gl.mk

34 ↗

(On Diff #47454)

I do not agree.
No port today has USES=gl and USE_GL not defined, so there is no backwards compatibility to take into account. I understand USE_GL=yes means glu, that's because it's been that way for a very long time. USES=gl is new, and can imply new things. It would surprise me as a developer that USES=gl actually means i get glu, not gl unless I specifically asked for glu with USE_GL.

[antoine]

I think USE_GL not defined should be forbidden.

Also, the DEV_WARNING should suggest to replace USE_GL=yes with USE_GL=glu

[zeising]

Updated diff with changes based on the latest round of comments.

I elected to make it an error not to specifu USE_GL at all, when USES=gl is set.

Changes to games/q3cellshading to make it compile again.

[mat]

Mk/Uses/gl.mk

34 ↗

(On Diff #47561)

Should say something about having to set USE_GL.

games/q3cellshading/Makefile

67–82 ↗

(On Diff #47561)

Why move this?

[zeising]

Mk/Uses/gl.mk

34 ↗

(On Diff #47561)

Noted, will fix.

games/q3cellshading/Makefile

67–82 ↗

(On Diff #47561)

To fix the build. USES need to be defined before bsd.port.pre.mk is included, and the port has USE_GL at line 42, which implies USES. See build failure log here:
http://package23.nyi.freebsd.org/data/111i386-default-PR230692PR230909/2018-08-29_18h58m56s/logs/errors/q3cellshading-1.0_3.log and comment from Antoine here: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=230692#c6

[mat]

games/q3cellshading/Makefile

67–82 ↗

(On Diff #47561)

Yeah, so, you changed the include to be bsd.port.options.mk, I get that, but why move this block, it was fine up there.

[zeising]

games/q3cellshading/Makefile

67–82 ↗

(On Diff #47561)

Because it had to be after the bsd.port.pre.mk include, otherwise it wouldn't build..

[zeising]

games/q3cellshading/Makefile

67–82 ↗

(On Diff #47561)

Just changing the include from bsd.port.pre.mk gives me this:

make: "/usr/ports/games/q3cellshading/Makefile" line 91: Malformed conditional (${COMPILER_TYPE} == clang)
make: "/usr/ports/Mk/bsd.port.mk" line 1834: Malformed conditional (${WITH_PKG} == devel)
make: "/usr/ports/Mk/bsd.port.mk" line 2140: Malformed conditional (${PREFIX} == /usr)
make: "/usr/ports/Mk/bsd.port.mk" line 2658: Malformed conditional (${PREFIX} == /usr)
make: "/usr/ports/Mk/bsd.port.mk" line 2671: Malformed conditional (${PREFIX} == /usr)
make: "/usr/ports/Mk/bsd.port.mk" line 4553: Malformed conditional ((${PREFIX} != ${LOCALBASE} && ${PREFIX} != ${LINUXBASE} && ${PREFIX} != "/usr" && ${PREFIX} != "/" && !defined(NO_PREFIX_RMDIR)))
make: Fatal errors encountered -- cannot continue
make: stopped in /usr/ports/games/q3cellshading

So bsd.port.pre.mk needs to be included before the block, but after the uses stuff, which is in the options block.

[zeising]

games/q3cellshading/Makefile

67–82 ↗

(On Diff #47561)

from bsd.port.pre.mk to bsd.port.options.mk

[mat]

games/q3cellshading/Makefile

67–82 ↗

(On Diff #47561)

This looks more like a symtom of not changing bsd.port.post.mk to bsd.port.mk at the end for the Makefile. So my point stands.

[antoine]

bsd.port.pre.mk / bsd.port.post.mk must be kept for the ${COMPILER_TYPE} check

[zeising]

games/q3cellshading/Makefile

67–82 ↗

(On Diff #47561)

Even changing the bsd.port.pre.mk to bsd.port.options.mk and bsd.port.post.mk to bsd.port.mk I still get the following error:

make: "/usr/ports/games/q3cellshading/Makefile" line 90: Malformed conditional (${COMPILER_TYPE} == clang)
make: Fatal errors encountered -- cannot continue

[zeising]

In D16774#362352, @antoine wrote:

bsd.port.pre.mk / bsd.port.post.mk must be kept for the ${COMPILER_TYPE} check

I think my suggested change, with moving the entire block is the only way to solve this.

[mat]

games/q3cellshading/Makefile

67–82 ↗

(On Diff #47561)

Adding bsd.port.pre.mk here is fine, but you do not need to move the rest.

[zeising]

Updated based on latest set of comments.

Restored the ordering in games/q3cellshading/Makefile and just fixed includes
Reworded IGNORE message with USE_GL isn't defined but USES=gl is.

[zeising]

Update comment in Mk/Uses/gl.mk to reflect reality.

[zeising]

What's missing to get this in?