Git development
 help / color / mirror / Atom feed
* Re: git describe not matching git log
From: Michael J Gruber @ 2012-11-29 14:06 UTC (permalink / raw)
  To: Steven Penny; +Cc: git, Jeff King
In-Reply-To: <CAAXzdLWNb7E1qincviX5y_uEsT71bbepuUtLR7Q_+Exm8Od6yw@mail.gmail.com>

Steven Penny venit, vidit, dixit 29.11.2012 10:04:
> It seems "git describe" is not matching "git log" as detailed in the help, in
> some cases. From git describe --help
> 
>   [torvalds@g5 git]$ git describe parent
>   v1.0.4-14-g2414721
> 
>   The number of additional commits is the number of commits which would
>   be displayed by "git log v1.0.4..parent".
> 
> GOOD
> 
>   $ git clone git://github.com/antirez/redis.git
> 
>   $ cd redis
> 
>   $ git describe unstable
>   with-deprecated-diskstore-1050-g7383c3b
> 
>   $ git log --oneline with-deprecated-diskstore..unstable | wc
>      1050   11779   78709
> 
> BAD
> 
>   $ git clone git://github.com/git/git.git
> 
>   $ cd git
> 
>   $ git describe master
>   v1.8.0.1-264-g226dcb5
> 
>   $ git log --oneline v1.8.0.1..master | wc
>       260    1650   14154
> 

This is due to date skew: git-describe uses "insert_by_date" when it
traverses the dag, and this can go wrong.

Interestingling, this seems to get fixed by using Jeff's generation
numbers and "insert_by_generation" instead, so it does seem go wrong for
226dcb5~60 or so. git-describe's logic is a bit convoluted and may
depend on how we insert when generation numbers are the same... Have to
do more testing.

Michael

^ permalink raw reply

* gitk: highlighting commits "touching path" with globs doesn't work for files list
From: Yaroslav Halchenko @ 2012-11-29 13:51 UTC (permalink / raw)
  To: git

Hi GIT Gurus,

Highlighting files (in patch view) and commits which modified those
files is a really nice feature to have.   Often it is needed to
highlight based on a glob expression for files, e.g. '*Makefile*'
-- that seems to highlight the commits but files in the patch view
are no longer highlighted, which significantly reduces usefulness of
such a feature.

Also, those choices (Exact/IgnCase/Regexp) seems to have no relation
with the edit box when looking for "touching path" files, so shouldn't
it be somehow disabled/emptied to avoid users trying to create the
ultimate filepattern regexp for no good reason?  (the same for next
field on where to search through -- All Fields/... ) ;)

-- 
Yaroslav O. Halchenko
Postdoctoral Fellow,   Department of Psychological and Brain Sciences
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834                       Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik        

^ permalink raw reply

* [PATCH v2] If `egrep` is aliased, temporary disable it in bash.completion
From: Adam Tkac @ 2012-11-29 15:14 UTC (permalink / raw)
  To: git

Originally reported as https://bugzilla.redhat.com/show_bug.cgi?id=863780

Signed-off-by: Adam Tkac <atkac@redhat.com>
Signed-off-by: Holger Arnold <holgerar@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0960acc..79073c2 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -565,7 +565,7 @@ __git_complete_strategy ()
 __git_list_all_commands ()
 {
 	local i IFS=" "$'\n'
-	for i in $(git help -a|egrep '^  [a-zA-Z0-9]')
+	for i in $(git help -a| \egrep '^  [a-zA-Z0-9]')
 	do
 		case $i in
 		*--*)             : helper pattern;;
-- 
1.8.0


-- 
Adam Tkac, Red Hat, Inc.

^ permalink raw reply related

* [PATCH] DESTDIR support in contrib/subtree/Makefile
From: Adam Tkac @ 2012-11-29 15:41 UTC (permalink / raw)
  To: git

Signed-off-by: Adam Tkac <atkac@redhat.com>
---

It is a good habit in Makefiles to honor DESTDIR variable to support

`make DESTDIR=/instalroot install`

syntax.

Comments are welcomed.

Regards, Adam

 contrib/subtree/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/subtree/Makefile b/contrib/subtree/Makefile
index 05cdd5c..36ae3e4 100644
--- a/contrib/subtree/Makefile
+++ b/contrib/subtree/Makefile
@@ -30,12 +30,12 @@ $(GIT_SUBTREE): $(GIT_SUBTREE_SH)
 doc: $(GIT_SUBTREE_DOC)
 
 install: $(GIT_SUBTREE)
-	$(INSTALL) -m 755 $(GIT_SUBTREE) $(libexecdir)
+	$(INSTALL) -m 755 $(GIT_SUBTREE) $(DESTDIR)$(libexecdir)
 
 install-doc: install-man
 
 install-man: $(GIT_SUBTREE_DOC)
-	$(INSTALL) -m 644 $^ $(man1dir)
+	$(INSTALL) -m 644 $^ $(DESTDIR)$(man1dir)
 
 $(GIT_SUBTREE_DOC): $(GIT_SUBTREE_XML)
 	xmlto -m $(MANPAGE_NORMAL_XSL)  man $^
-- 
1.8.0


-- 
Adam Tkac, Red Hat, Inc.

^ permalink raw reply related

* [RFC] git-submodule update: Add --commit option
From: W. Trevor King @ 2012-11-29 16:12 UTC (permalink / raw)
  To: Heiko Voigt, Git
  Cc: Junio C Hamano, Jeff King, Phil Hord, Shawn Pearce, Jens Lehmann,
	Nahor
In-Reply-To: <cover.1354130656.git.wking@tremily.us>

[-- Attachment #1: Type: text/plain, Size: 6638 bytes --]

This option triggers automatic commits when `submodule update` changes
any gitlinked submodule SHA-1s.  The commit message contains a
`shortlog` summary of the changes for each changed submodule.
---

On Tue, Nov 27, 2012 at 07:51:42PM +0100, Heiko Voigt wrote:
> BTW, I am more and more convinced that an automatically manufactured
> commit on update with --branch should be the default. What do other
> think? Sascha raised a concern that he would not want this, but as far as
> I understood he let the CI-server do that so I see no downside to
> natively adding that to git. People who want to manually craft those
> commits can still amend the generated commit. Since this is all about
> helping people keeping their submodules updated why not go the full way?

Here's a first pass (without documentation) for automatic commits on
submodule updates.  There have been a number of requests for
automatically-committed submodule updates due to submodule upstreams.
This patch shows how you can do that (if applied with my `submodule
update --remote` series), and reuse the same logic to automatically
commit changes due to local submodule changes (as shown here in the
new test).

I think the logic is pretty good, but the implementation is pretty
ugly due to POSIX shell variable limitations.  I'm basically trying to
pass an array of [(name, sm_path, sha1, subsha1), ...] into
commit_changes().  I though about perling-out in commit_changes(), but
I lack sufficient perl-fu to know how to tie clear_local_git_env, cd,
and shortlog up in a single open2 call.  If anyone can give me some
implementation pointers, that would be very helpful.

This is against v1.8.0 (without my --remote series).  To apply on top
of the --remote series, you'd have to save the original gitlinked
$sha1 and use that original value when constructing changed_modules.
I can attach this to the end of the --remote series if desired, but I
think this patch could also stand on its own.

Obviously this still needs documentation, etc., but I wanted feedback
on the implementation before I started digging into that.

Cheers,
Trevor

---
 git-submodule.sh            | 67 ++++++++++++++++++++++++++++++++++++++++++++-
 t/t7406-submodule-update.sh | 19 +++++++++++++
 2 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..d9a59af 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -8,7 +8,7 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
 USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
-   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
+   or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--commit] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
    or: $dashless [--quiet] sync [--] [<path>...]"
@@ -21,6 +21,7 @@ require_work_tree
 command=
 branch=
 force=
+commit=
 reference=
 cached=
 recursive=
@@ -240,6 +241,52 @@ module_clone()
 }
 
 #
+# Commit changed submodule gitlinks
+#
+# $1 = name-a;sha1-a;subsha1-a\n[name-b;sha1-b;subsha1-b\n...]
+#
+commit_changes()
+{
+	echo "commiting $1"
+	OIFS="$IFS"
+	IFS=";"
+	paths=$(echo "$1" |
+		while read name sm_path sha1 subsha1
+		do
+			echo "$sm_path"
+		done
+		)
+	names=$(echo "$1" |
+		while read name sm_path sha1 subsha1
+		do
+			printf ' %s' "$name"
+		done
+		)
+	summary="$(eval_gettext "Updated submodules:")$names"
+	body=$(echo "$1" |
+		while read name sm_path sha1 subsha1
+		do
+			if test "$name" = "$sm_path"
+			then
+				printf 'Changes to %s:\n\n' "$name"
+			else
+				printf 'Changes to %s (%s):\n\n' "$name" "$sm_path"
+			fi
+			(
+				clear_local_git_env
+				cd "$sm_path" &&
+				git shortlog "${sha1}..${subsha1}" ||
+				die "$(eval_gettext "Unable to generate shortlog in submodule path '\$sm_path'")"
+			)
+		done
+		)
+	IFS="$OIFS"
+	message="$(printf '%s\n\n%s\n' "$summary" "$body")"
+	echo "message: [$message]"
+	git commit -m "$message" $paths
+}
+
+#
 # Add a new submodule to the working tree, .gitmodules and the index
 #
 # $@ = repo path
@@ -515,6 +562,9 @@ cmd_update()
 		-f|--force)
 			force=$1
 			;;
+		--commit)
+			commit=1
+			;;
 		-r|--rebase)
 			update="rebase"
 			;;
@@ -557,6 +607,7 @@ cmd_update()
 	fi
 
 	cloned_modules=
+	changed_modules=
 	module_list "$@" | {
 	err=
 	while read mode sha1 stage sm_path
@@ -660,6 +711,15 @@ Maybe you want to use 'update --init'?")"
 				err="${err};$die_msg"
 				continue
 			fi
+
+			subsha1=$(clear_local_git_env; cd "$sm_path" &&
+				git rev-parse --verify HEAD) ||
+			die "$(eval_gettext "Unable to find new revision in submodule path '\$sm_path'")"
+
+			if test "$subsha1" != "$sha1"
+			then
+				changed_modules=$(printf '%s%s\n' "$changed_modules" "$name;$sm_path;$sha1;$subsha1")
+			fi
 		fi
 
 		if test -n "$recursive"
@@ -680,6 +740,11 @@ Maybe you want to use 'update --init'?")"
 		fi
 	done
 
+	if test -z "$err" -a -n "$commit" -a -n "$changed_modules"
+	then
+		commit_changes "$changed_modules"
+	fi
+
 	if test -n "$err"
 	then
 		OIFS=$IFS
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 1542653..4c8bb5d 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -163,6 +163,25 @@ test_expect_success 'submodule update --merge staying on master' '
 	)
 '
 
+test_expect_success 'submodule update --commit --rebase should commit gitlink changes' '
+	(cd super/submodule &&
+	 git reset --hard HEAD~1 &&
+	 echo "local change" > local-file &&
+	 git add local-file &&
+	 test_tick &&
+	 git commit -m "local change"
+	) &&
+	(cd super &&
+	 git submodule update --commit --rebase submodule &&
+	 test "$(git log -1 --oneline)" = "bbdbe2d Updated submodules: submodule"
+	) &&
+	(cd submodule &&
+	 git remote add super-submodule ../super/submodule &&
+	 git pull super-submodule master
+	) &&
+  test "a" = "b"
+'
+
 test_expect_success 'submodule update - rebase in .git/config' '
 	(cd super &&
 	 git config submodule.submodule.update rebase
-- 
1.8.0.1.gaaf2ac7.dirty

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply related

* Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)
From: Junio C Hamano @ 2012-11-29 16:13 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, Antoine Pelisse
In-Reply-To: <50B71B33.1090000@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

>> It turns out that there are at least two bugs in the diffstat
>> counting code.  This series comes on top of the earlier 74faaa1 (Fix
>> "git diff --stat" for interesting - but empty - file changes,
>> 2012-10-17) to fix them.
>
> The tests still fail on Windows. I am not sure whether there is a
> difference in comparing the file system against the index or a commit.
> If there is, then the updated tests might not test the same thing.

The hunks in the patch look fine.  The last one that tests unmerged
entries do not have to have "chmod" if it gives you trouble (you
would need to reduce number of files from 4 to 3 if you go that
route, I think).

^ permalink raw reply

* Re: [RFC] git-submodule update: Add --commit option
From: W. Trevor King @ 2012-11-29 16:21 UTC (permalink / raw)
  To: Heiko Voigt, Git
  Cc: Junio C Hamano, Jeff King, Phil Hord, Shawn Pearce, Jens Lehmann,
	Nahor
In-Reply-To: <20121129161216.GB23580@odin.tremily.us>

[-- Attachment #1: Type: text/plain, Size: 443 bytes --]

On Thu, Nov 29, 2012 at 11:12:16AM -0500, W. Trevor King wrote:
> +  test "a" = "b"

This kills the test (with --immediate) so you can look at the
generated commit.  If you actually want the test to pass (e.g. if this
becomes a PATCH and not an RFC), this line should be removed.

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] git-remote-mediawiki: escape double quotes and LF in file names
From: Junio C Hamano @ 2012-11-29 16:25 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <1354192413-9959-1-git-send-email-Matthieu.Moy@imag.fr>

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> A mediawiki page can contain, and even start with a " character, we have
> to escape it when generating the fast-export stream. While we're there,
> also escape newlines, but I don't think we can get them from MediaWiki
> pages.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>  contrib/mw-to-git/git-remote-mediawiki | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki
> index 68555d4..e7a0e7b 100755
> --- a/contrib/mw-to-git/git-remote-mediawiki
> +++ b/contrib/mw-to-git/git-remote-mediawiki
> @@ -711,6 +711,13 @@ sub fetch_mw_revisions {
>  	return ($n, @revisions);
>  }
>  
> +sub fe_escape_path {
> +    my $path = shift;
> +    $path =~ s/"/\\"/g;
> +    $path =~ s/\n/\\n/g;
> +    return $path;
> +}

Is this sufficient?

My reading of the big comment at the beginning of fast-import.c is
that you would also want to quote each backslash; otherwise a
character (or an octal) after it will be taken as a C-style quoted
special letter, no?

>  sub import_file_revision {
>  	my $commit = shift;
>  	my %commit = %{$commit};
> @@ -738,15 +745,17 @@ sub import_file_revision {
>  		print STDOUT "from refs/mediawiki/$remotename/master^0\n";
>  	}
>  	if ($content ne DELETED_CONTENT) {
> -		print STDOUT "M 644 inline $title.mw\n";
> +		print STDOUT "M 644 inline " .
> +		    fe_escape_path($title . ".mw") . "\n";
>  		literal_data($content);
>  		if (%mediafile) {
> -			print STDOUT "M 644 inline $mediafile{title}\n";
> +			print STDOUT "M 644 inline "
> +			    . fe_escape_path($mediafile{title}) . "\n";
>  			literal_data_raw($mediafile{content});
>  		}
>  		print STDOUT "\n\n";
>  	} else {
> -		print STDOUT "D $title.mw\n";
> +		print STDOUT "D " . fe_escape_path($title . ".mw") . "\n";
>  	}
>  
>  	# mediawiki revision number in the git note

^ permalink raw reply

* Re: [RFC] git-submodule update: Add --commit option
From: W. Trevor King @ 2012-11-29 16:27 UTC (permalink / raw)
  To: Heiko Voigt, Git
  Cc: Junio C Hamano, Jeff King, Phil Hord, Shawn Pearce, Jens Lehmann,
	Nahor
In-Reply-To: <20121129161216.GB23580@odin.tremily.us>

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

On Thu, Nov 29, 2012 at 11:12:16AM -0500, W. Trevor King wrote:
> +	 test "$(git log -1 --oneline)" = "bbdbe2d Updated submodules: submodule"

s/bbdbe2d/cd69713/

I forgot to update the SHA-1 here after tweaking the commit message
format.  I'd like to rewrite this test so it won't use the SHA-1, but
this was the quickest way to check that the commit message and gitlink
were both changed appropriately.

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: Millisecond precision in timestamps?
From: Junio C Hamano @ 2012-11-29 16:42 UTC (permalink / raw)
  To: esr
  Cc: Felipe Contreras, Steven Michalske, Thomas Berg, Jeff King,
	Shawn Pearce, git
In-Reply-To: <20121129103847.GA9264@thyrsus.com>

"Eric S. Raymond" <esr@thyrsus.com> writes:

> Felipe Contreras <felipe.contreras@gmail.com>:
>> On Thu, Nov 29, 2012 at 8:11 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> > Steven Michalske <smichalske@gmail.com> writes:
>> >
>> >> Would having arbitrary key value pairs be useful in the git data
>> >> model?
>> >
>> > My answer to the question is that it is harmful to the data model,
>> > but the benefit of going against the data model _may_ outweigh the
>> > downside.  It is all relative.
>
> My use case for a capability like this is one of the more common ones.
> I want to be able to store a fossil commit-ID inherited from another
> VCS outside the commit comment.

That is exactly why I said it is all relative.  If it helps your
application, you can weigh the pros-and-cons yourself and choose to
throw "junk" extended header fields in the commit objects you
create, using hash-object (or commit-tree).  You can read it out
using cat-file and do whatever you want to do with it, and modern
Git (v1.5.0 was from early 2007) and tools that are designed to work
with Git know to ignore such "junk" field.

> The absence of a key/value store forces me into some annoying
> kludges.

Do not do annoying kludge, then.  Come up with a method to encode
your list of (key,value) tuples into a single string, throw a
custom extra header after all the standard header fields in, perhaps
like this:

    tree 0664b9c82d87269b335ff78f32d0e4a504f58cfc
    author A U Thor <author@example.xz> 1355999999 +0900
    committer C O Mitter <committer@example.xz> 1355999999 +0900
    encoding iso-2022-jp
    reposurgeon-metadata your-serialized-list-of-key-value-tuples
     second-line-of-such-serialization
     third-line-of-such-serialization

    My first commit

    Signed-off-by: A U Thor <author@example.xz>
    Signed-off-by: C O Mitter <committer@example.xz>

^ permalink raw reply

* Re: [PATCH] git-remote-mediawiki: escape double quotes and LF in file names
From: Matthieu Moy @ 2012-11-29 16:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy5hkqsut.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

>> +sub fe_escape_path {
>> +    my $path = shift;
>> +    $path =~ s/"/\\"/g;
>> +    $path =~ s/\n/\\n/g;
>> +    return $path;
>> +}
>
> Is this sufficient?
>
> My reading of the big comment at the beginning of fast-import.c is
> that you would also want to quote each backslash;

Nice catch, thanks.

Also, I was lacking the double-quotes around $path. In my defense, I had
only read the doc, not the code, and git-fast-import.txt was less
complete than fast-import.c. New patch follows, fixing the doc too.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* [PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths
From: Matthieu Moy @ 2012-11-29 17:00 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <vpq624omjn4.fsf@grenoble-inp.fr>

The documentation mentionned only newlines and double quotes as
characters needing escaping, but the backslash also needs it. Also, the
documentation was not clearly saying that double quotes around the file
name were required (double quotes in the examples could be interpreted as
part of the sentence, not part of the actual string).

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/git-fast-import.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 6603a7a..35b909c 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -558,8 +558,9 @@ A `<path>` string must use UNIX-style directory separators (forward
 slash `/`), may contain any byte other than `LF`, and must not
 start with double quote (`"`).
 
-If an `LF` or double quote must be encoded into `<path>` shell-style
-quoting should be used, e.g. `"path/with\n and \" in it"`.
+If an `LF`, backslash or double quote must be encoded into `<path>`
+shell-style quoting should be used, and the complete name should be
+surrounded with double quotes e.g. `"path/with\n, \\ and \" in it"`.
 
 The value of `<path>` must be in canonical form. That is it must not:
 
-- 
1.8.0.319.g8abfee4

^ permalink raw reply related

* [PATCH 2/2 v2] git-remote-mediawiki: escape ", \, and LF in file names
From: Matthieu Moy @ 2012-11-29 17:00 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <1354208455-21228-1-git-send-email-Matthieu.Moy@imag.fr>

A mediawiki page can contain, and even start with a " character, we have
to escape it when generating the fast-export stream, as well as \
character. While we're there, also escape newlines, but I don't think we
can get them from MediaWiki pages.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 contrib/mw-to-git/git-remote-mediawiki      | 16 +++++++++++++---
 contrib/mw-to-git/t/t9362-mw-to-git-utf8.sh | 26 ++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/contrib/mw-to-git/git-remote-mediawiki b/contrib/mw-to-git/git-remote-mediawiki
index 68555d4..094129d 100755
--- a/contrib/mw-to-git/git-remote-mediawiki
+++ b/contrib/mw-to-git/git-remote-mediawiki
@@ -711,6 +711,14 @@ sub fetch_mw_revisions {
 	return ($n, @revisions);
 }
 
+sub fe_escape_path {
+    my $path = shift;
+    $path =~ s/\\/\\\\/g;
+    $path =~ s/"/\\"/g;
+    $path =~ s/\n/\\n/g;
+    return '"' . $path . '"';
+}
+
 sub import_file_revision {
 	my $commit = shift;
 	my %commit = %{$commit};
@@ -738,15 +746,17 @@ sub import_file_revision {
 		print STDOUT "from refs/mediawiki/$remotename/master^0\n";
 	}
 	if ($content ne DELETED_CONTENT) {
-		print STDOUT "M 644 inline $title.mw\n";
+		print STDOUT "M 644 inline " .
+		    fe_escape_path($title . ".mw") . "\n";
 		literal_data($content);
 		if (%mediafile) {
-			print STDOUT "M 644 inline $mediafile{title}\n";
+			print STDOUT "M 644 inline "
+			    . fe_escape_path($mediafile{title}) . "\n";
 			literal_data_raw($mediafile{content});
 		}
 		print STDOUT "\n\n";
 	} else {
-		print STDOUT "D $title.mw\n";
+		print STDOUT "D " . fe_escape_path($title . ".mw") . "\n";
 	}
 
 	# mediawiki revision number in the git note
diff --git a/contrib/mw-to-git/t/t9362-mw-to-git-utf8.sh b/contrib/mw-to-git/t/t9362-mw-to-git-utf8.sh
index 246d47d..b6405ce 100755
--- a/contrib/mw-to-git/t/t9362-mw-to-git-utf8.sh
+++ b/contrib/mw-to-git/t/t9362-mw-to-git-utf8.sh
@@ -318,4 +318,30 @@ test_expect_success 'git push with \ in format control' '
 '
 
 
+test_expect_success 'fast-import meta-characters in page name (mw -> git)' '
+	wiki_reset &&
+	wiki_editpage \"file\"_\\_foo "expect to be called \"file\"_\\_foo" false &&
+	git clone mediawiki::'"$WIKI_URL"' mw_dir_21 &&
+	test_path_is_file mw_dir_21/\"file\"_\\_foo.mw &&
+	wiki_getallpage ref_page_21 &&
+	test_diff_directories mw_dir_21 ref_page_21
+'
+
+
+test_expect_success 'fast-import meta-characters in page name (git -> mw) ' '
+	wiki_reset &&
+	git clone mediawiki::'"$WIKI_URL"' mw_dir_22 &&
+	(
+		cd mw_dir_22 &&
+		echo "this file is called \"file\"_\\_foo.mw" >\"file\"_\\_foo &&
+		git add . &&
+		git commit -am "file \"file\"_\\_foo" &&
+		git pull &&
+		git push
+	) &&
+	wiki_getallpage ref_page_22 &&
+	test_diff_directories mw_dir_22 ref_page_22
+'
+
+
 test_done
-- 
1.8.0.319.g8abfee4

^ permalink raw reply related

* Re: [PATCH v2] If `egrep` is aliased, temporary disable it in bash.completion
From: Junio C Hamano @ 2012-11-29 17:03 UTC (permalink / raw)
  To: Adam Tkac; +Cc: git, SZEDER Gábor, Felipe Contreras
In-Reply-To: <20121129151418.GA19169@redhat.com>

Adam Tkac <atkac@redhat.com> writes:

> Subject: Re: [PATCH v2] If `egrep` is aliased, temporary disable it in bash.completion

The code does not seem to do anything special if it is not aliased,
though, so "If ..." part does not sound correct; perhaps you meant
"just in case egrep is aliased to something totally wacky" or
something?

The script seems to use commands other than 'egrep' that too can be
aliased to do whatever unexpected things.  How does this patch get
away without backslashing them all, like

	\echo ...
        \sed ...
        \test ...
        \: comment ...
	\git args ...

and still fix problems for users?  Can't the same solution you would
give to users who alias one of the above to do something undesirable
be applied to those who alias egrep?

Puzzled...

> Originally reported as https://bugzilla.redhat.com/show_bug.cgi?id=863780
>
> Signed-off-by: Adam Tkac <atkac@redhat.com>
> Signed-off-by: Holger Arnold <holgerar@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 0960acc..79073c2 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -565,7 +565,7 @@ __git_complete_strategy ()
>  __git_list_all_commands ()
>  {
>  	local i IFS=" "$'\n'
> -	for i in $(git help -a|egrep '^  [a-zA-Z0-9]')
> +	for i in $(git help -a| \egrep '^  [a-zA-Z0-9]')
>  	do
>  		case $i in
>  		*--*)             : helper pattern;;
> -- 
> 1.8.0

^ permalink raw reply

* [StGit PATCH] Include emacs and vim contribs in source tarball
From: Zane Bitter @ 2012-11-29 17:06 UTC (permalink / raw)
  To: git; +Cc: catalin.marinas

This will enable downstream distros to package them.

Signed-off-by: Zane Bitter <zbitter@redhat.com>
---
 MANIFEST.in |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/MANIFEST.in b/MANIFEST.in
index c94eef6..a80f013 100644
--- a/MANIFEST.in
+++ b/MANIFEST.in
@@ -9,6 +9,9 @@ include examples/gitconfig
 include contrib/*.sh
 include contrib/*.bash
 include contrib/stg-*
+include contrib/Makefile
+include contrib/stgit.el
+recursive-include contrib/vim *.vim
 include t/t*.sh t/t*/* t/Makefile t/README
 include Documentation/*.txt Documentation/Makefile Documentation/*.conf
 include Documentation/build-docdep.perl Documentation/callouts.xsl

^ permalink raw reply related

* Re: [PATCH v2] If `egrep` is aliased, temporary disable it in bash.completion
From: Junio C Hamano @ 2012-11-29 17:33 UTC (permalink / raw)
  To: Adam Tkac; +Cc: git, SZEDER Gábor, Felipe Contreras
In-Reply-To: <7vpq2wqr3v.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Adam Tkac <atkac@redhat.com> writes:
>
>> Subject: Re: [PATCH v2] If `egrep` is aliased, temporary disable it in bash.completion
>
> The code does not seem to do anything special if it is not aliased,
> though, so "If ..." part does not sound correct; perhaps you meant
> "just in case egrep is aliased to something totally wacky" or
> something?
>
> The script seems to use commands other than 'egrep' that too can be
> aliased to do whatever unexpected things.  How does this patch get
> away without backslashing them all, like
>
> 	\echo ...
>         \sed ...
>         \test ...
>         \: comment ...
> 	\git args ...
>
> and still fix problems for users?  Can't the same solution you would
> give to users who alias one of the above to do something undesirable
> be applied to those who alias egrep?
>
> Puzzled...

Sorry for having been more snarky than necessary (blame it to lack
of caffeine).  What I was trying to get at were:

 * I have this suspicion that this patch exists only because you saw
   somebody who aliases egrep to something unexpected by the use of
   it in this script, and egrep *happened* to be the only such
   "unreasonable" alias.  The reporter may not have aliased echo or
   sed away, or the aliases to these command *happened* to produce
   "acceptable" output (even though it might have been slightly
   different from unaliased one, the difference *happened* not to
   matter for the purpose of this script).

 * To the person who observes the same aliasing breakage due to his
   aliasing sed to something else, you would solve his problem by
   telling him "don't do that, then".  If that is the solution, why
   wouldn't it work for egrep?

 * The next person who aliased other commands this script uses in
   such a way that the behaviour of the alias differs sufficiently
   from the unaliased version, you will have to patch the file
   again, with the same backslashing.  This patch is not a solution,
   but a band-aid that only works for a particular case you
   *happened* to have seen.

 * A complete solution that follows the direction this patch
   suggests would involve backslashing *all* commands that can
   potentially aliased away.  Is that really the direction we would
   want to go in (answer: I doubt it)?  Is that the only approach to
   solve this aliasing issue (answer: I don't know, but we should
   try to pursue it before applying a band-aid that is not a
   solution)?

Is there a way to tell bash "do not alias-expand from here up to
there"?  Perhaps "shopt -u expand_aliases" upon entry and restore
its original value when we exit, or something?

IOW, something along this line?

 contrib/completion/git-completion.bash | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git i/contrib/completion/git-completion.bash w/contrib/completion/git-completion.bash
index 0b77eb1..193f53c 100644
--- i/contrib/completion/git-completion.bash
+++ w/contrib/completion/git-completion.bash
@@ -23,6 +23,14 @@
 #    3) Consider changing your PS1 to also show the current branch,
 #       see git-prompt.sh for details.
 
+if shopt -q expand_aliases
+then
+	_git__aliases_were_enabled=yes
+else
+	_git__aliases_were_enabled=
+fi
+shopt -u expand_aliases
+
 case "$COMP_WORDBREAKS" in
 *:*) : great ;;
 *)   COMP_WORDBREAKS="$COMP_WORDBREAKS:"
@@ -2504,3 +2512,8 @@ __git_complete gitk __gitk_main
 if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
 __git_complete git.exe __git_main
 fi
+
+if test -n "$_git__aliases_were_enabled"
+then
+	shopt -s expand_aliases
+fi

^ permalink raw reply related

* Re: [PATCH] t4049: avoid test failures on filemode challenged file systems (Windows)
From: Junio C Hamano @ 2012-11-29 17:51 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, Antoine Pelisse
In-Reply-To: <7v38zss7zb.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Johannes Sixt <j.sixt@viscovery.net> writes:
>
>>> It turns out that there are at least two bugs in the diffstat
>>> counting code.  This series comes on top of the earlier 74faaa1 (Fix
>>> "git diff --stat" for interesting - but empty - file changes,
>>> 2012-10-17) to fix them.
>>
>> The tests still fail on Windows. I am not sure whether there is a
>> difference in comparing the file system against the index or a commit.
>> If there is, then the updated tests might not test the same thing.
>
> The hunks in the patch look fine.  The last one that tests unmerged
> entries do not have to have "chmod" if it gives you trouble (you
> would need to reduce number of files from 4 to 3 if you go that
> route, I think).

That is, something like this.

-- >8 --
Subject: [PATCH] t4049: refocus tests

The primary thing Linus's patch wanted to change was to make sure
that 0-line change appears for a mode-only change.  Update the
first test to chmod a file that we can see in the output (limited
by --stat-count) to demonstrate it.  Also make sure to use test_chmod
and compare the index and the tree, so that we can run this test
even on a filesystem without permission bits.

Later two tests are about fixes to separate issues that were
introduced and/or uncovered by Linus's patch as a side effect, but
the issues are not related to mode-only changes.  Remove chmod from
the tests.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t4049-diff-stat-count.sh | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/t/t4049-diff-stat-count.sh b/t/t4049-diff-stat-count.sh
index 37f50cd..5b594e8 100755
--- a/t/t4049-diff-stat-count.sh
+++ b/t/t4049-diff-stat-count.sh
@@ -13,32 +13,31 @@ test_expect_success 'setup' '
 	git commit -m initial
 '
 
-test_expect_success 'limit output to 2 (simple)' '
+test_expect_success 'mode-only change show as a 0-line change' '
 	git reset --hard &&
-	chmod +x c d &&
+	test_chmod +x b d &&
 	echo a >a &&
-	echo b >b &&
+	echo c >c &&
 	cat >expect <<-\EOF
 	 a | 1 +
-	 b | 1 +
+	 b | 0
 	 ...
 	 4 files changed, 2 insertions(+)
 	EOF
-	git diff --stat --stat-count=2 >actual &&
+	git diff --stat --stat-count=2 HEAD >actual &&
 	test_i18ncmp expect actual
 '
 
 test_expect_success 'binary changes do not count in lines' '
 	git reset --hard &&
-	chmod +x c d &&
 	echo a >a &&
-	echo b >b &&
+	echo c >c &&
 	cat "$TEST_DIRECTORY"/test-binary-1.png >d &&
 	cat >expect <<-\EOF
 	 a | 1 +
-	 b | 1 +
+	 c | 1 +
 	 ...
-	 4 files changed, 2 insertions(+)
+	 3 files changed, 2 insertions(+)
 	EOF
 	git diff --stat --stat-count=2 >actual &&
 	test_i18ncmp expect actual
@@ -56,12 +55,11 @@ test_expect_success 'exclude unmerged entries from total file count' '
 	done |
 	git update-index --index-info &&
 	echo d >d &&
-	chmod +x c d &&
 	cat >expect <<-\EOF
 	 a | 1 +
 	 b | 1 +
 	 ...
-	 4 files changed, 3 insertions(+)
+	 3 files changed, 3 insertions(+)
 	EOF
 	git diff --stat --stat-count=2 >actual &&
 	test_i18ncmp expect actual
-- 
1.8.0.1.407.g3c58eb7

^ permalink raw reply related

* Re: [PATCH] completion: fix warning for zsh
From: Junio C Hamano @ 2012-11-29 18:02 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git
In-Reply-To: <1354177257-5416-1-git-send-email-felipe.contreras@gmail.com>

Will apply directly on 'master'; thanks.

^ permalink raw reply

* Re: [PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths
From: Jeff King @ 2012-11-29 18:11 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git, gitster
In-Reply-To: <1354208455-21228-1-git-send-email-Matthieu.Moy@imag.fr>

On Thu, Nov 29, 2012 at 06:00:54PM +0100, Matthieu Moy wrote:

> The documentation mentionned only newlines and double quotes as

s/nn/n/

> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
> index 6603a7a..35b909c 100644
> --- a/Documentation/git-fast-import.txt
> +++ b/Documentation/git-fast-import.txt
> @@ -558,8 +558,9 @@ A `<path>` string must use UNIX-style directory separators (forward
>  slash `/`), may contain any byte other than `LF`, and must not
>  start with double quote (`"`).
>  
> -If an `LF` or double quote must be encoded into `<path>` shell-style
> -quoting should be used, e.g. `"path/with\n and \" in it"`.
> +If an `LF`, backslash or double quote must be encoded into `<path>`
> +shell-style quoting should be used, and the complete name should be
> +surrounded with double quotes e.g. `"path/with\n, \\ and \" in it"`.

I think the point of the original is that you do not _need_ to quote
unless you have those two characters. IOW, you can do:

  M 100644 :1 file \with \backslashes

and it will stay in the "literal to the end of line" code path because
the path does not begin with a double-quote. It is only when you trigger
the "shell-style quoting" code path is in effect that you must then
follow the rules of that quoting (which includes escaping backslashes).
So technically, your modification to the beginning of the sentence is
not correct.

That being said, I think what you have written is more helpful to an end
user. There is no harm in quoting when we do not have to, as fast-import
implementations must know how to unquote anyway (and we over-quote in
fast-export in this case). And while the example above does work (and
was always designed to), it is sort of an unintuitive area that I would
not be surprised to see other fast-import implementations get wrong. As
a writer of a stream, it probably pays to be defensive and err on the
side of quoting more.

As for the text itself, a few minor punctuation suggestions:

> If an `LF`, backslash or double quote must be encoded
                       ^
                       missing comma as list delimiter

> into `<path>` shell-style quoting should be used, and the complete
               ^
               missing comma in if/then clause

This one was in the original as well, but it makes it harder to read and
is worth fixing.

> surrounded with double quotes e.g. `"path/with\n, \\ and \" in it"`.

Should the parenthetical be in parentheses (or a separate sentence)?

-Peff

^ permalink raw reply

* Re: Ubuntu: gitweb always looks for projects in /var/cache/git (“404 - no projects found”)
From: Jeff King @ 2012-11-29 18:28 UTC (permalink / raw)
  To: Alfonso Muñoz-Pomer Fuentes; +Cc: git
In-Reply-To: <loom.20121129T145320-133@post.gmane.org>

On Thu, Nov 29, 2012 at 01:55:57PM +0000, Alfonso Muñoz-Pomer Fuentes wrote:

> I’ve discovered this weird behaviour in gitweb and documented a workaround in
> StackOverflow:
> http://stackoverflow.com/questions/13609475/ubuntu-gitweb-always-looks-for-projects-in-var-cache-git-404-no-projects-f
> 
> Basically, the variable $projectroot in gitweb.cgi in the beginning is reset to
> the system default value in git_get_projects_list, when it is declared again.
> 
> Is this a known bug? Or am I missing something?

I think the analysis in that stack overflow post is wrong. The use of
"our" in git_get_projects_list is not the culprit.

The problem is that one should not edit gitweb.cgi directly; its
built-in defaults (which you are tweaking) are overridden by
/etc/gitweb.conf, which is shipped by the Ubuntu package. You should
be making your changes in the config file, not the CGI script.

-Peff

^ permalink raw reply

* [RFC/PATCH 0/2] Fix "git reset" on unborn branch
From: Martin von Zweigbergk @ 2012-11-29 18:32 UTC (permalink / raw)
  To: git; +Cc: Martin von Zweigbergk
In-Reply-To: <CANiSa6isDKAgxHWqh5XiQ-adT3-ASFtvAshp028DTcotjQxzmQ@mail.gmail.com>

I decided to address this before "cherry-pick on unborn branch". RFC
mostly because I'm not sure about the user interface. When we have
agreed on that, I will add documentation.

Martin von Zweigbergk (2):
  reset: learn to reset to tree
  reset: learn to reset on unborn branch

 builtin/reset.c                     | 73 ++++++++++++++++++++++---------------
 t/t1512-rev-parse-disambiguation.sh |  4 --
 t/t7102-reset.sh                    | 26 +++++++++++++
 t/t7106-reset-unborn-branch.sh      | 52 ++++++++++++++++++++++++++
 4 files changed, 122 insertions(+), 33 deletions(-)
 create mode 100755 t/t7106-reset-unborn-branch.sh

-- 
1.8.0.1.240.ge8a1f5a

^ permalink raw reply

* [RFC/PATCH 1/2] reset: learn to reset to tree
From: Martin von Zweigbergk @ 2012-11-29 18:32 UTC (permalink / raw)
  To: git; +Cc: Martin von Zweigbergk
In-Reply-To: <1354213975-17866-1-git-send-email-martinvonz@gmail.com>

In cases where HEAD is not supposed to be updated, there is no reason
that "git reset" should require a commit, a tree should be enough. So
make "git reset $rev^{tree}" work just like "git reset $rev", except
that the former will not update HEAD (since there is no commit to
point it to).

Disallow --soft with trees, since that is about updating only HEAD.

By not updating HEAD, "git reset $rev^{tree}" behaves quite like "git
reset $rev .". One might therefore consider requiring a path when
using reset with a tree to make that similarity more obvious. However,
a future commit will make "git reset" work on an unborn branch by
interpreting it as "git reset $empty_tree" and it would seem
unintuitive to the user to say "git reset ." on an unborn
branch. Requiring a path would also make "git reset --hard $tree"
disallowed.

This commit effectively undoes some of commit 13243c2 (reset: the
command takes committish, 2012-07-03). The command line argument is
now required to be an unambiguous treeish.

---

My implementation of lookup_commit_or_tree looks a little clunky. I'm
not very familiar with the API. Suggestions welcome.

Why is the "HEAD is now at ..." message printed only for --hard reset?
After all, HEAD is updated for all types of reset not involving paths.

 builtin/reset.c                     | 67 +++++++++++++++++++++----------------
 t/t1512-rev-parse-disambiguation.sh |  4 ---
 t/t7102-reset.sh                    | 26 ++++++++++++++
 3 files changed, 65 insertions(+), 32 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 915cc9f..cec9874 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -225,6 +225,21 @@ static void die_if_unmerged_cache(int reset_type)
 
 }
 
+static struct object *lookup_commit_or_tree(const char *rev) {
+	unsigned char sha1[20];
+	struct commit *commit;
+	struct tree *tree;
+	if (get_sha1_treeish(rev, sha1))
+		die(_("Failed to resolve '%s' as a valid ref."), rev);
+	commit = lookup_commit_reference_gently(sha1, 1);
+	if (commit)
+		return (struct object *) commit;
+	tree = parse_tree_indirect(sha1);
+	if (tree)
+		return (struct object *) tree;
+	die(_("Could not parse object '%s'."), rev);
+}
+
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;
@@ -232,7 +247,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	const char *rev = "HEAD";
 	unsigned char sha1[20], *orig = NULL, sha1_orig[20],
 				*old_orig = NULL, sha1_old_orig[20];
-	struct commit *commit;
+	struct object *object;
+	struct commit *commit = NULL;
 	struct strbuf msg = STRBUF_INIT;
 	const struct option options[] = {
 		OPT__QUIET(&quiet, N_("be quiet, only report errors")),
@@ -276,7 +292,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		 * Otherwise, argv[i] could be either <rev> or <paths> and
 		 * has to be unambiguous.
 		 */
-		else if (!get_sha1_committish(argv[i], sha1)) {
+		else if (!get_sha1_treeish(argv[i], sha1)) {
 			/*
 			 * Ok, argv[i] looks like a rev; it should not
 			 * be a filename.
@@ -289,19 +305,12 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	if (get_sha1_committish(rev, sha1))
-		die(_("Failed to resolve '%s' as a valid ref."), rev);
-
-	/*
-	 * NOTE: As "git reset $treeish -- $path" should be usable on
-	 * any tree-ish, this is not strictly correct. We are not
-	 * moving the HEAD to any commit; we are merely resetting the
-	 * entries in the index to that of a treeish.
-	 */
-	commit = lookup_commit_reference(sha1);
-	if (!commit)
-		die(_("Could not parse object '%s'."), rev);
-	hashcpy(sha1, commit->object.sha1);
+	object = lookup_commit_or_tree(rev);
+	if (object->type == OBJ_COMMIT)
+		commit = (struct commit*) object;
+	else if (reset_type == SOFT)
+		die(_("--soft requires a commit, which '%s' is not"), rev);
+	hashcpy(sha1, object->sha1);
 
 	if (patch_mode) {
 		if (reset_type != NONE)
@@ -347,23 +356,25 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 			die(_("Could not reset index file to revision '%s'."), rev);
 	}
 
-	/* Any resets update HEAD to the head being switched to,
-	 * saving the previous head in ORIG_HEAD before. */
-	if (!get_sha1("ORIG_HEAD", sha1_old_orig))
-		old_orig = sha1_old_orig;
-	if (!get_sha1("HEAD", sha1_orig)) {
-		orig = sha1_orig;
-		set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
-		update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
+	if (commit) {
+		/* Any resets update HEAD to the head being switched to,
+		 * saving the previous head in ORIG_HEAD before. */
+		if (!get_sha1("ORIG_HEAD", sha1_old_orig))
+			old_orig = sha1_old_orig;
+		if (!get_sha1("HEAD", sha1_orig)) {
+			orig = sha1_orig;
+			set_reflog_message(&msg, "updating ORIG_HEAD", NULL);
+			update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
+		}
+		else if (old_orig)
+			delete_ref("ORIG_HEAD", old_orig, 0);
+		set_reflog_message(&msg, "updating HEAD", rev);
+		update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, MSG_ON_ERR);
 	}
-	else if (old_orig)
-		delete_ref("ORIG_HEAD", old_orig, 0);
-	set_reflog_message(&msg, "updating HEAD", rev);
-	update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, MSG_ON_ERR);
 
 	switch (reset_type) {
 	case HARD:
-		if (!update_ref_status && !quiet)
+		if (commit && !update_ref_status && !quiet)
 			print_new_head_line(commit);
 		break;
 	case SOFT: /* Nothing else to do. */
diff --git a/t/t1512-rev-parse-disambiguation.sh b/t/t1512-rev-parse-disambiguation.sh
index 6b3d797..bc1e40c 100755
--- a/t/t1512-rev-parse-disambiguation.sh
+++ b/t/t1512-rev-parse-disambiguation.sh
@@ -121,10 +121,6 @@ test_expect_success 'git log takes only commit-ish' '
 	git log 000000000
 '
 
-test_expect_success 'git reset takes only commit-ish' '
-	git reset 000000000
-'
-
 test_expect_success 'first tag' '
 	# create one tag 0000000000f8f
 	git tag -a -m j7cp83um v1.0.0
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index b096dc8..d723ef5 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -491,4 +491,30 @@ test_expect_success 'disambiguation (4)' '
 	test ! -f secondfile
 '
 
+test_expect_success 'reset to tree does not update HEAD' '
+	git reset --hard HEAD &&
+	rev_before=$(git rev-parse HEAD) &&
+	git reset HEAD^^{tree} &&
+	test $(git rev-parse HEAD) == $rev_before
+'
+
+test_expect_success 'reset to tree' '
+	# for simpler tests, drop last commit containing added files
+	git reset --hard HEAD^ &&
+	git reset HEAD^^{tree} &&
+	git diff --cached HEAD^ --exit-code &&
+	git diff HEAD --exit-code
+'
+
+test_expect_success 'reset --hard to tree' '
+	git reset --hard &&
+	git reset --hard HEAD^^{tree} &&
+	git diff --cached HEAD^ --exit-code &&
+	git diff HEAD^ --exit-code
+'
+
+test_expect_success 'reset to tree not allowed with --soft}' '
+	test_must_fail git reset --soft HEAD^^{tree}
+'
+
 test_done
-- 
1.8.0.1.240.ge8a1f5a

^ permalink raw reply related

* [RFC/PATCH 2/2] reset: learn to reset on unborn branch
From: Martin von Zweigbergk @ 2012-11-29 18:32 UTC (permalink / raw)
  To: git; +Cc: Martin von Zweigbergk
In-Reply-To: <1354213975-17866-1-git-send-email-martinvonz@gmail.com>

When run on an unborn branch, "git reset" currently fails with:

  fatal: Failed to resolve 'HEAD' as a valid ref.

Fix this by interpreting it as a reset to the empty tree.

If --patch is given, we currently pass the revision specifier, as
given on the command line, to interactive_reset(). On an unborn
branch, HEAD can of course not be resolved, so we need to pass the
sha1 of the empty tree to interactive_reset() as well. This is fine
since interactive_reset only needs the parameter to be a treeish and
doesn't use it for display purposes.

---

Is it correct that interactive_reset does not use the revision
specifier for display purposes? Or, worse, that it requires it to be a
commit in some cases? I tried it and didn't see any problem.

 builtin/reset.c                | 10 +++++---
 t/t7106-reset-unborn-branch.sh | 52 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 3 deletions(-)
 create mode 100755 t/t7106-reset-unborn-branch.sh

diff --git a/builtin/reset.c b/builtin/reset.c
index cec9874..3845225 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -229,7 +229,10 @@ static struct object *lookup_commit_or_tree(const char *rev) {
 	unsigned char sha1[20];
 	struct commit *commit;
 	struct tree *tree;
-	if (get_sha1_treeish(rev, sha1))
+	if (!strcmp(rev, "HEAD") && get_sha1("HEAD", sha1)) {
+		// unborn branch: reset to empty tree
+		hashcpy(sha1, EMPTY_TREE_SHA1_BIN);
+	} else if (get_sha1_treeish(rev, sha1))
 		die(_("Failed to resolve '%s' as a valid ref."), rev);
 	commit = lookup_commit_reference_gently(sha1, 1);
 	if (commit)
@@ -292,7 +295,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		 * Otherwise, argv[i] could be either <rev> or <paths> and
 		 * has to be unambiguous.
 		 */
-		else if (!get_sha1_treeish(argv[i], sha1)) {
+		else if (!strcmp(argv[i], "HEAD") ||
+			 !get_sha1_treeish(argv[i], sha1)) {
 			/*
 			 * Ok, argv[i] looks like a rev; it should not
 			 * be a filename.
@@ -315,7 +319,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (patch_mode) {
 		if (reset_type != NONE)
 			die(_("--patch is incompatible with --{hard,mixed,soft}"));
-		return interactive_reset(rev, argv + i, prefix);
+		return interactive_reset(sha1_to_hex(sha1), argv + i, prefix);
 	}
 
 	/* git reset tree [--] paths... can be used to
diff --git a/t/t7106-reset-unborn-branch.sh b/t/t7106-reset-unborn-branch.sh
new file mode 100755
index 0000000..67d45be
--- /dev/null
+++ b/t/t7106-reset-unborn-branch.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='git reset should work on unborn branch'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo a >a &&
+	echo b >b
+'
+
+test_expect_success 'reset' '
+	git add a b &&
+	git reset &&
+	test "$(git ls-files)" == ""
+'
+
+test_expect_success 'reset HEAD' '
+	rm .git/index &&
+	git add a b &&
+	git reset HEAD &&
+	test "$(git ls-files)" == ""
+'
+
+test_expect_success 'reset $file' '
+	rm .git/index &&
+	git add a b &&
+	git reset a &&
+	test "$(git ls-files)" == "b"
+'
+
+test_expect_success 'reset -p' '
+	rm .git/index &&
+	git add a &&
+	echo y | git reset -p &&
+	test "$(git ls-files)" == ""
+'
+
+test_expect_success 'reset --soft not allowed' '
+	rm .git/index &&
+	git add a &&
+	test_must_fail git reset --soft
+'
+
+test_expect_success 'reset --hard' '
+	rm .git/index &&
+	git add a &&
+	git reset --hard &&
+	test "$(git ls-files)" == "" &&
+	test_path_is_missing a
+'
+
+test_done
-- 
1.8.0.1.240.ge8a1f5a

^ permalink raw reply related

* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
From: Junio C Hamano @ 2012-11-29 18:47 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <1354213975-17866-2-git-send-email-martinvonz@gmail.com>

Martin von Zweigbergk <martinvonz@gmail.com> writes:

> In cases where HEAD is not supposed to be updated, there is no reason
> that "git reset" should require a commit, a tree should be enough. So
> make "git reset $rev^{tree}" work just like "git reset $rev", except
> that the former will not update HEAD (since there is no commit to
> point it to).

That is a horrible design I have to nack, unless you require
pathspec.  You cannot tell what "git reset $sha1" would do without
checking the type of the object $sha1 refers to.  If you do this
only when pathspec is present, then the design is very reasonable.

> Disallow --soft with trees, since that is about updating only HEAD.

Likewise.

^ permalink raw reply

* Re: [PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths
From: Matthieu Moy @ 2012-11-29 18:47 UTC (permalink / raw)
  To: Jeff King; +Cc: git, gitster
In-Reply-To: <20121129181141.GA17309@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> So technically, your modification to the beginning of the sentence is
> not correct.

I'd say the resulting sentence is somehow incorrect, but not more than
the previous one (both say "if ..." without really telling what the
condition was).

>> If an `LF`, backslash or double quote must be encoded
>                        ^
>                        missing comma as list delimiter

Google tells me that my version was UK-correct but not US-correct. As
french, I have no opinion on the subject, I take yours ;-).

How about this:

A path can use the C-style string quoting (this is accepted in all
cases and mandatory if the filename starts with double quote or
contains `LF`). In C-style quoting, `LF`, backslash, and double quote
characters must be escaped by preceding them with a backslash. Also,
the complete name should be surrounded with double quotes (e.g.
`"path/with\n, \\ and \" in it"`).

This should be technically correct, and "this is accepted in all cases"
should encourrage people to use it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox