Bug 218213 - print/hplip 3.16.11 always includes Qt
Summary: print/hplip 3.16.11 always includes Qt
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: amd64 Any
: --- Affects Some People
Assignee: Ben Woods
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-29 14:37 UTC by Tom Francis
Modified: 2017-04-02 10:34 UTC (History)
1 user (show)

See Also:
woodsb02: maintainer-feedback+


Attachments
Updated patch to improve clarity of configuration options (1.54 KB, patch)
2017-03-31 05:16 UTC, rkoberman
no flags Details | Diff
Update to improve clarity of GUI options (1.75 KB, patch)
2017-04-01 17:44 UTC, rkoberman
no flags Details | Diff
Update to pkg-plist for the re-work of the GUI options (53.85 KB, patch)
2017-04-01 17:48 UTC, rkoberman
no flags Details | Diff
Patch to fix options logic for x11/qt4/qt5 (1.28 KB, patch)
2017-04-02 03:09 UTC, Ben Woods
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Francis 2017-03-29 14:37:53 UTC
You can disable Xsane, but the port still wants to include Qt.  This happens because there is a radio choice between Qt4 and Qt5.  Whichever choice was selected (and one is always selected because these are radio buttons), will bet SET for building.

If X11 and XSANE are both UNSET, QT4 and QT5 should likewise be UNSET.

If you stop the process after "make config", modify /var/db/ports/print_hplip/options to list "OPTIONS_FILE_UNSET+=QT4" and "OPTIONS_FILE_UNSET+=QT5", then continue building the port, it will build without any problems.
Comment 1 Ben Woods freebsd_committer freebsd_triage 2017-03-30 23:10:24 UTC
Hi Tom,

Radio buttons do allow all options to be disabled (none or only one choice from each group is allowed). Refer to the porter's handbook description here:
https://www.freebsd.org/doc/en/books/porters-handbook/makefile-options.html

If using the dialog4ports to make your selection, simply navigate to the currently enabled option and press space to disable it. Now none of the radios will be selected.

Can you please let me know if this works for you?

Regards,
Ben
Comment 2 Tom Francis 2017-03-30 23:44:45 UTC
It does; this was not obvious, and I've been using CLI menu interfaces for decades. :)

However, I think it would make more sense to re-arrange things to simply make sense.  For example, a number of ports have added "GSSAPI_NONE" to the list of GSSAPI options, to allow users to explicitly disable GSSAPI.  This has been a boon for me, as I no longer have to explain why you can't just pick "GSSAPI_BASE" to the folks who don't need GSSAPI, but do have other ports installed that conflict (or even worse, like me, have ports installed that conflict with all of the GSSAPI options, except the new "NONE" choice).

It's been a long time since I looked at dialog (and I've never looked at dialog4ports), so I'm not sure of the best way to resolve it, but the current choice is far from optimal, especially since adding in Qt5 makes this appear as a regression.
Comment 3 rkoberman 2017-03-31 05:16:52 UTC
Created attachment 181343 [details]
Updated patch to improve clarity of configuration options
Comment 4 rkoberman 2017-03-31 05:19:22 UTC
Newly attached patch removes the X11 and changes the radio buttons to:
     X11_QT5=on: Build with Qt 5 toolkit
     X11_QT4=off: Build with Qt 4 toolkit
     X11_NONE=off: Build without GUI

X11_QT5 is the default.
Comment 5 rkoberman 2017-03-31 06:21:21 UTC
This patch was prepared too quickly. After more consideration, I have re-worked the patch to further clarify it and fix pkg-plist. I will not be able to test the new patch until tomorrow, so I will suggest that no action be taken on it until then.

Sorry for the poor, rushed effort. I should never try even trivial stuff when it'slate and I should be in bed.
Comment 6 rkoberman 2017-04-01 17:44:09 UTC
Created attachment 181386 [details]
Update to improve clarity of GUI options

Patch to Makefile to clarify GUI options and move to a three radio button config of:
QT5     Build with Qt5 GUI
QT4     Build with Qt4 GUI
NO_GUI  Build without GUI
with QT5 being the default

This was complicated because qt4 and gui-build are defaults and had to be disabled by force.
Comment 7 rkoberman 2017-04-01 17:48:23 UTC
Created attachment 181387 [details]
Update to pkg-plist for the re-work of the GUI options

This patch updates the pkg-plist to support the modified GUI options which eliminates the X11 option and replaces it with a NO_GUI radio button
Comment 8 rkoberman 2017-04-01 19:48:58 UTC
One issue that I am uncertain of is purely cosmetic. is the list of radio buttons clearer with:
QT5
QT4
NO_GUI
or with:
QT4_GUI
QT5_GUI
NO_GUI

This is pretty trivial and very trivial to change (though the pkg-plist would also need a quick sed run). The latter is a bit longer, but makes the parallelism correct. 

Blame a couple of junior high English teachers for my considering this issue,but every time I looks at it (since I submitted the patch), I feel more and more strongly that the latter version should be used.

Comments?
Comment 9 Ben Woods freebsd_committer freebsd_triage 2017-04-02 03:09:11 UTC
Created attachment 181397 [details]
Patch to fix options logic for x11/qt4/qt5

Hi Kevin,

Thanks for your patch, however I think I want to go a different way with this, and wanted to get your thoughts.

I think we should stick with having an X11 option, and radio buttons for QT4 or QT5. There are many ports out there with an X11 option (it is somewhat of a standard), and also many ports with QT4 or QT5 radio/single options.

I think we just need to fix the logic that is applied for the different possible combinations of X11/QT4/QT5 to be inline with POLA (the policy of least astonishment).

I propose the attached patch to achieve this:
- If the X11 is disabled, the QT4 or QT5 options are effectively ignored (no dependencies are added and the --disable-qt4 and --disable-qt5 configure arguments are set).
- If X11 is enabled and neither QT4 or QT5 are enabled, an error is given to the user when building to explain this is an invalid combination.

On a separate note, there is no need for QT4/5_DESC=, as they are already described in /usr/ports/Mk/bsd.options.desc.mk
Comment 10 Ben Woods freebsd_committer freebsd_triage 2017-04-02 04:18:05 UTC
To further explain why I wanted to keep the X11 option, it is common for people who are building their own package sets to add OPTIONS_UNSET=X11 to their make.conf(5) file to disable X11 across all ports which support it being disabled.

This make.conf functionality is described in the file below, but requires the ports providing the option to enable/disable GUI support to standardise on using an options named X11.
https://svnweb.freebsd.org/ports/head/Mk/bsd.options.mk?view=markup
Comment 11 rkoberman 2017-04-02 06:17:17 UTC
Comment on attachment 181397 [details]
Patch to fix options logic for x11/qt4/qt5

I had not thought about the global X11 option. I like this solution. I had not considered the BROKEN idea, but it does catch the edge case..

Thanks for the education. This is clearly the right solution.
Comment 12 Ben Woods freebsd_committer freebsd_triage 2017-04-02 10:34:15 UTC
Committed, thanks.
Comment 13 commit-hook freebsd_committer freebsd_triage 2017-04-02 10:34:32 UTC
A commit references this bug:

Author: woodsb02
Date: Sun Apr  2 10:34:07 UTC 2017
New revision: 437548
URL: https://svnweb.freebsd.org/changeset/ports/437548

Log:
  print/hplip: Improve option logic for X11, QT4, QT5

  If X11 option is disabled, the configure args should not enable
  QT4 or QT5, and their dependencies should not be added.

  PR:		218213
  Reported by:	Tom Francis <thomas.francis.jr@pobox.com>
  Reviewed by:	Kevin Oberman <rkoberman@gmail.com>

Changes:
  head/print/hplip/Makefile