* Re: Track /etc directory using Git
From: Pierre Habouzit @ 2007-09-15 15:27 UTC (permalink / raw)
To: martin f krafft; +Cc: git, Nicolas Vilz, Thomas Harning Jr., Francis Moreau
In-Reply-To: <20070915152455.GA16223@piper.oerlikon.madduck.net>
[-- Attachment #1: Type: text/plain, Size: 1006 bytes --]
On Sat, Sep 15, 2007 at 03:24:55PM +0000, martin f krafft wrote:
> also sprach Pierre Habouzit <madcoder@debian.org> [2007.09.15.1629 +0200]:
> > I fear that you'll end up with quite a big bloat of git, for a use
> > case that is fairly limited.
>
> I think it doesn't get bloated until you try to support the model of
> tracking different stuff for different files in the same repo. If
> you just track one set of data across all files in the repo, I don't
> think it'll cause too much bloat.
Yeah but if the stuff is opaque to git, you'll definitely end up with
security issues, which makes it also a no-go for /etc versionning.
Note that I don't specifically care about git being able to deal with
/etc, I was just pointing out some issues I can see with it, but I'm
neiter in favor nor against it.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Track /etc directory using Git
From: martin f krafft @ 2007-09-15 15:24 UTC (permalink / raw)
To: git; +Cc: Pierre Habouzit, Nicolas Vilz, Thomas Harning Jr., Francis Moreau
In-Reply-To: <20070915142932.GB27494@artemis.corp>
[-- Attachment #1: Type: text/plain, Size: 723 bytes --]
also sprach Pierre Habouzit <madcoder@debian.org> [2007.09.15.1629 +0200]:
> I fear that you'll end up with quite a big bloat of git, for a use
> case that is fairly limited.
I think it doesn't get bloated until you try to support the model of
tracking different stuff for different files in the same repo. If
you just track one set of data across all files in the repo, I don't
think it'll cause too much bloat.
--
.''`. martin f. krafft <madduck@debian.org>
: :' : proud Debian developer, author, administrator, and user
`. `'` http://people.debian.org/~madduck - http://debiansystem.info
`- Debian - when you have better things to do than fixing systems
gentoo: the performance placebo.
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] git-svn: remove --first-parent, add --upstream
From: Lars Hjemli @ 2007-09-15 15:24 UTC (permalink / raw)
To: Peter Baumann; +Cc: Eric Wong, Junio C Hamano, git
In-Reply-To: <20070915143743.GB4957@xp.machine.xx>
On 9/15/07, Peter Baumann <waste.manager@gmx.de> wrote:
> On Sat, Sep 15, 2007 at 04:08:31PM +0200, Lars Hjemli wrote:
> > On 9/7/07, Lars Hjemli <hjemli@gmail.com> wrote:
> > > On 9/7/07, Peter Baumann <waste.manager@gmx.de> wrote:
> > > > lets reset 'trunk' to its state before the merge and
> > > > 'branch1' to the merge commit, before fixing the bug in 'branch1'.
> > > >
> > > > a-b-c-d-e trunk
> > > > \ \
> > > > \ -x-y m branch1
> > >
> > > Yeah, this would certainly not be handled correctly by dcommit using
> > > --first-parent (but it could be handled by (a correct implementation
> > > of) --upstream).
> >
> > Actually, I don't think there's any way to handle this correctly. The
> > current git-svn will do the right thing except in cases like the one
> > you described, and in these cases it can be _forced_ to do the right
> > thing by editing the grafts file, so I'll drop the whole --upstream
> > idea.
> >
>
> What do you mean by editing the graft file? Remove (the wrong) parent
> from the merge commit by a graft?
I imagined just changing the order of the parents.
--
larsh
^ permalink raw reply
* inconsistent use of worktree? (resend)
From: martin f krafft @ 2007-09-15 15:23 UTC (permalink / raw)
To: git discussion list; +Cc: matled, Johannes.Schindelin
In-Reply-To: <20070906132906.GA7791@piper.oerlikon.madduck.net>
[-- Attachment #1: Type: text/plain, Size: 4064 bytes --]
[this message received no replies, I thus resend once with Matthias
and Johannes on Cc, and slightly modified]
Hi there,
I have a $GIT_DIR in ~/.bin/colgit.git, which is bare=false and has
worktree=.. (the parent). This relates back to a suggestion by
Johannes Schindelin [0] for maintaining files like ~/.vimrc and
~/.zshrc in separate repos sharing the same worktree. In short:
while ~/.bin/colgit.git holds all of colgit's objects, refs, and the
index, the repository's files are in ~/.bin (and thus in $PATH).
0. http://marc.info/?l=git&m=118418927823760&w=2
I am trying to figure out a sensible way to work with this, ideally
one which does not require me to set $GIT_DIR (which works):
GIT_DIR=~/.bin/colgit.git git status
GIT_DIR=~/.bin/colgit.git git add ...
GIT_DIR=~/.bin/colgit.git git commit ...
(or one export GIT_DIR)
but that's a lot of typing and thus error-prone.
I found this to work:
piper:~/.bin/colgit.git> git status
# On branch master
# Changed but not updated:
# (use "git add <file>..." to update what will be committed)
#
# modified: colgit
#
no changes added to commit (use "git add" and/or "git commit -a")
piper:~/.bin/colgit.git> git add ../colgit
piper:~/.bin/colgit.git> git status
# On branch master
# Changes to be committed:
# (use "git reset HEAD <file>..." to unstage)
#
# modified: colgit
and I rejoiced, because that's really pretty cool. But then, trying
to commit, reality hit back:
piper:~/.bin/colgit.git> git commit -m'initial commit'
nothing to commit (use "git add file1 file2" to include for commit)
So I had to revert to using GIT_DIR for this:
piper:~/.bin> GIT_DIR=colgit.git git commit -m'initial commit'
Created initial commit 008594d: initial commit
1 files changed, 104 insertions(+), 0 deletions(-)
create mode 100755 colgit
And then things got really weird, after I accidentally tried the
direct commit (without GIT_DIR) again, following some touchups
I made to the colgit file:
piper:~/.bin/colgit.git> git commit -m'repairing registration and other touchups'
Created commit acd86b9: Merge branch 'vimplate'
First, it seems that commits work just fine for changes made only to
files already tracked by git, but not if a commit introduces a new
file.
But second, git-commit does appear to get really confused, since
the commit it claims to have just created is actually not part of
the current repository, but rather the HEAD of the repository in
~/.git (which tracks ~).
git-commit -a also does not work:
piper:~/.bin/colgit.git> git commit -m'add list action to enumerate all known repos' -a
error: .bin/colgit: cannot add to the index - missing --add option?
fatal: Unable to process path .bin/colgit
And finally, git-diff seems to think that I removed the file, rather
than to show differences:
piper:~/.bin/colgit.git> git diff | diffstat #[334]
colgit | 138 -----------------------------------------------------------------
1 file changed, 138 deletions(-)
When in fact:
piper:~/.bin> GIT_DIR=colgit.git git diff | diffstat #[342]
colgit | 1 +
1 file changed, 1 insertion(+)
It seems to me that worktree support is still somewhat buggy. Has
anyone experienced similar things? Does anyone have an idea what
this could be about?
I hope to be able to go through the code in the near future but
I first have to learn a lot about git innards, so this could take
a while. Maybe Matthias has a clue? I'd be happy yo test.
--
martin; (greetings from the heart of the sun.)
\____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
"i wish there was a knob on the tv to turn up the intelligence.
there's a knob called 'brightness', but it doesn't seem to work."
-- gallagher
spamtraps: madduck.bogus@madduck.net
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: metastore (was: Track /etc directory using Git)
From: martin f krafft @ 2007-09-15 14:54 UTC (permalink / raw)
To: git
Cc: Johannes Schindelin, Thomas Harning Jr., Francis Moreau,
Nicolas Vilz, David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709151507310.28586@racer.site>
[-- Attachment #1: Type: text/plain, Size: 2913 bytes --]
also sprach Johannes Schindelin <Johannes.Schindelin@gmx.de> [2007.09.15.1610 +0200]:
> No. Git is a source code management system. Everything else that
> you can do with it is a bonus, a second class citizen. Should we
> really try to support your use case, we will invariably affect the
> primary use case.
I thought git was primarily a content tracker... so it all comes
down to how to define content, doesn't it? But either way, we need
not discuss that because that definition depends a lot on context
and purpose and thus cannot be answered once and for all.
I understand that for the primary use case, tracking nothing more
than +x makes sense and should not be interfered with. This is why
I was proposing a policy-based approach. The primary use case is
unaffected, it's the default policy. Someone may choose to track
other mode bits or file/inode attributes, according to one of
several policies available with git, or even a custom policy. In
that case, the repository needs to be appropriately configured.
The reason why I say this should be done inside git rather than with
hooks and an external tool, such as metastore is quite simple: git
knows about every content entity in any tree of a repo and already
has a data node for each object. Rather than introducing a parallel
object database (shadow hierarchy or single file), it would make
a lot more sense and be way more robust to attach additional
information to these object nodes, wouldn't it?
So with "appropriately configured" above, I meant that one should be
able to say
git-config core.track all
or
git-config core.track mode+attr
or the default:
git-config core.track 7666
(read that as a umask, which masks out everything but the three
x bits. I made it 7666 instead of 7677 because core.umask and
core.sharedrepository then override the group and world bits if
needed)
and have git do the right thing, rather than expecting those who
want to track more than the executable bit to assemble a brittle set
of hooks and metadata collectors+applicators and hope it all works.
I understand also that this is not top priority for git, which is
why I said earlier in the thread that the real difficulty might be
to get Junio to accept a patch. But I think that the patch would be
rather contained and small, having it all configurable would make it
unintrusive, and if we all test it real well, it should pass as
a bonus. After all, git can e.g upload patches to IMAP boxes, which
in my world clearly is bonus material as well.
Cheers,
--
martin; (greetings from the heart of the sun.)
\____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
"the well-bred contradict other people.
the wise contradict themselves."
-- oscar wilde
spamtraps: madduck.bogus@madduck.net
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] git-svn: remove --first-parent, add --upstream
From: Peter Baumann @ 2007-09-15 14:37 UTC (permalink / raw)
To: Lars Hjemli; +Cc: Eric Wong, Junio C Hamano, git
In-Reply-To: <8c5c35580709150708k5acdeabdh17fc7ef30ee3eb79@mail.gmail.com>
On Sat, Sep 15, 2007 at 04:08:31PM +0200, Lars Hjemli wrote:
> On 9/7/07, Lars Hjemli <hjemli@gmail.com> wrote:
> > On 9/7/07, Peter Baumann <waste.manager@gmx.de> wrote:
> > > lets reset 'trunk' to its state before the merge and
> > > 'branch1' to the merge commit, before fixing the bug in 'branch1'.
> > >
> > > a-b-c-d-e trunk
> > > \ \
> > > \ -x-y m branch1
> >
> > Yeah, this would certainly not be handled correctly by dcommit using
> > --first-parent (but it could be handled by (a correct implementation
> > of) --upstream).
>
> Actually, I don't think there's any way to handle this correctly. The
> current git-svn will do the right thing except in cases like the one
> you described, and in these cases it can be _forced_ to do the right
> thing by editing the grafts file, so I'll drop the whole --upstream
> idea.
>
What do you mean by editing the graft file? Remove (the wrong) parent
from the merge commit by a graft? This will help you commit on the right
branch, but 7b02b85a66fee6b357e02f9e70dd0baa0fd24308 removes the
possibility to get the same graph back, because now git-svn really
honors the graft in its rebase phase.
So you would get something like this back:
a-b - c - d - e trunk
\
\ -x'-y'- m branch1
Notice that you've lost the merge information :-(
-Peter
^ permalink raw reply
* Re: Track /etc directory using Git
From: Pierre Habouzit @ 2007-09-15 14:29 UTC (permalink / raw)
To: Nicolas Vilz; +Cc: Thomas Harning Jr., martin f krafft, git, Francis Moreau
In-Reply-To: <20070914212643.GA10970@amy.inscure.wireless.home.vilz.de>
[-- Attachment #1: Type: text/plain, Size: 2138 bytes --]
On Fri, Sep 14, 2007 at 09:26:43PM +0000, Nicolas Vilz wrote:
> On Fri, Sep 14, 2007 at 01:31:06PM -0400, Thomas Harning Jr. wrote:
> > On 9/14/07, martin f krafft <madduck@madduck.net> wrote:
> > > also sprach Francis Moreau <francis.moro@gmail.com> [2007.09.14.1008 +0200]:
> > > > Did you find an alternative to git in this case ?
> > >
> > > No, and I did not look anywhere, but I know of no other VCS that can
> > > adequatly track permissions.
> > Has anyone checked out metastore? http://repo.or.cz/w/metastore.git
> > ... there's an XML error in there somewhere, so its not loading the
> > 'main' page, but http://repo.or.cz/w/metastore.git?a=shortlog should
> > work.
> >
> > It looks like it could work.... any thoughts on this?
>
> I use that tool. If you just have one branch, it works. With the
> commit-hook, which also updates the metadata, you have current
> permission tracking.
>
> There is a lack of a checkout-hook, which sets the permissions, so you
> have to remeber todo a metastore -a after you checked out a revision.
Note that having metastore run by a hook makes it unsuitable for /etc
versioning, because you may have short period of times during which
s3kr3t files are readable by more people that what it should be.
The sole sane way to do that would be to track permissions, acls,
whatever _in_ git. Though, I'm still not convinced that it is such a
good idea at all. I mean for source code you absolutely _don't_ want git
to track permissions (outside from the +x bit). You don't want git to
try to chown your files to "madcoder:madcoder" because I was the last
one committing. So that would mean that you want sometimes to track
permissions, sometimes not. So you need a bunch of tools to list files
whose permissions have to be tracked, and whose permissions don't need
to be.
I fear that you'll end up with quite a big bloat of git, for a use
case that is fairly limited.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: STG, problem with pop/push of alternative versions of a patch
From: Jon Smirl @ 2007-09-15 14:25 UTC (permalink / raw)
To: Catalin Marinas; +Cc: Git Mailing List
In-Reply-To: <b0943d9e0709150107o27571446v9bef8e31517777e1@mail.gmail.com>
On 9/15/07, Catalin Marinas <catalin.marinas@gmail.com> wrote:
> On 15/09/2007, Jon Smirl <jonsmirl@gmail.com> wrote:
> > I trying to test two different versions of a patch that add files.
> > These patches create a new directory and add several files. When I pop
> > a version of the patch the directory and files and not getting
> > removed. This causes an error when I push the alternative version of
> > the patch.
>
> This shouldn't happen AFAICT (at least for the files, as GIT doesn't
> care much about directories). What GIT/StGIT version are you using?
> StGIT simply calls GIT to do the HEAD switch.
I have played around with some more. It is more complicated than the
simple case I described. Earlier I noticed a message about applying a
patch that was empty that shouldn't have been. I checked and the patch
is indeed empty. The empty patch probably caused the files to be left.
I had been using hide/unide and reordering with on the patch and had
encountered a couple errors in stg. I'll try and track down the
sequence that caused the contents of the patch to be lost.
>
> Could you run 'stg patches drivers/net/fec_mpc52xx/Kconfig' with the
> initial series pushed? It should show which patches touch this file.
> If it doesn't show any, maybe the files weren't added to any patch and
> hence the error.
>
> Thanks.
>
> --
> Catalin
>
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: metastore
From: David Kastrup @ 2007-09-15 14:16 UTC (permalink / raw)
To: Johannes Schindelin
Cc: martin f krafft, git, Thomas Harning Jr., Francis Moreau,
Nicolas Vilz, David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709151507310.28586@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi,
>
> On Sat, 15 Sep 2007, martin f krafft wrote:
>
>> The problem with metdata getting corrupted, which Nicolas reported,
>> may well have to do with the use of a single file.
>
> Then the tool is corrupt. Introducing a shadow hierarchy, as you
> propose, is very inefficient.
>
>> Anyway, this *really* should go into git itself!
>
> No. Git is a source code management system. Everything else that
> you can do with it is a bonus, a second class citizen. Should we
> really try to support your use case, we will invariably affect the
> primary use case.
That's what bad design is all about, after all.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* [PATCH] builtin-apply: use strbuf's instead of buffer_desc's.
From: Pierre Habouzit @ 2007-09-15 14:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20070915141210.GA27494@artemis.corp>
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
builtin-apply.c | 203 ++++++++++++++++++++-----------------------------------
1 files changed, 73 insertions(+), 130 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 7057d0d..6c24f40 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1442,39 +1442,34 @@ static void show_stats(struct patch *patch)
free(qname);
}
-static int read_old_data(struct stat *st, const char *path, char **buf_p, unsigned long *alloc_p, unsigned long *size_p)
+static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)
{
int fd;
- unsigned long got;
unsigned long nsize;
char *nbuf;
- unsigned long size = *size_p;
- char *buf = *buf_p;
switch (st->st_mode & S_IFMT) {
case S_IFLNK:
- return readlink(path, buf, size) != size;
+ strbuf_grow(buf, st->st_size);
+ if (readlink(path, buf->buf, st->st_size) != st->st_size)
+ return -1;
+ strbuf_setlen(buf, st->st_size);
+ return 0;
case S_IFREG:
fd = open(path, O_RDONLY);
if (fd < 0)
return error("unable to open %s", path);
- got = 0;
- for (;;) {
- ssize_t ret = xread(fd, buf + got, size - got);
- if (ret <= 0)
- break;
- got += ret;
+ if (strbuf_read(buf, fd, st->st_size) < 0) {
+ close(fd);
+ return -1;
}
close(fd);
- nsize = got;
- nbuf = convert_to_git(path, buf, &nsize);
- if (nbuf) {
- free(buf);
- *buf_p = nbuf;
- *alloc_p = nsize;
- *size_p = nsize;
- }
- return got != size;
+
+ nsize = buf->len;
+ nbuf = convert_to_git(path, buf->buf, &nsize);
+ if (nbuf)
+ strbuf_embed(buf, nbuf, nsize, nsize);
+ return 0;
default:
return -1;
}
@@ -1579,12 +1574,6 @@ static void remove_last_line(const char **rbuf, int *rsize)
*rsize = offset + 1;
}
-struct buffer_desc {
- char *buffer;
- unsigned long size;
- unsigned long alloc;
-};
-
static int apply_line(char *output, const char *patch, int plen)
{
/* plen is number of bytes to be copied from patch,
@@ -1654,10 +1643,9 @@ static int apply_line(char *output, const char *patch, int plen)
return output + plen - buf;
}
-static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, int inaccurate_eof)
+static int apply_one_fragment(struct strbuf *buf, struct fragment *frag, int inaccurate_eof)
{
int match_beginning, match_end;
- char *buf = desc->buffer;
const char *patch = frag->patch;
int offset, size = frag->size;
char *old = xmalloc(size);
@@ -1768,24 +1756,17 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i
lines = 0;
pos = frag->newpos;
for (;;) {
- offset = find_offset(buf, desc->size,
+ offset = find_offset(buf->buf, buf->len,
oldlines, oldsize, pos, &lines);
- if (match_end && offset + oldsize != desc->size)
+ if (match_end && offset + oldsize != buf->len)
offset = -1;
if (match_beginning && offset)
offset = -1;
if (offset >= 0) {
- int diff;
- unsigned long size, alloc;
-
if (new_whitespace == strip_whitespace &&
- (desc->size - oldsize - offset == 0)) /* end of file? */
+ (buf->len - oldsize - offset == 0)) /* end of file? */
newsize -= new_blank_lines_at_end;
- diff = newsize - oldsize;
- size = desc->size + diff;
- alloc = desc->alloc;
-
/* Warn if it was necessary to reduce the number
* of context lines.
*/
@@ -1795,19 +1776,8 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i
" to apply fragment at %d\n",
leading, trailing, pos + lines);
- if (size > alloc) {
- alloc = size + 8192;
- desc->alloc = alloc;
- buf = xrealloc(buf, alloc);
- desc->buffer = buf;
- }
- desc->size = size;
- memmove(buf + offset + newsize,
- buf + offset + oldsize,
- size - offset - newsize);
- memcpy(buf + offset, newlines, newsize);
+ strbuf_splice(buf, offset, oldsize, newlines, newsize);
offset = 0;
-
break;
}
@@ -1843,12 +1813,11 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i
return offset;
}
-static int apply_binary_fragment(struct buffer_desc *desc, struct patch *patch)
+static int apply_binary_fragment(struct strbuf *buf, struct patch *patch)
{
- unsigned long dst_size;
struct fragment *fragment = patch->fragments;
- void *data;
- void *result;
+ unsigned long len;
+ void *dst;
/* Binary patch is irreversible without the optional second hunk */
if (apply_in_reverse) {
@@ -1859,29 +1828,23 @@ static int apply_binary_fragment(struct buffer_desc *desc, struct patch *patch)
? patch->new_name : patch->old_name);
fragment = fragment->next;
}
- data = (void*) fragment->patch;
switch (fragment->binary_patch_method) {
case BINARY_DELTA_DEFLATED:
- result = patch_delta(desc->buffer, desc->size,
- data,
- fragment->size,
- &dst_size);
- free(desc->buffer);
- desc->buffer = result;
- break;
+ dst = patch_delta(buf->buf, buf->len, fragment->patch,
+ fragment->size, &len);
+ if (!dst)
+ return -1;
+ /* XXX patch_delta NUL-terminates */
+ strbuf_embed(buf, dst, len, len + 1);
+ return 0;
case BINARY_LITERAL_DEFLATED:
- free(desc->buffer);
- desc->buffer = data;
- dst_size = fragment->size;
- break;
+ strbuf_embed(buf, fragment->patch, fragment->size, fragment->size);
+ return 0;
}
- if (!desc->buffer)
- return -1;
- desc->size = desc->alloc = dst_size;
- return 0;
+ return -1;
}
-static int apply_binary(struct buffer_desc *desc, struct patch *patch)
+static int apply_binary(struct strbuf *buf, struct patch *patch)
{
const char *name = patch->old_name ? patch->old_name : patch->new_name;
unsigned char sha1[20];
@@ -1900,7 +1863,7 @@ static int apply_binary(struct buffer_desc *desc, struct patch *patch)
/* See if the old one matches what the patch
* applies to.
*/
- hash_sha1_file(desc->buffer, desc->size, blob_type, sha1);
+ hash_sha1_file(buf->buf, buf->len, blob_type, sha1);
if (strcmp(sha1_to_hex(sha1), patch->old_sha1_prefix))
return error("the patch applies to '%s' (%s), "
"which does not match the "
@@ -1909,16 +1872,14 @@ static int apply_binary(struct buffer_desc *desc, struct patch *patch)
}
else {
/* Otherwise, the old one must be empty. */
- if (desc->size)
+ if (buf->len)
return error("the patch applies to an empty "
"'%s' but it is not empty", name);
}
get_sha1_hex(patch->new_sha1_prefix, sha1);
if (is_null_sha1(sha1)) {
- free(desc->buffer);
- desc->alloc = desc->size = 0;
- desc->buffer = NULL;
+ strbuf_release(buf);
return 0; /* deletion patch */
}
@@ -1926,43 +1887,44 @@ static int apply_binary(struct buffer_desc *desc, struct patch *patch)
/* We already have the postimage */
enum object_type type;
unsigned long size;
+ char *result;
- free(desc->buffer);
- desc->buffer = read_sha1_file(sha1, &type, &size);
- if (!desc->buffer)
+ result = read_sha1_file(sha1, &type, &size);
+ if (!result)
return error("the necessary postimage %s for "
"'%s' cannot be read",
patch->new_sha1_prefix, name);
- desc->alloc = desc->size = size;
- }
- else {
- /* We have verified desc matches the preimage;
+ /* XXX read_sha1_file NUL-terminates */
+ strbuf_embed(buf, result, size, size + 1);
+ } else {
+ /* We have verified buf matches the preimage;
* apply the patch data to it, which is stored
* in the patch->fragments->{patch,size}.
*/
- if (apply_binary_fragment(desc, patch))
+ if (apply_binary_fragment(buf, patch))
return error("binary patch does not apply to '%s'",
name);
/* verify that the result matches */
- hash_sha1_file(desc->buffer, desc->size, blob_type, sha1);
+ hash_sha1_file(buf->buf, buf->len, blob_type, sha1);
if (strcmp(sha1_to_hex(sha1), patch->new_sha1_prefix))
- return error("binary patch to '%s' creates incorrect result (expecting %s, got %s)", name, patch->new_sha1_prefix, sha1_to_hex(sha1));
+ return error("binary patch to '%s' creates incorrect result (expecting %s, got %s)",
+ name, patch->new_sha1_prefix, sha1_to_hex(sha1));
}
return 0;
}
-static int apply_fragments(struct buffer_desc *desc, struct patch *patch)
+static int apply_fragments(struct strbuf *buf, struct patch *patch)
{
struct fragment *frag = patch->fragments;
const char *name = patch->old_name ? patch->old_name : patch->new_name;
if (patch->is_binary)
- return apply_binary(desc, patch);
+ return apply_binary(buf, patch);
while (frag) {
- if (apply_one_fragment(desc, frag, patch->inaccurate_eof)) {
+ if (apply_one_fragment(buf, frag, patch->inaccurate_eof)) {
error("patch failed: %s:%ld", name, frag->oldpos);
if (!apply_with_reject)
return -1;
@@ -1973,76 +1935,57 @@ static int apply_fragments(struct buffer_desc *desc, struct patch *patch)
return 0;
}
-static int read_file_or_gitlink(struct cache_entry *ce, char **buf_p,
- unsigned long *size_p)
+static int read_file_or_gitlink(struct cache_entry *ce, struct strbuf *buf)
{
if (!ce)
return 0;
if (S_ISGITLINK(ntohl(ce->ce_mode))) {
- *buf_p = xmalloc(100);
- *size_p = snprintf(*buf_p, 100,
- "Subproject commit %s\n", sha1_to_hex(ce->sha1));
+ strbuf_grow(buf, 100);
+ strbuf_addf(buf, "Subproject commit %s\n", sha1_to_hex(ce->sha1));
} else {
enum object_type type;
- *buf_p = read_sha1_file(ce->sha1, &type, size_p);
- if (!*buf_p)
+ unsigned long sz;
+ char *result;
+
+ result = read_sha1_file(ce->sha1, &type, &sz);
+ if (!result)
return -1;
+ /* XXX read_sha1_file NUL-terminates */
+ strbuf_embed(buf, result, sz, sz + 1);
}
return 0;
}
static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)
{
- char *buf;
- unsigned long size, alloc;
- struct buffer_desc desc;
+ struct strbuf buf;
- size = 0;
- alloc = 0;
- buf = NULL;
+ strbuf_init(&buf, 0);
if (cached) {
- if (read_file_or_gitlink(ce, &buf, &size))
+ if (read_file_or_gitlink(ce, &buf))
return error("read of %s failed", patch->old_name);
- alloc = size;
} else if (patch->old_name) {
if (S_ISGITLINK(patch->old_mode)) {
- if (ce)
- read_file_or_gitlink(ce, &buf, &size);
- else {
+ if (ce) {
+ read_file_or_gitlink(ce, &buf);
+ } else {
/*
* There is no way to apply subproject
* patch without looking at the index.
*/
patch->fragments = NULL;
- size = 0;
}
- }
- else {
- size = xsize_t(st->st_size);
- alloc = size + 8192;
- buf = xmalloc(alloc);
- if (read_old_data(st, patch->old_name,
- &buf, &alloc, &size))
- return error("read of %s failed",
- patch->old_name);
+ } else {
+ if (read_old_data(st, patch->old_name, &buf))
+ return error("read of %s failed", patch->old_name);
}
}
- desc.size = size;
- desc.alloc = alloc;
- desc.buffer = buf;
-
- if (apply_fragments(&desc, patch) < 0)
+ if (apply_fragments(&buf, patch) < 0)
return -1; /* note with --reject this succeeds. */
-
- /* NUL terminate the result */
- if (desc.alloc <= desc.size)
- desc.buffer = xrealloc(desc.buffer, desc.size + 1);
- desc.buffer[desc.size] = 0;
-
- patch->result = desc.buffer;
- patch->resultsize = desc.size;
+ patch->result = buf.buf;
+ patch->resultsize = buf.len;
if (0 < patch->is_delete && patch->resultsize)
return error("removal patch leaves file contents");
--
1.5.3.1
^ permalink raw reply related
* [PATCH] New strbuf APIs: splice and embed.
From: Pierre Habouzit @ 2007-09-15 13:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20070915141210.GA27494@artemis.corp>
* strbuf_splice replace a portion of the buffer with another.
* strbuf_embed replace a strbuf buffer with the given one, that should be
malloc'ed. Then it enforces strbuf's invariants. If alloc > len, then this
function has negligible cost, else it will perform a realloc, possibly
with a cost.
---
strbuf.c | 31 +++++++++++++++++++++++++++++--
strbuf.h | 5 +++++
2 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index d919047..ad82eaf 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -24,14 +24,22 @@ char *strbuf_detach(struct strbuf *sb) {
return res;
}
+void strbuf_embed(struct strbuf *sb, void *buf, size_t len, size_t alloc) {
+ strbuf_release(sb);
+ sb->buf = buf;
+ sb->len = len;
+ sb->alloc = alloc;
+ strbuf_grow(sb, 0);
+ sb->buf[sb->len] = '\0';
+}
+
void strbuf_grow(struct strbuf *sb, size_t extra) {
if (sb->len + extra + 1 <= sb->len)
die("you want to use way too much memory");
ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
}
-void strbuf_rtrim(struct strbuf *sb)
-{
+void strbuf_rtrim(struct strbuf *sb) {
while (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))
sb->len--;
sb->buf[sb->len] = '\0';
@@ -48,6 +56,25 @@ void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len)
strbuf_setlen(sb, sb->len + len);
}
+void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
+ const void *data, size_t dlen)
+{
+ if (pos + len < pos)
+ die("you want to splice outside from the buffer");
+ if (pos > sb->len)
+ pos = sb->len;
+ if (pos + len > sb->len)
+ len = sb->len - pos;
+
+ if (dlen >= len)
+ strbuf_grow(sb, dlen - len);
+ memmove(sb->buf + pos + dlen,
+ sb->buf + pos + len,
+ sb->len - pos - len);
+ memcpy(sb->buf + pos, data, dlen);
+ strbuf_setlen(sb, sb->len + dlen - len);
+}
+
void strbuf_add(struct strbuf *sb, const void *data, size_t len) {
strbuf_grow(sb, len);
memcpy(sb->buf + sb->len, data, len);
diff --git a/strbuf.h b/strbuf.h
index 21fc111..7e009d1 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -55,6 +55,7 @@ extern void strbuf_init(struct strbuf *, size_t);
extern void strbuf_release(struct strbuf *);
extern void strbuf_reset(struct strbuf *);
extern char *strbuf_detach(struct strbuf *);
+extern void strbuf_embed(struct strbuf *, void *, size_t, size_t);
/*----- strbuf size related -----*/
static inline size_t strbuf_avail(struct strbuf *sb) {
@@ -81,6 +82,10 @@ static inline void strbuf_addch(struct strbuf *sb, int c) {
/* inserts after pos, or appends if pos >= sb->len */
extern void strbuf_insert(struct strbuf *, size_t pos, const void *, size_t);
+/* splice pos..pos+len with given data */
+extern void strbuf_splice(struct strbuf *, size_t pos, size_t len,
+ const void *, size_t);
+
extern void strbuf_add(struct strbuf *, const void *, size_t);
static inline void strbuf_addstr(struct strbuf *sb, const char *s) {
strbuf_add(sb, s, strlen(s));
--
1.5.3.1
^ permalink raw reply related
* [RFC] strbuf's in builtin-apply
From: Pierre Habouzit @ 2007-09-15 14:12 UTC (permalink / raw)
To: git, Junio C Hamano
[-- Attachment #1: Type: text/plain, Size: 1907 bytes --]
I promised it, here it comes. It comes though with some possible
problems, so I'd like to see them discussed first.
builtin-apply.c was messing with its custom buffers a lot,
reallocating the buffers and so on, hence I had to create strbuf_embed
to have a strbuf-safe API doing exactly the same. Though, because of the
"I want an extra NUL in the end"-invariant, this can come with a quite
high price. in the second patch, there is a hunk:
- nsize = got;
- nbuf = convert_to_git(path, buf, &nsize);
- if (nbuf) {
- free(buf);
- *buf_p = nbuf;
- *alloc_p = nsize;
- *size_p = nsize;
- }
- return got != size;
+
+ nsize = buf->len;
+ nbuf = convert_to_git(path, buf->buf, &nsize);
+ if (nbuf)
+ strbuf_embed(buf, nbuf, nsize, nsize);
+ return 0;
Here, I've not been able to check if convert_to_git was in fact always
dealing with a NUL-terminated buffer (That would be in fact nsize+1) or
not, hence here this strbuf_embed will likely perform a realloc. I don't
know git enough to know if this can become an horrible burden though.
Another suspicious hunk is:
- data = (void*) fragment->patch;
[...]
case BINARY_LITERAL_DEFLATED:
- free(desc->buffer);
- desc->buffer = data;
- dst_size = fragment->size;
- break;
+ strbuf_embed(buf, fragment->patch, fragment->size, fragment->size);
+ return 0;
TTBOMK the ->patch pointer is a pointer inside a buffer, not a buffer
that has been malloc'ed (and there are code paths before my patch that
would still realloc the buffer so I don't think I introduce an issue).
It passes the test-suite without crashing, but well, maybe this should
be a copy instead.
The rest is pretty straightforward.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: metastore (was: Track /etc directory using Git)
From: Johannes Schindelin @ 2007-09-15 14:10 UTC (permalink / raw)
To: martin f krafft
Cc: git, Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
David Härdeman
In-Reply-To: <20070915132632.GA31610@piper.oerlikon.madduck.net>
Hi,
On Sat, 15 Sep 2007, martin f krafft wrote:
> The problem with metdata getting corrupted, which Nicolas reported,
> may well have to do with the use of a single file.
Then the tool is corrupt. Introducing a shadow hierarchy, as you propose,
is very inefficient.
> Anyway, this *really* should go into git itself!
No. Git is a source code management system. Everything else that you can
do with it is a bonus, a second class citizen. Should we really try to
support your use case, we will invariably affect the primary use case.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git-svn: remove --first-parent, add --upstream
From: Lars Hjemli @ 2007-09-15 14:08 UTC (permalink / raw)
To: Peter Baumann; +Cc: Eric Wong, Junio C Hamano, git
In-Reply-To: <8c5c35580709070313l4b815ddbg70be8fb0aef4eefd@mail.gmail.com>
On 9/7/07, Lars Hjemli <hjemli@gmail.com> wrote:
> On 9/7/07, Peter Baumann <waste.manager@gmx.de> wrote:
> > lets reset 'trunk' to its state before the merge and
> > 'branch1' to the merge commit, before fixing the bug in 'branch1'.
> >
> > a-b-c-d-e trunk
> > \ \
> > \ -x-y m branch1
>
> Yeah, this would certainly not be handled correctly by dcommit using
> --first-parent (but it could be handled by (a correct implementation
> of) --upstream).
Actually, I don't think there's any way to handle this correctly. The
current git-svn will do the right thing except in cases like the one
you described, and in these cases it can be _forced_ to do the right
thing by editing the grafts file, so I'll drop the whole --upstream
idea.
--
larsh
^ permalink raw reply
* Re: Data Integrity & un-Commited Branches
From: David Kastrup @ 2007-09-15 13:59 UTC (permalink / raw)
To: Nikodemus Siivola
Cc: Shawn O. Pearce, Jan Hudec, Brian Scott Dobrovodsky, git
In-Reply-To: <6bcc356f0709150611i97d31f0yb91016e53c4f5e9f@mail.gmail.com>
"Nikodemus Siivola" <nikodemus@random-state.net> writes:
> One thing that I've been bitten a couple of times is that
> I think I'm on branch X, which should be clean, whereas
> I'm really on branch Y with uncommitted changes. Then I
> checkout another branch, and see the uncommitted work -- and
> given that I have a couple of dozen related feature branches
> in my tree it may take a while to figure which branch the
> uncommitted work came from.
"Take a while"? What's wrong with git-reflog?
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* metastore (was: Track /etc directory using Git)
From: martin f krafft @ 2007-09-15 13:26 UTC (permalink / raw)
To: git; +Cc: Thomas Harning Jr., Francis Moreau, Nicolas Vilz,
David Härdeman
In-Reply-To: <e47324780709141031t79981b04q3a91984668ea723e@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1737 bytes --]
also sprach Thomas Harning Jr. <harningt@gmail.com> [2007.09.14.1931 +0200]:
> > No, and I did not look anywhere, but I know of no other VCS that can
> > adequatly track permissions.
> Has anyone checked out metastore? http://repo.or.cz/w/metastore.git
> ... there's an XML error in there somewhere, so its not loading the
> 'main' page, but http://repo.or.cz/w/metastore.git?a=shortlog should
> work.
This looks interesting, though I guess getfacl/setfacl and
getfattr/setfattr can pretty much do the same job, especially if you
can call them from shell scripts/hooks (except for mtime). Or did
I misunderstand something?
The problem with metdata getting corrupted, which Nicolas reported,
may well have to do with the use of a single file. It may be worth
to consider using a shadow hierarchy of files, each containing the
metadata, e.g. for a project with foo, and bar/foo and bar/baz
files, you might have
.metastore/foo
.metastore/bar/.<uniqueid>.dir
.metastore/bar/foo
.metastore/bar/baz
and each file could just be an rfc822-style file:
Owner: root
Group: root
Mode: 4754
Mtime: 1234567890
Fattr-<key1>: <value1>
Fattr-<key2>: <value2>
This would be my approach, which should probably be a little better
at preventing corruption.
Anyway, this *really* should go into git itself!
--
martin; (greetings from the heart of the sun.)
\____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck
"i like wagner's music better than anybody's. it is so loud that one
can talk the whole time without other people hearing what one says."
-- oscar wilde
spamtraps: madduck.bogus@madduck.net
[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Data Integrity & un-Commited Branches
From: Nikodemus Siivola @ 2007-09-15 13:11 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Jan Hudec, Brian Scott Dobrovodsky, git
In-Reply-To: <20070915075144.GB3099@spearce.org>
One thing that I've been bitten a couple of times is that
I think I'm on branch X, which should be clean, whereas
I'm really on branch Y with uncommitted changes. Then I
checkout another branch, and see the uncommitted work -- and
given that I have a couple of dozen related feature branches
in my tree it may take a while to figure which branch the
uncommitted work came from.
It would be nice if the "uncommitted changes" message when
swithching branches told you which branch you came from...
Cheers,
-- Nikodemus
^ permalink raw reply
* Re: git commit workflow question
From: David Watson @ 2007-09-15 12:07 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Shawn O. Pearce, Brian Swetland, git
In-Reply-To: <BCFC81AB-0EDD-4C3C-B7D4-DEC60E2565C3@wincent.com>
<shameless plug>The Eclipse plugin (egit) actually already supports this
feature, and I use it all the time. It's incredibly handy, since I can
stag things as much as I want, and then commit then piecemeal.</shameless
plug>
However, I'm not convinced this should necessarily be included in git, at
least not by editing the commit message. Perhaps it should be a task for a
porcelain? I know you can do something similar using git-gui as
well, just by clicking on files in its top view to stage/unstage. Or like
rebase -i, where you first get a buffer with a list of stuff to be done,
and then in a separate buffer you get the commit message.
On Sat, Sep 15, 2007 at 01:31:37PM +0200, Wincent Colaiuta wrote:
> El 14/9/2007, a las 20:14, Shawn O. Pearce escribi?:
>
> > I'm not sure how the Git community would react to being able to edit
> > the list of files being committed from within the commit message
> > buffer. I think most Git users run at least `git diff --cached`
> > before they commit to make sure they are happy with the difference.
> > I know a lot of users who do that.
>
> Yes, I generally check what's in the index before going ahead with a commit; in
> fact I have the following alias in my .bash_profile so that I can just type
> "staged" to see what'll be in the commit, along with an "unstaged" alias for
> the opposite:
>
> alias staged='git diff --cached'
>
> Having said that, it would be very useful to be able to edit the list within
> the commit message buffer for those occasions where you realise that stuff you
> have staged in the index really should be two separate commits. It would enable
> this very simple workflow:
>
> 1. review changes, realize that some of the changes belong in a separate
> commit
> 2. commit, omitting the unwanted changes
> 3. commit again, this time with the remainder of the changes
>
> Without the ability to edit the list within the commit message buffer your
> workflow becomes a bit more cumbersome:
>
> 1. review changes, realize that some of the changes belong in a separate
> commit
> 2a. explicitly pass files to commit on the commandline (cumbersome if number
> of files is large); or:
> 2b. use git-commit --interactive (again can be relatively cumbersome); or:
> 2c. explicitly unstage unwanted files, commit, then restage them and commit
>
> So, yes, the proposed functionality isn't necessary by any means, but it would
> make some nice usability sugar. I know that in the past my experience with
> other SCMs that can do this has made me mistakenly believe that Git does too.
>
> Cheers,
> Wincent
>
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Dave Watson
Software Engineer
MIMvista Corp
^ permalink raw reply
* Re: [PATCH 3/3] rev-list --bisect: Bisection "distance" clean up.
From: Christian Couder @ 2007-09-15 11:47 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio Hamano, git
In-Reply-To: <Pine.LNX.4.64.0709151215090.28586@racer.site>
Hi Dscho,
These are a bunch of cleanup patchs that I had already sent.
See:
http://thread.gmane.org/gmane.comp.version-control.git/54345
http://thread.gmane.org/gmane.comp.version-control.git/54346
http://thread.gmane.org/gmane.comp.version-control.git/54344
http://thread.gmane.org/gmane.comp.version-control.git/54347
Yeah, I should have said it somewhere, sorry about that.
Christian.
Le samedi 15 septembre 2007, Johannes Schindelin a écrit :
> Hi Christian,
>
> you did not provide any meaningful commit message. I feel that this
> would be needed in this case, _especially_ since there was no cover
> letter to describe what the heck this patch series fixes.
>
> Ciao,
> Dscho
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: git commit workflow question
From: Wincent Colaiuta @ 2007-09-15 11:31 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Brian Swetland, git
In-Reply-To: <20070914181417.GU3099@spearce.org>
El 14/9/2007, a las 20:14, Shawn O. Pearce escribió:
> I'm not sure how the Git community would react to being able to edit
> the list of files being committed from within the commit message
> buffer. I think most Git users run at least `git diff --cached`
> before they commit to make sure they are happy with the difference.
> I know a lot of users who do that.
Yes, I generally check what's in the index before going ahead with a
commit; in fact I have the following alias in my .bash_profile so
that I can just type "staged" to see what'll be in the commit, along
with an "unstaged" alias for the opposite:
alias staged='git diff --cached'
Having said that, it would be very useful to be able to edit the list
within the commit message buffer for those occasions where you
realise that stuff you have staged in the index really should be two
separate commits. It would enable this very simple workflow:
1. review changes, realize that some of the changes belong in a
separate commit
2. commit, omitting the unwanted changes
3. commit again, this time with the remainder of the changes
Without the ability to edit the list within the commit message buffer
your workflow becomes a bit more cumbersome:
1. review changes, realize that some of the changes belong in a
separate commit
2a. explicitly pass files to commit on the commandline (cumbersome
if number of files is large); or:
2b. use git-commit --interactive (again can be relatively
cumbersome); or:
2c. explicitly unstage unwanted files, commit, then restage them
and commit
So, yes, the proposed functionality isn't necessary by any means, but
it would make some nice usability sugar. I know that in the past my
experience with other SCMs that can do this has made me mistakenly
believe that Git does too.
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH 3/3] rev-list --bisect: Bisection "distance" clean up.
From: Johannes Schindelin @ 2007-09-15 11:15 UTC (permalink / raw)
To: Christian Couder; +Cc: Junio Hamano, git
In-Reply-To: <20070915130016.eac885f4.chriscool@tuxfamily.org>
Hi Christian,
you did not provide any meaningful commit message. I feel that this would
be needed in this case, _especially_ since there was no cover letter to
describe what the heck this patch series fixes.
Ciao,
Dscho
^ permalink raw reply
* [PATCH 3/3] rev-list --bisect: Bisection "distance" clean up.
From: Christian Couder @ 2007-09-15 11:00 UTC (permalink / raw)
To: Junio Hamano; +Cc: git
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-rev-list.c | 18 +++++++-----------
1 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index c2ce1fc..6ade3b7 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -189,7 +189,7 @@ static int count_interesting_parents(struct commit *commit)
return count;
}
-static inline int halfway(struct commit_list *p, int distance, int nr)
+static inline int halfway(struct commit_list *p, int nr)
{
/*
* Don't short-cut something we are not going to return!
@@ -202,8 +202,7 @@ static inline int halfway(struct commit_list *p, int distance, int nr)
* 2 and 3 are halfway of 5.
* 3 is halfway of 6 but 2 and 4 are not.
*/
- distance *= 2;
- switch (distance - nr) {
+ switch (2 * weight(p) - nr) {
case -1: case 0: case 1:
return 1;
default:
@@ -322,7 +321,7 @@ static struct commit_list *find_bisection(struct commit_list *list,
static struct commit_list *do_find_bisection(struct commit_list *list,
int nr, int *weights)
{
- int n, counted, distance;
+ int n, counted;
struct commit_list *p;
counted = 0;
@@ -373,15 +372,13 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
for (p = list; p; p = p->next) {
if (p->item->object.flags & UNINTERESTING)
continue;
- n = weight(p);
- if (n != -2)
+ if (weight(p) != -2)
continue;
- distance = count_distance(p);
+ weight_set(p, count_distance(p));
clear_distance(list);
- weight_set(p, distance);
/* Does it happen to be at exactly half-way? */
- if (halfway(p, distance, nr))
+ if (halfway(p, nr))
return p;
counted++;
}
@@ -419,8 +416,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
weight_set(p, weight(q));
/* Does it happen to be at exactly half-way? */
- distance = weight(p);
- if (halfway(p, distance, nr))
+ if (halfway(p, nr))
return p;
}
}
--
1.5.3.1.20.g2a16-dirty
^ permalink raw reply related
* [PATCH 2/3] rev-list --bisect: Move some bisection code into best_bisection.
From: Christian Couder @ 2007-09-15 11:00 UTC (permalink / raw)
To: Junio Hamano; +Cc: git
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-rev-list.c | 19 +++++++++++++++----
1 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index f32c712..c2ce1fc 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -258,6 +258,8 @@ static void show_list(const char *debug, int counted, int nr,
static struct commit_list *do_find_bisection(struct commit_list *list,
int nr, int *weights);
+static struct commit_list *best_bisection(struct commit_list *list, int nr);
+
/*
* zero or positive weight is the number of interesting commits it can
* reach, including itself. Especially, weight = 0 means it does not
@@ -321,7 +323,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
int nr, int *weights)
{
int n, counted, distance;
- struct commit_list *p, *best;
+ struct commit_list *p;
counted = 0;
@@ -426,9 +428,17 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
show_list("bisection 2 counted all", counted, nr, list);
/* Then find the best one */
- counted = -1;
+ return best_bisection(list, nr);
+}
+
+static struct commit_list *best_bisection(struct commit_list *list, int nr)
+{
+ struct commit_list *p, *best;
+ int best_distance = -1;
+
best = list;
for (p = list; p; p = p->next) {
+ int distance;
unsigned flags = p->item->object.flags;
if (revs.prune_fn && !(flags & TREECHANGE))
@@ -436,11 +446,12 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
distance = weight(p);
if (nr - distance < distance)
distance = nr - distance;
- if (distance > counted) {
+ if (distance > best_distance) {
best = p;
- counted = distance;
+ best_distance = distance;
}
}
+
return best;
}
--
1.5.3.1.20.g2a16-dirty
^ permalink raw reply related
* [PATCH 1/3] rev-list --bisect: Move finding bisection into do_find_bisection.
From: Christian Couder @ 2007-09-15 10:59 UTC (permalink / raw)
To: Junio Hamano; +Cc: git
This factorises some code and make a big function smaller.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
builtin-rev-list.c | 40 +++++++++++++++++++++++++---------------
1 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index ac551d5..f32c712 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -255,6 +255,9 @@ static void show_list(const char *debug, int counted, int nr,
}
#endif /* DEBUG_BISECT */
+static struct commit_list *do_find_bisection(struct commit_list *list,
+ int nr, int *weights);
+
/*
* zero or positive weight is the number of interesting commits it can
* reach, including itself. Especially, weight = 0 means it does not
@@ -272,7 +275,7 @@ static void show_list(const char *debug, int counted, int nr,
static struct commit_list *find_bisection(struct commit_list *list,
int *reaches, int *all)
{
- int n, nr, on_list, counted, distance;
+ int nr, on_list;
struct commit_list *p, *best, *next, *last;
int *weights;
@@ -301,6 +304,25 @@ static struct commit_list *find_bisection(struct commit_list *list,
*all = nr;
weights = xcalloc(on_list, sizeof(*weights));
+
+ /* Do the real work of finding bisection commit. */
+ best = do_find_bisection(list, nr, weights);
+
+ if (best)
+ best->next = NULL;
+
+ *reaches = weight(best);
+ free(weights);
+
+ return best;
+}
+
+static struct commit_list *do_find_bisection(struct commit_list *list,
+ int nr, int *weights)
+{
+ int n, counted, distance;
+ struct commit_list *p, *best;
+
counted = 0;
for (n = 0, p = list; p; p = p->next) {
@@ -357,12 +379,8 @@ static struct commit_list *find_bisection(struct commit_list *list,
weight_set(p, distance);
/* Does it happen to be at exactly half-way? */
- if (halfway(p, distance, nr)) {
- p->next = NULL;
- *reaches = distance;
- free(weights);
+ if (halfway(p, distance, nr))
return p;
- }
counted++;
}
@@ -400,12 +418,8 @@ static struct commit_list *find_bisection(struct commit_list *list,
/* Does it happen to be at exactly half-way? */
distance = weight(p);
- if (halfway(p, distance, nr)) {
- p->next = NULL;
- *reaches = distance;
- free(weights);
+ if (halfway(p, distance, nr))
return p;
- }
}
}
@@ -425,12 +439,8 @@ static struct commit_list *find_bisection(struct commit_list *list,
if (distance > counted) {
best = p;
counted = distance;
- *reaches = weight(p);
}
}
- if (best)
- best->next = NULL;
- free(weights);
return best;
}
--
1.5.3.1.20.g2a16-dirty
^ permalink raw reply related
* Re: [ANNOUNCE] tig-0.9
From: Jonas Fonseca @ 2007-09-15 9:43 UTC (permalink / raw)
To: Brian Gernhardt; +Cc: Steven Grimm, git
In-Reply-To: <719DD02E-5BA3-477E-ABBB-C385BF43EF9E@silverinsanity.com>
Brian Gernhardt <benji@silverinsanity.com> wrote Fri, Sep 14, 2007:
>
> On Sep 14, 2007, at 1:32 PM, Steven Grimm wrote:
>
> >Brian Gernhardt wrote:
> >>Complete build failure using autoconf here. Just using the
> >>Makefile like I always have works fine, but "autoconf ; ./
> >>configure" (from the git repo) fails with "configure: error: iconv
> >>() not found. Please install libiconv." This confuses me because
> >>I have /usr/lib/libiconv.dylib, and compiling with -liconv works.
> >>I fail to have the autoconf-foo to figure out what's wrong.
> >
> >Try "aclocal ; autoconf ; ./configure".
>
> Yes, remembering to regenerate the configure script would have been
> smart. Works perfectly, thank you.
Great, I will probably roll out a 0.9.1 with this fix tomorrow.
--
Jonas Fonseca
^ permalink raw reply
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