Git development
 help / color / mirror / Atom feed
* Re: [PATCH] grep: detect number of CPUs for thread spawning
From: Eric Herman @ 2011-11-06 18:00 UTC (permalink / raw)
  To: Pete Wyckoff
  Cc: Ævar Arnfjörð Bjarmason, git, Junio C Hamano,
	Sverre Rabbelier, Fernando Vezzosi
In-Reply-To: <20111106145050.GA4219@arf.padd.com>

Hello Pete,

Thank you for the feedback.

On 11/06/2011 03:50 PM, Pete Wyckoff wrote:

>> From: Eric Herman<eric@freesa.org>
>>
>> Change the number of threads that we spawn from a hardcoded value of
>> "8" to what online_cpus() returns.


> I agree with the need to exploit>8 CPUs, but I lose a lot of
> performance when limiting the threads to the number of physical
> CPUs.

Ah, yes, Being focused on big machines, I did not actually test with low 
CPU machines, certainly not with NFS mounts.

>
> Tests without your patch on master, just changing "#define
> THREADS" from 8 to 2.  On a 2-core Intel Core2 Duo.
>
> Producing lots of output:
>
>      8 threads:
>
> 	$ time ~/u/src/git/bin-wrappers/git grep f>  /dev/null
> 	0m14.02s user 0m3.64s sys 0m11.93s elapsed 148.07 %CPU
> 	$ time ~/u/src/git/bin-wrappers/git grep f>  /dev/null
> 	0m13.86s user 0m3.70s sys 0m11.82s elapsed 148.57 %CPU
>
>      2 threads:
>
> 	$ time ~/u/src/git/bin-wrappers/git grep f>  /dev/null
> 	0m15.14s user 0m3.52s sys 0m24.22s elapsed 77.05 %CPU
> 	$ time ~/u/src/git/bin-wrappers/git grep f>  /dev/null
> 	0m14.85s user 0m3.79s sys 0m24.20s elapsed 77.05 %CPU
>
> Producing no output:
>
>      8 threads:
>
> 	$ time ~/u/src/git/bin-wrappers/git grep unfindable-string
> 	0m1.14s user 0m3.68s sys 0m5.17s elapsed 93.22 %CPU
> 	$ time ~/u/src/git/bin-wrappers/git grep unfindable-string
> 	0m1.28s user 0m3.56s sys 0m5.15s elapsed 94.22 %CPU
>
>      2 threads:
>
> 	$ time ~/u/src/git/bin-wrappers/git grep unfindable-string
> 	0m1.36s user 0m3.64s sys 0m16.82s elapsed 29.75 %CPU
> 	$ time ~/u/src/git/bin-wrappers/git grep unfindable-string
> 	0m1.38s user 0m3.66s sys 0m16.81s elapsed 30.04 %CPU
>
> My workdir is on NFS, where even though the repository is fully
> cached, the open()s must go to the server.  Using more threads
> than CPUs makes it more likely that some thread isn't blocked.

This is good data.
It gives me ideas for how I can do some more testing.

>
> You could add a #threads knob,

Sure, adding a knob is not a bad idea.


> but then we'd have to get
> everybody on NFS to set that properly.

Indeed, I think you agree that it would be better if there was no need 
for most people to fiddle with yet another knob.


>  Or take a look at
> preload_index() to see how it guesses at how many threads it
> needs.

Good tip.
A quick peek at preload_index suggests that it was a bit of guesswork:

/*
  * Mostly randomly chosen maximum thread counts: we
  * cap the parallelism to 20 threads, and we want
  * to have at least 500 lstat's per thread for it to
  * be worth starting a thread.
  */

However, your comments make me wonder if a rule-of-thumb like "3 + 
online_cpus()" would yield better results across both large and small 
numbers of cores with either blazing fast or very slow storage.

I will create a setup similar to the one you describe and do some 
exploration.

Cheers,
  -Eric

-- 
http://www.freesa.org/ -- mobile: +31 620719662
aim: ericigps -- skype: eric_herman -- jabber: eric.herman@gmail.com

^ permalink raw reply

* Re: A Python script to put CTAN into git (from DVDs)
From: Jakub Narebski @ 2011-11-06 16:42 UTC (permalink / raw)
  To: Jonathan Fine; +Cc: python-list, git
In-Reply-To: <4EB6A522.3020909@pytex.org>

Jonathan Fine <jfine@pytex.org> writes:

> Hi
> 
> This it to let you know that I'm writing (in Python) a script that
> places the content of CTAN into a git repository.
>      https://bitbucket.org/jfine/python-ctantools

I hope that you meant "repositories" (plural) here, one per tool,
rather than putting all of CTAN into single Git repository.
 
> I'm working from the TeX Collection DVDs that are published each year
> by the TeX user groups, which contain a snapshot of CTAN (about
> 100,000 files occupying 4Gb), which means I have to unzip folders and
> do a few other things.

There is 'contrib/fast-import/import-zips.py' in git.git repository.
If you are not using it, or its equivalent, it might be worth checking
out.
 
> CTAN is the Comprehensive TeX Archive Network.  CTAN keeps only the
> latest version of each file, but old CTAN snapshots will provide many
> earlier versions.

There was similar effort done in putting CPAN (Comprehensive _Perl_
Archive Network) in Git, hosting repositories on GitHub[1], by the name
of gitPAN, see e.g.:

  "The gitPAN Import is Complete"
  http://perlisalive.com/articles/36
 
[1]: https://github.com/gitpan

> I'm working on putting old CTAN files into modern version
> control. Martin Scharrer is working in the other direction.  He's
> putting new files added to CTAN into Mercurial.
>      http://ctanhg.scharrer-online.de/

Nb. thanks to tools such as git-hg and fast-import / fast-export
we have quite good interoperability and convertability between
Git and Mercurial.

P.S. I'd point to reposurgeon tool, which can be used to do fixups
after import, but it would probably won't work on such large (set of)
repositories.

P.P.S. Can you forward it to comp.text.tex?
-- 
Jakub Narębski

^ permalink raw reply

* Re: [PATCH] gc --auto: warn gc will soon run, give users a chance to run manually
From: Matthieu Moy @ 2011-11-06 16:37 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1320489212-524-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> +		warning(_("Too many loose objects. \"git gc\" will soon run automatically"));
[...]
> +		warning(_("Too many packs, \"git gc\" will soon run automatically."));

Nit: you have a period in the second message, but not for the first.

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

^ permalink raw reply

* A Python script to put CTAN into git (from DVDs)
From: Jonathan Fine @ 2011-11-06 15:17 UTC (permalink / raw)
  To: python-list; +Cc: git

Hi

This it to let you know that I'm writing (in Python) a script that 
places the content of CTAN into a git repository.
     https://bitbucket.org/jfine/python-ctantools

I'm working from the TeX Collection DVDs that are published each year by 
the TeX user groups, which contain a snapshot of CTAN (about 100,000 
files occupying 4Gb), which means I have to unzip folders and do a few 
other things.

CTAN is the Comprehensive TeX Archive Network.  CTAN keeps only the 
latest version of each file, but old CTAN snapshots will provide many 
earlier versions.

I'm working on putting old CTAN files into modern version control. 
Martin Scharrer is working in the other direction.  He's putting new 
files added to CTAN into Mercurial.
     http://ctanhg.scharrer-online.de/

My script works already as a proof of concept, but needs more work (and 
documentation) before it becomes useful.  I've requested that follow up 
goes to comp.text.tex.

Longer terms goals are git as
* http://en.wikipedia.org/wiki/Content-addressable_storage
* a resource editing and linking system

If you didn't know, a git tree is much like an immutable JSON object, 
except that it does not have arrays or numbers.

If my project interests you, reply to this message or contact me 
directly (or both).

-- 
Jonathan

^ permalink raw reply

* Re: [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
From: Andreas Schwab @ 2011-11-06 15:03 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Junio C Hamano, Jim Meyering, Fredrik Gustafsson
In-Reply-To: <1320581184-4557-4-git-send-email-avarab@gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> Remove an "p->field < 0" comparison in grep.c that'll always be
> false. In this case "p" is a "grep_pat" where "field" is defined as:
>
> 	enum grep_header_field field;
>
> And grep_header_field is in turn defined as:
>
>     enum grep_header_field {
>     	GREP_HEADER_AUTHOR = 0,
>     	GREP_HEADER_COMMITTER
>     };
>
> Meaning that this comparison will always be false.

The underlying integer type is implementation-defined, and can be any
signed or unsigned integer type that can represent all enumerations.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH] grep: detect number of CPUs for thread spawning
From: Pete Wyckoff @ 2011-11-06 14:50 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Junio C Hamano, Eric Herman, Sverre Rabbelier,
	Fernando Vezzosi
In-Reply-To: <1320502568-14085-1-git-send-email-avarab@gmail.com>

avarab@gmail.com wrote on Sat, 05 Nov 2011 14:16 +0000:
> From: Eric Herman <eric@freesa.org>
> 
> Change the number of threads that we spawn from a hardcoded value of
> "8" to what online_cpus() returns.
> 
> Back in v1.7.0-rc1~19^2 when threaded grep was introduced the number
> of threads was hardcoded at compile time to 8, but this value was only
> used if online_cpus() returned greater than 1.
> 
> However just using 8 threads regardless of the actual number of CPUs
> is inefficient if we have more than 8 CPUs, and potentially wasteful
> if we have fewer than 8 CPUs.

I agree with the need to exploit >8 CPUs, but I lose a lot of
performance when limiting the threads to the number of physical
CPUs.

Tests without your patch on master, just changing "#define
THREADS" from 8 to 2.  On a 2-core Intel Core2 Duo.

Producing lots of output:

    8 threads:

	$ time ~/u/src/git/bin-wrappers/git grep f > /dev/null
	0m14.02s user 0m3.64s sys 0m11.93s elapsed 148.07 %CPU
	$ time ~/u/src/git/bin-wrappers/git grep f > /dev/null
	0m13.86s user 0m3.70s sys 0m11.82s elapsed 148.57 %CPU

    2 threads:

	$ time ~/u/src/git/bin-wrappers/git grep f > /dev/null
	0m15.14s user 0m3.52s sys 0m24.22s elapsed 77.05 %CPU
	$ time ~/u/src/git/bin-wrappers/git grep f > /dev/null
	0m14.85s user 0m3.79s sys 0m24.20s elapsed 77.05 %CPU

Producing no output:

    8 threads:

	$ time ~/u/src/git/bin-wrappers/git grep unfindable-string
	0m1.14s user 0m3.68s sys 0m5.17s elapsed 93.22 %CPU
	$ time ~/u/src/git/bin-wrappers/git grep unfindable-string
	0m1.28s user 0m3.56s sys 0m5.15s elapsed 94.22 %CPU

    2 threads:

	$ time ~/u/src/git/bin-wrappers/git grep unfindable-string
	0m1.36s user 0m3.64s sys 0m16.82s elapsed 29.75 %CPU
	$ time ~/u/src/git/bin-wrappers/git grep unfindable-string
	0m1.38s user 0m3.66s sys 0m16.81s elapsed 30.04 %CPU

My workdir is on NFS, where even though the repository is fully
cached, the open()s must go to the server.  Using more threads
than CPUs makes it more likely that some thread isn't blocked.

You could add a #threads knob, but then we'd have to get
everybody on NFS to set that properly.  Or take a look at
preload_index() to see how it guesses at how many threads it
needs.

		-- Pete

^ permalink raw reply

* Re: [PATCH] Introduce commit.verbose config option
From: SZEDER Gábor @ 2011-11-06 12:52 UTC (permalink / raw)
  To: Fernando Vezzosi; +Cc: git
In-Reply-To: <20111105182339.27C069004A@inscatolati.net>

On Sat, Nov 05, 2011 at 07:07:35PM +0100, Fernando Vezzosi wrote:
> Enabling commit.verbose will make git commit behave as if --verbose was
> passed on the command line.
> 
> Reviewed-by: Sverre Rabbelier <srabbelier@gmail.com>
> Signed-off-by: Fernando Vezzosi <buccia@repnz.net>
> ---
>  Documentation/config.txt |    3 +++
>  builtin/commit.c         |    4 ++++
>  2 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 5a841da..6826788 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -832,6 +832,9 @@ commit.template::
>  	"{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the
>  	specified user's home directory.
>  
> +commit.verbose::
> +	A boolean to enable verbose mode like the --verbose flag does.
> +

I just tried out this patch, and found that setting commit.verbose to
true also affects git commit's behavior when invoked from scripts
where you can't pass --verbose to git commit.  I.e. the diff to be
committed is shown for git revert, interactive rebase's reword or
squash commands, or during 'git rebase --continue' after resolving a
conflict.  I think this should be mentioned both in the commit message
and in the documentation.

Anyway, I like this change because I have a somewhat convoluted and
flaky combination of hooks and editor scripts to do the same, and now
I can finally get rid of them.


Best,
Gábor

^ permalink raw reply

* Re: [PATCH 0/3] Fix code issues spotted by clang
From: Ævar Arnfjörð Bjarmason @ 2011-11-06 12:33 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jim Meyering, Fredrik Gustafsson,
	Ævar Arnfjörð Bjarmason, Martin Waitz,
	Elijah Newren
In-Reply-To: <1320581184-4557-1-git-send-email-avarab@gmail.com>

On Sun, Nov 6, 2011 at 13:06, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> This series fixes some of the code issues raised by clang.

Out of curiosity I also compiled git on "Sun Studio 12 Update 1" on
ee6dfb2d83ba1b057943e705f707fa27e34e47f9 with this patch series
applied and it emits the following warnings, quoted below along with
my comments:

    "pack-write.c", line 76: warning: name redefined by pragma
redefine_extname declared static: tmpfile

Presumably this means that we've imported the function tmpfile() and
Sun's CC doesn't like that we're creating a variable with that name.

    "read-cache.c", line 761: warning: statement not reached

SunCC's brain is melting on this particularly clever piece of code:

    	goto inside;
    	for (;;) {
    		if (!c)
    			return 1;
    		if (is_dir_sep(c)) {
    inside:

    "sha1_file.c", line 2453: warning: name redefined by pragma
redefine_extname declared static: tmpfile

same tempfile issue.

    "compat/../git-compat-util.h", line 4: warning: macro redefined:
_FILE_OFFSET_BITS
    "compat/../git-compat-util.h", line 4: warning: macro redefined:
_FILE_OFFSET_BITS

Seems we're clashing with this:

    $ grep -r '#define.*_FILE_OFFSET_BITS' /usr/include/
    /usr/include/sys/feature_tests.h:#define   _FILE_OFFSET_BITS   64
    /usr/include/sys/feature_tests.h:#define   _FILE_OFFSET_BITS   32

    "xdiff/xutils.c", line 194: warning: statement not reached

This was added in b97e911643341cb31e6b97029b9ffd96fc675b1d as a
workaround on systems using glibc, should this even be run on Sun
libc? Martin?

Another clever bit of code blowing SunCC's brain:

	if (flags & XDF_IGNORE_WHITESPACE) {
		goto skip_ws;
		while (i1 < s1 && i2 < s2) {
			if (l1[i1++] != l2[i2++])
				return 0;
		skip_ws:
			while (i1 < s1 && XDL_ISSPACE(l1[i1]))
				i1++;
			while (i2 < s2 && XDL_ISSPACE(l2[i2]))
				i2++;
		}
	}

    "fast-import.c", line 858: warning: name redefined by pragma
redefine_extname declared static: tmpfile

ditto tempfile issue.

    "builtin/fast-export.c", line 54: warning: enum type mismatch: op "="

This seems to me to be a legitimate issue we introduced in
2d8ad46919213ebbd7bb72eb5b56cca8cc3ae07f, Elijah?

We're defining an enum like this:

    static enum { ABORT, VERBATIM, WARN, STRIP } signed_tag_mode = ABORT;
    static enum { ERROR, DROP, REWRITE } tag_of_filtered_mode = ABORT;

And then doing:

	if (unset || !strcmp(arg, "abort"))
		tag_of_filtered_mode = ABORT; <-- Line 54
	else if (!strcmp(arg, "drop"))
		tag_of_filtered_mode = DROP;
	else if (!strcmp(arg, "rewrite"))
		tag_of_filtered_mode = REWRITE;

Presumably that assignment should be "= ERROR".

    "builtin/index-pack.c", line 175: warning: name redefined by
pragma redefine_extname declared static: tmpfile

ditto tempfile.

    "vcs-svn/string_pool.c", line 11: warning: initializer will be
sign-extended: -1
    "vcs-svn/string_pool.c", line 81: warning: initializer will be
sign-extended: -1
    "vcs-svn/repo_tree.c", line 112: warning: initializer will be
sign-extended: -1
    "vcs-svn/repo_tree.c", line 112: warning: initializer will be
sign-extended: -1
    "test-treap.c", line 34: warning: initializer will be sign-extended: -1

All of these come down to assigning ~0 to an uint32_t variable, e.g.:

	uint32_t token = ~0;

^ permalink raw reply

* [PATCH 3/3] grep: get rid of useless x < 0 comparison on an enum member
From: Ævar Arnfjörð Bjarmason @ 2011-11-06 12:06 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jim Meyering, Fredrik Gustafsson,
	Ævar Arnfjörð Bjarmason
In-Reply-To: <1320581184-4557-1-git-send-email-avarab@gmail.com>

Remove an "p->field < 0" comparison in grep.c that'll always be
false. In this case "p" is a "grep_pat" where "field" is defined as:

	enum grep_header_field field;

And grep_header_field is in turn defined as:

    enum grep_header_field {
    	GREP_HEADER_AUTHOR = 0,
    	GREP_HEADER_COMMITTER
    };

Meaning that this comparison will always be false. This was spotted by
clang 2.9 which produced the following warning while compiling grep.c:

    grep.c:330:16: warning: comparison of unsigned enum expression < 0 is always false [-Wtautological-compare]
                    if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)
                        ~~~~~~~~ ^ ~

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 grep.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/grep.c b/grep.c
index b29d09c..025d3f8 100644
--- a/grep.c
+++ b/grep.c
@@ -327,7 +327,7 @@ static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
 	for (p = opt->header_list; p; p = p->next) {
 		if (p->token != GREP_PATTERN_HEAD)
 			die("bug: a non-header pattern in grep header list.");
-		if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)
+		if (GREP_HEADER_FIELD_MAX <= p->field)
 			die("bug: unknown header field %d", p->field);
 		compile_regexp(p, opt);
 	}
-- 
1.7.6.3

^ permalink raw reply related

* [PATCH 2/3] diff/apply: cast variable in call to free()
From: Ævar Arnfjörð Bjarmason @ 2011-11-06 12:06 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jim Meyering, Fredrik Gustafsson,
	Ævar Arnfjörð Bjarmason
In-Reply-To: <1320581184-4557-1-git-send-email-avarab@gmail.com>

Both of these free() calls are freeing a "const unsigned char (*)[20]"
type while free() expects a "void *". This results in the following
warning under clang 2.9:

    builtin/diff.c:185:7: warning: passing 'const unsigned char (*)[20]' to parameter of type 'void *' discards qualifiers
            free(parent);
                 ^~~~~~

    submodule.c:394:7: warning: passing 'const unsigned char (*)[20]' to parameter of type 'void *' discards qualifiers
            free(parents);
                 ^~~~~~~

This free()-ing without a cast was added by Jim Meyering to
builtin/diff.c in v1.7.6-rc3~4 and later by Fredrik Gustafsson in
submodule.c in v1.7.7-rc1~25^2.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/diff.c |    2 +-
 submodule.c    |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index 1118689..0fe638f 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -182,7 +182,7 @@ static int builtin_diff_combined(struct rev_info *revs,
 		hashcpy((unsigned char *)(parent + i), ent[i].item->sha1);
 	diff_tree_combined(parent[0], parent + 1, ents - 1,
 			   revs->dense_combined_merges, revs);
-	free(parent);
+	free((void *)parent);
 	return 0;
 }
 
diff --git a/submodule.c b/submodule.c
index 0fd10a0..52cdcc6 100644
--- a/submodule.c
+++ b/submodule.c
@@ -391,7 +391,7 @@ static void commit_need_pushing(struct commit *commit, struct commit_list *paren
 	rev.diffopt.format_callback_data = needs_pushing;
 	diff_tree_combined(commit->object.sha1, parents, n, 1, &rev);
 
-	free(parents);
+	free((void *)parents);
 }
 
 int check_submodule_needs_pushing(unsigned char new_sha1[20], const char *remotes_name)
-- 
1.7.6.3

^ permalink raw reply related

* [PATCH 1/3] apply: get rid of useless x < 0 comparison on a size_t type
From: Ævar Arnfjörð Bjarmason @ 2011-11-06 12:06 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jim Meyering, Fredrik Gustafsson,
	Ævar Arnfjörð Bjarmason
In-Reply-To: <1320581184-4557-1-git-send-email-avarab@gmail.com>

According to the C standard size_t is always unsigned, therefore the
comparison "n1 < 0 || n2 < 0" when n1 and n2 are size_t will always be
false.

This was raised by clang 2.9 which throws this warning when compiling
apply.c:

    builtin/apply.c:253:9: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
            if (n1 < 0 || n2 < 0)
                ~~ ^ ~
    builtin/apply.c:253:19: warning: comparison of unsigned expression < 0 is always false [-Wtautological-compare]
            if (n1 < 0 || n2 < 0)
                          ~~ ^ ~

This check was originally added in v1.6.5-rc0~53^2 by Giuseppe Bilotta
while adding an option to git-apply to ignore whitespace differences.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/apply.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 84a8a0b..b3b59db 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -250,9 +250,6 @@ static int fuzzy_matchlines(const char *s1, size_t n1,
 	const char *last2 = s2 + n2 - 1;
 	int result = 0;
 
-	if (n1 < 0 || n2 < 0)
-		return 0;
-
 	/* ignore line endings */
 	while ((*last1 == '\r') || (*last1 == '\n'))
 		last1--;
-- 
1.7.6.3

^ permalink raw reply related

* [PATCH 0/3] Fix code issues spotted by clang
From: Ævar Arnfjörð Bjarmason @ 2011-11-06 12:06 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jim Meyering, Fredrik Gustafsson,
	Ævar Arnfjörð Bjarmason

This series fixes some of the code issues raised by clang. This leaves
the following warnings that I haven't addressed:

    revision.c:766:25: warning: implicit truncation from 'unsigned int' to bitfield changes value from 4294967279 to 134217711
          [-Wconstant-conversion]
                    p->item->object.flags &= ~TMP_MARK;
                                          ^  ~~~~~~~~~
    revision.c:768:25: warning: implicit truncation from 'unsigned int' to bitfield changes value from 4294967279 to 134217711
          [-Wconstant-conversion]
                    p->item->object.flags &= ~TMP_MARK;
                                          ^  ~~~~~~~~~
    revision.c:1875:25: warning: implicit truncation from 'unsigned int' to bitfield changes value from 4294967279 to 134217711
          [-Wconstant-conversion]
                    p->item->object.flags &= ~TMP_MARK;
                                          ^  ~~~~~~~~~
    revision.c:2202:25: warning: implicit truncation from 'unsigned int' to bitfield changes value from 4294967158 to 134217590
          [-Wconstant-conversion]
                            commit->object.flags &= ~(ADDED | SEEN | SHOWN);
                                                 ^  ~~~~~~~~~~~~~~~~~~~~~~~
    upload-pack.c:115:12: warning: implicit truncation from 'unsigned int' to bitfield changes value from 4294967293 to 134217725
          [-Wconstant-conversion]
                    o->flags &= ~UNINTERESTING;
                             ^  ~~~~~~~~~~~~~~
    upload-pack.c:689:19: warning: implicit truncation from 'unsigned int' to bitfield changes value from 4294705151 to 133955583
          [-Wconstant-conversion]
                                    object->flags &= ~CLIENT_SHALLOW;
                                                  ^  ~~~~~~~~~~~~~~~
    builtin/checkout.c:676:16: warning: implicit truncation from 'unsigned int' to bitfield changes value from 4294967293 to 134217725
          [-Wconstant-conversion]
            object->flags &= ~UNINTERESTING;
                          ^  ~~~~~~~~~~~~~~
    builtin/reflog.c:173:32: warning: implicit truncation from 'unsigned int' to bitfield changes value from 4294965247 to 134215679
          [-Wconstant-conversion]
                    found.objects[i].item->flags &= ~STUDYING;
                                                 ^  ~~~~~~~~~
    builtin/reflog.c:232:31: warning: implicit truncation from 'unsigned int' to bitfield changes value from 4294963199 to 134213631
          [-Wconstant-conversion]
                    pending->item->object.flags &= ~REACHABLE;
                                                ^  ~~~~~~~~~~
    bisect.c:66:24: warning: implicit truncation from 'unsigned int' to bitfield changes value from 4294901759 to 134152191
          [-Wconstant-conversion]
                    commit->object.flags &= ~COUNTED;
                                         ^  ~~~~~~~~
    
Ævar Arnfjörð Bjarmason (3):
  apply: get rid of useless x < 0 comparison on a size_t type
  diff/apply: cast variable in call to free()
  grep: get rid of useless x < 0 comparison on an enum member

 builtin/apply.c |    3 ---
 builtin/diff.c  |    2 +-
 grep.c          |    2 +-
 submodule.c     |    2 +-
 4 files changed, 3 insertions(+), 6 deletions(-)

-- 
1.7.6.3

^ permalink raw reply

* [PATCH v2] pull: introduce a pull.rebase option to enable --rebase
From: Ævar Arnfjörð Bjarmason @ 2011-11-06  9:50 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Eric Herman, Sverre Rabbelier, Fernando Vezzosi,
	Johannes Schindelin, Ævar Arnfjörð Bjarmason
In-Reply-To: <7v8vnt7kvd.fsf@alter.siamese.dyndns.org>

Currently we either need to set branch.<name>.rebase for existing
branches if we'd like "git pull" to mean "git pull --rebase", or have
the forethought of setting "branch.autosetuprebase" before we create
the branch.

But there's no way to globally configure "git pull" to mean "git pull
--rebase" for existing branches, introduce a "pull.rebase" option to
do that.

This option will be considered at a lower priority than
branch.<name>.rebase, i.e. we could set pull.rebase=true and
branch.<name>.rebase=false and the latter configuration option would
win.

Reviewed-by: Sverre Rabbelier <srabbelier@gmail.com>
Reviewed-by: Fernando Vezzosi <buccia@repnz.net>
Reviewed-by: Eric Herman <eric@freesa.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

On Sun, Nov 6, 2011 at 08:47, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> This option will be considered at a lower priority than
>> branch.<name>.rebase, i.e. we could set pull.rebase=true and
>> branch.<name>.rebase=false and the latter configuration option would
>> win.
>>
>> Reviewed-by: Sverre Rabbelier <srabbelier@gmail.com>
>> Reviewed-by: Fernando Vezzosi <buccia@repnz.net>
>> Reviewed-by: Eric Herman <eric@freesa.org>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
> I see many reviewed-by lines, but what kind of review did this patch have,
> exactly? It seem to break its own test (branch.to-rebase.rebase should
> override pull.rebase).

We all stood behind my laptop while I explained what it did and
why. Sverre pointed out that I should use the test_when_finished()
function for unsetting the config variables, Eric and Fernando looked
it over as well.

> I think I've queued most (if not all) of the patches in flight except for
> Ram's sequencer reroll to 'pu' and pu^ passes the test but the tip of pu
> does not due to this topic.

Due to a trivial error of mine. I sent out the wrong patch as Sverre
thought, the problem was that I was trying to --unset a config
variable twice, that's now fixed and the tests pass with this patch.

 Documentation/config.txt   |   14 +++++++++++++-
 Documentation/git-pull.txt |    2 +-
 git-pull.sh                |    4 ++++
 t/t5520-pull.sh            |   23 +++++++++++++++++++++--
 4 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5a841da..b2d7d92 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -677,7 +677,9 @@ branch.<name>.mergeoptions::
 branch.<name>.rebase::
 	When true, rebase the branch <name> on top of the fetched branch,
 	instead of merging the default branch from the default remote when
-	"git pull" is run.
+	"git pull" is run. See "pull.rebase" for doing this in a non
+	branch-specific manner.
+
 	*NOTE*: this is a possibly dangerous operation; do *not* use
 	it unless you understand the implications (see linkgit:git-rebase[1]
 	for details).
@@ -1590,6 +1592,16 @@ pretty.<name>::
 	Note that an alias with the same name as a built-in format
 	will be silently ignored.
 
+pull.rebase::
+	When true, rebase branches on top of the fetched branch, instead
+	of merging the default branch from the default remote when "git
+	pull" is run. See "branch.<name>.rebase" for setting this on a
+	per-branch basis.
+
+	*NOTE*: this is a possibly dangerous operation; do *not* use
+	it unless you understand the implications (see linkgit:git-rebase[1]
+	for details).
+
 pull.octopus::
 	The default merge strategy to use when pulling multiple branches
 	at once.
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index e1da468..0f18ec8 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -108,7 +108,7 @@ include::merge-options.txt[]
 	fetched, the rebase uses that information to avoid rebasing
 	non-local changes.
 +
-See `branch.<name>.rebase` and `branch.autosetuprebase` in
+See `pull.rebase`, `branch.<name>.rebase` and `branch.autosetuprebase` in
 linkgit:git-config[1] if you want to make `git pull` always use
 `{litdd}rebase` instead of merging.
 +
diff --git a/git-pull.sh b/git-pull.sh
index 9868a0b..24b6b7c 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -44,6 +44,10 @@ merge_args=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
+if test -z "$rebase"
+then 
+	rebase=$(git config --bool pull.rebase)
+fi
 dry_run=
 while :
 do
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 0e5eb67..35304b4 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -94,16 +94,35 @@ test_expect_success '--rebase' '
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
 '
+test_expect_success 'pull.rebase' '
+	git reset --hard before-rebase &&
+	git config --bool pull.rebase true &&
+	test_when_finished "git config --unset pull.rebase" &&
+	git pull . copy &&
+	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
+	test new = $(git show HEAD:file2)
+'
 
 test_expect_success 'branch.to-rebase.rebase' '
 	git reset --hard before-rebase &&
-	git config branch.to-rebase.rebase 1 &&
+	git config --bool branch.to-rebase.rebase true &&
+	test_when_finished "git config --unset branch.to-rebase.rebase" &&
 	git pull . copy &&
-	git config branch.to-rebase.rebase 0 &&
 	test $(git rev-parse HEAD^) = $(git rev-parse copy) &&
 	test new = $(git show HEAD:file2)
 '
 
+test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
+	git reset --hard before-rebase &&
+	git config --bool pull.rebase true &&
+	test_when_finished "git config --unset pull.rebase" &&
+	git config --bool branch.to-rebase.rebase false &&
+	test_when_finished "git config --unset branch.to-rebase.rebase" &&
+	git pull . copy &&
+	test $(git rev-parse HEAD^) != $(git rev-parse copy) &&
+	test new = $(git show HEAD:file2)
+'
+
 test_expect_success '--rebase with rebased upstream' '
 
 	git remote add -f me . &&
-- 
1.7.6.3

^ permalink raw reply related

* Re: [PATCH] pull: introduce a pull.rebase option to enable --rebase
From: Sverre Rabbelier @ 2011-11-06  9:23 UTC (permalink / raw)
  To: Ævar Arnfjörð, Junio C Hamano
  Cc: git, Eric Herman, Fernando Vezzosi, Johannes Schindelin
In-Reply-To: <7v8vnt7kvd.fsf@alter.siamese.dyndns.org>

Heya,

On Sun, Nov 6, 2011 at 08:47, Junio C Hamano <gitster@pobox.com> wrote:
> I see many reviewed-by lines, but what kind of review did this patch have,
> exactly? It seem to break its own test (branch.to-rebase.rebase should
> override pull.rebase).

We looked at the patch the same way one would on list, and I assumed
that the tests passed. Ævar, did you sent a wrong version by accident
perhaps?

> Thanks for other topics from the Amsterdam together, by the way. I did not
> comment on them individually but they looked mostly reasonable from a
> quick glance.

Thanks.

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH na/strtoimax] Compatibility: declare strtoimax() under NO_STRTOUMAX
From: Johannes Sixt @ 2011-11-06  8:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nick Alcock, Git Mailing List
In-Reply-To: <7vlirt7pdk.fsf@alter.siamese.dyndns.org>

Am 06.11.2011 07:10, schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> Commit f696543d (Add strtoimax() compatibility function) introduced an
>> implementation of the function, but forgot to add a declaration.
>>
>> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
>> ---
> 
> Thanks, but I think f696543d is v1.7.6 and not that patch.

Oops, sorry for that.

>  I hope you do
> not mind if I just squashed this in, instead of leaving it as a separate
> patch.

I do not mind at all.

-- Hannes

^ permalink raw reply

* Re: [PATCH] pull: introduce a pull.rebase option to enable --rebase
From: Junio C Hamano @ 2011-11-06  7:47 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason
  Cc: git, Eric Herman, Sverre Rabbelier, Fernando Vezzosi,
	Johannes Schindelin
In-Reply-To: <1320507358-3407-1-git-send-email-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> This option will be considered at a lower priority than
> branch.<name>.rebase, i.e. we could set pull.rebase=true and
> branch.<name>.rebase=false and the latter configuration option would
> win.
>
> Reviewed-by: Sverre Rabbelier <srabbelier@gmail.com>
> Reviewed-by: Fernando Vezzosi <buccia@repnz.net>
> Reviewed-by: Eric Herman <eric@freesa.org>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

I see many reviewed-by lines, but what kind of review did this patch have,
exactly? It seem to break its own test (branch.to-rebase.rebase should
override pull.rebase).

I think I've queued most (if not all) of the patches in flight except for
Ram's sequencer reroll to 'pu' and pu^ passes the test but the tip of pu
does not due to this topic.

Thanks for other topics from the Amsterdam together, by the way. I did not
comment on them individually but they looked mostly reasonable from a
quick glance.

^ permalink raw reply

* Re: [PATCH na/strtoimax] Compatibility: declare strtoimax() under NO_STRTOUMAX
From: Junio C Hamano @ 2011-11-06  6:10 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Nick Alcock, Git Mailing List
In-Reply-To: <4EB5583E.2030306@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

> Commit f696543d (Add strtoimax() compatibility function) introduced an
> implementation of the function, but forgot to add a declaration.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---

Thanks, but I think f696543d is v1.7.6 and not that patch.  I hope you do
not mind if I just squashed this in, instead of leaving it as a separate
patch.

>  git-compat-util.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index feb6f8e..4efef46 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -354,6 +354,8 @@ extern size_t gitstrlcpy(char *, const char *, size_t);
>  #ifdef NO_STRTOUMAX
>  #define strtoumax gitstrtoumax
>  extern uintmax_t gitstrtoumax(const char *, char **, int);
> +#define strtoimax gitstrtoimax
> +extern intmax_t gitstrtoimax(const char *, char **, int);
>  #endif
>  
>  #ifdef NO_STRTOK_R

^ permalink raw reply

* Re: [PATCH] fsck: print progress
From: Junio C Hamano @ 2011-11-06  6:05 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: Stephen Boyd, git, Jeff King
In-Reply-To: <CACsJy8DDKLrtmA3rGdW6xghwSs_e+zi1o2LzPDbmryH+XufpvQ@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> Because in verbose mode, the screen is filled with "checking ..."
> lines for evevry object, there'll be no wonder why it stops for so
> long.

Ah, OK. It makes sense then, but the fact that you needed to explain it
would suggest that explanation deserves to be in the proposed commit log
message.

I was wondering if it was a typo of either "If --quiet is used, progress
is not shown", or "If --verbose=no is used, ..."

^ permalink raw reply

* Re: [PATCH 09/10] fmt-merge-msg: Add contents of merged tag in the merge message
From: Junio C Hamano @ 2011-11-06  6:03 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <4EB4F74D.7090004@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

> Since the tag signature depends on the tag message, including all spelin
> errors, the integrator must not change the text so that third parties
> can repeat the verification. (Correct?) I assume this is the reason that
> you put 'tag:' in front of the tag message, to discourage any changes.

Yes, but after reading response from Linus in the other thread, I think we
came up with even a better plan.

> What if the tag is not signed?

Thanks for raising a good point. See 

    http://thread.gmane.org/gmane.linux.ide/50518/focus=1211568

for the revised plan.

^ permalink raw reply

* Re: [PATCH 3/3] fast-export: output reset command for commandline revs
From: Jonathan Nieder @ 2011-11-06  5:01 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Junio C Hamano, Jeff King, Git List, Daniel Barkalow,
	Ramkumar Ramachandra, Dmitry Ivankov, Johannes Schindelin,
	Ævar Arnfjörð Bjarmason, Eric Herman,
	Fernando Vezzosi
In-Reply-To: <1320535407-4933-4-git-send-email-srabbelier@gmail.com>

Sverre Rabbelier wrote:

> When a revision is specified on the commandline we explicitly output
> a 'reset' command for it if it was not handled already. This allows
> for example the remote-helper protocol to use fast-export to create
> branches that point to a commit that has already been exported.
>
> Initial-patch-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>

Thanks.  I'd suggest squashing in the test from patch 1/3 for easy
reference (since each patch makes the other easier to understand).

> ---
>   The if statement dealing with tag_of_filtered_mode is not as
>   elegant as either me or Dscho would have liked, but we couldn't
>   find a better way to determine if a ref is a tag at this point
>   in the code.
>
>   Additionally, the elem->whence != REV_CMD_RIGHT case should really
>   check if REV_CMD_RIGHT_REF, but as this is not provided by the
>   ref_info structure this is left as is. A result of this is that
>   incorrect input will result in incorrect output, rather than an
>   error message. That is: `git fast-export a..<sha1>` will
>   incorrectly generate a `reset <sha1>` statement in the fast-export
>   stream.
>
>   The dwim_ref bit is a double work (it has already been done by the
>   caller of this function), but I decided it would be more work to
>   pass this information along than to recompute it for the few
>   commandline refs that were relevant.

These details seem like good details for the commit message, so the
next puzzled person looking at the code can see what behavior is
deliberate and what are the incidental side-effects.

The "git fast-export a..$(git rev-parse HEAD^{commit})" case sounds
worth a test.

> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -18,6 +18,8 @@
>  #include "parse-options.h"
>  #include "quote.h"
> 
> +#define REF_HANDLED (ALL_REV_FLAGS + 1)

Could TMP_MARK be used for this?

[...]
> @@ -541,10 +543,34 @@ static void handle_reset(const char *name, struct object *object)
>  		       sha1_to_hex(object->sha1));
>  }
>  
> -static void handle_tags_and_duplicates(struct string_list *extra_refs)
> +static void handle_tags_and_duplicates(struct rev_info *revs, struct string_list *extra_refs)
>  {
>  	int i;
>  
> +	/* even if no commits were exported, we need to export the ref */
> +	for (i = 0; i < revs->cmdline.nr; i++) {

Might be clearer in a new function.

> +		struct rev_cmdline_entry *elem = &revs->cmdline.rev[i];
> +
> +		if (elem->flags & UNINTERESTING)
> +			continue;
> +
> +		if (elem->whence != REV_CMD_REV && elem->whence != REV_CMD_RIGHT)
> +			continue;

Oh, neat.

> +
> +		char *full_name;

declaration-after-statement

> +		dwim_ref(elem->name, strlen(elem->name), elem->item->sha1, &full_name);
> +
> +		if (!prefixcmp(full_name, "refs/tags/") &&

What happens if dwim_ref fails, perhaps because a ref was deleted in
the meantime?

> +			(tag_of_filtered_mode != REWRITE ||
> +			!get_object_mark(elem->item)))
> +			continue;

Style nit: this would be easier to read if the "if" condition doesn't
line up with the code below it:

		if (!prefixcmp(full_name, "refs/tags/")) {
			if (tag_of_filtered_mode != REWRITE ||
			    !get_object_mark(elem->item))
				continue;
		}

If tag_of_filtered_mode == ABORT, we are going to die() soon, right?
So this seems to be about tag_of_filtered_mode == DROP --- makes
sense.

When does the !get_object_mark() case come up?

> +
> +		if (!(elem->flags & REF_HANDLED)) {
> +			handle_reset(full_name, elem->item);
> +			elem->flags |= REF_HANDLED;
> +		}

Just curious: is the REF_HANDLED handling actually needed?  What
would happen if fast-export included the redundant resets?

> +	}
[...]
> --- a/t/t9350-fast-export.sh
> +++ b/t/t9350-fast-export.sh
> @@ -446,7 +446,7 @@ from $(git rev-parse master)
>  
>  EOF
>  
> -test_expect_failure 'refs are updated even if no commits need to be exported' '
> +test_expect_success 'refs are updated even if no commits need to be exported' '
>  	git fast-export master..master > actual &&
>  	test_cmp expected actual
>  '

Thanks for a pleasant read.

^ permalink raw reply

* Re: [PATCH 2/3] fast-export: do not refer to non-existing marks
From: Jonathan Nieder @ 2011-11-06  4:45 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Junio C Hamano, Jeff King, Git List, Daniel Barkalow,
	Ramkumar Ramachandra, Dmitry Ivankov, Johannes Schindelin,
	Ævar Arnfjörð Bjarmason, Eric Herman,
	Fernando Vezzosi
In-Reply-To: <1320535407-4933-3-git-send-email-srabbelier@gmail.com>

Hi,

Sverre Rabbelier wrote:

> When calling `git fast-export a..a b` when a and b refer to the same
> commit, nothing would be exported, and an incorrect reset line would
> be printed for b ('from :0').

Hm, seems problematic indeed.

> Extract a handle_reset function that deals with this, which can then
> be re-used in a later commit.

So, does this patch drop the confusing behavior and add one that is
more intuitive for remote helpers?  It's not clear from this
description what sort of deal the patch makes and whether it is a good
or bad one.

[...]
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -529,9 +529,20 @@ static void get_tags_and_duplicates(struct object_array *pending,
>  	}
>  }
>  
> +static void handle_reset(const char *name, struct object *object)

Nit: the other handle_* functions are about acting on objects
encountered during revision traversal from the object store.  In other
words, the things being handled are the git objects.

By contrast, this function is about writing a "reset" command to the
fast-import stream.  I'd be tempted to call it reset_ref() or
something like that.

> +{
> +	int mark = get_object_mark(object);
> +
> -	commit = (struct commit *)object;
> -	printf("reset %s\nfrom :%d\n\n", name,
> -	       get_object_mark(&commit->object));
> +	if (mark)
> +		printf("reset %s\nfrom :%d\n\n", name,
> +		       get_object_mark(object));
> +	else
> +		printf("reset %s\nfrom %s\n\n", name,
> +		       sha1_to_hex(object->sha1));

Ah --- the functional change is to use a sha1 when there is no mark
corresponding to the object.

Why is this codepath being run at all when b is excluded by the
revision range (a..a a = ^a a a)?  Is this the same bug tested
for in patch 1/3 or something separate?

^ permalink raw reply

* Re: [PATCH 1/3] t9350: point out that refs are not updated correctly
From: Jonathan Nieder @ 2011-11-06  4:31 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Junio C Hamano, Jeff King, Git List, Daniel Barkalow,
	Ramkumar Ramachandra, Dmitry Ivankov, Johannes Schindelin,
	Ævar Arnfjörð Bjarmason, Eric Herman,
	Fernando Vezzosi
In-Reply-To: <1320535407-4933-2-git-send-email-srabbelier@gmail.com>

Sverre Rabbelier wrote:

> This happens only when the corresponding commits are not exported in
> the current fast-export run. This can happen either when the relevant
> commit is already marked, or when the commit is explicitly marked
> as UNINTERESTING with a negative ref by another argument.

The above "This" has no antecedent.  I guess you mean that
fast-export writes no output when passed a range of the form A..A.

> This breaks fast-export based remote helpers,

Makes sense.

> as they use marks
> files to store which commits have already been seen. The call graph
> is something as follows:
>
> $ # push master to remote repo
> $ git fast-export --{im,ex}port-marks=marksfile master
> $ # make a commit on master and push it to remote
> $ git fast-export --{im,ex}port-marks=marksfile master
> $ # run `git branch foo` and push it to remote
> $ git fast-export --{im,ex}port-marks=marksfile foo
>
> When fast-export imports the marksfile and sees that all commits in
> foo are marked as UNINTERESTING

Hmm, I didn't know about this behavior.  Would it be possible to add
a test for it, too?

>  t/t9350-fast-export.sh |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)

With or without the change suggested above, this new test seems to me
like a good thing, even though in the longer term it might be nicer to
teach fast-export to understand a syntax like

	git fast-import ^master master:master

Put another way, the possibility of something nicer later shouldn't
stop us from adding an incremental refinement that improves things
today.

Thanks for working on this,
Jonathan

^ permalink raw reply

* Re: [PATCH] Add abbreviated commit hash to rebase conflict message
From: Junio C Hamano @ 2011-11-06  4:14 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Ævar Arnfjörð, Jonas Flodén, Eric Herman,
	Fernando Vezzosi, Git List
In-Reply-To: <CAGdFq_hw1630ELQP3+AEaCmUTEjYq7K1j8ZB-n0_rb1VN=wQgA@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> We had a little Git hackathon in Amsterdam today,...

Ah, that was the context I was missing.

>> What happens when threeway is not enabled, and especially when "git am" is
>> used for applying patches, not within rebase?
>
> The same thing that already happens. I'm not sure what it is, but
> whatever title/number is shown, the matching hash is now shown as
> well. This patch does not change that behavior.

I am puzzled, but that cannot be true. The existing message uses $msgnum
and $FIRSTLINE but does not use $commit because it does not necessarily
exist.

What a value would the variable contain when I am applying your original
patch message using "git am -s" (or "git am -s3")?

^ permalink raw reply

* [ANNOUNCE] git-cola v1.7.0 tagged and released
From: David Aguilar @ 2011-11-06  4:11 UTC (permalink / raw)
  To: git-cola, git

The latest feature release of git-cola is available on github:

http://git-cola.github.com/

git-cola is a sleek and powerful git GUI.
It's fast, featureful, and fun to use.

Development is active after almost 4 years of development
so checkout our github repo for the latest and greatest:

https://github.com/git-cola/git-cola

New features since the last release
-----------------------------------
* A graphical DAG visualizer is available as `git dag`.
* The 'stash' widget includes a preview pane for instantly
  showing the contents of a stash.
* Usability improvements

`git-dag` is probably the most interesting new feature.
It visualizes DAGs using a 2D canvas which allows for unique
visualizations over a project's history.

`git-cola` has long had advanced interactive staging /
index-manipulation capabilities, difftool integration,
and much more.  It's written in python, portable to all
platforms where git is available, and GPL2 licensed.


Have fun,
-- 
				David @ https://github.com/davvid

^ permalink raw reply

* Re: [PATCH] Introduce commit.verbose config option
From: Nguyen Thai Ngoc Duy @ 2011-11-06  2:51 UTC (permalink / raw)
  To: Fernando Vezzosi; +Cc: git
In-Reply-To: <20111105182339.27C069004A@inscatolati.net>

On Sun, Nov 6, 2011 at 1:07 AM, Fernando Vezzosi <buccia@repnz.net> wrote:
> Enabling commit.verbose will make git commit behave as if --verbose was
> passed on the command line.

Is it necessary? If you want short command, you would use alias
instead of typing "commit". Adding --verbose to the alias to make the
command even shorter is straightforward.
-- 
Duy

^ 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