Don't allow invisible blanket option changes for bulk runs

[marino]

Summary

Some people may disagree, but I believe it's a bad idea in general to
invisibly and automatically change port options such as is done by the
DEVELOPER setting in relation to any defined TEST option.

I believe this was intended as a per-port change, but how it is
implemented causes a blanket change for bulk runs. Not only would I
classify it as a POLA violation, but it also causes massive rebuilds
for things like poudriere and synth where the port options do not
match already built packages.

Please consider limiting the invisible DEVELOPER manipulation of
TEST options to non-bulk runs.

[marino]

Now that weekend and BSDCan are over, can somebody review this one? It is more a philophical review than a technical one, although there is plenty of technical reasons to approve it as well.

[bdrewery]

I don't really see the problem, especially since it was made 8 months ago. Bapt and amdmi3 should weigh on this change since they made it.

At least for Poudriere it removes user-supplied DEVELOPER and only passes it in check-sanity, stage-qa and check-plist phases which it explicitly calls. So it shouldn't be modifying any of the dependencies that are pulled into the packages.

I think this is something that should be discussed on ports@

[marino]

bdrewery,
The poudriere test mode adds DEVELOPER=yes, I beleive. By default, it's only for the port being tested, but there's a non-default option to "test" every dependency. If you set that non-default option, ports like "devel/nspr" will fail the sanity check because of an options mismatch. That will cascade to huge losses. If I took the time, i could probably write a test case to show that it affects poudriere too in specific configurations.

My position is that auto-setting options in general is a bad idea and always was a bad idea. The compromise was to limit this auto-setting to manual builds only.

[marino]

circling back to this comment, "I don't really see the problem, especially since it was made 8 months ago.", I should have mentioned that this has been negatively affecting me for 8 months. I just finally got around to doing something about it, e.g. tracking down the culprit. It's been a problem for long time.

Other than the options like NLS, DOCS which automatically set to "on", no other port options should be automagically set on the basis of POLA violations.

[marino]

Actually, thinking about this more, I'm not sure blocking this for bulk runs goes far enough.

Even on the command line, something like "cd /usr/ports/mail/thunderbird; make DEVELOPER=yes" would expose every unbuilt dependency to DEVELOPER flag, meaning something like devel/nspr would be built with the TEST option. Invisible, unexpected.

It might be worth revisiting why it was deemed a good idea to couple TEST option with DEVELOPER setting anyway. If somebody wants to build a port with the test option on, they should explicitly set it. This option often is a very heavy build sequence, something that should not be automatically started when somebody wants to do plist checks. (for example)

[bapt]

I agree that the options should not vary. a TEST option is definitely a bad idea and this patch does not solve it. I haven't thought about it when accepting the proposal from @AMDmi3 .

I think we should explain that a TEST option is a bad idea now that we have the test framework, and maybe introduce a RUN_TESTSUITE knob (autoactivated by DEVELOPERS)

options should be for "users" so they know what options has been activated that "inpacts" the package IMHO

[rene]

I think this can be closed now as of https://svnweb.freebsd.org/changeset/ports/430773 by @antoine

[marino]

Yes, I agree. Thank you Antoine!
I can't "accept" it though, I can only "abandon" the review.
I think somebody on portmgr has to accept/close it.

[rene]

Committed independently by @antoine in https://svnweb.freebsd.org/changeset/ports/430773