[Spice-devel] Make asciidoc a hard requirement

Submitted by Eduardo Lima (Etrunko) on April 25, 2016, 9:23 p.m.

Details

Message ID 1461619405-18193-1-git-send-email-etrunko@redhat.com
State New
Headers show
Series "Make asciidoc a hard requirement" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Eduardo Lima (Etrunko) April 25, 2016, 9:23 p.m.
The problem happens when you run 'make dist' in a system without
asciidoc installed. Even though in configure time there is a check for
building the manual, it is required to be built for distribution.

Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
---
 configure.ac            | 25 ++++++-------------------
 docs/Makefile.am        |  2 --
 docs/manual/Makefile.am |  8 +-------
 3 files changed, 7 insertions(+), 28 deletions(-)

Patch hide | download patch | download mbox

diff --git a/configure.ac b/configure.ac
index 8419508..18b907a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -155,24 +155,12 @@  if test "x$enable_automated_tests" = "xyes"; then
 fi
 
 
-AC_ARG_ENABLE([manual],
-               AS_HELP_STRING([--enable-manual=@<:@auto/yes/no@:>@],
-                              [Build SPICE manual]),
-               [],
-               [enable_manual="auto"])
-if test "x$enable_manual" != "xno"; then
-    AC_PATH_PROG([ASCIIDOC], [asciidoc])
-    AS_IF([test -z "$ASCIIDOC" && test "x$enable_manual" = "xyes"],
-          [AC_MSG_ERROR([asciidoc is missing and build of manual was requested])])
-    AC_PATH_PROG([A2X], [a2x])
-    AS_IF([test -z "$A2X" && test "x$enable_manual" = "xyes"],
-          [AC_MSG_ERROR([a2x is missing and build of manual was requested])])
-fi
-AS_IF([test -n "$ASCIIDOC"], [have_asciidoc=yes], [have_asciidoc=no])
-AM_CONDITIONAL([BUILD_MANUAL], [test -n "$ASCIIDOC" || test -n "$A2X"])
-AM_CONDITIONAL([BUILD_HTML_MANUAL], [test -n "$ASCIIDOC"])
-AM_CONDITIONAL([BUILD_CHUNKED_MANUAL], [test -n "$A2X"])
-
+AC_PATH_PROG([ASCIIDOC], [asciidoc])
+AS_IF([test -z "$ASCIIDOC"],
+      [AC_MSG_ERROR([asciidoc is missing])])
+AC_PATH_PROG([A2X], [a2x])
+AS_IF([test -z "$A2X"],
+      [AC_MSG_ERROR([a2x is missing])])
 
 dnl ===========================================================================
 dnl check compiler flags
@@ -245,7 +233,6 @@  AC_MSG_NOTICE([
         Smartcard:                ${have_smartcard}
         SASL support:             ${have_sasl}
         Automated tests:          ${enable_automated_tests}
-        Manual:                   ${have_asciidoc}
 
         Now type 'make' to build $PACKAGE
 ])
diff --git a/docs/Makefile.am b/docs/Makefile.am
index 18e785f..e76efaf 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -1,3 +1 @@ 
-if BUILD_MANUAL
 SUBDIRS = manual
-endif
diff --git a/docs/manual/Makefile.am b/docs/manual/Makefile.am
index 24a11fe..beda615 100644
--- a/docs/manual/Makefile.am
+++ b/docs/manual/Makefile.am
@@ -16,13 +16,7 @@  EXTRA_DIST =					\
 manual.chunked: manual.txt
 	$(AM_V_GEN) $(A2X) -f chunked -D $(builddir) $(ASCIIDOC_FLAGS) $<
 
-docfiles =
-if BUILD_HTML_MANUAL
-docfiles += manual.html
-endif
-if BUILD_CHUNKED_MANUAL
-docfiles += manual.chunked
-endif
+docfiles = manual.html manual.chunked
 
 all-local: $(docfiles)
 

Comments

Any opinions here?

On 04/25/2016 06:23 PM, Eduardo Lima (Etrunko) wrote:
> The problem happens when you run 'make dist' in a system without
> asciidoc installed. Even though in configure time there is a check for
> building the manual, it is required to be built for distribution.
> 
> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
> ---
>  configure.ac            | 25 ++++++-------------------
>  docs/Makefile.am        |  2 --
>  docs/manual/Makefile.am |  8 +-------
>  3 files changed, 7 insertions(+), 28 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 8419508..18b907a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -155,24 +155,12 @@ if test "x$enable_automated_tests" = "xyes"; then
>  fi
>  
>  
> -AC_ARG_ENABLE([manual],
> -               AS_HELP_STRING([--enable-manual=@<:@auto/yes/no@:>@],
> -                              [Build SPICE manual]),
> -               [],
> -               [enable_manual="auto"])
> -if test "x$enable_manual" != "xno"; then
> -    AC_PATH_PROG([ASCIIDOC], [asciidoc])
> -    AS_IF([test -z "$ASCIIDOC" && test "x$enable_manual" = "xyes"],
> -          [AC_MSG_ERROR([asciidoc is missing and build of manual was requested])])
> -    AC_PATH_PROG([A2X], [a2x])
> -    AS_IF([test -z "$A2X" && test "x$enable_manual" = "xyes"],
> -          [AC_MSG_ERROR([a2x is missing and build of manual was requested])])
> -fi
> -AS_IF([test -n "$ASCIIDOC"], [have_asciidoc=yes], [have_asciidoc=no])
> -AM_CONDITIONAL([BUILD_MANUAL], [test -n "$ASCIIDOC" || test -n "$A2X"])
> -AM_CONDITIONAL([BUILD_HTML_MANUAL], [test -n "$ASCIIDOC"])
> -AM_CONDITIONAL([BUILD_CHUNKED_MANUAL], [test -n "$A2X"])
> -
> +AC_PATH_PROG([ASCIIDOC], [asciidoc])
> +AS_IF([test -z "$ASCIIDOC"],
> +      [AC_MSG_ERROR([asciidoc is missing])])
> +AC_PATH_PROG([A2X], [a2x])
> +AS_IF([test -z "$A2X"],
> +      [AC_MSG_ERROR([a2x is missing])])
>  
>  dnl ===========================================================================
>  dnl check compiler flags
> @@ -245,7 +233,6 @@ AC_MSG_NOTICE([
>          Smartcard:                ${have_smartcard}
>          SASL support:             ${have_sasl}
>          Automated tests:          ${enable_automated_tests}
> -        Manual:                   ${have_asciidoc}
>  
>          Now type 'make' to build $PACKAGE
>  ])
> diff --git a/docs/Makefile.am b/docs/Makefile.am
> index 18e785f..e76efaf 100644
> --- a/docs/Makefile.am
> +++ b/docs/Makefile.am
> @@ -1,3 +1 @@
> -if BUILD_MANUAL
>  SUBDIRS = manual
> -endif
> diff --git a/docs/manual/Makefile.am b/docs/manual/Makefile.am
> index 24a11fe..beda615 100644
> --- a/docs/manual/Makefile.am
> +++ b/docs/manual/Makefile.am
> @@ -16,13 +16,7 @@ EXTRA_DIST =					\
>  manual.chunked: manual.txt
>  	$(AM_V_GEN) $(A2X) -f chunked -D $(builddir) $(ASCIIDOC_FLAGS) $<
>  
> -docfiles =
> -if BUILD_HTML_MANUAL
> -docfiles += manual.html
> -endif
> -if BUILD_CHUNKED_MANUAL
> -docfiles += manual.chunked
> -endif
> +docfiles = manual.html manual.chunked
>  
>  all-local: $(docfiles)
>  
>
Hey,

On Mon, Apr 25, 2016 at 06:23:25PM -0300, Eduardo Lima (Etrunko) wrote:
> The problem happens when you run 'make dist' in a system without
> asciidoc installed. Even though in configure time there is a check for
> building the manual, it is required to be built for distribution.
> 
> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
> ---
>  configure.ac            | 25 ++++++-------------------
>  docs/Makefile.am        |  2 --
>  docs/manual/Makefile.am |  8 +-------
>  3 files changed, 7 insertions(+), 28 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 8419508..18b907a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -155,24 +155,12 @@ if test "x$enable_automated_tests" = "xyes"; then
>  fi
>  
>  
> -AC_ARG_ENABLE([manual],
> -               AS_HELP_STRING([--enable-manual=@<:@auto/yes/no@:>@],
> -                              [Build SPICE manual]),
> -               [],
> -               [enable_manual="auto"])
> -if test "x$enable_manual" != "xno"; then
> -    AC_PATH_PROG([ASCIIDOC], [asciidoc])
> -    AS_IF([test -z "$ASCIIDOC" && test "x$enable_manual" = "xyes"],
> -          [AC_MSG_ERROR([asciidoc is missing and build of manual was requested])])
> -    AC_PATH_PROG([A2X], [a2x])
> -    AS_IF([test -z "$A2X" && test "x$enable_manual" = "xyes"],
> -          [AC_MSG_ERROR([a2x is missing and build of manual was requested])])
> -fi
> -AS_IF([test -n "$ASCIIDOC"], [have_asciidoc=yes], [have_asciidoc=no])
> -AM_CONDITIONAL([BUILD_MANUAL], [test -n "$ASCIIDOC" || test -n "$A2X"])
> -AM_CONDITIONAL([BUILD_HTML_MANUAL], [test -n "$ASCIIDOC"])
> -AM_CONDITIONAL([BUILD_CHUNKED_MANUAL], [test -n "$A2X"])
> -
> +AC_PATH_PROG([ASCIIDOC], [asciidoc])
> +AS_IF([test -z "$ASCIIDOC"],
> +      [AC_MSG_ERROR([asciidoc is missing])])
> +AC_PATH_PROG([A2X], [a2x])
> +AS_IF([test -z "$A2X"],
> +      [AC_MSG_ERROR([a2x is missing])])

I agree with by depending on asciidoc but why are you removing the
enable-manual check?

>
>  dnl ===========================================================================
>  dnl check compiler flags
> @@ -245,7 +233,6 @@ AC_MSG_NOTICE([
>          Smartcard:                ${have_smartcard}
>          SASL support:             ${have_sasl}
>          Automated tests:          ${enable_automated_tests}
> -        Manual:                   ${have_asciidoc}

I think we should keep this with the using the ${enable_manual}

Reviewed-by: Victor Toso <victortoso@redhat.com>

>
>          Now type 'make' to build $PACKAGE
>  ])
> diff --git a/docs/Makefile.am b/docs/Makefile.am
> index 18e785f..e76efaf 100644
> --- a/docs/Makefile.am
> +++ b/docs/Makefile.am
> @@ -1,3 +1 @@
> -if BUILD_MANUAL
>  SUBDIRS = manual
> -endif
> diff --git a/docs/manual/Makefile.am b/docs/manual/Makefile.am
> index 24a11fe..beda615 100644
> --- a/docs/manual/Makefile.am
> +++ b/docs/manual/Makefile.am
> @@ -16,13 +16,7 @@ EXTRA_DIST =					\
>  manual.chunked: manual.txt
>  	$(AM_V_GEN) $(A2X) -f chunked -D $(builddir) $(ASCIIDOC_FLAGS) $<
>  
> -docfiles =
> -if BUILD_HTML_MANUAL
> -docfiles += manual.html
> -endif
> -if BUILD_CHUNKED_MANUAL
> -docfiles += manual.chunked
> -endif
> +docfiles = manual.html manual.chunked
>  
>  all-local: $(docfiles)
>  
> -- 
> 2.5.5
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> Hey,
> 
> On Mon, Apr 25, 2016 at 06:23:25PM -0300, Eduardo Lima (Etrunko) wrote:
> > The problem happens when you run 'make dist' in a system without
> > asciidoc installed. Even though in configure time there is a check for
> > building the manual, it is required to be built for distribution.
> > 
> > Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
> > ---
> >  configure.ac            | 25 ++++++-------------------
> >  docs/Makefile.am        |  2 --
> >  docs/manual/Makefile.am |  8 +-------
> >  3 files changed, 7 insertions(+), 28 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 8419508..18b907a 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -155,24 +155,12 @@ if test "x$enable_automated_tests" = "xyes"; then
> >  fi
> >  
> >  
> > -AC_ARG_ENABLE([manual],
> > -               AS_HELP_STRING([--enable-manual=@<:@auto/yes/no@:>@],
> > -                              [Build SPICE manual]),
> > -               [],
> > -               [enable_manual="auto"])
> > -if test "x$enable_manual" != "xno"; then
> > -    AC_PATH_PROG([ASCIIDOC], [asciidoc])
> > -    AS_IF([test -z "$ASCIIDOC" && test "x$enable_manual" = "xyes"],
> > -          [AC_MSG_ERROR([asciidoc is missing and build of manual was
> > requested])])
> > -    AC_PATH_PROG([A2X], [a2x])
> > -    AS_IF([test -z "$A2X" && test "x$enable_manual" = "xyes"],
> > -          [AC_MSG_ERROR([a2x is missing and build of manual was
> > requested])])
> > -fi
> > -AS_IF([test -n "$ASCIIDOC"], [have_asciidoc=yes], [have_asciidoc=no])
> > -AM_CONDITIONAL([BUILD_MANUAL], [test -n "$ASCIIDOC" || test -n "$A2X"])
> > -AM_CONDITIONAL([BUILD_HTML_MANUAL], [test -n "$ASCIIDOC"])
> > -AM_CONDITIONAL([BUILD_CHUNKED_MANUAL], [test -n "$A2X"])
> > -
> > +AC_PATH_PROG([ASCIIDOC], [asciidoc])
> > +AS_IF([test -z "$ASCIIDOC"],
> > +      [AC_MSG_ERROR([asciidoc is missing])])
> > +AC_PATH_PROG([A2X], [a2x])
> > +AS_IF([test -z "$A2X"],
> > +      [AC_MSG_ERROR([a2x is missing])])
> 
> I agree with by depending on asciidoc but why are you removing the
> enable-manual check?
> 

Because user need to build the documentation (I think this is what Eduardo means
by hard requirement).

> >
> >  dnl
> >  ===========================================================================
> >  dnl check compiler flags
> > @@ -245,7 +233,6 @@ AC_MSG_NOTICE([
> >          Smartcard:                ${have_smartcard}
> >          SASL support:             ${have_sasl}
> >          Automated tests:          ${enable_automated_tests}
> > -        Manual:                   ${have_asciidoc}
> 
> I think we should keep this with the using the ${enable_manual}
> 
> Reviewed-by: Victor Toso <victortoso@redhat.com>
> 
> >
> >          Now type 'make' to build $PACKAGE
> >  ])
> > diff --git a/docs/Makefile.am b/docs/Makefile.am
> > index 18e785f..e76efaf 100644
> > --- a/docs/Makefile.am
> > +++ b/docs/Makefile.am
> > @@ -1,3 +1 @@
> > -if BUILD_MANUAL
> >  SUBDIRS = manual
> > -endif
> > diff --git a/docs/manual/Makefile.am b/docs/manual/Makefile.am
> > index 24a11fe..beda615 100644
> > --- a/docs/manual/Makefile.am
> > +++ b/docs/manual/Makefile.am
> > @@ -16,13 +16,7 @@ EXTRA_DIST =					\
> >  manual.chunked: manual.txt
> >  	$(AM_V_GEN) $(A2X) -f chunked -D $(builddir) $(ASCIIDOC_FLAGS) $<
> >  
> > -docfiles =
> > -if BUILD_HTML_MANUAL
> > -docfiles += manual.html
> > -endif
> > -if BUILD_CHUNKED_MANUAL
> > -docfiles += manual.chunked
> > -endif
> > +docfiles = manual.html manual.chunked
> >  
> >  all-local: $(docfiles)
> >  

One different approach IMHO could be:
- remove enable-manual option;
- look for asciidoc and a2x;
- give error trying to "make dist" if asciidoc or a2x are missing.
This could be easily done replacing ASCIIDOC and A2X with "false" command.

I think what Eduardo wants to achieve is avoiding make dist to package
a partial build (but maybe I'm wrong).

Frediano
On 04/26/2016 12:23 AM, Eduardo Lima (Etrunko) wrote:
> The problem happens when you run 'make dist' in a system without
> asciidoc installed. Even though in configure time there is a check for
> building the manual, it is required to be built for distribution.

Hi,

I think we should not force building the manual nor require asciidoc
to build spice-server.

We better find another way to make 'make dist' work

Regards,
     Uri.


>
> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
> ---
>  configure.ac            | 25 ++++++-------------------
>  docs/Makefile.am        |  2 --
>  docs/manual/Makefile.am |  8 +-------
>  3 files changed, 7 insertions(+), 28 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 8419508..18b907a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -155,24 +155,12 @@ if test "x$enable_automated_tests" = "xyes"; then
>  fi
>
>
> -AC_ARG_ENABLE([manual],
> -               AS_HELP_STRING([--enable-manual=@<:@auto/yes/no@:>@],
> -                              [Build SPICE manual]),
> -               [],
> -               [enable_manual="auto"])
> -if test "x$enable_manual" != "xno"; then
> -    AC_PATH_PROG([ASCIIDOC], [asciidoc])
> -    AS_IF([test -z "$ASCIIDOC" && test "x$enable_manual" = "xyes"],
> -          [AC_MSG_ERROR([asciidoc is missing and build of manual was requested])])
> -    AC_PATH_PROG([A2X], [a2x])
> -    AS_IF([test -z "$A2X" && test "x$enable_manual" = "xyes"],
> -          [AC_MSG_ERROR([a2x is missing and build of manual was requested])])
> -fi
> -AS_IF([test -n "$ASCIIDOC"], [have_asciidoc=yes], [have_asciidoc=no])
> -AM_CONDITIONAL([BUILD_MANUAL], [test -n "$ASCIIDOC" || test -n "$A2X"])
> -AM_CONDITIONAL([BUILD_HTML_MANUAL], [test -n "$ASCIIDOC"])
> -AM_CONDITIONAL([BUILD_CHUNKED_MANUAL], [test -n "$A2X"])
> -
> +AC_PATH_PROG([ASCIIDOC], [asciidoc])
> +AS_IF([test -z "$ASCIIDOC"],
> +      [AC_MSG_ERROR([asciidoc is missing])])
> +AC_PATH_PROG([A2X], [a2x])
> +AS_IF([test -z "$A2X"],
> +      [AC_MSG_ERROR([a2x is missing])])
>
>  dnl ===========================================================================
>  dnl check compiler flags
> @@ -245,7 +233,6 @@ AC_MSG_NOTICE([
>          Smartcard:                ${have_smartcard}
>          SASL support:             ${have_sasl}
>          Automated tests:          ${enable_automated_tests}
> -        Manual:                   ${have_asciidoc}
>
>          Now type 'make' to build $PACKAGE
>  ])
> diff --git a/docs/Makefile.am b/docs/Makefile.am
> index 18e785f..e76efaf 100644
> --- a/docs/Makefile.am
> +++ b/docs/Makefile.am
> @@ -1,3 +1 @@
> -if BUILD_MANUAL
>  SUBDIRS = manual
> -endif
> diff --git a/docs/manual/Makefile.am b/docs/manual/Makefile.am
> index 24a11fe..beda615 100644
> --- a/docs/manual/Makefile.am
> +++ b/docs/manual/Makefile.am
> @@ -16,13 +16,7 @@ EXTRA_DIST =					\
>  manual.chunked: manual.txt
>  	$(AM_V_GEN) $(A2X) -f chunked -D $(builddir) $(ASCIIDOC_FLAGS) $<
>
> -docfiles =
> -if BUILD_HTML_MANUAL
> -docfiles += manual.html
> -endif
> -if BUILD_CHUNKED_MANUAL
> -docfiles += manual.chunked
> -endif
> +docfiles = manual.html manual.chunked
>
>  all-local: $(docfiles)
>
>
On Mon, Apr 25, 2016 at 06:23:25PM -0300, Eduardo Lima (Etrunko) wrote:
> The problem happens when you run 'make dist' in a system without
> asciidoc installed. Even though in configure time there is a check for
> building the manual, it is required to be built for distribution.

Dunno if we should change it, or live with it, make distcheck will be
using --enable-asciidoc. glib is not doing much better iirc.
Maybe we could have something like
if !BUILD_MANUAL
dist-hook:
  //error out saying manual build must be enabled
endif

dunno if this can work.

Christophe
On 04/30/2016 04:19 AM, Victor Toso wrote:
> Hey,
> 
> On Mon, Apr 25, 2016 at 06:23:25PM -0300, Eduardo Lima (Etrunko) wrote:
>> The problem happens when you run 'make dist' in a system without
>> asciidoc installed. Even though in configure time there is a check for
>> building the manual, it is required to be built for distribution.
>>
>> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
>> ---
>>  configure.ac            | 25 ++++++-------------------
>>  docs/Makefile.am        |  2 --
>>  docs/manual/Makefile.am |  8 +-------
>>  3 files changed, 7 insertions(+), 28 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 8419508..18b907a 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -155,24 +155,12 @@ if test "x$enable_automated_tests" = "xyes"; then
>>  fi
>>  
>>  
>> -AC_ARG_ENABLE([manual],
>> -               AS_HELP_STRING([--enable-manual=@<:@auto/yes/no@:>@],
>> -                              [Build SPICE manual]),
>> -               [],
>> -               [enable_manual="auto"])
>> -if test "x$enable_manual" != "xno"; then
>> -    AC_PATH_PROG([ASCIIDOC], [asciidoc])
>> -    AS_IF([test -z "$ASCIIDOC" && test "x$enable_manual" = "xyes"],
>> -          [AC_MSG_ERROR([asciidoc is missing and build of manual was requested])])
>> -    AC_PATH_PROG([A2X], [a2x])
>> -    AS_IF([test -z "$A2X" && test "x$enable_manual" = "xyes"],
>> -          [AC_MSG_ERROR([a2x is missing and build of manual was requested])])
>> -fi
>> -AS_IF([test -n "$ASCIIDOC"], [have_asciidoc=yes], [have_asciidoc=no])
>> -AM_CONDITIONAL([BUILD_MANUAL], [test -n "$ASCIIDOC" || test -n "$A2X"])
>> -AM_CONDITIONAL([BUILD_HTML_MANUAL], [test -n "$ASCIIDOC"])
>> -AM_CONDITIONAL([BUILD_CHUNKED_MANUAL], [test -n "$A2X"])
>> -
>> +AC_PATH_PROG([ASCIIDOC], [asciidoc])
>> +AS_IF([test -z "$ASCIIDOC"],
>> +      [AC_MSG_ERROR([asciidoc is missing])])
>> +AC_PATH_PROG([A2X], [a2x])
>> +AS_IF([test -z "$A2X"],
>> +      [AC_MSG_ERROR([a2x is missing])])
> 
> I agree with by depending on asciidoc but why are you removing the
> enable-manual check?
> 

As it says in the commit message, the problem is that --enable-manual
does not take effect with make dist or even distcheck, and it will fail
if there is no asciidoc installed. In my head, it does not make to use
an enable flag when it actually fails to build a tarball, hence I simply
removed that check.


>>
>>  dnl ===========================================================================
>>  dnl check compiler flags
>> @@ -245,7 +233,6 @@ AC_MSG_NOTICE([
>>          Smartcard:                ${have_smartcard}
>>          SASL support:             ${have_sasl}
>>          Automated tests:          ${enable_automated_tests}
>> -        Manual:                   ${have_asciidoc}
> 
> I think we should keep this with the using the ${enable_manual}
> 
> Reviewed-by: Victor Toso <victortoso@redhat.com>
> 
>>
>>          Now type 'make' to build $PACKAGE
>>  ])
>> diff --git a/docs/Makefile.am b/docs/Makefile.am
>> index 18e785f..e76efaf 100644
>> --- a/docs/Makefile.am
>> +++ b/docs/Makefile.am
>> @@ -1,3 +1 @@
>> -if BUILD_MANUAL
>>  SUBDIRS = manual
>> -endif
>> diff --git a/docs/manual/Makefile.am b/docs/manual/Makefile.am
>> index 24a11fe..beda615 100644
>> --- a/docs/manual/Makefile.am
>> +++ b/docs/manual/Makefile.am
>> @@ -16,13 +16,7 @@ EXTRA_DIST =					\
>>  manual.chunked: manual.txt
>>  	$(AM_V_GEN) $(A2X) -f chunked -D $(builddir) $(ASCIIDOC_FLAGS) $<
>>  
>> -docfiles =
>> -if BUILD_HTML_MANUAL
>> -docfiles += manual.html
>> -endif
>> -if BUILD_CHUNKED_MANUAL
>> -docfiles += manual.chunked
>> -endif
>> +docfiles = manual.html manual.chunked
>>  
>>  all-local: $(docfiles)
>>  
>> -- 
>> 2.5.5
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
On 05/02/2016 07:39 AM, Uri Lublin wrote:
> On 04/26/2016 12:23 AM, Eduardo Lima (Etrunko) wrote:
>> The problem happens when you run 'make dist' in a system without
>> asciidoc installed. Even though in configure time there is a check for
>> building the manual, it is required to be built for distribution.
> 
> Hi,
> 
> I think we should not force building the manual nor require asciidoc
> to build spice-server.
> 

I disagree. The output of make dist should be the same no matter what
system the command is run. Users should have the required dependencies
installed if they are interested in building spice-server from the
source code tarball.

> We better find another way to make 'make dist' work
> 
> Regards,
>     Uri.
> 

The only other option I see is to keep track of the generated files in
git, and if there is any modifications, generate it again manually.

> 
>>
>> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
>> ---
>>  configure.ac            | 25 ++++++-------------------
>>  docs/Makefile.am        |  2 --
>>  docs/manual/Makefile.am |  8 +-------
>>  3 files changed, 7 insertions(+), 28 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 8419508..18b907a 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -155,24 +155,12 @@ if test "x$enable_automated_tests" = "xyes"; then
>>  fi
>>
>>
>> -AC_ARG_ENABLE([manual],
>> -               AS_HELP_STRING([--enable-manual=@<:@auto/yes/no@:>@],
>> -                              [Build SPICE manual]),
>> -               [],
>> -               [enable_manual="auto"])
>> -if test "x$enable_manual" != "xno"; then
>> -    AC_PATH_PROG([ASCIIDOC], [asciidoc])
>> -    AS_IF([test -z "$ASCIIDOC" && test "x$enable_manual" = "xyes"],
>> -          [AC_MSG_ERROR([asciidoc is missing and build of manual was
>> requested])])
>> -    AC_PATH_PROG([A2X], [a2x])
>> -    AS_IF([test -z "$A2X" && test "x$enable_manual" = "xyes"],
>> -          [AC_MSG_ERROR([a2x is missing and build of manual was
>> requested])])
>> -fi
>> -AS_IF([test -n "$ASCIIDOC"], [have_asciidoc=yes], [have_asciidoc=no])
>> -AM_CONDITIONAL([BUILD_MANUAL], [test -n "$ASCIIDOC" || test -n "$A2X"])
>> -AM_CONDITIONAL([BUILD_HTML_MANUAL], [test -n "$ASCIIDOC"])
>> -AM_CONDITIONAL([BUILD_CHUNKED_MANUAL], [test -n "$A2X"])
>> -
>> +AC_PATH_PROG([ASCIIDOC], [asciidoc])
>> +AS_IF([test -z "$ASCIIDOC"],
>> +      [AC_MSG_ERROR([asciidoc is missing])])
>> +AC_PATH_PROG([A2X], [a2x])
>> +AS_IF([test -z "$A2X"],
>> +      [AC_MSG_ERROR([a2x is missing])])
>>
>>  dnl
>> ===========================================================================
>>
>>  dnl check compiler flags
>> @@ -245,7 +233,6 @@ AC_MSG_NOTICE([
>>          Smartcard:                ${have_smartcard}
>>          SASL support:             ${have_sasl}
>>          Automated tests:          ${enable_automated_tests}
>> -        Manual:                   ${have_asciidoc}
>>
>>          Now type 'make' to build $PACKAGE
>>  ])
>> diff --git a/docs/Makefile.am b/docs/Makefile.am
>> index 18e785f..e76efaf 100644
>> --- a/docs/Makefile.am
>> +++ b/docs/Makefile.am
>> @@ -1,3 +1 @@
>> -if BUILD_MANUAL
>>  SUBDIRS = manual
>> -endif
>> diff --git a/docs/manual/Makefile.am b/docs/manual/Makefile.am
>> index 24a11fe..beda615 100644
>> --- a/docs/manual/Makefile.am
>> +++ b/docs/manual/Makefile.am
>> @@ -16,13 +16,7 @@ EXTRA_DIST =                    \
>>  manual.chunked: manual.txt
>>      $(AM_V_GEN) $(A2X) -f chunked -D $(builddir) $(ASCIIDOC_FLAGS) $<
>>
>> -docfiles =
>> -if BUILD_HTML_MANUAL
>> -docfiles += manual.html
>> -endif
>> -if BUILD_CHUNKED_MANUAL
>> -docfiles += manual.chunked
>> -endif
>> +docfiles = manual.html manual.chunked
>>
>>  all-local: $(docfiles)
>>
>>
>
On 05/02/2016 03:32 PM, Eduardo Lima (Etrunko) wrote:
> On 05/02/2016 07:39 AM, Uri Lublin wrote:
>> On 04/26/2016 12:23 AM, Eduardo Lima (Etrunko) wrote:
>>> The problem happens when you run 'make dist' in a system without
>>> asciidoc installed. Even though in configure time there is a check for
>>> building the manual, it is required to be built for distribution.
>>
>> Hi,
>>
>> I think we should not force building the manual nor require asciidoc
>> to build spice-server.
>>
>
> I disagree. The output of make dist should be the same no matter what
> system the command is run. Users should have the required dependencies
> installed if they are interested in building spice-server from the
> source code tarball.
>
>> We better find another way to make 'make dist' work
>>
>> Regards,
>>     Uri.
>>
>
> The only other option I see is to keep track of the generated files in
> git, and if there is any modifications, generate it again manually.

For example, one can configure --enable-manual before running make dist.

Or install asciidoc before running configure.

Uri.

>
>>
>>>
>>> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
>>> ---
>>>  configure.ac            | 25 ++++++-------------------
>>>  docs/Makefile.am        |  2 --
>>>  docs/manual/Makefile.am |  8 +-------
>>>  3 files changed, 7 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index 8419508..18b907a 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -155,24 +155,12 @@ if test "x$enable_automated_tests" = "xyes"; then
>>>  fi
>>>
>>>
>>> -AC_ARG_ENABLE([manual],
>>> -               AS_HELP_STRING([--enable-manual=@<:@auto/yes/no@:>@],
>>> -                              [Build SPICE manual]),
>>> -               [],
>>> -               [enable_manual="auto"])
>>> -if test "x$enable_manual" != "xno"; then
>>> -    AC_PATH_PROG([ASCIIDOC], [asciidoc])
>>> -    AS_IF([test -z "$ASCIIDOC" && test "x$enable_manual" = "xyes"],
>>> -          [AC_MSG_ERROR([asciidoc is missing and build of manual was
>>> requested])])
>>> -    AC_PATH_PROG([A2X], [a2x])
>>> -    AS_IF([test -z "$A2X" && test "x$enable_manual" = "xyes"],
>>> -          [AC_MSG_ERROR([a2x is missing and build of manual was
>>> requested])])
>>> -fi
>>> -AS_IF([test -n "$ASCIIDOC"], [have_asciidoc=yes], [have_asciidoc=no])
>>> -AM_CONDITIONAL([BUILD_MANUAL], [test -n "$ASCIIDOC" || test -n "$A2X"])
>>> -AM_CONDITIONAL([BUILD_HTML_MANUAL], [test -n "$ASCIIDOC"])
>>> -AM_CONDITIONAL([BUILD_CHUNKED_MANUAL], [test -n "$A2X"])
>>> -
>>> +AC_PATH_PROG([ASCIIDOC], [asciidoc])
>>> +AS_IF([test -z "$ASCIIDOC"],
>>> +      [AC_MSG_ERROR([asciidoc is missing])])
>>> +AC_PATH_PROG([A2X], [a2x])
>>> +AS_IF([test -z "$A2X"],
>>> +      [AC_MSG_ERROR([a2x is missing])])
>>>
>>>  dnl
>>> ===========================================================================
>>>
>>>  dnl check compiler flags
>>> @@ -245,7 +233,6 @@ AC_MSG_NOTICE([
>>>          Smartcard:                ${have_smartcard}
>>>          SASL support:             ${have_sasl}
>>>          Automated tests:          ${enable_automated_tests}
>>> -        Manual:                   ${have_asciidoc}
>>>
>>>          Now type 'make' to build $PACKAGE
>>>  ])
>>> diff --git a/docs/Makefile.am b/docs/Makefile.am
>>> index 18e785f..e76efaf 100644
>>> --- a/docs/Makefile.am
>>> +++ b/docs/Makefile.am
>>> @@ -1,3 +1 @@
>>> -if BUILD_MANUAL
>>>  SUBDIRS = manual
>>> -endif
>>> diff --git a/docs/manual/Makefile.am b/docs/manual/Makefile.am
>>> index 24a11fe..beda615 100644
>>> --- a/docs/manual/Makefile.am
>>> +++ b/docs/manual/Makefile.am
>>> @@ -16,13 +16,7 @@ EXTRA_DIST =                    \
>>>  manual.chunked: manual.txt
>>>      $(AM_V_GEN) $(A2X) -f chunked -D $(builddir) $(ASCIIDOC_FLAGS) $<
>>>
>>> -docfiles =
>>> -if BUILD_HTML_MANUAL
>>> -docfiles += manual.html
>>> -endif
>>> -if BUILD_CHUNKED_MANUAL
>>> -docfiles += manual.chunked
>>> -endif
>>> +docfiles = manual.html manual.chunked
>>>
>>>  all-local: $(docfiles)
>>>
>>>
>>
>
>
On 05/03/2016 06:08 AM, Uri Lublin wrote:
> On 05/02/2016 03:32 PM, Eduardo Lima (Etrunko) wrote:
>> On 05/02/2016 07:39 AM, Uri Lublin wrote:
>>> On 04/26/2016 12:23 AM, Eduardo Lima (Etrunko) wrote:
>>>> The problem happens when you run 'make dist' in a system without
>>>> asciidoc installed. Even though in configure time there is a check for
>>>> building the manual, it is required to be built for distribution.
>>>
>>> Hi,
>>>
>>> I think we should not force building the manual nor require asciidoc
>>> to build spice-server.
>>>
>>
>> I disagree. The output of make dist should be the same no matter what
>> system the command is run. Users should have the required dependencies
>> installed if they are interested in building spice-server from the
>> source code tarball.
>>
>>> We better find another way to make 'make dist' work
>>>
>>> Regards,
>>>     Uri.
>>>
>>
>> The only other option I see is to keep track of the generated files in
>> git, and if there is any modifications, generate it again manually.
> 
> For example, one can configure --enable-manual before running make dist.
> 
> Or install asciidoc before running configure.
> 

Sure, but the idea is exactly to avoid those extra additional steps for
specific cases. Is it really too much to ask for another build dependency?
> 
> On 05/02/2016 07:39 AM, Uri Lublin wrote:
> > On 04/26/2016 12:23 AM, Eduardo Lima (Etrunko) wrote:
> >> The problem happens when you run 'make dist' in a system without
> >> asciidoc installed. Even though in configure time there is a check for
> >> building the manual, it is required to be built for distribution.
> > 
> > Hi,
> > 
> > I think we should not force building the manual nor require asciidoc
> > to build spice-server.
> > 
> 
> I disagree. The output of make dist should be the same no matter what
> system the command is run. Users should have the required dependencies
> installed if they are interested in building spice-server from the
> source code tarball.
> 

I agree with Eduardo. The make dist should build a tarball containing
everything and be the same.

However users should also have the option to build spice-server, install
it on their system without having to build the documentation.

So asciidoc should be required for "make dist" but not "make all" or
"make install", despite any possible --enable/disable-manual.

Frediano

> > We better find another way to make 'make dist' work
> > 
> > Regards,
> >     Uri.
> > 
> 
> The only other option I see is to keep track of the generated files in
> git, and if there is any modifications, generate it again manually.
> 
> > 
> >>
> >> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
> >> ---
> >>  configure.ac            | 25 ++++++-------------------
> >>  docs/Makefile.am        |  2 --
> >>  docs/manual/Makefile.am |  8 +-------
> >>  3 files changed, 7 insertions(+), 28 deletions(-)
> >>
> >> diff --git a/configure.ac b/configure.ac
> >> index 8419508..18b907a 100644
> >> --- a/configure.ac
> >> +++ b/configure.ac
> >> @@ -155,24 +155,12 @@ if test "x$enable_automated_tests" = "xyes"; then
> >>  fi
> >>
> >>
> >> -AC_ARG_ENABLE([manual],
> >> -               AS_HELP_STRING([--enable-manual=@<:@auto/yes/no@:>@],
> >> -                              [Build SPICE manual]),
> >> -               [],
> >> -               [enable_manual="auto"])
> >> -if test "x$enable_manual" != "xno"; then
> >> -    AC_PATH_PROG([ASCIIDOC], [asciidoc])
> >> -    AS_IF([test -z "$ASCIIDOC" && test "x$enable_manual" = "xyes"],
> >> -          [AC_MSG_ERROR([asciidoc is missing and build of manual was
> >> requested])])
> >> -    AC_PATH_PROG([A2X], [a2x])
> >> -    AS_IF([test -z "$A2X" && test "x$enable_manual" = "xyes"],
> >> -          [AC_MSG_ERROR([a2x is missing and build of manual was
> >> requested])])
> >> -fi
> >> -AS_IF([test -n "$ASCIIDOC"], [have_asciidoc=yes], [have_asciidoc=no])
> >> -AM_CONDITIONAL([BUILD_MANUAL], [test -n "$ASCIIDOC" || test -n "$A2X"])
> >> -AM_CONDITIONAL([BUILD_HTML_MANUAL], [test -n "$ASCIIDOC"])
> >> -AM_CONDITIONAL([BUILD_CHUNKED_MANUAL], [test -n "$A2X"])
> >> -
> >> +AC_PATH_PROG([ASCIIDOC], [asciidoc])
> >> +AS_IF([test -z "$ASCIIDOC"],
> >> +      [AC_MSG_ERROR([asciidoc is missing])])
> >> +AC_PATH_PROG([A2X], [a2x])
> >> +AS_IF([test -z "$A2X"],
> >> +      [AC_MSG_ERROR([a2x is missing])])
> >>
> >>  dnl
> >> ===========================================================================
> >>
> >>  dnl check compiler flags
> >> @@ -245,7 +233,6 @@ AC_MSG_NOTICE([
> >>          Smartcard:                ${have_smartcard}
> >>          SASL support:             ${have_sasl}
> >>          Automated tests:          ${enable_automated_tests}
> >> -        Manual:                   ${have_asciidoc}
> >>
> >>          Now type 'make' to build $PACKAGE
> >>  ])
> >> diff --git a/docs/Makefile.am b/docs/Makefile.am
> >> index 18e785f..e76efaf 100644
> >> --- a/docs/Makefile.am
> >> +++ b/docs/Makefile.am
> >> @@ -1,3 +1 @@
> >> -if BUILD_MANUAL
> >>  SUBDIRS = manual
> >> -endif
> >> diff --git a/docs/manual/Makefile.am b/docs/manual/Makefile.am
> >> index 24a11fe..beda615 100644
> >> --- a/docs/manual/Makefile.am
> >> +++ b/docs/manual/Makefile.am
> >> @@ -16,13 +16,7 @@ EXTRA_DIST =                    \
> >>  manual.chunked: manual.txt
> >>      $(AM_V_GEN) $(A2X) -f chunked -D $(builddir) $(ASCIIDOC_FLAGS) $<
> >>
> >> -docfiles =
> >> -if BUILD_HTML_MANUAL
> >> -docfiles += manual.html
> >> -endif
> >> -if BUILD_CHUNKED_MANUAL
> >> -docfiles += manual.chunked
> >> -endif
> >> +docfiles = manual.html manual.chunked
> >>
> >>  all-local: $(docfiles)
> >>
> >>
> > 
> 
> 
> --
> Eduardo de Barros Lima (Etrunko)
> Software Engineer - RedHat
> etrunko@redhat.com
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
On 05/03/2016 06:36 PM, Frediano Ziglio wrote:
>>
>> On 05/02/2016 07:39 AM, Uri Lublin wrote:
>>> On 04/26/2016 12:23 AM, Eduardo Lima (Etrunko) wrote:
>>>> The problem happens when you run 'make dist' in a system without
>>>> asciidoc installed. Even though in configure time there is a check for
>>>> building the manual, it is required to be built for distribution.
>>>
>>> Hi,
>>>
>>> I think we should not force building the manual nor require asciidoc
>>> to build spice-server.
>>>
>>
>> I disagree. The output of make dist should be the same no matter what
>> system the command is run. Users should have the required dependencies
>> installed if they are interested in building spice-server from the
>> source code tarball.
>>
>
> I agree with Eduardo. The make dist should build a tarball containing
> everything and be the same.
>
> However users should also have the option to build spice-server, install
> it on their system without having to build the documentation.
>
> So asciidoc should be required for "make dist" but not "make all" or
> "make install", despite any possible --enable/disable-manual.
>
> Frediano

I do not see any problem with having to configure with --enable-manual.
I tried make dist  with --enable-manual and without and the result
is the same.

However, --enable-manual is not even needed, as long as asciidoc is 
installed. So it's only required to install asciidoc before running
make dist (and if you did not and make dist fails just install
the package and reconfigure).

I have no problem accepting patches that make "make dist" require
asciidoc, but I think we should not build the manual by default.

Regards,
     Uri.
On 05/04/2016 06:23 AM, Uri Lublin wrote:
> On 05/03/2016 06:36 PM, Frediano Ziglio wrote:
>>>
>>> On 05/02/2016 07:39 AM, Uri Lublin wrote:
>>>> On 04/26/2016 12:23 AM, Eduardo Lima (Etrunko) wrote:
>>>>> The problem happens when you run 'make dist' in a system without
>>>>> asciidoc installed. Even though in configure time there is a check for
>>>>> building the manual, it is required to be built for distribution.
>>>>
>>>> Hi,
>>>>
>>>> I think we should not force building the manual nor require asciidoc
>>>> to build spice-server.
>>>>
>>>
>>> I disagree. The output of make dist should be the same no matter what
>>> system the command is run. Users should have the required dependencies
>>> installed if they are interested in building spice-server from the
>>> source code tarball.
>>>
>>
>> I agree with Eduardo. The make dist should build a tarball containing
>> everything and be the same.
>>
>> However users should also have the option to build spice-server, install
>> it on their system without having to build the documentation.
>>
>> So asciidoc should be required for "make dist" but not "make all" or
>> "make install", despite any possible --enable/disable-manual.
>>
>> Frediano
> 
> I do not see any problem with having to configure with --enable-manual.
> I tried make dist  with --enable-manual and without and the result
> is the same.
> 
> However, --enable-manual is not even needed, as long as asciidoc is
> installed. So it's only required to install asciidoc before running
> make dist (and if you did not and make dist fails just install
> the package and reconfigure).
> 

The thing to notice here is that we are talking about two different
events, one is configure and the other one is make. Make depends on the
results of configure, but there is no way to know, at configure time,
which arguments the make will receive, so the right thing to do is to
prepare for all of them.

Checking for a dependency that will be only used in make dist is wrong
and adds unnecessary maintenance burden in autofoo files which are by
themselves already too complex to mess with.

> I have no problem accepting patches that make "make dist" require
> asciidoc, but I think we should not build the manual by default.
> 

It is possible to have the manual built only for make dist, but I would
still keep asciidoc as a mandatory dependency.
On Wed, May 04, 2016 at 09:09:41AM -0300, Eduardo Lima (Etrunko) wrote:
> The thing to notice here is that we are talking about two different
> events, one is configure and the other one is make. Make depends on the
> results of configure, but there is no way to know, at configure time,
> which arguments the make will receive, so the right thing to do is to
> prepare for all of them.
> 
> Checking for a dependency that will be only used in make dist is wrong
> and adds unnecessary maintenance burden in autofoo files which are by
> themselves already too complex to mess with.
> 
> > I have no problem accepting patches that make "make dist" require
> > asciidoc, but I think we should not build the manual by default.
> > 
> 
> It is possible to have the manual built only for make dist, but I would
> still keep asciidoc as a mandatory dependency.

That would be a bit weird imo. Since 'make dist' is not something I
expect a lot of people to run, I think it's fine to not check for what
it requires in configure, we can nicely error out when someone tries to
run make dist without --enable-manual.

Christophe