Page MenuHomeFreeBSD
Differential D25450

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

Authored by dvl on Jun 25 2020, 1:58 PM.
Tags
None
Referenced Files
F106656451: D25450.id.diff
Fri, Jan 3, 11:55 AM
Unknown Object (File)
Wed, Dec 25, 2:35 PM
Unknown Object (File)
Thu, Dec 5, 5:19 PM
Unknown Object (File)
Dec 4 2024, 2:40 AM
Unknown Object (File)
Nov 2 2024, 8:15 AM
Unknown Object (File)
Nov 1 2024, 11:58 AM
Unknown Object (File)
Oct 29 2024, 9:09 PM
Unknown Object (File)
Oct 24 2024, 4:31 PM
View All 90 Files
Subscribers
None

Details

Reviewers
mat
swills
Group Reviewers
O5: Ports Framework
portmgr
Commits
rP542671: At present, the policy implemented in Mk/Scripts/qa.sh prevents me
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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dvl created this revision.Jun 25 2020, 1:58 PM
Owners added a reviewer: O5: Ports Framework.Jun 25 2020, 1:58 PM
Herald added a reviewer: portmgr. · View Herald TranscriptJun 25 2020, 1:58 PM
Harbormaster completed remote builds in B31953: Diff 73632.Jun 25 2020, 1:58 PM
dvl requested review of this revision.Jun 25 2020, 1:58 PM
mat added a comment.Jun 25 2020, 2:19 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 updated this revision to Diff 73647.Jun 25 2020, 5:22 PM
dvl marked an inline comment as done.

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

Harbormaster completed remote builds in B31959: Diff 73647.Jun 25 2020, 5:22 PM
dvl added a comment.Jun 25 2020, 5:24 PM

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
dvl added a comment.Jul 1 2020, 8:08 PM

What is the next step?

mat added inline comments.Jul 6 2020, 12:37 PM
Mk/Scripts/qa.sh
1015 ↗(On Diff #73647)

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

dvl updated this revision to Diff 74112.Jul 6 2020, 2:02 PM
  • don't rename checks to CHECKS
  • keep it all on one line
Harbormaster completed remote builds in B32155: Diff 74112.Jul 6 2020, 2:02 PM
dvl added inline comments.Jul 6 2020, 2:03 PM
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
mat added a comment.Jul 7 2020, 11:21 AM

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

mat added a comment.Jul 7 2020, 11:23 AM

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

dvl added a comment.Jul 8 2020, 6:09 PM
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.

dvl updated this revision to Diff 74208.Jul 8 2020, 6:09 PM

Just one loop.

Harbormaster completed remote builds in B32205: Diff 74208.Jul 8 2020, 6:09 PM
mat added inline comments.Jul 20 2020, 3:23 PM
Mk/Scripts/qa.sh
1018 ↗(On Diff #74208)

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

dvl updated this revision to Diff 74696.Jul 20 2020, 3:53 PM

Restore original checks variable definition

Harbormaster completed remote builds in B32438: Diff 74696.Jul 20 2020, 3:53 PM
dvl marked an inline comment as done.Jul 20 2020, 3:53 PM
mat added inline comments.Jul 20 2020, 3:57 PM
Mk/Scripts/qa.sh
1024 ↗(On Diff #74696)

You need to quote the variable in there.

dvl added inline comments.Jul 20 2020, 3:59 PM
Mk/Scripts/qa.sh
1024 ↗(On Diff #74696)

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

mat added inline comments.Jul 20 2020, 4:01 PM
Mk/Scripts/qa.sh
1024 ↗(On Diff #74696)

Yes, only one variable in the line I quoted.

dvl updated this revision to Diff 74697.Jul 20 2020, 4:08 PM

Put ${check_test} in "quotes" when testing

Harbormaster completed remote builds in B32439: Diff 74697.Jul 20 2020, 4:08 PM
dvl marked an inline comment as done.Jul 20 2020, 4:08 PM
mat accepted this revision as: O5: Ports Framework, portmgr.Jul 20 2020, 4:11 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jul 20 2020, 7:22 PM
Closed by commit rP542671: At present, the policy implemented in Mk/Scripts/qa.sh prevents me (authored by dvl). · Explain Why
This revision was automatically updated to reflect the committed changes.
dvl added a commit: rP542671: At present, the policy implemented in Mk/Scripts/qa.sh prevents me.

Revision Contents
Changeset List

PathSizePackages
head/
Mk/
Scripts/
7 linesO5: Ports Framework

Diff 74714

View Options

head/Mk/Scripts/qa.sh

Loading...