Bug 211569 - x11-themes/slim-themes: "magic" to determine ports options isn't valid
Summary: x11-themes/slim-themes: "magic" to determine ports options isn't valid
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Eygene Ryabinkin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-04 05:10 UTC by John Marino
Modified: 2016-08-18 06:03 UTC (History)
0 users

See Also:
bugzilla: maintainer-feedback? (rea)


Attachments
Fix slim-themes options (2.32 KB, patch)
2016-08-05 01:16 UTC, John Marino
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Marino freebsd_committer freebsd_triage 2016-08-04 05:10:32 UTC
while the command "cd /usr/ports/x11-themes/slim-themes; make -V PORT_OPTIONS" might led one to believe the magic used to generate the options is valid, unfortunately it is not.  The PORTS_OPTIONS cannot be manipulated after bsd.ports.pre.mk is included; any change after that point isn't seen.

the prove is this:
make -V SELECTED_OPTIONS
(blank)
make -V UNSELECTED_OPTIONS
ALL_THEMES

If OPTIONS_DEFINE were set legally, they would be distributed across SELECTED_OPTIONS and UNSELECTED_OPTIONS.

Unfortunately another method needs to be found with the last result as just entering the options by hand.  with the exception of another couple of ports and slim-themes, all of ports in the tree have valid SELECTED_OPTIONS/UNSELECTED_OPTIONS.
Comment 1 John Marino freebsd_committer freebsd_triage 2016-08-05 00:36:02 UTC
correction: it's supposed to be DESELECTED_OPTIONS which is actually showing the expected values.

There was another port similar to this one, mail/thunderbird-dictionaries, which I just fixed.  So this is last port I'm aware of with invalid option settings.
Comment 2 John Marino freebsd_committer freebsd_triage 2016-08-05 00:40:38 UTC
by the way, the WWW site is gone (berlios) so that also needs to be replaced or removed ...
Comment 3 John Marino freebsd_committer freebsd_triage 2016-08-05 00:46:37 UTC
I think I can fix this without too many changes.  Let me test my idea and if it passes the build tests, I'll post the patch.
Comment 4 John Marino freebsd_committer freebsd_triage 2016-08-05 01:16:39 UTC
Created attachment 173303 [details]
Fix slim-themes options

Okay, this works for me.
I did remove the "ALL_THEMES" options.  It doesn't buy anything (all options are "ON" by default anyway) and it can actually be confusing when ALL_THEMES is set but an individual theme is unchecked (it would have still installed).

Moreover, using the MULTI option enforces the "at least one selected" requirement so it's also less complex.

On your approval, I can commit it.
Comment 5 commit-hook freebsd_committer freebsd_triage 2016-08-18 06:02:27 UTC
A commit references this bug:

Author: marino
Date: Thu Aug 18 06:02:13 UTC 2016
New revision: 420386
URL: https://svnweb.freebsd.org/changeset/ports/420386

Log:
  x11-themes/slim-themes: fix invalid option definitions

  Defining PORT_OPTIONS directly, especially after makefile fragment
  inclusions, is not valid and results in missing options values (yet masked
  when they are evaluated later after they are needed).

  Rework the automagic to function as intended with the exception of removing
  the redundant and potentially misleading "ALL_THEMES" option.  All the
  themes are already set on by default and to change that required the extra
  step of unchecking ALL_THEMES (without doing so still resulted in all
  being selected even when options are unchecked which is confusing).

  Finally, use the option framework to enforce one choice with MULTI option
  rather than roll-your-own logic.

  PR:		211569
  Approved by:	maintainer timeout
  Discovered by:	Synth sanity check failure

Changes:
  head/x11-themes/slim-themes/Makefile