Re: [lmi] [lmi-commits] master e653dbd 03/10: Refactor: transplant two recipe lines

[Vadim Zeitlin]

On Tue, 30 Apr 2019 22:17:55 -0400 (EDT) Greg Chicares <address@hidden> wrote:

GC> branch: master
GC> commit e653dbd9d57f5d490864b7efb67c90657e9aae18
GC> Author: Gregory W. Chicares <address@hidden>
GC> Commit: Gregory W. Chicares <address@hidden>
GC> 
GC>     Refactor: transplant two recipe lines
GC>     
GC>     Writing the mkdir commands just before the cp command makes it plain
GC>     that both directories exist.
GC> ---
GC>  install_mingw.make | 4 ++--
GC>  1 file changed, 2 insertions(+), 2 deletions(-)
GC> 
GC> diff --git a/install_mingw.make b/install_mingw.make
GC> index eb28c0b..9246f5b 100644
GC> --- a/install_mingw.make
GC> +++ b/install_mingw.make
GC> @@ -107,6 +107,8 @@ ad_hoc_dir_exists = \
GC>  
GC>  .PHONY: all
GC>  all: $(file_list)
GC> +   $(MKDIR) --parents $(prefix)
GC> +   $(MKDIR) --parents $(ad_hoc_dir)
GC>     $(CP) --archive $(ad_hoc_dir)/mingw32 $(prefix)
GC>     $(RM) --force --recursive $(ad_hoc_dir)

 Sorry, but this doesn't look right: first of all, adding $(MKDIR) here is
superfluous, because clearly $(ad_hoc_dir) must already exist if we're
using its subdirectory $(ad_hoc_dir)/mingw32 as the source for a copy
operation just below.

GC> @@ -117,8 +119,6 @@ initial_setup:
GC>     type "$(WGET)" >/dev/null || { printf '%b' $(wget_missing)      && 
false; }
GC>     [ ! -e $(prefix)     ]    || { printf '%b' $(prefix_exists)     && 
false; }
GC>     [ ! -e $(ad_hoc_dir) ]    || { printf '%b' $(ad_hoc_dir_exists) && 
false; }
GC> -   $(MKDIR) --parents $(prefix)
GC> -   $(MKDIR) --parents $(ad_hoc_dir)

 But second, and worse, removing it from the "initial_setup" target is
actively harmful because this means that extracting the archive with bsdtar
using --directory=$(ad_hoc_dir) will fail: while bsdtar(1) is not totally
clear about whether the directory specified by this option must exist, a
simple test shows that it really must and that bsdtar exits with "could not
chdir to $directory" error if it doesn't.

 So I'm afraid we really need to move at least the second mkdir command
back. AFAICS creating $(prefix) just before copying into it should be fine,
however.

 Please let me know if you think I'm missing something here,
VZ