* [PATCH] missing return
From: Jerald Fitzjerald @ 2007-05-30 12:39 UTC (permalink / raw)
To: git; +Cc: jfj
OK for mainline?
---
diff --git a/base85.c b/base85.c
--- a/base85.c
+++ b/base85.c
@@ -66,7 +66,7 @@ int decode_85(char *dst, const char *buf
*/
if (0x03030303 < acc ||
0xffffffff - de < (acc *= 85))
- error("invalid base85 sequence %.5s", buffer-5);
+ return error("invalid base85 sequence %.5s", buffer-5);
acc += de;
say1(" %08x", acc);
^ permalink raw reply
* Re: [PATCH] Introduce git-supported-features for porcelain use
From: Johannes Schindelin @ 2007-05-30 12:34 UTC (permalink / raw)
To: Alex Riesen; +Cc: Shawn O. Pearce, Junio C Hamano, git
In-Reply-To: <81b0412b0705300504h24c4c32ew8460dede4aeffd52@mail.gmail.com>
Hi,
On Wed, 30 May 2007, Alex Riesen wrote:
> On 5/30/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> > As a porcelain author I'm finding it difficult to keep track of
> > what features I can use in git-gui. Newer versions of Git have
> > newer capabilities but they don't always immediately get newer
> > version numbers that I can easily test for.
>
> git-version --features?
Melikes. (Even if it is not strictly separating between plumbing and
porcelain; I consider git-version porcelain...)
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Don't ignore write failure from git-diff, git-log, etc.
From: Jim Meyering @ 2007-05-30 12:25 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <alpine.LFD.0.98.0705291412060.26602@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Tue, 29 May 2007, Jim Meyering wrote:
>> >
>> > Maybe you have not noticed, but my argument has ben about EPIPE.
>>
>> Ha ha. That's a good one.
>> The point was that even you must see that your
>> "[Jim's] WHOLE patch is crap" statement was wrong.
>
> Ehh. That's a rather edited version of what I said, isn't it?
No. I'm glad to see that perhaps even you are surprised by your words.
The only editing was to capitalize WHOLE. Here's what you wrote:
> I think this patch is fundamentally WRONG. This fragment is just a prime
> example of why the whole patch is crap. The old code was correct, and you
> broke it.
Umm... are the above three lines the only part of my message you're
prepared to talk about? You haven't addressed any of the interesting
(technical) parts.
> That's after I explicitly _quoted_ the part where you actively removed the
> code that said "EPIPE is right", and also after I had told you several
> times that you should consider EPIPE as a special case in your other part.
>
> In other words, yes, EVERY SINGLE HUNK of your patch was wrong, and I had
> told you exactly why.
And I told you why I think my patch was on the right track.
i.e., why it now appears to be fine to treat EPIPE like any other error.
It is interesting to see that you've elided all of my arguments rather
than make an attempt to rebut them. I'm trying to keep my side of this
discussion professional, so I'll ignore parts of what you've written
below.
> How wrong does a patch have to be to be "crap"? Maybe I have higher
> standards than you do...
> And now you have trouble accepting that, even after you have sent out a
> fixed patch without the crap. Thanks for finally bothering to get the
> patch right, but I don't see why you have to try to make-believe that it
> was ever about anything but EPIPE.
My original patch was about ENOSPC and EIO. EPIPE was mostly incidental.
I don't care about EPIPE, and think it deserves no special treatment.
You appear to be obsessed with it now, perhaps because SIGPIPE-ignoring
porcelain (now long gone) once caused trouble.
> So go back and read my emails.
Did you read mine? I explained why EPIPE is no longer a problem for
git, even if you're using stock bash-2.05b or bash-3.0.
> You'll see that in every single one I made
> it very clear that EPIPE was special.
No. You merely *said* it was special.
You haven't demonstrated that it's special enough (and still common
enough) to pollute all code that tests for file-close failure. I hear
that it *used to be* common enough to merit such treatment. Now, it is
a much harder case to make. But from what I've seen, you haven't even
tried to do that much. Hmm... or maybe you did try to make the case,
and came up short. Is that why you are resorting to hyperbole, and ad
hominem arguments?
> From the very first one (where I
> didn't call your patch crap, btw: I said it was wrong, and that some
> errors are expected and good, and I explicitly told you about EPIPE).
>
> So what did you do? Instead of acknowledging that EPIPE was different, you
> actually *expanded* on that original patch, and made the other places
> where we _did_ handle EPIPE correctly, and made those places handle it
> _incorrectly_.
>
> And then you expect me to be _polite_ about it? Grow up. I was polite
> before you started explicitly doing the reverse of what I told you you
> should do. At that point, your patch went from "meant well, but the patch
> was wrong" to "That's just obviously crap".
Let's see... I dared to post code contrary to your unsubstantiated claim,
and therefore you describe that code as "obviously crap".
Just because you are Linus doesn't mean you can decree that
"EPIPE must be ignored" and make everyone take it on faith.
Can you substantiate your claim that my proposed changes cause trouble
*in practice*? So far, all I've heard is FUD, and all of my explanations
of why EPIPE no longer matters seem to have been ignored.
^ permalink raw reply
* Re: [PATCH] Introduce git-supported-features for porcelain use
From: Alex Riesen @ 2007-05-30 12:04 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <20070530043113.GA12300@spearce.org>
On 5/30/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> As a porcelain author I'm finding it difficult to keep track of
> what features I can use in git-gui. Newer versions of Git have
> newer capabilities but they don't always immediately get newer
> version numbers that I can easily test for.
git-version --features?
^ permalink raw reply
* Re: [PATCH 4/4] Don't link with libsocket in MINGW port
From: Johannes Sixt @ 2007-05-30 12:01 UTC (permalink / raw)
To: Nguyá»…n Thái Ngá»c Duy; +Cc: git
In-Reply-To: <1180492848275-git-send-email-pclouds@gmail.com>
"Nguyá»…n Thái Ngá»c Duy" wrote:
>
> From: Nguyá»…n Thái Ngá»c Duy <pclouds@gmail.com>
>
> There is a check in configure.ac to determine if libc has function
> socket(). It is used to link with libsocket on SunOS.
>
> On Windows, libc does not have socket() as well but it is provided
> by winsock, not libsocket. So don't link with libsocket if you are
> on Windows.
> ifdef NEEDS_SOCKET
> +ifndef MINGW
> EXTLIBS += -lsocket
> endif
> +endif
This looks wrong. NEEDS_SOCKET is set in the architecture sections of
Makefile where necessary, but the MinGW section doesn't want it and
doesn't set it. If ./configure sets it, then you should fix
configure.ac, not work around in Makefile.
-- Hannes
PS: Appologies for the mangled name - this MUA is from the stone age.
^ permalink raw reply
* Re: [PATCH] Don't ignore write failure from git-diff, git-log, etc.
From: Jim Meyering @ 2007-05-30 11:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v1wh0bpv2.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> Jim Meyering <jim@meyering.net> writes:
>
>> Of course error messages are annoying when your short-pipe-read is
>> _deliberate_ (tho, most real uses of git tools will actually get no
>> message to be annoyed about[*]), but what if there really *is* a mistake?
>> Try this:
>>
>> # You want to force git to ignore the error.
>> $ trap '' PIPE; git-rev-list HEAD | sync
>> $
>
> It is perfectly valid (although it is stupid) for a Porcelain
> script to do this:
>
> latest_by_jim=$(git log --pretty=oneline --author='Jim' | head -n 1)
> case "$latest_by_jim" in
> '') echo "No commit by Jim" ;;
> *) # do something interesting on the commit
> ;;;
> esac
Hi Junio,
The above snippet (prepending a single #!/bin/bash line) doesn't provoke
an EPIPE diagnostic from my patched git. In fact, even if you're using
an old, unpatched version of bash, it provokes *no* diagnostic at all.
To provoke a diagnostic (from bash, not git), using old unpatched bash,
you need a script doing output from a subshell, e.g.:
#!/tmp/bash-3.0/bash
for x in 1; do
git-log
done | head -1
With unpatched bash-3.0, it does this:
commit 42e3a6f676e9ae4e9640bc2ff36b7ab0b061a60e
/tmp/bp-demo: line 2: 24864 Broken pipe git-log
It's only if you try to avoid the above and change your script to
ignore SIGPIPE do you finally get an offending EPIPE diagnostic:
#!/tmp/bash-3.0/bash
trap '' PIPE
for x in 1; do
./git-log; echo $? 1>&2
done | head -1
Here's its output, using my patch:
commit 42e3a6f676e9ae4e9640bc2ff36b7ab0b061a60e
fatal: write failure on standard output
128
That trap has the nasty side effect of making the poorly written script
wait until "git-log" has completed (before, it was interrupted right
away), so it can take a lot longer. With my patch, it also gives a
diagnostic, which might serve to inform someone that they should not
ignore SIGPIPE.
No porcelain (modulo [*]) in git proper or cogito ignores SIGPIPE,
so I don't see how EPIPE error diagnostics can be a problem.
[*] These scripts do ignore SIGPIPE, but either don't need to,
or can/should be fixed not to:
git-archimport.perl
git-cvsimport.perl
git-svnimport.perl
And, yes, I'd be happy to fix them, if anyone is interested.
> In such a case, it is a bit too much for my taste to force the
> script to redirect what comes out of fd 2 of the upstream of the
> pipe, so that it can filter out only the "write error" message
> but still show other kinds of error messages. You could do so
> by elaborate shell magic, perhaps like this:
>
> filter_pipe_error () {
> exec 3>&1
> (eval "$1" 2>&1 1>&3 | grep >&2 -v 'Broken pipe')
> }
>
> latest_by_jim=$(filter_pipe_error \
> 'git log --pretty=oneline --author='\''Jim'\'' | head -n 1'
> )
I agree that would be extreme. But it's not necessary, since the
'Broken pipe' diagnostic appears now only in contrived circumstances.
> but what's the point?
>
> I think something like this instead might be more palatable.
...[patch to make git/EPIPE exit nonzero, but with no diagnostic]
Thank you for taking the time to reply and to come up with a compromise.
At first I thought this would be a step in the right direction, but,
now that I understand how infrequently EPIPE actually comes into play,
I think it'd be better to avoid a half-measure fix, since that would
just perpetuate the idea that EPIPE is worth handling specially.
Jim
^ permalink raw reply
* Re: [RFC] super indexes to span multiple packfiles
From: Avi Kivity @ 2007-05-30 9:40 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Jon Smirl, Shawn O. Pearce, git, Dana How
In-Reply-To: <alpine.LFD.0.99.0705291227010.11491@xanadu.home>
Nicolas Pitre wrote:
>>
>> Another way to achieve that is to place objects that are accessed together
>> nearby, and issue a larger read so as to bring them into cache. I imagine
>> that placing commit objects and associated tree and blobs in history order
>> should help here (but maybe git already does that, I'm not familiar with the
>> internals).
>>
>
> GIT already does that indeed, except for commit objects which are all
> together for better performances on history traversal operations.
>
> After a fresh repack, the checkout of the latest revision should produce
> a nearly perfect linear and contigous access into the early portion of
> the same pack. Things will get more random with access to objects
> further back in history of course, but those objects are less likely to
> be accessed as often.
>
>
Thanks. Actually I should have deduced this from the speed of 'git log' ;-)
--
error compiling committee.c: too many arguments to function
^ permalink raw reply
* Re: [PATCH] Don't ignore write failure from git-diff, git-log, etc.
From: Jim Meyering @ 2007-05-30 7:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Marco Roeland, git
In-Reply-To: <7v646b5gw5.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
>> - exit(p->fn(argc, argv, prefix));
>> + status = p->fn(argc, argv, prefix);
>> +
>> + /* Close stdout if necessary, and diagnose any failure
>> + other than EPIPE. */
>> + if (fcntl(fileno (stdout), F_GETFD) >= 0) {
>> + errno = 0;
>> + if ((ferror(stdout) || fclose(stdout))
>> + && errno != EPIPE) {
>> + if (errno == 0)
>> + die("write failure on standard output");
>> + else
>> + die("write failure on standard output"
>> + ": %s", strerror(errno));
>> + }
>
> This makes the final write failure trump the breakage p->fn()
> already diagnosed, doesn't it?
Yes. Are there circumstances in which a nonzero status from
some cmd_* function would mean something so grave that you
wouldn't also want to know that standard output is incomplete
or corrupt (and possibly use a different exit status)?
So far, after a quick and incomplete survey, I haven't found any.
However, if some git command is documented to exit with
status N for some listed values of N, e.g.,
1 A happened
2 B happened
3 any other failure
then the above choice of dying with "die" would be wrong.
E.g. git-diff's --exit-code comes close:
--exit-code
Make the program exit with codes similar to diff(1). That is, it
exits with 1 if there were differences and 0 means no differences.
but doesn't say how it handles errors.
[ OT: Perhaps that documentation should be changed to look more like diff's,
so that it says there is a different exit code for the third
case (some failure):
$ diff --help|tail -3|head -1
Exit status is 0 if inputs are the same, 1 if different, 2 if trouble.
]
> Maybe if (fcntrl(...) >=0 )
> should read if (!status && fcntrl(...) >= 0).
No, because then something like git-diff's --exit-code could hide
a write error.
If you want to preserve the exit status, then it should be enough
to call set_die_routine with a function that will work just like
"die" but exit with a specified (status) value.
^ permalink raw reply
* Re: [PATCH] Make git-k an alias to gitk
From: Johannes Sixt @ 2007-05-30 7:06 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Nguyen Thai Ngoc Duy, Marius Storm-Olsen, git
In-Reply-To: <Pine.LNX.4.64.0705300402050.4011@racer.site>
Johannes Schindelin wrote:
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
> On Tue, 29 May 2007, Nguyen Thai Ngoc Duy wrote:
>
> > On 5/29/07, Johannes Sixt <J.Sixt@eudaptics.com> wrote:
> > > pointy..clicky..pointy..clicky (aka: cp gitk git-k)
> > >
> > > $ git k
> > >
> > > :)
> >
> > Maybe we should teach git.c to try gitk if git-k is not found ;)
>
> Something like this?
>
> +static int cmd_gitk(int argc, const char **argv, const char *prefix)
> +{
> + return execv("gitk", (char *const *)argv);
> +}
> +
This does not work. Windows's execv() does not know how to invoke shell
(or tcl) scripts. Only execve() does, because we have our own
implementation.
I'd prefer to install gitk under both names gitk and git-k (as hard
links; which would amount to copies on Windows, but we don't care).
-- Hannes
^ permalink raw reply
* Re: [PATCH 0/4] Make MINGW port cross-compilable from Linux
From: Johannes Sixt @ 2007-05-30 6:51 UTC (permalink / raw)
To: Nguyá»…n Thái Ngá»c Duy; +Cc: git
In-Reply-To: <11804928354163-git-send-email-pclouds@gmail.com>
"Nguyá»…n Thái Ngá»c Duy" wrote:
>
> The following patches allow to cross compile mingw port from Linux less painful.
Thank you. Could you please push this to the mob branch?
git push mob@repo.or.cz:/srv/git/git/mingw.git +mybranch:mob
The series should be based on mingw.git's master branch (not devel)!
-- Hannes
^ permalink raw reply
* Re: [PATCH] Add basic test-script for git-submodule
From: Lars Hjemli @ 2007-05-30 6:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7v7iqs6r6p.fsf@assigned-by-dhcp.cox.net>
On 5/29/07, Junio C Hamano <junkio@cox.net> wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
>
> > +# create a submodule repository
> > +mkdir lib && cd lib
> > +git-init >/dev/null
> > +echo a >a && git-add a && git-commit -q -m "submodule commit 1"
> > +git-tag -a -m "rev-1" rev-1
> > +rev1=$(git-rev-parse HEAD)
> > +cd ..
> > +
> > +# add submodule and other files to super repo
> > +echo a >a && echo z >z
> > +git-add a lib z && git-commit -q -m "super commit 1"
> > +
> > +# move submodule to another location, register repo url in .gitmodules
> > +mv lib .subrepo
> > +GIT_CONFIG=.gitmodules git-config module.lib.url ./.subrepo
>
> We typically try to catch malfunction even while setting up the test case.
Ok
>
> > +test_expect_success 'status is "missing"' \
> > + 'git-submodule status | grep "^-$rev1"'
> > +
> > +# make sure 'init' will not overwrite a regular file
> > +touch lib
> > +test_expect_failure 'init fails when path is used by a file' \
> > + 'git-submodule init'
>
> I am guilty for introducing "expect-failure", but it is usually
> a mistake to use it unless you are testing something very trivial.
>
> For example, for this case, you would want to make sure the
> command "git-submodule init" exits with non-zero status, but
> also you would want to make sure that it does not disturb the
> existing file "lib".
Would you prefer a patch on top of this version of the test-script or
on top of the later patch which uses .git/config to store submodule
url?
--
larsh
^ permalink raw reply
* Re: [PATCH] Test for recent rev-parse $abbrev_sha1 regression
From: Junio C Hamano @ 2007-05-30 6:28 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20070530055806.GQ7044@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> But to me, anything that hits 'next' that breaks Git this badly is
> a regression. Why? Because I run my production repositories off
> next, that's why. Of course I do this to exercise next more fully...
> to prevent this sort of stuff from getting to master. ;-)
I'm almost always on 'next'. I switch to run 'master' a few
days before -rc0 and a few days after the final feature release
I switch back to 'next'.
The tip of 'next' may not be perfect, but it should not be so
broken that it is beyond a quick repair to inconvenience users
that rely on 'next' working. And it helps us catch problems
before the topics hit 'master'.
Because the tip of 'master' gets tagged only once every 6 weeks
or so, its tip is not an official release at any other times,
but we effectively have the usual pre-release QA continuously
running on the stuff that hits 'master'. That's what 'next' is
all about.
And that's how we can keep our 'master' a lot more robust than
the development branches of other open source projects.
^ permalink raw reply
* Re: [PATCH] diff-delta: use realloc instead of xrealloc
From: Junio C Hamano @ 2007-05-30 6:17 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Martin Koegler, git
In-Reply-To: <alpine.LFD.0.99.0705292252220.11491@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
> On Tue, 29 May 2007, Junio C Hamano wrote:
>
>> Wasn't there a discussion around this exact issue when the
>> original patch was applied?
>
> Maybe, but I cannot remember it, not can I justify that xrealloc now.
Actually, the discussion was older than the patch.
http://thread.gmane.org/gmane.comp.version-control.git/43766/focus=43783
And I myself suggested to use non x- variant of allocation
routines in diff-delta.c as the callers know how to handle near
oom condition ;-).
^ permalink raw reply
* [PATCH 3/3] Style nit - don't put space after function names
From: Shawn O. Pearce @ 2007-05-30 6:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Our style is to not put a space after a function name. I did here,
and Junio applied the patch with the incorrect formatting. So I'm
cleaning up after myself since I noticed it upon review.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
sha1_file.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index a3637d7..3093ac9 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -531,7 +531,7 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
return 0;
}
-int open_pack_index (struct packed_git *p)
+int open_pack_index(struct packed_git *p)
{
char *idx_name;
int ret;
--
1.5.2.869.g6b3ba
^ permalink raw reply related
* [PATCH 2/3] Ensure the pack index is opened before access
From: Shawn O. Pearce @ 2007-05-30 6:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In this particular location of fsck the index should have already
been opened by verify_pack, which is called just before we get
here and loop through the object names. However, just in case a
future version of that function does not use the index file we'll
double-check its open before we access the num_objects field.
Better safe now than sorry later.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
builtin-fsck.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/builtin-fsck.c b/builtin-fsck.c
index cbbcaf0..9959818 100644
--- a/builtin-fsck.c
+++ b/builtin-fsck.c
@@ -668,7 +668,10 @@ int cmd_fsck(int argc, char **argv, const char *prefix)
verify_pack(p, 0);
for (p = packed_git; p; p = p->next) {
- uint32_t i, num = p->num_objects;
+ uint32_t i, num;
+ if (open_pack_index(p))
+ continue;
+ num = p->num_objects;
for (i = 0; i < num; i++)
fsck_sha1(nth_packed_object_sha1(p, i));
}
--
1.5.2.869.g6b3ba
^ permalink raw reply related
* [PATCH 1/3] Simplify index access condition in count-objects, pack-redundant
From: Shawn O. Pearce @ 2007-05-30 6:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
My earlier lazy index opening patch changed this condition to check
index_data and call open_pack_index if it was NULL. In truth we only
care about num_objects. Since open_pack_index does no harm if the
index is already open, and all indexes are likely to be closed in
this application, the "performance optimization" of inlining the
index_data check here was wrong.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
First of a 3 part series to cleanup behind the lazy index patch.
This particular one was stupid; the lazy index patch never should
have been submitted with the conditions like this.
After this series, I'm pretty sure we've got everything covered.
Additional eyes from other people wouldn't hurt, but I have gone
through every caller several times now and am pretty sure they
are all correct.
Note that we are *not* safe to unload an index once opened; unlike
the use_pack() interface the index_data interface does not offer a
way for a caller to pin the index_data into memory for the duration
of the block. That's one reason I haven't tried to GC indexes yet.
builtin-count-objects.c | 2 +-
pack-redundant.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin-count-objects.c b/builtin-count-objects.c
index ac65e03..4274ec1 100644
--- a/builtin-count-objects.c
+++ b/builtin-count-objects.c
@@ -111,7 +111,7 @@ int cmd_count_objects(int ac, const char **av, const char *prefix)
for (p = packed_git; p; p = p->next) {
if (!p->pack_local)
continue;
- if (!p->index_data && open_pack_index(p))
+ if (open_pack_index(p))
continue;
packed += p->num_objects;
num_pack++;
diff --git a/pack-redundant.c b/pack-redundant.c
index 0617320..6bc3bdf 100644
--- a/pack-redundant.c
+++ b/pack-redundant.c
@@ -550,7 +550,7 @@ static struct pack_list * add_pack(struct packed_git *p)
l.pack = p;
llist_init(&l.all_objects);
- if (!p->index_data && open_pack_index(p))
+ if (open_pack_index(p))
return NULL;
base = p->index_data;
--
1.5.2.869.g6b3ba
^ permalink raw reply related
* Re: [PATCH] Test for recent rev-parse $abbrev_sha1 regression
From: Shawn O. Pearce @ 2007-05-30 5:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlkf6508y.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > My recent patch "Lazily open pack index files on demand" caused a
> > regression in the case of parsing abbreviated SHA-1 object names.
> > Git was unable to translate the abbreviated name into the full name
> > if the object was packed, as the pack .idx files were not opened
> > before being accessed.
>
> Thanks. As long as we catch it before it goes to 'master', it
> is not a regression ;-)
Good point. Next time I won't call it a regression.
But to me, anything that hits 'next' that breaks Git this badly is
a regression. Why? Because I run my production repositories off
next, that's why. Of course I do this to exercise next more fully...
to prevent this sort of stuff from getting to master. ;-)
For what its worth, I'm going through the code again and auditing
our use of the index related variables in packed_git that are now
lazily loaded. I'm not seeing anything that is critical. I do
have two minor patches queued up, but they are just to make things
look prettier. Will send soon.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Test for recent rev-parse $abbrev_sha1 regression
From: Junio C Hamano @ 2007-05-30 5:49 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20070530045026.GA12380@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> My recent patch "Lazily open pack index files on demand" caused a
> regression in the case of parsing abbreviated SHA-1 object names.
> Git was unable to translate the abbreviated name into the full name
> if the object was packed, as the pack .idx files were not opened
> before being accessed.
Thanks. As long as we catch it before it goes to 'master', it
is not a regression ;-)
^ permalink raw reply
* Re: [PATCH] Silence $(CC) command line when compiling test tools
From: Junio C Hamano @ 2007-05-30 5:48 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20070530031123.GA12044@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Like our other $(CC) rules in the Makefile we typically build with
> $(QUIET_CC) in front of $(CC) so that we hide the compiler options.
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
> Makefile | 14 +++++++-------
> 1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 3680b09..25c3f37 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -956,25 +956,25 @@ test: all
> $(MAKE) -C t/ all
>
> test-date$X: test-date.c date.o ctype.o
> - $(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) test-date.c date.o ctype.o
> + $(QUIET_CC)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) test-date.c date.o ctype.o
>
> test-delta$X: test-delta.o diff-delta.o patch-delta.o $(GITLIBS)
> - $(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
> + $(QUIET_CC)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
>
> test-dump-cache-tree$X: dump-cache-tree.o $(GITLIBS)
> - $(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
> + $(QUIET_CC)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
>
> test-sha1$X: test-sha1.o $(GITLIBS)
> - $(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
> + $(QUIET_CC)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
>
> test-match-trees$X: test-match-trees.o $(GITLIBS)
> - $(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
> + $(QUIET_CC)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
>
> test-chmtime$X: test-chmtime.c
> - $(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $<
> + $(QUIET_CC)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $<
>
> test-genrandom$X: test-genrandom.c
> - $(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $<
> + $(QUIET_CC)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $<
>
> check-sha1:: test-sha1$X
> ./test-sha1.sh
Hmmm. One wonders if we can do something about these apparent
similarities with all these rules...
^ permalink raw reply
* [PATCH] Test for recent rev-parse $abbrev_sha1 regression
From: Shawn O. Pearce @ 2007-05-30 4:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
My recent patch "Lazily open pack index files on demand" caused a
regression in the case of parsing abbreviated SHA-1 object names.
Git was unable to translate the abbreviated name into the full name
if the object was packed, as the pack .idx files were not opened
before being accessed.
This is a simple test to repack a repository then test for an
abbreviated SHA-1 within the packfile.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
t/t6101-rev-parse-parents.sh | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
index 7d354a1..b0252b9 100755
--- a/t/t6101-rev-parse-parents.sh
+++ b/t/t6101-rev-parse-parents.sh
@@ -29,5 +29,15 @@ test_expect_success 'final^1^3 not valid' "if git-rev-parse --verify final^1^3;
test_expect_failure '--verify start2^1' 'git-rev-parse --verify start2^1'
test_expect_success '--verify start2^0' 'git-rev-parse --verify start2^0'
+test_expect_success 'repack for next test' 'git repack -a -d'
+test_expect_success 'short SHA-1 works' '
+ start=`git rev-parse --verify start` &&
+ echo $start &&
+ abbrv=`echo $start | sed s/.\$//` &&
+ echo $abbrv &&
+ abbrv=`git rev-parse --verify $abbrv` &&
+ echo $abbrv &&
+ test $start = $abbrv'
+
test_done
--
1.5.2.869.g6b3ba
^ permalink raw reply related
* [PATCH 2/2] gitview: Define __slots__ for Commit
From: Michael Ellerman @ 2007-05-30 4:47 UTC (permalink / raw)
To: aneesh.kumar; +Cc: git
In-Reply-To: <f623da83269ba030fabc64777cdb1071e5ab00bf.1180500418.git.michael@ellerman.id.au>
Define __slots__ for the Commit class. This reserves space in each Commit
object for only the defined variables. On my system this reduces heap usage
when viewing a kernel repo by 12% ~= 55868 KB.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
contrib/gitview/gitview | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
index 6b54bc0..4baa1dd 100755
--- a/contrib/gitview/gitview
+++ b/contrib/gitview/gitview
@@ -263,6 +263,9 @@ class Commit(object):
""" This represent a commit object obtained after parsing the git-rev-list
output """
+ __slots__ = ['children_sha1', 'message', 'author', 'date', 'committer',
+ 'commit_date', 'commit_sha1', 'parent_sha1']
+
children_sha1 = {}
def __init__(self, commit_lines):
--
1.5.1.3.g7a33b
^ permalink raw reply related
* [PATCH 1/2] gitview: Use new-style classes
From: Michael Ellerman @ 2007-05-30 4:47 UTC (permalink / raw)
To: aneesh.kumar; +Cc: git
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
contrib/gitview/gitview | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
index 2d80e2b..6b54bc0 100755
--- a/contrib/gitview/gitview
+++ b/contrib/gitview/gitview
@@ -259,7 +259,7 @@ class CellRendererGraph(gtk.GenericCellRenderer):
self.set_colour(ctx, colour, 0.0, 0.5)
ctx.show_text(name)
-class Commit:
+class Commit(object):
""" This represent a commit object obtained after parsing the git-rev-list
output """
@@ -339,7 +339,7 @@ class Commit:
fp.close()
return diff
-class AnnotateWindow:
+class AnnotateWindow(object):
"""Annotate window.
This object represents and manages a single window containing the
annotate information of the file
@@ -519,7 +519,7 @@ class AnnotateWindow:
self.io_watch_tag = gobject.io_add_watch(fp, gobject.IO_IN, self.data_ready)
-class DiffWindow:
+class DiffWindow(object):
"""Diff window.
This object represents and manages a single window containing the
differences between two revisions on a branch.
@@ -674,7 +674,7 @@ class DiffWindow:
fp.close()
dialog.destroy()
-class GitView:
+class GitView(object):
""" This is the main class
"""
version = "0.9"
--
1.5.1.3.g7a33b
^ permalink raw reply related
* [PATCH] Introduce git-supported-features for porcelain use
From: Shawn O. Pearce @ 2007-05-30 4:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin
As a porcelain author I'm finding it difficult to keep track of
what features I can use in git-gui. Newer versions of Git have
newer capabilities but they don't always immediately get newer
version numbers that I can easily test for.
This is a simple plumbing command that lets a porcelain ask the
plumbing for its capabilities, at which point the porcelain can
work around anything missing, or recommend to the user that they
upgrade their plumbing layer.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.gitignore | 1 +
Documentation/git-supported-features.txt | 37 +++++++++++++++++
Makefile | 1 +
builtin-supported-features.c | 66 ++++++++++++++++++++++++++++++
builtin.h | 1 +
git.c | 1 +
t/t0000-basic.sh | 10 +++++
7 files changed, 117 insertions(+), 0 deletions(-)
create mode 100644 Documentation/git-supported-features.txt
create mode 100644 builtin-supported-features.c
diff --git a/.gitignore b/.gitignore
index 4dc0c39..6ff18d4 100644
--- a/.gitignore
+++ b/.gitignore
@@ -126,6 +126,7 @@ git-ssh-push
git-ssh-upload
git-status
git-stripspace
+git-supported-features
git-svn
git-svnimport
git-symbolic-ref
diff --git a/Documentation/git-supported-features.txt b/Documentation/git-supported-features.txt
new file mode 100644
index 0000000..28e2c21
--- /dev/null
+++ b/Documentation/git-supported-features.txt
@@ -0,0 +1,37 @@
+git-supported-features(1)
+===================
+
+NAME
+----
+git-supported-features - Report capabilities of the plumbing
+
+SYNOPSIS
+--------
+'git-supported-features' [<name>]
+
+DESCRIPTION
+-----------
+Given no arguments this plumbing utility displays a list of feature
+strings, one per line, that this particular version of the Git
+plumbing supports. These strings can be matched by higher level
+porcelain to determine what capabilities are available to its use.
+
+Given one argument (the name of the feature to test) the program
+exits with 0 if the feature is supported, and 1 if it is not.
+Other exit codes are undefined at this time.
+
+NOTES
+-----
+The exact list of feature strings is undefined by this manual page.
+
+Feature strings use a restricted character set, relying only on
+`a`-`z` (lowercase), `0`-`9` and `-` (hypen). This restriction is
+intentional, as it makes parsing output in shell quite simple.
+
+Author
+------
+Written by Shawn O. Pearce <spearce@spearce.org>.
+
+GIT
+---
+Part of the gitlink:git[7] suite
diff --git a/Makefile b/Makefile
index 7527734..03ab471 100644
--- a/Makefile
+++ b/Makefile
@@ -372,6 +372,7 @@ BUILTIN_OBJS = \
builtin-shortlog.o \
builtin-show-branch.o \
builtin-stripspace.o \
+ builtin-supported-features.o \
builtin-symbolic-ref.o \
builtin-tar-tree.o \
builtin-unpack-objects.o \
diff --git a/builtin-supported-features.c b/builtin-supported-features.c
new file mode 100644
index 0000000..79007f4
--- /dev/null
+++ b/builtin-supported-features.c
@@ -0,0 +1,66 @@
+#include "builtin.h"
+
+static const char *features[] = {
+ "git-k",
+ "redirect-stderr",
+ "supported-features",
+};
+
+static int is_sane(const char *a)
+{
+ if (!*a || *a == '-')
+ return 0;
+ for (; *a; a++) {
+ if (! ((*a >= 'a' && *a <= 'z')
+ || (*a >= '0' && *a <= '9')
+ || *a == '-'))
+ return 0;
+ }
+ return 1;
+}
+
+static int sort_feature(const void *a_, const void *b_)
+{
+ const char *a = *((const char **)a_);
+ const char *b = *((const char **)b_);
+ return strcmp(a, b);
+}
+
+static void list_features()
+{
+ unsigned cnt = ARRAY_SIZE(features);
+ unsigned i;
+
+ qsort(features, cnt, sizeof(features[0]), sort_feature);
+ for (i = 0; i < cnt; i++) {
+ if (!is_sane(features[i]))
+ die("feature name \"%s\" is bogus", features[i]);
+ printf("%s\n", features[i]);
+ }
+}
+
+static int test_feature(const char *the_feature)
+{
+ unsigned cnt = ARRAY_SIZE(features);
+ unsigned i;
+
+ for (i = 0; i < cnt; i++) {
+ if (!strcmp(features[i], the_feature))
+ return 0;
+ }
+ return 1;
+}
+
+static const char supported_features_usage[] =
+"git-supported-features [<feature>]";
+
+int cmd_supported_features(int argc, const char **argv, const char *prefix)
+{
+ if (argc == 1)
+ list_features();
+ else if (argc == 2)
+ return test_feature(argv[1]);
+ else
+ usage(supported_features_usage);
+ return 0;
+}
diff --git a/builtin.h b/builtin.h
index 39290d1..f33432c 100644
--- a/builtin.h
+++ b/builtin.h
@@ -71,6 +71,7 @@ extern int cmd_shortlog(int argc, const char **argv, const char *prefix);
extern int cmd_show(int argc, const char **argv, const char *prefix);
extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
+extern int cmd_supported_features(int argc, const char **argv, const char *prefix);
extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);
extern int cmd_unpack_objects(int argc, const char **argv, const char *prefix);
diff --git a/git.c b/git.c
index 29b55a1..377fae9 100644
--- a/git.c
+++ b/git.c
@@ -284,6 +284,7 @@ static void handle_internal_command(int argc, const char **argv, char **envp)
{ "show-branch", cmd_show_branch, RUN_SETUP },
{ "show", cmd_show, RUN_SETUP | USE_PAGER },
{ "stripspace", cmd_stripspace },
+ { "supported-features", cmd_supported_features },
{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
{ "tar-tree", cmd_tar_tree },
{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 186de70..7d0d9c9 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -31,6 +31,16 @@ fi
. ./test-lib.sh
################################################################
+# git-supported-features should always work.
+
+test_expect_success \
+ 'supported-features is functional' \
+ 'git-supported-features'
+test_expect_success \
+ 'supported-features recognizes itself' \
+ 'git-supported-features supported-features'
+
+################################################################
# git-init has been done in an empty repository.
# make sure it is empty.
--
1.5.2.869.g6b3ba
^ permalink raw reply related
* Re: [PATCH] Git wrapper: add --redirect-stderr option
From: Johannes Schindelin @ 2007-05-30 4:12 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Han-Wen Nienhuys, Johannes Sixt, Nguyen Thai Ngoc Duy,
Marius Storm-Olsen, git
In-Reply-To: <20070530035335.GP7044@spearce.org>
Hi,
On Tue, 29 May 2007, Shawn O. Pearce wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Tue, 29 May 2007, Shawn O. Pearce wrote:
> >
> > > Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > >
> > > > With this option, stderr is redirected to stdout. The short option is '-2'.
> > > >
> > > > Alternatively, you can say '--redirect-stderr=<filename>' to redirect
> > > > stderr to a file.
> > >
> > > Yes, that works nicely. ;-)
> > >
> > > Now here's my other problem: How does git-gui know the underlying
> > > git will accept --redirect-stderr? Or that it supports any other
> > > recent features we've developed?
> >
> > I thought that git-gui is now really closely coupled with core-git.
>
> Not really. It requires 1.5.0 and later, but after that it should
> be OK with any 1.5.x version of Git. Junio graciously bundles it
> with core Git, to help it get a wider audience. ;-)
>
> However I'd like to let it be smarter about what it expects from the
> underling core Git. I'm sure the other Porcelain authors would also
> like to be able to determine if they can rely on something, or not.
> At the least we can just tell the user "Your Git old. Upgrade to new
> Git today!". Yes, bad Engrish included... ;-)
When in doubt, you can always test the return value to check if the
command line option is accepted...
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Git wrapper: add --redirect-stderr option
From: Shawn O. Pearce @ 2007-05-30 3:53 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Han-Wen Nienhuys, Johannes Sixt, Nguyen Thai Ngoc Duy,
Marius Storm-Olsen, git
In-Reply-To: <Pine.LNX.4.64.0705300444020.4011@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> On Tue, 29 May 2007, Shawn O. Pearce wrote:
>
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > >
> > > With this option, stderr is redirected to stdout. The short option is '-2'.
> > >
> > > Alternatively, you can say '--redirect-stderr=<filename>' to redirect
> > > stderr to a file.
> >
> > Yes, that works nicely. ;-)
> >
> > Now here's my other problem: How does git-gui know the underlying
> > git will accept --redirect-stderr? Or that it supports any other
> > recent features we've developed?
>
> I thought that git-gui is now really closely coupled with core-git.
Not really. It requires 1.5.0 and later, but after that it should
be OK with any 1.5.x version of Git. Junio graciously bundles it
with core Git, to help it get a wider audience. ;-)
However I'd like to let it be smarter about what it expects from the
underling core Git. I'm sure the other Porcelain authors would also
like to be able to determine if they can rely on something, or not.
At the least we can just tell the user "Your Git old. Upgrade to new
Git today!". Yes, bad Engrish included... ;-)
> > Sure I can check the version, but until I know what version of Git
> > its shipping in I cannot put the check into git-gui.
> >
> > I was thinking about adding a "git-supported-features" plumbing
> > command that prints back feature code strings, much as our
> > network protocol supplies back the few feature codes it supports
> > ("multi-ack", "sideband", etc.).
>
> Problem is: it would be more interesting to have this program in _older_
> versions... ;-)
Lack of git-supported-features means only features before it coming
into existance? ;-)
--
Shawn.
^ 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