Change USE_XORG to USES=xorg and USES=xorg-cat

[zeising]

Summary

Change USE_XORG to use the USES= framework.

Split up the old bsd.xorg.mk into two parts, Uses/xorg.mk and Uses/xorg-cat.mk.
Uses/xorg.mk contains the "user-facing" parts, related to how to depend on various xorg components. This relates to USE_XORG=
Uses/xorg-cat.mk contains the internal stuff, used by various xorg ports. This is for ports that set XORG_CAT=
Add legacy shims in bsd.port.mk to add USES=xorg or USES=xorg-cat if USE_XORG or XORG_CAT is defined.

Add two new (experimental) features to Uses/xorg-cat.mk:
Add provisions to Uses/xorg-cat.mk to use the meson build system instead of the autotools build system. This is done by specifying an optional build system argument to USES=xorg-cat.
Add provisions to Uses/xorg-cat.mk to pull sources straight from freedesktop.org gitlab (gitlab.freedesktop.org). This is mainly to make it easier for the development team. This can be used by adding USE_GITLAB=yes to an xorg port Makefile.

This change will eventually need an exp-run, but I'd like a review of the changes before that.

Branch with current work in progress can also be found here: https://github.com/FreeBSDDesktop/freebsd-ports/tree/feature/usesxorg

Test Plan

exp-run

[zeising]

Thank you very much for the review @tobik !
I think I've fixed everything, I'm just going to do a local smoke test, and then I'll update the patch in phabricator as well.

Mk/Uses/xorg-cat.mk

47 ↗

(On Diff #58904)

Oops. And the joy of copy-pasting means it is everywhere. :)

55 ↗

(On Diff #58904)

Thank you, didn't know it was possible to do it that way.

Mk/Uses/xorg.mk

19–20 ↗

(On Diff #58904)

Oops again. I thought I added it, but maybe I forgot about it. I realized I'm also not looking to see if USE_XORG is actually set, before looking for modules.

155–157 ↗

(On Diff #58904)

This is where things get interesting. Looking at the history of things, I introduced the fontutil:build in December 2013, but it has probably never worked as intended. It might be a mis-merge from the SVN development repo we used back in the day. The check is even older than that, from when bsd.xorg.mk was first created, in 2007, and always accepted anything after the ':'.

Almost all modules are various types of libraries, and as such, it makes no sense to have them as build only dependencies. xorgproto is already a build only dependency by default, so multimedia/gstreamer1-vaapi can easily be changed. It will still support :both for the cases where it's needed (or at least done that way, I found a few instances).

For fontutil, it is slightly trickier. For some reason, it is done as a library dependency, even if it doesn't provide any shared libraries. fontutil is only used by the various x servers and a couple of fonts. I'll make it a build dependency, and the exp-run will show if that's a mistake or not. I doubt it is used in runtime at all.

As a side note, all those LIB_PC_DEPENDS should probably be changed to look for shared libraries instead of .pc files, and be made proper library depends, instead of build/runtime depends. That is out of scope for this though, and will have to happen at a later stage.

[kwm]

This porters-handbook chapter needs to be updated/rewritten, maybe there are other bits elsewhere in the porters-handbook that also need attention.
https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/using-x11.html

A warning in bsd.port.mk that the use of USE_XORG without USES=xorg is deprecated (like with USE_GNOME)

[zeising]

In D20724#448218, @kwm wrote:

This porters-handbook chapter needs to be updated/rewritten, maybe there are other bits elsewhere in the porters-handbook that also need attention.
https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/using-x11.html

I am aware of that. I'll get it done in time, but it will be another review.

A warning in bsd.port.mk that the use of USE_XORG without USES=xorg is deprecated (like with USE_GNOME)

See L1348-L1357 of bsd.port.mk above, and you'll see it's already included.

[zeising]

Updated based on comments from @tobik .

[mat]

Also, could you please run both new mk files through Tools/scripts/indent_make_if.pl so that the indentations is consistent with other files in that directory?

Mk/Uses/xorg-cat.mk

93–94 ↗

(On Diff #58917)

I am not sure this is correct, it should probably be:

GL_SITE?=               https://gitlab.freedesktop.org/xorg
GL_ACCOUNT?=            ${_XORG_CAT}

97 ↗

(On Diff #58917)

You should probably add a IGNORE="not supported yet" or something so that it is clear.

104–105 ↗

(On Diff #58917)

Please, do not add more MASTER_SITE_SUBDIR, it should be:

MASTER_SITES?=          XORG/individual/${_XORG_CAT}

122–130 ↗

(On Diff #58917)

This should probably be a long .if / .elif / elif / ... as if the first is true, the next ones will not be.

174–175 ↗

(On Diff #58917)

This should probably only set DISTNAME?=xorg-server-${PORTVERSION} as the .tar.bz2 is already taken care of earlier, and both variables depend on DISTNAME being correct.

Mk/Uses/xorg.mk

148–150 ↗

(On Diff #58917)

To be consistent with other USES, this should be at the beginning.

152–154 ↗

(On Diff #58917)

And this too, probably.

[mat]

Also, to be consistent with the rest of the Ports framework, do not quote words in tests.

Do this:

.if ${foo} == bar

No quotes around bar.

[zeising]

Updated diff based on comments from @mat

Mk/Uses/xorg-cat.mk

93–94 ↗

(On Diff #58917)

Yeah, that is better, at least from the way we handle DIST_SUBDIR.
Interestingly enough, there are several different areas, such as gitlab.freedesktop.org/mesa and /wayland. But They will probably not use the same infrastructure.

104–105 ↗

(On Diff #58917)

This was copied from the original bsd.xorg.mk, I've changed it.

122–130 ↗

(On Diff #58917)

The style was copied from how it was in bsd.xorg.mk. I'll change it to an if/elif/.../else/endif style. I've tried to keep some white space around, to make it easier to see which block belongs to which _xorg_cat. I know this isn't 100% style, but I think it makes it more readable.

Mk/Uses/xorg.mk

148–150 ↗

(On Diff #58917)

I've moved them to the beginning of the POSTMKINCLUDED block, in case we for some reason stumple through this file without defining them in the PRE phase.

[zeising]

Update patch again.
Move things around in Uses/autoreconf.mk to check for libtool things later. Uses/autoreconf.mk checks if libtool is being used, by checking libtool_ARGS, and if it's set, add a dependency on libtoolize. Move this check later, in the POSTMKINCLUDED phase, to avoid issues where we're hitting autoreconf before setting libtool_ARGS, and then not bringing in libtoolize.

[tijl]

I think the problem is you are including autoreconf.mk before setting libtool_ARGS. Try moving "libtool_ARGS?= #empty" to right before each libtool.mk include (there are two) so it's only defined if libtool is used. Then include autoreconf.mk if "USE_GITLAB && BUILDSYS == autotools" right after ". endif # ${_XORG_CAT} == <category>".

[zeising]

In D20724#449034, @tijl wrote:

I think the problem is you are including autoreconf.mk before setting libtool_ARGS. Try moving "libtool_ARGS?= #empty" to right before each libtool.mk include (there are two) so it's only defined if libtool is used. Then include autoreconf.mk if "USE_GITLAB && BUILDSYS == autotools" right after ". endif # ${_XORG_CAT} == <category>".

Hi!
Setting libtool_ARGS early or later shouldn't matter, but I moved it right before the include of libtool.mk. Then I did a check for if USES contains autoreconf (if ${USES:Mautoreconf}), instead of checking for USE_GITLAB, since we might need autoreconf even if GITLAB isn't being used (for instance, if we patch configure.ac and configure needs to be regenerated). However, this results in the same issue, because autoreconf is already walked through before the inclusion of libtool.mk, probably by some other part of the ports infrastructure. I then also tried to set variables indicating if I wanted autotools and/or autoreconf, and including everythign at the end. This failed in the same way. My attempts can be seen in this branch on github: https://github.com/FreeBSDDesktop/freebsd-ports/tree/usesxorg/autoreconf , look at 8c89b76 and 9e63b55.

Perhaps I am misunderstanding exactly what you are suggesting, but from my point of view, it looks like autoreconf is pulled in before we know we want libtool, and then the depedency on libtoolize isn't found. By checking for libtool_ARGS later in autoreconf, this works. Note that autoreconf.mk doesn't have to come from xorg-cat.mk, it could be someone setting it as a USES=autoreconf in a ports makefile.

[mat]

I think you are trying to put really too much magic in xorg-cat.

It would probably be much cleaner to have ports that need meson, autoreconf, pkgconfig, libtool, pathfix... actually set something like:

USES= autoreconf libtool xorg xorg-cat 
USE_XORG= xorgproto x11

Mk/Uses/xorg-cat.mk

169 ↗

(On Diff #59024)

Remove WRKSRC, it is at its default value.

[zeising]

In D20724#449059, @mat wrote:

I think you are trying to put really too much magic in xorg-cat.

I am merely replicating the functionality that was in bsd.xorg.mk. All of this is in the old bsd.xorg.mk, and there it works. bsd.xorg.mk is only pulled in from bsd.ports.mk if USE_XORG or XORG_CAT is defined, perhaps it is better left this way, rather than converted to USES?

Removing the handling of dependencies from xorg-cat.mk means I have to walk through a couple of hundred ports and make these changes. The whole point of having them in a central place is to make it easier to update, change and add xorg ports. I have suggested a way to work around the libtool and autoreconf issues, which at least works for my testing, although I haven't built the entire ports tree. Won't this libtool/autoreconf issue also affect other ports that first walk through libtool, and then realize they need autoreconf/libtoolize as well?

[tijl]

I found two ports with USES=autoreconf and XORG_CAT: x11-drivers/xf86-input-egalax and x11-drivers/xf86-video-intel. The former isn't maintained by x11@ and is fetched from github. If such non-X.org ports are allowed to use XORG_CAT then they may be fetched from gitlab and not need autoreconf for instance, so you cannot include autoreconf.mk. If XORG_CAT is only meant for X.org ports and use by other ports is unsupported then you can keep the libtool.mk and autoreconf.mk includes (as in 9e63b55 but without " || ${USES:Mautoreconf}") and for special cases like x11-drivers/xf86-video-intel you can add "USES= autoreconf libtool" to the port Makefile instead of just USES=autoreconf.

That said, I'm ok with the autoreconf.mk change. I don't see any problems with it. I just think that sooner or later something is going to decide it needs libtool and autoreconf in the post.mk section and run into the same problem. So far we've avoided the need for ordering between Uses includes. So "USES= autoreconf libtool" seems like the proper solution to me. If a port knows it needs autoreconf it also knows if it needs libtoolize so it's never a problem to use "USES= autoreconf libtool". It never happens that the need for libtool can only be determined later. And "USES= autoreconf libtool" is just the way we specify autoreconf with libtoolize at the moment, rather than say USES=autoreconf:libtoolize because libtool is always needed if libtoolize is used and "USES=autoreconf:libtoolize libtool" is a bit redundant.

[tijl]

x11-drivers/xf86-video-intel/Makefile

29 ↗

(On Diff #59024)

If xorg-cat.mk includes autoreconf.mk now (because of the USE_GITLAB above) then you can remove this autoreconf. For egalax I would just add libtool to USES in the port Makefile.

[mat]

In D20724#449167, @zeising wrote:

In D20724#449059, @mat wrote:

I think you are trying to put really too much magic in xorg-cat.

I am merely replicating the functionality that was in bsd.xorg.mk. All of this is in the old bsd.xorg.mk, and there it works. bsd.xorg.mk is only pulled in from bsd.ports.mk if USE_XORG or XORG_CAT is defined, perhaps it is better left this way, rather than converted to USES?

Save for the generic framework files, nothing should be left as a bsd.*.mk file, everything should be moved to a USES. The move is better if it is done by someone who actually understands what the file does.

Removing the handling of dependencies from xorg-cat.mk means I have to walk through a couple of hundred ports and make these changes. The whole point of having them in a central place is to make it easier to update, change and add xorg ports. I have suggested a way to work around the libtool and autoreconf issues, which at least works for my testing, although I haven't built the entire ports tree. Won't this libtool/autoreconf issue also affect other ports that first walk through libtool, and then realize they need autoreconf/libtoolize as well?

It was just a thought, I am sure all quirks can be addressed.

Mk/Uses/xorg-cat.mk

109 ↗

(On Diff #59024)

This should go earlier, before the inclusion of autoreconf.mk, you would not need to patch autoreconf then.

[zeising]

In D20724#449170, @tijl wrote:

I found two ports with USES=autoreconf and XORG_CAT: x11-drivers/xf86-input-egalax and x11-drivers/xf86-video-intel. The former isn't maintained by x11@ and is fetched from github. If such non-X.org ports are allowed to use XORG_CAT then they may be fetched from gitlab and not need autoreconf for instance, so you cannot include autoreconf.mk. If XORG_CAT is only meant for X.org ports and use by other ports is unsupported then you can keep the libtool.mk and autoreconf.mk includes (as in 9e63b55 but without " || ${USES:Mautoreconf}") and for special cases like x11-drivers/xf86-video-intel you can add "USES= autoreconf libtool" to the port Makefile instead of just USES=autoreconf.

I am not entirely sure what you mean here. Ports fetched directly from gitlab (or github) will require autoreconf, since when fetching arbitrary revisions, no pre-generated autotools scripts are provided. xorg-cat.mk takes care of the gitlab case, however, there might be other reasons for defining XORG_CAT= and USES=autoreconf, as xf86-input-egalax shows. This is xorg driver and while it doesn't come from X.org, it's tightly coupled to X.org stuff, and should probably use the USES=xorg-cat infrastructure. Another reason for using USES=autoreconf without using gitlab could be because we need to make local changes to configure.ac or similar.
Having ports that need both libtool and autoreconf keep track of that themselves kind of defeat the purpose of keeping track of libtool requirements in xorg-cat.mk.

That said, I'm ok with the autoreconf.mk change. I don't see any problems with it. I just think that sooner or later something is going to decide it needs libtool and autoreconf in the post.mk section and run into the same problem. So far we've avoided the need for ordering between Uses includes. So "USES= autoreconf libtool" seems like the proper solution to me. If a port knows it needs autoreconf it also knows if it needs libtoolize so it's never a problem to use "USES= autoreconf libtool". It never happens that the need for libtool can only be determined later. And "USES= autoreconf libtool" is just the way we specify autoreconf with libtoolize at the moment, rather than say USES=autoreconf:libtoolize because libtool is always needed if libtoolize is used and "USES=autoreconf:libtoolize libtool" is a bit redundant.

I'm not sure about this. What of the case if something needs libtool (which most libraries do, iirc), and then needs to patch configure.ac and run autoreconf based on if an option is selected or not? Ordering between Uses includes is tricky, especially in other Uses/*.mk files. There are a couple of Uses/*.mk that rely on other Uses/*.mk currently, and all of them use .include <${USESDIR}/feature.mk>.

Perhaps a policy should be made that all Uses/*.mk do the POSTMKINCLUDED thing, as I've done for xorg.mk above, and then add a requirement that variables are only checked in the post phase? My understanding of things is that this would let mk files create needed variables in the pre phase, which get checked in the post phase. I might be wrong in this, howerver.

[zeising]

In D20724#449198, @mat wrote:

In D20724#449167, @zeising wrote:

In D20724#449059, @mat wrote:

I think you are trying to put really too much magic in xorg-cat.

I am merely replicating the functionality that was in bsd.xorg.mk. All of this is in the old bsd.xorg.mk, and there it works. bsd.xorg.mk is only pulled in from bsd.ports.mk if USE_XORG or XORG_CAT is defined, perhaps it is better left this way, rather than converted to USES?

Save for the generic framework files, nothing should be left as a bsd.*.mk file, everything should be moved to a USES. The move is better if it is done by someone who actually understands what the file does.

Ok. I'm sure we/I can get this done for xorg stuff. Apologies if I was overly harsh in my comment above.

Removing the handling of dependencies from xorg-cat.mk means I have to walk through a couple of hundred ports and make these changes. The whole point of having them in a central place is to make it easier to update, change and add xorg ports. I have suggested a way to work around the libtool and autoreconf issues, which at least works for my testing, although I haven't built the entire ports tree. Won't this libtool/autoreconf issue also affect other ports that first walk through libtool, and then realize they need autoreconf/libtoolize as well?

It was just a thought, I am sure all quirks can be addressed.

I am still having trouble getting this to work. Ports that need autoreconf for some reason (other than USE_GITLAB), and get libtool pulled in from xorg-cat.mk still fail, since libtool_ARGS isn't set when autoreconf.mk is walked through.

Mk/Uses/xorg-cat.mk

109 ↗

(On Diff #59024)

This does not work. The variable should move earlier, but ports with USES=autoreconf and libtool from xorg-cat.mk still fail.

x11-drivers/xf86-video-intel/Makefile

29 ↗

(On Diff #59024)

Ports that get libtool from xorg-cat.mk should not need to also have it as a USES. There are several reasons for needing autoreconf, and keeping track of if a port gets libtool from the framework and suddenly need it as USES in the port will lead to strange issues.

I can fix the specific issue here with a work-around, but it doesn't fix the larger issue.

As a side note, without the change to xf86-video-intel to pull things from gitlab instead of cgit.freedesktop.org, the build also fails, so this is not an issue specific to the USE_GITLAB stuff.

[mat]

Mk/Uses/xorg-cat.mk

109 ↗

(On Diff #59024)

Well, as it is handled by xorg-cat, simply remove autoreconf from those ports. No? Seems easier to fix a few ports with this than try to force a shoe in the framework because of them.

And add a DEV_WARNING if a port sets USES=autoreconf xorg-cat telling them how to do it properly.

[zeising]

Mk/Uses/xorg-cat.mk

109 ↗

(On Diff #59024)

But what is the right way? Ports having USES=autoreconf needs to set USES=libtool as needed, even if it's normally in the framework? Isn't it better to solve the ordering issue in the framework itself?

I also realized that setting lbtool_ARGS can't be done in cases where Uses/libtool.mk isn't needed. Ports might require autoreconf without needing libtoolize, and setting libtool_ARGS unconditionally would always pull in libtoolize in the autoreconf case. This can probably be fixed in xorg-cat.mk by doing the USE_GITLAB preparation at the end of the file.

[tijl]

In D20724#449276, @zeising wrote:

I am not entirely sure what you mean here. Ports fetched directly from gitlab (or github) will require autoreconf, since when fetching arbitrary revisions, no pre-generated autotools scripts are provided.

Some projects keep pre-generated scripts in their repo.

xorg-cat.mk takes care of the gitlab case, however, there might be other reasons for defining XORG_CAT= and USES=autoreconf, as xf86-input-egalax shows. This is xorg driver and while it doesn't come from X.org, it's tightly coupled to X.org stuff, and should probably use the USES=xorg-cat infrastructure. Another reason for using USES=autoreconf without using gitlab could be because we need to make local changes to configure.ac or similar.

ok

Having ports that need both libtool and autoreconf keep track of that themselves kind of defeat the purpose of keeping track of libtool requirements in xorg-cat.mk.

Well, autoreconf and libtool are both autotools. They tend to go hand in hand. In theory they can be separate though. A port may patch a configure.ac that does not use libtool and set USES=autoreconf. Then later some Uses/*.mk file might patch configure.ac to make it use libtool and includes both libtool.mk and autoreconf.mk. In this case the libtool and autoreconf requirements are really separate. So your change is correct. autoreconf.mk should check libtool_ARGS as late as possible.

What of the case if something needs libtool (which most libraries do, iirc), and then needs to patch configure.ac and run autoreconf based on if an option is selected or not?

Then the port would have USES=libtool and add autoreconf to USES based on the option. The order of USES doesn't matter. bsd.port.mk first defines all <feature>_ARGS variables and then includes all <feature>.mk files.

Ordering between Uses includes is tricky, especially in other Uses/*.mk files. There are a couple of Uses/*.mk that rely on other Uses/*.mk currently, and all of them use .include <${USESDIR}/feature.mk>.

The ordering of the includes really shouldn't matter because depending on options things can be ordered arbitrarily in USES. Uses/*.mk can include other Uses/*.mk files but they cannot assume that their include is the first one.

[tijl]

Mk/Uses/xorg-cat.mk

109 ↗

(On Diff #59024)

libtool_ARGS needs to be defined right before you include Uses/libtool.mk and only if you include Uses/libtool.mk. That way you essentially replicate the way USES works.

x11-drivers/xf86-video-intel/Makefile

29 ↗

(On Diff #59024)

But the need for libtool isn't sudden. The xorg-cat framework isn't creating the need for libtool. The need for autoreconf and libtool are determined by upstream. Normally the port Makefile would need "USES=autoreconf libtool". The fact that they can be omitted by setting XORG_CAT is just convenient shorthand and sometimes shorthand doesn't work and you have to spell it out. I don't see the strange issues you talk about.

[zeising]

In D20724#449462, @tijl wrote:

In D20724#449276, @zeising wrote:

I am not entirely sure what you mean here. Ports fetched directly from gitlab (or github) will require autoreconf, since when fetching arbitrary revisions, no pre-generated autotools scripts are provided.

Some projects keep pre-generated scripts in their repo.

In my experience, that is not very common. But for those, simply not depending on autoreconf in those ports solve that issue.

xorg-cat.mk takes care of the gitlab case, however, there might be other reasons for defining XORG_CAT= and USES=autoreconf, as xf86-input-egalax shows. This is xorg driver and while it doesn't come from X.org, it's tightly coupled to X.org stuff, and should probably use the USES=xorg-cat infrastructure. Another reason for using USES=autoreconf without using gitlab could be because we need to make local changes to configure.ac or similar.

ok

Having ports that need both libtool and autoreconf keep track of that themselves kind of defeat the purpose of keeping track of libtool requirements in xorg-cat.mk.

Well, autoreconf and libtool are both autotools. They tend to go hand in hand. In theory they can be separate though. A port may patch a configure.ac that does not use libtool and set USES=autoreconf. Then later some Uses/*.mk file might patch configure.ac to make it use libtool and includes both libtool.mk and autoreconf.mk. In this case the libtool and autoreconf requirements are really separate. So your change is correct. autoreconf.mk should check libtool_ARGS as late as possible.

Yes, but libtool can be required even if autoreconf is not required. And an option can change the autoreconf requirement, for instance.

What of the case if something needs libtool (which most libraries do, iirc), and then needs to patch configure.ac and run autoreconf based on if an option is selected or not?

Then the port would have USES=libtool and add autoreconf to USES based on the option. The order of USES doesn't matter. bsd.port.mk first defines all <feature>_ARGS variables and then includes all <feature>.mk files.

OK, thank you for the explanation.

Ordering between Uses includes is tricky, especially in other Uses/*.mk files. There are a couple of Uses/*.mk that rely on other Uses/*.mk currently, and all of them use .include <${USESDIR}/feature.mk>.

The ordering of the includes really shouldn't matter because depending on options things can be ordered arbitrarily in USES. Uses/*.mk can include other Uses/*.mk files but they cannot assume that their include is the first one.

Perhaps some ordering guarantees need to be made, or at least a way in an Uses/*.mk file to tell which other Uses/*.mk files it needs, so that variables can be set up properly before they are included?

x11-drivers/xf86-video-intel/Makefile

29 ↗

(On Diff #59024)

A lot of XORG_CAT ports need libtool, so it is handled by the current framework in bsd.xorg.mk. Some ports also require autoreconf, and are currently responsible for pulling in autoreconf themselves. This works in bsd.xorg.mk. I'm trying to get the same thing to work in the new Uses/xorg-cat.mk. Currently, my best idea is to move the check for libtool_ARGS later in autoreconf.mk (in the MKPOSTINCLUDE stage. This seems to solve the issue where the check for if a port needs libtoolize is done before libtool_ARGS is defined.

Removing the libtool include from xorg-cat.mk would mean that a big number of ports needs to change, and suddenly keep track of the dependency on libtool themselves. It is possible to do this, but would make this change quite a lot larger, and also means functionality we have today get lost.

[tijl]

In D20724#449550, @zeising wrote:

In D20724#449462, @tijl wrote:

In D20724#449276, @zeising wrote:

I am not entirely sure what you mean here. Ports fetched directly from gitlab (or github) will require autoreconf, since when fetching arbitrary revisions, no pre-generated autotools scripts are provided.

Some projects keep pre-generated scripts in their repo.

In my experience, that is not very common. But for those, simply not depending on autoreconf in those ports solve that issue.

But xorg-cat.mk pulls in autoreconf anyway if these ports set USE_GITLAB.

Having ports that need both libtool and autoreconf keep track of that themselves kind of defeat the purpose of keeping track of libtool requirements in xorg-cat.mk.

Well, autoreconf and libtool are both autotools. They tend to go hand in hand. In theory they can be separate though. A port may patch a configure.ac that does not use libtool and set USES=autoreconf. Then later some Uses/*.mk file might patch configure.ac to make it use libtool and includes both libtool.mk and autoreconf.mk. In this case the libtool and autoreconf requirements are really separate. So your change is correct. autoreconf.mk should check libtool_ARGS as late as possible.

Yes, but libtool can be required even if autoreconf is not required. And an option can change the autoreconf requirement, for instance.

This works fine. You can decide you need autoreconf after you've decided you need libtool. The problem is deciding you need libtool after you've decided you need autoreconf. It's just that this problem never happens in practice. The case I described above, where it does happen, is highly unlikely. In practice you always know whether you need libtool or not when you decide you need autoreconf because the need for libtool is determined by configure.ac containing LT_INIT (or equivalent) and configure.ac is input for autoreconf. A port maintainer always knows if "USES=autoreconf libtool" is needed when he needs autoreconf, without looking at the framework. The only case where he cannot know is the one I described above where a hypothetical Uses/*.mk file adds LT_INIT to configure.ac. This will probably never happen but it's still a valid case and so I approve your autoreconf.mk change. I also agree that what xorg-cat.mk tries to do (allow USES=libtool to be omitted from port Makefiles) should be possible. I think the only thing we disagree on is whether adding USES=libtool to ports like egalax is somehow difficult, but this is irrelevant now.

Ordering between Uses includes is tricky, especially in other Uses/*.mk files. There are a couple of Uses/*.mk that rely on other Uses/*.mk currently, and all of them use .include <${USESDIR}/feature.mk>.

The ordering of the includes really shouldn't matter because depending on options things can be ordered arbitrarily in USES. Uses/*.mk can include other Uses/*.mk files but they cannot assume that their include is the first one.

Perhaps some ordering guarantees need to be made, or at least a way in an Uses/*.mk file to tell which other Uses/*.mk files it needs, so that variables can be set up properly before they are included?

That sounds complicated to me. I like your earlier idea better then, where Uses/*.mk set variables in the pre phase and take action in the post phase.

x11-drivers/xf86-video-intel/Makefile

29 ↗

(On Diff #59024)

Agreed.

[zeising]

Updated based on comments.

[zeising]

Patch is updated based on last round of comments. The only outstanding issue is the handling of libtool and autoreconf. Currently, the way it's handled is by moving the check for libtool_ARGS into the POSTMKINCLUDED stage of autoreconf.mk. Based on the discussion, perhaps that is the least bad idea?

[tijl]

Either USES=autoreconf in the intel driver can be removed or the autoreconf.mk include in xorg-cat.mk can be removed, but that's just a minor thing, so please go ahead with the exp-run.

[zeising]

Update based on latest comments. Ready for exp-run now.

[zeising]

Submitted for exp-run in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=238988 .

[tcberner]

Mk/bsd.port.mk

1348 ↗

(On Diff #59399)

why have these shims? I would simply make it invalid and give an error via bsd.sanity.mk.

[tcberner]

Mk/Uses/xorg.mk

149 ↗

(On Diff #59399)

^ as you're already touching these, how about prefixing them with something like xorg-.
It might be less likely for these components to collide with another definitions from Mk/Uses/* but it might be a good precaution.

[zeising]

Mk/bsd.port.mk

1348 ↗

(On Diff #59399)

It's the normal way to do it. And this way I don't have to update *all* the ports at once, they can be updated with this as they are updated.

[zeising]

Mk/Uses/xorg.mk

149 ↗

(On Diff #59399)

I doubt they will collide. Especially since they've had these names since 2007 or so.
Changing the names also means that all ports using them need to be updated.

[tcberner]

Mk/bsd.port.mk

1348 ↗

(On Diff #59399)

You have to update them at some point, why not now. It has been ages since for example USES=gnome has been introduced and a big chunk still does not have USES.

I would prefer a bigger change resulting in a clean tree :)

[tcberner]

Mk/Uses/xorg.mk

149 ↗

(On Diff #59399)

Nope, see for example qt.mk and py-qt.mk -- the components are named the same for the ports that use them -- but the
foo_LIB_DEPENDS are just called baz-foo_LIB_DEPENDS and moop-foo_LIB_DEPENDS.

So you can have

USE_BAZ=foo
USE_MOOP=foo

The only change required for is in the lines 160ff to use stuff like

LIB_PC_DEPENDS+=${xorg-${_module}_LIB_PC_DEPENDS}

[antoine]

This breaks ports that test something and set USE_XORG between bsd.port.pre.mk and bsd.port.post.mk.

Also, I have the feeling that there is a .include "${USESDIR}/*.mk" abuse in xorg-cat.mk, in my opinion this should be pushed to the USES line in individual ports.

Mk/bsd.port.mk

1910 ↗

(On Diff #59399)

This breaks ports that test something and set USE_XORG between bsd.port.pre.mk and bsd.port.post.mk.

[zeising]

In D20724#455153, @antoine wrote:

This breaks ports that test something and set USE_XORG between bsd.port.pre.mk and bsd.port.post.mk.

Also, I have the feeling that there is a .include "${USESDIR}/*.mk" abuse in xorg-cat.mk, in my opinion this should be pushed to the USES line in individual ports.

What do you suggest instead? Several other files in Uses/ are already including other files, I'm just following the precedent, as it seems to be working.
The whole idea of XORG_CAT is to set up an environment, including common dependencies to make it easier to compile xorg ports. This works in bsd.xorg.mk, and I'm trying to get it working here. Changing this, and having to walk through and test all ports that define XORG_CAT, and manually add dependencies, and environment set up is quite error prone, and will make it much harder to maintain the various xorg ports.

[antoine]

Mk/bsd.port.mk

1910 ↗

(On Diff #59399)

And this breaks USES=gl too which sets USE_XORG during bsd.port.pre.mk

[zeising]

Mk/bsd.port.mk

1910 ↗

(On Diff #59399)

Should this be supported? I'm unsure if it's supported adding things to USES after bsd.port.pre.mk is included. My testing seem to indicate it is not. If it isn't, then defining USE_XORG= after bsd.port.pre.mk should probably not be supported either, since it's the same as adding USES=xorg after bsd.port.pre.mk. Appending to USE_XORG after bsd.port.pre.mk should be supported by this patch.

[antoine]

Mk/bsd.port.mk

1910 ↗

(On Diff #59399)

It has to be supported (it was supported before).

[zeising]

Mk/bsd.port.mk

1910 ↗

(On Diff #59399)

Do you have any suggestions on how to do this?
If USES=xorg isn't allowed to be set after bsd.port.pre.mk is included, I'm not sure how to fix it.

The USES=gl case has to be fixed of course.

[antoine]

Mk/bsd.port.mk

1910 ↗

(On Diff #59399)

Maybe something like:

+.if defined(USE_XORG) && (!defined(USES) || !${USES:Mxorg})
+DEV_WARNING+= "Using USE_XORG alone is deprecated, please use USES=xorg"
+_USES_POST+= xorg
+.endif

[zeising]

Patch updated to fix remaining comments.

I believe this is ready for a exp-run now.

[mat]

Mk/bsd.port.mk

1353 ↗

(On Diff #60931)

This does not match if USES=xorg-cat:foo is set.

[zeising]

Mk/bsd.port.mk

1353 ↗

(On Diff #60931)

Actually, the check is wrong. Defining XORG_CAT should always be a warning, even if USES:xorg-cat:foo is defined.. I'll rework it.