* Re: What's cooking in git.git (Dec 2010, #01; Sat, 4)
From: Junio C Hamano @ 2010-12-05 20:23 UTC (permalink / raw)
To: Yann Dirson; +Cc: git
In-Reply-To: <20101205101333.GA7466@home.lan>
Yann Dirson <ydirson@free.fr> writes:
> On Sat, Dec 04, 2010 at 10:30:21PM -0800, Junio C Hamano wrote:
>> * kb/diff-C-M-synonym (2010-11-29) 1 commit
>> - diff: add --detect-copies-harder as a synonym for --find-copies-harder
>>
>> Will merge to 'next' soon.
>
> If we go this way, don't we want to deprecate --find-copies-harder as well ?
Why?
We are being nice to people who did not know --find-copies-harder but
learned the --detect-renames long name before learning it, which by
definition is are people because the long names have been there only for
the last few months; they may expect "detect" to work there. That is the
sole purpose of the additional synonym.
I do not think we would get much benefit from panalizing people who knew
about and have used the --find-copies-harder option for long time by
marking it to be deprecated.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2010, #01; Sat, 4)
From: Junio C Hamano @ 2010-12-05 20:15 UTC (permalink / raw)
To: kusmabite; +Cc: git
In-Reply-To: <AANLkTimX215CNAG_mosqzOh-EPBoSimK0s-n4YmKm1rb@mail.gmail.com>
Erik Faye-Lund <kusmabite@gmail.com> writes:
>> Reroll, or squash?
>
> I was planning on re-rolling, but looking at it with fresh eyes it
> doesn't seem like there's much useful I can do. So feel free to just
> squash it. I think the original commit message still makes sense.
Thanks.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2010, #01; Sat, 4)
From: Junio C Hamano @ 2010-12-05 20:06 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <4CFBA912.2080905@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> Junio C Hamano venit, vidit, dixit 05.12.2010 07:30:
> ...
>>
>> * mg/cvsimport (2010-11-28) 3 commits
>> - cvsimport.txt: document the mapping between config and options
>> - cvsimport: fix the parsing of uppercase config options
>> - cvsimport: partial whitespace cleanup
>>
>> I was being lazy and said "Ok" to "cvsimport.capital-r" but luckily other
>> people injected sanity to the discussion. Weatherbaloon patch sent, but
>
> I assume I should try and not read too much into this...
No, you shouldn't. I wasn't questioning _your_ sanity, and if you took it
that way, I apologize.
I as the maintainer have different priority from contributors. The
contributed patches want to "get the job done" first, and their solution
only need to be "correct and not too ugly".
I however in addition need to make sure that the changes make sense in the
longer term, and my saying "Ok" to "capital-r" instead of rejecting the
patch totally went against that obligation of mine. I'll have a hard time
justifying why users need to type that long string that does not convey
much information in three months. It was _my_ temporary insanity that
came from fatigue.
So an weatherbaloon patch was sent as a counterproposal, which I think it
shows the best course of action given the existing constraints, but there
may be better ones. If you agree, and if you have time and inclination, a
reroll in that direction would be appreciated. And it does not have to be
you but others on the list.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2010, #01; Sat, 4)
From: Junio C Hamano @ 2010-12-05 19:54 UTC (permalink / raw)
To: Jeff King; +Cc: Aleksi Aalto, git
In-Reply-To: <20101205073938.GB15233@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sat, Dec 04, 2010 at 10:30:21PM -0800, Junio C Hamano wrote:
>
>> * aa/status-hilite-branch (2010-11-18) 1 commit
>> - status: show branchname with a configurable color
>>
>> I am indifferent/uninterested; I don't see anything wrong with it, but I
>> do not find coloring the field particularly useful myself.
>
> I am not particularly interested, either, but FWIW, the gitcommit syntax
> highlighting that ships with vim does highlight this, so there are at
> least other people who think this is a good idea.
As you already know, when I say "'Meh' personally", I am not saying "I
want to forbid others to want it".
How does vim highlight the other parts of that particular line? Does it
keep them intact, or paint them in some other color?
> However, I'm not sure about the default. The original patch defaulted to
> magenta. Your fixup defaults to "plain", but that is a regression
> (albeit a minor one) for people who have status.header set.
This patch is a regression for them either way, isn't it? Except for
those who chose to use magenta to paint status.header, that is.
I had this suspicion that the class of people who choose a non default
status.header color and the class of people who choose plain there (or
have been happy with the default) expect different things. The former
prefer louder output, different pieces of information painted in different
colors to help them chromatically distinguish them. The latter (including
myself) favor subdued output, without too many colors distacting them
while reading the output.
This suspicion further led me to think that the former would want this new
feature to paint the branch name in a color different from status.header
color, while the latter would want it in plain. So the default of "plain"
would be a win for both audiences.
Another thiking behind "plain" is that it avoids using "magenta" which we
didn't use in the default colored output from this command. We have been
trying to make the default coloring not too loud, and keeping the number
of colors used low has been one way of doing so.
^ permalink raw reply
* [PATCH] git-filter-branch.txt: (Checklist for Shrinking): Adjust commands and options
From: jari.aalto @ 2010-12-05 19:14 UTC (permalink / raw)
To: git; +Cc: Jari Aalto
From: Jari Aalto <jari.aalto@cante.net>
Add a little more explanation to bullet that talks about cryptic
--tag-name-filter options. Suggest also option --expire-unreachable=0 in
"git reflog" call. Add option --aggressive to garbage collect call.
Signed-off-by: Jari Aalto <jari.aalto@cante.net>
---
Documentation/git-filter-branch.txt | 16 +++++++++-------
1 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index 796e748..180fdaa 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -380,8 +380,10 @@ objects until you tell it to. First make sure that:
over its lifetime. `git log \--name-only \--follow \--all \--
filename` can help you find renames.
-* You really filtered all refs: use `\--tag-name-filter cat \--
- \--all` when calling git-filter-branch.
+* You really filtered all refs. Without the following, you could have
+ some branches or tags still pointing to the commits before
+ filtering:
+ `\--tag-name-filter cat \-- \--all`
Then there are two ways to get a smaller repository. A safer way is
to clone, that keeps your original intact.
@@ -399,12 +401,12 @@ warned.
for-each-ref \--format="%(refname)" refs/original/ | xargs -n 1 git
update-ref -d`.
-* Expire all reflogs with `git reflog expire \--expire=now \--all`.
-
-* Garbage collect all unreferenced objects with `git gc \--prune=now`
- (or if your git-gc is not new enough to support arguments to
- `\--prune`, use `git repack -ad; git prune` instead).
+* Expire all reflogs with `git reflog expire \--expire=now \--all
+ \--expire-unreachable=0`.
+* Garbage collect all unreferenced objects with `git gc \--aggressive
+ \--prune=now`. Or if your git-gc is not new enough to support
+ arguments to `\--prune`, use `git repack -ad; git prune` instead.
Author
------
--
1.7.2.3
^ permalink raw reply related
* Re: [PATCH 01/10] parse-options: Don't call parse_options_check() so much
From: René Scharfe @ 2010-12-05 18:14 UTC (permalink / raw)
To: Jonathan Nieder
Cc: git, Stephen Boyd, Nguyễn Thái Ngọc Duy,
Pierre Habouzit
In-Reply-To: <20101201232852.GB31815@burratino>
Am 02.12.2010 00:28, schrieb Jonathan Nieder:
> From: Stephen Boyd <bebarino@gmail.com>
>
> parse_options_check() is being called for each invocation of
> parse_options_step() which can be quite a bit for some commands. The
> commit introducing this function cb9d398 (parse-options: add
> parse_options_check to validate option specs., 2009-06-09) had the
> correct motivation and explicitly states that parse_options_check()
> should be called from parse_options_start(). However, the implementation
> differs from the motivation. Fix it.
Good idea.
> void parse_options_start(struct parse_opt_ctx_t *ctx,
> int argc, const char **argv, const char *prefix,
> - int flags)
> + int flags, const struct option *options)
It might be better to put options before flags, i.e. to use the same
order as in parse_options().
René
^ permalink raw reply
* Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.
From: Thiago Farina @ 2010-12-05 17:32 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git
In-Reply-To: <AANLkTikL4BWtzNgx1+MBYxRRdfL=Gu71KPjaiKXprvnb@mail.gmail.com>
On Sun, Dec 5, 2010 at 3:29 PM, Thiago Farina <tfransosi@gmail.com> wrote:
> On Sun, Dec 5, 2010 at 3:09 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> - "At my office there is a style guide indicating that each function
>> should live in a module with some other functions and be named to
>> indicate so (like perf, with its sched__* etc functions). The idea
>> is that code with a simple high-level structure tends to be easier
>> to understand and we need to understand the code we use. Can we
>> start changing the code to fit this style guide, so there is less
>> resistance to using it at my office?"
>>
> For me that is a good reason and I think it matches with what I had in
> mind but didn't write. Thanks for pointing it out.
>
Also I thought that as Junio already picked up the other patch. It's
was a hint that the other functions that has "struct commit_list *l"
as its parameters could be renamed as well. But I was wrong it seems.
^ permalink raw reply
* Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.
From: Thiago Farina @ 2010-12-05 17:29 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git
In-Reply-To: <20101205170919.GA7913@burratino>
On Sun, Dec 5, 2010 at 3:09 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> - "At my office there is a style guide indicating that each function
> should live in a module with some other functions and be named to
> indicate so (like perf, with its sched__* etc functions). The idea
> is that code with a simple high-level structure tends to be easier
> to understand and we need to understand the code we use. Can we
> start changing the code to fit this style guide, so there is less
> resistance to using it at my office?"
>
For me that is a good reason and I think it matches with what I had in
mind but didn't write. Thanks for pointing it out.
^ permalink raw reply
* Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.
From: Jonathan Nieder @ 2010-12-05 17:09 UTC (permalink / raw)
To: Thiago Farina; +Cc: git
In-Reply-To: <AANLkTinAT3kotKQTS6eS1SLigNzSp6grAU7WNRbHf3N=@mail.gmail.com>
Thiago Farina wrote:
> I think it's a good procedure for someone more familiar with this
> functions to do this. Perhaps, you or Junio?
If you are not familiar enough with the functions to document them
(perhaps with help from the list) then yes, renaming them is a bad
idea. I am not inclined to do it because I like the current name.
The ideal patch is a great sort of present: first a bug report, then
the resolution to that bug. When the patch proper goes awry, at least
there is the bug report. I think you are trying to convey a bug but
you haven't explained it. Maybe it is
- "The reduce_heads function being used in various contexts, where it
is not obvious what it means. If you add commit_list to the name,
then <such and such> becomes obvious. So I suggest renaming." or
- "In my program, I have my _own_ reduce_heads function with
different meaning so I cannot easily copy the commit_list functions
to use them. Please make it easier by putting commit_list functions
in a well defined namespace." or
- "Some code is manipulating commit_lists directly and violating
their invariants. Please make it easier to build a cheat-sheet
listing commit_list functions, to translate from
bad-field-manipulation-ese to using-the-right-functions-ese." or
- "At my office there is a style guide indicating that each function
should live in a module with some other functions and be named to
indicate so (like perf, with its sched__* etc functions). The idea
is that code with a simple high-level structure tends to be easier
to understand and we need to understand the code we use. Can we
start changing the code to fit this style guide, so there is less
resistance to using it at my office?"
In a way, these are straw men; sorry about that. The answer to each
would be different. FWIW from my pov the answer to _none_ of these
would be "sure, let's rename the functions", for different reasons in
each case.
I do not think this is an atypical example at all. I would have
prefered not to spend time on patches that require guessing what
problem is being solved.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2010, #01; Sat, 4)
From: Patrick Rouleau @ 2010-12-05 15:51 UTC (permalink / raw)
To: git
In-Reply-To: <alpine.SOC.2.00.1012051420230.18993@kekkonen.cs.hut.fi>
<aleksi.aalto <at> iki.fi> writes:
>
> On Sat, 4 Dec 2010, Junio C Hamano wrote:
> > * aa/status-hilite-branch (2010-11-18) 1 commit
> > - status: show branchname with a configurable color
> >
> > I am indifferent/uninterested; I don't see anything wrong with it, but I
> > do not find coloring the field particularly useful myself.
>
> The idea for this patch came from my daywork, where I have lately been
> trying to teach new users effective use of Git. We promote heavy usage of
> "git status" for new users in order for them to understand what all the
> basic commands do. A great amount of users fail to notice in which branch
> they are even when looking at the status message. I think this small tweak
> could help at least some of such new users without causing considerable
> harm for more advanced users.
I see the same problem at my daywork.
But like others, I'm not sure about the default color. I would opt for green
which is the default used by the branch command for highlight the current branch.
Maybe we can add an empty line after the branch name, to make it stand out a bit
more, with or without color?
^ permalink raw reply
* Missing tags v1.0.1 and v1.0.2 on kernel.org
From: Yann Dirson @ 2010-12-05 15:04 UTC (permalink / raw)
To: GIT list
When fetching from git://github.com/gitster/git.git in my local repo
originally cloned from git://git.kernel.org/pub/scm/git/git.git, I
notice 2 unexpected new tags: v1.0.1 and v1.0.2.
It looks like those 2 old tags were not pushed to kernel.org:
$ git ls-remote origin|grep '1\.0\.[0-4]$'
f665776185ad074b236c00751d666da7d1977dbe refs/tags/v1.0.0
ccefd6e899a861f911052e47eabe4f77c09163ec refs/tags/v1.0.3
1e6455d0c3f9ee79c461bd68aafad8b7f8765c8a refs/tags/v1.0.4
OTOH there are v1.0.0a and v1.0.0b on kernel.org, which match the same
revs:
$ git rev-parse v1.0.1^{}
e4e79a217576d24ef4d73b620766f62b155bcd98
$ git rev-parse v1.0.0a^{}
e4e79a217576d24ef4d73b620766f62b155bcd98
$ git rev-parse v1.0.2^{}
8d712aafd2df3c1f5147a28947f98cefe667cf76
$ git rev-parse v1.0.0b^{}
8d712aafd2df3c1f5147a28947f98cefe667cf76
Don't we want those 2 tags on kernel.org too for consistency ?
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2010, #01; Sat, 4)
From: Michael J Gruber @ 2010-12-05 15:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7voc90wx36.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 05.12.2010 07:30:
...
>
> * mg/cvsimport (2010-11-28) 3 commits
> - cvsimport.txt: document the mapping between config and options
> - cvsimport: fix the parsing of uppercase config options
> - cvsimport: partial whitespace cleanup
>
> I was being lazy and said "Ok" to "cvsimport.capital-r" but luckily other
> people injected sanity to the discussion. Weatherbaloon patch sent, but
I assume I should try and not read too much into this...
Michael
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2010, #01; Sat, 4)
From: Erik Faye-Lund @ 2010-12-05 13:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7voc90wx36.fsf@alter.siamese.dyndns.org>
On Sun, Dec 5, 2010 at 7:30 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Here are the topics that have been cooking. Commits prefixed with '-' are
> only in 'pu' while commits prefixed with '+' are in 'next'. The ones
> marked with '.' do not appear in any of the integration branches, but I am
> still holding onto them.
>
> --------------------------------------------------
> [New Topics]
>
> * aa/status-hilite-branch (2010-11-18) 1 commit
> - status: show branchname with a configurable color
>
> I am indifferent/uninterested; I don't see anything wrong with it, but I
> do not find coloring the field particularly useful myself.
>
> * ef/help-cmd-prefix (2010-11-26) 2 commits
> - (jc) review comments
> - help: always suggest common-cmds if prefix of cmd
>
> Reroll, or squash?
I was planning on re-rolling, but looking at it with fresh eyes it
doesn't seem like there's much useful I can do. So feel free to just
squash it. I think the original commit message still makes sense.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2010, #01; Sat, 4)
From: aleksi.aalto @ 2010-12-05 12:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7voc90wx36.fsf@alter.siamese.dyndns.org>
On Sat, 4 Dec 2010, Junio C Hamano wrote:
> * aa/status-hilite-branch (2010-11-18) 1 commit
> - status: show branchname with a configurable color
>
> I am indifferent/uninterested; I don't see anything wrong with it, but I
> do not find coloring the field particularly useful myself.
The idea for this patch came from my daywork, where I have lately been
trying to teach new users effective use of Git. We promote heavy usage of
"git status" for new users in order for them to understand what all the
basic commands do. A great amount of users fail to notice in which branch
they are even when looking at the status message. I think this small tweak
could help at least some of such new users without causing considerable
harm for more advanced users.
And as Jeff pointed out, this is already the default beharivour in vim for
commit messages. I have always found it quite reasonable.
:Aga
^ permalink raw reply
* Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.
From: Thiago Farina @ 2010-12-05 12:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsjycx05o.fsf@alter.siamese.dyndns.org>
On Sun, Dec 5, 2010 at 3:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Thiago Farina <tfransosi@gmail.com> writes:
>
>> Signed-off-by: Thiago Farina <tfransosi@gmail.com>
>
> I really do not like this.
>
I don't feel very strong about it. And as I learned from Jonathan, I
don't care if you will take or not. I think my intention was good, but
I can't please everybody
I was just trying to put commit_list in a better shape and resemble it
in a more explicit API.
> The use of type "struct commit_list" to hold the set of parent commits is
> incidental; if we had "struct commit_set", we would have written a
> function with the same purpose, and named it the same "reduce_HEADS".
>
> Adding commit_list to the name makes the code harder to read (and type)
> with little added benefit. "LIST"-ness is not the important part.
>
> If a function takes a commit_list, named "reduce_HEADS",
What? reduce_HEADS ? HEADS with CAPSLOCK?
^ permalink raw reply
* Re: [PATCH] commit: Add commit_list prefix to reduce_heads function.
From: Thiago Farina @ 2010-12-05 12:18 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git
In-Reply-To: <20101205021837.GA24614@burratino>
On Sun, Dec 5, 2010 at 12:18 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Thiago Farina wrote:
>
>> Signed-off-by: Thiago Farina <tfransosi@gmail.com>
>
> I know that the context is part of an effort to make the commit_list
> functions into something more of a self-contained API, but the
> reader does not know that. Perhaps you could say some words about
> that in the change description: what's wrong with the current
> situation, what context does this change come from, and what positive
> effect would it have?
>
> Beyond that, I must say I do not think this goes far enough to seem
> useful. If I wondered what reduce_heads did, wouldn't
> commit_list_reduce_heads be even more confusing? (ignoring the typo)
>
> Perhaps a more natural way to proceed would be as follows:
>
> . first, collect the functions to be treated as a module and
> list them in Documentation/technical (in this case, perhaps
> api-revision-walking or a new api-commit-list)
>
What you want here? That I describe the functions in these files? Why
me? Why not the person who wrote them?
> . next, describe their current meaning. If this requires
> apologizing for the name,
Apologize? For what? I don't understand what you mean here.
> that's a good hint that a name change might be worthwhile
>
> . finally, tweak signatures (names and arguments) based on the
> results from step 2 and update the documentation at the same
> time.
>
I'd prefer to do just that step.
> That way, people used to the current functions would at least have
> some documentation to help them adjust. What do you think?
>
I think it's a good procedure for someone more familiar with this
functions to do this. Perhaps, you or Junio?
^ permalink raw reply
* Re: [PATCH v5] generalizing sorted-array handling
From: Yann Dirson @ 2010-12-05 12:02 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git
In-Reply-To: <20101205104426.GG4332@burratino>
On Sun, Dec 05, 2010 at 04:44:26AM -0600, Jonathan Nieder wrote:
> Yann Dirson wrote:
>
> > * better API documentation (was previously lacking or plain obsolete)
>
> Thanks! In general I find it is easiest to read and write
> documentation out of line for this sort of thing. That way, even
> after the documentation grows obsolete it doesn't seem so out of
> place.
>
> See Documentation/technical/api-strbuf.txt, api-sigchain, and
> api-allocation-growing for some nice (up-to-date) examples.
OK, can do that.
> In particular:
>
> > * This API is very verbose, and I'm not happy with that aspect.
>
> Could you give a quick stripped-down usage example?
Well, patches 2-5 in the series provide good examples - probably
better seen with the "New version" checkbos in gitk, did not find a
commandline flag equivalent (is there one ?).
Patch 4 is probably the simplest example: we use the new macros to
define the same insert API (except for the "number of element" var,
which used a non-standard naming scheme here). Since the lookup API
was only used inside the insert func, there was no need for a lookup
wrapper here, so we just declare the generic search+ insert funcs, and
an insert wrapper.
---------------------------- builtin/pack-objects.c ----------------------------
index 3cbeb29..887a55c 100644
@@ -16,6 +16,7 @@
#include "list-objects.h"
#include "progress.h"
#include "refs.h"
+#include "sorted-array.h"
#ifndef NO_PTHREADS
#include <pthread.h>
@@ -871,45 +872,23 @@ static void add_pbase_object(struct tree_desc *tree,
}
}
+static int unsigned_cmp(unsigned ref, unsigned *elem)
{
+ if (ref == *elem)
+ return 0;
+ if (ref < *elem)
+ return -1;
+ return 1;
}
+static void unsigned_init(unsigned *elem, unsigned ref)
{
+ *elem = ref;
}
+declare_sorted_array(static, unsigned, done_pbase_paths);
+declare_gen_binsearch(static, unsigned, done_pbase_path_pos, unsigned);
+declare_gen_sorted_insert(static, unsigned, _check_pbase_path, done_pbase_path_pos, unsigned);
+declare_sorted_array_insert_checkbool(static, check_pbase_path, unsigned, _check_pbase_path,
+ done_pbase_paths, unsigned_cmp, unsigned_init);
static void add_preferred_base_object(const char *name)
{
@@ -987,7 +966,7 @@ static void cleanup_preferred_base(void)
free(done_pbase_paths);
done_pbase_paths = NULL;
+ done_pbase_paths_nr = done_pbase_paths_alloc = 0;
}
static void check_object(struct object_entry *entry)
----------------------------
Patch 2 is a more complete example, where the oiginal API used a
single function with an additional boolean arg to select the
behaviour. So here we also define a search wrapper, and this make the
callsites more explicit.
------------------------------ diffcore-rename.c ------------------------------
index df41be5..a655017 100644
@@ -5,52 +5,36 @@
#include "diff.h"
#include "diffcore.h"
#include "hash.h"
+#include "sorted-array.h"
/* Table of rename/copy destinations */
+struct diff_rename_dst {
struct diff_filespec *two;
struct diff_filepair *pair;
+};
+static int rename_dst_cmp(struct diff_filespec *ref_spec, struct diff_rename_dst *elem)
{
+ return strcmp(ref_spec->path, elem->two->path);
+}
+static void rename_dst_init(struct diff_rename_dst *elem, struct diff_filespec *ref_spec)
+{
+ elem->two = alloc_filespec(ref_spec->path);
+ fill_filespec(elem->two, ref_spec->sha1, ref_spec->mode);
+ elem->pair = NULL;
}
+declare_sorted_array(static, struct diff_rename_dst, rename_dst);
+declare_gen_binsearch(static, struct diff_rename_dst, _locate_rename_dst,
+ struct diff_filespec *);
+declare_sorted_array_search_elem(static, struct diff_rename_dst, locate_rename_dst,
+ struct diff_filespec *, _locate_rename_dst,
+ rename_dst, rename_dst_cmp);
+declare_gen_sorted_insert(static, struct diff_rename_dst, _register_rename_dst,
+ _locate_rename_dst, struct diff_filespec *);
+declare_sorted_array_insert_checkbool(static, register_rename_dst, struct diff_filespec *,
+ _register_rename_dst,
+ rename_dst, rename_dst_cmp, rename_dst_init);
/* Table of rename/copy src files */
static struct diff_rename_src {
@@ -437,7 +421,7 @@ void diffcore_rename(struct diff_options *options)
strcmp(options->single_follow, p->two->path))
continue; /* not interested */
else
+ register_rename_dst(p->two);
}
else if (!DIFF_FILE_VALID(p->two)) {
/*
@@ -582,7 +566,7 @@ void diffcore_rename(struct diff_options *options)
* not been turned into a rename/copy already.
*/
struct diff_rename_dst *dst =
+ locate_rename_dst(p->two);
if (dst && dst->pair) {
diff_q(&outq, dst->pair);
pair_to_free = p;
@@ -613,7 +597,7 @@ void diffcore_rename(struct diff_options *options)
if (DIFF_PAIR_BROKEN(p)) {
/* broken delete */
struct diff_rename_dst *dst =
+ locate_rename_dst(p->one);
if (dst && dst->pair)
/* counterpart is now rename/copy */
pair_to_free = p;
------------------------------
> [...]
> > Adding "simple" API variants that would call all the necessary stuff
> > would help code readability, but adding yet more entry points seems a
> > dubious approach.
>
> On the contrary, simple API variants don't sound so bad to me,
> once the fundamentals are in good shape.
The problem is with the number of combinations. We already have
potentially 6 wrappers (not all of which are defined yet), with:
* operation: search | insert
* return-value semantic: check | checkbool | elem
If we add to these basic building-blocks:
* wrapper variants that declare the generic func: we double the count
* insert-wrapper variants that declare the generic search: *1.5
... which gives something like 18 wrappers. And this number will
still raise by 6 each time we feel the need for a new return-value
semantic.
^ permalink raw reply
* Status of the svn remote helper project (Dec 2010, #1)
From: Jonathan Nieder @ 2010-12-05 11:37 UTC (permalink / raw)
To: git
Cc: Ramkumar Ramachandra, Sverre Rabbelier, David Barr, Sam Vilain,
Stephen Bash, Tomas Carnecky
In-Reply-To: <20101121063149.GA15449@burratino>
It's been a couple of months since I mentioned that it would be
possible for a simple svn remote helper on top of svn-fe to be usable
in a couple of weeks. I still believe a read-only importer is _just_
within grasp; but there are plenty of tempting and interesting topics
to work on in the stack below that, too.
That said, the collection of topics cooking is already satisfying.
Time for cleaning up additional branches for the tree seems short;
patch series and branches that are ready for wider testing as-is would
be fantastic (and should be based on a merge of 'master' and as few of
the topics listed below as possible).
Excluding changes taken from the 'jch' branch, relative to last time
we have
Makefile | 4 +-
contrib/svn-fe/svn-filter-root.py | 107 ++++++++++++++++++++++++
contrib/svn-fe/testme.sh | 4 +-
fast-import.c | 164 ++++++++++++++++++++++++++++++++++---
t/t9011-svn-da.sh | 10 +-
t/t9300-fast-import.sh | 6 +-
test-treap.c | 11 ++-
vcs-svn/repo_tree.c | 4 +-
vcs-svn/sliding_window.c | 11 ++-
vcs-svn/trp.h | 3 +-
vcs-svn/trp.txt | 10 ++-
11 files changed, 298 insertions(+), 36 deletions(-)
The added fast-import code implements the 'ls' command.
Most of the changes are one-liners and small cleanups. The most
visible change in practice should be the fix for the mysterious
historical-files-appearing-as-directories bug in repo_tree.
As always, a merge of the branches listed below is available as
git://repo.or.cz/git/jrn.git vcs-svn-pu
and individual topic branches are also available in that repository
under the refs/topics namespace.
--------------------------------------------------
[Cooking]
* jn/svndiff0 (2010-11-06) 24 commits
- vcs-svn: Allow deltas to copy from preimage
- vcs-svn: Reject deltas that read past end of preimage
- vcs-svn: Let deltas use data from postimage
- vcs-svn: Reject deltas that do not consume all inline data
- vcs-svn: Check declared number of output bytes
- vcs-svn: Implement copyfrom_data delta instruction
- vcs-svn: Read instructions from deltas
- vcs-svn: Read inline data from deltas
- vcs-svn: Read the preimage while applying deltas
- vcs-svn: Skeleton of an svn delta parser
- compat: helper for detecting unsigned overflow
- vcs-svn: Learn to check for SVN\0 magic
- vcs-svn: Learn to parse variable-length integers
- vcs-svn: Add code to maintain a sliding view of a file
- vcs-svn: Allow character-oriented input
- vcs-svn: Allow input errors to be detected early
- vcs-svn: Let callers peek ahead to find stream end
- vcs-svn: Add binary-safe read() function
- vcs-svn: Improve support for reading large files
- vcs-svn: Make buffer_skip_bytes() report partial reads
- vcs-svn: Teach line_buffer to handle multiple input files
- vcs-svn: Collect line_buffer data in a struct
- vcs-svn: Replace buffer_read_string() memory pool with a strbuf
- vcs-svn: Eliminate global byte_buffer[] array
Well tested and should be ready for wide use.
Is there some tool (a hex editor?) that can be used to easily read
and write deltas? The "printf and test against 'svnadmin load'"
method is a bit time confusing.
* db/recognize-v3 (2010-11-20) 2 commits
- vcs-svn: Allow simple v3 dumps (no deltas yet)
- vcs-svn: Error out for v3 dumps
A bugfix and the framework for a feature. Probably the bugfix
should be pushed toward 'maint'.
* db/prop-delta (2010-11-20) 16 commits
- vcs-svn: Simplify handling of deleted properties
- vcs-svn: Implement Prop-delta handling
- vcs-svn: Sharpen parsing of property lines
- vcs-svn: Split off function for handling of individual properties
- vcs-svn: Make source easier to read on small screens
- vcs-svn: More dump format sanity checks
- vcs-svn: Reject path nodes without Node-action
- vcs-svn: Delay read of per-path properties
- vcs-svn: Combine repo_replace and repo_modify functions
- vcs-svn: Replace = Delete + Add
- vcs-svn: handle_node: Handle deletion case early
- vcs-svn: Use mark to indicate nodes with included text
- vcs-svn: Unclutter handle_node by introducing have_props var
- vcs-svn: Eliminate node_ctx.mark global
- vcs-svn: Eliminate node_ctx.srcRev global
- vcs-svn: Check for errors from open()
(this branch uses db/recognize-v3.)
All but the tip commit are from 'pu'.
* db/fast-import-blob-access (2010-12-04) 5 commits
- fast-import: add 'ls' command
- fast-import: Allow cat-blob requests at arbitrary points in stream
- fast-import: let importers retrieve blobs
- fast-import: clarify documentation of "feature" command
- fast-import: stricter parsing of integer options
A proof of concept for the protocol targetting another repository
format (hg?) would be a great comfort.
Synchronization overhead seems to be a big problem. If someone can
use "top -b" output to produce a nice timechart explaining this then
I would be happy to take a look.
* db/fast-import-object-reuse (2010-11-24) 1 commit
- fast-import: insert new object entries at start of hash bucket
A speedup. Acked by Shawn, taken from 'pu'.
* jn/fast-import-ondemand-checkpoint (2010-11-24) 1 commit
- fast-import: treat SIGUSR1 as a request to access objects early
Taken from 'pu'.
* jn/svn-fe-makefile (2010-12-04) 1 commit
- Makefile: dependencies for vcs-svn tests
Seems to have been forgotten? Simplified commit message, otherwise
unchanged.
* xx/thinner-wrapper-svndiff0 (2010-11-07) 1 commit
- svn-fe: stop linking to libz and libxdiff
(this branch uses jn/svndiff0 and jn/thinner-wrapper.)
Simplification. jn/thinner-wrapper is part of 'jch'.
* rr/svnfe-tests-no-perl (2010-11-23) 1 commit
- t9010 (svn-fe): Eliminate dependency on svn perl bindings
>From 'pu'.
* db/text-delta (2010-11-20) 10 commits
- fixup! vcs-svn: tweak sliding window code to tolerate excessive
readahead
- fixup! svn-fe: Test script for handling of dumps with --deltas
- svn-fe: Test script for handling of dumps with --deltas
- vcs-svn: Implement text-delta handling
- vcs-svn: Teach line_buffer about temporary files
- vcs-svn: tweak sliding window code to tolerate excessive readahead
- vcs-svn: Let caller set up sliding window for delta preimage
- vcs-svn: Read delta preimage from file descriptor
- vcs-svn: Introduce fd_buffer routines
- vcs-svn: Introduce repo_read_path to check the content at a path
- vcs-svn: Internal fast_export_save_blob helper
- Merge branch 'db/fast-import-blob-access' (early part) into
db/text-delta
- Merge branch 'jn/svndiff0' into db/text-delta
(this branch uses db/recognize-v3, db/prop-delta, db/fast-import-blob-access,
and jn/svndiff0.)
It works!
* db/svn-extract-branches (2010-11-20) 1 commit
- svn-fe: Script to remap svn history
Very rough but let's merge it so it doesn't get forgotten.
* jn/maint-svn-fe (2010-12-05) 2 commits
- vcs-svn: fix intermittent repo_tree corruption
- treap: make treap_insert return inserted node
Fixes an old bug. Hoping for feedback or an ack from someone familiar
svn-fe internals; afterwards, would fast-track to maint.
---------------------------------------------------
[Graduated]
* jn/thinner-wrapper (2010-11-06) 7 commits
- Remove pack file handling dependency from wrapper.o
- pack-objects: mark file-local variable static
- wrapper: give zlib wrappers their own translation unit
- strbuf: move strbuf_branchname to sha1_name.c
- path helpers: move git_mkstemp* to wrapper.c
- wrapper: move odb_* to environment.c
- wrapper: move xmmap() to sha1_file.c
Part of 'jch' now.
--------------------------------------------------
[Out of tree, stalled]
* tc/remote-helper-usability: $gmane/157860
. Register new packs after the remote helper is done fetching
. Properly record history of the notes ref
. Fix ls-remote output when displaying impure refs
. Add git-remote-svn
. Introduce the git fast-import-helper
. Rename get_mode() to decode_tree_mode() and export it
. Allow the transport fetch command to add additional refs
. Allow more than one keepfile in the transport
. Remote helper: accept ':<value> <name>' as a response to 'list'
The fourth-from-top seems a bit hard to review. If it really is
necessary to introduce a separate program with a separate interface,
maybe a compile-time flag to choose between them would help?
* rr/remote-helper: http://github.com/artagnon/git
. remote-svn: Write in fetch functionality
. run-command: Protect the FD 3 from being grabbed
. remote-svn: Build a pipeline for the import using svnrdump
. run-command: Extend child_process to include a backchannel FD
. Allow the transport fetch command to add additional refs
. Remote helper: accept ':<value> <name>' as a response to 'list'
. test-svn-fe: Allow for a dumpfile on stdin
. contrib/svn-fe: Fast script to remap svn history
. Add Tom's remote helper for reference
. Add a stubby remote-svn remote helper
. Add a correct svndiff applier
Work in progress, waiting on lower levels to be more functional
(in particular, svn-fe does not support incremental imports yet).
* sb/svn-fe-example: $gmane/159054
^ permalink raw reply
* Re: [PATCH v5] generalizing sorted-array handling
From: Jonathan Nieder @ 2010-12-05 10:44 UTC (permalink / raw)
To: Yann Dirson; +Cc: git
In-Reply-To: <1291545247-4151-1-git-send-email-ydirson@altern.org>
Yann Dirson wrote:
> * better API documentation (was previously lacking or plain obsolete)
Thanks! In general I find it is easiest to read and write
documentation out of line for this sort of thing. That way, even
after the documentation grows obsolete it doesn't seem so out of
place.
See Documentation/technical/api-strbuf.txt, api-sigchain, and
api-allocation-growing for some nice (up-to-date) examples.
In particular:
> * This API is very verbose, and I'm not happy with that aspect.
Could you give a quick stripped-down usage example?
[...]
> Adding "simple" API variants that would call all the necessary stuff
> would help code readability, but adding yet more entry points seems a
> dubious approach.
On the contrary, simple API variants don't sound so bad to me,
once the fundamentals are in good shape.
^ permalink raw reply
* [PATCH 2/6] Convert diffcore-rename's rename_dst to the new sorted-array API.
From: Yann Dirson @ 2010-12-05 10:34 UTC (permalink / raw)
To: git; +Cc: Yann Dirson
In-Reply-To: <1291545247-4151-1-git-send-email-ydirson@altern.org>
The sorted-array API splits search and insert into two separated
functions, which makes the caller code more clear.
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
diffcore-rename.c | 66 ++++++++++++++++++++---------------------------------
1 files changed, 25 insertions(+), 41 deletions(-)
diff --git a/diffcore-rename.c b/diffcore-rename.c
index df41be5..a655017 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -5,52 +5,36 @@
#include "diff.h"
#include "diffcore.h"
#include "hash.h"
+#include "sorted-array.h"
/* Table of rename/copy destinations */
-static struct diff_rename_dst {
+struct diff_rename_dst {
struct diff_filespec *two;
struct diff_filepair *pair;
-} *rename_dst;
-static int rename_dst_nr, rename_dst_alloc;
+};
-static struct diff_rename_dst *locate_rename_dst(struct diff_filespec *two,
- int insert_ok)
+static int rename_dst_cmp(struct diff_filespec *ref_spec, struct diff_rename_dst *elem)
{
- int first, last;
-
- first = 0;
- last = rename_dst_nr;
- while (last > first) {
- int next = (last + first) >> 1;
- struct diff_rename_dst *dst = &(rename_dst[next]);
- int cmp = strcmp(two->path, dst->two->path);
- if (!cmp)
- return dst;
- if (cmp < 0) {
- last = next;
- continue;
- }
- first = next+1;
- }
- /* not found */
- if (!insert_ok)
- return NULL;
- /* insert to make it at "first" */
- if (rename_dst_alloc <= rename_dst_nr) {
- rename_dst_alloc = alloc_nr(rename_dst_alloc);
- rename_dst = xrealloc(rename_dst,
- rename_dst_alloc * sizeof(*rename_dst));
- }
- rename_dst_nr++;
- if (first < rename_dst_nr)
- memmove(rename_dst + first + 1, rename_dst + first,
- (rename_dst_nr - first - 1) * sizeof(*rename_dst));
- rename_dst[first].two = alloc_filespec(two->path);
- fill_filespec(rename_dst[first].two, two->sha1, two->mode);
- rename_dst[first].pair = NULL;
- return &(rename_dst[first]);
+ return strcmp(ref_spec->path, elem->two->path);
+}
+static void rename_dst_init(struct diff_rename_dst *elem, struct diff_filespec *ref_spec)
+{
+ elem->two = alloc_filespec(ref_spec->path);
+ fill_filespec(elem->two, ref_spec->sha1, ref_spec->mode);
+ elem->pair = NULL;
}
+declare_sorted_array(static, struct diff_rename_dst, rename_dst);
+declare_gen_binsearch(static, struct diff_rename_dst, _locate_rename_dst,
+ struct diff_filespec *);
+declare_sorted_array_search_elem(static, struct diff_rename_dst, locate_rename_dst,
+ struct diff_filespec *, _locate_rename_dst,
+ rename_dst, rename_dst_cmp);
+declare_gen_sorted_insert(static, struct diff_rename_dst, _register_rename_dst,
+ _locate_rename_dst, struct diff_filespec *);
+declare_sorted_array_insert_checkbool(static, register_rename_dst, struct diff_filespec *,
+ _register_rename_dst,
+ rename_dst, rename_dst_cmp, rename_dst_init);
/* Table of rename/copy src files */
static struct diff_rename_src {
@@ -437,7 +421,7 @@ void diffcore_rename(struct diff_options *options)
strcmp(options->single_follow, p->two->path))
continue; /* not interested */
else
- locate_rename_dst(p->two, 1);
+ register_rename_dst(p->two);
}
else if (!DIFF_FILE_VALID(p->two)) {
/*
@@ -582,7 +566,7 @@ void diffcore_rename(struct diff_options *options)
* not been turned into a rename/copy already.
*/
struct diff_rename_dst *dst =
- locate_rename_dst(p->two, 0);
+ locate_rename_dst(p->two);
if (dst && dst->pair) {
diff_q(&outq, dst->pair);
pair_to_free = p;
@@ -613,7 +597,7 @@ void diffcore_rename(struct diff_options *options)
if (DIFF_PAIR_BROKEN(p)) {
/* broken delete */
struct diff_rename_dst *dst =
- locate_rename_dst(p->one, 0);
+ locate_rename_dst(p->one);
if (dst && dst->pair)
/* counterpart is now rename/copy */
pair_to_free = p;
--
1.7.2.3
^ permalink raw reply related
* [PATCH 3/6] Convert diffcore-rename's rename_src to the new sorted-array API.
From: Yann Dirson @ 2010-12-05 10:34 UTC (permalink / raw)
To: git; +Cc: Yann Dirson
In-Reply-To: <1291545247-4151-1-git-send-email-ydirson@altern.org>
There was no compelling reason to pass separately two members of a
single struct to the insert function. That's a happy coincidence.
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
diffcore-rename.c | 57 ++++++++++++++++++----------------------------------
1 files changed, 20 insertions(+), 37 deletions(-)
diff --git a/diffcore-rename.c b/diffcore-rename.c
index a655017..7e35a82 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -37,46 +37,29 @@ declare_sorted_array_insert_checkbool(static, register_rename_dst, struct diff_f
rename_dst, rename_dst_cmp, rename_dst_init);
/* Table of rename/copy src files */
-static struct diff_rename_src {
+
+struct diff_rename_src {
struct diff_filespec *one;
unsigned short score; /* to remember the break score */
-} *rename_src;
-static int rename_src_nr, rename_src_alloc;
+};
-static struct diff_rename_src *register_rename_src(struct diff_filespec *one,
- unsigned short score)
+static int rename_src_cmp(struct diff_filepair *ref_pair, struct diff_rename_src *elem)
{
- int first, last;
-
- first = 0;
- last = rename_src_nr;
- while (last > first) {
- int next = (last + first) >> 1;
- struct diff_rename_src *src = &(rename_src[next]);
- int cmp = strcmp(one->path, src->one->path);
- if (!cmp)
- return src;
- if (cmp < 0) {
- last = next;
- continue;
- }
- first = next+1;
- }
-
- /* insert to make it at "first" */
- if (rename_src_alloc <= rename_src_nr) {
- rename_src_alloc = alloc_nr(rename_src_alloc);
- rename_src = xrealloc(rename_src,
- rename_src_alloc * sizeof(*rename_src));
- }
- rename_src_nr++;
- if (first < rename_src_nr)
- memmove(rename_src + first + 1, rename_src + first,
- (rename_src_nr - first - 1) * sizeof(*rename_src));
- rename_src[first].one = one;
- rename_src[first].score = score;
- return &(rename_src[first]);
+ return strcmp(ref_pair->one->path, elem->one->path);
+}
+static void rename_src_init(struct diff_rename_src *elem, struct diff_filepair *ref_pair)
+{
+ elem->one = ref_pair->one;
+ elem->score = ref_pair->score;
}
+declare_sorted_array(static, struct diff_rename_src, rename_src);
+declare_gen_binsearch(static, struct diff_rename_src, _locate_rename_src,
+ struct diff_filepair *);
+declare_gen_sorted_insert(static, struct diff_rename_src, _register_rename_src,
+ _locate_rename_src, struct diff_filepair *);
+declare_sorted_array_insert_checkbool(static, register_rename_src, struct diff_filepair *,
+ _register_rename_src,
+ rename_src, rename_src_cmp, rename_src_init);
static int basename_same(struct diff_filespec *src, struct diff_filespec *dst)
{
@@ -433,7 +416,7 @@ void diffcore_rename(struct diff_options *options)
*/
if (p->broken_pair && !p->score)
p->one->rename_used++;
- register_rename_src(p->one, p->score);
+ register_rename_src(p);
}
else if (detect_rename == DIFF_DETECT_COPY) {
/*
@@ -441,7 +424,7 @@ void diffcore_rename(struct diff_options *options)
* one, to indicate ourselves as a user.
*/
p->one->rename_used++;
- register_rename_src(p->one, p->score);
+ register_rename_src(p);
}
}
if (rename_dst_nr == 0 || rename_src_nr == 0)
--
1.7.2.3
^ permalink raw reply related
* [PATCH 4/6] Convert pack-objects.c to the new sorted-array API.
From: Yann Dirson @ 2010-12-05 10:34 UTC (permalink / raw)
To: git; +Cc: Yann Dirson
In-Reply-To: <1291545247-4151-1-git-send-email-ydirson@altern.org>
In this file the "list size" variable was named in a non-standard way.
The new API forces to use a more common convention.
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
builtin/pack-objects.c | 51 ++++++++++++++---------------------------------
1 files changed, 15 insertions(+), 36 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3cbeb29..887a55c 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -16,6 +16,7 @@
#include "list-objects.h"
#include "progress.h"
#include "refs.h"
+#include "sorted-array.h"
#ifndef NO_PTHREADS
#include <pthread.h>
@@ -871,45 +872,23 @@ static void add_pbase_object(struct tree_desc *tree,
}
}
-static unsigned *done_pbase_paths;
-static int done_pbase_paths_num;
-static int done_pbase_paths_alloc;
-static int done_pbase_path_pos(unsigned hash)
+static int unsigned_cmp(unsigned ref, unsigned *elem)
{
- int lo = 0;
- int hi = done_pbase_paths_num;
- while (lo < hi) {
- int mi = (hi + lo) / 2;
- if (done_pbase_paths[mi] == hash)
- return mi;
- if (done_pbase_paths[mi] < hash)
- hi = mi;
- else
- lo = mi + 1;
- }
- return -lo-1;
+ if (ref == *elem)
+ return 0;
+ if (ref < *elem)
+ return -1;
+ return 1;
}
-
-static int check_pbase_path(unsigned hash)
+static void unsigned_init(unsigned *elem, unsigned ref)
{
- int pos = (!done_pbase_paths) ? -1 : done_pbase_path_pos(hash);
- if (0 <= pos)
- return 1;
- pos = -pos - 1;
- if (done_pbase_paths_alloc <= done_pbase_paths_num) {
- done_pbase_paths_alloc = alloc_nr(done_pbase_paths_alloc);
- done_pbase_paths = xrealloc(done_pbase_paths,
- done_pbase_paths_alloc *
- sizeof(unsigned));
- }
- done_pbase_paths_num++;
- if (pos < done_pbase_paths_num)
- memmove(done_pbase_paths + pos + 1,
- done_pbase_paths + pos,
- (done_pbase_paths_num - pos - 1) * sizeof(unsigned));
- done_pbase_paths[pos] = hash;
- return 0;
+ *elem = ref;
}
+declare_sorted_array(static, unsigned, done_pbase_paths);
+declare_gen_binsearch(static, unsigned, done_pbase_path_pos, unsigned);
+declare_gen_sorted_insert(static, unsigned, _check_pbase_path, done_pbase_path_pos, unsigned);
+declare_sorted_array_insert_checkbool(static, check_pbase_path, unsigned, _check_pbase_path,
+ done_pbase_paths, unsigned_cmp, unsigned_init);
static void add_preferred_base_object(const char *name)
{
@@ -987,7 +966,7 @@ static void cleanup_preferred_base(void)
free(done_pbase_paths);
done_pbase_paths = NULL;
- done_pbase_paths_num = done_pbase_paths_alloc = 0;
+ done_pbase_paths_nr = done_pbase_paths_alloc = 0;
}
static void check_object(struct object_entry *entry)
--
1.7.2.3
^ permalink raw reply related
* [PATCH 6/6] [WIP] subvert sorted-array to replace binary-search in unpack-objects.
From: Yann Dirson @ 2010-12-05 10:34 UTC (permalink / raw)
To: git; +Cc: Yann Dirson
In-Reply-To: <1291545247-4151-1-git-send-email-ydirson@altern.org>
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
builtin/unpack-objects.c | 41 ++++++++++++++++++++++++++---------------
1 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index f63973c..b0c15e6 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -11,6 +11,7 @@
#include "progress.h"
#include "decorate.h"
#include "fsck.h"
+#include "sorted-array.h"
static int dry_run, quiet, recover, has_errors, strict;
static const char unpack_usage[] = "git unpack-objects [-n] [-q] [-r] [--strict] < pack-file";
@@ -157,7 +158,25 @@ struct obj_info {
#define FLAG_OPEN (1u<<20)
#define FLAG_WRITTEN (1u<<21)
-static struct obj_info *obj_list;
+/*
+ * FIXME: obj_info is a sorted array, but we read it as a whole, we
+ * don't need insertion features. This allows us to abuse unused
+ * obj_info_nr later as a means of specifying an upper bound for
+ * binary search. obj_info_alloc shall be eliminated by the compiler
+ * as unused.
+ */
+static int obj_info_cmp(off_t ref, struct obj_info *elem)
+{
+ if (ref == elem->offset)
+ return 0;
+ if (ref < elem->offset)
+ return -1;
+ return 1;
+}
+declare_sorted_array(static, struct obj_info, obj_list);
+declare_gen_binsearch(static, struct obj_info, _obj_list_check, off_t);
+declare_sorted_array_search_check(static, obj_list_check, off_t, _obj_list_check,
+ obj_list, obj_info_cmp);
static unsigned nr_objects;
/*
@@ -356,7 +375,7 @@ static void unpack_delta_entry(enum object_type type, unsigned long delta_size,
unsigned base_found = 0;
unsigned char *pack, c;
off_t base_offset;
- unsigned lo, mid, hi;
+ int pos;
pack = fill(1);
c = *pack;
@@ -380,19 +399,11 @@ static void unpack_delta_entry(enum object_type type, unsigned long delta_size,
free(delta_data);
return;
}
- lo = 0;
- hi = nr;
- while (lo < hi) {
- mid = (lo + hi)/2;
- if (base_offset < obj_list[mid].offset) {
- hi = mid;
- } else if (base_offset > obj_list[mid].offset) {
- lo = mid + 1;
- } else {
- hashcpy(base_sha1, obj_list[mid].sha1);
- base_found = !is_null_sha1(base_sha1);
- break;
- }
+ obj_list_nr = nr; /* kludge to bound the search */
+ pos = obj_list_check(base_offset);
+ if (pos >= 0) {
+ hashcpy(base_sha1, obj_list[pos].sha1);
+ base_found = !is_null_sha1(base_sha1);
}
if (!base_found) {
/*
--
1.7.2.3
^ permalink raw reply related
* [PATCH 5/6] Use sorted-array API for commit.c's commit_graft.
From: Yann Dirson @ 2010-12-05 10:34 UTC (permalink / raw)
To: git; +Cc: Yann Dirson
In-Reply-To: <1291545247-4151-1-git-send-email-ydirson@altern.org>
Factorizing code fixes off-by-one error in the duplicated code (caused
mostly harmless anticipated growing of the array).
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
commit.c | 62 +++++++++++++++++++++++++++-----------------------------------
1 files changed, 27 insertions(+), 35 deletions(-)
diff --git a/commit.c b/commit.c
index 2d9265d..b7aeee4 100644
--- a/commit.c
+++ b/commit.c
@@ -6,6 +6,7 @@
#include "diff.h"
#include "revision.h"
#include "notes.h"
+#include "sorted-array.h"
int save_commit_buffer = 1;
@@ -76,33 +77,37 @@ static unsigned long parse_commit_date(const char *buf, const char *tail)
return strtoul(dateptr, NULL, 10);
}
-static struct commit_graft **commit_graft;
-static int commit_graft_alloc, commit_graft_nr;
-
-static int commit_graft_pos(const unsigned char *sha1)
+static int commit_graft_cmp(const unsigned char *ref_sha1, struct commit_graft **elem)
{
- int lo, hi;
- lo = 0;
- hi = commit_graft_nr;
- while (lo < hi) {
- int mi = (lo + hi) / 2;
- struct commit_graft *graft = commit_graft[mi];
- int cmp = hashcmp(sha1, graft->sha1);
- if (!cmp)
- return mi;
- if (cmp < 0)
- hi = mi;
- else
- lo = mi + 1;
- }
- return -lo - 1;
+ return hashcmp(ref_sha1, (*elem)->sha1);
}
+declare_sorted_array(static, struct commit_graft *, commit_graft);
+declare_gen_binsearch(static, struct commit_graft *, _commit_graft_pos,
+ const unsigned char *);
+declare_sorted_array_search_check(static, commit_graft_pos, const unsigned char *,
+ _commit_graft_pos, commit_graft, commit_graft_cmp);
+// FIXME: do we want to/can we remove INITTYPE from gen_binsearch ?
+static int commit_graft_cmp2(struct commit_graft *ref_graft, struct commit_graft **elem)
+{
+ return commit_graft_cmp(ref_graft->sha1, elem);
+}
+declare_gen_binsearch(static, struct commit_graft *, _commit_graft_pos2,
+ struct commit_graft *);
+static void commit_graft_init(struct commit_graft **elem, struct commit_graft *ref_graft)
+{
+ *elem = ref_graft;
+}
+declare_gen_sorted_insert(static, struct commit_graft *, _register_commit_graft0,
+ _commit_graft_pos2, struct commit_graft *)
+declare_sorted_array_insert_check(static, register_commit_graft0, struct commit_graft *,
+ _register_commit_graft0, commit_graft,
+ commit_graft_cmp2, commit_graft_init);
int register_commit_graft(struct commit_graft *graft, int ignore_dups)
{
- int pos = commit_graft_pos(graft->sha1);
+ int pos = register_commit_graft0(graft);
- if (0 <= pos) {
+ if (pos >= 0) {
if (ignore_dups)
free(graft);
else {
@@ -111,19 +116,6 @@ int register_commit_graft(struct commit_graft *graft, int ignore_dups)
}
return 1;
}
- pos = -pos - 1;
- if (commit_graft_alloc <= ++commit_graft_nr) {
- commit_graft_alloc = alloc_nr(commit_graft_alloc);
- commit_graft = xrealloc(commit_graft,
- sizeof(*commit_graft) *
- commit_graft_alloc);
- }
- if (pos < commit_graft_nr)
- memmove(commit_graft + pos + 1,
- commit_graft + pos,
- (commit_graft_nr - pos - 1) *
- sizeof(*commit_graft));
- commit_graft[pos] = graft;
return 0;
}
--
1.7.2.3
^ permalink raw reply related
* [PATCH 1/6] Introduce sorted-array binary-search function.
From: Yann Dirson @ 2010-12-05 10:34 UTC (permalink / raw)
To: git; +Cc: Yann Dirson
In-Reply-To: <1291545247-4151-1-git-send-email-ydirson@altern.org>
We use a cpp-based template mechanism to declare the array and its
management data, as well as a search function.
Thanks to Jonathan Nieder for this design idea.
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
Makefile | 1 +
sorted-array.h | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 154 insertions(+), 0 deletions(-)
create mode 100644 sorted-array.h
diff --git a/Makefile b/Makefile
index 1d42413..ced07df 100644
--- a/Makefile
+++ b/Makefile
@@ -539,6 +539,7 @@ LIB_H += run-command.h
LIB_H += sha1-lookup.h
LIB_H += sideband.h
LIB_H += sigchain.h
+LIB_H += sorted-array.h
LIB_H += strbuf.h
LIB_H += string-list.h
LIB_H += submodule.h
diff --git a/sorted-array.h b/sorted-array.h
new file mode 100644
index 0000000..dc4be87
--- /dev/null
+++ b/sorted-array.h
@@ -0,0 +1,153 @@
+#ifndef SORTED_ARRAY_H_
+#define SORTED_ARRAY_H_
+
+/*
+ * Declare an array of given type, together with its management
+ * variable holding currently-allocated number of elements and number
+ * of elements effectively used.
+ */
+#define declare_sorted_array(MAYBESTATIC,ELEMTYPE,LIST) \
+MAYBESTATIC ELEMTYPE *LIST; \
+MAYBESTATIC int LIST##_nr, LIST##_alloc;
+
+/*
+ * Declare FUNCNAME as a binary-search function on sorted-arrays of
+ * ELEMTYPE elements, search term being be of type INITTYPE.
+ *
+ * The resulting function can act on any ELEMTYPE* list, using any
+ * suitable comparison function taking an INITTYPE argument and a
+ * pointer to an ELEMTYPE argument, and returning an int with the same
+ * meaning as strcmp. If the element is found, it returns the index
+ * in the list where it was found; if it is not found, it returns
+ * (-pos - 1), where "pos" is the index in the list where the element
+ * would be inserted.
+ *
+ * See below for macros to define more specific functions tailored to
+ * a given list, and with output suitable to various usages.
+ */
+#define declare_gen_binsearch(MAYBESTATIC,ELEMTYPE,FUNCNAME,INITTYPE) \
+MAYBESTATIC int FUNCNAME( \
+ ELEMTYPE *list, int list_nr, \
+ int(*cmp_func)(INITTYPE ref, ELEMTYPE *elem), \
+ INITTYPE data) \
+{ \
+ int lo, hi; \
+ \
+ lo = 0; \
+ hi = list_nr; \
+ while (hi > lo) { \
+ int mid = (hi + lo) >> 1; \
+ int cmp = cmp_func(data, list + mid); \
+ if (!cmp) \
+ return mid; \
+ if (cmp < 0) \
+ hi = mid; \
+ else \
+ lo = mid + 1; \
+ } \
+ return -lo - 1; \
+}
+
+/*
+ * Declare FUNCNAME as a function to search for an element in
+ * sorted-arrays of ELEMTYPE elements, inserting it if it was not
+ * found, search term being be of type INITTYPE. The position where
+ * to insert will be given found by SEARCHFUNC, which must be
+ * compatible with the search functions defined by
+ * declare_gen_binsearch().
+ *
+ * The resulting function takes the same arguments as similar search
+ * functions, with the addition of a function to initialize the
+ * newly-allocated element from the search term.
+ */
+#define declare_gen_sorted_insert(MAYBESTATIC,ELEMTYPE,FUNCNAME,SEARCHFUNC,INITTYPE) \
+MAYBESTATIC int FUNCNAME( \
+ ELEMTYPE **list_p, int *list_nr_p, int *list_alloc_p, \
+ int(*cmp_func)(INITTYPE ref, ELEMTYPE *elem), \
+ void(*init_func)(ELEMTYPE *elem, INITTYPE init), \
+ INITTYPE data) \
+{ \
+ int pos = SEARCHFUNC(*list_p, *list_nr_p, cmp_func, data); \
+ if (pos >= 0) \
+ return pos; \
+ /* not found */ \
+ pos = -pos - 1; \
+ /* insert to make it at "pos" */ \
+ if (*list_alloc_p <= *list_nr_p) { \
+ (*list_alloc_p) = alloc_nr((*list_alloc_p)); \
+ *list_p = xrealloc(*list_p, \
+ (*list_alloc_p) * sizeof(**list_p)); \
+ } \
+ (*list_nr_p)++; \
+ if (pos < *list_nr_p) \
+ memmove(*list_p + pos + 1, *list_p + pos, \
+ (*list_nr_p - pos - 1) * sizeof(**list_p)); \
+ init_func(&(*list_p)[pos], data); \
+ return -pos - 1; \
+}
+
+/*
+ * Returns the position of the element if found pre-existing in the
+ * list, or if not found, -pos-1 where pos is where the element would
+ * have been inserted.
+ */
+#define declare_sorted_array_search_check(MAYBESTATIC,FUNCNAME,INITTYPE,GENSEARCH,LIST,CMP) \
+MAYBESTATIC int FUNCNAME(INITTYPE data) \
+{ \
+ return GENSEARCH(LIST, LIST##_nr, CMP, data); \
+}
+
+/*
+ * Returns the position of the element if found pre-existing in the
+ * list, or if not found inserts it, and returns -pos-1 where pos is
+ * where the element was inserted.
+ */
+#define declare_sorted_array_insert_check(MAYBESTATIC,FUNCNAME,INITTYPE,GENINSERT,LIST,CMP,INIT) \
+MAYBESTATIC int FUNCNAME(INITTYPE data) \
+{ \
+ return GENINSERT(&LIST, &LIST##_nr, &LIST##_alloc, \
+ CMP, INIT, data); \
+}
+
+/*
+ * Insert, and just tell whether the searched element was pre-existing
+ * in the list or not.
+ */
+#define declare_sorted_array_insert_checkbool(MAYBESTATIC,FUNCNAME,INITTYPE,GENINSERT,LIST,CMP,INIT) \
+MAYBESTATIC int FUNCNAME(INITTYPE data) \
+{ \
+ int idx = GENINSERT(&LIST, &LIST##_nr, &LIST##_alloc, \
+ CMP, INIT, data); \
+ if (idx < 0) \
+ return 0; \
+ return 1; \
+}
+
+/*
+ * Search for element. Returns address of the element found, or NULL
+ * if not found.
+ */
+#define declare_sorted_array_search_elem(MAYBESTATIC,ELEMTYPE,FUNCNAME,INITTYPE,GENSEARCH,LIST,CMP) \
+MAYBESTATIC ELEMTYPE *FUNCNAME(INITTYPE data) \
+{ \
+ int idx = GENSEARCH(LIST, LIST##_nr, CMP, data); \
+ if (idx < 0) \
+ return NULL; \
+ return &(LIST[idx]); \
+}
+
+/*
+ * Insert element if not there already. Returns address of the
+ * element found or newly-inserted.
+ */
+#define declare_sorted_array_insert_elem(MAYBESTATIC,ELEMTYPE,FUNCNAME,INITTYPE,GENINSERT,LIST,CMP,INIT) \
+MAYBESTATIC ELEMTYPE *FUNCNAME(INITTYPE data) \
+{ \
+ int idx = GENINSERT(&LIST, &LIST##_nr, &LIST##_alloc, \
+ CMP, INIT, data); \
+ if (idx < 0) \
+ idx = -idx - 1; \
+ return &(LIST[idx]); \
+}
+
+#endif
--
1.7.2.3
^ 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