[PATCH] Don't use cpio in git-clone when not installed

[PATCH] Don't use cpio in git-clone when not installed

[Mike Hommey]

Signed-off-by: Mike Hommey <mh@glandium.org>
---
 git-clone.sh |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/git-clone.sh b/git-clone.sh
index 0ea3c24..57e96ae 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -191,7 +191,9 @@ fi
 # it is local
 if base=$(get_repo_base "$repo"); then
 	repo="$base"
-	local=yes
+	if type cpio > /dev/null 2>&1; then
+		local=yes
+	fi
 fi
 
 dir="$2"
-- 
1.5.3.4

Re: [PATCH] Don't use cpio in git-clone when not installed

[Mike Hommey]

Damn, and I forgot git-send-email doesn't add the <> to In-Reply-To.

Mike

Re: [PATCH] Don't use cpio in git-clone when not installed

[Junio C Hamano]

"type"?  That's probably better than using "which", but sounds
quite wrong.  Why not add Makefile target that the shell scripts
depend on to see if necessary tools are available and let the
builder know if they are not?

Re: [PATCH] Don't use cpio in git-clone when not installed

[Johannes Schindelin]

Hi,

On Wed, 31 Oct 2007, Junio C Hamano wrote:

> "type"?  That's probably better than using "which", but sounds quite 
> wrong.  Why not add Makefile target that the shell scripts depend on to 
> see if necessary tools are available and let the builder know if they 
> are not?

I thought the accord was that "cpio" is a _runtime_ dependency.  But yes, 
I agree "type" is not necessarily the best choice.  Maybe testing the exit 
code for 127 (not found) is the better choice?

Ciao,
Dscho

Re: [PATCH] Don't use cpio in git-clone when not installed

[Linus Torvalds]

On Wed, 31 Oct 2007, Mike Hommey wrote:
> +	if type cpio > /dev/null 2>&1; then
> +		local=yes
> +	fi

Isn't "type" a bashism?

Maybe just do

	if echo . | cpio -o > /dev/null 2>&1; then

instead? Maybe even doing this at install time to avoid the overhead of 
executing another process..

		Linus

Re: [PATCH] Don't use cpio in git-clone when not installed

[Jakub Narebski]

Linus Torvalds wrote:

> 
> 
> On Wed, 31 Oct 2007, Mike Hommey wrote:
>> +    if type cpio > /dev/null 2>&1; then
>> +            local=yes
>> +    fi
> 
> Isn't "type" a bashism?
> 
> Maybe just do
> 
>       if echo . | cpio -o > /dev/null 2>&1; then
> 
> instead? Maybe even doing this at install time to avoid the overhead of 
> executing another process..

Or perhaps trap / check actual execution of cpio in git-clone, and
fallback to ln -s / link if it fails...

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] Don't use cpio in git-clone when not installed

[Nguyen Thai Ngoc Duy]

On 11/1/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Wed, 31 Oct 2007, Mike Hommey wrote:
> > +     if type cpio > /dev/null 2>&1; then
> > +             local=yes
> > +     fi
>
> Isn't "type" a bashism?

busybox ash has "type". I'm happy.

-- 
Duy

Re: [PATCH] Don't use cpio in git-clone when not installed

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Wed, 31 Oct 2007, Mike Hommey wrote:
>> +	if type cpio > /dev/null 2>&1; then
>> +		local=yes
>> +	fi
>
> Isn't "type" a bashism?

I seem to recall that it is in POSIX.

Re: [PATCH] Don't use cpio in git-clone when not installed

[Mike Hommey]

On Wed, Oct 31, 2007 at 05:46:40PM -0700, Linus Torvalds wrote:
> 
> 
> On Wed, 31 Oct 2007, Mike Hommey wrote:
> > +	if type cpio > /dev/null 2>&1; then
> > +		local=yes
> > +	fi
> 
> Isn't "type" a bashism?

I think it's POSIX, and since I found the same construct in
git-mergetool.sh, I thought it would be okay for git.

Mike

Re: [PATCH] Don't use cpio in git-clone when not installed

[Alexander Skwar]

Linus Torvalds <torvalds <at> linux-foundation.org> writes:

> 
> 
> On Wed, 31 Oct 2007, Mike Hommey wrote:
> > +	if type cpio > /dev/null 2>&1; then
> > +		local=yes
> > +	fi
> 
> Isn't "type" a bashism?

ksh also has type. But SUS states, that "command -v" can be used.

   Options

[...]

   -v
       (On systems supporting the User Portability Utilities option.)
       Write a string to standard output that indicates the pathname or
       command that will be used by the shell, in the current shell
       execution environment (see Shell Execution Environment ), to
       invoke command_name, but do not invoke command_name.

           * Utilities, regular built-in utilities, command_names
             including a slash character, and any implementation-defined
             functions that are found using the PATH variable (as
             described in Command Search and Execution ), shall be
             written as absolute pathnames.

           * Shell functions, special built-in utilities, regular
             built-in utilities not associated with a PATH search, and
             shell reserved words shall be written as just their names.

           * An alias shall be written as a command line that represents
             its alias definition.

           * Otherwise, no output shall be written and the exit status
             shall reflect that the name was not found.

Regards,
Alexander

Re: [PATCH] Don't use cpio in git-clone when not installed

[Nguyen Thai Ngoc Duy]

BTW, you have workaround for git-merge also? It uses cpio to save/restore state.

On 11/1/07, Mike Hommey <mh@glandium.org> wrote:
>
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
>  git-clone.sh |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/git-clone.sh b/git-clone.sh
> index 0ea3c24..57e96ae 100755
> --- a/git-clone.sh
> +++ b/git-clone.sh
> @@ -191,7 +191,9 @@ fi
>  # it is local
>  if base=$(get_repo_base "$repo"); then
>         repo="$base"
> -       local=yes
> +       if type cpio > /dev/null 2>&1; then
> +               local=yes
> +       fi
>  fi
>
>  dir="$2"
> --
> 1.5.3.4
>
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Duy

Re: [PATCH] Don't use cpio in git-clone when not installed

[Junio C Hamano]

"Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:

> BTW, you have workaround for git-merge also? It uses cpio to save/restore state.

Why do people want "workaround"?  Is installing cpio such a
hassle?

Re: [PATCH] Don't use cpio in git-clone when not installed

[Nguyen Thai Ngoc Duy]

On 11/1/07, Junio C Hamano <gitster@pobox.com> wrote:
> "Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
>
> > BTW, you have workaround for git-merge also? It uses cpio to save/restore state.
>
> Why do people want "workaround"?  Is installing cpio such a
> hassle?

It is on Windows because busybox cpio is not really good and busybox
tar is even worse (for cpio emulation). Maybe I should just improve
busybox cpio :-)
-- 
Duy

Re: [PATCH] Don't use cpio in git-clone when not installed

[Junio C Hamano]

"Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:

> It is on Windows because busybox cpio is not really good and busybox
> tar is even worse (for cpio emulation). Maybe I should just improve
> busybox cpio :-)

Sounds sensible, as it (at least its -p mode of operation) is
one of the programs that is very useful yet not so well known to
people new to UNIX.

Re: [PATCH] Don't use cpio in git-clone when not installed

[Mike Hommey]

On Wed, Oct 31, 2007 at 06:15:27PM -0700, Junio C Hamano wrote:
> "Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
> 
> > BTW, you have workaround for git-merge also? It uses cpio to save/restore state.
> 
> Why do people want "workaround"?  Is installing cpio such a
> hassle?

Note that to do what git-merge does with cpio, i wonder if it wouldn't
be sensible to use git stash, now.

Mike

Re: [PATCH] Don't use cpio in git-clone when not installed

[Junio C Hamano]

Mike Hommey <mh@glandium.org> writes:

> On Wed, Oct 31, 2007 at 06:15:27PM -0700, Junio C Hamano wrote:
>> "Nguyen Thai Ngoc Duy" <pclouds@gmail.com> writes:
>> 
>> > BTW, you have workaround for git-merge also? It uses cpio to save/restore state.
>> 
>> Why do people want "workaround"?  Is installing cpio such a
>> hassle?
>
> Note that to do what git-merge does with cpio, i wonder if it wouldn't
> be sensible to use git stash, now.

Like this?  That's an excellent suggestion.

The patch uses the 'git stash create' which is in 'next'
(jc/stash-create topic).

Having said that, the savestate()/restorestate() codepaths are
only relevant to the "try multiple strategies and pick the best
one" feature of git-merge, and that is where cpio is used (and
the patch rewrites it to use stash).  I am not sure if anybody
ever used it in practice.  I admit I am guilty of inventing it,
but I certainly do not.

It might make sense to remove that try-multiple-strategies
feature from git-merge and be done with it.  It would certainly
make the eventual rewrite to C much easier.

---
 git-merge.sh |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/git-merge.sh b/git-merge.sh
index 3a01db0..e8916cc 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -28,20 +28,19 @@ allow_trivial_merge=t
 
 dropsave() {
 	rm -f -- "$GIT_DIR/MERGE_HEAD" "$GIT_DIR/MERGE_MSG" \
-		 "$GIT_DIR/MERGE_SAVE" || exit 1
+		 "$GIT_DIR/MERGE_STASH" || exit 1
 }
 
 savestate() {
 	# Stash away any local modifications.
-	git diff-index -z --name-only $head |
-	cpio -0 -o >"$GIT_DIR/MERGE_SAVE"
+	git stash create >"$GIT_DIR/MERGE_STASH"
 }
 
 restorestate() {
-        if test -f "$GIT_DIR/MERGE_SAVE"
+        if test -f "$GIT_DIR/MERGE_STASH"
 	then
 		git reset --hard $head >/dev/null
-		cpio -iuv <"$GIT_DIR/MERGE_SAVE"
+		git stash apply --index $(cat "$GIT_DIR/MERGE_STASH")
 		git update-index --refresh >/dev/null
 	fi
 }
@@ -386,7 +385,7 @@ case "$use_strategies" in
     single_strategy=no
     ;;
 *)
-    rm -f "$GIT_DIR/MERGE_SAVE"
+    rm -f "$GIT_DIR/MERGE_STASH"
     single_strategy=yes
     ;;
 esac