Git development
 help / color / mirror / Atom feed
* Re: [PATCH 2/5] branch: introduce --list argument
From: Junio C Hamano @ 2011-08-25 18:34 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Junio C Hamano, Jeff King, Michael Schubert
In-Reply-To: <32d0954697da9ac96cc11af0d8cc1390d93fd037.1314259226.git.git@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Currently, there is no way to invoke the list mode with a pattern
> because this is interpreted as branch creation.
>
> Introduce a --list argument which invokes the list mode.

Probably this is --list "option", but I think this is a good idea to allow
"we are in list mode" without having to invoke "verbose" output.

^ permalink raw reply

* Re: Re* git clean --exclude broken?
From: Michael Schubert @ 2011-08-25 18:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Todd Rinaldo
In-Reply-To: <7vpqjtl4yi.fsf_-_@alter.siamese.dyndns.org>

On 08/25/2011 08:29 PM, Junio C Hamano wrote:
> diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
> index 974e04e..a7a18e3 100644
> --- a/Documentation/git-clean.txt
> +++ b/Documentation/git-clean.txt
> @@ -47,9 +47,9 @@ OPTIONS
>  
>  -e <pattern>::
>  --exclude=<pattern>::
> -	Specify special exceptions to not be cleaned.  Each <pattern> is
> -	the same form as in $GIT_DIR/info/excludes and this option can be
> -	given multiple times.
> +	In addition to what are found in .gitignore (per directory) and
> +	$GIT_DIR/info/exclude, also consider these patterns to be in the
> +	set of the ignore rules in effect.

Nitpick: Shouldn't this be "In addition to what is found in .." or
"In addition to those found in .."?

^ permalink raw reply

* Re: [PATCH] git-notes.txt: clarify -C vs. copy and -F
From: Johan Herland @ 2011-08-25 18:50 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Junio C Hamano
In-Reply-To: <0b124a705cf63d7c531a3a097a158dbaeaf6d298.1314267281.git.git@drmicha.warpmail.net>

On Thursday 25 August 2011 12:26:37 Michael J Gruber wrote:
> The current description of '-C' together with the analogy to 'git commit
> -C' can lead to the wrong conclusion that '-C' copies notes between
> objects. Make this clearer by rewording and pointing to 'copy'.
> 
> The example for attaching binary notes with 'git hash-object' followed
> by 'git notes add -C' immediately raises the question: "Why not use 'git
> notes add -F'?". Answer it (the latter is not binary-safe).
> 
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> This one has been lying around and fell under the rugs of the discussion
> for a ui redesign which never happened. So I think it's still worth it.

ACK

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: [PATCH 4/5] branch: restructure -v vs. -vv
From: Junio C Hamano @ 2011-08-25 19:02 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Jeff King, Michael Schubert
In-Reply-To: <f471a616e4f08e729384138d8cb8afff4e9dbcbf.1314259226.git.git@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> ahead/behind info is expensive, upstream name info cheap. Therefore,
> make -v output the upstream branch name and -vv add the ahead/behind
> info.

While I do not care too much about the end result, and I even suspect that
swapping the meaning might be better at the semantic level, I do not agree
with the above line of reasoning. It is not based on what the end user
wants at all, but is based on how easily you can produce results.

If a relatively expensive information is what is more often wanted and the
users are willing to pay the price to compute it, it is very reasonable to
use short-and-sweet -v for showing only that information, and have them
type -vv to ask for a less common information.

I also suspect that "ahead/behind" and "who-is-upstream" may be orthogonal
and we may not tie these to degree of verbosity. Jeff's suggestion to
teach these to for-each-ref may be along a similar thought.

You however may be able to argue "When the user wants A/B, s/he always
wants who-is-upstream, and asking for A/B and not wanting who-is-upstream
is insane". If "A/B" does not make sense without seeing "who-is-upstream",
but if seeing "who-is-upstream" alone makes sense without seeing "A/B",
then you can argue that these two are indeed degrees of verbosity, and it
may be justifiable to make -v give "who-is-upstream", and make -vv give
"A/B and who-is-upstream".

But I suspect that much more people have more than one topics cooking on
top of upstream (i.e. need for A/B) than people who work with these topics
based on different upstreams (i.e. need for "who-is-upstream"). People who
base their topics on only one upstream do not need "who-is-upstream" to
understand A/B.

I am wondering if "branch -v=<comma separated tokens>", where currently
available tokens are "sha1", "upstream", "ahead/behind", is the right way
to keep these orthogonal and extensible (of course you would want a
shorter mnemonic). Then existing -v and -vv can be understood as aliases
to:

	-v   ===  -v=sha1,ahead/behind
        -vv  ===  -v=sha1,upstream,ahead/behind

and you can naturally extend the vocabulary with "patchsame" to define
your -vvv introduced in [5/5].

^ permalink raw reply

* Re: [PATCH] replace: List replacement along with the object
From: Junio C Hamano @ 2011-08-25 19:07 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Christian Couder
In-Reply-To: <bae5a8f7d30417864d972390f9f6b4470cf4e5bf.1314283118.git.git@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> The documentation could be misunderstood as if "git replace -l" lists
> the replacements of the specified objects. Currently, it lists the
> replaced objects.

Seeing that you had to change existing tests, I do not think this is an
improvement. The existing scripts can read the list of objects and find
replacement themselves (if they want to find that out, that is), no?

^ permalink raw reply

* git diff annoyance / feature request
From: Boaz Harrosh @ 2011-08-25 19:14 UTC (permalink / raw)
  To: git discussion list


git diff has this very annoying miss-fixture where it will state
as hunk header the closest label instead of the function name.

So I get:
@@ -675,9 +670,23 @@ try_again:
 	}
 
 	if (flag) {
-		foo();
+		bazz();
 	}
 
 
Instead of what I'd like:
@@ -563,12 +563,7 @@ static int write_exec(struct page_collect *pcol)
 	}
 
 	if (flag) {
-		foo();
+		bazz();
 	}
 

I mean. The label "try_again" is not at all unique in my file. As a
reader I would like to see where is that code going to. The function
name is a unique file identifier that tells me exactly where the change
is going. The label is not. (It's not freaking BASIC)

I bet all this was just inherited from diff. Would it be accepted if
I send a patch to fix it? What you guys think a goto label makes any
sense at all?

Thanks
Boaz

^ permalink raw reply

* [PATCH] check-ref-format --print: Normalize refnames that start with slashes
From: Michael Haggerty @ 2011-08-25 19:19 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, cmn, Michael Haggerty
In-Reply-To: <7v39gpmk3x.fsf@alter.siamese.dyndns.org>

And add tests that such refnames are accepted and normalized
correctly.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---

I didn't convince Junio that it is preferable to reject refnames that
start with slashes, so go the other way instead.

This patch should be applied to maint, as it fixes a bug.  The
normalization should probably be moved to refs.[ch] as mentioned on
the mailing list, but that can be done separately, on master.

 builtin/check-ref-format.c  |    6 ++++--
 t/t1402-check-ref-format.sh |    7 +++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index ae3f281..7118021 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -12,8 +12,8 @@ static const char builtin_check_ref_format_usage[] =
 "   or: git check-ref-format --branch <branchname-shorthand>";
 
 /*
- * Replace each run of adjacent slashes in src with a single slash,
- * and write the result to dst.
+ * Remove leading slashes and replace each run of adjacent slashes in
+ * src with a single slash, and write the result to dst.
  *
  * This function is similar to normalize_path_copy(), but stripped down
  * to meet check_ref_format's simpler needs.
@@ -23,6 +23,8 @@ static void collapse_slashes(char *dst, const char *src)
 	char ch;
 	char prev = '\0';
 
+	while (*src == '/')
+		src++;
 	while ((ch = *src++) != '\0') {
 		if (prev == '/' && ch == prev)
 			continue;
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 1b0f82f..e6fafb2 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -18,6 +18,9 @@ invalid_ref 'foo'
 valid_ref 'foo/bar/baz'
 valid_ref 'refs///heads/foo'
 invalid_ref 'heads/foo/'
+valid_ref '/heads/foo'
+valid_ref '///heads/foo'
+invalid_ref '/foo'
 invalid_ref './foo'
 invalid_ref '.refs/foo'
 invalid_ref 'heads/foo..bar'
@@ -28,6 +31,7 @@ valid_ref 'heads/foo@bar'
 invalid_ref 'heads/v@{ation'
 invalid_ref 'heads/foo\bar'
 
+
 test_expect_success "check-ref-format --branch @{-1}" '
 	T=$(git write-tree) &&
 	sha1=$(echo A | git commit-tree $T) &&
@@ -70,7 +74,10 @@ invalid_ref_normalized() {
 
 valid_ref_normalized 'heads/foo' 'heads/foo'
 valid_ref_normalized 'refs///heads/foo' 'refs/heads/foo'
+valid_ref_normalized '/heads/foo' 'heads/foo'
+valid_ref_normalized '///heads/foo' 'heads/foo'
 invalid_ref_normalized 'foo'
+invalid_ref_normalized '/foo'
 invalid_ref_normalized 'heads/foo/../bar'
 invalid_ref_normalized 'heads/./foo'
 invalid_ref_normalized 'heads\foo'
-- 
1.7.6.8.gd2879

^ permalink raw reply related

* Re: [PATCH 1/5] branch: allow pattern arguments
From: Junio C Hamano @ 2011-08-25 19:29 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Jeff King, git, Michael Schubert
In-Reply-To: <7vliuhl4us.fsf@alter.siamese.dyndns.org>

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

> Jeff King <peff@peff.net> writes:
>
>> On Thu, Aug 25, 2011 at 10:30:17AM +0200, Michael J Gruber wrote:
>>
>>> -	else if (argc == 0)
>>> -		return print_ref_list(kinds, detached, verbose, abbrev, with_commit);
>>> +	else if (argc == 0 || (verbose && argc == 1))
>>> +		return print_ref_list(kinds, detached, verbose, abbrev, with_commit, argc ? argv[0] : NULL);
>>
>> Note that "git tag -l" takes multiple patterns these days (it used to
>> silently ignore everything after the first one!). "git branch" should
>> probably do the same.
>
> Agreed.

... and can be done without too much additional code.

An offtopic side note about naming:

I called it "const char **pattern" not "patterns" and that is
deliberate. I find it natural to be able to call the 2nd pattern, counting
from 0 as any CS person would, by spelling "pattern[2]". "patterns[2]"
sounds as if I am have multiple sets of patterns and refering to the
second set of patterns.

 builtin/branch.c |   25 +++++++++++++++++++------
 1 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 495bf2e..0fa62bd 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -260,10 +260,22 @@ static char *resolve_symref(const char *src, const char *prefix)
 
 struct append_ref_cb {
 	struct ref_list *ref_list;
-	const char *pattern;
+	const char **pattern;
 	int ret;
 };
 
+static int match_patterns(const char **pattern, const char *refname)
+{
+	if (!*pattern)
+		return 1; /* no pattern always matches */
+	while (*pattern) {
+		if (!fnmatch(*pattern, refname, 0))
+			return 1;
+		pattern++;
+	}
+	return 0;
+}
+
 static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
 {
 	struct append_ref_cb *cb = (struct append_ref_cb *)(cb_data);
@@ -298,7 +310,7 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,
 	if ((kind & ref_list->kinds) == 0)
 		return 0;
 
-	if (cb->pattern && fnmatch(cb->pattern, refname, 0))
+	if (!match_patterns(cb->pattern, refname))
 		return 0;
 
 	commit = NULL;
@@ -496,7 +508,7 @@ static void show_detached(struct ref_list *ref_list)
 	}
 }
 
-static int print_ref_list(int kinds, int detached, int verbose, int abbrev, struct commit_list *with_commit, const char *pattern)
+static int print_ref_list(int kinds, int detached, int verbose, int abbrev, struct commit_list *with_commit, const char **pattern)
 {
 	int i;
 	struct append_ref_cb cb;
@@ -528,7 +540,7 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
 	qsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp);
 
 	detached = (detached && (kinds & REF_LOCAL_BRANCH));
-	if (detached && (!pattern || !fnmatch(pattern, "HEAD", 0)))
+	if (detached && match_patterns(pattern, "HEAD"))
 		show_detached(&ref_list);
 
 	for (i = 0; i < ref_list.index; i++) {
@@ -703,8 +715,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 	if (delete)
 		return delete_branches(argc, argv, delete > 1, kinds);
-	else if (argc == 0 || (verbose && argc == 1))
-		return print_ref_list(kinds, detached, verbose, abbrev, with_commit, argc ? argv[0] : NULL);
+	else if (argc == 0 || (verbose && argc))
+		return print_ref_list(kinds, detached, verbose, abbrev,
+				      with_commit, argv);
 	else if (rename && (argc == 1))
 		rename_branch(head, argv[0], rename > 1);
 	else if (rename && (argc == 2))

^ permalink raw reply related

* Re: [PATCH 2/5] branch: introduce --list argument
From: Junio C Hamano @ 2011-08-25 19:52 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Jeff King, Michael Schubert
In-Reply-To: <32d0954697da9ac96cc11af0d8cc1390d93fd037.1314259226.git.git@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> @@ -695,12 +696,15 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>  
>  	argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
>  			     0);
> -	if (!!delete + !!rename + !!force_create > 1)
> +	if (!!delete + !!rename + !!force_create + !!list > 1)
>  		usage_with_options(builtin_branch_usage, options);
>  
> +	if (argc == 0 || (verbose && argc == 1))
> +		list = 1;
> +

I am afraid this is not quite right.

What happens if one of delete/rename/force-create can take a single
argument, or more importantly, if we someday gain another option that can
take zero or one argument and is incompatible with other operating mode?

For example, what happens to this command line with your "git branch":

	$ git branch -v -m boo

The first test passes (no explicit --list), and then you violate the "only
one of these operating mode can be set" assertion you made yourself by
flipping list on, no?

It is a good practice to always run the defaulting tweak *before* checking
options that are mutually incompatible, to catch your own mistake in the
tweaking code.

On top of my previous "multiple patterns allowed" tweak, a replacement
patch to this file may look like this.  I suspect there should be a better
way to represent the mode in a single variable and enforce that there is
only one chosen, but that is left for another day.

 builtin/branch.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 0fa62bd..4243e7c 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -629,7 +629,7 @@ static int opt_parse_merge_filter(const struct option *opt, const char *arg, int
 
 int cmd_branch(int argc, const char **argv, const char *prefix)
 {
-	int delete = 0, rename = 0, force_create = 0;
+	int delete = 0, rename = 0, force_create = 0, list = 0;
 	int verbose = 0, abbrev = DEFAULT_ABBREV, detached = 0;
 	int reflog = 0;
 	enum branch_track track;
@@ -668,6 +668,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		OPT_BIT('D', NULL, &delete, "delete branch (even if not merged)", 2),
 		OPT_BIT('m', NULL, &rename, "move/rename a branch and its reflog", 1),
 		OPT_BIT('M', NULL, &rename, "move/rename a branch, even if target exists", 2),
+		OPT_BOOLEAN(0, "list", &list, "list branch names"),
 		OPT_BOOLEAN('l', NULL, &reflog, "create the branch's reflog"),
 		OPT__FORCE(&force_create, "force creation (when already exists)"),
 		{
@@ -710,12 +711,17 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
 			     0);
-	if (!!delete + !!rename + !!force_create > 1)
+
+	if (!delete && !rename && !force_create &&
+	    (argc == 0 || (verbose && argc)))
+		list = 1;
+
+	if (!!delete + !!rename + !!force_create + !!list > 1)
 		usage_with_options(builtin_branch_usage, options);
 
 	if (delete)
 		return delete_branches(argc, argv, delete > 1, kinds);
-	else if (argc == 0 || (verbose && argc))
+	else if (list)
 		return print_ref_list(kinds, detached, verbose, abbrev,
 				      with_commit, argv);
 	else if (rename && (argc == 1))

^ permalink raw reply related

* Re: git diff annoyance / feature request
From: Jeff King @ 2011-08-25 20:00 UTC (permalink / raw)
  To: Boaz Harrosh; +Cc: git discussion list
In-Reply-To: <4E569F10.8060808@panasas.com>

On Thu, Aug 25, 2011 at 12:14:24PM -0700, Boaz Harrosh wrote:

> git diff has this very annoying miss-fixture where it will state
> as hunk header the closest label instead of the function name.
> [...]
>
> I bet all this was just inherited from diff. Would it be accepted if
> I send a patch to fix it? What you guys think a goto label makes any
> sense at all?

Unless you tell git what type of content is in your file, it uses the same
basic heuristics for finding a hunk-header line that diff does. Namely,
the most recent line that starts with an alphabetic character,
underscore, or dollar sign.

If you want language-specific hunk headers, you can use gitattributes to
tell git what's in your files. We already have a builtin C driver that
will do what you want. You just need to do[1]:

  echo '*.c diff=cpp' >.gitattributes

Note that it handles both C and C++, hence the name. See "git help
gitattributes" for details (the section "Defining a custom hunk-header"
is what you want).

If your upstream (which looks like linux-2.6) doesn't want
.gitattributes files in the repository, you can also put the entry into
.git/info/attributes.

-Peff

[1] Since we have builtin funcname patterns for many types, we arguably
could also have a builtin mapping of common extensions to diff drivers.

^ permalink raw reply

* Re: What's cooking in git.git (Aug 2011, #07; Wed, 24)
From: Jeff King @ 2011-08-25 20:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk4a2mjx6.fsf@alter.siamese.dyndns.org>

On Wed, Aug 24, 2011 at 05:09:09PM -0700, Junio C Hamano wrote:

> * jk/add-i-hunk-filter (2011-07-27) 5 commits
>   (merged to 'next' on 2011-08-11 at 8ff9a56)
>  + add--interactive: add option to autosplit hunks
>  + add--interactive: allow negatation of hunk filters
>  + add--interactive: allow hunk filtering on command line
>  + add--interactive: factor out regex error handling
>  + add--interactive: refactor patch mode argument processing
> 
> Needs documentation updates.

I think Duy already mentioned this, but you may want to update your
"what's cooking" note: it needs not just doc updates, but code to
actually pass the options along from real git commands that use
add--interactive, like add, checkout, reset, and stash.

> * jk/generation-numbers (2011-07-14) 7 commits
>  - limit "contains" traversals based on commit generation
>  - check commit generation cache validity against grafts
>  - pretty: support %G to show the generation number of a commit
>  - commit: add commit_generation function
>  - add metadata-cache infrastructure
>  - decorate: allow storing values instead of pointers
>  - Merge branch 'jk/tag-contains-ab' (early part) into HEAD
> 
> The initial "tag --contains" de-pessimization without need for generation
> numbers is already in; backburnered.

So...what next? I don't really like leaving the contains traversal
as-is. It's often much faster than the original version (because we do
at most one to-the-roots traversal), but in some cases is slower
(because the depth-first search doesn't cut itself off sanely, so an
unlucky traversal means going to the roots instead of ending early).

Here are the options:

  1. Stick with timestamps as a proxy for generation numbers.

     a. Assume they're good enough. Possibly provide options to turn off
        this optimization (here, and possibly in name-rev) if you want
        to be more correct in the face of skew (at the cost of
        efficiency).

     b. Implement some kind of "this commit timestamp is bogus" cache.

     c. Make it easier to use and share replace objects with correct
        commit timestamps.

  2. Add generation numbers to headers, and try to calculate them on the
     fly as much as possible (i.e., Linus's approach).

  3. Add separate storage of generation numbers.

     a. As an on-the-fly cache (i.e., my patches).

     b. Combined with packfiles somehow (more efficient, less flaky, but
        may require a bump in index format version; alternatively, you
        could have pack-*.generation, which is a sparsely populated list
        of 32-bit ints, in the exact same order as the pack-*.idx file.
        No backwards compatibility issues, but slightly less efficient).

Thoughts?

> * jk/http-auth-keyring (2011-08-03) 13 commits
>   (merged to 'next' on 2011-08-03 at b06e80e)
>  + 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
> 
> Looked mostly reasonable except for the limitation that it is not clear
> how to deal with a site at which a user needs to use different passwords 
> for different repositories. Will keep it in "next" at least for one cycle,
> until we start hearing real-world success reports on the list.
> 
> Not urgent; will not be in 1.7.7.

I'm OK with holding this off for another round. I'd really like to get
more feedback from third-party helper writers. OTOH, I think we have a
little bit of a chicken-and-egg problem with testing. Most experienced
git users aren't that interested in this, because they are just using
ssh keys and ssh-agent, anyway. I wonder if we would do better to put it
in a released version with a big "warning: this is ridiculously
experimental and the interface to helpers is subject to change" warning.
And that puts something in the hands of the people it was meant to help,
and can get the attention of people to write helpers. I dunno.

-Peff

^ permalink raw reply

* Re: [PATCH 10/14] http: use hostname in credential description
From: Jeff King @ 2011-08-25 20:23 UTC (permalink / raw)
  To: git
In-Reply-To: <87d3g1k3u6.fsf@lifelogs.com>

On Fri, Aug 19, 2011 at 07:01:21AM -0500, Ted Zlatanov wrote:

> I see some info in "What's Cooking" about this patch but it's unclear to
> me whether the hostname issue (where it's hard to have multiple
> identities on a single server, which I think is all right) is blocking
> the inclusion of the patch into the next release, or if it will be
> included eventually if no one complains about that issue, or something
> else...

Junio and I discussed it a bit in another thread. I think the ability to
use "user@hostname" to disambiguate means the problem is dealt with at a
high level. And the "cache" helper handles that just fine. But the
"store" helper will conflate two entries for the same host. I'll see if
I can work on a patch for that.

It looks like Junio is planning to hold the series off until 1.7.8. Have
you been working on a Secrets API helper? If so, I'd love to get
feedback on how well the interface is serving your needs.

-Peff

^ permalink raw reply

* Re: [PATCH 0/10] color and pager improvements
From: Jeff King @ 2011-08-25 20:25 UTC (permalink / raw)
  To: Ingo Brückl; +Cc: git
In-Reply-To: <4e4e03fb.6d8e455c.bm000@wupperonline.de>

On Fri, Aug 19, 2011 at 08:34:13AM +0200, Ingo Brückl wrote:

> Jeff King wrote on Thu, 18 Aug 2011 15:46:44 -0700:
> 
> > On Fri, Aug 19, 2011 at 12:33:01AM +0200, Ingo Brückl wrote:
> 
> >> My goal was to be able to turn off paging for "stash list" only while all
> >> other stash commands should continue paging.
> 
> > Ah, OK. I think the only other stash command that pages is "stash show",
> > but I don't think it's unreasonable to want paging for that but not for
> > "list".
> 
> Maybe "stash list" simply should - like other commands - not paginate by
> default.

I have no real opinion on that. It only paginates as a side effect of
calling log.

I do think "git stash show" paginating by default is probably helpful,
though.

The best way to get people's attention is probably to post a patch
adding --no-pager to the git-log invocation of "git stash list". :)

-Peff

^ permalink raw reply

* Re: git diff annoyance / feature request
From: Junio C Hamano @ 2011-08-25 20:27 UTC (permalink / raw)
  To: Boaz Harrosh; +Cc: git discussion list
In-Reply-To: <4E569F10.8060808@panasas.com>

Boaz Harrosh <bharrosh@panasas.com> writes:

> I mean. The label "try_again" is not at all unique in my file. As a
> reader I would like to see where is that code going to. The function
> name is a unique file identifier that tells me exactly where the change
> is going. The label is not. (It's not freaking BASIC)
>
> I bet all this was just inherited from diff. Would it be accepted if
> I send a patch to fix it? What you guys think a goto label makes any
> sense at all?

The default tries to mimic what GNU used to do when we added the feature.

The diff.*.xfuncname configuration variable is there exactly for people
like you to tweak what we use for hunk headers. Please experiment with it
and if you come up with a better set of patterns, people may want to copy
it and use it themselves. we may even consider updating the built-in
default with your patterns, once they got adopted by wider audiences.

Personally, I would have to say that the source wouldn't be using too many
labels with the same name for this behaviour to be problematic, especially
if it is not freaking BASIC ;-), so...

^ permalink raw reply

* Re: Re* git clean --exclude broken?
From: Junio C Hamano @ 2011-08-25 20:28 UTC (permalink / raw)
  To: Michael Schubert; +Cc: git, Todd Rinaldo
In-Reply-To: <4E5696AE.4010201@elegosoft.com>

Michael Schubert <mschub@elegosoft.com> writes:

> Nitpick: Shouldn't this be "In addition to what is found in .." or
> "In addition to those found in .."?

Thanks.

^ permalink raw reply

* [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions
From: Jeff King @ 2011-08-25 20:40 UTC (permalink / raw)
  To: Boaz Harrosh; +Cc: git
In-Reply-To: <20110825200001.GA6165@sigill.intra.peff.net>

We already provide sane hunk-header patterns for specific
languages. However, the user has to manually map common
extensions to use them. It's not that hard to do, but it's
an extra step that the user might not even know is an
option. Let's be nice and do it automatically.

Signed-off-by: Jeff King <peff@peff.net>
---
I tried to think of negative side effects.

The userdiff drivers we have are pretty conservative; they just specify
hunk headers. So if you have a binary file named "foo.c", we still do
the regular binary detection.

If you have any matching attribute line in your own files, it should
override. So:

  foo/* -diff

will still mark foo/bar.c as binary, even with this change.

Can anyone think of other possible side effects?

Also, any other extensions that would go into such a list? I have no
idea what the common extension is for something like pascal or csharp.

 attr.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/attr.c b/attr.c
index da29c8e..5118a14 100644
--- a/attr.c
+++ b/attr.c
@@ -294,6 +294,18 @@ static void free_attr_elem(struct attr_stack *e)
 
 static const char *builtin_attr[] = {
 	"[attr]binary -diff -text",
+	"*.html diff=html",
+	"*.java diff=java",
+	"*.perl diff=perl",
+	"*.pl diff=perl",
+	"*.php diff=php",
+	"*.py diff=python",
+	"*.rb diff=ruby",
+	"*.bib diff=bibtex",
+	"*.tex diff=tex",
+	"*.c diff=cpp",
+	"*.cc diff=cpp",
+	"*.cxx diff=cpp",
 	NULL,
 };
 
-- 
1.7.6.10.g62f04

^ permalink raw reply related

* Re: [PATCH] check-ref-format --print: Normalize refnames that start with slashes
From: Junio C Hamano @ 2011-08-25 20:42 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git, cmn
In-Reply-To: <1314299964-21866-1-git-send-email-mhagger@alum.mit.edu>

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

> And add tests that such refnames are accepted and normalized
> correctly.

Thanks. I think this is a bit simpler, though...

 builtin/check-ref-format.c  |    4 +---
 t/t1402-check-ref-format.sh |    1 -
 2 files changed, 1 insertions(+), 4 deletions(-)

diff --git b/builtin/check-ref-format.c a/builtin/check-ref-format.c
index 7118021..0723cf2 100644
--- b/builtin/check-ref-format.c
+++ a/builtin/check-ref-format.c
@@ -21,10 +21,8 @@ static const char builtin_check_ref_format_usage[] =
 static void collapse_slashes(char *dst, const char *src)
 {
 	char ch;
-	char prev = '\0';
+	char prev = '/';
 
-	while (*src == '/')
-		src++;
 	while ((ch = *src++) != '\0') {
 		if (prev == '/' && ch == prev)
 			continue;
diff --git b/t/t1402-check-ref-format.sh a/t/t1402-check-ref-format.sh
index e6fafb2..7563043 100755
--- b/t/t1402-check-ref-format.sh
+++ a/t/t1402-check-ref-format.sh
@@ -31,7 +31,6 @@ valid_ref 'heads/foo@bar'
 invalid_ref 'heads/v@{ation'
 invalid_ref 'heads/foo\bar'
 
-
 test_expect_success "check-ref-format --branch @{-1}" '
 	T=$(git write-tree) &&
 	sha1=$(echo A | git commit-tree $T) &&

^ permalink raw reply related

* Re: git-config: case insensitivity for subsections
From: Jeff King @ 2011-08-25 20:58 UTC (permalink / raw)
  To: milki; +Cc: git
In-Reply-To: <20110818063528.GH13342@hal.rescomp.berkeley.edu>

On Wed, Aug 17, 2011 at 11:35:28PM -0700, milki wrote:

> If I define [section.SUBSECTION] (aka, not all lowercase), I cannot
> use: git config section.SUBSECTION.option, but rather only git config
> section.subsection.option.

The way the config code works (both internally and via git-config), is
to read through the config files, convert each key into a canonical
format (downcasing the section and key, and either preserving the
case for the subsection in '[section "FOO"]' or downcasing it for
'[section.FOO]'), and then compare the result to the canonical version
of what you're looking for.

In other words, if you want to match section.SUBSECTION, you should
always ask for the canonical version "section.subsection.whatever".

We could try to be nicer and handle this automatically, but it's
nontrivial. When you say "git config foo.BAR.baz", we don't know if you
mean for "BAR" to be case-insensitive or not. So it would involve
carrying more information around about how the section header in the
config file was actually parsed. Not impossible, but it would involve
changing the internal git_config interface and tweaking a lot of code to
match.

Is there a reason that you can't use the canonical version in your "git
config" invocation? Or was it simply confusing that it didn't work? I'd
much prefer to document this limitation in git-config(1) than change the
code.

> Furthermore, If I also define a [section "SUBSECTION"], the two
> sections are not merged.

I'm not sure it makes sense to do so. I can see how:

  [section.SUBSECTION]

and

  [section.subsection]

should be merged. But isn't:

  [section "SUBSECTION"]

conceptually a different section entirely?

Again, do you have a real-world use for this?

-Peff

^ permalink raw reply

* Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions
From: Eric Sunshine @ 2011-08-25 21:00 UTC (permalink / raw)
  To: Jeff King; +Cc: Boaz Harrosh, git
In-Reply-To: <20110825204047.GA9948@sigill.intra.peff.net>

On Thu, Aug 25, 2011 at 4:40 PM, Jeff King <peff@peff.net> wrote:
> We already provide sane hunk-header patterns for specific
> languages. However, the user has to manually map common
> extensions to use them. It's not that hard to do, but it's
> an extra step that the user might not even know is an
> option. Let's be nice and do it automatically.
>
> Also, any other extensions that would go into such a list? I have no
> idea what the common extension is for something like pascal or csharp.

C# uses extension ".cs".

".cpp" is common, in fact often required, by Windows compilers.

What about ".h" and ".hpp"?

-- ES

^ permalink raw reply

* Re: [RFC/PATCH] attr: map builtin userdiff drivers to well-known extensions
From: Jeff King @ 2011-08-25 21:06 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Boaz Harrosh, git
In-Reply-To: <CAPig+cQ33PESWC5fzN8enLFRwNPx8o+PgRUTeCva4dSJ_EdwOw@mail.gmail.com>

On Thu, Aug 25, 2011 at 05:00:51PM -0400, Eric Sunshine wrote:

> > Also, any other extensions that would go into such a list? I have no
> > idea what the common extension is for something like pascal or csharp.
> 
> C# uses extension ".cs".
> 
> ".cpp" is common, in fact often required, by Windows compilers.

Thanks, added both to my list.

> What about ".h" and ".hpp"?

How well do our cpp patterns do with header files? I imagine they're
better than the default, but I don't think I've ever really tried
anything tricky.

-Peff

^ permalink raw reply

* Re: What's the difference between `git show branch:file | diff -u - file` vs `git diff branch file`?
From: Junio C Hamano @ 2011-08-25 21:10 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: Marat Radchenko, git
In-Reply-To: <4E54088F.7050700@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

>> The default diff backend tries to come up with minimal changes by spending
>> extra cycles, so it is not so surprising if the file compared is large-ish
>> and/or has very many similar lines in itself (in which case there are many
>> potential matching line pairs between the preimage and the postimage to be
>> examined to produce a minimal diff).
>
> But the file in this case is not that large, and "git diff" spends 30s!

If the difference was literally between

 $ git diff branch -- file
 $ git show branch:file | diff -u - file

that is, "file" is the name of a file in the top-level directory, I would
expect that former would open the top-level tree object for the branch,
read it thru until it finds "file", grabs a single blob and deflate it in
core, and compare that with the contents of a single file read from the
filesystem.

An interesting comparison may be to run this once:

   $ git show branch:file >fileI

and then compare between these two:

    $ diff -u fileI file
    $ git diff --no-index fileI file

If the latter is slower than the former in the same way as the original
experiment, that would mean that the tree traversal time does not have
anything to do with it (iow, your "The way 'git diff' is now, it does
that" is not just incorrect---we don't read the full tree to begin
with---but irrelevant).

If the "we try to come up with minimal changes by spending extra cycles" I
mentioned in my messages is indeed the cause, you may see diffferences
running the "git diff --no-index" version with backend options, e.g.

    $ git diff --no-index --patience fileI file
    $ git diff --no-index --histogram fileI file

Another thing to try may be to run the version from "next" that has
27af01d (xdiff/xprepare: improve O(n*m) performance in xdl_cleanup_records(),
2011-08-17), without any backend options.

^ permalink raw reply

* [PATCH] allow multiple calls to submodule merge search for the same path
From: Heiko Voigt @ 2011-08-25 21:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Brad King, git
In-Reply-To: <7v39gqo2fn.fsf@alter.siamese.dyndns.org>

When multiple merge-bases are found for two commits to be merged the
merge machinery will ask twice for a merge resolution. Currently its not
possible to use the revision-walking api for walking the same commits
multiple times. Thats why we now run the revision walking in a seperate
git process.

Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
Hi,

On Wed, Aug 24, 2011 at 03:43:56PM -0700, Junio C Hamano wrote:
> Heiko Voigt <hvoigt@hvoigt.net> writes:
> 
> >> I have been suspecting that most of this should be done in a separate
> >> helper program that is run via run_command() interface, without
> >> contaminating the object pool the main merge process has with data from
> >> the submodule object store to begin with (i.e. add_submodule_odb() and
> >> everything below should go). Wouldn't it be a lot cleaner solution?
> >
> > Hmm, I would like to keep it in process. Since there are platforms where
> > spawning new processes is very slow.
> 
> Adding submodule's odb into the main process _will_ also have performance
> penalties because it will make it more expensive to look up objects that
> belong to the superproject when the superproject wants its own look up.
> 
> In case you haven't realized yet, walking revision graph multiple times
> while making sure that you do not affect other revision traversals in
> effect is hard to arrange right. But more importantly, correctness counts
> more than performing quickly and giving a bogus result with premature
> optimization that makes it harder to implement things correctly (and
> harder to verify the change is correct).

Yes of course correctness has priority one. And so here is a patch
implementing the rev-iteration using a seperate process. Once the
revision walking api is able to walk multiple times we get back to it
again.

I noticed that the suggestion for the merge resolution presented is
wrong which Brad also seems to have found as presented in his patch in
message:

<438ea0b254ccafb3fc9f3431f8f86007cc03132b.1314290439.git.brad.king@kitware.com>

But this breakage depends on the wrong output from rev-list with the
--ancestry-path option. This patch should be correct on its own.

 submodule.c                |   31 ++++++++++++++++++++++---------
 t/t7405-submodule-merge.sh |    2 +-
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/submodule.c b/submodule.c
index 1ba9646..9820df7 100644
--- a/submodule.c
+++ b/submodule.c
@@ -581,29 +581,42 @@ static int find_first_merges(struct object_array *result, const char *path,
 	char merged_revision[42];
 	const char *rev_args[] = { "rev-list", "--merges", "--ancestry-path",
 				   "--all", merged_revision, NULL };
-	struct rev_info revs;
-	struct setup_revision_opt rev_opts;
+	struct child_process cp;
+	struct strbuf one_rev = STRBUF_INIT;
 
 	memset(&merges, 0, sizeof(merges));
 	memset(result, 0, sizeof(struct object_array));
-	memset(&rev_opts, 0, sizeof(rev_opts));
+	memset(&cp, 0, sizeof(cp));
 
 	/* get all revisions that merge commit a */
 	snprintf(merged_revision, sizeof(merged_revision), "^%s",
 			sha1_to_hex(a->object.sha1));
-	init_revisions(&revs, NULL);
-	rev_opts.submodule = path;
-	setup_revisions(sizeof(rev_args)/sizeof(char *)-1, rev_args, &revs, &rev_opts);
+
+	cp.argv = rev_args;
+	cp.env = local_repo_env;
+	cp.git_cmd = 1;
+	cp.no_stdin = 1;
+	cp.out = -1;
+	cp.dir = path;
+	if (start_command(&cp))
+		die("Could not run 'git rev-list --merges --ancestry-path --all %s' "
+				"command in submodule %s", merged_revision, path);
+	FILE *out = fdopen(cp.out, "r");
+	if (!out)
+		die("Could not open pipe of rev-list command.");
 
 	/* save all revisions from the above list that contain b */
-	if (prepare_revision_walk(&revs))
-		die("revision walk setup failed");
-	while ((commit = get_revision(&revs)) != NULL) {
+	while (strbuf_getline(&one_rev, out, '\n') != EOF) {
+		commit = lookup_commit_reference_by_name(one_rev.buf);
 		struct object *o = &(commit->object);
 		if (in_merge_bases(b, &commit, 1))
 			add_object_array(o, NULL, &merges);
 	}
 
+	fclose(out);
+	finish_command(&cp);
+	strbuf_release(&one_rev);
+
 	/* Now we've got all merges that contain a and b. Prune all
 	 * merges that contain another found merge and save them in
 	 * result.
diff --git a/t/t7405-submodule-merge.sh b/t/t7405-submodule-merge.sh
index 8f6f2d6..603fb72 100755
--- a/t/t7405-submodule-merge.sh
+++ b/t/t7405-submodule-merge.sh
@@ -269,7 +269,7 @@ test_expect_success 'setup for recursive merge with submodule' '
 '
 
 # merge should leave submodule unmerged in index
-test_expect_failure 'recursive merge with submodule' '
+test_expect_success 'recursive merge with submodule' '
 	(cd merge-recursive &&
 	 test_must_fail git merge top-bc &&
 	 echo "160000 $(git rev-parse top-cb:sub) 2	sub" > expect2 &&
-- 
1.7.6.551.g0f51b.dirty

^ permalink raw reply related

* PROTECTED PROJECT
From: Mr. Vincent Cheng @ 2011-08-25 18:52 UTC (permalink / raw)




-- 
I am Mr. Vincent Cheng, GBS, JP Chairman of the Hong Kong and Shanghai
Banking Corporation Limited.i have a business proposal of Twenty Two
million Five Hundred Thousand United State Dollars only for you to
transact with me from my bank to your country.Having gone through a
methodical search, I decided to contact you hoping that you will find
this proposal interesting. Please on your confirmation of this message
and indicating your interest

All confirmable documents to back up the claims will be made available
to you prior to your acceptance and as soon as I receive your return 
mail
Via my email address:vincent-cheng26@hotmail.com and I will letyou know 
what
is required of you,your earliest response to this letter will be 
appreciated.

Endeavor to let me know your decision rather than keep me waiting

Best Regards,
Mr. Vincent Chen

^ permalink raw reply

* Re: [PATCHv2] log: Do not decorate replacements with --no-replace-objects
From: Junio C Hamano @ 2011-08-25 21:22 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <739ff64bdbd0c099b3baf0755c0350de82c1ec0a.1314284204.git.git@drmicha.warpmail.net>

Thanks, both. Will apply.

^ permalink raw reply

* Re: git-config: case insensitivity for subsections
From: Junio C Hamano @ 2011-08-25 21:32 UTC (permalink / raw)
  To: Jeff King; +Cc: milki, git
In-Reply-To: <20110825205849.GA10384@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I'm not sure it makes sense to do so. I can see how:
>
>   [section.SUBSECTION]
>
> and
>
>   [section.subsection]
>
> should be merged. But isn't:
>
>   [section "SUBSECTION"]
>
> conceptually a different section entirely?

I still recall getting scolded by Linus after writing [sec.tion]; this was
way back when he was still active on this list. I essentially was told
that [sec "tion"] is _the_ only supported way, and [sec.tion] may work but
it purely does by accident, not by design.

Do we still even list the bogus [section.SUBSECTION] syntax anywhere in
our docs? If so, we should remove them and if not we simply just should
deprecate the code to read such input.

^ 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