[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Vadim Zeitlin
Subject: Re: [lmi] [lmi-commits] master e653dbd 03/10: Refactor: transplant two recipe lines
Date: Thu, 2 May 2019 01:05:34 +0200

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>     Refactor: transplant two recipe lines
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> 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>  .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,

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]