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

  • use !empty instead of defined.
  • Change OPTIONS_NAME to include the FLAVOR, if defined and not default.
  • Rename .for variable.
  • Add a first idea for what to do with options.
  • Fix the helpers I added.
  • Add a guard, it seems when we get through this code more than once.
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.

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.

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.

  • Rename option helpers to _FORCE and _EXCLUDE.
mat marked 3 inline comments as done.Oct 4 2017, 1:42 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 :)

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.

  • Add IGNORE/BROKEN flavor helpers.
  • Remove flavors options helpers.
  • Add *_DEPENDS for flavors helpers.
  • Remove the OPTIONS_NAME munging wrt FLAVORS.
  • 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 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.

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.

  • Move helpers list to variables.
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
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.