gnulib-tool: Improve handling of multiple --local-dir options

[Bruno Haible]

Today I needed to use gnulib-tool with multiple --local-dir options for
the first time. Thanks to Pavel Raiskup, who implemented this feature
which I always found desirable but too hard to implement!

However, there are two nits in the way it's implemented:

  * The last specified dir takes the highest precedence. This is in
    contrast to
      - the '-I' options of a compiler,
      - the '-L' options of a compiler,
      - the '-I' options of 'aclocal',
    where always the first such option specifies the one with the
    highest precedence, the last such option the one with the second-to-
    lowest precedence, and the default directory finally has the lowest
    precendence.
    Let's have it (and document it) with highest-precedence-first
    semantics.

  * It is not possible to have a .diff file in one local-dir that
    applies to a file found in another local-dir. In other words, once
    you start using local-dirs for the original file, you need code
    duplication instead of .diff files.
    The useful behaviour is that a .diff file in a local-dir of higher
    precedence applies to the file found in a local-dir of lower
    precendence (or Gnulib, as lowest-precedence fallback).
    Yes, it is even possible to have multiple .diff files that are applied
    one after the other.

This patch fixes both issues, and makes the IFS handling a bit more robust.


2019-02-14  Bruno Haible  <address@hidden>

        gnulib-tool: Improve handling of multiple --local-dir options.
        * doc/gnulib.texi (Extending Gnulib): Explain how multiple --local-dir
        options work.
        * gnulib-tool (func_path_prepend): Remove function.
        (func_path_foreach): Make IFS handling more robust.
        (local_gnulib_path): Collect --local-dir values using func_path_append,
        not func_path_prepend.
        (func_determine_path_separator): Make IFS handling more robust.
        (func_lookup_file_cb): New function.
        (func_lookup_file): Rewritten to use func_lookup_file_cb instead of
        func_lookup_local_file. Apply the patches in the reverse order of their
        origin in $local_gnulib_path.
        (func_count_relative_local_gnulib_path): Make IFS handling more robust.
        * NEWS: Mention that the first --local-dir option is the one with
        highest priority.

diff --git a/NEWS b/NEWS
index 481524b..6127784 100644
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,10 @@ Important general notes
 
 Date        Modules         Changes
 
+2019-02-14  gnulib-tool     If you use multiple --local-dir options at once:
+                            The first one now has the highest priority, not the
+                            last one.
+
 2019-01-04  (all)           The meaning of the 'Link' section in the module
                             descriptions has been clarified: It overrides the
                             combined 'Link' sections from the dependencies.
diff --git a/doc/gnulib.texi b/doc/gnulib.texi
index f7709c9..802e39b 100644
--- a/doc/gnulib.texi
+++ b/doc/gnulib.texi
@@ -714,6 +714,15 @@ using the @command{patch} program.
 Otherwise, @command{gnulib-tool} uses the file included in Gnulib.
 @end itemize
 
+You can specify the @option{--local-dir} multiple times.  In this case,
+the first specified directory has the highest precedence.  That is, a
address@hidden@var{file}} found in one directory will shadow any @address@hidden
+and @address@hidden in the later directories and in the Gnulib
+directory.  And a file @address@hidden found in one directory will
+be applied on top of the combination of @address@hidden and
address@hidden@var{file}.diff} files found in the later directories and in the
+Gnulib directory.
+
 Please make wise use of this option.  It also allows you to easily
 hold back modifications you make to Gnulib macros in cases it may be
 better to share them.
diff --git a/gnulib-tool b/gnulib-tool
index 44cfeb3..3dc9a1d 100755
--- a/gnulib-tool
+++ b/gnulib-tool
@@ -556,21 +556,8 @@ func_determine_path_separator ()
   fi
 }
 
-# func_path_prepend pathvar directory
-# puts directory before pathvar, delimiting directories by PATH_SEPARATOR.
-# Newly added directory into pathvar has the highest priority.
-func_path_prepend ()
-{
-  if eval "test -n \"\$$1\""; then
-    eval "$1=\$2\$PATH_SEPARATOR\$$1"
-  else
-    eval "$1=\$2"
-  fi
-}
-
 # func_path_append pathvar directory
-# Similar to func_path_prepend except that the newest directory has the lowest
-# priority.
+# appends directory to pathvar, delimiting directories by PATH_SEPARATOR.
 func_path_append ()
 {
   if eval "test -n \"\$$1\""; then
@@ -590,7 +577,7 @@ func_path_foreach_inner ()
   set %start% "$@"
   for _fpf_arg
   do
-    case $_fpf_arg in
+    case "$_fpf_arg" in
       %start%)
         set dummy
         ;;
@@ -613,17 +600,18 @@ func_path_foreach_inner ()
 # with processed directory from path.
 func_path_foreach ()
 {
-  fpf_save_IFS=$IFS
-  fpf_dirs=$1 ; shift
-  fpf_cb=$1 ; shift
+  fpf_dirs="$1"; shift
+  fpf_cb="$1"; shift
   fpf_rc=false
 
-  IFS=$PATH_SEPARATOR
+  fpf_save_IFS="$IFS"
+  IFS="$PATH_SEPARATOR"
   for fpf_dir in $fpf_dirs
   do
+    IFS="$fpf_save_IFS"
     func_path_foreach_inner "$@" && fpf_rc=:
   done
-  IFS=$fpf_save_IFS
+  IFS="$fpf_save_IFS"
   $fpf_rc
 }
 
@@ -1049,7 +1037,7 @@ func_determine_path_separator
 #                   update, create-testdir, create-megatestdir, test, megatest,
 #                   copy-file
 # - destdir         from --dir
-# - local_gnulib_path  from --local-dir
+# - local_gnulib_path  from --local-dir, highest priority dir comes first
 # - modcache        true or false, from --cache-modules/--no-cache-modules
 # - verbose         integer, default 0, inc/decremented by --verbose/--quiet
 # - libname, supplied_libname  from --lib
@@ -1197,11 +1185,11 @@ func_determine_path_separator
         if test $# = 0; then
           func_fatal_error "missing argument for --local-dir"
         fi
-        func_path_prepend local_gnulib_path "$1"
+        func_path_append local_gnulib_path "$1"
         shift ;;
       --local-dir=* )
         local_dir=`echo "X$1" | sed -e 's/^X--local-dir=//'`
-        func_path_prepend local_gnulib_path "$local_dir"
+        func_path_append local_gnulib_path "$local_dir"
         shift ;;
       --cache-modules | --cache-module | --cache-modul | --cache-modu | 
--cache-mod | --cache-mo | --cache-m | --cache- | --cache | --cach | --cac | 
--ca )
         modcache=true
@@ -1575,18 +1563,19 @@ func_determine_path_separator
   # Remove trailing slashes from the directory names. This is necessary for
   # m4base (to avoid an error in func_import) and optional for the others.
   sed_trimtrailingslashes='s,\([^/]\)//*$,\1,'
-  old_local_gnulib_path=$local_gnulib_path
-  save_IFS=$IFS
-  IFS=:
+  old_local_gnulib_path="$local_gnulib_path"
   local_gnulib_path=
+  save_IFS="$IFS"
+  IFS=:
   for dir in $old_local_gnulib_path
   do
+    IFS="$save_IFS"
     case "$dir" in
       */ ) dir=`echo "$dir" | sed -e "$sed_trimtrailingslashes"` ;;
     esac
     func_path_append local_gnulib_path "$dir"
   done
-  IFS=$save_IFS
+  IFS="$save_IFS"
   case "$sourcebase" in
     */ ) sourcebase=`echo "$sourcebase" | sed -e "$sed_trimtrailingslashes"` ;;
   esac
@@ -1630,8 +1619,8 @@ else
 fi
 
 # func_lookup_local_file_cb dir file
-# return true and set func_lookup_local_file_result if the file 'dir/file'
-# exists
+# returns true and sets func_lookup_local_file_result if the file $dir/$file
+# exists.
 func_lookup_local_file_cb ()
 {
   test -n "$func_lookup_local_file_result" && return 1 # already found?
@@ -1653,6 +1642,25 @@ func_lookup_local_file ()
   func_path_foreach "$local_gnulib_path" func_lookup_local_file_cb %dir% "$1"
 }
 
+# func_lookup_file_cb dir
+# does one step in processing the $local_gnulib_path, looking for $dir/$lkfile
+# and $dir/$lkfile.diff.
+func_lookup_file_cb ()
+{
+  # If we found the file already in a --local-dir of higher priority, nothing
+  # more to do.
+  if test -z "$lookedup_file"; then
+    # Otherwise, look for $dir/$lkfile and $dir/$lkfile.diff.
+    if test -f "$1/$lkfile"; then
+      lookedup_file="$1/$lkfile"
+    else
+      if test -f "$1/$lkfile.diff"; then
+        lkpatches="$1/$lkfile.diff${lkpatches:+$PATH_SEPARATOR$lkpatches}"
+      fi
+    fi
+  fi
+}
+
 # func_lookup_file file
 # looks up a file in $local_gnulib_path or $gnulib_dir, or combines it through
 # 'patch'.
@@ -1664,27 +1672,39 @@ func_lookup_local_file ()
 func_lookup_file ()
 {
   lkfile="$1"
-  if func_lookup_local_file "$lkfile"; then
-    lookedup_file=$func_lookup_local_file_result
-    lookedup_tmp=
-  else
+  # Each element in $local_gnulib_path is a directory whose contents overrides
+  # or amends the result of the lookup in the rest of $local_gnulib_path and
+  # $gnulib_dir. So, the first element of $local_gnulib_path is the highest
+  # priority one.
+  lookedup_file=
+  lkpatches=
+  func_path_foreach "$local_gnulib_path" func_lookup_file_cb %dir%
+  # Treat $gnulib_dir like a lowest-priority --local-dir, except that here we
+  # don't look for .diff files.
+  if test -z "$lookedup_file"; then
     if test -f "$gnulib_dir/$lkfile"; then
-      if func_lookup_local_file "$lkfile.diff"; then
-        lkbase=`echo "$lkfile" | sed -e 's,^.*/,,'`
-        rm -f "$tmp/$lkbase"
-        cp "$gnulib_dir/$lkfile" "$tmp/$lkbase"
-        patch -s "$tmp/$lkbase" < "$func_lookup_local_file_result" >&2 \
-          || func_fatal_error "patch file $func_lookup_local_file_result 
didn't apply cleanly"
-        lookedup_file="$tmp/$lkbase"
-        lookedup_tmp=true
-      else
-        lookedup_file="$gnulib_dir/$lkfile"
-        lookedup_tmp=
-      fi
+      lookedup_file="$gnulib_dir/$lkfile"
     else
       func_fatal_error "file $gnulib_dir/$lkfile not found"
     fi
   fi
+  # Now apply the patches, from lowest-priority to highest-priority.
+  lookedup_tmp=
+  if test -n "$lkpatches"; then
+    lkbase=`echo "$lkfile" | sed -e 's,^.*/,,'`
+    rm -f "$tmp/$lkbase"
+    cp "$lookedup_file" "$tmp/$lkbase"
+    save_IFS="$IFS"
+    IFS="$PATH_SEPARATOR"
+    for patchfile in $lkpatches; do
+      IFS="$save_IFS"
+      patch -s "$tmp/$lkbase" < "$patchfile" >&2 \
+        || func_fatal_error "patch file $patchfile didn't apply cleanly"
+    done
+    IFS="$save_IFS"
+    lookedup_file="$tmp/$lkbase"
+    lookedup_tmp=true
+  fi
 }
 
 # func_sanitize_modulelist
@@ -5562,11 +5582,12 @@ s,//*$,/,'
   # - relative_local_dir  path to be stored into gl_LOCAL_DIR
   func_count_relative_local_gnulib_path ()
   {
-    save_IFS=$IFS
-    IFS=$PATH_SEPARATOR
     relative_local_gnulib_path=
+    save_IFS="$IFS"
+    IFS="$PATH_SEPARATOR"
     for local_dir in $local_gnulib_path
     do
+      IFS="$save_IFS"
       # Store the local_dir relative to destdir.
       case "$local_dir" in
         "" | /*)
@@ -5582,7 +5603,7 @@ s,//*$,/,'
       esac
       func_path_append relative_local_gnulib_path "$relative_local_dir"
     done
-    IFS=$save_IFS
+    IFS="$save_IFS"
   }
 
   # Create m4/gnulib-cache.m4.