* Re: git-cvsserver wart?
From: Martin Langhoff @ 2006-05-26 3:28 UTC (permalink / raw)
To: Cameron McBride; +Cc: git
In-Reply-To: <dcedf5e20605252024q5bf51486o7cbf6cc396b18b5d@mail.gmail.com>
On 5/26/06, Cameron McBride <cameron.mcbride@gmail.com> wrote:
> WARN - file 'index' already exists in the git repository
This warning means that you are running git-cvsserver off a repo where
you also have a checkout. git-cvsserver really expects to be running
off a 'naked' or 'bare' repo. For read only ops I think it kind-of
works in a 'checkout' repo, but commits are a different story.
cheers,
m
^ permalink raw reply
* Re: Git 1.3.2 on Solaris
From: Stefan Pfetzing @ 2006-05-26 3:30 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <f3d7535d0605222020j2d581bd9j602752659a4b3ac2@mail.gmail.com>
Hi list,
2006/5/23, Stefan Pfetzing <stefan.pfetzing@gmail.com>:
> 2006/5/17, Jason Riedy <ejr@eecs.berkeley.edu>:
> > And pkgsrc itself works just fine without the silly g prefix,
> > or at least does for me as a mere user (and as well as it does
> > work). But if you intend on adding the package upstream, it'll
> > need something to cope with the g. And pkgsrc handles local
> > patches...
>
> Well I had some problems on NetBSD without the g prefix for the
> gnu coreutils - since then I always used that prefix.
...
Well finally - after some patching around access() and after figuring
out "merge" was broken in pkgsrc (and still is - I had to open a
problem report) - I got all tests to complete successfully.
bye
Stefan
P.S.: merge from devel/rcs uses /bin/diff3 on solaris which somehow
breaks merge.
--
http://www.dreamind.de/
Oroborus and Debian GNU/Linux Developer.
^ permalink raw reply
* Re: git-cvsserver wart?
From: Cameron McBride @ 2006-05-26 3:34 UTC (permalink / raw)
To: Martin Langhoff, git
In-Reply-To: <46a038f90605252028h556d0b2aob43f5c3dca8a5392@mail.gmail.com>
On 5/25/06, Martin Langhoff <martin.langhoff@gmail.com> wrote:
> On 5/26/06, Cameron McBride <cameron.mcbride@gmail.com> wrote:
> > WARN - file 'index' already exists in the git repository
>
> This warning means that you are running git-cvsserver off a repo where
> you also have a checkout. git-cvsserver really expects to be running
> off a 'naked' or 'bare' repo. For read only ops I think it kind-of
> works in a 'checkout' repo, but commits are a different story.
hmmm, it is supposed to be a bare repo. perhaps I flubbed something
up. Can I safely delete this index file? It's in the repo.git
directory. Perhaps it got created on the initial import from CVS
(using cvsimport and cvsps)?
Cameron
^ permalink raw reply
* Re: git-cvsserver wart?
From: Cameron McBride @ 2006-05-26 3:57 UTC (permalink / raw)
To: Martin Langhoff; +Cc: git
In-Reply-To: <46a038f90605252023v5ff3fd65l9a991b3bbfa0a024@mail.gmail.com>
On 5/25/06, Martin Langhoff <martin.langhoff@gmail.com> wrote:
> On 5/26/06, Cameron McBride <cameron.mcbride@gmail.com> wrote:
> Ok. You might want to retain that latest, it has some further fixes ;-)
sounds good.
> Yes, I was guessing as much. I am still curious about what parameters
> (and in what order) cvs 1.11.7 sends...
sure! now that I know CVS_CLIENT_LOG exists ... ;)
It points in basically the same direction, which is to say the newer
client passes an 'Argument -- ' which is what git-cvsserver seems to
be choking on.
The 'in' versions of the two
noup: cvs 1.11.1p1 that failed plane cvs update
work: cvs 1.11.17 that worked
--- cvs_noup.log.in 2006-05-25 23:44:21.000000000 -0400
+++ cvs_work.log.in 2006-05-25 23:48:38.000000000 -0400
@@ -1,6 +1,7 @@
Root /export/home/cameron/ws/git_test/ntropy.git
-Valid-responses ok error Valid-requests Checked-in New-entry Checksum
Copy-file Updated Created Update-existing Merged Patched Rcs-diff Mode
Mod-time Removed Remove-entry Set-static-directory
Clear-static-directory Set-sticky Clear-sticky Template
Set-checkin-prog Set-update-prog Notified Module-expansion
Wrapper-rcsOption M Mbinary E F MT
+Valid-responses ok error Valid-requests Checked-in New-entry Checksum
Copy-file Updated Created Update-existing Merged Patched Rcs-diff Mode
Mod-time Removed Remove-entry Set-static-directory
Clear-static-directory Set-sticky Clear-sticky Template Notified
Module-expansion Wrapper-rcsOption M Mbinary E F MT
valid-requests
+Argument --
Directory .
/export/home/cameron/ws/git_test/ntropy.git/master
Entry /.gitignore/1.1///
Thanks for your help!
Cameron
^ permalink raw reply
* Re: file name case-sensitivity issues
From: Christopher Faylor @ 2006-05-26 3:59 UTC (permalink / raw)
To: git
In-Reply-To: <7vac96ufxv.fsf@assigned-by-dhcp.cox.net>
On Thu, May 25, 2006 at 11:17:48AM -0700, Junio C Hamano wrote:
>I have git installed on a Cygwin on NTFS at work...
Maybe this has been mentioned already but I wanted to point out that
Cygwin's mount has a "managed" option: "mount -o managed c:/foo /foo"
which causes cygwin to encode "problem" characters into the filename.
This means that there is a possibility that you'll run into the Windows
260 character max filename limit sooner so many people don't like to use
this option. However, since only uppercase characters and characters
like ">", ":", etc. are encoded, in practice you wouldn't see path
length problems *from this* very often. There is, of course, some
processing overhead involved in this, too, so using managed mode
will slow things down slightly.
We've been contemplating using Unicode functions in cygwin for a while
since those allow much longer path lengths but this is a massive change
and would potentially cause problems on Windows 9x. There has also been
some discussion of using native NT calls which, I believe, allow case
preservation like linux. However, those have a similar set of problems.
FYI,
cgf
^ permalink raw reply
* Re: t8001-annotate.sh fails on Mac OS X
From: Fredrik Kuivinen @ 2006-05-26 4:32 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Stefan Pfetzing, Git Mailing List
In-Reply-To: <20060526011153.GA27720@spearce.org>
On Thu, May 25, 2006 at 09:11:53PM -0400, Shawn Pearce wrote:
> Stefan Pfetzing <stefan.pfetzing@gmail.com> wrote:
> > Hi,
> >
> > for some reason I could not yet figure out, t8001-annotate.sh fails at test
> > 18.
> >
> > --- snip ---
> > * ok 17: some edit
> > * expecting success: check_count A 1 B 1 B1 1 B2 1 "A U Thor" 1 C 1 D 1
> > Author A (expected 1, attributed 1) good
> > Author B1 (expected 1, attributed 1) good
> > Author D (expected 1, attributed 2) bad
> > Author A U Thor (expected 1, attributed 1) good
> > Author B2 (expected 1, attributed 1) good
> > Author B (expected 1, attributed 1) good
> > * FAIL 18: some edit
> > check_count A 1 B 1 B1 1 B2 1 "A U Thor" 1 C 1 D 1
> > * failed 1 among 18 test(s)
>
> I've been seeing the same failed test case for a long time now on
> my own Mac OS X system. I think it has to do with the "git blame"
> vs. "git annotate" war which never really happened.
>
> I think we had hoped that one of the two tools would prove to be
> _the_ annotation/blame tool and would get used but thus far that
> hasn't happened. Since they are two different implementations
> they also differ slightly over how they attribute a change across
> a merge, and in this case annotate is producing a different result
> from blame - but that different result isn't considered to be wrong
> so it hasn't been changed in annotate. Meanwhile the test has stayed
> broken as a reminder that these two generate different results.
>
I have planned to come up with a nice test suite for blame/annotate,
but I haven't got around to it yet.
I don't see this test failure on my Debian system. But it is true that
for some cases different blame/annotate outputs are equally correct,
however not in this case. Note that the incomplete line is attributed
to the commit with author D, but this commit did clearly not introduce
that line. The only correct answer, for that particular line, is the
commit with author C.
- Fredrik
^ permalink raw reply
* Re: [PATCH] cvsimport: introduce -L<imit> option to workaround memory leaks
From: Linus Torvalds @ 2006-05-26 5:20 UTC (permalink / raw)
To: Martin Langhoff
Cc: Martin Langhoff, Git Mailing List, Junio C Hamano,
Johannes.Schindelin, spyderous, smurf
In-Reply-To: <46a038f90605251742p2435ae23k8bfbb98409a30c1c@mail.gmail.com>
On Fri, 26 May 2006, Martin Langhoff wrote:
>
> Call me slow, but I am still running and rerunning that gentoo import. ;-)
I'm doing it too, just for fun.
Of course, since I'm doing this on a machine that basically has a laptop
disk, the "just for fun" part is a bit sad. It's waiting for disk about
25% of the time ;/
And it's slow as hell. I really wish we could do better on the CVS import
front.
> The current import has reached ~200K commits, and .git is 450MB, while
> the checked out tree is 230MB (680MB with .git). At this stage, git
> repack -a -d is too memory hungry.
I've got 2GB in that puppy, and "repack -a -d" is fine for me. I'm not
quite up to 200k commits yet (I'm at 160k), but the repacking is certainly
faster than the rest of the import.. Gaah.
It's "git-rev-list --objects" that is the memory sucker for me, the
packing itself doesn't seem to be too bad.
The biggest cost seems to be git-write-tree, which is about 0.225 seconds
for me on that tree on that machine. Which _should_ mean that we could do
4 commits a second, but that sure as hell ain't how it works out. It seems
to do about 1.71 commits a second for me on that tree, which is pretty
damn pitiful. Some cvs overhead, and probably some other git overhead too.
(That's a 2GHz Merom, so the fact that you get ~6k commits per hour on
your 2GHz Opteron is about the same speed - I suspect you're also at least
partly limited by disk, our numbers seem to match pretty well).
200k commits at 6k commits per hour is about a day and a half (plus the
occasional packing load). Taking that long to import a CVS archive is
horrible. But I guess it _is_ several years of work, and I guess you
really have to do it only once, but still.
Linus
^ permalink raw reply
* Re: [PATCH] cvsimport: introduce -L<imit> option to workaround memory leaks
From: Jakub Narebski @ 2006-05-26 5:29 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.64.0605252204590.5623@g5.osdl.org>
Linus Torvalds wrote:
> 200k commits at 6k commits per hour is about a day and a half (plus the
> occasional packing load). Taking that long to import a CVS archive is
> horrible. But I guess it _is_ several years of work, and I guess you
> really have to do it only once, but still.
And how parsecvs (which as far as I remember didn't have incremental mode)
compares wrt speed to git-cvsimport? It is supposed to be faster...
--
Jakub Narebski
Warsaw, Poland
^ permalink raw reply
* Re: Clean up sha1 file writing
From: Junio C Hamano @ 2006-05-26 5:36 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <7virnv3qi6.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
>> I think "kompare" (the KDE diff tool) is nicer.
>
> I'd love to give it a whirl, but aptitude says it will consume
> 73.5MB diskspace to install it, with download size 22.4MB, which
> makes me go ... hmmmm (my machines are currently KDE free so the
> above counts slurping in the kdelibs essentials).
It indeed is nicer ;-). I got a good laugh watching it smoothly
move the inserted hunk as I scrolled down.
^ permalink raw reply
* Re: [PATCH] cvsimport: introduce -L<imit> option to workaround memory leaks
From: Martin Langhoff @ 2006-05-26 6:02 UTC (permalink / raw)
To: Linus Torvalds
Cc: Martin Langhoff, Git Mailing List, Junio C Hamano,
Johannes.Schindelin, spyderous, smurf
In-Reply-To: <Pine.LNX.4.64.0605252204590.5623@g5.osdl.org>
On 5/26/06, Linus Torvalds <torvalds@osdl.org> wrote:
> I'm doing it too, just for fun.
Well, it's good to not be so alone in our definition of fun ;-)
> Of course, since I'm doing this on a machine that basically has a laptop
> disk, the "just for fun" part is a bit sad. It's waiting for disk about
> 25% of the time ;/
Ouch.
> And it's slow as hell. I really wish we could do better on the CVS import
> front.
Me too. However, I don't think the perl part is so costly anymore.
It's down to waiting on IO. git-write-tree is also prominently there.
It takes a lot of memory in some writes -- I had thought it'd be
cheaper as it takes one tree object at the time...
I also have a trivial patch that I haven't posted yet, that runs cvsps
to a tempfile, and then reads the file. Serialising the tasks means
that we don't carry around cvsps' memory footprint during the import
itself.
...
> It's "git-rev-list --objects" that is the memory sucker for me, the
> packing itself doesn't seem to be too bad.
No, you're right, it's git-rev-list that gets called during the
repack. But it was pushing everything it could to swap. Once it didn't
fit in memory, it hit a brick wall :(
> The biggest cost seems to be git-write-tree, which is about 0.225 seconds
> for me on that tree on that machine. Which _should_ mean that we could do
> 4 commits a second, but that sure as hell ain't how it works out. It seems
> to do about 1.71 commits a second for me on that tree, which is pretty
> damn pitiful. Some cvs overhead, and probably some other git overhead too.
Well, we _have_ to fetch the file. I guess you are thinking of
extracting if frrom the RCS ,v file directly? One tihng that I found
that seemed to speed things up a bit was to declare TMPDIR to be a
directory in the same partition.
> (That's a 2GHz Merom, so the fact that you get ~6k commits per hour on
> your 2GHz Opteron is about the same speed - I suspect you're also at least
> partly limited by disk, our numbers seem to match pretty well).
Yup. This is _very_ diskbound.
> 200k commits at 6k commits per hour is about a day and a half (plus the
> occasional packing load). Taking that long to import a CVS archive is
> horrible. But I guess it _is_ several years of work, and I guess you
> really have to do it only once, but still.
And it's a huge CVS archive too.
martin
^ permalink raw reply
* Re: git-format-patch possible regressions
From: Marco Costalba @ 2006-05-26 6:09 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git, Linus Torvalds
In-Reply-To: <Pine.LNX.4.63.0605260125420.16816@wbgn013.biozentrum.uni-wuerzburg.de>
On 5/26/06, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Thu, 25 May 2006, Junio C Hamano wrote:
>
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> > > Thinking about this again, it makes more sense not to imply --numbered:
> >
> > Yes, that makes sense. That way you can say "Please start
> > naming the output files at 0032-xxxx.txt, because you gave me 31
> > patch series last time, but I do not want [PATCH x/y] on the
> > subject line, just [PATCH]".
> >
> > That brings up another issue. Don't we need to have another
> > option --total-number that overrides the /y part above?
>
> I thought about that, too. Isn't the --numbered only useful for submitting
> a patch series via mail? And isn't it necessary to make certain that these
> patches really apply in that order? Isn't it then sensible to force the
> user to have a branch (at least a throw-away one) having exactly these
> patches, just to make sure that the patches really, really apply in that
> order?
>
> If all that is true, then --start-number && --numbered does not make sense
> at all.
>
I was thinking, probably wrong, that the number prepended in file name
is used also to disambiguate two patches with the same subject.
Marco
^ permalink raw reply
* Re: git-format-patch possible regressions
From: Junio C Hamano @ 2006-05-26 6:16 UTC (permalink / raw)
To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550605252309h2c4b74bcp50b095e09e6c133f@mail.gmail.com>
"Marco Costalba" <mcostalba@gmail.com> writes:
> I was thinking, probably wrong, that the number prepended in file name
> is used also to disambiguate two patches with the same subject.
What Johannes and I were discussing was the other number -- the
total in the series. IOW, y in "[PATCH x/y]". OTOH, the number
used for disambiguation you care about is x, which is made
adjustable with --start-number patch.
So the way for qgit to use it would become something like this.
Instead of giving a list of ranges like "a..b c..d e..f":
* Run "format-patch a..b"; by reading from its stdout you know
what patches you got -- you count them.
* Run "format-patch --start-number=6 c..d" (if you got 5 out of
a..b);
* Run "format-patch --start-number=n e..f" (now you know the drill).
Then the sequence out of c..d would start with a file 0006-xxxx.txt,
which is what you want for disambiguation.
^ permalink raw reply
* Re: git-format-patch possible regressions
From: Jakub Narebski @ 2006-05-26 6:26 UTC (permalink / raw)
To: git
In-Reply-To: <7v4pzdqpit.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> So the way for qgit to use it would become something like this.
> Instead of giving a list of ranges like "a..b c..d e..f":
>
> * Run "format-patch a..b"; by reading from its stdout you know
> what patches you got -- you count them.
>
> * Run "format-patch --start-number=6 c..d" (if you got 5 out of
> a..b);
[...]
I still think that having _shortcut notation_ being different for very
different commands is not a bad idea.
If one is really concerned about consistency of rev-list options, we could
use ',' or something to separate separate lists of commits, e.g.
git format-patch a..b , c..d , e..f
or
git format-patch a..b --then c..d --then e..f
What do you think about the idea?
--
Jakub Narebski
Warsaw, Poland
^ permalink raw reply
* Re: [PATCH 5/6] More accurately detect header lines in read_one_header_line
From: Junio C Hamano @ 2006-05-26 7:29 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: git
In-Reply-To: <m1mzd8iklr.fsf_-_@ebiederm.dsl.xmission.com>
ebiederm@xmission.com (Eric W. Biederman) writes:
> Only count lines of the form '^.*: ' and '^From ' as email
> header lines.
I am having trouble with this patch.
> diff --git a/mailinfo.c b/mailinfo.c
> index 99989c2..c642ff4 100644
> --- a/mailinfo.c
> +++ b/mailinfo.c
> @@ -385,20 +385,29 @@ static int read_one_header_line(char *li
> {
> int ofs = 0;
> while (ofs < sz) {
> + const char *colon;
> int peek, len;
> if (fgets(line + ofs, sz - ofs, in) == NULL)
> + break;
> len = eatspace(line + ofs);
> if (len == 0)
> + break;
> + colon = strchr(line, ':');
> + if (!colon || !isspace(colon[1])) {
> + /* Readd the newline */
> + line[ofs + len] = '\n';
> + line[ofs + len + 1] = '\0';
> + break;
> }
Because eatspace() eats the trailing space, although your commit
message say lines matching "^.*: " are headers, this does not
match the criteria:
X-Spam-Checker-Version: SpamAssassin 3.1.1 (2006-03-10) on
gitster.siamese.dyndns.org
-> X-Spam-Level:
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00
autolearn=ham version=3.1.1
Notice that the field body for this unstructured header
X-Spam-Level (an optional field) consists of a single
whitespace. It will be gone because of eatspace() when your
check sees the line, so the header parsing stops prematurely.
Was there a particular reason you needed this change? That is,
did you have to parse mail-looking input that does not have a
blank line between runs of headers and the body of the message?
If so, I'd at least like to remove the || !isspace(colon[1])
from the test. After all, I do not think RFC2822 requires a
whitespace after the colon there.
^ permalink raw reply
* Re: [PATCH 5/6] More accurately detect header lines in read_one_header_line
From: Eric W. Biederman @ 2006-05-26 8:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr72hns7h.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> Junio C Hamano <junkio@cox.net> writes:
>
>> Was there a particular reason you needed this change? That is,
>> did you have to parse mail-looking input that does not have a
>> blank line between runs of headers and the body of the message?
Yes. I had patches that had a subject line followed by a blank line,
and the problem was that the old check thought the subject was a
header line, despite not even having a colon in it.
>> If so, I'd at least like to remove the || !isspace(colon[1])
>> from the test. After all, I do not think RFC2822 requires a
>> whitespace after the colon there.
>
> In other words, something like this (tested):
Looks good to me, sorry for missing that one.
Eric
^ permalink raw reply
* Re: [RFC][PATCH] Allow transfer of any valid sha1
From: Eric W. Biederman @ 2006-05-26 8:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7wpsu5c.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> ebiederm@xmission.com (Eric W. Biederman) writes:
>
>> So fixing fetch-pack is easy and pretty non-controversial.
>> The patch below handles that.
>
> I am at work so I cannot really spend time on this right now,
> but I am OK with letting it send arbitrary SHA1 the caller
> obtained out of band. I do not know about your implementation,
> since I haven't really looked at it.
Agreed. I'm not certain about my implementation yet either I
just know I was in the ball park.
I needed the conversation to understand what the limits were.
>> (The movement of filter_refs may actually be overkill)
>
> It may not just overkill but may actively be wrong, but again I
> haven't looked at it yet.
>
> Will take a look tonight.
Sure. The code was all a work in progress so I don't expect to
have all of the details ironed out. In particular I didn't
even look at the non fetch-pack case, and I didn't update the
documentation.
Eric
^ permalink raw reply
* Re: [RFC][PATCH] Allow transfer of any valid sha1
From: Eric W. Biederman @ 2006-05-26 8:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7virntsto6.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> ebiederm@xmission.com (Eric W. Biederman) writes:
>> - It feels really weird when everything else allows me to use sha1s
>> for git-fetch to deny them.
>
> That is a real argument and I am not opposed to change
> fetch-pack to ask for an arbitrary SHA1 the caller obtained out
> of band.
Good this was the primary reason I kept pursuing the issue after
I figured out what it was.
>> Then there is the big hole in my plan to get better changelog information
>> that it appears that after Andrew pulls a branch he resolves some
>> merge conflicts. If that is right I need to figure out how to address
>> that before I can improve git-quiltimport.sh.
>
> The last time I talked with Andrew, he is not doing a merge nor
> resolving merge conflicts. He treats git primarily as a
> patchbomb distribution mechanism, and works on (a rough
> equivalent of) the output of format-patch from merge base
> between his base tree and individual subsystem tree. After that
> things are normal quilt workflow outside git, whatever it is.
That sounds right. I just know that there I had some strange
merge conflicts on the second git tree I pulled from. Something
about a file being added twice. It was one thing too many to
investigate this round.
Eric
^ permalink raw reply
* Re: [RFC][PATCH] Allow transfer of any valid sha1
From: Junio C Hamano @ 2006-05-26 10:04 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: Linus Torvalds, git
In-Reply-To: <m13bexetj1.fsf@ebiederm.dsl.xmission.com>
ebiederm@xmission.com (Eric W. Biederman) writes:
> diff --git a/fetch-pack.c b/fetch-pack.c
> index a3bcad0..c767d84 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -260,6 +260,27 @@ static void mark_recent_complete_commits
> }
> }
>
> +static struct ref **get_sha1_heads(struct ref **refs, int nr_heads, char **head)
> +{
> + int i;
> + for (i = 0; i < nr_heads; i++) {
> + struct ref *ref;
> + unsigned char sha1[20];
> + char *s = head[i];
> + int len = strlen(s);
> +
> + if (len != 40 || get_sha1_hex(s, sha1))
> + continue;
So the new convention is fetch-pack can take ref name (as
before), or a bare 40-byte hexadecimal. I think sane people
would not use ambiguous refname that says "deadbeef" five times,
and even if the do so they could disambiguate by explicitly
saying "refs/heads/" followed by "deadbeef" five times, so it
should be OK.
> +
> + ref = xcalloc(1, sizeof(*ref) + len + 1);
> + memcpy(ref->old_sha1, sha1, 20);
> + memcpy(ref->name, s, len + 1);
> + *refs = ref;
> + refs = &ref->next;
> + }
> + return refs;
> +}
> +
This function takes the pointer to a location that holds a
pointer to a "struct ref" -- it is the location to store the
newly allocated ref structure, i.e. the next pointer of the last
element in the list. When it returns, the location pointed at
by the pointer given to you points at the first element you
allocated, and it returns the next pointer of the last element
allocated by it. That is the same calling convention as
connect.c::get_remote_heads(). So when calling this function to
append to a list you already have, you would give the next
pointer to the last element of the existing list. But you do
not seem to do that.
I think the body of fetch_pack() should become something like:
struct ref *ref, **tail;
tail = get_remote_heads(fd[0], &ref, 0, NULL, 0);
if (server_supports("multi_ack")) {
...
}
tail = get_sha1_heads(tail, nr_match, match);
if (everything_local(&ref, nr_match, match)) {
...
> @@ -311,6 +332,8 @@ static int everything_local(struct ref *
> if (cutoff)
> mark_recent_complete_commits(cutoff);
>
> + filter_refs(refs, nr_match, match);
> +
I am not sure about this change.
In the original code we do not let get_remote_heads() to filter
the refs but call filter_refs() after the "mark all complete
remote refs as common" step for a reason. Even though we may
not be fetching from some remote refs, we would want to take
advantage of the knowledge of what objects they have so that we
can mark as many objects as common as possible in the early
stage. I suspect this change defeats that optimization.
So instead I would teach "mark all complete remote refs" loop
that not everything in refs list is a valid remote ref, and skip
what get_sha1_heads() injected, because these arbitrary ones we
got from the command line are not something we know exist on the
remote side. Maybe something like this.
/*
* Mark all complete remote refs as common refs.
* Don't mark them common yet; the server has to be told so first.
*/
for (ref = *refs; ref; ref = ref->next) {
struct object *o;
if (ref is SHA1 from the command line)
continue;
o = deref_tag(lookup_object(ref->old_sha1), NULL, 0);
if (!o || o->type != commit_type || !(o->flags & COMPLETE))
continue;
...
To implement "ref is SHA1 from the command line", I would add
another 1-bit field to "struct ref" and mark the new ones you
create in get_sha1_heads() as such (existing "force" field
could also become an 1-bit field -- we do not neeed a char).
> @@ -373,6 +394,7 @@ static int fetch_pack(int fd[2], int nr_
> packet_flush(fd[1]);
> die("no matching remote head");
> }
> + get_sha1_heads(&ref, nr_match, match);
I talked about this one already...
> diff --git a/git-parse-remote.sh b/git-parse-remote.sh
> index 187f088..2372df8 100755
> --- a/git-parse-remote.sh
> +++ b/git-parse-remote.sh
> @@ -105,6 +105,7 @@ canon_refs_list_for_fetch () {
> '') remote=HEAD ;;
> refs/heads/* | refs/tags/* | refs/remotes/*) ;;
> heads/* | tags/* | remotes/* ) remote="refs/$remote" ;;
> + [0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F]) ;;
Yuck. Don't we have $_x40 somewhere?
We never use uppercase so at least we could save 24 columns from
here ;-).
^ permalink raw reply
* Re: [PATCH] gitk: Replace "git-" commands with "git "
From: Paul Mackerras @ 2006-05-26 11:31 UTC (permalink / raw)
To: Timo Hirvonen; +Cc: Alex Riesen, git
In-Reply-To: <20060524133455.f78b11a4.tihirvon@gmail.com>
Timo Hirvonen writes:
> Many commands are already built-in so I don't think it's a problem
> anymore.
Which ones are built in now?
Paul.
^ permalink raw reply
* Re: [PATCH] gitk: Replace "git-" commands with "git "
From: Timo Hirvonen @ 2006-05-26 11:59 UTC (permalink / raw)
To: Paul Mackerras; +Cc: raa.lkml, git
In-Reply-To: <17526.59159.484712.500414@cargo.ozlabs.ibm.com>
Paul Mackerras <paulus@samba.org> wrote:
> Timo Hirvonen writes:
>
> > Many commands are already built-in so I don't think it's a problem
> > anymore.
>
> Which ones are built in now?
git-log$X git-whatchanged$X git-show$X \
git-count-objects$X git-diff$X git-push$X \
git-grep$X git-add$X git-rm$X git-rev-list$X \
git-check-ref-format$X \
git-init-db$X git-tar-tree$X git-upload-tar$X git-format-patch$X \
git-ls-files$X git-ls-tree$X \
git-read-tree$X git-commit-tree$X \
git-apply$X git-show-branch$X git-diff-files$X \
git-diff-index$X git-diff-stages$X git-diff-tree$X git-cat-file$X
--
http://onion.dynserv.net/~timo/
^ permalink raw reply
* Re: [PATCH 2/4] tests: Remove heredoc usage inside quotes
From: Herbert Xu @ 2006-05-26 12:22 UTC (permalink / raw)
To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <11486091783808-git-send-email-normalperson@yhbt.net>
On Thu, May 25, 2006 at 07:06:16PM -0700, Eric Wong wrote:
> The use of heredoc inside quoted strings doesn't seem to be
> supported by dash. pdksh seems to handle it fine, however.
This is a bug in dash and should be fixed there instead.
Thanks for drawing my attention to it.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] gitk: Replace "git-" commands with "git "
From: Paul Mackerras @ 2006-05-26 12:26 UTC (permalink / raw)
To: Timo Hirvonen; +Cc: raa.lkml, git
In-Reply-To: <20060526145954.cea5613c.tihirvon@gmail.com>
OK, thanks. I have applied your patch. The ones that gitk uses that
aren't built-in now are git-rev-parse and git-repo-config, which
aren't performance-critical.
Paul.
^ permalink raw reply
* Re: [PATCH 3/4] t5500-fetch-pack: remove local (bashism) usage.
From: Herbert Xu @ 2006-05-26 12:23 UTC (permalink / raw)
To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <11486091793385-git-send-email-normalperson@yhbt.net>
On Thu, May 25, 2006 at 07:06:17PM -0700, Eric Wong wrote:
> None of the variables seem to conflict, so local was unnecessary.
BTW, dash supports (and has always supported) local which is a quite
useful feature.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 4/4] t6000lib: workaround a possible dash bug
From: Herbert Xu @ 2006-05-26 12:33 UTC (permalink / raw)
To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <11486091792604-git-send-email-normalperson@yhbt.net>
On Thu, May 25, 2006 at 07:06:18PM -0700, Eric Wong wrote:
>
> t/t6000lib.sh | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> cba907ce0b1c0927fb15cbb5dd91a4129ff9a950
> diff --git a/t/t6000lib.sh b/t/t6000lib.sh
> index c6752af..d402621 100755
> --- a/t/t6000lib.sh
> +++ b/t/t6000lib.sh
> @@ -69,7 +69,9 @@ on_committer_date()
> {
> _date=$1
> shift 1
> - GIT_COMMITTER_DATE=$_date "$@"
> + export GIT_COMMITTER_DATE="$_date"
> + "$@"
> + unset GIT_COMMITTER_DATE
The original code looks correct to me. So I think this too should
be fixed in dash instead.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: t8001-annotate.sh fails on Mac OS X
From: Johannes Schindelin @ 2006-05-26 13:36 UTC (permalink / raw)
To: Shawn Pearce; +Cc: Stefan Pfetzing, Git Mailing List
In-Reply-To: <20060526011153.GA27720@spearce.org>
Hi,
On Thu, 25 May 2006, Shawn Pearce wrote:
> Stefan Pfetzing <stefan.pfetzing@gmail.com> wrote:
> >
> > for some reason I could not yet figure out, t8001-annotate.sh fails at test
> > 18.
>
> I've been seeing the same failed test case for a long time now on
> my own Mac OS X system.
... which is sort of funny, because I don't see it on my system. Running
an iBook G3 with Mac OS X 10.2.8. "make test" runs through, and no, AFAICT
I do not have any local modifications which could be responsible for that.
Ciao,
Dscho
^ 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