Bug 113020 - bad OPTIONS syntax and checks in ports/textproc/htmldoc
Summary: bad OPTIONS syntax and checks in ports/textproc/htmldoc
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Ion-Mihai "IOnut" Tetcu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-05-26 05:20 UTC by Eygene Ryabinkin
Modified: 2007-05-28 15:40 UTC (History)
1 user (show)

See Also:


Attachments
file.diff (723 bytes, patch)
2007-05-26 05:20 UTC, Eygene Ryabinkin
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eygene Ryabinkin 2007-05-26 05:20:03 UTC
* OPTIONS statements should carry 'on' or 'off'.  HTMLDOCGUI was
  using 'no'.
* One should check the WITH_/WITHOUT_ values of the opposite kind
  relative to what is told in the OPTIONS.  For the HTMLDOCGUI='off'
  WITH_HTMLDOCGUI should be checked.  See Porter Handbook for the
  explanation.

Fix: The following patch will go.  May be the PORTREVISION should be
bumped.
How-To-Repeat: Define BATCH=yes and compile the port.  It will be built with the
GUI support, but it is intended to be built without GUI by-default.
Comment 1 Edwin Groothuis freebsd_committer freebsd_triage 2007-05-26 05:20:12 UTC
Responsible Changed
From-To: freebsd-ports-bugs->itetcu

itetcu@ wants to have OPTIONS PRs
Comment 2 Edwin Groothuis freebsd_committer freebsd_triage 2007-05-26 05:20:19 UTC
State Changed
From-To: open->feedback

Awaiting maintainers feedback
Comment 3 Rong-En Fan freebsd_committer freebsd_triage 2007-05-26 05:28:24 UTC
On Sat, May 26, 2007 at 08:10:34AM +0400, Eygene Ryabinkin wrote:
> >Description:
> * One should check the WITH_/WITHOUT_ values of the opposite kind
>   relative to what is told in the OPTIONS.  For the HTMLDOCGUI='off'
>   WITH_HTMLDOCGUI should be checked.  See Porter Handbook for the
>   explanation.

No, this should not be the case anymore after OPTIONS
commit to bsd.port.mk few months ago. Porter's Handbook
no longer has that description. 

Regards,
Rong-En Fan
Comment 4 Eygene Ryabinkin 2007-05-26 06:06:33 UTC
Rong-En, good day.

Sat, May 26, 2007 at 12:28:24PM +0800, Rong-En Fan wrote:
> On Sat, May 26, 2007 at 08:10:34AM +0400, Eygene Ryabinkin wrote:
> > >Description:
> > * One should check the WITH_/WITHOUT_ values of the opposite kind
> >   relative to what is told in the OPTIONS.  For the HTMLDOCGUI='off'
> >   WITH_HTMLDOCGUI should be checked.  See Porter Handbook for the
> >   explanation.
> 
> No, this should not be the case anymore after OPTIONS
> commit to bsd.port.mk few months ago. Porter's Handbook
> no longer has that description. 

Ah, you're right: I missed that change.  Thank you!

But still, the right value of the HTMLDOCGUI should be passed.  And
I found the following nit: when I am saying WITHOUT_HTMLDOCGUI=yes,
but fltk is present on my system, then I am getting htmldoc build
with the GUI.  The corrected patch that completely disables GUI if
user does not want it follows.  It still has the WITH_HTMLDOCGUI
check that can be removed.

diff -ur htmldoc.orig/Makefile htmldoc/Makefile
--- htmldoc.orig/Makefile	Sat May 26 07:20:48 2007
+++ htmldoc/Makefile	Sat May 26 08:40:35 2007
@@ -19,7 +19,7 @@
 LIB_DEPENDS=	jpeg:${PORTSDIR}/graphics/jpeg	\
 		png:${PORTSDIR}/graphics/png
 
-OPTIONS=	HTMLDOCGUI "Build GUI front-end" no
+OPTIONS=	HTMLDOCGUI "Build GUI front-end" off
 # .bz2 file is available since Aug 2, but we don't want to
 # start mirroring _in addition to_ the .gz variant. When the
 # vendor's release is updated again, we should switch to .bz2
@@ -48,8 +48,10 @@
 
 .include <bsd.port.pre.mk>
 
-.if !defined(WITHOUT_HTMLDOCGUI)
+.if defined(WITH_HTMLDOCGUI)
 LIB_DEPENDS+=	fltk:${PORTSDIR}/x11-toolkits/fltk
 CONFIGURE_ARGS+=--enable-gui
+.else
+CONFIGURE_ARGS+=--with-gui=no
 .endif
 .include <bsd.port.post.mk>
Comment 5 Jose Fernandes 2007-05-26 07:47:40 UTC
hello I check and this is correct !

I approve the user patch

Edwin Groothuis wrote:
> Maintainer of textproc/htmldoc,
>
> Please note that PR ports/113020 has just been submitted.
>
> If it contains a patch for an upgrade, an enhancement or a bug fix
> you agree on, reply to this email stating that you approve the patch
> and a committer will take care of it.
>
> The full text of the PR can be found at:
>     http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/113020
>
>
Comment 6 Mikhail Teterin 2007-05-26 17:38:38 UTC
On Saturday 26 May 2007 01:06, Eygene Ryabinkin wrote:
= Rong-En, good day.
= 
= Sat, May 26, 2007 at 12:28:24PM +0800, Rong-En Fan wrote:
= > On Sat, May 26, 2007 at 08:10:34AM +0400, Eygene Ryabinkin wrote:
= > > >Description:
= > > * One should check the WITH_/WITHOUT_ values of the opposite kind
= > >   relative to what is told in the OPTIONS.  For the HTMLDOCGUI='off'
= > >   WITH_HTMLDOCGUI should be checked.  See Porter Handbook for the
= > >   explanation.
= > 
= > No, this should not be the case anymore after OPTIONS
= > commit to bsd.port.mk few months ago. Porter's Handbook
= > no longer has that description. 
= 
= Ah, you're right: I missed that change.  Thank you!
= 
= But still, the right value of the HTMLDOCGUI should be passed.  And
= I found the following nit: when I am saying WITHOUT_HTMLDOCGUI=yes,
= but fltk is present on my system, then I am getting htmldoc build
= with the GUI.  The corrected patch that completely disables GUI if
= user does not want it follows.  It still has the WITH_HTMLDOCGUI
= check that can be removed.

What if -- after applying your patch -- a user tries to build with 
WITH_HTMLDOCGUI=no set?

It is my understanding, that if an option FOO is enabled, then WITHOUT_FOO 
will not be set, and WITH_FOO will be. Checking the possible values of each 
of both variables (off, si, no, da, ya) in each port's Makefile is 
impractical.

If anything should do that at all, it may be done by the bsd.port.mk, which 
may want to ensure, the most eggregious cases (like WITH_ and WITHOUT_ set at 
the same time) are reported.

Or is my understanding incorrect? Yours,

	-mi
Comment 7 Eygene Ryabinkin 2007-05-28 06:07:48 UTC
Mikhail, good day.

Sat, May 26, 2007 at 12:38:38PM -0400, Mikhail Teterin wrote:
> On Saturday 26 May 2007 01:06, Eygene Ryabinkin wrote:
> = Sat, May 26, 2007 at 12:28:24PM +0800, Rong-En Fan wrote:
> = > On Sat, May 26, 2007 at 08:10:34AM +0400, Eygene Ryabinkin wrote:
> = > > >Description:
> = > > * One should check the WITH_/WITHOUT_ values of the opposite kind
> = > >   relative to what is told in the OPTIONS.  For the HTMLDOCGUI='off'
> = > >   WITH_HTMLDOCGUI should be checked.  See Porter Handbook for the
> = > >   explanation.
> = > 
> = > No, this should not be the case anymore after OPTIONS
> = > commit to bsd.port.mk few months ago. Porter's Handbook
> = > no longer has that description. 
> = 
> = Ah, you're right: I missed that change.  Thank you!
> = 
> = But still, the right value of the HTMLDOCGUI should be passed.  And
> = I found the following nit: when I am saying WITHOUT_HTMLDOCGUI=yes,
> = but fltk is present on my system, then I am getting htmldoc build
> = with the GUI.  The corrected patch that completely disables GUI if
> = user does not want it follows.  It still has the WITH_HTMLDOCGUI
> = check that can be removed.
> 
> What if -- after applying your patch -- a user tries to build with 
> WITH_HTMLDOCGUI=no set?

WITH_*/WITHOUT_* values were always been just flags, if I understand
the things correctly.  So, if WITH_HTMLDOCGUI is defined, then user
wants no GUI, regardless of the exact value of the variable. See

  http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/makefile-options.html#AEN2518

it has the example and, by accident, it explains why the second,
corrected patch is needed.

> It is my understanding, that if an option FOO is enabled, then WITHOUT_FOO 
> will not be set, and WITH_FOO will be. Checking the possible values of each 
> of both variables (off, si, no, da, ya) in each port's Makefile is 
> impractical.

You're probably referring to the 'no' -> 'off' replacement in the
OPTIONS in my patch?  If yes, then the problem was in the batch
build, when the BATCH variable is set (automatic package building
seems to be also affected).  So I am referring to the cases when no
fancy options menu is ever invoked and no WITH_/WITHOUT_ values are
specified: port just builds with its defaults.  Perhaps I should
mention this in the original PR -- mea culpa.

The bsd.port.mk checks only for the 'on'/'off' tokens in the OPTIONS:
-----
WITHOUT:=
WITH:=
.       if defined(OPTIONS)
REALOPTIONS=${OPTIONS:C/".*"//g}
.       for O in ${REALOPTIONS}
RO:=${O}
.       if ${RO:L} == off
WITHOUT:=       ${WITHOUT} ${OPT}
.       endif
.       if ${RO:L} == on
WITH:=          ${WITH} ${OPT}
.       endif
OPT:=${RO}
.       endfor
-----
So, if no options menu is invoked, then for the original Makefile
neither WITH_HTMLDOCGUI nor WITHOUT_HTMLDOCGUI will be defined, so
port will be always built with the flkt dependency and the GUI due
to the nature of the original check: '.if !defined(WITHOUT_HTMLDOCGUI)'

In the original PR I was referring to the old (and obsoleted)
situation when we had to check the opposite variable and Rong-En
kindly pointed me to that.  But the bad thing was that 'no' token
was specified in OPTIONS instead of 'off'. See
  http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/makefile-options.html#AEN2472

With options menu all had worked as expected, because 'dialog'
had treated your 'no' as the absence of the option: it treats
all unknown 'status' verbs as 'off': see
/usr/src/gnu/lib/libdialog/checklist.c, below the line 59:
-----
    if (cnt >= 0) {
        items = it;
        ditems = NULL;
        item_no = cnt;
        /* Initializes status */
        for (i = 0; i < item_no; i++)
            status[i] = !strcasecmp(items[i*3 + 2], "on");
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    }
-----

But again, the right values for the 'status' are 'on'/'off': 'man
dialog_checklist' and '/title and state '.

> If anything should do that at all, it may be done by the bsd.port.mk, which 
> may want to ensure, the most eggregious cases (like WITH_ and WITHOUT_ set at 
> the same time) are reported.

The problem is not about the strange cases you're describing above,
sorry.  But maybe the sanity check about WITH_<VERB> and WITHOUT_<VERB>
variables from OPTIONS being defined at the same time will be good.
-- 
Eygene
Comment 8 Mikhail Teterin 2007-05-28 10:18:35 UTC
I must've misunderstood then. Can the proposed change be summarized in one 
hunk:

--- Makefile    2 May 2007 19:35:53 -0000       1.13
+++ Makefile    28 May 2007 09:18:01 -0000
@@ -22 +22 @@
-OPTIONS=       HTMLDOCGUI "Build GUI front-end" no
+OPTIONS=       HTMLDOCGUI "Build GUI front-end" off

Is this, what you mean? Thanks!

	-mi
Comment 9 Eygene Ryabinkin 2007-05-28 10:34:28 UTC
Mikhail, good day.

Mon, May 28, 2007 at 05:18:35AM -0400, Mikhail Teterin wrote:
> I must've misunderstood then. Can the proposed change be summarized in one 
> hunk:
>
> --- Makefile    2 May 2007 19:35:53 -0000       1.13
> +++ Makefile    28 May 2007 09:18:01 -0000
> @@ -22 +22 @@
> -OPTIONS=       HTMLDOCGUI "Build GUI front-end" no
> +OPTIONS=       HTMLDOCGUI "Build GUI front-end" off

No, you should also add the 'else' clause to the
'!defined(WITHOUT_HTMLDOCGUI)' branch and disable GUI building
there: fltk can be present in the system and GUI will still be
installed in the current port variant.  Something like this:
-----
 LIB_DEPENDS+=  fltk:${PORTSDIR}/x11-toolkits/fltk
 CONFIGURE_ARGS+=--enable-gui
+.else
+CONFIGURE_ARGS+=--with-gui=no
 .endif
 .include <bsd.port.post.mk>
-----

Thank you!
-- 
Eygene
Comment 10 dfilter service freebsd_committer freebsd_triage 2007-05-28 15:37:59 UTC
mi          2007-05-28 14:37:53 UTC

  FreeBSD ports repository

  Modified files:
    textproc/htmldoc     Makefile 
  Log:
  Use `off' instead of `no' for the default value of OPTION.
  Add an explicit --disable-gui to CONFIGURE_ARGS, when no GUI
  is requested. Without this, vendors software would still build
  with GUI-support, if fltk is present on the system.
  
  Submitted by:    Eygene Ryabinkin
  PR:     ports/113020
  
  Revision  Changes    Path
  1.15      +3 -1      ports/textproc/htmldoc/Makefile
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
Comment 11 Mikhail Teterin freebsd_committer freebsd_triage 2007-05-28 15:38:07 UTC
State Changed
From-To: feedback->closed

Problems fixed. Thanks!