Page MenuHomeFreeBSD

Mk/Scripts/qa.sh: Allow users to override the depends_blacklist QA check
ClosedPublic

Authored by dvl on Jun 25 2020, 1:58 PM.

Details

Summary

At present, the policy implemented in Mk/Scripts/qa.sh prevents me
from adding lang/python, lang/python2, or lang/python3 as a dependency of
another port. "This is to prevent adding dependencies to meta ports that
are only there to improve the end user experience." - I build my own packages
via poudriere. I want to create my own meta-package which has such packages
as RUN_DEPENDS. It's been suggested that I patch my own copy of the tree.
This patch moves towards tools, not policy.

This patch allows me to set this variable in a poudriere make.conf file:

QA_ENV+= IGNORE_DEPENDS_BLACKLIST="YES"

Test Plan

By default, existing behavior is preserved

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dvl requested review of this revision.Jun 25 2020, 1:58 PM

Could you make it more generic, doing a test in the loop, so that I can add IGNORE_foo to ignore the foo check?

Mk/Scripts/qa.sh
1019 ↗(On Diff #73632)

We try to move away from having magic strings, I would rather it be something like this.

dvl marked an inline comment as done.

Make the change generic, so that any check can be IGNOREd

Good idea mat.

Mk/Scripts/qa.sh
1018 ↗(On Diff #73647)

I tried to avoid CHECKS= and keep all the options in the if, but

  • that doesn't work, you get one loop through
  • creating CHECKS makes the for easier to read
1023 ↗(On Diff #73647)

Any IGNORE option gets flagged in the output

linimon retitled this revision from Allow users to override the depends_blacklist QA check to Mk/Scripts/qa.sh: Allow users to override the depends_blacklist QA check.Jun 26 2020, 7:56 AM
Mk/Scripts/qa.sh
1015 ↗(On Diff #73647)

Why rename checks to CHECKS, and why make it a multiline variable?

  • don't rename checks to CHECKS
  • keep it all on one line
Mk/Scripts/qa.sh
1015 ↗(On Diff #73647)
  • CHECKS is habit; it is what I do at home
  • multi-line was to wrap at 80 characters

Ok, can you now put the variable like it was before, you only need to add the new check at the end.

Also, please don't add a second loop, just do the test in the loop at the end.

In D25450#565818, @mat wrote:

Also, please don't add a second loop, just do the test in the loop at the end.

Oh now I knew why I was using CHECKS.

Mk/Scripts/qa.sh
1018 ↗(On Diff #74208)

Can you please put back the check variable definition as it was before?

Restore original checks variable definition

dvl marked an inline comment as done.Jul 20 2020, 3:53 PM
Mk/Scripts/qa.sh
1024 ↗(On Diff #74696)

You need to quote the variable in there.

Mk/Scripts/qa.sh
1024 ↗(On Diff #74696)

Do you mean: [ -z "${check_test}" ]

Mk/Scripts/qa.sh
1024 ↗(On Diff #74696)

Yes, only one variable in the line I quoted.

Put ${check_test} in "quotes" when testing

dvl marked an inline comment as done.Jul 20 2020, 4:08 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jul 20 2020, 7:22 PM
This revision was automatically updated to reflect the committed changes.