[bug] Working files created in bare repository when pushing to a rewound bare repository

[bug] Working files created in bare repository when pushing to a rewound bare repository

[Ping Yin]

Following scripts can reproduce the problem: in the final line,
foo.txt is generated in bare foo.git

# create bare foo.git and its clone foo
mkdir foo
cd foo && echo foo>foo.txt &&
git init && git add . && git commit -m 'create project foo' && cd ..
cd foo && echo foo1>foo.txt && git commit -a -m "update foo.txt" && cd ..
git clone --bare foo foo.git && rm -rf foo
git clone foo.git foo

# reset bare and then push
cd foo.git && git reset --hard HEAD^ && cd ..
cd foo && git push && cd ..
ls foo.git

branches  config  description  *foo.txt*  HEAD  hooks  index  info
objects  ORIG_HEAD  refs

-- 
Ping Yin

Re: [bug] Working files created in bare repository when pushing to a rewound bare repository

[Jeff King]

On Mon, Dec 31, 2007 at 02:42:42PM +0800, Ping Yin wrote:

> Following scripts can reproduce the problem: in the final line,
> foo.txt is generated in bare foo.git

No, your script is wrong.

> # create bare foo.git and its clone foo
> mkdir foo
> cd foo && echo foo>foo.txt &&
> git init && git add . && git commit -m 'create project foo' && cd ..
> cd foo && echo foo1>foo.txt && git commit -a -m "update foo.txt" && cd ..
> git clone --bare foo foo.git && rm -rf foo
> git clone foo.git foo
> 
> # reset bare and then push
> cd foo.git && git reset --hard HEAD^ && cd ..

Try looking in foo.git after this step. The "--hard" to git-reset is
creating the file foo.txt. --hard makes no sense in a bare repository (I
thought we were disallowing it, but apparently not).

-Peff

Re: [bug] Working files created in bare repository when pushing to a rewound bare repository

[Junio C Hamano]

"Ping Yin" <pkufranky@gmail.com> writes:

> Following scripts can reproduce the problem: in the final line,
> foo.txt is generated in bare foo.git

> # reset bare and then push
> cd foo.git && git reset --hard HEAD^ && cd ..

"git reset" is about resetting *both* the branch tip and the
work tree.  Doesn't it create unwanted work tree files when
misused this way at this point already?

> cd foo && git push && cd ..

"git push" never touches the work tree.  It is possible that
there is a PEBCAK hook enabled in that bare repository that
creates random files upon receiving a push, but that is hardly
worth reporting _here_ on the list ;-).

Puzzled...

Re: [bug] Working files created in bare repository when pushing to a rewound bare repository

[Ping Yin]

On Dec 31, 2007 2:47 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Dec 31, 2007 at 02:42:42PM +0800, Ping Yin wrote:
>
> > Following scripts can reproduce the problem: in the final line,
> > foo.txt is generated in bare foo.git
>
> No, your script is wrong.
>
> > # create bare foo.git and its clone foo
> > mkdir foo
> > cd foo && echo foo>foo.txt &&
> > git init && git add . && git commit -m 'create project foo' && cd ..
> > cd foo && echo foo1>foo.txt && git commit -a -m "update foo.txt" && cd ..
> > git clone --bare foo foo.git && rm -rf foo
> > git clone foo.git foo
> >
> > # reset bare and then push
> > cd foo.git && git reset --hard HEAD^ && cd ..
>
> Try looking in foo.git after this step. The "--hard" to git-reset is
> creating the file foo.txt. --hard makes no sense in a bare repository (I
> thought we were disallowing it, but apparently not).
>

Sorry for my inattention. However, i remembered the reason i added
'--hard' is to avoid  the warn 'foo.txt: needs update'  when 'git
reset HEAD^'. Now i know  '--soft' will do the right thing.

So how about use '--soft' as default in the bare repository?



-- 
Ping Yin

Re: [bug] Working files created in bare repository when pushing to a rewound bare repository

[Jeff King]

On Mon, Dec 31, 2007 at 03:02:25PM +0800, Ping Yin wrote:

> Sorry for my inattention. However, i remembered the reason i added
> '--hard' is to avoid  the warn 'foo.txt: needs update'  when 'git
> reset HEAD^'. Now i know  '--soft' will do the right thing.
> 
> So how about use '--soft' as default in the bare repository?

I started on this when I realized that --soft doesn't even work. I
remember posting a patch to make --soft work in a bare repo when
git-reset was still a script. However, Junio indicated that using "git
branch -f" is the recommended workflow. Perhaps git-reset should be
disabled in bare repos totally?

-Peff

Re: [bug] Working files created in bare repository when pushing to a rewound bare repository

[Jeff King]

On Mon, Dec 31, 2007 at 02:07:50AM -0500, Jeff King wrote:

> I started on this when I realized that --soft doesn't even work. I

Er, sorry, I'm somehow incompetent and failed to perform my test
correctly. --soft does work. I think the following is probably worth
doing.

-- >8 --
git-reset: refuse to do hard reset in a bare repository

It makes no sense since there is no working tree. A soft
reset should be fine, though.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-reset.c       |    3 +++
 t/t7103-reset-bare.sh |   28 ++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 0 deletions(-)
 create mode 100755 t/t7103-reset-bare.sh

diff --git a/builtin-reset.c b/builtin-reset.c
index 713c2d5..10dba60 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -244,6 +244,9 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (reset_type == NONE)
 		reset_type = MIXED; /* by default */
 
+	if (reset_type == HARD && is_bare_repository())
+		die("hard reset makes no sense in a bare repository");
+
 	/* Soft reset does not touch the index file nor the working tree
 	 * at all, but requires them in a good order.  Other resets reset
 	 * the index file to the tree object we are switching to. */
diff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh
new file mode 100755
index 0000000..333d5ea
--- /dev/null
+++ b/t/t7103-reset-bare.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+test_description='git-reset in a bare repository'
+. ./test-lib.sh
+
+test_expect_success 'setup non-bare' '
+	echo one >file &&
+	git add file &&
+	git commit -m one
+	echo two >file &&
+	git commit -a -m two
+'
+
+test_expect_success 'setup bare' '
+	git clone --bare . bare.git &&
+	cd bare.git
+'
+
+test_expect_success 'hard reset is not allowed' '
+	! git reset --hard HEAD^
+'
+
+test_expect_success 'soft reset is allowed' '
+	git reset --soft HEAD^ &&
+	test "`git show --pretty=format:%s | head -n 1`" = "one"
+'
+
+test_done
-- 
1.5.4.rc2.1101.g236e-dirty

Re: [bug] Working files created in bare repository when pushing to a rewound bare repository

[Jeff King]

On Mon, Dec 31, 2007 at 02:13:52AM -0500, Jeff King wrote:

> +test_expect_success 'setup non-bare' '
> +	echo one >file &&
> +	git add file &&
> +	git commit -m one

This is of course missing a && at the end.

-Peff

Re: [bug] Working files created in bare repository when pushing to a rewound bare repository

[Jeff King]

On Mon, Dec 31, 2007 at 02:13:52AM -0500, Jeff King wrote:

> git-reset: refuse to do hard reset in a bare repository
> 
> It makes no sense since there is no working tree. A soft
> reset should be fine, though.

And then on top of this (plus one-liner fix I posted), as Ping Yin
suggested, we can do:

-- >8 --
git-reset: default to --soft in a bare repo

--mixed doesn't make sense, since we don't generally have an
index.

Signed-off-by: Jeff King <peff@peff.net>
---
This is a bit more contentious. There's no reason you can't have an
index in a bare repo, and this is changing the behavior of those who do.
They can always explicitly specify --mixed (since we haven't disallowed
that), but it might break some scripts.

 builtin-reset.c       |    2 +-
 t/t7103-reset-bare.sh |    9 ++++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/builtin-reset.c b/builtin-reset.c
index 10dba60..44e4eb4 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -242,7 +242,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		return read_from_tree(prefix, argv + i, sha1);
 	}
 	if (reset_type == NONE)
-		reset_type = MIXED; /* by default */
+		reset_type = is_bare_repository() ? SOFT : MIXED;
 
 	if (reset_type == HARD && is_bare_repository())
 		die("hard reset makes no sense in a bare repository");
diff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh
index b25a77f..c2cdba4 100755
--- a/t/t7103-reset-bare.sh
+++ b/t/t7103-reset-bare.sh
@@ -7,8 +7,10 @@ test_expect_success 'setup non-bare' '
 	echo one >file &&
 	git add file &&
 	git commit -m one &&
+	git tag one &&
 	echo two >file &&
-	git commit -a -m two
+	git commit -a -m two &&
+	git tag two
 '
 
 test_expect_success 'setup bare' '
@@ -25,4 +27,9 @@ test_expect_success 'soft reset is allowed' '
 	test "`git show --pretty=format:%s | head -n 1`" = "one"
 '
 
+test_expect_success 'default to soft reset' '
+	git reset two &&
+	test "`git show --pretty=format:%s | head -n 1`" = "two"
+'
+
 test_done
-- 
1.5.4.rc2.1102.g4735f-dirty

Re: [bug] Working files created in bare repository when pushing to a rewound bare repository

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> And then on top of this (plus one-liner fix I posted), as Ping Yin
> suggested, we can do:
>
> -- >8 --
> git-reset: default to --soft in a bare repo
>
> --mixed doesn't make sense, since we don't generally have an
> index.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This is a bit more contentious. There's no reason you can't have an
> index in a bare repo, and this is changing the behavior of those who do.
> They can always explicitly specify --mixed (since we haven't disallowed
> that), but it might break some scripts.

I'd prefer to error out if something does not make sense rather
than defaulting differently depending on the bareness of the
repository, but that is probably the matter of taste.

Please re-raise the issue post 1.5.4