I am not sure this is a good idea.

Great, thank you for this work.

Great, thank you for this work.

Whatever is in the review description will do just fine for the CHANGES entry.

Whatever is in the review description will do just fine for the CHANGES entry.

Oh, please add a entry to the CHANGES file too.

Oh, please add a entry to the CHANGES file too.

In D48201#1100173, @bofh wrote:

In D48201#1099478, @mat wrote:

It would be nice to really make use of the USES framework by using arguments, I think the JAVA_VERSION, JAVA_BUILD, JAVA_EXTRACT, JAVA_RUN and probably the USE_ANT variable should be turned into USES=java arguments, like USES=java:18+,build.

Except version everything has been converted to respective arguments.

Approved as long as it's been tested with a "today" ports tree. ("today" being regarding to the day it will being pushed.)

It would be nice to really make use of the USES framework by using arguments, I think the JAVA_VERSION, JAVA_BUILD, JAVA_EXTRACT, JAVA_RUN and probably the USE_ANT variable should be turned into USES=java arguments, like USES=java:18+,build.

This looks promising.
Could you update the diff with either using arcanist or git diff -U9999 so we get context ?

Yes, please, go ahead.

still looks good :-)

No git command should be run in WRKDIR, ever. If a port does this, it should be fixed.

And when that is changed, please ask for an exp-run to see if things break.

This looks like a good idea, but could you please add the new environment variable to where WRK_ENV is first appended to, about 100 lines before.

Well, if you feel confident that all ports will continue to build if you push this commit, then you have portmgr's approval to go ahead.
If you think it would be better to make sure everything still build, then ask for an exp-run and wait for the result.

With your explanation and seeing the other review, I get it. It would have been nice to have all in the same review, it would have saved me some time trying to figure things out :-)
Please go ahead with both reviews.

In D47380#1083512, @bz wrote:

In D47380#1083399, @mat wrote:

Looking at the spaghetti in Mk, I cannot see if there is some target we can run from the root directory that would run check-license in every port.
But it's probably just adding that target in the targets in bsd.ports.subdir.mk to run the test.

Yes, seems to work and catch cases.

I'll work through them. @mat. Should I add the changes here to the same review or open a separate review for them?

I don't quite understand where the problem is.
Is the fact that there are two / sometime breaks stuff?
If this is actually the case, I'd rather the source of the second / be somewhat fixed.

I'm sorry, I am not quite sure I understand what this is for.
Could you add a small documentation at the top of the file explaining exactly why this is needed, and in which cases one should add USES=sbrk?
Also, maybe in the ignore message, add that it does not take any argument.

This looks sound.
I have only run it in my head though.

I can't spot anything wrong, so

In D39303#1064275, @vishwin wrote:

No, Herald (phab) added it automatically. This has already been exp-ran by both portmgr and myself with no problems. I'm basically acting as Python here but leaving this open for a bit to collect feedback on the CHANGES wording.

Are you waiting for portmgr's approval ?
This file is maintained by python@, so, all you need is an exp-run to make sure everything is fine.

The change is wrong, the code is correct. See sh(1):

I have not needed WITH_DEBUG for a while, thanks.

Something like this

In D46059#1053621, @michaelo wrote:

Anyone else, any objections?

We probably need a message on each list asking to keep it in sync with the two other.

Assuming you tested this, and you probably know better than me what this actually does 😇

make -C dns/bind918 -V USE_PERL5 WITH_DEBUG=yes WITHOUT_DEBUG_PORTS=dns/bind918

Results in "build".
This means that the port still believes that the "debug" feature is enabled, while it is actually disabled by the second knob.