Git development
 help / color / mirror / Atom feed
* 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

* Re: Re: [PATCH v4 0/4] git-submodule add: Add --local-branch option
From: Phil Hord @ 2012-11-29 18:51 UTC (permalink / raw)
  To: W. Trevor King
  Cc: Heiko Voigt, Junio C Hamano, Git, Jeff King, Shawn Pearce,
	Jens Lehmann, Nahor
In-Reply-To: <20121128024205.GG15213@odin.tremily.us>

On Tue, Nov 27, 2012 at 6:28 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
>
> Hi,
>
> On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote:
> > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
> > The v4 series leaves the remote branch amigious, but it helps you
> > point the local branch at the right hash so that future calls to
> >
> >   $ git submodule foreach 'git pull'
> >
> > can use the branch's .git/modules/<name>/config settings.
>
> But IMO thats the functionality which should be implemented in submodule
> update and not left to the user.
>
> > > I would think more of some convention like:
> > >
> > >     $ git checkout -t origin/$branch
> > >
> > > when first initialising the submodule with e.g.
> > >
> > >     $ git submodule update --init --branch
> > >
> > > Then later calls of
> > >
> > >     $ git submodule update --branch
> > >
> > > would have a branch configured to pull from. I imagine that results in
> > > a similar behavior gerrit is doing on the server side?
> >
> > That sounds like it's doing pretty much the same thing.  Can you think
> > of a test that would distinguish it from my current v4 implementation?
>
> Well the main difference is that gerrit is automatically updating the
> superproject AFAIK. I would like it if we could implement the same
> workflow support in the submodule script. It seems to me that this is
> already proven to be useful workflow.


It is proven in Gerrit, but Gerrit implements a central-server
workflow.  That is, only Gerrit ever floats the submodules, and he
pushes the result for everyone else to share.  I fear the consequences
of everyone pulling submodules and then later trying to merge
superprojects with someone else's breadcrumbs.

Do you have some idea how this would be handled?

Phil

ps. Apologies for my lateness on this topic. I'm trying to catch up now.

pps. Re-sent since Gmail has hidden the "plain text" option in a
different place, now.

On Tue, Nov 27, 2012 at 9:42 PM, W. Trevor King <wking@tremily.us> wrote:
> On Wed, Nov 28, 2012 at 12:28:58AM +0100, Heiko Voigt wrote:
>> On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote:
>> > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
>> > The v4 series leaves the remote branch amigious, but it helps you
>> > point the local branch at the right hash so that future calls to
>> >
>> >   $ git submodule foreach 'git pull'
>> >
>> > can use the branch's .git/modules/<name>/config settings.
>>
>> But IMO thats the functionality which should be implemented in submodule
>> update and not left to the user.
>
> Then you might need submodule.<name>.local-branch,
> submodule.<name>.remote-repository, and submodule.<name>.remote-branch
> to configure
>
>   $ git checkout submodule.<name>.local-branch
>   $ git pull submodule.<name>.remote-repository submodule.<name>.remote-branch
>
> and this would ignore the $sha1 stored in the gitlink (which all of
> the other update commands use).  This ignoring-the-$sha1 bit made me
> think that a built-in pull wasn't a good fit for 'submodule update'.
> Maybe if it went into a new 'submodule pull'?  Then users have a clear
> distinction:
>
> * 'update' to push superproject $sha1 changes into the submodules
> * 'pull' to push upstream-branch changes into the submodules
>
>> > > I would think more of some convention like:
>> > >
>> > >   $ git checkout -t origin/$branch
>> > >
>> > > when first initialising the submodule with e.g.
>> > >
>> > >   $ git submodule update --init --branch
>> > >
>> > > Then later calls of
>> > >
>> > >   $ git submodule update --branch
>> > >
>> > > would have a branch configured to pull from. I imagine that results in
>> > > a similar behavior gerrit is doing on the server side?
>> >
>> > That sounds like it's doing pretty much the same thing.  Can you think
>> > of a test that would distinguish it from my current v4 implementation?
>>
>> Well the main difference is that gerrit is automatically updating the
>> superproject AFAIK. I would like it if we could implement the same
>> workflow support in the submodule script. It seems to me that this is
>> already proven to be useful workflow.
>
> Ah, sorry, I meant the configuring which remote branch you were
> pulling from happens at submodule initialization (via .git/modules/…)
> for both your workflow and my v4.
>
> You're right that having a builtin pull is different from my v4.
>
>> https://github.com/hvoigt/git/commits/hv/floating_submodules_draft
>
> I looked over this before, but maybe not thoroughly enough ;).
>
>> > > How about reusing the -b|--branch option for add? Since we only change
>> > > the behavior when submodule.$name.update is set to branch it seems
>> > > reasonable to me. Opinions?
>> >
>> > That was the approach I used in v1, but people were concerned that we
>> > would be stomping on previously unclaimed config space.  Since noone
>> > has pointed out other uses besides Gerrit's very similar case, I'm not
>> > sure if that is still an issue.
>>
>> Could you point me to that mail? I cannot seem to find it in my archive.
>
> Hmm.  It seems like Phil's initial response was (accidentally?) off
> list.  The relevant portion was:
>
> On Mon, Oct 22, 2012 at 06:03:53PM -0400, Phil Hord wrote:
>> Some projects now use the 'branch' config value to record the tracking
>> branch for the submodule.  Some ascribe different meaning to the
>> configuration if the value is given vs. undefined.  For example, see
>> the Gerrit submodule-subscription mechanism.  This change will cause
>> those workflows to behave differently than they do now.
>>
>> I do like the idea, but I wish it had a different name for the
>> recording.  Maybe --record-branch=${BRANCH} as an extra switch so the
>> action is explicitly requested.
>
> As I said, I'm happy to go back to --branch if opinions have changed.
>
> On Wed, Nov 28, 2012 at 12:28:58AM +0100, Heiko Voigt wrote:
>> On Tue, Nov 27, 2012 at 02:01:05PM -0500, W. Trevor King wrote:
>> > On Tue, Nov 27, 2012 at 07:31:25PM +0100, Heiko Voigt wrote:
>> > > > Because you need to recurse through submodules for `update --branch`
>> > > > even if "$subsha1" == "$sha1", I had to amend the conditional
>> > > > controlling that block.  This broke one of the existing tests, which I
>> > > > "fixed" in patch 4.  I think a proper fix would involve rewriting
>> > > >
>> > > >   (clear_local_git_env; cd "$sm_path" &&
>> > > >    ( (rev=$(git rev-list -n 1 $sha1 --not --all 2>/dev/null) &&
>> > > >     test -z "$rev") || git-fetch)) ||
>> > > >   die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
>> > > >
>> > > > but I'm not familiar enough with rev-list to want to dig into that
>> > > > yet.  If feedback for the earlier three patches is positive, I'll work
>> > > > up a clean fix and resubmit.
>> > >
>> > > You probably need to separate your handling here. The comparison of the
>> > > currently checked out sha1 and the recorded sha1 is an optimization
>> > > which skips unnecessary fetching in case the submodules commits are
>> > > already correct. This code snippet checks whether the to be checked out
>> > > sha1 is already local and also skips the fetch if it is. We should not
>> > > break that.
>> >
>> > Agreed.  However, determining if the target $sha1 is local should have
>> > nothing to do with the current checked out $subsha1.
>>
>> See my draft or the diff below for an illustration of the splitup.
>>
>> [snip diff]
>
> This looks fine, but my current --branch implementation (which doesn't
> pull) is only a thin branch-checkout layer on top of the standard
> `update` functionality.  I'm still unsure if built-in pulls are worth
> the configuration trouble.  I'll sleep on it.  Maybe I'll feel better
> about them tomorrow ;).
>
> Cheers,
> Trevor
>
> --
> 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

^ permalink raw reply

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

On Thu, Nov 29, 2012 at 07:47:32PM +0100, Matthieu Moy wrote:

> 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).

That's fair. There is a lot left unsaid in the original. :)

> >> 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 ;-).

Thanks, I had a vague recollection that it might be regional. I probably
should have looked it up myself.

> 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.

I think that is much better, but it reads a little more easily to me if
we rearrange the second sentence. To complete my English bikeshedding,
here is how I would have written the whole paragraph:

  A path can use 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, the complete name should be surrounded with
  double quotes, and any `LF`, backslash, or double quote characters
  must be escaped by preceding them with a backslash (e.g.,
  `"path/with\n, \\ and \" in it"`).

Feel free to incorporate or ignore any of my tweaks from that version.

-Peff

^ permalink raw reply

* Re: Millisecond precision in timestamps?
From: Eric S. Raymond @ 2012-11-29 19:02 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Felipe Contreras, Steven Michalske, Thomas Berg, Jeff King,
	Shawn Pearce, git
In-Reply-To: <7vtxs8qs1r.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com>:
> 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.

A good start.  But remember that reposurgeon's entire interface to the
git object level is through fast-export/fast-import.  I need import-
stream syntax for these.

bzr's syntax would do:

-------------------------------------------
mark :1
committer Eric S. Raymond <esr@thyrsus.com> 1289147634 -0500
data 14
First commit.

property branch-nick 12 bzr-testrepo
M 644 inline README
data 41
This is a test file in a dummy bzr repo.
-------------------------------------------

If we actually care about keys being full utf-8 with embedded whitespace
it should look more like this:

-------------------------------------------
mark :1
committer Eric S. Raymond <esr@thyrsus.com> 1289147634 -0500
data 14
First commit.

property 11
branch-nick
propval 12 
bzr-testrepo
M 644 inline README
data 41
This is a test file in a dummy bzr repo.
-------------------------------------------
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

^ permalink raw reply

* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
From: Martin von Zweigbergk @ 2012-11-29 19:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4nk8qmaj.fsf@alter.siamese.dyndns.org>

On Thu, Nov 29, 2012 at 10:47 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 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.

Very good point. Thanks! I now see that "git checkout" also requires a
path when given a tree.

So then "git reset" on an unborn branch would imply "git reset
$empty_tree -- ." instead. And "git reset --hard $tree" would not be
allowed. And the intersection of these -- "git reset --hard" on and
unborn branch -- would also not work. Would the correct fix be to
first make "git reset --hard -- $path" work (*sigh*)? I have never
understood why that doesn't (shouldn't) work.

^ permalink raw reply

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

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>
---
cut-and-paste of Peff's version, adapted from mine.

 Documentation/git-fast-import.txt | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 959e4d3..d1844ea 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -562,8 +562,12 @@ 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"`.
+A path can use 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, the complete name should be surrounded with
+double quotes, and any `LF`, backslash, or double quote characters
+must be escaped by preceding them with a backslash (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 v3] git-remote-mediawiki: escape ", \, and LF in file names
From: Matthieu Moy @ 2012-11-29 19:11 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <1354216293-21921-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: [RFC/PATCH 1/2] reset: learn to reset to tree
From: Junio C Hamano @ 2012-11-29 19:13 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <7v4nk8qmaj.fsf@alter.siamese.dyndns.org>

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

> 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.

The above applies to an _arbitrary_ $sha1.

Allowing "reset $tree -- $pathspec" is a very good addition in the
same sense that "git checkout $tree -- $pathspec" is useful.  These
two commands, "reset" and "checkout", share that the source we grab
the blobs out of only need to be a tree and does not have to be a
commit, and the only difference between them is where the blobs we
grabbed out of that tree go, either only to the index or to both the
index and the working tree.

But I do not think it is connected, at least at the level the end
users perceive, to the issue of "reset" issued while on an unborn
branch.

If you limit the scope of the behaviour change exposed to the end
users so that you would make

	$ git reset [HEAD]

act as a short-hand for

	$ rm -f $GIT_DIR/index

when HEAD points at an unborn branch, and similarly make

	$ git reset --hard [HEAD]

act as a short-hand for

	$ rm -f $GIT_DIR/index
        $ git clean -f -d

in such a case, I do not think it is unreasonable at all.

In such a case,

	$ git reset --soft [HEAD]

would become just a no-op.  Earlier you were on an unborn branch,
and after "reset --soft", nothing changes.

Hmm?

^ permalink raw reply

* Re: [PATCH v5 0/2] submodule update: add --remote for submodule's upstream changes
From: W. Trevor King @ 2012-11-29 19:13 UTC (permalink / raw)
  To: Git, Phil Hord
  Cc: Junio C Hamano, Heiko Voigt, Jeff King, Shawn Pearce,
	Jens Lehmann, Nahor
In-Reply-To: <cover.1354130656.git.wking@tremily.us>

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


On Thu, Nov 29, 2012 at 01:29:12PM -0500, Phil Hord wrote:
> On Fri, Nov 23, 2012 at 12:54 PM, W. Trevor King <wking@tremily.us> wrote:
> > [snip initial thoughts leading to the update --remote v5]
>
> I was thinking the same thing, but reading this whole thread a couple of
> weeks late.  Thanks for noticing.
> 
> Moreover, I think that 'git submodule update --pull' is also the wrong way
> to spell this action.   Maybe you are misled from the outset by your
> current workflow:

Did you see my v5 (add --remote) series?

> For that reason, I don't like the --pull switch since it implies a
> fetch, but I will not always want to do a fetch.

  $ git submodule update --remote --no-fetch

will not fetch the submodule remotes.

> I don't know which remote I should be tracking, though.  I suppose
> it is 'origin' for now, but maybe it is just whatever
> $superproject's HEAD's remote-tracking branch indicates.

With the --remote series, I always use "origin" because that's what
`submodule add` should be setting up.  If people want to change that
up by hand, we may need a submodule.<name>.remote configuration
option.

> I am not sure I want the gitlinks in superproject to update automatically
> in the index, but I definitely do not want to automatically create a commit
> for them.

Commits are dissabled by default (see my recent --commit RFC for how
they would be enabled).

> But I really don't want to figure out how to handle submodule
> collisions during a merge (or rebase!) of my superproject with changes that
> someone else auto-committed in his local $superproject as he and I
> arbitrarily floated up the upstream independently.  There is nothing but
> loathing down that path.

This is true.  I'm not sure how gitlink collisions are currently
handled…

Trevor

-- 
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: Phil Hord @ 2012-11-29 19:14 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Shawn Pearce, Felipe Contreras, Eric Raymond, git
In-Reply-To: <7v7gp6i3rx.fsf@alter.siamese.dyndns.org>

On Wed, Nov 28, 2012 at 2:29 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
>> There is room for new headers, and older versions of git will ignore
>> them. You could add a new "committer-timestamp" field that elaborates on
>> the timestamp included on the committer line. Newer versions of git
>> would respect it, and older versions would fall back to using the
>> committer timestamp.
>>
>> But I really wonder if anybody actually cares about adding sub-second
>> timestamp support, or if it is merely "because SVN has it".
>
> Roundtrip conversions may benefit from sub-second timestamps, but
> personally I think negative timestamps are more interesting and of
> practical use.  Prehistoric projects need them even if they intend
> to switch to Git, never to go back to their original tarballs and
> collection of RCS ,v files.
>
> And if we were to add "committer-timestamp" and friends to support
> negative timestamps anyway (because older tools will not support
> them), supporting sub-second part might be something we want to
> think about at the same time.

Posix-time is signed, but I suppose the git tools do not expect/allow
a '-' character in the stream.  Has git considered the year-2038
problem?

No hurry...

Phil

^ permalink raw reply

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

Jeff King <peff@peff.net> writes:

> 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"`.

That "shell-style" contradicts with what fast-import.c says, though.
It claims to grok \octal and described as C-style.

^ permalink raw reply

* Re: [PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths
From: Jeff King @ 2012-11-29 19:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Matthieu Moy, git
In-Reply-To: <7vvccop6dv.fsf@alter.siamese.dyndns.org>

On Thu, Nov 29, 2012 at 11:15:56AM -0800, Junio C Hamano wrote:

> >> -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"`.
> 
> That "shell-style" contradicts with what fast-import.c says, though.
> It claims to grok \octal and described as C-style.

Yeah, I think it was just laziness by the original author to use
"shell-style" to mean "quotes and backslash escaping" without thinking
too hard about which escape sequences are available. Saying C-style is
more accurate (and Matthieu's more recent update does that).

-Peff

^ permalink raw reply

* Re: [PATCH 1/2 v3] git-fast-import.txt: improve documentation for quoted paths
From: Junio C Hamano @ 2012-11-29 19:33 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <1354216293-21921-1-git-send-email-Matthieu.Moy@imag.fr>

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

> 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>
> ---
> cut-and-paste of Peff's version, adapted from mine.
>
>  Documentation/git-fast-import.txt | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
> index 959e4d3..d1844ea 100644
> --- a/Documentation/git-fast-import.txt
> +++ b/Documentation/git-fast-import.txt
> @@ -562,8 +562,12 @@ 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"`.
> +A path can use C-style string quoting; this is accepted in all cases
> +and mandatory if the filename starts with double quote or contains
> +`LF`.

... or backslash?

> +In C-style quoting, the complete name should be surrounded with
> +double quotes, and any `LF`, backslash, or double quote characters
> +must be escaped by preceding them with a backslash (e.g.,
> +`"path/with\n, \\ and \" in it"`).
>  
>  The value of `<path>` must be in canonical form. That is it must not:

^ permalink raw reply

* Re: [RFC/PATCH 1/2] reset: learn to reset to tree
From: Junio C Hamano @ 2012-11-29 19:36 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git
In-Reply-To: <CANiSa6j2sriXaGr0yH9kMrxDEvKHsjNPX_Exbc2_6ecnPYdroQ@mail.gmail.com>

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

> Would the correct fix be to
> first make "git reset --hard -- $path" work (*sigh*)? I have never
> understood why that doesn't (shouldn't) work.

What does it even mean, even when you are on an existing commit, to
hard reset partially?

Perhaps you looking for "git checkout $tree -- $path"?

^ 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