Re: problem with texi2dvi from texinfo 6.1

[Kurt Hornik]

>>>>> Vincent Belaïche writes:

Vincent, 

Thanks millions again for looking into this.

We have used the patch you sent first (I guess flavor one) to test
against our complete R/CRAN build/check process (which includes using
texi2dvi to build the reference manuals and other LaTeX docs for almost
8000 packages.  With the patch applied, everything seems to work fine
again.  

It would be great if the patch could quickly make it into the Texinfo
SVN.  Once there, I am happy to try to get the Debian texinfo maintainer
to apply the patch and get the 6.1 packages in testing/unstable updated
as quickly as possible.

Best
-k



> Dear Gavin,
> I have attached the patch in two flavors. Please apply your favorite
> one. The first flavor is the same as the original patch. And the second
> flavor keep the same code but just add some additional `true' just
> before the function return in order to force the return status to 0.

> The second flavor has the merits that I added some comment into the code
> to document why we should beware || and && stuff when in one-liner
> functions.

> I have tested the second flavor and it also fixes the bug, which proves
> that the exception for && and || still applies.

> The problem is that because filter_files has a *sole* line, so when
> xref_files_new is empty, then the return status of filter_files becomes
> non zero. So the premature exit is not inside filter_files itself, but
> it is in the calling of filter_files where there is no such thing as
> boolean testing exception such as &&, || or if/ while.

> VBR,
>       Vincent.

> Le 24/02/2016 08:54, Gavin Smith a écrit :
>> On 23 February 2016 at 23:15, Vincent Belaïche
>> <address@hidden> wrote:
>>> 
>>> OK, I got it. The problem is in function filter_files. Unfortunately
>>> when we made the changes to solve the missing \openout in log problem,
>>> we did not make any tests with not any new xref files after the first
>>> run. When this happens (ie variable xref_files_new empty) the function
>>> filter_files crashes and exits the script. I have not really understood
>>> the details why, but the attached patch fixes that.
>>> 
>>> It would be good to understand exactly what is happening with the bug
>>> because there are similar pieces of coding for instance in
>>> 
>>> if test -n "$COMSPEC$ComSpec" \
>>> && MSYSTEM=$OSTYPE uname | $EGREP -iv 'cygwin|msys' >/dev/null; then
>>> 
>>> which was also changed at the same time.
>> 
>> Thanks for sending the patch, can you send a ChangeLog entry as well
>> and I'll install it?
>> 
>> The conditionals with "if" should be OK.
>> 
>> I think I've seen a bug like this before. The function has only one line:
>> 
>> test -n "$xref_files_new" && echo "$xref_files_new" |
>> # Filter existing files matching the criterion.
>> #
>> while read file; do
>> $1 "$file"
>> done |
>> sort |
>> # Some files are opened several times, e.g., listings.sty's *.vrb.
>> uniq
>> 
>> This would not normally cause an exit with set -e. From (bash)The Set 
>> Builtin:
>> 
>> `-e'
>> Exit immediately if a pipeline (*note Pipelines::), which may
>> consist of a single simple command (*note Simple Commands::),
>> a subshell command enclosed in parentheses (*note Command
>> Grouping::), or one of the commands executed as part of a
>> command list enclosed by braces (*note Command Grouping::)
>> returns a non-zero status.  The shell does not exit if the
>> command that fails is part of the command list immediately
>> following a `while' or `until' keyword, part of the test in
>> an `if' statement, part of any command executed in a `&&' or
>> `||' list except the command following the final `&&' or `||',
>> any command in a pipeline but the last, or if the command's
>> return status is being inverted with `!'.  A trap on `ERR',
>> if set, is executed before the shell exits.
>> 
>> However, it's hidden inside a function, so the exception for && and ||
>> doesn't apply, apparently.
>> 
>> Any use of && for a conditional is suspect.

> Index: ChangeLog

> ===================================================================

> --- ChangeLog (revision 6649)

> +++ ChangeLog (working copy)

> @@ -1,3 +1,11 @@

> +2016-02-24  Vincent Belaïche  <address@hidden>
> +
> +     * util/texi2dvi (filter_files): Ensure that this function does not
> +     make any non zero return status when the list of new xref files is
> +     empty. Otherwise that causes a prematurate exit of calling
> +     function, notably run_index. Remember texi2dvi is running under
> +     set -e regime.
> +
>  2015-09-27  Gavin Smith  <address@hidden>
 
>       * tp/t/README: Mention tests under tp/tests.  Remove "texinfo/" 
> Index: util/texi2dvi

> ===================================================================

> --- util/texi2dvi     (revision 7030)

> +++ util/texi2dvi     (working copy)

> @@ -988,7 +988,8 @@

>  #                         that match PREDICATE.
>  filter_files ()
>  {
> -  test -n "$xref_files_new" && echo "$xref_files_new" |
> +  test -n "$xref_files_new" || return 0
> +  echo "$xref_files_new" |
>    # Filter existing files matching the criterion.
>    #
>    while read file; do
> Index: ChangeLog

> ===================================================================

> --- ChangeLog (revision 6649)

> +++ ChangeLog (working copy)

> @@ -1,3 +1,11 @@

> +2016-02-24  Vincent Belaïche  <address@hidden>
> +
> +     * util/texi2dvi (filter_files): Ensure that this function does not
> +     make any non zero return status when the list of new xref files is
> +     empty. Otherwise that causes a prematurate exit of calling
> +     function, notably run_index. Remember texi2dvi is running under
> +     set -e regime.
> +
>  2015-09-27  Gavin Smith  <address@hidden>
 
>       * tp/t/README: Mention tests under tp/tests.  Remove "texinfo/" 
> Index: util/texi2dvi

> ===================================================================

> --- util/texi2dvi     (revision 7030)

> +++ util/texi2dvi     (working copy)

> @@ -997,6 +997,10 @@

>    sort |
>    # Some files are opened several times, e.g., listings.sty's *.vrb.
>    uniq
> +  # force 0 return status, even when xref_files_new is empty,
> +  # otherwise calling this function may cause a premature exit due to
> +  # texi2dvi running under set -e regime
> +  :
>  }
 
>  # run_index - Run texindex (or makeindex or texindy) on current index