[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>
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
- Re: [lmi] [lmi-commits] master e653dbd 03/10: Refactor: transplant two recipe lines,
Vadim Zeitlin <=