Re: [PATCH] Texinfo: inconsistent behavior: cmd line vs. env

[Gavin Smith]

On Thu, Mar 23, 2023 at 05:57:58PM +0100, Bogdan wrote:
> > There is still the minor question of what should happen if
> > TEXI2DVI_BUILD_DIRECTORY is set to '.' (the default).  Since this is the
> > default value, it doesn't seem right that explicitly setting
> > TEXI2DVI_BUILD_DIRECTORY=. in the environment should change the
> > behaviour of texi2dvi.
> 
> 
>  I *was* thinking about this, too. On the other hand, if the user gives
> "--build-dir=.", it *would* imply --tidy, even though this is the default.
> The sole presence of the switch changes the behaviour.

It occured to me that we didn't have to make '.' the default value.  We
can make the default value empty and avoid this as a special case.  The
build_dir variable is not used that much in the program, so it is easy
to accommodate a change in default value.

>  In other words, for consistency and no-surprises, it should be:
>       --build-dir given => --tidy
>       TEXI2DVI_BUILD_DIRECTORY given => --tidy
> whatever the values.

Yes, that makes the most sense.  I realised this when trying to update
the help message.

> > Hence, I propose the following patch:

Here's an updated patch:

diff --git a/ChangeLog b/ChangeLog
index 92bc4d6325..2707f6bad5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2023-03-23  Gavin Smith <gavinsmith0123@gmail.com>
+
+       Make setting TEXI2DVI_BUILD_DIRECTORY equivalent to --build-dir option
+
+       * util/texi2dvi: Make setting build dir to a directory other
+       than '.' change build mode from 'local' to 'tidy' regardless of
+       whether it is given with --build-dir or setting the
+       TEXI2DVI_BUILD_DIRECTORY envvar.  Report from Bogdan Drozdowski.
+
 2023-03-21  Patrice Dumas  <pertusus@free.fr>
 
        Index commands do not end paragraphs in texi2any
diff --git a/util/texi2dvi b/util/texi2dvi
index 28c53ebe44..bd4f3df9a8 100755
--- a/util/texi2dvi
+++ b/util/texi2dvi
@@ -36,7 +36,7 @@ set -e
 program=`echo $0 | $SED -e 's!.*/!!'`
 
 build_mode=${TEXI2DVI_BUILD_MODE:-local}
-build_dir=${TEXI2DVI_BUILD_DIRECTORY:-.}
+build_dir=${TEXI2DVI_BUILD_DIRECTORY:-}
 
 orig_pwd=`pwd`
 
@@ -151,12 +151,7 @@ Build modes:
       --tidy           same as --build=tidy
   -c, --clean          same as --build=clean
       --build-dir=DIR  specify where the tidy compilation is performed;
-                         implies --tidy;
-                         defaults to TEXI2DVI_BUILD_DIRECTORY [$build_dir]
-
-The MODE specifies where the TeX compilation takes place, and, as a
-consequence, how auxiliary files are treated.  The build mode can also
-be set using the environment variable TEXI2DVI_BUILD_MODE.
+                         implies --tidy
 
 Valid values of MODE are:
   \`local'      compile in the current directory, leaving all the auxiliary
@@ -166,6 +161,10 @@ Valid values of MODE are:
   \`clean'      same as \`tidy', but remove the auxiliary directory afterwards.
                Every compilation therefore requires the full cycle.
 
+The build mode can also be set using the environment variable
+TEXI2DVI_BUILD_MODE.  The build directory can also be set using
+the environment variable TEXI2DVI_BUILD_DIRECTORY.
+
 The values of these environment variables are used to run the
 corresponding commands, if they are set:
 
@@ -1694,7 +1693,7 @@ while test x"$1" != x"$arg_sep"; do
     -~ ) verbose "Option -~ is obsolete: texi2dvi ignores it.";;
     -b | --batch) ;; # Obsolete
          --build)      shift; build_mode=$1;;
-         --build-dir)  shift; build_dir=$1; build_mode=tidy;;
+         --build-dir)  shift; build_dir=$1;;
     -c | --clean) build_mode=clean;;
     -D | --debug) debug=true;;
     -e | -E | --expand) expand=true;;
@@ -1743,6 +1742,11 @@ done
 # Pop the token
 shift
 
+# If build_dir given, switch from --local to --tidy
+if test x"$build_dir" != x"" && test x"$build_mode" = x"local" ; then
+  build_mode=tidy
+fi
+
 # $tidy:  compile in a t2d directory.
 # $clean: remove all the aux files.
 case $build_mode in
@@ -1848,7 +1852,8 @@ do
   # in compiling this document.
   case $build_dir in
       '' | . ) t2ddir=$out_noext.t2d ;;
-      *) # Avoid collisions between multiple occurrences of the same
+      *) ensure_dir "$build_dir"
+         # Avoid collisions between multiple occurrences of the same
          # file, so depend on the output path.  Remove leading `./',
          # at least to avoid creating a file starting with `.!', i.e.,
          # an invisible file. The sed expression is fragile if the cwd
@@ -1860,7 +1865,7 @@ do
   # Remove it at exit if clean mode.
   trap "cleanup" 0 1 2 15
 
-  ensure_dir "$build_dir" "$t2ddir"
+  ensure_dir "$t2ddir"
 
   # Sometimes there are incompatibilities between auxiliary files for
   # DVI and PDF.  The contents can also change whether we work on PDF

>  Remember also about the help message:
> 
> "
>       --build-dir=DIR  specify where the tidy compilation is performed;
>                          implies --tidy;
>                          defaults to TEXI2DVI_BUILD_DIRECTORY [$build_dir]
> 
> "
> 
> For some reason, it was made so that passing --build-dir implies --tidy...
> But, maybe just for the non-default values, as I wrote above ("external
> directories").

It's slightly more complicated than it needs to be due to the
indirection through TEXI2DVI_BUILD_DIRECTORY.  I've reworded this in the
patch above.

>  From a user's point of view - looks fine. Generally, I have nothing against
> your change.
>  Since I began playing with fixing Automake I'm just worried if its tests
> would need to pass --tidy now, which wasn't present in the older versions,
> so what do we do now to have a command line that works with all versions
> (maybe except really ancient ones)?
> But it seems we're always passing a directory which isn't ".", so hopefully
> it will work.

In the new patch above "." is no longer a special case.