Git development
 help / color / mirror / Atom feed
* Re: [RFC] teach --edit to git rebase
From: Jean Privat @ 2011-10-12 13:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvcrubyz1.fsf@alter.siamese.dyndns.org>

> The only thing you can do with this new option is "update one commit
> buried in the history, and rebase everything that build on top of it",

It is a good summary indeed.

> as
> far as I can tell. It feels a shame to waste the generic word "--edit" for
> such a narrow option.

I'm bad at bikesheeding. The 'edit' come from the 'edit' command in
rebase interactive. I am open to other names. Note that in the
following I keep the --edit name, not because I am suborn but because
I do not have better to propose yet.

> At the UI level, "git commit --amend HEAD~4" might be a more natural way
> to invoke such an operation, I would think.

As I say in the original email the point of the 'rebase --edit
some-commit' is to temporally checkout some-commit so that edits are
done in the context of the commit and not in the context of the head
of the branch.
One has to do a rebase --edit prior to modification (although we can
imagine a possibility to bring back the index or the content of the
working directory with us either automatically as with a branch
checkout or manually with the help of stash).

Unless I misinterpret the 'git commit --amend HEAD~4' you suggest, it
means that you have to prepare the commit in the head of the branch.
It may be difficult if what was in HEAD~4 is altered by HEAD~2.
My argument is that if preparing a patch to HEAD~4 in HEAD is easy, a
git commit --fixup will do the tick.
If the preparation is difficult because I have to work on (or more
insidious, near) change that occurs between HEAD~4 and HEAD, I need
something like my proposal. For example I added a line in HEAD~2 but I
prefer now to have this line to appears in HEAD~4.

The workflow I propose is :

$ # we are on master
$ git rebase --edit HEAD~4 # workdir is a detached master~4 like with a
                           # git checkout master~4
$ hack hack hack; git add files
$ git commit --amend
$ git rebase --continue # conflict is detected with master~2, resolve it
                        # manually
$ git rebase --continue # workdir is now a rebased master

I do no see what is the workflow with an extend git commit --amend
Do you mean something like the following ?

$ git checkout HEAD~4 -- . # bring back the content of master~4 but
                           # HEAD still points master
$ hack hack hack; git add files # And try to now be disturbed by the fact
                                # that diff and status are polluted
                                # with things related to master
$ git commit --amend HEAD~4 # conflict is detected with master~2, resolve it
$ git rebase --continue # Do we really want using "git rebase"?

This last workflow seems so awkward to me that I might miss something.

-- Jean Privat

^ permalink raw reply

* Re: [PATCH 1/3] t5403.1: simplify commit creation
From: Johannes Sixt @ 2011-10-12 14:14 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <1318412105-13595-1-git-send-email-pclouds@gmail.com>

Am 10/12/2011 11:35, schrieb Nguyễn Thái Ngọc Duy:
>  test_expect_success setup '
>  	echo Data for commit0. >a &&
>  	echo Data for commit0. >b &&
> -	git update-index --add a &&
> -	git update-index --add b &&
> -	tree0=$(git write-tree) &&
> -	commit0=$(echo setup | git commit-tree $tree0) &&
> -	git update-ref refs/heads/master $commit0 &&
> +	git add a b &&
> +	git commit -m setup &&
>  	git clone ./. clone1 &&
>  	git clone ./. clone2 &&
>  	GIT_DIR=clone2/.git git branch new2 &&

I don't think this change is necessary. It doesn't hurt to use plumbing
commands here and there in the test suite to exercise them to a degree
that they deserve.

-- Hannes

^ permalink raw reply

* Re: [PATCH 00/20] [GIT PULL][v3.2] tracing: queued updates
From: Jeff King @ 2011-10-12 14:19 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Valdis.Kletnieks, git, Steven Rostedt, linux-kernel,
	Andrew Morton, Thomas Gleixner, Frederic Weisbecker
In-Reply-To: <20111012080711.GM18618@elte.hu>

On Wed, Oct 12, 2011 at 10:07:14AM +0200, Ingo Molnar wrote:

> > On Tue, 11 Oct 2011 07:50:17 +0200, Ingo Molnar said:
> > 
> > >  $ git pull git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git tip/perf/core
> > >  fatal: The remote end hung up unexpectedly
> > 
> > Is it possible to get 'git' to say something more informative than 
> > "hung up unexpectedly"? "Tree not found, check URL" or similar 
> > would be nice...

It's not possible for the client to say anything more. The server sees
that the request isn't valid and hangs up without saying anything. So
the server needs to be changed to output better responses.

> Firstly, arguably, typoing something is not 'fatal' really - it's 
> just a resource that was not found on the server.
> 
> Secondly, and more importantly, the reason for the failed pull is 
> indeed important to know, if you want to resolve the problem with a 
> minimum fuss:
> 
>  - Was it the tree that didnt exist?
>  - Or the branch?
>  - Or was there some other problem [such as a truly unexpectedly 
>                                     closed transport socket]?
> 
> It's really useful for a painless UI flow to disambiguate failure 
> messages into clearly actionable variants.

I agree. I think some people are concerned with leaking information
about which repos exist and how they are configured. That is probably
not a big problem for a public site like kernel.org, though.

You might find this thread interesting:

  http://thread.gmane.org/gmane.comp.version-control.git/182529/focus=182642

It seems to have resulted in a patch that will at least say "access
denied" for every error. Which is a step up from "the remote end hung up
unexpectedly", but I do think most users would appreciate it being more
specific.

Perhaps we just need a config option to turn on more verbose messages,
if the site decides that there's no security implications to doing so.

-Peff

^ permalink raw reply

* Re: [PATCH] fix "git apply --index ..." not to deref NULL
From: Jeff King @ 2011-10-12 14:27 UTC (permalink / raw)
  To: Jim Meyering; +Cc: git
In-Reply-To: <87lisq8vye.fsf@rho.meyering.net>

On Wed, Oct 12, 2011 at 10:18:01AM +0200, Jim Meyering wrote:

> I noticed this when "git am CORRUPTED" unexpectedly failed with an
> odd diagnostic, and even removed one of the files it was supposed
> to have patched.
> 
> Reproduce with any valid old/new patch from which you have removed
> the "+++ b/FILE" line.  You'll see a diagnostic like this
> 
>     fatal: unable to write file '(null)' mode 100644: Bad address
> 
> and you'll find that FILE has been removed.

Yikes. Your fix looks right to me.

>  builtin/apply.c       |    3 +++
>  t/t4254-am-corrupt.sh |   43 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+), 0 deletions(-)
>  create mode 100644 t/t4254-am-corrupt.sh

Missing executable bit on the new test.

-Peff

^ permalink raw reply

* Re: [PATCH] fix "git apply --index ..." not to deref NULL
From: Jim Meyering @ 2011-10-12 14:33 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20111012142750.GB25085@sigill.intra.peff.net>

Jeff King wrote:
> On Wed, Oct 12, 2011 at 10:18:01AM +0200, Jim Meyering wrote:
>
>> I noticed this when "git am CORRUPTED" unexpectedly failed with an
>> odd diagnostic, and even removed one of the files it was supposed
>> to have patched.
>>
>> Reproduce with any valid old/new patch from which you have removed
>> the "+++ b/FILE" line.  You'll see a diagnostic like this
>>
>>     fatal: unable to write file '(null)' mode 100644: Bad address
>>
>> and you'll find that FILE has been removed.
>
> Yikes. Your fix looks right to me.
>
>>  builtin/apply.c       |    3 +++
>>  t/t4254-am-corrupt.sh |   43 +++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 46 insertions(+), 0 deletions(-)
>>  create mode 100644 t/t4254-am-corrupt.sh
>
> Missing executable bit on the new test.

Thanks.
Fixed with this:

-- >8 --
Subject: [PATCH] fix "git apply --index ..." not to deref NULL

I noticed this when "git am CORRUPTED" unexpectedly failed with an
odd diagnostic, and even removed one of the files it was supposed
to have patched.

Reproduce with any valid old/new patch from which you have removed
the "+++ b/FILE" line.  You'll see a diagnostic like this

    fatal: unable to write file '(null)' mode 100644: Bad address

and you'll find that FILE has been removed.

The above is on glibc-based systems.  On other systems, rather than
getting "null", you may provoke a segfault as git tries to
dereference the NULL file name.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 builtin/apply.c       |    3 +++
 t/t4254-am-corrupt.sh |   43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 0 deletions(-)
 create mode 100755 t/t4254-am-corrupt.sh

diff --git a/builtin/apply.c b/builtin/apply.c
index f2edc52..aaa39fe 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1407,6 +1407,9 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc
 					    "%d leading pathname components (line %d)" , p_value, linenr);
 				patch->old_name = patch->new_name = patch->def_name;
 			}
+			if (!patch->is_delete && !patch->new_name)
+				die("git diff header lacks filename information "
+				    "(line %d)", linenr);
 			patch->is_toplevel_relative = 1;
 			*hdrsize = git_hdr_len;
 			return offset;
diff --git a/t/t4254-am-corrupt.sh b/t/t4254-am-corrupt.sh
new file mode 100755
index 0000000..b7da95f
--- /dev/null
+++ b/t/t4254-am-corrupt.sh
@@ -0,0 +1,43 @@
+#!/bin/sh
+
+test_description='git am with corrupt input'
+. ./test-lib.sh
+
+# Note the missing "+++" line:
+cat > bad-patch.diff <<'EOF'
+From: A U Thor <au.thor@example.com>
+diff --git a/f b/f
+index 7898192..6178079 100644
+--- a/f
+@@ -1 +1 @@
+-a
++b
+EOF
+
+test_expect_success setup '
+	test $? = 0 &&
+	echo a > f &&
+	git add f &&
+	test_tick &&
+	git commit -m initial
+'
+
+# This used to fail before, too, but with a different diagnostic.
+#   fatal: unable to write file '(null)' mode 100644: Bad address
+# Also, it had the unwanted side-effect of deleting f.
+test_expect_success 'try to apply corrupted patch' '
+	git am bad-patch.diff 2> actual
+	test $? = 1
+'
+
+cat > expected <<EOF
+fatal: git diff header lacks filename information (line 4)
+EOF
+
+test_expect_success 'compare diagnostic; ensure file is still here' '
+	test $? = 0 &&
+	test -f f &&
+	test_cmp expected actual
+'
+
+test_done
--
1.7.7

^ permalink raw reply related

* Re: [PATCH v3] gitk: Teach gitk to respect log.showroot
From: Marcus Karlsson @ 2011-10-12 14:36 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: zbyszek, gitster, git
In-Reply-To: <20111008064704.GA27056@bloggs.ozlabs.ibm.com>

On Sat, Oct 08, 2011 at 05:47:04PM +1100, Paul Mackerras wrote:
> On Tue, Oct 04, 2011 at 10:08:13PM +0200, Marcus Karlsson wrote:
> > Teach gitk to respect log.showroot.
> 
> Sounds reasonable, ...
> 
> > -	set cmd [concat | git diff-tree -r $flags $ids]
> > +	set cmd [concat | git diff-tree -r]
> > +	if {$log_showroot eq true} {
> > +	    set cmd [concat $cmd --root]
> > +	}
> > +	set cmd [concat $cmd $flags $ids]
> 
> but is there any reason not to do it like this?
> 
> 	if {$log_showroot} {
> 	    lappend flags --root
> 	}
> 	set cmd [concat | git diff-tree -r $flags $ids]
> 
> I.e., do you particularly want the --root before the other flags?
> 
> Paul.

Not really, that would work very well.

Marcus

^ permalink raw reply

* [PATCH] Documentation: update [section.subsection] to reflect what git does
From: Carlos Martín Nieto @ 2011-10-12 15:52 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano

Using the [section.subsection] syntax, the subsection is transformed
to lower-case and is matched case sensitively. Say so in the
documentation and mention that you shouldn't be using it anyway.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---

This bit me recently when I was creating a parser. See Jeff's
explanation here:
http://thread.gmane.org/gmane.comp.version-control.git/179569/focus=180290

 Documentation/config.txt |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0658ffb..1212c47 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -45,9 +45,10 @@ lines.  Variables may belong directly to a section or to a given subsection.
 You can have `[section]` if you have `[section "subsection"]`, but you
 don't need to.
 
-There is also a case insensitive alternative `[section.subsection]` syntax.
-In this syntax, subsection names follow the same restrictions as for section
-names.
+There is also a deprecated `[section.subsection]` syntax. With this
+syntax, the subsection name is converted to lower-case and is also
+compared case sensitively. These subsection names follow the same
+restrictions as section names.
 
 All the other lines (and the remainder of the line after the section
 header) are recognized as setting variables, in the form
-- 
1.7.6.557.gcee4

^ permalink raw reply related

* Re: [PATCH] Documentation: update [section.subsection] to reflect what git does
From: Jeff King @ 2011-10-12 16:29 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git, Junio C Hamano
In-Reply-To: <1318434726-5556-1-git-send-email-cmn@elego.de>

On Wed, Oct 12, 2011 at 05:52:06PM +0200, Carlos Martín Nieto wrote:

> -There is also a case insensitive alternative `[section.subsection]` syntax.
> -In this syntax, subsection names follow the same restrictions as for section
> -names.
> +There is also a deprecated `[section.subsection]` syntax. With this
> +syntax, the subsection name is converted to lower-case and is also
> +compared case sensitively. These subsection names follow the same
> +restrictions as section names.

Hmm. While technically more correct, I think it is a little more
confusing to read. The lower-case canonicalization thing is actually
used for the other case-insensitive parts, too. So maybe it makes sense
to describe that in detail, and then just note that
"[section.subsection]" uses the same mechanism.

The patch below does this, and then the original text in the section you
tweaked above hopefully makes more sense in the new context.

The explanation matches what we do now, but it did end up a bit longer
than I had hoped. We could make it a lot shorter by:

  1. Canonicalizing the section and key names that the caller gives to
     git-config.

  2. Not mentioning the "section.foo" syntax. We can't canonicalize the
     subsection in (1) because of this syntax. But we can at least gloss
     over the detail, and then maybe just mention it much later in the
     file format. Or even deprecate it.

-- >8 --
Subject: [PATCH] docs/config: explain case-insensitive matching

We generally think of key matching as case-insensitive, but
it's not exactly. It's about canonicalizing one side, and
comparing it byte-wise with the canonical key name given to
git-config.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/git-config.txt |   50 +++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 49 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index e7ecf5d..e92aee9 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -28,7 +28,7 @@ DESCRIPTION
 -----------
 You can query/set/replace/unset options with this command. The name is
 actually the section and the key separated by a dot, and the value will be
-escaped.
+escaped. See the section on name matching below.
 
 Multiple lines can be added to an option by using the '--add' option.
 If you want to update or unset an option which can occur on multiple
@@ -178,6 +178,54 @@ See also <<FILES>>.
 	Opens an editor to modify the specified config file; either
 	'--system', '--global', or repository (default).
 
+
+NAME MATCHING
+-------------
+
+Configuration key names are matched using an algorithm that allows for
+partial case sensitivity. Section and key names are read from the config
+files, canonicalized according to the rules below, and then compared
+case-sensitively with the input given to git-config. Therefore any
+callers to git-config should request the canonicalized version of the
+name. This typically means lowercasing the section and key names, and
+leaving the subsection (if any) intact. For example, ask for
+`git config core.eol`, not `git config CoRe.EOL`.
+
+The canonicalization rules are:
+
+1. Lowercase the section and key names.
+
+2. If a literal subsection (like `[section "foo"]`) is used, leave it
+   intact.
+
+3. If a non-literal subsection (like `[section.foo]`) is used, lowercase
+   the subsection.
+
+4. Concatenate the resulting section, subsection, and key, separated by
+   a dot ('.').
+
+For example, this configuration file:
+
+-----------------------------------------------
+[CORE]
+eol = true
+
+[branch "Foo"]
+REMOTE = origin
+
+[color.DIFF]
+new = blue
+-----------------------------------------------
+
+would yield the following three canonicalized names:
+
+-----------------------------------------------
+core.eol
+branch.Foo.remote
+color.diff.new
+-----------------------------------------------
+
+
 [[FILES]]
 FILES
 -----
-- 
1.7.7.rc2.21.gb9948

^ permalink raw reply related

* Re: [PATCH 00/20] [GIT PULL][v3.2] tracing: queued updates
From: Ingo Molnar @ 2011-10-12 16:29 UTC (permalink / raw)
  To: Jeff King
  Cc: Valdis.Kletnieks, git, Steven Rostedt, linux-kernel,
	Andrew Morton, Thomas Gleixner, Frederic Weisbecker
In-Reply-To: <20111012141939.GA25085@sigill.intra.peff.net>


* Jeff King <peff@peff.net> wrote:

> > It's really useful for a painless UI flow to disambiguate failure 
> > messages into clearly actionable variants.
> 
> I agree. I think some people are concerned with leaking information 
> about which repos exist and how they are configured. That is 
> probably not a big problem for a public site like kernel.org, 
> though.

Well, a gitconfig option could be provided to not leak that small 
amount of info - but it would otherwise be weird for an OSS SCM to 
default to a behavior that only makes sense with closed source, 
right? :-)

Thanks,

	Ingo

^ permalink raw reply

* Re: Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted references
From: Junio C Hamano @ 2011-10-12 16:52 UTC (permalink / raw)
  To: Lars Hjemli, Jeff King
  Cc: Michael Haggerty, git, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <20111011225434.GA24142@sigill.intra.peff.net>

Subject: [PATCH] branch -m/-M: remove undocumented RENAMED-REF

The commit message for c976d41 (git-branch: add options and tests for
branch renaming, 2006-11-28) mentions RENAME_REF but otherwise this is not
documented anywhere, and it does not appear in any of the tests.

Worse yet, the name of the actual file is "RENAMED-REF".

This was supposed to hold the commit object name at the tip of the branch
the most recent "branch -m/-M" renamed, but that is not necessary in order
to be able to recover from a mistake. Even when "branch -M A B" overwrites
an existing branch B, what is kept in RENAMED-REF is the commit at the tip
of the original branch A, not the commit B from the now-lost branch.

Just remove this unused "feature".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * Let's do this.

 refs.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/refs.c b/refs.c
index cdedb45..ecfc22c 100644
--- a/refs.c
+++ b/refs.c
@@ -786,7 +786,6 @@ int delete_ref(const char *refname, unsigned char *sha1)
 
 int rename_ref(const char *oldref, const char *newref)
 {
-	static const char renamed_ref[] = "RENAMED-REF";
 	unsigned char sha1[20], orig_sha1[20];
 	int flag = 0, logmoved = 0;
 	struct ref_lock *lock;
@@ -809,13 +808,6 @@ int rename_ref(const char *oldref, const char *newref)
 	if (snprintf(msg, sizeof(msg), "renamed %s to %s", oldref, newref) > sizeof(msg))
 		return error("Refnames to long");
 
-	lock = lock_ref_sha1_basic(renamed_ref, NULL, NULL);
-	if (!lock)
-		return error("unable to lock %s", renamed_ref);
-	lock->force_write = 1;
-	if (write_ref_sha1(lock, orig_sha1, msg))
-		return error("unable to save current sha1 in %s", renamed_ref);
-
 	if (log && rename(git_path("logs/%s", oldref), git_path("tmp-renamed-log")))
 		return error("unable to move logfile logs/%s to tmp-renamed-log: %s",
 			oldref, strerror(errno));
-- 
1.7.7.213.g8b0e1

^ permalink raw reply related

* Re: [PATCH] Documentation: update [section.subsection] to reflect what git does
From: Jeff King @ 2011-10-12 17:46 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git, Junio C Hamano
In-Reply-To: <20111012162939.GA3055@sigill.intra.peff.net>

On Wed, Oct 12, 2011 at 12:29:39PM -0400, Jeff King wrote:

> The explanation matches what we do now, but it did end up a bit longer
> than I had hoped. We could make it a lot shorter by:
> 
>   1. Canonicalizing the section and key names that the caller gives to
>      git-config.

Hmm. Scratch that. We seem to do this already in my tests. I'll look
further and try to make a better documentation patch.

-Peff

^ permalink raw reply

* Re: [PATCH] Documentation: update [section.subsection] to reflect what git does
From: Junio C Hamano @ 2011-10-12 17:46 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git, Jeff King
In-Reply-To: <1318434726-5556-1-git-send-email-cmn@elego.de>

Carlos Martín Nieto <cmn@elego.de> writes:

> This bit me recently when I was creating a parser. See Jeff's
> explanation here:
> http://thread.gmane.org/gmane.comp.version-control.git/179569/focus=180290

I think you meant to focus on 180123 but anyway I think the updated text reads
much better.

Thanks.

>  Documentation/config.txt |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 0658ffb..1212c47 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -45,9 +45,10 @@ lines.  Variables may belong directly to a section or to a given subsection.
>  You can have `[section]` if you have `[section "subsection"]`, but you
>  don't need to.
>  
> -There is also a case insensitive alternative `[section.subsection]` syntax.
> -In this syntax, subsection names follow the same restrictions as for section
> -names.
> +There is also a deprecated `[section.subsection]` syntax. With this
> +syntax, the subsection name is converted to lower-case and is also
> +compared case sensitively. These subsection names follow the same
> +restrictions as section names.
>  
>  All the other lines (and the remainder of the line after the section
>  header) are recognized as setting variables, in the form

^ permalink raw reply

* [PATCH 1/2] refs.c: move dwim_ref()/dwim_log() from sha1_name.c
From: Junio C Hamano @ 2011-10-12 17:48 UTC (permalink / raw)
  To: Jeff King
  Cc: Michael Haggerty, git, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <20111012045004.GA22413@sigill.intra.peff.net>

OK, so here is a re-roll of the one from yesterday. This one is new but is
a mere preparatory step.

-- >8 --

Both dwim_ref()/dwim_log() functions are intimately related to the ref
parsing rules defined in refs.c and better fits there. Move them together
with substitute_branch_name(), a file scope static helper function.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 refs.c      |   85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 sha1_name.c |   85 -----------------------------------------------------------
 2 files changed, 85 insertions(+), 85 deletions(-)

diff --git a/refs.c b/refs.c
index 832a52f..e3692bd 100644
--- a/refs.c
+++ b/refs.c
@@ -1067,6 +1067,91 @@ static int is_refname_available(const char *ref, const char *oldref,
 	return 1;
 }
 
+/*
+ * *string and *len will only be substituted, and *string returned (for
+ * later free()ing) if the string passed in is a magic short-hand form
+ * to name a branch.
+ */
+static char *substitute_branch_name(const char **string, int *len)
+{
+	struct strbuf buf = STRBUF_INIT;
+	int ret = interpret_branch_name(*string, &buf);
+
+	if (ret == *len) {
+		size_t size;
+		*string = strbuf_detach(&buf, &size);
+		*len = size;
+		return (char *)*string;
+	}
+
+	return NULL;
+}
+
+int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
+{
+	char *last_branch = substitute_branch_name(&str, &len);
+	const char **p, *r;
+	int refs_found = 0;
+
+	*ref = NULL;
+	for (p = ref_rev_parse_rules; *p; p++) {
+		char fullref[PATH_MAX];
+		unsigned char sha1_from_ref[20];
+		unsigned char *this_result;
+		int flag;
+
+		this_result = refs_found ? sha1_from_ref : sha1;
+		mksnpath(fullref, sizeof(fullref), *p, len, str);
+		r = resolve_ref(fullref, this_result, 1, &flag);
+		if (r) {
+			if (!refs_found++)
+				*ref = xstrdup(r);
+			if (!warn_ambiguous_refs)
+				break;
+		} else if ((flag & REF_ISSYMREF) && strcmp(fullref, "HEAD"))
+			warning("ignoring dangling symref %s.", fullref);
+	}
+	free(last_branch);
+	return refs_found;
+}
+
+int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
+{
+	char *last_branch = substitute_branch_name(&str, &len);
+	const char **p;
+	int logs_found = 0;
+
+	*log = NULL;
+	for (p = ref_rev_parse_rules; *p; p++) {
+		struct stat st;
+		unsigned char hash[20];
+		char path[PATH_MAX];
+		const char *ref, *it;
+
+		mksnpath(path, sizeof(path), *p, len, str);
+		ref = resolve_ref(path, hash, 1, NULL);
+		if (!ref)
+			continue;
+		if (!stat(git_path("logs/%s", path), &st) &&
+		    S_ISREG(st.st_mode))
+			it = path;
+		else if (strcmp(ref, path) &&
+			 !stat(git_path("logs/%s", ref), &st) &&
+			 S_ISREG(st.st_mode))
+			it = ref;
+		else
+			continue;
+		if (!logs_found++) {
+			*log = xstrdup(it);
+			hashcpy(sha1, hash);
+		}
+		if (!warn_ambiguous_refs)
+			break;
+	}
+	free(last_branch);
+	return logs_found;
+}
+
 static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char *old_sha1, int flags, int *type_p)
 {
 	char *ref_file;
diff --git a/sha1_name.c b/sha1_name.c
index 143fd97..d423635 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -241,91 +241,6 @@ static int ambiguous_path(const char *path, int len)
 	return slash;
 }
 
-/*
- * *string and *len will only be substituted, and *string returned (for
- * later free()ing) if the string passed in is a magic short-hand form
- * to name a branch.
- */
-static char *substitute_branch_name(const char **string, int *len)
-{
-	struct strbuf buf = STRBUF_INIT;
-	int ret = interpret_branch_name(*string, &buf);
-
-	if (ret == *len) {
-		size_t size;
-		*string = strbuf_detach(&buf, &size);
-		*len = size;
-		return (char *)*string;
-	}
-
-	return NULL;
-}
-
-int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
-{
-	char *last_branch = substitute_branch_name(&str, &len);
-	const char **p, *r;
-	int refs_found = 0;
-
-	*ref = NULL;
-	for (p = ref_rev_parse_rules; *p; p++) {
-		char fullref[PATH_MAX];
-		unsigned char sha1_from_ref[20];
-		unsigned char *this_result;
-		int flag;
-
-		this_result = refs_found ? sha1_from_ref : sha1;
-		mksnpath(fullref, sizeof(fullref), *p, len, str);
-		r = resolve_ref(fullref, this_result, 1, &flag);
-		if (r) {
-			if (!refs_found++)
-				*ref = xstrdup(r);
-			if (!warn_ambiguous_refs)
-				break;
-		} else if ((flag & REF_ISSYMREF) && strcmp(fullref, "HEAD"))
-			warning("ignoring dangling symref %s.", fullref);
-	}
-	free(last_branch);
-	return refs_found;
-}
-
-int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
-{
-	char *last_branch = substitute_branch_name(&str, &len);
-	const char **p;
-	int logs_found = 0;
-
-	*log = NULL;
-	for (p = ref_rev_parse_rules; *p; p++) {
-		struct stat st;
-		unsigned char hash[20];
-		char path[PATH_MAX];
-		const char *ref, *it;
-
-		mksnpath(path, sizeof(path), *p, len, str);
-		ref = resolve_ref(path, hash, 1, NULL);
-		if (!ref)
-			continue;
-		if (!stat(git_path("logs/%s", path), &st) &&
-		    S_ISREG(st.st_mode))
-			it = path;
-		else if (strcmp(ref, path) &&
-			 !stat(git_path("logs/%s", ref), &st) &&
-			 S_ISREG(st.st_mode))
-			it = ref;
-		else
-			continue;
-		if (!logs_found++) {
-			*log = xstrdup(it);
-			hashcpy(sha1, hash);
-		}
-		if (!warn_ambiguous_refs)
-			break;
-	}
-	free(last_branch);
-	return logs_found;
-}
-
 static inline int upstream_mark(const char *string, int len)
 {
 	const char *suffix[] = { "@{upstream}", "@{u}" };
-- 
1.7.7.213.g8b0e1

^ permalink raw reply related

* [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR
From: Junio C Hamano @ 2011-10-12 17:49 UTC (permalink / raw)
  To: Jeff King
  Cc: Michael Haggerty, git, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <20111012045004.GA22413@sigill.intra.peff.net>

We have always dwimmed the user input $string into a ref by first looking
directly inside $GIT_DIR, and then in $GIT_DIR/refs, $GIT_DIR/refs/tags,
etc., and that is what made

    git log HEAD..MERGE_HEAD

work correctly. This however means that

    git rev-parse config
    git log index

would look at $GIT_DIR/config and $GIT_DIR/index and see if they are valid
refs.

To reduce confusion, let's not dwim a path immediately below $GIT_DIR that
is not all-caps.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * And this adds coverage to refname_match() and shorten_unambiguous_ref()
   on top of the one from yesterday.
 
 refs.c |   36 ++++++++++++++++++++++++++++++++++--
 1 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index e3692bd..e54c482 100644
--- a/refs.c
+++ b/refs.c
@@ -994,12 +994,34 @@ const char *ref_fetch_rules[] = {
 	NULL
 };
 
+static int refname_ok_at_root_level(const char *str, int len)
+{
+	int seen_non_root_char = 0;
+
+	while (len--) {
+		char ch = *str++;
+
+		if (ch == '/')
+			return 1;
+		/*
+		 * Only accept likes of .git/HEAD, .git/MERGE_HEAD at
+		 * the root level as a ref.
+		 */
+		if (ch != '_' && (ch < 'A' || 'Z' < ch))
+			seen_non_root_char = 1;
+	}
+	return !seen_non_root_char;
+}
+
 int refname_match(const char *abbrev_name, const char *full_name, const char **rules)
 {
 	const char **p;
 	const int abbrev_name_len = strlen(abbrev_name);
 
 	for (p = rules; *p; p++) {
+		if (p == rules &&
+		    !refname_ok_at_root_level(abbrev_name, abbrev_name_len))
+			continue;
 		if (!strcmp(full_name, mkpath(*p, abbrev_name_len, abbrev_name))) {
 			return 1;
 		}
@@ -1100,6 +1122,8 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 		unsigned char *this_result;
 		int flag;
 
+		if (p == ref_rev_parse_rules && !refname_ok_at_root_level(str, len))
+			continue;
 		this_result = refs_found ? sha1_from_ref : sha1;
 		mksnpath(fullref, sizeof(fullref), *p, len, str);
 		r = resolve_ref(fullref, this_result, 1, &flag);
@@ -1128,6 +1152,8 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 		char path[PATH_MAX];
 		const char *ref, *it;
 
+		if (p == ref_rev_parse_rules && !refname_ok_at_root_level(str, len))
+			continue;
 		mksnpath(path, sizeof(path), *p, len, str);
 		ref = resolve_ref(path, hash, 1, NULL);
 		if (!ref)
@@ -2045,12 +2071,14 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
 	/* buffer for scanf result, at most ref must fit */
 	short_name = xstrdup(ref);
 
-	/* skip first rule, it will always match */
-	for (i = nr_rules - 1; i > 0 ; --i) {
+	for (i = nr_rules - 1; i >= 0; i--) {
 		int j;
 		int rules_to_fail = i;
 		int short_name_len;
 
+		if (!i && !refname_ok_at_root_level(ref, strlen(ref)))
+			continue;
+
 		if (1 != sscanf(ref, scanf_fmts[i], short_name))
 			continue;
 
@@ -2076,6 +2104,10 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
 			if (i == j)
 				continue;
 
+			if (!j &&
+			    !refname_ok_at_root_level(short_name, short_name_len))
+				continue;
+
 			/*
 			 * the short name is ambiguous, if it resolves
 			 * (with this previous rule) to a valid ref
-- 
1.7.7.213.g8b0e1

^ permalink raw reply related

* Re: [PATCH] Make is_gitfile a non-static generic function
From: Phil Hord @ 2011-10-12 18:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Phil Hord, git@vger.kernel.org
In-Reply-To: <7vipnvccso.fsf@alter.siamese.dyndns.org>

On Tue, Oct 11, 2011 at 7:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
> After looking at this patch and the way the other caller in transport.c
> uses it, I am more and more convinced that "is_gitfile()" is a stupid and
> horrible mistake.

I think I misunderstood your objection before.  Now I think I
understand.  Tell me if I am right.


I think you mean that instead of this:
        } else if (is_local(url) && is_file(url) && !is_gitfile(url)) {

you would like to see this:
        } else if (is_local(url) && is_file(url) && is_bundle(url)) {

Or maybe even this:
        } else if (is_bundle(url)) {

Phil

^ permalink raw reply

* Re: [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR
From: Michael Haggerty @ 2011-10-12 18:01 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, git, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <7vvcru9k22.fsf_-_@alter.siamese.dyndns.org>

On 10/12/2011 07:49 PM, Junio C Hamano wrote:
> diff --git a/refs.c b/refs.c
> index e3692bd..e54c482 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -994,12 +994,34 @@ const char *ref_fetch_rules[] = {
>  	NULL
>  };
>  
> +static int refname_ok_at_root_level(const char *str, int len)
> +{
> +	int seen_non_root_char = 0;
> +
> +	while (len--) {
> +		char ch = *str++;
> +
> +		if (ch == '/')
> +			return 1;
> +		/*
> +		 * Only accept likes of .git/HEAD, .git/MERGE_HEAD at
> +		 * the root level as a ref.
> +		 */
> +		if (ch != '_' && (ch < 'A' || 'Z' < ch))
> +			seen_non_root_char = 1;
> +	}
> +	return !seen_non_root_char;
> +}
> +

Nit: the seen_non_root_char variable can be replaced by an early "return
0" from the loop and "return 1" if the loop falls through.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR
From: Junio C Hamano @ 2011-10-12 18:07 UTC (permalink / raw)
  To: Michael Haggerty
  Cc: Jeff King, git, cmn, A Large Angry SCM, Daniel Barkalow,
	Sverre Rabbelier
In-Reply-To: <4E95D60B.5030904@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> On 10/12/2011 07:49 PM, Junio C Hamano wrote:
>> diff --git a/refs.c b/refs.c
>> index e3692bd..e54c482 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -994,12 +994,34 @@ const char *ref_fetch_rules[] = {
>>  	NULL
>>  };
>>  
>> +static int refname_ok_at_root_level(const char *str, int len)
>> +{
>> +	int seen_non_root_char = 0;
>> +
>> +	while (len--) {
>> +		char ch = *str++;
>> +
>> +		if (ch == '/')
>> +			return 1;
>> +		/*
>> +		 * Only accept likes of .git/HEAD, .git/MERGE_HEAD at
>> +		 * the root level as a ref.
>> +		 */
>> +		if (ch != '_' && (ch < 'A' || 'Z' < ch))
>> +			seen_non_root_char = 1;
>> +	}
>> +	return !seen_non_root_char;
>> +}
>> +
>
> Nit: the seen_non_root_char variable can be replaced by an early "return
> 0" from the loop and "return 1" if the loop falls through.

Hmm, I thought that would fail when you feed "refs/heads/master" to the
function.

^ permalink raw reply

* Re: [PATCH] Make is_gitfile a non-static generic function
From: Junio C Hamano @ 2011-10-12 18:08 UTC (permalink / raw)
  To: Phil Hord; +Cc: Phil Hord, git@vger.kernel.org
In-Reply-To: <CABURp0p4_oVMmkFDPue4kvhO_bEr_dBh-XFGArdSJFMz-Eboeg@mail.gmail.com>

Phil Hord <phil.hord@gmail.com> writes:

> On Tue, Oct 11, 2011 at 7:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> After looking at this patch and the way the other caller in transport.c
>> uses it, I am more and more convinced that "is_gitfile()" is a stupid and
>> horrible mistake.
>
> I think I misunderstood your objection before.  Now I think I
> understand.  Tell me if I am right.
>
>
> I think you mean that instead of this:
>         } else if (is_local(url) && is_file(url) && !is_gitfile(url)) {
>
> you would like to see this:
>         } else if (is_local(url) && is_file(url) && is_bundle(url)) {
>
> Or maybe even this:
>         } else if (is_bundle(url)) {

Exactly.

^ permalink raw reply

* Re: [PATCH] Documentation: update [section.subsection] to reflect what git does
From: Jeff King @ 2011-10-12 18:27 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git, Junio C Hamano
In-Reply-To: <20111012174643.GA14336@sigill.intra.peff.net>

On Wed, Oct 12, 2011 at 01:46:43PM -0400, Jeff King wrote:

> On Wed, Oct 12, 2011 at 12:29:39PM -0400, Jeff King wrote:
> 
> > The explanation matches what we do now, but it did end up a bit longer
> > than I had hoped. We could make it a lot shorter by:
> > 
> >   1. Canonicalizing the section and key names that the caller gives to
> >      git-config.
> 
> Hmm. Scratch that. We seem to do this already in my tests. I'll look
> further and try to make a better documentation patch.

OK, I was all set to do a patch to git-config for this, but it seems the
code is already there.  It's only the subsections which are the sticking
point, and those can't be canonicalized, because in most cases we need
to match them exactly.

In the process, I did some cleanup and added some new tests to t1300,
which I think are probably worth applying anyway.

  [1/2]: t1300: put git invocations inside test function
  [2/2]: t1300: test mixed-case variable retrieval

-Peff

^ permalink raw reply

* [PATCH 1/2] t1300: put git invocations inside test function
From: Jeff King @ 2011-10-12 18:29 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: Junio C Hamano, git
In-Reply-To: <20111012182742.GA14543@sigill.intra.peff.net>

This is a very old script, and did a lot of:

  echo whatever >expect
  git config foo bar
  test_expect_success 'cmp .git/config expect'

which meant that we didn't actually check that the call to
git-config succeeded. Fix this, and while we're at it,
modernize the style to use test_cmp.

Signed-off-by: Jeff King <peff@peff.net>
---
There are still a few 'cp' and 'rm' calls outside of the test functions,
but we can generally expect those to work (as we do with the 'cat'
calls).

 t/t1300-repo-config.sh |  176 +++++++++++++++++++++++++-----------------------
 1 files changed, 93 insertions(+), 83 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 3e140c1..cf508af 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -7,28 +7,28 @@ test_description='Test git config in different settings'
 
 . ./test-lib.sh
 
-test -f .git/config && rm .git/config
-
-git config core.penguin "little blue"
+test_expect_success 'clear default config' '
+	rm -f .git/config
+'
 
 cat > expect << EOF
 [core]
 	penguin = little blue
 EOF
-
-test_expect_success 'initial' 'cmp .git/config expect'
-
-git config Core.Movie BadPhysics
+test_expect_success 'initial' '
+	git config core.penguin "little blue" &&
+	test_cmp expect .git/config
+'
 
 cat > expect << EOF
 [core]
 	penguin = little blue
 	Movie = BadPhysics
 EOF
-
-test_expect_success 'mixed case' 'cmp .git/config expect'
-
-git config Cores.WhatEver Second
+test_expect_success 'mixed case' '
+	git config Core.Movie BadPhysics &&
+	test_cmp expect .git/config
+'
 
 cat > expect << EOF
 [core]
@@ -37,10 +37,10 @@ cat > expect << EOF
 [Cores]
 	WhatEver = Second
 EOF
-
-test_expect_success 'similar section' 'cmp .git/config expect'
-
-git config CORE.UPPERCASE true
+test_expect_success 'similar section' '
+	git config Cores.WhatEver Second
+	test_cmp expect .git/config
+'
 
 cat > expect << EOF
 [core]
@@ -50,8 +50,10 @@ cat > expect << EOF
 [Cores]
 	WhatEver = Second
 EOF
-
-test_expect_success 'similar section' 'cmp .git/config expect'
+test_expect_success 'uppercase section' '
+	git config CORE.UPPERCASE true &&
+	test_cmp expect .git/config
+'
 
 test_expect_success 'replace with non-match' \
 	'git config core.penguin kingpin !blue'
@@ -69,7 +71,7 @@ cat > expect << EOF
 	WhatEver = Second
 EOF
 
-test_expect_success 'non-match result' 'cmp .git/config expect'
+test_expect_success 'non-match result' 'test_cmp expect .git/config'
 
 cat > .git/config <<\EOF
 [alpha]
@@ -88,7 +90,7 @@ bar = foo
 [beta]
 EOF
 
-test_expect_success 'unset with cont. lines is correct' 'cmp .git/config expect'
+test_expect_success 'unset with cont. lines is correct' 'test_cmp expect .git/config'
 
 cat > .git/config << EOF
 [beta] ; silly comment # another comment
@@ -116,7 +118,7 @@ noIndent= sillyValue ; 'nother silly comment
 [nextSection] noNewline = ouch
 EOF
 
-test_expect_success 'multiple unset is correct' 'cmp .git/config expect'
+test_expect_success 'multiple unset is correct' 'test_cmp expect .git/config'
 
 cp .git/config2 .git/config
 
@@ -140,9 +142,7 @@ noIndent= sillyValue ; 'nother silly comment
 [nextSection] noNewline = ouch
 EOF
 
-test_expect_success 'all replaced' 'cmp .git/config expect'
-
-git config beta.haha alpha
+test_expect_success 'all replaced' 'test_cmp expect .git/config'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -153,10 +153,10 @@ noIndent= sillyValue ; 'nother silly comment
 	haha = alpha
 [nextSection] noNewline = ouch
 EOF
-
-test_expect_success 'really mean test' 'cmp .git/config expect'
-
-git config nextsection.nonewline wow
+test_expect_success 'really mean test' '
+	git config beta.haha alpha &&
+	test_cmp expect .git/config
+'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -168,11 +168,12 @@ noIndent= sillyValue ; 'nother silly comment
 [nextSection]
 	nonewline = wow
 EOF
-
-test_expect_success 'really really mean test' 'cmp .git/config expect'
+test_expect_success 'really really mean test' '
+	git config nextsection.nonewline wow &&
+	test_cmp expect .git/config
+'
 
 test_expect_success 'get value' 'test alpha = $(git config beta.haha)'
-git config --unset beta.haha
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -183,10 +184,10 @@ noIndent= sillyValue ; 'nother silly comment
 [nextSection]
 	nonewline = wow
 EOF
-
-test_expect_success 'unset' 'cmp .git/config expect'
-
-git config nextsection.NoNewLine "wow2 for me" "for me$"
+test_expect_success 'unset' '
+	git config --unset beta.haha &&
+	test_cmp expect .git/config
+'
 
 cat > expect << EOF
 [beta] ; silly comment # another comment
@@ -198,8 +199,10 @@ noIndent= sillyValue ; 'nother silly comment
 	nonewline = wow
 	NoNewLine = wow2 for me
 EOF
-
-test_expect_success 'multivar' 'cmp .git/config expect'
+test_expect_success 'multivar' '
+	git config nextsection.NoNewLine "wow2 for me" "for me$" &&
+	test_cmp expect .git/config
+'
 
 test_expect_success 'non-match' \
 	'git config --get nextsection.nonewline !for'
@@ -214,8 +217,6 @@ test_expect_success 'ambiguous get' '
 test_expect_success 'get multivar' \
 	'git config --get-all nextsection.nonewline'
 
-git config nextsection.nonewline "wow3" "wow$"
-
 cat > expect << EOF
 [beta] ; silly comment # another comment
 noIndent= sillyValue ; 'nother silly comment
@@ -226,8 +227,10 @@ noIndent= sillyValue ; 'nother silly comment
 	nonewline = wow3
 	NoNewLine = wow2 for me
 EOF
-
-test_expect_success 'multivar replace' 'cmp .git/config expect'
+test_expect_success 'multivar replace' '
+	git config nextsection.nonewline "wow3" "wow$" &&
+	test_cmp expect .git/config
+'
 
 test_expect_success 'ambiguous value' '
 	test_must_fail git config nextsection.nonewline
@@ -241,8 +244,6 @@ test_expect_success 'invalid unset' '
 	test_must_fail git config --unset somesection.nonewline
 '
 
-git config --unset nextsection.nonewline "wow3$"
-
 cat > expect << EOF
 [beta] ; silly comment # another comment
 noIndent= sillyValue ; 'nother silly comment
@@ -253,7 +254,10 @@ noIndent= sillyValue ; 'nother silly comment
 	NoNewLine = wow2 for me
 EOF
 
-test_expect_success 'multivar unset' 'cmp .git/config expect'
+test_expect_success 'multivar unset' '
+	git config --unset nextsection.nonewline "wow3$" &&
+	test_cmp expect .git/config
+'
 
 test_expect_success 'invalid key' 'test_must_fail git config inval.2key blabla'
 
@@ -276,7 +280,7 @@ noIndent= sillyValue ; 'nother silly comment
 	Alpha = beta
 EOF
 
-test_expect_success 'hierarchical section value' 'cmp .git/config expect'
+test_expect_success 'hierarchical section value' 'test_cmp expect .git/config'
 
 cat > expect << EOF
 beta.noindent=sillyValue
@@ -304,15 +308,16 @@ EOF
 test_expect_success '--get-regexp' \
 	'git config --get-regexp in > output && cmp output expect'
 
-git config --add nextsection.nonewline "wow4 for you"
-
 cat > expect << EOF
 wow2 for me
 wow4 for you
 EOF
 
-test_expect_success '--add' \
-	'git config --get-all nextsection.nonewline > output && cmp output expect'
+test_expect_success '--add' '
+	git config --add nextsection.nonewline "wow4 for you" &&
+	git config --get-all nextsection.nonewline > output &&
+	test_cmp expect output
+'
 
 cat > .git/config << EOF
 [novalue]
@@ -361,8 +366,6 @@ cat > .git/config << EOF
 	c = d
 EOF
 
-git config a.x y
-
 cat > expect << EOF
 [a.b]
 	c = d
@@ -370,10 +373,10 @@ cat > expect << EOF
 	x = y
 EOF
 
-test_expect_success 'new section is partial match of another' 'cmp .git/config expect'
-
-git config b.x y
-git config a.b c
+test_expect_success 'new section is partial match of another' '
+	git config a.x y &&
+	test_cmp expect .git/config
+'
 
 cat > expect << EOF
 [a.b]
@@ -385,7 +388,11 @@ cat > expect << EOF
 	x = y
 EOF
 
-test_expect_success 'new variable inserts into proper section' 'cmp .git/config expect'
+test_expect_success 'new variable inserts into proper section' '
+	git config b.x y &&
+	git config a.b c &&
+	test_cmp expect .git/config
+'
 
 test_expect_success 'alternative GIT_CONFIG (non-existing file should fail)' \
 	'test_must_fail git config --file non-existing-config -l'
@@ -399,9 +406,10 @@ cat > expect << EOF
 ein.bahn=strasse
 EOF
 
-GIT_CONFIG=other-config git config -l > output
-
-test_expect_success 'alternative GIT_CONFIG' 'cmp output expect'
+test_expect_success 'alternative GIT_CONFIG' '
+	GIT_CONFIG=other-config git config -l >output &&
+	test_cmp expect output
+'
 
 test_expect_success 'alternative GIT_CONFIG (--file)' \
 	'git config --file other-config -l > output && cmp output expect'
@@ -417,8 +425,6 @@ test_expect_success 'refer config from subdirectory' '
 
 '
 
-GIT_CONFIG=other-config git config anwohner.park ausweis
-
 cat > expect << EOF
 [ein]
 	bahn = strasse
@@ -426,7 +432,10 @@ cat > expect << EOF
 	park = ausweis
 EOF
 
-test_expect_success '--set in alternative GIT_CONFIG' 'cmp other-config expect'
+test_expect_success '--set in alternative GIT_CONFIG' '
+	GIT_CONFIG=other-config git config anwohner.park ausweis &&
+	test_cmp expect other-config
+'
 
 cat > .git/config << EOF
 # Hallo
@@ -531,7 +540,7 @@ test_expect_success 'section ending' '
 	git config gitcvs.enabled true &&
 	git config gitcvs.ext.dbname %Ggitcvs1.%a.%m.sqlite &&
 	git config gitcvs.dbname %Ggitcvs2.%a.%m.sqlite &&
-	cmp .git/config expect
+	test_cmp expect .git/config
 
 '
 
@@ -750,13 +759,6 @@ test_expect_success NOT_MINGW 'get --path copes with unset $HOME' '
 	test_cmp expect result
 '
 
-rm .git/config
-
-git config quote.leading " test"
-git config quote.ending "test "
-git config quote.semicolon "test;test"
-git config quote.hash "test#test"
-
 cat > expect << EOF
 [quote]
 	leading = " test"
@@ -764,8 +766,14 @@ cat > expect << EOF
 	semicolon = "test;test"
 	hash = "test#test"
 EOF
-
-test_expect_success 'quoting' 'cmp .git/config expect'
+test_expect_success 'quoting' '
+	rm .git/config &&
+	git config quote.leading " test" &&
+	git config quote.ending "test " &&
+	git config quote.semicolon "test;test" &&
+	git config quote.hash "test#test" &&
+	test_cmp expect .git/config
+'
 
 test_expect_success 'key with newline' '
 	test_must_fail git config "key.with
@@ -790,9 +798,10 @@ section.noncont=not continued
 section.quotecont=cont;inued
 EOF
 
-git config --list > result
-
-test_expect_success 'value continued on next line' 'cmp result expect'
+test_expect_success 'value continued on next line' '
+	git config --list > result &&
+	cmp result expect
+'
 
 cat > .git/config <<\EOF
 [section "sub=section"]
@@ -813,16 +822,17 @@ barQsection.sub=section.val3
 Qsection.sub=section.val4
 Qsection.sub=section.val5Q
 EOF
+test_expect_success '--null --list' '
+	git config --null --list | nul_to_q >result &&
+	echo >>result &&
+	test_cmp expect result
+'
 
-git config --null --list | perl -pe 'y/\000/Q/' > result
-echo >>result
-
-test_expect_success '--null --list' 'cmp result expect'
-
-git config --null --get-regexp 'val[0-9]' | perl -pe 'y/\000/Q/' > result
-echo >>result
-
-test_expect_success '--null --get-regexp' 'cmp result expect'
+test_expect_success '--null --get-regexp' '
+	git config --null --get-regexp "val[0-9]" | nul_to_q >result &&
+	echo >>result &&
+	test_cmp expect result
+'
 
 test_expect_success 'inner whitespace kept verbatim' '
 	git config section.val "foo 	  bar" &&
-- 
1.7.7.rc2.21.gb9948

^ permalink raw reply related

* [PATCH 2/2] t1300: test mixed-case variable retrieval
From: Jeff King @ 2011-10-12 18:30 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: Junio C Hamano, git
In-Reply-To: <20111012182742.GA14543@sigill.intra.peff.net>

We should be able to ask for a config value both by its
canonical all-lowercase name (as git does internally), as
well as by random mixed-case (which will be canonicalized by
git-config for us).

Subsections are a tricky point, though. Since we have both

  [section "Foo"]

and

  [section.Foo]

you might want git-config to canonicalize the subsection or
not, depending on which you are expecting. But there's no
way to communicate this; git-config sees only the key, and
doesn't know which type of section name will be in the
config file.

So it must leave the subsection intact, and it is up to the
caller to provide a canonical version of the subsection if
they want to match the latter form.

Signed-off-by: Jeff King <peff@peff.net>
---
I was surprised this wasn't tested anywhere, but I couldn't find any
such place. I think it makes sense to document the desired behavior in
the form of a few tests.

 t/t1300-repo-config.sh |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index cf508af..8a37f96 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -73,6 +73,33 @@ EOF
 
 test_expect_success 'non-match result' 'test_cmp expect .git/config'
 
+test_expect_success 'find mixed-case key by canonical name' '
+	echo Second >expect &&
+	git config cores.whatever >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'find mixed-case key by non-canonical name' '
+	echo Second >expect &&
+	git config CoReS.WhAtEvEr >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'subsections are not canonicalized by git-config' '
+	cat >>.git/config <<-\EOF &&
+	[section.SubSection]
+	key = one
+	[section "SubSection"]
+	key = two
+	EOF
+	echo one >expect &&
+	git config section.subsection.key >actual &&
+	test_cmp expect actual &&
+	echo two >expect &&
+	git config section.SubSection.key >actual &&
+	test_cmp expect actual
+'
+
 cat > .git/config <<\EOF
 [alpha]
 bar = foo
-- 
1.7.7.rc2.21.gb9948

^ permalink raw reply related

* Re: [PATCH] Documentation: update [section.subsection] to reflect what git does
From: Jeff King @ 2011-10-12 18:34 UTC (permalink / raw)
  To: Carlos Martín Nieto; +Cc: git, Junio C Hamano
In-Reply-To: <1318434726-5556-1-git-send-email-cmn@elego.de>

On Wed, Oct 12, 2011 at 05:52:06PM +0200, Carlos Martín Nieto wrote:

> -There is also a case insensitive alternative `[section.subsection]` syntax.
> -In this syntax, subsection names follow the same restrictions as for section
> -names.
> +There is also a deprecated `[section.subsection]` syntax. With this
> +syntax, the subsection name is converted to lower-case and is also
> +compared case sensitively. These subsection names follow the same
> +restrictions as section names.

OK, now having looked thoroughly at the problem again, I agree that your
documentation update is much better than the one I posted earlier
(technically we could still talk about canonicalizing the subsection
earlier in git-config(1), but section.foo names are the deprecated
minority, so it's probably not worth cluttering the page with
explanations).

I do think the "compared case sensitively" bit is a little confusing
here, because it's not clear what is being compared here (as we're quite
deep into a discussion of the file format, and away from the git-config
usage). It might be more clear to say:

  With this syntax, the subsection name is converted to lower-case, and
  the result is compared case sensitively against the subsection name
  provided to git-config.

or something like that.

-Peff

^ permalink raw reply

* What's cooking in git.git (Oct 2011, #04; Wed, 12)
From: Junio C Hamano @ 2011-10-12 18:48 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking.  Commits prefixed with '-' are
only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'.

As reported earlier, 'next' has been rebuilt with some topics kicked back
to 'pu'.  Also, k.org is back.

The second wave of topics are starting to graduate to 'master'.

Here are the repositories that have my integration branches:

With maint, master, next, pu, todo, html and man:

	git://git.kernel.org/pub/scm/git/git.git
	git://repo.or.cz/alt-git.git
	https://code.google.com/p/git-core/
	https://github.com/git/git

With only maint, master, html and man:

	git://git.sourceforge.jp/gitroot/git-core/git.git
	git://git-core.git.sourceforge.net/gitroot/git-core/git-core

With all the topics and integration branches but not todo, html or man:

	https://github.com/gitster/git

--------------------------------------------------
[New Topics]

* cn/fetch-prune (2011-10-07) 4 commits
 - fetch: treat --tags like refs/tags/*:refs/tags/* when pruning
 - fetch: honor the user-provided refspecs when pruning refs
 - t5510: add tests for fetch --prune
 - fetch: free all the additional refspecs

* ef/mingw-syslog (2011-10-07) 1 commit
  (merged to 'next' on 2011-10-11 at d5d6945)
 + mingw: avoid using strbuf in syslog

* jk/name-hash-dirent (2011-10-07) 1 commit
  (merged to 'next' on 2011-10-11 at e2ea68b)
 + fix phantom untracked files when core.ignorecase is set

* jn/gitweb-manpages (2011-10-10) 6 commits
 . gitweb: Add gitweb manpages to 'gitweb' package in git.spec
 . Documentation: Add gitweb config variables to git-config(1)
 . Documentation: Link to gitweb(1) and gitweb.conf(5) in other manpages
 . gitweb: Add manpage for gitweb
 . gitweb: Add manpage for gitweb configuration files
 . Documentation: Preparation for gitweb manpages

A re-roll already being discussed.

* js/merge-edit-option (2011-10-10) 2 commits
 - fixup! 0f6adfe2a26287c5c13366975f621c818bb3214d
 - Teach merge the '[-e|--edit]' option

The part to tweak builtin/merge.c in the fix-up should be removed but
otherwise ready to be squashed into one and merged to 'next'.

* mm/maint-config-explicit-bool-display (2011-10-10) 1 commit
  (merged to 'next' on 2011-10-11 at 795939f)
 + config: display key_delim for config --bool --get-regexp

* rr/revert-cherry-pick (2011-10-10) 7 commits
 - fixup! 82d520bd
 - revert: Simplify passing command-line arguments around
 - revert: Allow mixed pick and revert instructions
 - revert: Make commit descriptions in insn sheet optional
 - revert: Fix buffer overflow in insn sheet parser
 - revert: Simplify getting commit subject
 - revert: Free memory after get_message call

Probably needs a little bit more polish.

* rs/diff-whole-function (2011-10-10) 2 commits
  (merged to 'next' on 2011-10-11 at 6196752)
 + diff: add option to show whole functions as context
 + xdiff: factor out get_func_line()

* rs/pickaxe (2011-10-07) 7 commits
  (merged to 'next' on 2011-10-11 at 27d02b2)
 + pickaxe: factor out pickaxe
 + pickaxe: give diff_grep the same signature as has_changes
 + pickaxe: pass diff_options to contains and has_changes
 + pickaxe: factor out has_changes
 + pickaxe: plug regex/kws leak
 + pickaxe: plug regex leak
 + pickaxe: plug diff filespec leak with empty needle

* sc/difftool-skip (2011-10-10) 1 commit
  (merged to 'next' on 2011-10-11 at 38d7e84)
 + git-difftool: allow skipping file by typing 'n' at prompt

* sg/completion (2011-10-10) 2 commits
  (merged to 'next' on 2011-10-11 at 4724640)
 + completion: unite --format and --pretty for 'log' and 'show'
 + completion: unite --reuse-message and --reedit-message for 'notes'
 (this branch uses tm/completion-commit-fixup-squash.)

* tc/fetch-leak (2011-10-07) 1 commit
  (merged to 'next' on 2011-10-11 at d867153)
 + fetch: plug two leaks on error exit in store_updated_refs

* jc/check-ref-format-fixup (2011-10-12) 2 commits
 - Restrict ref-like names immediately below $GIT_DIR
 - refs.c: move dwim_ref()/dwim_log() from sha1_name.c

An attempt to fix-up unfortunate side effect of mh/check-ref-format-3
topic. "git show -s config" is never meant to refer to $GIT_DIR/config
and treat it as a file that records an object name.

* jc/maint-remove-renamed-ref (2011-10-12) 1 commit
 - branch -m/-M: remove undocumented RENAMED-REF

--------------------------------------------------
[Graduated to "master"]

* cb/common-prefix-unification (2011-09-12) 3 commits
  (merged to 'next' on 2011-10-06 at 8349bca)
 + rename pathspec_prefix() to common_prefix() and move to dir.[ch]
 + consolidate pathspec_prefix and common_prefix
 + remove prefix argument from pathspec_prefix

Originally merged to 'next' on 2011-09-14.

* dm/tree-walk (2011-09-28) 2 commits
  (merged to 'next' on 2011-10-06 at 76e90c3)
 + tree-walk: micro-optimization in tree_entry_interesting
 + tree-walk: drop unused parameter from match_dir_prefix

Originally merged to 'next' on 2011-10-05.

* fg/submodule-git-file-git-dir (2011-08-22) 2 commits
  (merged to 'next' on 2011-10-06 at 3526bb9)
 + Move git-dir for submodules
 + rev-parse: add option --resolve-git-dir <path>

Originally merged to 'next' on 2011-08-23.

* hv/submodule-update-none (2011-08-11) 2 commits
  (merged to 'next' on 2011-10-06 at 4c105df)
 + add update 'none' flag to disable update of submodule by default
 + submodule: move update configuration variable further up

Originally merged to 'next' on 2011-08-24.

* jc/maint-diffstat-numstat-context (2011-09-22) 1 commit
  (merged to 'next' on 2011-10-06 at 36c972d)
 + diff: teach --stat/--numstat to honor -U$num

"diff" is allowed to match the common lines differently depending on how
many context lines it is showing, so running --(num)stat with 0 lines of
context internally gives a result that may be surprising to some people.

Originally merged to 'next' on 2011-09-26.

* jm/mergetool-pathspec (2011-09-26) 2 commits
  (merged to 'next' on 2011-10-06 at b8e830f)
 + mergetool: no longer need to save standard input
 + mergetool: Use args as pathspec to unmerged files

Originally merged to 'next' on 2011-09-26.

* jn/maint-http-error-message (2011-09-06) 2 commits
  (merged to 'next' on 2011-10-06 at 668a706)
 + http: avoid empty error messages for some curl errors
 + http: remove extra newline in error message

Originally merged to 'next' on 2011-09-12.

* jp/get-ref-dir-unsorted (2011-09-30) 2 commits
  (merged to 'next' on 2011-10-06 at 69fe65d)
 + refs: Use binary search to lookup refs faster
 + Don't sort ref_list too early

* mh/check-ref-format-3 (2011-10-05) 23 commits
  (merged to 'next' on 2011-10-06 at c277498)
 + add_ref(): verify that the refname is formatted correctly
 + resolve_ref(): expand documentation
 + resolve_ref(): also treat a too-long SHA1 as invalid
 + resolve_ref(): emit warnings for improperly-formatted references
 + resolve_ref(): verify that the input refname has the right format
 + remote: avoid passing NULL to read_ref()
 + remote: use xstrdup() instead of strdup()
 + resolve_ref(): do not follow incorrectly-formatted symbolic refs
 + resolve_ref(): extract a function get_packed_ref()
 + resolve_ref(): turn buffer into a proper string as soon as possible
 + resolve_ref(): only follow a symlink that contains a valid, normalized refname
 + resolve_ref(): use prefixcmp()
 + resolve_ref(): explicitly fail if a symlink is not readable
 + Change check_refname_format() to reject unnormalized refnames
 + Inline function refname_format_print()
 + Make collapse_slashes() allocate memory for its result
 + Do not allow ".lock" at the end of any refname component
 + Refactor check_refname_format()
 + Change check_ref_format() to take a flags argument
 + Change bad_ref_char() to return a boolean value
 + git check-ref-format: add options --allow-onelevel and --refspec-pattern
 + t1402: add some more tests
 + get_sha1_hex(): do not read past a NUL character

An unpleasant minor side effect was discovered with this one. If you try
"git show -s config", the code warns that $GIT_DIR/config is not formatted
correctly as a ref, which is technically correct but totally unwarranted.

* mh/iterate-refs (2011-09-11) 7 commits
  (merged to 'next' on 2011-10-06 at c7a33e5)
 + refs.c: make create_cached_refs() static
 + Retain caches of submodule refs
 + Store the submodule name in struct cached_refs
 + Allocate cached_refs objects dynamically
 + Change the signature of read_packed_refs()
 + Access reference caches only through new function get_cached_refs()
 + Extract a function clear_cached_refs()

Originally merged to 'next' on 2011-09-27.

* mh/maint-notes-merge-pathbuf-fix (2011-09-27) 1 commit
  (merged to 'next' on 2011-10-06 at 0af69bb)
 + notes_merge_commit(): do not pass temporary buffer to other function

Originally merged to 'next' on 2011-10-05.

* mz/remote-rename (2011-09-11) 4 commits
  (merged to 'next' on 2011-10-06 at 96db20d)
 + remote: only update remote-tracking branch if updating refspec
 + remote rename: warn when refspec was not updated
 + remote: "rename o foo" should not rename ref "origin/bar"
 + remote: write correct fetch spec when renaming remote 'remote'

Originally merged to 'next' on 2011-09-26.

* nd/sparse-doc (2011-09-26) 1 commit
  (merged to 'next' on 2011-10-06 at f6b8355)
 + git-read-tree.txt: update sparse checkout examples

Originally merged to 'next' on 2011-10-05.

* ps/gitweb-js-with-lineno (2011-09-27) 1 commit
  (merged to 'next' on 2011-10-06 at 9236f5e)
 + gitweb: Fix links to lines in blobs when javascript-actions are enabled

Originally merged to 'next' on 2011-10-05.

--------------------------------------------------
[Stalled]

* jk/http-auth-keyring (2011-09-28) 22 commits
 - credential-cache: don't cache items without context
 - check_expirations: don't copy over same element
 - t0300: add missing EOF terminator for <<
 - credential-store: use a better storage format
 - t0300: make alternate username tests more robust
 - t0300: make askpass tests a little more robust
 - credential-cache: fix expiration calculation corner cases
 - docs: minor tweaks to credentials API
 - credentials: make credential_fill_gently() static
 - credentials: add "getpass" helper
 - credentials: add "store" helper
 - credentials: add "cache" helper
 - docs: end-user documentation for the credential subsystem
 - http: use hostname in credential description
 - allow the user to configure credential helpers
 - look for credentials in config before prompting
 - http: use credential API to get passwords
 - introduce credentials API
 - http: retry authentication failures for all http requests
 - remote-curl: don't retry auth failures with dumb protocol
 - improve httpd auth tests
 - url: decode buffers that are not NUL-terminated
 (this branch is used by js/cred-macos-x-keychain-2.)

Kicked back to 'pu' to allow design level discussions to continue.

* js/cred-macos-x-keychain-2 (2011-10-06) 1 commit
 - contrib: add a pair of credential helpers for Mac OS X's keychain
 (this branch uses jk/http-auth-keyring.)

Kicked back to 'pu' to allow design level discussions to continue.

* hv/submodule-merge-search (2011-08-26) 5 commits
 - submodule: Search for merges only at end of recursive merge
 - allow multiple calls to submodule merge search for the same path
 - submodule: Demonstrate known breakage during recursive merge
 - push: Don't push a repository with unpushed submodules
 - push: teach --recurse-submodules the on-demand option
 (this branch is tangled with fg/submodule-auto-push.)

The second from the bottom one needs to be replaced with a properly
written commit log message.

* fg/submodule-auto-push (2011-09-11) 2 commits
 - submodule.c: make two functions static
 - push: teach --recurse-submodules the on-demand option
 (this branch is tangled with hv/submodule-merge-search.)

What the topic aims to achieve may make sense, but the implementation
looked somewhat suboptimal.

* sr/transport-helper-fix-rfc (2011-07-19) 2 commits
 - t5800: point out that deleting branches does not work
 - t5800: document inability to push new branch with old content

Perhaps 281eee4 (revision: keep track of the end-user input from the
command line, 2011-08-25) would help.

--------------------------------------------------
[Cooking]

* jc/signed-commit (2011-10-05) 4 commits
 - commit: teach --gpg-sign option
 - Split GPG interface into its own helper library
 - rename "match_refs()" to "match_push_refs()"
 - send-pack: typofix error message

This is to replace the earlier "signed push" experiments.

* js/maint-merge-one-file-osx-expr (2011-10-06) 1 commit
  (merged to 'next' on 2011-10-07 at fbb28a2)
 + merge-one-file: fix "expr: non-numeric argument"

Will merge to 'master' in the third wave.

* tm/completion-commit-fixup-squash (2011-10-06) 2 commits
  (merged to 'next' on 2011-10-11 at 6bb192e)
 + completion: commit --fixup and --squash
 + completion: unite --reuse-message and --reedit-message handling
 (this branch is used by sg/completion.)

* tm/completion-push-set-upstream (2011-10-06) 1 commit
  (merged to 'next' on 2011-10-11 at 85544e5)
 + completion: push --set-upstream

* js/no-cherry-pick-head-after-punted (2011-10-06) 1 commit
  (merged to 'next' on 2011-10-10 at acb29ee)
 + Merge branch 'js/maint-no-cherry-pick-head-after-punted' into js/no-cherry-pick-head-after-punted
 (this branch uses js/maint-no-cherry-pick-head-after-punted.)

Will merge to 'master' in the third wave.

* js/maint-no-cherry-pick-head-after-punted (2011-10-06) 2 commits
 + cherry-pick: do not give irrelevant advice when cherry-pick punted
 + revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so
 (this branch is used by js/no-cherry-pick-head-after-punted.)

Will merge to 'maint' later.

* sp/smart-http-failure (2011-10-04) 1 commit
  (merged to 'next' on 2011-10-06 at 02f9982)
 + remote-curl: Fix warning after HTTP failure

Will merge to 'master' in the second wave.

* cb/do-not-pretend-to-hijack-long-help (2011-10-05) 1 commit
  (merged to 'next' on 2011-10-06 at 46851fe)
 + use -h for synopsis and --help for manpage consistently

Will merge to 'master' in the second wave.

* cp/git-web-browse-browsers (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at da42ad0)
 + git-web--browse: avoid the use of eval

Will merge to 'master' in the third wave.

* il/archive-err-signal (2011-10-05) 1 commit
  (merged to 'next' on 2011-10-06 at 7e3083f)
 + Support ERR in remote archive like in fetch/push

Will merge to 'master' in the third wave.

* nd/daemon-log-sock-errors (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at 5f3630f)
 + daemon: log errors if we could not use some sockets

Will merge to 'master' in the third wave.

* nd/document-err-packet (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at 0c5f5d0)
 + pack-protocol: document "ERR" line

Will merge to 'master' in the second wave.

* nd/git-daemon-error-msgs (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at 209126d)
 + daemon: return "access denied" if a service is not allowed

Will merge to 'master' in the third wave.

* jc/is-url-simplify (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at d6c6741)
 + url.c: simplify is_url()

Will merge to 'master' in the third wave.

* jn/ident-from-etc-mailname (2011-10-06) 2 commits
  (merged to 'next' on 2011-10-06 at a68770d)
 + ident: do not retrieve default ident when unnecessary
 + ident: check /etc/mailname if email is unknown

Will merge to 'master' in the third wave.

* jn/no-g-plus-s-on-bsd (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at 3d85674)
 + Makefile: do not set setgid bit on directories on GNU/kFreeBSD

Will merge to 'master' in the third wave.

* js/log-show-children (2011-10-04) 1 commit
  (merged to 'next' on 2011-10-06 at de8f6f2)
 + log --children

Will merge to 'master' in the third wave.

* rs/name-rev-usage (2011-10-03) 1 commit
  (merged to 'next' on 2011-10-06 at e51878e)
 + name-rev: split usage string

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the second wave.

* rs/test-ctype (2011-10-03) 2 commits
  (merged to 'next' on 2011-10-06 at b8c26d2)
 + test-ctype: add test for is_pathspec_magic
 + test-ctype: macrofy

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the second wave.

* rs/pending (2011-10-03) 8 commits
  (merged to 'next' on 2011-10-06 at 998462b)
 + commit: factor out clear_commit_marks_for_object_array
 + checkout: use leak_pending flag
 + bundle: use leak_pending flag
 + bisect: use leak_pending flag
 + revision: add leak_pending flag
 + checkout: use add_pending_{object,sha1} in orphan check
 + revision: factor out add_pending_sha1
 + checkout: check for "Previous HEAD" notice in t2020

Will merge to 'master' in the third wave.

* ph/transport-with-gitfile (2011-10-11) 5 commits
  (merged to 'next' on 2011-10-12 at 6d58417)
 + Fix is_gitfile() for files too small or larger than PATH_MAX to be a gitfile
  (merged to 'next' on 2011-10-06 at 891b8b6)
 + Add test showing git-fetch groks gitfiles
 + Teach transport about the gitfile mechanism
 + Learn to handle gitfiles in enter_repo
 + enter_repo: do not modify input

Will merge to 'master' in the third wave.

* jc/grep-untracked-exclude (2011-10-04) 1 commit
  (merged to 'next' on 2011-10-06 at b16cffe)
 + Merge branch 'jc/maint-grep-untracked-exclude' into jc/grep-untracked-exclude
 (this branch uses bw/grep-no-index-no-exclude and jc/maint-grep-untracked-exclude.)

Will merge to 'master' in the third wave.

* jc/maint-grep-untracked-exclude (2011-10-04) 1 commit
 + grep: teach --untracked and --exclude-standard options
 (this branch is used by jc/grep-untracked-exclude; uses bw/grep-no-index-no-exclude.)

Will merge to 'maint' later.

* cs/perl-config-path-send-email (2011-09-30) 2 commits
  (merged to 'next' on 2011-10-06 at 93c00f0)
 + use new Git::config_path() for aliasesfile
 + Add Git::config_path()

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the second wave.

* jc/checkout-from-tree-keep-local-changes (2011-09-30) 1 commit
  (merged to 'next' on 2011-10-06 at 64061aa)
 + checkout $tree $path: do not clobber local changes in $path not in $tree

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the fourth wave.

* jc/apply-blank-at-eof-fix (2011-09-26) 1 commit
  (merged to 'next' on 2011-10-06 at a9dfd8f)
 + apply --whitespace=error: correctly report new blank lines at end

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the third wave.

* jc/parse-options-boolean (2011-09-28) 5 commits
  (merged to 'next' on 2011-10-06 at dd4936c)
 + apply: use OPT_NOOP_NOARG
 + revert: use OPT_NOOP_NOARG
 + parseopt: add OPT_NOOP_NOARG
 + archive.c: use OPT_BOOL()
 + parse-options: deprecate OPT_BOOLEAN

Will merge to 'master' in the second wave.

* ph/push-to-delete-nothing (2011-09-30) 1 commit
  (merged to 'next' on 2011-10-06 at 33ac777)
 + receive-pack: don't pass non-existent refs to post-{receive,update} hooks

Will merge to 'master' in the fourth wave.

* zj/send-email-authen-sasl (2011-09-29) 1 commit
  (merged to 'next' on 2011-10-06 at 78b31cd)
 + send-email: auth plain/login fix

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the second wave.

* nd/maint-sparse-errors (2011-09-22) 2 commits
  (merged to 'next' on 2011-10-06 at e3cbb90)
 + Add explanation why we do not allow to sparse checkout to empty working tree
 + sparse checkout: show error messages when worktree shaping fails

Originally merged to 'next' on 2011-09-22.
Will merge to 'master' in the third wave.

* rs/diff-cleanup-records-fix (2011-10-03) 2 commits
  (merged to 'next' on 2011-10-06 at 91f035f)
 + diff: resurrect XDF_NEED_MINIMAL with --minimal
 + Revert removal of multi-match discard heuristic in 27af01

Will merge to 'master' in the third wave.

* di/fast-import-empty-tag-note-fix (2011-09-22) 2 commits
  (merged to 'next' on 2011-10-06 at 3a01ef1)
 + fast-import: don't allow to note on empty branch
 + fast-import: don't allow to tag empty branch

Originally merged to 'next' on 2011-10-05.
Will merge to 'master' in the fourth wave.

* bw/grep-no-index-no-exclude (2011-09-15) 2 commits
  (merged to 'next' on 2011-10-06 at 325270b)
 + grep --no-index: don't use git standard exclusions
 + grep: do not use --index in the short usage output
 (this branch is used by jc/grep-untracked-exclude and jc/maint-grep-untracked-exclude.)

Originally merged to 'next' on 2011-09-26.
Will merge to 'master' in the third wave.

* js/bisect-no-checkout (2011-09-21) 1 commit
  (merged to 'next' on 2011-10-06 at 0354e94)
 + bisect: fix exiting when checkout failed in bisect_start()

Originally merged to 'next' on 2011-09-21.
Will merge to 'master' in the third wave.

* jc/request-pull-show-head-4 (2011-10-09) 10 commits
  (merged to 'next' on 2011-10-10 at 092175e)
 + environment.c: Fix an sparse "symbol not declared" warning
 + builtin/log.c: Fix an "Using plain integer as NULL pointer" warning
  (merged to 'next' on 2011-10-07 at fcaeca0)
 + fmt-merge-msg: use branch.$name.description
  (merged to 'next' on 2011-10-06 at fa5e0fe)
 + request-pull: use the branch description
 + request-pull: state what commit to expect
 + request-pull: modernize style
 + branch: teach --edit-description option
 + format-patch: use branch description in cover letter
 + branch: add read_branch_desc() helper function
 + Merge branch 'bk/ancestry-path' into jc/branch-desc

Will merge to 'master' in the fourth wave.

* nd/maint-autofix-tag-in-head (2011-09-18) 4 commits
  (merged to 'next' on 2011-10-06 at c083e69)
 + Accept tags in HEAD or MERGE_HEAD
 + merge: remove global variable head[]
 + merge: use return value of resolve_ref() to determine if HEAD is invalid
 + merge: keep stash[] a local variable

Originally merged to 'next' on 2011-09-27.
Will merge to 'master' in the third wave.

* bc/attr-ignore-case (2011-10-11) 5 commits
  (merged to 'next' on 2011-10-11 at daa6b51)
 + attr.c: respect core.ignorecase when matching attribute patterns
 + attr: read core.attributesfile from git_default_core_config
 + builtin/mv.c: plug miniscule memory leak
 + cleanup: use internal memory allocation wrapper functions everywhere
 + attr.c: avoid inappropriate access to strbuf "buf" member

Re-rolled.

* jc/lookup-object-hash (2011-08-11) 6 commits
 - object hash: replace linear probing with 4-way cuckoo hashing
 - object hash: we know the table size is a power of two
 - object hash: next_size() helper for readability
 - pack-objects --count-only
 - object.c: remove duplicated code for object hashing
 - object.c: code movement for readability

I do not think there is anything fundamentally wrong with this series, but
the risk of breakage far outweighs observed performance gain in one
particular workload. Will keep it in 'next' at least for one cycle.

^ permalink raw reply

* [PATCH v3 1/7] invalidate_ref_cache(): rename function from invalidate_cached_refs()
From: Michael Haggerty @ 2011-10-12 18:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318445067-19279-1-git-send-email-mhagger@alum.mit.edu>

It is the cache that is being invalidated, not the references.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/refs.c b/refs.c
index 9911c97..120b8e4 100644
--- a/refs.c
+++ b/refs.c
@@ -202,7 +202,7 @@ static struct cached_refs *get_cached_refs(const char *submodule)
 	return refs;
 }
 
-static void invalidate_cached_refs(void)
+static void invalidate_ref_cache(void)
 {
 	struct cached_refs *refs = cached_refs;
 	while (refs) {
@@ -1228,7 +1228,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 	ret |= repack_without_ref(refname);
 
 	unlink_or_warn(git_path("logs/%s", lock->ref_name));
-	invalidate_cached_refs();
+	invalidate_ref_cache();
 	unlock_ref(lock);
 	return ret;
 }
@@ -1527,7 +1527,7 @@ int write_ref_sha1(struct ref_lock *lock,
 		unlock_ref(lock);
 		return -1;
 	}
-	invalidate_cached_refs();
+	invalidate_ref_cache();
 	if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
 	    (strcmp(lock->ref_name, lock->orig_ref_name) &&
 	     log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
-- 
1.7.7.rc2

^ permalink raw reply related

* [PATCH v3 3/7] invalidate_ref_cache(): expose this function in refs API
From: Michael Haggerty @ 2011-10-12 18:44 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Johan Herland, Julian Phillips, Michael Haggerty
In-Reply-To: <1318445067-19279-1-git-send-email-mhagger@alum.mit.edu>

Make invalidate_ref_cache() an official part of the refs API.  It is
currently a fact of life that code outside of refs.c mucks about with
references.  This change gives such code a way of informing the refs
module that it should no longer trust its cache.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 refs.c |    2 +-
 refs.h |    8 ++++++++
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/refs.c b/refs.c
index cc72609..b08d476 100644
--- a/refs.c
+++ b/refs.c
@@ -202,7 +202,7 @@ static struct cached_refs *get_cached_refs(const char *submodule)
 	return refs;
 }
 
-static void invalidate_ref_cache(const char *submodule)
+void invalidate_ref_cache(const char *submodule)
 {
 	clear_cached_refs(get_cached_refs(submodule));
 }
diff --git a/refs.h b/refs.h
index 0229c57..f439c54 100644
--- a/refs.h
+++ b/refs.h
@@ -80,6 +80,14 @@ extern void unlock_ref(struct ref_lock *lock);
 /** Writes sha1 into the ref specified by the lock. **/
 extern int write_ref_sha1(struct ref_lock *lock, const unsigned char *sha1, const char *msg);
 
+/*
+ * Invalidate the reference cache for the specified submodule.  Use
+ * submodule=NULL to invalidate the cache for the main module.  This
+ * function must be called if references are changed via a mechanism
+ * other than the refs API.
+ */
+extern void invalidate_ref_cache(const char *submodule);
+
 /** Setup reflog before using. **/
 int log_ref_setup(const char *ref_name, char *logfile, int bufsize);
 
-- 
1.7.7.rc2

^ permalink raw reply related


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