threads / patch / 6330

patchMakefile: add clean-obsolete-scripts target

Subject: [PATCH] Makefile: add clean-obsolete-scripts target

## tl;dr

10 messages between Jan 10, 2007 and Jan 11, 2007. Diffs are folded; open one to read it.

replies: 9people: 3as markdown or json

Johannes Schindelin· Jan 10, 2007, 12:20 UTC · lore

On Cygwin, newly builtins are not recognized, because there exist both the executable binaries (with .exe extension) _and_ the now-obsolete scripts (without extension), but the script is executed.

"make clean-obsolete-scripts" removes these ambiguities by removing the older of these file pairs.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 Makefile |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)
Show changes to Makefile +19 −0
diff --git a/Makefile b/Makefile
index 007ceb2..a46683a 100644
--- a/Makefile
+++ b/Makefile
@@ -905,6 +905,25 @@ dist-doc:
 
 ### Cleaning rules
 
+clean-obsolete-scripts:
+	@if test -n "$X"; \
+	then \
+		ls *$X '$(DESTDIR_SQ)$(gitexecdir_SQ)'/*$X | while read f; do \
+			script="`echo "$$f" | sed 's/$X\$$/./'`"; \
+			if test -f "$$script"; \
+			then \
+				if test "$$script" -ot "$$f"; \
+				then \
+					echo removing "$$script"; \
+					rm "$$script"; \
+				else \
+					echo removing "$$f"; \
+					rm "$$f"; \
+				fi; \
+			fi; \
+		done; \
+	fi
+
 clean:
 	rm -f *.o mozilla-sha1/*.o arm/*.o ppc/*.o compat/*.o xdiff/*.o \
 		$(LIB_FILE) $(XDIFF_LIB)

-- 
1.4.4.4.g774d-dirty
Alex Riesen· Jan 10, 2007, 12:53 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Makefile: add clean-obsolete-scripts target

On 1/10/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 7 quoted lines
>
> On Cygwin, newly builtins are not recognized, because there exist both
> the executable binaries (with .exe extension) _and_ the now-obsolete
> scripts (without extension), but the script is executed.
>
> "make clean-obsolete-scripts" removes these ambiguities by removing the
> older of these file pairs.

Not enough. You have to cleanup the build directory too, otherwise tests fail, and git-hacking on git itself is ... complicated.

Johannes Schindelin· Jan 10, 2007, 13:42 UTC · re: Alex Riesen · lore

Re: [PATCH] Makefile: add clean-obsolete-scripts target

Hi,
On Wed, 10 Jan 2007, Alex Riesen wrote:
Show 11 quoted lines
> On 1/10/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > 
> > On Cygwin, newly builtins are not recognized, because there exist both
> > the executable binaries (with .exe extension) _and_ the now-obsolete
> > scripts (without extension), but the script is executed.
> > 
> > "make clean-obsolete-scripts" removes these ambiguities by removing the
> > older of these file pairs.
> 
> Not enough. You have to cleanup the build directory too,
> otherwise tests fail, and git-hacking on git itself is ... complicated.
Doesn't it handle that?
	ls *$X '$(DESTDIR_SQ)$(gitexecdir_SQ)'/*$X

Note the first "*$X". It really should pick up on all executables in the build directory. Or do I miss something?

Ciao, Dscho

Alex Riesen· Jan 10, 2007, 13:52 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Makefile: add clean-obsolete-scripts target

On 1/10/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 9 quoted lines
> > Not enough. You have to cleanup the build directory too,
> > otherwise tests fail, and git-hacking on git itself is ... complicated.
>
> Doesn't it handle that?
>
>         ls *$X '$(DESTDIR_SQ)$(gitexecdir_SQ)'/*$X
>
> Note the first "*$X". It really should pick up on all executables in the
> build directory. Or do I miss something?

No-no, sorry. I didn't noticed the first *$X, which will of course catch the execs in the current directory.

Junio C Hamano· Jan 10, 2007, 20:22 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Makefile: add clean-obsolete-scripts target

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 6 quoted lines
> On Cygwin, newly builtins are not recognized, because there exist both
> the executable binaries (with .exe extension) _and_ the now-obsolete
> scripts (without extension), but the script is executed.
>
> "make clean-obsolete-scripts" removes these ambiguities by removing the
> older of these file pairs.
I would prefer it to be rather automatic -- how about this instead?
-- >8 --
Show changes to Makefile +9 −3
diff --git a/Makefile b/Makefile
index e4f520a..9077941 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,5 @@
 # The default target of this Makefile is...
-all:
+all::
 
 # Define NO_OPENSSL environment variable if you do not have OpenSSL.
 # This also implies MOZILLA_SHA1.
@@ -605,9 +605,12 @@ export prefix TAR INSTALL DESTDIR SHELL_PATH template_dir
 
 ### Build rules
 
-all: $(ALL_PROGRAMS) $(BUILT_INS) git$X gitk gitweb/gitweb.cgi
+all:: $(ALL_PROGRAMS) $(BUILT_INS) git$X gitk gitweb/gitweb.cgi
+ifneq (,$X)
+	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), echo rm -f '$p';) 
+endif
 
-all:
+all::
 	$(MAKE) -C perl PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all
 	$(MAKE) -C templates
 
@@ -849,6 +852,9 @@ install: all
 			'$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'; \
 	fi
 	$(foreach p,$(BUILT_INS), rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
+ifneq (,$X)
+	$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), echo rm -f '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';) 
+endif
 
 install-doc:
 	$(MAKE) -C Documentation install
Alex Riesen· Jan 10, 2007, 21:26 UTC · re: Junio C Hamano · lore

Re: [PATCH] Makefile: add clean-obsolete-scripts target

On 1/10/07, Junio C Hamano <junkio@cox.net> wrote:
>
> I would prefer it to be rather automatic -- how about this instead?
>
...
> +ifneq (,$X)
Perfect! :)
Junio C Hamano· Jan 11, 2007, 00:34 UTC · re: Alex Riesen · lore

Re: [PATCH] Makefile: add clean-obsolete-scripts target

"Alex Riesen" <raa.lkml@gmail.com> writes:
Show 8 quoted lines
> On 1/10/07, Junio C Hamano <junkio@cox.net> wrote:
>>
>> I would prefer it to be rather automatic -- how about this instead?
>>
> ...
>> +ifneq (,$X)
>
> Perfect! :)

Heh, you couldn't have tested -- I still had "echo rm -f" in the patch ;-).

Alex Riesen· Jan 11, 2007, 21:41 UTC · re: Junio C Hamano · lore

Re: [PATCH] Makefile: add clean-obsolete-scripts target

Junio C Hamano, Thu, Jan 11, 2007 01:34:41 +0100:
Show 8 quoted lines
> >> I would prefer it to be rather automatic -- how about this instead?
> > ...
> >> +ifneq (,$X)
> >
> > Perfect! :)
> 
> Heh, you couldn't have tested -- I still had "echo rm -f" in the
> patch ;-).
Didn't. Just liked the approach :)
Johannes Schindelin· Jan 11, 2007, 08:22 UTC · re: Junio C Hamano · lore

Re: [PATCH] Makefile: add clean-obsolete-scripts target

Hi,
On Wed, 10 Jan 2007, Junio C Hamano wrote:
Show 10 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Cygwin, newly builtins are not recognized, because there exist both
> > the executable binaries (with .exe extension) _and_ the now-obsolete
> > scripts (without extension), but the script is executed.
> >
> > "make clean-obsolete-scripts" removes these ambiguities by removing the
> > older of these file pairs.
> 
> I would prefer it to be rather automatic -- how about this instead?

I like your method better. However, does it execute an rm -f for _every_ builtin, even if it does not exist? This is a huge performance eater on cygwin. Every fork() is _expensive_.

Even worse: are you sure that it does not remove the _builtins_? Maybe you noticed that I asked for "git-bla." instead of "git-bla", since the latter will happily access "git-bla.exe" (at least when you do an "ls git-bla" and "git-bla.exe" exists, but not "git-bla", it _will_ show the metadata of "git-bla.exe", but with the name "git-bla").

Will test.

Ciao, Dscho

Johannes Schindelin· Jan 11, 2007, 08:32 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Makefile: add clean-obsolete-scripts target

Hi,
On Thu, 11 Jan 2007, Johannes Schindelin wrote:
Show 24 quoted lines
> On Wed, 10 Jan 2007, Junio C Hamano wrote:
> 
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > 
> > > On Cygwin, newly builtins are not recognized, because there exist both
> > > the executable binaries (with .exe extension) _and_ the now-obsolete
> > > scripts (without extension), but the script is executed.
> > >
> > > "make clean-obsolete-scripts" removes these ambiguities by removing the
> > > older of these file pairs.
> > 
> > I would prefer it to be rather automatic -- how about this instead?
> 
> I like your method better. However, does it execute an rm -f for _every_ 
> builtin, even if it does not exist? This is a huge performance eater on 
> cygwin. Every fork() is _expensive_.
> 
> Even worse: are you sure that it does not remove the _builtins_? Maybe you 
> noticed that I asked for "git-bla." instead of "git-bla", since the latter 
> will happily access "git-bla.exe" (at least when you do an "ls git-bla" 
> and "git-bla.exe" exists, but not "git-bla", it _will_ show 
> the metadata of "git-bla.exe", but with the name "git-bla").
> 
> Will test.

Okay, I tested. And it works, indeed. No idea why, though. And while it looks a bit strange [*1*], the performance is not as bad as I expected.

Ciao, Dscho

[*1*]

$ make [...] rm -f git-pack-refs.exe && ln git.exe git-pack-refs.exe rm -f 'git-convert-objects'; rm -f 'git-fetch-pack'; rm -f 'git-fsck-objects'; rm -f 'git-hash-object'; rm -f 'git-index-pack'; rm -f 'git-local-fetch'; rm -f 'git-daemon'; rm -f 'git-merge-index'; rm -f 'git-mktag'; rm -f 'git-mktree'; rm -f 'git-patch-id'; rm -f 'git-peek-remote'; rm -f 'git-receive-pack'; rm -f 'git-send-pack'; rm -f 'git-shell'; rm -f 'git-show-index'; rm -f 'git-ssh-fetch'; rm -f 'git-ssh-upload'; rm -f 'git-unpack-file'; rm -f 'git-update-server-info'; rm -f 'git-upload-pack'; rm -f 'git-verify-pack'; rm -f 'git-pack-redundant'; rm -f 'git-var'; rm -f 'git-merge-tree'; rm -f 'git-imap-send'; rm -f 'git-merge-recursive'; rm -f 'git-ssh-pull'; rm -f 'git-ssh-push'; rm -f 'git-http-fetch'; rm -f 'git-http-push'; rm -f 'git-merge-recur'; rm -f 'git-format-patch'; rm -f 'git-show'; rm -f 'git-whatchanged'; rm -f 'git-cherry'; rm -f 'git-get-tar-commit-id'; rm -f 'git-init'; rm -f 'git-add'; rm -f 'git-annotate'; rm -f 'git-apply'; rm -f 'git-archive'; rm -f 'git-blame'; rm -f 'git-branch'; rm -f 'git-cat-file'; rm -f 'git-checkout-index'; rm -f 'git-check-ref-format'; rm -f 'git-commit-tree'; rm -f 'git-count-objects'; rm -f 'git-describe'; rm -f 'git-diff'; rm -f 'git-diff-files'; rm -f 'git-diff-index'; rm -f 'git-diff-stages'; rm -f 'git-diff-tree'; rm -f 'git-fmt-merge-msg'; rm -f 'git-for-each-ref'; rm -f 'git-grep'; rm -f 'git-init-db'; rm -f 'git-log'; rm -f 'git-ls-files'; rm -f 'git-ls-tree'; rm -f 'git-mailinfo'; rm -f 'git-mailsplit'; rm -f 'git-merge-base'; rm -f 'git-merge-file'; rm -f 'git-mv'; rm -f 'git-name-rev'; rm -f 'git-pack-objects'; rm -f 'git-prune'; rm -f 'git-prune-packed'; rm -f 'git-push'; rm -f 'git-read-tree'; rm -f 'git-reflog'; rm -f 'git-repo-config'; rm -f 'git-rerere'; rm -f 'git-rev-list'; rm -f 'git-rev-parse'; rm -f 'git-rm'; rm -f 'git-runstatus'; rm -f 'git-shortlog'; rm -f 'git-show-branch'; rm -f 'git-stripspace'; rm -f 'git-symbolic-ref'; rm -f 'git-tar-tree'; rm -f 'git-unpack-objects'; rm -f 'git-update-index'; rm -f 'git-update-ref'; rm -f 'git-upload-archive'; rm -f 'git-verify-pack'; rm -f 'git-write-tree'; rm -f 'git-show-ref'; rm -f 'git-pack-refs'; rm -f 'git'; [...]

← back to recent threads