* Re: commit summary, --pretty=short and other tools
From: Johannes Schindelin @ 2007-09-18 10:13 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Benoit SIGOURE, Mike Hommey, git
In-Reply-To: <46EF7BF7.3070107@op5.se>
Hi,
On Tue, 18 Sep 2007, Andreas Ericsson wrote:
> const char *find_commit_subject_end(const char *commit_msg)
> {
> const char *dot, *paragraph_end;
> paragraph_end = strstr(commit_msg, "\n\n");
> dot = strchr(commit_msg, '.');
> return min_non_null(dot, paragraph_end); }
>
> would probably get it right very nearly always.
Counterexample (not even mentioning the missing handling of NULL):
http://brick.kernel.dk/git/?p=qemu.git;a=commit;h=eb66d86e295cd5a8f13221589806e15db62a62fa
And no, the responsible developer showed a strong unwillingness to adapt
to better tools and workflows.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] Fix lapsus in builtin-apply.c
From: Pierre Habouzit @ 2007-09-18 10:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 653 bytes --]
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
I found this issue trying to refactor the "quote" module. It's
definitely worth to push this in maint.
builtin-apply.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 05ce220..86d89a4 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -254,7 +254,7 @@ static char *find_name(const char *line, char *def, int p_value, int terminate)
if (name) {
char *cp = name;
while (p_value) {
- cp = strchr(name, '/');
+ cp = strchr(cp, '/');
if (!cp)
break;
cp++;
--
1.5.3.1
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply related
* [PATCH] Mention that 'push .. master' is in explicit form master:refs/heads/master
From: Jari Aalto @ 2007-09-18 9:59 UTC (permalink / raw)
To: git
Signed-off-by: Jari Aalto <jari.aalto AT cante.net>
---
Documentation/git-push.txt | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 7b8e075..71ac450 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -105,7 +105,9 @@ git push origin master::
Find a ref that matches `master` in the source repository
(most likely, it would find `refs/heads/master`), and update
the same ref (e.g. `refs/heads/master`) in `origin` repository
- with it.
+ with it. The following would be exactly same command:
+
+ git push origin master:refs/heads/master
git push origin :experimental::
Find a ref that matches `experimental` in the `origin` repository
--
1.5.3
--
Welcome to FOSS revolution: we fix and modify until it shines
^ permalink raw reply related
* Re: git-fsck/lost-found's speed vs git-prune's
From: Mike Hommey @ 2007-09-18 9:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4phswcuj.fsf@gitster.siamese.dyndns.org>
On Tue, Sep 18, 2007 at 02:18:44AM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> Mike Hommey <mh@glandium.org> writes:
>
> > I was wondering if that was to be expected for git-fsck to be
> > significantly slower than git-prune (by several orders of magnitude) ?
>
> fsck validates objects are correct and sane. prune only looks
> at reachability.
Now, the speed difference makes sense, but I wouldn't expect lost-found
to actually bother validating objects...
Mike
^ permalink raw reply
* Re: [PATCH] Drop UTF-8 characters in manual pages
From: Mike Hommey @ 2007-09-18 9:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd4wgwczs.fsf@gitster.siamese.dyndns.org>
On Tue, Sep 18, 2007 at 02:15:35AM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> Mike Hommey <mh@glandium.org> writes:
>
> > The default character encoding for english manual pages is ISO8859-1,
>
> On which distro?
Debian, at least, though transition to UTF-8 manpages is going to happen
(it seems there was a lack of proper encoding handling in man-db until
very recently)
> > so
> > UTF-8 characters are just displayed as their sequence of bytes, which is
> > not very appealing.
>
> Perhaps not. I cannot decide what to do with
> Documentation/git-pack-redundant.txt, though.
I'd say just use a transliterated form until man-db mess is sorted out.
Mike
^ permalink raw reply
* Re: [PATCH] git-svnimport: Use separate arguments in the pipe for git-rev-parse
From: Matthias Urlichs @ 2007-09-18 9:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Dan Libby, git
In-Reply-To: <7vlkb4wdzq.fsf@gitster.siamese.dyndns.org>
Some people seem to create SVN branch names with spaces
or other shell metacharacters.
Signed-Off-By: Matthias Urlichs <smurf@smurf.noris.de>
---
Junio C Hamano:
> > - open(H,"git-rev-parse --verify $parent |");
> > + open(H,'-|',"git-rev-parse","--verify",$parent);
>
> I seem to be missing the context, but please describe what
> problem this fixes in the commit log message. I guess some
> people use shell metacharacters and/or SP in their branch names
> and this is about that problem?
Exactly. Sorry; it seems that the original question hasn't been posted
to the mailing list.
diff --git a/git-svnimport.perl b/git-svnimport.perl
index d3ad5b9..aa5b3b2 100755
--- a/git-svnimport.perl
+++ b/git-svnimport.perl
@@ -633,7 +633,7 @@ sub commit {
my $rev;
if($revision > $opt_s and defined $parent) {
- open(H,"git-rev-parse --verify $parent |");
+ open(H,'-|',"git-rev-parse","--verify",$parent);
$rev = <H>;
close(H) or do {
print STDERR "$revision: cannot find commit '$parent'!\n";
--
Matthias Urlichs | {M:U} IT Design @ m-u-it.de | smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
- -
BOFH excuse #11:
magnetic interference from money/credit cards
^ permalink raw reply related
* Re: git-fsck/lost-found's speed vs git-prune's
From: Junio C Hamano @ 2007-09-18 9:18 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
In-Reply-To: <20070918090926.GA8927@glandium.org>
Mike Hommey <mh@glandium.org> writes:
> I was wondering if that was to be expected for git-fsck to be
> significantly slower than git-prune (by several orders of magnitude) ?
fsck validates objects are correct and sane. prune only looks
at reachability.
^ permalink raw reply
* Re: diffcore-rename performance mode
From: Junio C Hamano @ 2007-09-18 9:17 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20070918090105.GA11854@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Sep 18, 2007 at 01:58:17AM -0700, Junio C Hamano wrote:
>
>> > I thought we were holding counts of hashes, in which case there _is_ no
>> > overflow.
>>
>> The raw hashval (the fingerprint recorded in struct spanhash) is
>> further reduced and used as an index into spahash_top.data[].
>> So more than one hashval can try to sit in the same slot in
>> spanhash_top.data[] array.
>
> Right, that's sort of what I was hinting at in the original message. Can
> we just make the hash table big enough to use the fingerprint hashes
> directly? It's going to use a bit more memory, but lookups should be
> very fast. I'll try to experiment and get some numbers.
Thanks -- I vaguely recall large hash was disastrous for me
(trashed cache), but that was on a different hardware, different
time.
^ permalink raw reply
* Re: [PATCH] Drop UTF-8 characters in manual pages
From: Junio C Hamano @ 2007-09-18 9:15 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
In-Reply-To: <11901003792475-git-send-email-mh@glandium.org>
Mike Hommey <mh@glandium.org> writes:
> The default character encoding for english manual pages is ISO8859-1,
On which distro?
> so
> UTF-8 characters are just displayed as their sequence of bytes, which is
> not very appealing.
Perhaps not. I cannot decide what to do with
Documentation/git-pack-redundant.txt, though.
^ permalink raw reply
* Re: git-fsck/lost-found's speed vs git-prune's
From: Sam Vilain @ 2007-09-18 9:14 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
In-Reply-To: <20070918090926.GA8927@glandium.org>
Mike Hommey wrote:
> Hi,
>
> I was wondering if that was to be expected for git-fsck to be
> significantly slower than git-prune (by several orders of magnitude) ?
>
> $ time git-lost-found
>
> real 8m22.167s
> user 6m44.153s
> sys 1m16.613s
>
> $ time git-prune
>
> real 0m0.376s
> user 0m0.304s
> sys 0m0.000s
>
You're probably already packed. I'd expect a similar speed difference
between git-fsck and git-fsck --full.
Sam.
^ permalink raw reply
* Re: [PATCH] git-merge: add option --no-ff
From: Sam Vilain @ 2007-09-18 9:12 UTC (permalink / raw)
To: Sam Vilain
Cc: Junio C Hamano, Lars Hjemli, Eric Wong, Andreas Ericsson,
Johannes Schindelin, Chris Shoemaker, git
In-Reply-To: <46EF7EA1.6020402@vilain.net>
Sam Vilain wrote:
>>> I'd say 'git-svn merge' as a wrapper for 'git merge --no-ff' would be cleaner.
>>>
>> That unfortunately does not solve the problem.
>>
>
> I think we 'just' need to fix pushing merges back to SVN - so that they
> properly set Subversion 1.5+ (and possibly SVK) merge attributes - and
> if it is ambiguous which branch to push to, force the user to decide.
>
Whoops, I missed the thrust of the current issue; it won't be ambiguous,
it'll be unambiguously wrong, so this doesn't apply.
In which case I'd guess the moral equivalent of --track would have to go
forward, or a per-branch basis.
I think that writing a real fast-forward merge should only happen on
dcommit, not git merge, because that is what is required for SVN.
Ideally, it should also have the property that it doesn't cycle; null
merges between two branches should not carry on indefinitely.
Sam.
^ permalink raw reply
* git-fsck/lost-found's speed vs git-prune's
From: Mike Hommey @ 2007-09-18 9:09 UTC (permalink / raw)
To: git
Hi,
I was wondering if that was to be expected for git-fsck to be
significantly slower than git-prune (by several orders of magnitude) ?
$ time git-lost-found
real 8m22.167s
user 6m44.153s
sys 1m16.613s
$ time git-prune
real 0m0.376s
user 0m0.304s
sys 0m0.000s
Mike
^ permalink raw reply
* Re: diffcore-rename performance mode
From: Jeff King @ 2007-09-18 9:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhclswdsm.fsf@gitster.siamese.dyndns.org>
On Tue, Sep 18, 2007 at 01:58:17AM -0700, Junio C Hamano wrote:
> > I thought we were holding counts of hashes, in which case there _is_ no
> > overflow.
>
> The raw hashval (the fingerprint recorded in struct spanhash) is
> further reduced and used as an index into spahash_top.data[].
> So more than one hashval can try to sit in the same slot in
> spanhash_top.data[] array.
Right, that's sort of what I was hinting at in the original message. Can
we just make the hash table big enough to use the fingerprint hashes
directly? It's going to use a bit more memory, but lookups should be
very fast. I'll try to experiment and get some numbers.
-Peff
^ permalink raw reply
* Re: diffcore-rename performance mode
From: Junio C Hamano @ 2007-09-18 8:58 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20070918085413.GA11751@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I thought we were holding counts of hashes, in which case there _is_ no
> overflow.
The raw hashval (the fingerprint recorded in struct spanhash) is
further reduced and used as an index into spahash_top.data[].
So more than one hashval can try to sit in the same slot in
spanhash_top.data[] array.
^ permalink raw reply
* Re: [PATCH 1/3] git-apply: fix whitespace stripping
From: David Kastrup @ 2007-09-18 8:55 UTC (permalink / raw)
To: git
In-Reply-To: <11899829424173-git-send-email-bfields@citi.umich.edu>
"J. Bruce Fields" <bfields@citi.umich.edu> writes:
> The algorithm isn't right here: it accumulates any set of 8 spaces into
> tabs even if they're separated by tabs, so
>
> <four spaces><tab><four spaces><tab>
>
> is converted to
>
> <tab><tab><tab>
>
> when it should be just
>
> <tab><tab>
>
> So teach git-apply that a tab hides any group of less than 8 previous
> spaces in a row.
>
> Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
> ---
> builtin-apply.c | 13 ++++++++++---
> 1 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/builtin-apply.c b/builtin-apply.c
> index 976ec77..70359c1 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -1642,15 +1642,22 @@ static int apply_line(char *output, const char *patch, int plen)
>
> buf = output;
> if (need_fix_leading_space) {
> + int consecutive_spaces = 0;
> /* between patch[1..last_tab_in_indent] strip the
> * funny spaces, updating them to tab as needed.
> */
> for (i = 1; i < last_tab_in_indent; i++, plen--) {
> char ch = patch[i];
> - if (ch != ' ')
> + if (ch != ' ') {
> + consecutive_spaces = 0;
> *output++ = ch;
> - else if ((i % 8) == 0)
> - *output++ = '\t';
> + } else {
> + consecutive_spaces++;
> + if (consecutive_spaces == 8) {
> + *output++ = '\t';
> + consecutive_spaces = 0;
> + }
> + }
> }
> fixed = 1;
> i = last_tab_in_indent;
> --
> 1.5.3.1.42.gfe5df
As far as I can see, this does not really work since it does not
maintain an idea of a current column.
If you have
abcd<four spaces><tab><four spaces><tab>
then indeed the resulting conversion needs to be <tab><tab><tab>
whereas with
abc<four spaces><tab><four spaces><tab>
the resulting conversion needs to be just <tab><tab>
--
David Kastrup
^ permalink raw reply
* [PATCH 5/5] Use 'unsigned:1' when we mean boolean options
From: Shawn O. Pearce @ 2007-09-18 8:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
These options are all strictly boolean (true/false). Its easier to
document this implicitly by making their storage type a single bit.
There is no compelling memory space reduction reason for this change,
it just makes the structure definition slightly more readable.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
fetch-pack.h | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fetch-pack.h b/fetch-pack.h
index cdcd84f..ad13076 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -4,14 +4,14 @@
struct fetch_pack_args
{
const char *uploadpack;
- int quiet;
- int keep_pack;
int unpacklimit;
- int use_thin_pack;
- int fetch_all;
- int verbose;
int depth;
- int no_progress;
+ unsigned quiet:1,
+ keep_pack:1,
+ use_thin_pack:1,
+ fetch_all:1,
+ verbose:1,
+ no_progress:1;
};
void setup_fetch_pack(struct fetch_pack_args *args);
--
1.5.3.1.1000.g7319b
^ permalink raw reply related
* [PATCH 4/5] Avoid printing unnecessary warnings during fetch and push
From: Shawn O. Pearce @ 2007-09-18 8:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
If a transport doesn't support an option we already are telling
the higher level application (fetch or push) that the option is not
valid by sending back a >0 return value from transport_set_option
so there's not a strong motivation to have the function perform the
output itself. Instead we should let the higher level application
do the output if it is necessary. This avoids always telling the
user that depth isn't supported on HTTP urls even when they did
not pass a --depth option to git-fetch.
If the user passes an option and the option value is invalid we now
properly die in git-fetch instead of just spitting out a message
and running anyway. This mimics prior behavior better where
incorrect/malformed options are not accepted by the process.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
builtin-fetch.c | 18 +++++++++++++++---
transport.c | 11 ++---------
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 670af0b..b9722e5 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -430,6 +430,17 @@ static int do_fetch(struct transport *transport,
return 0;
}
+static void set_option(const char *name, const char *value)
+{
+ int r = transport_set_option(transport, name, value);
+ if (r < 0)
+ die("Option \"%s\" value \"%s\" is not valid for %s\n",
+ name, value, transport->url);
+ if (r > 0)
+ warning("Option \"%s\" is ignored for %s\n",
+ name, transport->url);
+}
+
int cmd_fetch(int argc, const char **argv, const char *prefix)
{
struct remote *remote;
@@ -525,10 +536,11 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
if (quiet)
transport->verbose = 0;
if (upload_pack)
- transport_set_option(transport, TRANS_OPT_UPLOADPACK, upload_pack);
+ set_option(TRANS_OPT_UPLOADPACK, upload_pack);
if (keep)
- transport_set_option(transport, TRANS_OPT_KEEP, "yes");
- transport_set_option(transport, TRANS_OPT_DEPTH, depth);
+ set_option(TRANS_OPT_KEEP, "yes");
+ if (depth)
+ set_option(TRANS_OPT_DEPTH, depth);
if (!transport->url)
die("Where do you want to fetch from today?");
diff --git a/transport.c b/transport.c
index 7f94d30..cc76e3f 100644
--- a/transport.c
+++ b/transport.c
@@ -460,16 +460,9 @@ struct transport *transport_get(struct remote *remote, const char *url)
int transport_set_option(struct transport *transport,
const char *name, const char *value)
{
- int ret = 1;
if (transport->ops->set_option)
- ret = transport->ops->set_option(transport, name, value);
- if (ret < 0)
- fprintf(stderr, "For '%s' option %s cannot be set to '%s'\n",
- transport->url, name, value);
- if (ret > 0)
- fprintf(stderr, "For '%s' option %s is ignored\n",
- transport->url, name);
- return ret;
+ return transport->ops->set_option(transport, name, value);
+ return 1;
}
int transport_push(struct transport *transport,
--
1.5.3.1.1000.g7319b
^ permalink raw reply related
* [PATCH 3/5] Correct handling of branch.$name.merge in builtin-fetch
From: Shawn O. Pearce @ 2007-09-18 8:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
My prior bug fix for git-push titled "Don't configure remote "." to
fetch everything to itself" actually broke t5520 as we were unable
to evaluate a branch configuration of:
[branch "copy"]
remote = .
merge = refs/heads/master
as remote "." did not have a "remote...fetch" configuration entry to
offer up refs/heads/master as a possible candidate available to be
fetched and merged. In shell script git-fetch and prior to the above
mentioned commit this was hardcoded for a url of "." to be the set of
local branches.
Chasing down this bug led me to the conclusion that our prior behavior
with regards to branch.$name.merge was incorrect. In the shell script
based git-fetch implementation we only fetched and merged a branch if
it appeared both in branch.$name.merge *and* in remote.$r.fetch, where
$r = branch.$name.remote. In other words in the following config file:
[remote "origin"]
url = git://git.kernel.org/pub/scm/git/git.git
fetch = refs/heads/master:refs/remotes/origin/master
[branch "master"]
remote = origin
merge = refs/heads/master
[branch "pu"]
remote = origin
merge = refs/heads/pu
Attempting to run `git pull` while on branch "pu" would always give
the user "Already up-to-date" as git-fetch did not fetch pu and thus
did not mark it for merge in .git/FETCH_HEAD. The configured merge
would always be ignored and the user would be left scratching her
confused head wondering why merge did not work on "pu" but worked
fine on "master".
If we are using the "default fetch" specification for the current
branch and the current branch has a branch.$name.merge configured
we now union it with the list of refs in remote.$r.fetch. This
way the above configuration does what the user expects it to do,
which is to fetch only "master" by default but when on "pu" to
fetch both "master" and "pu".
This uncovered some breakage in the test suite where old-style Cogito
branches (.git/branches/$r) did not fetch the branches listed in
.git/config for merging and thus did not actually merge them if the
user tried to use `git pull` on that branch. Junio and I discussed
it on list and felt that the union approach here makes more sense to
DWIM for the end-user than silently ignoring their configured request
so the test vectors for t5515 have been updated to include for-merge
lines in .git/FETCH_HEAD where they have been configured for-merge
in .git/config.
Since we are now performing a union of the fetch specification and
the merge specification and we cannot allow a branch to be listed
twice (otherwise it comes out twice in .git/FETCH_HEAD) we need to
perform a double loop here over all of the branch.$name.merge lines
and try to set their merge flag if we have already schedule that
branch for fetching by remote.$r.fetch. If no match is found then
we must add new specifications to fetch the branch but not store it
as no local tracking branch has been designated.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
builtin-fetch.c | 48 ++++++++++++++-----
remote.c | 13 ++---
remote.h | 3 +-
t/t5515/fetch.br-branches-default-merge | 1 +
...etch.br-branches-default-merge_branches-default | 1 +
t/t5515/fetch.br-branches-default-octopus | 2 +
...ch.br-branches-default-octopus_branches-default | 2 +
t/t5515/fetch.br-branches-one-merge | 1 +
t/t5515/fetch.br-branches-one-merge_branches-one | 1 +
t/t5515/fetch.br-branches-one-octopus | 1 +
t/t5515/fetch.br-branches-one-octopus_branches-one | 1 +
11 files changed, 51 insertions(+), 23 deletions(-)
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 20926e0..670af0b 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -28,20 +28,37 @@ static void unlock_pack_on_signal(int signo)
raise(signo);
}
-static void find_merge_config(struct ref *ref_map, struct remote *remote)
+static void add_merge_config(struct ref **head,
+ struct ref *remote_refs,
+ struct branch *branch,
+ struct ref ***tail)
{
- struct ref *rm = ref_map;
- struct branch *branch = branch_get(NULL);
+ int i;
- for (rm = ref_map; rm; rm = rm->next) {
- if (!branch_has_merge_config(branch)) {
- if (remote && remote->fetch &&
- !strcmp(remote->fetch[0].src, rm->name))
- rm->merge = 1;
- } else {
- if (branch_merges(branch, rm->name))
+ for (i = 0; i < branch->merge_nr; i++) {
+ struct ref *rm, **old_tail = *tail;
+ struct refspec refspec;
+
+ for (rm = *head; rm; rm = rm->next) {
+ if (branch_merge_matches(branch, i, rm->name)) {
rm->merge = 1;
+ break;
+ }
}
+ if (rm)
+ continue;
+
+ /* Not fetched to a tracking branch? We need to fetch
+ * it anyway to allow this branch's "branch.$name.merge"
+ * to be honored by git-pull.
+ */
+ refspec.src = branch->merge[i]->src;
+ refspec.dst = NULL;
+ refspec.pattern = 0;
+ refspec.force = 0;
+ get_fetch_map(remote_refs, &refspec, tail);
+ for (rm = *old_tail; rm; rm = rm->next)
+ rm->merge = 1;
}
}
@@ -76,17 +93,22 @@ static struct ref *get_ref_map(struct transport *transport,
} else {
/* Use the defaults */
struct remote *remote = transport->remote;
- if (remote->fetch_refspec_nr) {
+ struct branch *branch = branch_get(NULL);
+ int has_merge = branch_has_merge_config(branch);
+ if (remote && (remote->fetch_refspec_nr || has_merge)) {
for (i = 0; i < remote->fetch_refspec_nr; i++) {
get_fetch_map(remote_refs, &remote->fetch[i], &tail);
if (remote->fetch[i].dst &&
remote->fetch[i].dst[0])
*autotags = 1;
+ if (!i && !has_merge && ref_map &&
+ !strcmp(remote->fetch[0].src, ref_map->name))
+ ref_map->merge = 1;
}
- find_merge_config(ref_map, remote);
+ if (has_merge)
+ add_merge_config(&ref_map, remote_refs, branch, &tail);
} else {
ref_map = get_remote_ref(remote_refs, "HEAD");
-
ref_map->merge = 1;
}
}
diff --git a/remote.c b/remote.c
index af3c46b..31e2b70 100644
--- a/remote.c
+++ b/remote.c
@@ -772,16 +772,13 @@ int branch_has_merge_config(struct branch *branch)
return branch && !!branch->merge;
}
-int branch_merges(struct branch *branch, const char *refname)
+int branch_merge_matches(struct branch *branch,
+ int i,
+ const char *refname)
{
- int i;
- if (!branch)
+ if (!branch || i < 0 || i >= branch->merge_nr)
return 0;
- for (i = 0; i < branch->merge_nr; i++) {
- if (ref_matches_abbrev(branch->merge[i]->src, refname))
- return 1;
- }
- return 0;
+ return ref_matches_abbrev(branch->merge[i]->src, refname);
}
static struct ref *get_expanded_map(struct ref *remote_refs,
diff --git a/remote.h b/remote.h
index 8994052..b5b558f 100644
--- a/remote.h
+++ b/remote.h
@@ -88,7 +88,6 @@ struct branch {
struct branch *branch_get(const char *name);
int branch_has_merge_config(struct branch *branch);
-
-int branch_merges(struct branch *branch, const char *refname);
+int branch_merge_matches(struct branch *, int n, const char *);
#endif
diff --git a/t/t5515/fetch.br-branches-default-merge b/t/t5515/fetch.br-branches-default-merge
index 828bfd8..ca2cc1d 100644
--- a/t/t5515/fetch.br-branches-default-merge
+++ b/t/t5515/fetch.br-branches-default-merge
@@ -1,5 +1,6 @@
# br-branches-default-merge
754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge branch 'master' of ../
+0567da4d5edd2ff4bb292a465ba9e64dcad9536b branch 'three' of ../
754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge tag 'tag-master' of ../
8e32a6d901327a23ef831511badce7bf3bf46689 not-for-merge tag 'tag-one' of ../
22feea448b023a2d864ef94b013735af34d238ba not-for-merge tag 'tag-one-tree' of ../
diff --git a/t/t5515/fetch.br-branches-default-merge_branches-default b/t/t5515/fetch.br-branches-default-merge_branches-default
index f148673..7d947cd 100644
--- a/t/t5515/fetch.br-branches-default-merge_branches-default
+++ b/t/t5515/fetch.br-branches-default-merge_branches-default
@@ -1,5 +1,6 @@
# br-branches-default-merge branches-default
754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge branch 'master' of ../
+0567da4d5edd2ff4bb292a465ba9e64dcad9536b branch 'three' of ../
754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge tag 'tag-master' of ../
8e32a6d901327a23ef831511badce7bf3bf46689 not-for-merge tag 'tag-one' of ../
22feea448b023a2d864ef94b013735af34d238ba not-for-merge tag 'tag-one-tree' of ../
diff --git a/t/t5515/fetch.br-branches-default-octopus b/t/t5515/fetch.br-branches-default-octopus
index bb1a191..ec39c54 100644
--- a/t/t5515/fetch.br-branches-default-octopus
+++ b/t/t5515/fetch.br-branches-default-octopus
@@ -1,5 +1,7 @@
# br-branches-default-octopus
754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge branch 'master' of ../
+8e32a6d901327a23ef831511badce7bf3bf46689 branch 'one' of ../
+6134ee8f857693b96ff1cc98d3e2fd62b199e5a8 branch 'two' of ../
754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge tag 'tag-master' of ../
8e32a6d901327a23ef831511badce7bf3bf46689 not-for-merge tag 'tag-one' of ../
22feea448b023a2d864ef94b013735af34d238ba not-for-merge tag 'tag-one-tree' of ../
diff --git a/t/t5515/fetch.br-branches-default-octopus_branches-default b/t/t5515/fetch.br-branches-default-octopus_branches-default
index 970fc93..6bf42e2 100644
--- a/t/t5515/fetch.br-branches-default-octopus_branches-default
+++ b/t/t5515/fetch.br-branches-default-octopus_branches-default
@@ -1,5 +1,7 @@
# br-branches-default-octopus branches-default
754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge branch 'master' of ../
+8e32a6d901327a23ef831511badce7bf3bf46689 branch 'one' of ../
+6134ee8f857693b96ff1cc98d3e2fd62b199e5a8 branch 'two' of ../
754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge tag 'tag-master' of ../
8e32a6d901327a23ef831511badce7bf3bf46689 not-for-merge tag 'tag-one' of ../
22feea448b023a2d864ef94b013735af34d238ba not-for-merge tag 'tag-one-tree' of ../
diff --git a/t/t5515/fetch.br-branches-one-merge b/t/t5515/fetch.br-branches-one-merge
index 24099fd..b4b3b35 100644
--- a/t/t5515/fetch.br-branches-one-merge
+++ b/t/t5515/fetch.br-branches-one-merge
@@ -1,5 +1,6 @@
# br-branches-one-merge
8e32a6d901327a23ef831511badce7bf3bf46689 not-for-merge branch 'one' of ../
+0567da4d5edd2ff4bb292a465ba9e64dcad9536b branch 'three' of ../
754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge tag 'tag-master' of ../
8e32a6d901327a23ef831511badce7bf3bf46689 not-for-merge tag 'tag-one' of ../
22feea448b023a2d864ef94b013735af34d238ba not-for-merge tag 'tag-one-tree' of ../
diff --git a/t/t5515/fetch.br-branches-one-merge_branches-one b/t/t5515/fetch.br-branches-one-merge_branches-one
index e4b4fde..2ecef38 100644
--- a/t/t5515/fetch.br-branches-one-merge_branches-one
+++ b/t/t5515/fetch.br-branches-one-merge_branches-one
@@ -1,5 +1,6 @@
# br-branches-one-merge branches-one
8e32a6d901327a23ef831511badce7bf3bf46689 not-for-merge branch 'one' of ../
+0567da4d5edd2ff4bb292a465ba9e64dcad9536b branch 'three' of ../
754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge tag 'tag-master' of ../
8e32a6d901327a23ef831511badce7bf3bf46689 not-for-merge tag 'tag-one' of ../
22feea448b023a2d864ef94b013735af34d238ba not-for-merge tag 'tag-one-tree' of ../
diff --git a/t/t5515/fetch.br-branches-one-octopus b/t/t5515/fetch.br-branches-one-octopus
index 53fe808..96e3029 100644
--- a/t/t5515/fetch.br-branches-one-octopus
+++ b/t/t5515/fetch.br-branches-one-octopus
@@ -1,5 +1,6 @@
# br-branches-one-octopus
8e32a6d901327a23ef831511badce7bf3bf46689 branch 'one' of ../
+6134ee8f857693b96ff1cc98d3e2fd62b199e5a8 branch 'two' of ../
754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge tag 'tag-master' of ../
8e32a6d901327a23ef831511badce7bf3bf46689 not-for-merge tag 'tag-one' of ../
22feea448b023a2d864ef94b013735af34d238ba not-for-merge tag 'tag-one-tree' of ../
diff --git a/t/t5515/fetch.br-branches-one-octopus_branches-one b/t/t5515/fetch.br-branches-one-octopus_branches-one
index 41b18ff..55e0bad 100644
--- a/t/t5515/fetch.br-branches-one-octopus_branches-one
+++ b/t/t5515/fetch.br-branches-one-octopus_branches-one
@@ -1,5 +1,6 @@
# br-branches-one-octopus branches-one
8e32a6d901327a23ef831511badce7bf3bf46689 branch 'one' of ../
+6134ee8f857693b96ff1cc98d3e2fd62b199e5a8 branch 'two' of ../
754b754407bf032e9a2f9d5a9ad05ca79a6b228f not-for-merge tag 'tag-master' of ../
8e32a6d901327a23ef831511badce7bf3bf46689 not-for-merge tag 'tag-one' of ../
22feea448b023a2d864ef94b013735af34d238ba not-for-merge tag 'tag-one-tree' of ../
--
1.5.3.1.1000.g7319b
^ permalink raw reply related
* [PATCH 2/5] Don't attempt to merge non-existant remotes in t5515
From: Shawn O. Pearce @ 2007-09-18 8:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This was actually reverted in 756373da by Junio. We no longer
support merging the right hand side of a fetchspec in a branch's
branch.$name.merge configuration setting as we interpret these
names as being only those published by the remote we are going to
fetch from.
The older shell based implementation of git-fetch did not report an
error when branch.$name.merge was referencing a branch that does
not exist on the remote and we are running `git fetch` for the
current branch. The new builtin-fetch does notice this failure
and aborts the fetch, thus breaking the tests.
Junio and I kicked it around on #git earlier today and decided that
the best approach here is to error out and tell the user that their
configuration is wrong, as this is likely more user friendly than
silently ignoring the user's request. Since the new builtin-fetch
is already issuing the error there is no code change required, we
just need to remove the bad configuration from our test.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
t/t5515-fetch-merge-logic.sh | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh
index 6c9cc67..31c1081 100755
--- a/t/t5515-fetch-merge-logic.sh
+++ b/t/t5515-fetch-merge-logic.sh
@@ -84,8 +84,7 @@ test_expect_success setup '
git config branch.br-$remote-merge.merge refs/heads/three &&
git config branch.br-$remote-octopus.remote $remote &&
git config branch.br-$remote-octopus.merge refs/heads/one &&
- git config --add branch.br-$remote-octopus.merge two &&
- git config --add branch.br-$remote-octopus.merge remotes/rem/three
+ git config --add branch.br-$remote-octopus.merge two
done
'
--
1.5.3.1.1000.g7319b
^ permalink raw reply related
* [PATCH 1/5] builtin-fetch: Don't segfault on "fetch +foo"
From: Shawn O. Pearce @ 2007-09-18 8:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
If we are fetching something and were configured to do a forced
fetch and have no local ref to store the fetched object into we
cannot mark the local ref as having a forced update. Instead we
should just silently discard the + request.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
remote.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/remote.c b/remote.c
index 73a34c9..af3c46b 100644
--- a/remote.c
+++ b/remote.c
@@ -875,8 +875,7 @@ int get_fetch_map(struct ref *remote_refs,
refspec->src : "HEAD");
ref_map->peer_ref = get_local_ref(refspec->dst);
-
- if (refspec->force)
+ if (ref_map->peer_ref && refspec->force)
ref_map->peer_ref->force = 1;
}
--
1.5.3.1.1000.g7319b
^ permalink raw reply related
* [PATCH 0/5] More builtin-fetch fixes
From: Shawn O. Pearce @ 2007-09-18 8:54 UTC (permalink / raw)
To: Junio C Hamano, Daniel Barkalow; +Cc: git
Yet another round of builtin-fetch fixes. This series fixes the
latest breakage in `pu` for this topic along with an annoying
warning when using the http transport.
The third patch in the series is a behavior change for git-fetch.
The commit message discusses it in detail. Junio and I kicked this
around on #git earlier this morning.
At this point myself and a few other experienced-with-Git coworkers
are running this builtin-fetch "in production" for all daily tasks.
We don't use *everything* that the tool supports as I did not know
about this breakage in branch.$name.merge until tonight when Junio
mentioned it, but for some of the really common cases we are quite
happy with builtin-fetch. Especially its performance as we're
seeing speedups of 25x or more on Cygwin/Windows.
I still believe there's work yet to be done on this topic as I'm
quite sure the transfer.unpackLimit is not being honored. I meant
to look at that tonight but wound up wasting all night and morning on
the 3rd patch of this series. I will try to work on the unpackLimit
issue Tuesday or Wednesday this week.
--
Shawn.
^ permalink raw reply
* Re: diffcore-rename performance mode
From: Jeff King @ 2007-09-18 8:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsl5cwe6p.fsf@gitster.siamese.dyndns.org>
On Tue, Sep 18, 2007 at 01:49:50AM -0700, Junio C Hamano wrote:
> > However, keeping around _just_ the
> > cnt_data caused only about 100M of extra memory consumption (and gave
> > the same performance boost).
>
> That would be an interesting and relatively low-hanging optimization.
OK, I will work up a patch. Is it worth making it configurable? Since it
is a space-time tradeoff, if you are tight on memory, it might actually
hurt performance. However, I have only looked at the numbers for my
massive data set...I can produce memory usage numbers for the kernel,
too.
> I think it was just a hash table with linear overflow (if your
> spot is occupied by somebody else, you look for the next
> available vacant spot -- works only if you do not ever delete
> items from the table) but sorry, I do not recall the rationale
> for picking that data structure. I vaguely recall I did some
> measurement between that and the usual "an array that is indexed
> with a hash value that holds heads of linked lists" and pointer
> chasing appeared quite cache-unfriendly to the point that it
> actually degraded performance, but did not try very hard to
> optimize it.
I thought we were holding counts of hashes, in which case there _is_ no
overflow. We only care if you hit the hash fingerprint or not. But
perhaps I am mistaken...I will have to look more closely at the code.
-Peff
^ permalink raw reply
* Re: [PATCH] git-svnimport: Use separate arguments in the pipe for git-rev-parse
From: Junio C Hamano @ 2007-09-18 8:54 UTC (permalink / raw)
To: Matthias Urlichs; +Cc: Dan Libby, git
In-Reply-To: <20070918074753.GQ31176@kiste.smurf.noris.de>
Matthias Urlichs <smurf@smurf.noris.de> writes:
> Signed-Off-By: Matthias Urlichs <smurf@smurf.noris.de>
> ---
> Please tell me whether that works for you.
>
> Somebody else, preferably its author, can fix git-svn. ;-)
>
> diff --git a/git-svnimport.perl b/git-svnimport.perl
> index d3ad5b9..aa5b3b2 100755
> --- a/git-svnimport.perl
> +++ b/git-svnimport.perl
> @@ -633,7 +633,7 @@ sub commit {
>
> my $rev;
> if($revision > $opt_s and defined $parent) {
> - open(H,"git-rev-parse --verify $parent |");
> + open(H,'-|',"git-rev-parse","--verify",$parent);
> $rev = <H>;
> close(H) or do {
> print STDERR "$revision: cannot find commit '$parent'!\n";
I seem to be missing the context, but please describe what
problem this fixes in the commit log message. I guess some
people use shell metacharacters and/or SP in their branch names
and this is about that problem?
^ permalink raw reply
* Re: diffcore-rename performance mode
From: Junio C Hamano @ 2007-09-18 8:49 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20070918082321.GA9883@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> There seems to be a serious performance problem in diffcore-rename.
> There is infrastructure to cache the "cnt_data" member of each filespec,
> but it never gets used because we immediately free the filespec data
> after use. Oops.
>
> With this patch:
> ...
> My 20-minute diff becomes a 2-minute diff. The downside is that the
> memory usage is much increased (for obvious reasons, it should increase
> by the dataset size, since we are keeping pointers to the data around --
> in my case, around 1G extra).
Yes, these early freeing of filespec_data() were introducd later
specifically to address the memory usage issue.
> However, keeping around _just_ the
> cnt_data caused only about 100M of extra memory consumption (and gave
> the same performance boost).
That would be an interesting and relatively low-hanging optimization.
> The spanhash data structure is a bit confusing. At first, it looked like
> we were doing a linear search for a matching hash, but it's not quite,
> since we seem to start at some magic spot based on the hashval we're
> looking up.
I think it was just a hash table with linear overflow (if your
spot is occupied by somebody else, you look for the next
available vacant spot -- works only if you do not ever delete
items from the table) but sorry, I do not recall the rationale
for picking that data structure. I vaguely recall I did some
measurement between that and the usual "an array that is indexed
with a hash value that holds heads of linked lists" and pointer
chasing appeared quite cache-unfriendly to the point that it
actually degraded performance, but did not try very hard to
optimize it.
^ permalink raw reply
* diffcore-rename performance mode
From: Jeff King @ 2007-09-18 8:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
[this is a resend of some comments I made deep in another thread on
rename limiting; I wanted to get your comments, Junio, but I was afraid
you didn't see it buried in the mess. If you have already read it and
have nothing to say, just tell me to shut up.
I was able to get 1000% speedup on a (perhaps pathological) diff, and I
suspect there may be more speedups possible in the spanhash lookups. So
I think it's worth pursuing.]
Hmm. Actually, doing some profiling, it looks like about 75% of the time
is spent not in the O(n^2) comparison, but in generating the hash
fingerprints of each file.
There seems to be a serious performance problem in diffcore-rename.
There is infrastructure to cache the "cnt_data" member of each filespec,
but it never gets used because we immediately free the filespec data
after use. Oops.
With this patch:
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 6bde439..531a844 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -362,10 +362,7 @@ void diffcore_rename(struct diff_options *options)
m->score = estimate_similarity(one, two,
minimum_score);
m->name_score = basename_same(one, two);
- diff_free_filespec_data(one);
}
- /* We do not need the text anymore */
- diff_free_filespec_data(two);
dst_cnt++;
}
/* cost matrix sorted by most to least similar pair */
My 20-minute diff becomes a 2-minute diff. The downside is that the
memory usage is much increased (for obvious reasons, it should increase
by the dataset size, since we are keeping pointers to the data around --
in my case, around 1G extra). However, keeping around _just_ the
cnt_data caused only about 100M of extra memory consumption (and gave
the same performance boost).
Of course, 2 minutes for a git-status is still way too slow, so there we
might still need a limiter. It also looks like 57% of my time is spent
in spanhash_find, and another 29% in diffcore_count_changes.
The spanhash data structure is a bit confusing. At first, it looked like
we were doing a linear search for a matching hash, but it's not quite,
since we seem to start at some magic spot based on the hashval we're
looking up.
But it seems to be an array of (hash, count) pairs for each file. Is
there a reason not to use a hash table mapping hash -> count? That would
make insertion and lookup O(1), presumably at the cost of a bit more
memory (since each filespec would have the full table).
Junio, can you shed some light on that decision?
-Peff
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox