Page MenuHomeFreeBSD

Add flavors helpers.
ClosedPublic

Authored by mat on Sep 24 2017, 12:07 PM.

Details

Summary

This is not a final patch, it is ideas of what could be.

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

mat created this revision.Sep 24 2017, 12:07 PM
mat updated this revision to Diff 33383.Sep 24 2017, 6:50 PM
  • use !empty instead of defined.
mat updated this revision to Diff 33384.Sep 24 2017, 6:55 PM
  • Change OPTIONS_NAME to include the FLAVOR, if defined and not default.
mat updated this revision to Diff 33385.Sep 24 2017, 6:59 PM
  • Rename .for variable.
  • Add a first idea for what to do with options.
mat edited the summary of this revision. (Show Details)Sep 24 2017, 9:29 PM
mat updated this revision to Diff 33450.Sep 26 2017, 2:47 PM
  • Fix the helpers I added.
mat updated this revision to Diff 33454.Sep 26 2017, 3:44 PM
  • Add a guard, it seems when we get through this code more than once.
adamw added a subscriber: adamw.Sep 26 2017, 3:48 PM
adamw added inline comments.
Mk/bsd.port.mk
1112 ↗(On Diff #33454)

OPTIONS_OFF here conflicts with the OPTIONS helper usage of "_OFF".

_FORCE and _EXCLUDE, or _ENABLE/_DISABLE might be clearer.

mat added inline comments.Sep 26 2017, 4:08 PM
Mk/bsd.port.mk
1112 ↗(On Diff #33454)

Mmmm, I think whatever we do, we'll use keywords that have already been used before :-)

I may prefer _FORCE and _EXCLUDE though, because it is what they do.

adamw added inline comments.Sep 26 2017, 5:40 PM
Mk/bsd.port.mk
1112 ↗(On Diff #33454)

Definitely agree on the used-before thing. It's just that _OFF suggests that those options are enabled when this is not the current flavour.

mat updated this revision to Diff 33657.Oct 3 2017, 4:48 PM

Rebase.

mat updated this revision to Diff 33685.Oct 4 2017, 1:41 PM
  • Rename option helpers to _FORCE and _EXCLUDE.
mat marked 3 inline comments as done.Oct 4 2017, 1:42 PM
bapt added a subscriber: bapt.Oct 4 2017, 1:44 PM

This is strange, I think it would be a good idea to differentiate them from options helpers by prefixing them with FLAVOR_ or somethinng like that, what do you think?
Another side note is we should probably add them by small chunks rather than all in one :)

mat updated this revision to Diff 33689.Oct 4 2017, 1:48 PM

rebase

mat added a comment.Oct 9 2017, 4:18 PM

I am not against adding a FLAVOR_ prefix, but I think it will make very long variable names, say, FLAVOR_python3_OPTIONS_EXCLUDE= BAR. I think, as options are all UPPERCASE, we simply need to say that flavors are all lowercase, and thus, they cannot conflict.

mat updated this revision to Diff 33942.Oct 13 2017, 12:14 PM
  • Add IGNORE/BROKEN flavor helpers.
mat updated this revision to Diff 34197.Oct 20 2017, 9:09 PM
  • Remove flavors options helpers.
  • Add *_DEPENDS for flavors helpers.
mat updated this revision to Diff 34327.Oct 25 2017, 2:26 PM

rebase.

mat updated this revision to Diff 34454.Oct 30 2017, 11:30 AM

rebase & squash.

mat updated this revision to Diff 34455.Oct 30 2017, 11:35 AM

only helpers.

mat updated this revision to Diff 34539.Oct 31 2017, 2:12 PM
  • Remove the OPTIONS_NAME munging wrt FLAVORS.
mat updated this revision to Diff 34676.Nov 2 2017, 3:09 PM

rebase.

mat updated this revision to Diff 35032.Nov 10 2017, 12:41 PM

rebase.

mat updated this revision to Diff 35193.Nov 13 2017, 2:38 PM
  • Add flavor helpers.
mat updated this revision to Diff 35666.Nov 23 2017, 4:44 PM
  • squash! Add flavor helpers.
In D12483#262180, @mat wrote:

I am not against adding a FLAVOR_ prefix, but I think it will make very long variable names, say, FLAVOR_python3_OPTIONS_EXCLUDE= BAR. I think, as options are all UPPERCASE, we simply need to say that flavors are all lowercase, and thus, they cannot conflict.

I'm already confused.

bdrewery requested changes to this revision.Nov 28 2017, 11:40 PM
bdrewery added inline comments.
Mk/bsd.port.mk
1090 ↗(On Diff #35666)

move to its own variable IMO, easier to maintain

1097–1099 ↗(On Diff #35666)

move to its own variable IMO, easier to maintain

This revision now requires changes to proceed.Nov 28 2017, 11:40 PM
In D12483#262180, @mat wrote:

I am not against adding a FLAVOR_ prefix, but I think it will make very long variable names, say, FLAVOR_python3_OPTIONS_EXCLUDE= BAR. I think, as options are all UPPERCASE, we simply need to say that flavors are all lowercase, and thus, they cannot conflict.

I'm already confused.

But I do see there is logic to restrict flavors to lowercase so it is probably fine to keep how you have it.

mat added a comment.Nov 29 2017, 11:39 AM
In D12483#262180, @mat wrote:

I am not against adding a FLAVOR_ prefix, but I think it will make very long variable names, say, FLAVOR_python3_OPTIONS_EXCLUDE= BAR. I think, as options are all UPPERCASE, we simply need to say that flavors are all lowercase, and thus, they cannot conflict.

I'm already confused.

But I do see there is logic to restrict flavors to lowercase so it is probably fine to keep how you have it.

I think it is better to have different namespaces for flavors and OPTIONS, so that they cannot possibly be confused with one another.

mat updated this revision to Diff 35950.Nov 29 2017, 11:41 AM
  • Move helpers list to variables.
bdrewery accepted this revision.Nov 29 2017, 10:21 PM
In D12483#277278, @mat wrote:
In D12483#262180, @mat wrote:

I am not against adding a FLAVOR_ prefix, but I think it will make very long variable names, say, FLAVOR_python3_OPTIONS_EXCLUDE= BAR. I think, as options are all UPPERCASE, we simply need to say that flavors are all lowercase, and thus, they cannot conflict.

I'm already confused.

But I do see there is logic to restrict flavors to lowercase so it is probably fine to keep how you have it.

I think it is better to have different namespaces for flavors and OPTIONS, so that they cannot possibly be confused with one another.

You agree with bapt that we should FLAVOR_<flavor>_BROKEN rather than <flavor>_BROKEN then? You didn't make that change.

This revision is now accepted and ready to land.Nov 29 2017, 10:21 PM
mat added a comment.Nov 30 2017, 9:49 AM
In D12483#277278, @mat wrote:
In D12483#262180, @mat wrote:

I am not against adding a FLAVOR_ prefix, but I think it will make very long variable names, say, FLAVOR_python3_OPTIONS_EXCLUDE= BAR. I think, as options are all UPPERCASE, we simply need to say that flavors are all lowercase, and thus, they cannot conflict.

I'm already confused.

But I do see there is logic to restrict flavors to lowercase so it is probably fine to keep how you have it.

I think it is better to have different namespaces for flavors and OPTIONS, so that they cannot possibly be confused with one another.

You agree with bapt that we should FLAVOR_<flavor>_BROKEN rather than <flavor>_BROKEN then? You didn't make that change.

No, I did not, I said that <flavor>'s namespace was lowercase, and <OPTIONS> were uppercase, so they could not be mixed up.

This revision was automatically updated to reflect the committed changes.