* Re: [PATCH] Teach "git remote" a mirror mode
From: Johannes Schindelin @ 2007-09-03 11:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmyw4majd.fsf@gitster.siamese.dyndns.org>
Hi,
On Mon, 3 Sep 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > When using the "--mirror" option to "git remote add", the refs will
> > not be stored in the refs/remotes/ namespace, but in the same location
> > as on the remote side.
>
> Thanks. With this and the "git remote rm" I think we would be in much
> better shape. Another thing that would be needed further before we can
> rewriting git-clone would be the "guessing where HEAD points at" and we
> would be in a very good shape.
Okay, that should be fixable.
> I notice you did not add any tests, though...
You noticed. Darn. ;-)
I briefly considered it, but decided I did not have the energy to cut a
test suite for "git remote"... Will do so today.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Add a new lstat implementation based on Win32 API, and make stat use that implementation too.
From: Johannes Schindelin @ 2007-09-03 11:39 UTC (permalink / raw)
To: Marius Storm-Olsen; +Cc: Robin Rosenberg, Johannes Sixt, git
In-Reply-To: <46DBA672.9050308@trolltech.com>
Hi,
On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:
> Robin Rosenberg said the following on 02.09.2007 22:27:
> > s?ndag 02 september 2007 skrev Marius Storm-Olsen:
> > > (Also, since Windows doesn't really handle symlinks, it's fine that
> > > stat just uses lstat)
> >
> > It does now: See
> > http://msdn2.microsoft.com/en-us/library/aa363866.aspx
>
> Yeah, I know about Vista's improved support for symbolic links. However,
> I think we can let that lay for a while, until we decide to make Git
> generate proper symlinks on Vista. I don't see it as a 1st priority at
> the moment, and we can always add the needed functionality in a separate
> stat() function later.
... and force everybody to upgrade to Vista, thereby working for Microsoft
for free? You _know_ that I will oppose that change.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Add a new lstat implementation based on Win32 API, and make stat use that implementation too.
From: David Kastrup @ 2007-09-03 11:32 UTC (permalink / raw)
To: git
In-Reply-To: <20070903112110.GE14853@genesis.frugalware.org>
Miklos Vajna <vmiklos@frugalware.org> writes:
> On Mon, Sep 03, 2007 at 09:07:42AM +0200, Johannes Sixt <j.sixt@eudaptics.com> wrote:
>> >$ ls -ld x y
>> >lrwxrwxrwx 1 me me 8 sep 2 23:36 x -> Makefile
>> >-rw-r--r-- 1 me me 32164 sep 2 23:36 y
>
>> And if I understand the documentation correctly, then
>
>> $ mkdir foo && cd foo
>> $ cat ../x
>> x: No such file or directory
>
>> Right?
>
> correct.
Have you tested this, or is this from reading the documentation? In
either case: brilliant, but the former would be funnier (depending on
one's sense of humor, of course).
--
David Kastrup
^ permalink raw reply
* Re: [ANNOUNCE] GIT 1.5.3
From: Andreas Ericsson @ 2007-09-03 11:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sean, H. Peter Anvin, David Kastrup, git, linux-kernel
In-Reply-To: <7v4picpvgq.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
>
> Assuming that we do not give the old git-p4import script
> packaged in "git-p4 package", would the following patch be all
> that is needed, or do we need other things in the spec file?
>
> -- snipsnap clipcrap --
> +Obsoletes: git-p4
That depends. If packages outside of git requires the git-p4 package to function
then this will not suffice and a line saying
Provides: git-p4
would have to be added instead. If some other already installed package Require:'s
/usr/bin/git-p4import, then that other package is broken and we're toast.
I have a hard time seeing what other package would depend on the git-p4import
script so this was more along the lines of "how rpm dependencies work, kinda"
than a real issue.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: problem with git cvsimport
From: Simon 'corecode' Schubert @ 2007-09-03 11:28 UTC (permalink / raw)
To: Jean-François Veillette; +Cc: git
In-Reply-To: <5951BCFB-D3EF-4423-86F2-50F1D080721B@yahoo.ca>
Jean-François Veillette wrote:
> Again, any pointer is welcome to help resolve issues with importing cvs
> projects.
> Thanks to Michael for pointing out cvs2svn,I haven't tried it yet.
You might also want to give fromcvs/togit a try, which has been doing a
good job for me (obviously, since I wrote it for my own consumption).
Best try all converters and take the one conversion you like the most.
And report bugs/inconsistencies.
cheers
simon
^ permalink raw reply
* Re: [PATCH] Add a new lstat implementation based on Win32 API, and make stat use that implementation too.
From: Miklos Vajna @ 2007-09-03 11:21 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Johannes Schindelin, Marius Storm-Olsen, Johannes Sixt
In-Reply-To: <46DBB2BE.8030505@eudaptics.com>
[-- Attachment #1: Type: text/plain, Size: 357 bytes --]
On Mon, Sep 03, 2007 at 09:07:42AM +0200, Johannes Sixt <j.sixt@eudaptics.com> wrote:
> >$ ls -ld x y
> >lrwxrwxrwx 1 me me 8 sep 2 23:36 x -> Makefile
> >-rw-r--r-- 1 me me 32164 sep 2 23:36 y
> And if I understand the documentation correctly, then
> $ mkdir foo && cd foo
> $ cat ../x
> x: No such file or directory
> Right?
correct.
- VMiklos
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] gitweb: Fix and simplify "split patch" detection
From: Junio C Hamano @ 2007-09-03 11:00 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Yann Dirson, Petr Baudis
In-Reply-To: <200709031147.32910.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> I wonder if the post-image name is unique in raw format of diff
> output, and can be used alone to check if there are two patches
> per one raw output format line...
I think that is a very sane assumption.
You may have a situation that a single preimage is used to
produce more than one postimage, but it does not make sense to
say this postimage is produced by doing this to that preimage
and there is another way to produce the same postimage using
something else.
At least, I do not think the diffcore chain is _capable_ of
coming up with such alternatives even if it wanted to ;-).
^ permalink raw reply
* Re: [PATCH] gitweb: Fix file name quoting in simplified out patch detection for --cc output
From: Jakub Narebski @ 2007-09-03 9:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vabs4oaxh.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index 8c1e02c..5d424e5 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -3140,8 +3140,8 @@ sub git_patchset_body {
> > # compact combined diff output can have some patches skipped
> > # find which patch (using pathname of result) we are at now
> > my $to_name;
> > - if ($patch_line =~ m!^diff --cc "?(.*)"?$!) {
> > - $to_name = $1;
> > + if ($patch_line =~ m!^diff --cc ("?.*"?)$!) {
> > + $to_name = unquote($1);
> > }
>
> This patch obviously makes sense, but I wonder why you use the
> form m!foobar! here, instead of far simpler to read /foobar/
> form?
Consistency (kind of). For the ordinary diff output I use m!regexp!
instead of simpler /regexp/ to avoid 'leaning toothpicks' syndrome:
m!^diff --git "?a/.*"? "?b/.*"?!
But as it is now not near, then I guess /regexp/ would be better...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] gitweb: Fix and simplify "split patch" detection
From: Jakub Narebski @ 2007-09-03 9:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Yann Dirson, Petr Baudis
In-Reply-To: <7vmyw4ob7z.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > Alternate solution, which we did chose, is to check when git splits
> > patches, and do not check if parsed info from current patch corresponds
> > to current or next raw diff format output line. Git splits patches
> > only for 'T' (typechange) status filepair, and there always two patches
> > corresponding to one raw diff line.
>
> Not that I can think of a better way offhand than what you
> already mentioned, but I have to say that I am not entirely
> happy with this implementation.
I'm also bit unhappy with tying git_patchset_body code to the
minute details of git-diff output.
> A really old git showed two
> patches (one creation and one deletion) for "complete rewrite",
> which was corrected long time ago. I do not think we will
> change the typechange output in a similar way in the future, but
> relying on that level of detail feels somewhat ugly.
Gitweb requires git with --git-dir at least, so I don't think it
(it meaning "complete rewrite" patch being "split patch") is
a problem here.
> As you are reading from --patch-with-raw, you already know the
> order of patches that will be given to you when you finished
> reading the "raw" part. The patches will come in the same
> order. So it might be possible to keep track of patches to what
> path you are expecting and decide if it is part of what you are
> processing at the point you process "diff --git" line.
I'll try to come with the second solution.
I wonder if the post-image name is unique in raw format of diff
output, and can be used alone to check if there are two patches
per one raw output format line...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH 0/5] gitweb: Support for arbitrary diffs
From: Jakub Narebski @ 2007-09-03 8:23 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20070903013330.GR1219@pasky.or.cz>
On Mon, 3 September 2007, Petr "Pasky" Baudis wrote:
> To hijack this post a bit, another patch in the queue (the incremental
> blame thingie) introduces blame.js. Do you think that we should keep the
> .js files separate, or instead have one big gitweb.js with everything?
> I'm inclined to the second possibility in order to reduce the number of
> requests, but it comes at a price of slightly worse maintainability.
On the other hand if we have blame.js separate, we could load it
(require it) only for the 'blame' view, it means only when needed.
gitweb.js would contain JavaScript code used by all (or almost all)
views, then...
I don't think gitweb.js would be as large as gitweb.perl, if we are
talking about maintability ;-)
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH] remote: add 'rm' subcommand
From: Junio C Hamano @ 2007-09-03 10:07 UTC (permalink / raw)
To: James Bowes; +Cc: git
In-Reply-To: <11810859232541-git-send-email-jbowes@dangerouslyinc.com>
James Bowes <jbowes@dangerouslyinc.com> writes:
> Introduce git-remote rm <name> which will:
> - Remove the remote config entry for <name>.
> - Remove any config entries for tracking branches of <name>.
> - Remove any stored remote branches of <name>.
>
> Signed-off-by: James Bowes <jbowes@dangerouslyinc.com>
Let's continue with this in 1.5.4 cycle. Together with Dscho's
"--mirror" patch, this will make "remote" more complete.
> If there is any interest in this, I'll follow up later with additions
> to the docs.
This is not a good thing to say here. If even the original
author cannot be bothered to perfect it unconditionally (iow
even when other people do not realize how useful it would be
initially and nobody seem to be interested), it does not help
convincing others that it is a good thing for the community to
take the patch.
> +sub rm_remote {
> + my ($name) = @_;
> + if (!exists $remote->{$name}) {
> + print STDERR "No such remote $name\n";
> + return;
> + }
> +
> + $git->command('config', '--remove-section', "remote.$name");
> +
> + eval {
> + my @trackers = $git->command('config', '--get-regexp',
> + 'branch.*.remote', $name);
Is this correct, or should it be '^branch\..*\.remote$'
> + for (@trackers) {
> + /^branch\.(.*)?\.remote/;
And this one. Why do we have '?' there? Perhaps...
if (/^branch\.(.*\)\.remote /) {
$git->config('--unset', "branch.$1.remote");
$git->config('--unset', "branch.$1.merge");
} else {
die "Gaah, why $_ is not branch.<<name>>.remote???"
}
We seem to have another subroutine to prune remote tracking
branches, which does it slightly differently. Maybe we would
want to share code with that codepath?
Other than that, I think the patch is sane, with your later
"documentation patch".
We would want a handful tests, including ones to check error
conditions, such as trying to remove a remote that does not
exist.
^ permalink raw reply
* Re: StGIT experimental branches updated
From: Catalin Marinas @ 2007-09-03 9:32 UTC (permalink / raw)
To: Karl Hasselström; +Cc: git
In-Reply-To: <20070903090121.GA5051@diana.vm.bytemark.co.uk>
On 03/09/07, Karl Hasselström <kha@treskal.com> wrote:
> On 2007-09-03 09:29:18 +0100, Catalin Marinas wrote:
>
> > BTW, would an extension to 'stg import' to import patches from
> > mailing lists (NNTP or maybe just mbox) directly be useful? There is
> > support for mbox importing anyway and it would need a procmail
> > filter to generate the mbox and just a regex filtering in 'import'
> > to apply the StGIT patches only.
>
> I don't think I'd use that function -- it's very convenient to just
> manually copy the patch mails I want to a separate mbox, which I then
> pipe to git-am. (Yes, I know, I should be using "stg import", but "stg
> assimilate" is just too useful.)
I usually save them to an mbox as well (but use stg import). Until
recently, when I started merging your 'safe' branch :-).
--
Catalin
^ permalink raw reply
* Re: strbuf API
From: Junio C Hamano @ 2007-09-03 9:18 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, Pierre Habouzit
In-Reply-To: <20070903084927.GB21759@artemis.corp>
Pierre Habouzit <madcoder@debian.org> writes:
> On Mon, Sep 03, 2007 at 08:32:16AM +0000, Matthieu Moy wrote:
>> ...
>> For example, it would be very tempting to compare files with
>> "strcmp(buf1, buf2)", but that would just fail silently when the file
>> contains a '\0' byte.
>
> Indeed, OTHO doing that would be pretty silly, as embending NULs in a
> strbuf is wrong, it's a _str_buf, not a random-binary-buffer. It's meant
> to make the use of strings easier, not to use as generic purpose byte
> buffers. Of course they can, but well, it's not what they are designed
> for in the first place.
People, please realize strbuf "API" is not a serious API.
It wasn't even intended to be anything more than just a
quick-and-dirty implementation of fgets that can grow
dynamically. The other callers added by people to have it do
general string manipulations were just bolted-on, not designed.
I haven't taken a serious look at bstring nor any of the
alternatives yet, but defending strbuf as if it was designed to
be a sane API is just silly.
^ permalink raw reply
* Re: [ANNOUNCE] GIT 1.5.3
From: H. Peter Anvin @ 2007-09-03 9:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sean, David Kastrup, git
In-Reply-To: <7vlkboktce.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> "H. Peter Anvin" <hpa@zytor.com> writes:
>
>> Junio C Hamano wrote:
>>> "H. Peter Anvin" <hpa@zytor.com> writes:
>>>
>>>>> Ok, should I then do that single change, cut 1.5.3.1 with it and
>>>>> ping you?
>>>> Sounds good to me.
>>> Thanks, and sorry for the trouble. I am building one on k.org,
>>> and after placing the result in the RPMS/x86-64 and running the
>>> yummy script, I'll ping you again. If it installs fine for you,
>>> I'll boot my wife's machine to do i386 as well, but it is
>>> getting a bit late now, so it might have to be tomorrow.
>> git.kernel.org is actually an i386 machine (the only one we have left), too.
>
> Ok, I did the same for i386, up to "yummy" part. Could you take
> care of the rest of the installation procedure, please?
>
Done, all good.
-hpa
^ permalink raw reply
* Re: [ANNOUNCE] GIT 1.5.3
From: Junio C Hamano @ 2007-09-03 9:10 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: Sean, David Kastrup, git
In-Reply-To: <46DBCA31.5010607@zytor.com>
"H. Peter Anvin" <hpa@zytor.com> writes:
> Junio C Hamano wrote:
>> "H. Peter Anvin" <hpa@zytor.com> writes:
>>
>>>> Ok, should I then do that single change, cut 1.5.3.1 with it and
>>>> ping you?
>>> Sounds good to me.
>>
>> Thanks, and sorry for the trouble. I am building one on k.org,
>> and after placing the result in the RPMS/x86-64 and running the
>> yummy script, I'll ping you again. If it installs fine for you,
>> I'll boot my wife's machine to do i386 as well, but it is
>> getting a bit late now, so it might have to be tomorrow.
>
> git.kernel.org is actually an i386 machine (the only one we have left), too.
Ok, I did the same for i386, up to "yummy" part. Could you take
care of the rest of the installation procedure, please?
^ permalink raw reply
* Re: [StGIT PATCH 4/4] Add optional logging of subprocess execution
From: Karl Hasselström @ 2007-09-03 9:04 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0709030136y4f1f241cge28fb9607ec559c1@mail.gmail.com>
On 2007-09-03 09:36:26 +0100, Catalin Marinas wrote:
> On 29/08/07, Karl Hasselström <kha@treskal.com> wrote:
>
> > Any chance we can drop Python 2.3 support any time soon, by the
> > way? I've confined all the ickyness to one place, but it would
> > still be good to get rid of it (not to mention being able to use
> > sets and generator expressions).
>
> Unless there is no objection, I'm actually OK with this. Feel free
> to break the 2.3 support from now on. I'll add a note stating the
> minimum versions for Python and GIT in the 0.14 release.
Great!
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: strbuf API
From: Matthieu Moy @ 2007-09-03 9:02 UTC (permalink / raw)
To: git
In-Reply-To: <20070903084927.GB21759@artemis.corp>
Pierre Habouzit <madcoder@debian.org> writes:
> Indeed, OTHO doing that would be pretty silly, as embending NULs in a
> strbuf is wrong, it's a _str_buf, not a random-binary-buffer.
I see nothing in strbuf that prevents you from embedding '\0' in it.
And the git code calls read_line on stdin, which may well contain
'\0'. So, either I missed something, or strbuf can contain '\0', or
the git code is not robust to null-characters on stdin, and should be
fixed.
--
Matthieu
^ permalink raw reply
* Re: StGIT experimental branches updated
From: Karl Hasselström @ 2007-09-03 9:01 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0709030129y2ced4f61jaf42c07a7798f47a@mail.gmail.com>
On 2007-09-03 09:29:18 +0100, Catalin Marinas wrote:
> BTW, would an extension to 'stg import' to import patches from
> mailing lists (NNTP or maybe just mbox) directly be useful? There is
> support for mbox importing anyway and it would need a procmail
> filter to generate the mbox and just a regex filtering in 'import'
> to apply the StGIT patches only.
I don't think I'd use that function -- it's very convenient to just
manually copy the patch mails I want to a separate mbox, which I then
pipe to git-am. (Yes, I know, I should be using "stg import", but "stg
assimilate" is just too useful.)
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [ANNOUNCE] GIT 1.5.3
From: Junio C Hamano @ 2007-09-03 8:53 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: Sean, David Kastrup, git
In-Reply-To: <46DBCA31.5010607@zytor.com>
"H. Peter Anvin" <hpa@zytor.com> writes:
> Junio C Hamano wrote:
>> "H. Peter Anvin" <hpa@zytor.com> writes:
>>
>>>> Ok, should I then do that single change, cut 1.5.3.1 with it and
>>>> ping you?
>>> Sounds good to me.
>>
>> Thanks, and sorry for the trouble. I am building one on k.org,
>> and after placing the result in the RPMS/x86-64 and running the
>> yummy script, I'll ping you again. If it installs fine for you,
>> I'll boot my wife's machine to do i386 as well, but it is
>> getting a bit late now, so it might have to be tomorrow.
>>
>
> git.kernel.org is actually an i386 machine (the only one we have left), too.
I see /usr/bin/git on the other machine is already 1.5.3.1 so I
take the experiment went well. I am building the i386 set now.
^ permalink raw reply
* Re: strbuf API
From: Pierre Habouzit @ 2007-09-03 8:49 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqk5r8m9nz.fsf@bauges.imag.fr>
[-- Attachment #1: Type: text/plain, Size: 1731 bytes --]
On Mon, Sep 03, 2007 at 08:32:16AM +0000, Matthieu Moy wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
>
> > First, and that's the most important one: the buffer is always NUL
> > terminated, after its official "len". That means, in terms of strbuf
> > API, that "alloc" is always greater or equal to len+1 to be able to
> > store the ending NUL[1]. The advantages are obvious: you can pass the
> > buffer to any legacy C string function without any fear of read
> > overflow.
>
> No overflow, but it's still potentially dangerous: if your strbuf
> contains a '\0', you can't use any standard C string function.
>
> For example, it would be very tempting to compare files with
> "strcmp(buf1, buf2)", but that would just fail silently when the file
> contains a '\0' byte.
Indeed, OTHO doing that would be pretty silly, as embending NULs in a
strbuf is wrong, it's a _str_buf, not a random-binary-buffer. It's meant
to make the use of strings easier, not to use as generic purpose byte
buffers. Of course they can, but well, it's not what they are designed
for in the first place.
Also note that it generates bugs, but never crashes if you still do
that, bugs suck, but security bugs, buffer overflows, ... suck more.
So I'm tempted to say, it's not really a problem.
> > The other shortcoming is that you cannot tell the buffer "Hey, it's
> > very likely that you'll end up being _that_ long.
>
> 100% ACK on this point.
Also note that it seems that bstring lack this, but maybe I'm wrong.
--
·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: [ANNOUNCE] GIT 1.5.3
From: H. Peter Anvin @ 2007-09-03 8:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sean, David Kastrup, git
In-Reply-To: <7vfy1wm9ik.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> "H. Peter Anvin" <hpa@zytor.com> writes:
>
>>> Ok, should I then do that single change, cut 1.5.3.1 with it and
>>> ping you?
>> Sounds good to me.
>
> Thanks, and sorry for the trouble. I am building one on k.org,
> and after placing the result in the RPMS/x86-64 and running the
> yummy script, I'll ping you again. If it installs fine for you,
> I'll boot my wife's machine to do i386 as well, but it is
> getting a bit late now, so it might have to be tomorrow.
>
git.kernel.org is actually an i386 machine (the only one we have left), too.
-hpa
^ permalink raw reply
* Re: strbuf API
From: Pierre Habouzit @ 2007-09-03 8:46 UTC (permalink / raw)
To: Johan Herland; +Cc: git
In-Reply-To: <200709030743.44188.johan@herland.net>
[-- Attachment #1: Type: text/plain, Size: 2759 bytes --]
On Mon, Sep 03, 2007 at 05:43:44AM +0000, Johan Herland wrote:
> On Monday 03 September 2007, Pierre Habouzit wrote:
> > , and would
> > patches that would refactor the strbuf module to have those functions,
> > and would fix the code that uses strbuf's to interact properly, be
> > accepted ?
>
> I don't know. Keep in mind that there is a parallel process (cf. the
> continuation of the "Buffer overflows" thread) to evaluate the Bstring
> library ( http://bstring.sourceforge.net/ ), and possibly substitute that
> for the strbuf "module".
>
> I wouldn't want the work done by you or others to be wasted (depending on
> which solution wins out in the end), so I suggest you take a look at
> bstring and offer up arguments why your solution would be better for git.
Oh I completely missed that the thread was still alive.
> I'd like to know how your solution compare with the alternatives (in this
> case bstring): What's the performance compared to bstring? How much thought
> has been given to the usability of the API? What functionality is covered
> (compared to bstring)? In short: Why is your alternative the best for git?
Well, technically, I don't mind, I wasn't really pushing this or this
implementation. I had a look at bstring, I dislike the code that looks
pretty ugly, but they almost took the same decisions, so if the
consensus is that bstring gives flexibility, so be it.
For the record, I dislike the fact that bstring pullutes the b*
namespace, whereas it should use bstr as a prefix everywhere, and some
functions (like bstrcpy, that should definitely be named bstrdup) are
really named the _wrong_ way: you can dislike legacy C string APIs all
the way you want, if you name a function from your foostr library
foostrbar, it better behave the way strbar does in the legacy C API. In
the legacy C, strcpy clobbers the destination, here there is no
destination as bstrcpy is definitely a strdup operation. I only had a
quick look, and maybe it was the sole of the kind, but ... well.
I also dislike the fact that there is no inline version of the
function that appends C strings, because it means that when you append a
constant string into a bstr, you'll need to write:
bstrcat(bstr, "foo", sizeof("foo") - 1) or bstrcat(bstr, "foo", 3),
both suck a lot, and with an inline function using strlen, most
compilers will optimize the strlen away, and the code is way more
readable. But again, this is trivial to implement as an extension as
well, so I really don't mind that much either.
--
·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: strbuf API
From: Matthieu Moy @ 2007-09-03 8:32 UTC (permalink / raw)
To: git
In-Reply-To: <20070902224213.GB431@artemis.corp>
Pierre Habouzit <madcoder@debian.org> writes:
> First, and that's the most important one: the buffer is always NUL
> terminated, after its official "len". That means, in terms of strbuf
> API, that "alloc" is always greater or equal to len+1 to be able to
> store the ending NUL[1]. The advantages are obvious: you can pass the
> buffer to any legacy C string function without any fear of read
> overflow.
No overflow, but it's still potentially dangerous: if your strbuf
contains a '\0', you can't use any standard C string function.
For example, it would be very tempting to compare files with
"strcmp(buf1, buf2)", but that would just fail silently when the file
contains a '\0' byte.
I don't have a perfect solution, but having something explicit to say
"hey, I'm going to use a legacy C function" doesn't sound like a bad
idea.
> The other shortcoming is that you cannot tell the buffer "Hey, it's
> very likely that you'll end up being _that_ long.
100% ACK on this point.
--
Matthieu
^ permalink raw reply
* Re: [StGIT PATCH 0/4] Miscellaneous fixes
From: Catalin Marinas @ 2007-09-03 8:40 UTC (permalink / raw)
To: Karl Hasselström; +Cc: git
In-Reply-To: <20070829111759.GB29980@diana.vm.bytemark.co.uk>
On 29/08/07, Karl Hasselström <kha@treskal.com> wrote:
> On 2007-08-29 11:18:34 +0100, Catalin Marinas wrote:
>
> > On 26/08/2007, Karl Hasselström <kha@treskal.com> wrote:
> > > examples/gitconfig | 2 +-
> > > stgit/git.py | 3 ++-
> > > stgit/stack.py | 7 ++++---
> > > t/t2101-pull-policy-pull.sh | 4 ++--
> > > 4 files changed, 9 insertions(+), 7 deletions(-)
> >
> > BTW, did you generate the above text by hand and copied it to the
> > e-mail?
>
> Yes. It's the output of git-request-pull.
>
> > People asked in the past for this kind of text to be automatically
> > generated by 'stg mail'.
>
> I might very well have been one of them.
On a related note, it might make sense for 'stg export' to add (with
an additional option) a comment line beside each patch in the series
file containing the short description.
--
Catalin
^ permalink raw reply
* Re: [PATCH] diff: resurrect the traditional empty "diff --git" behaviour
From: Junio C Hamano @ 2007-09-03 8:36 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Steven Grimm, git, Michael S. Tsirkin, Jeff King
In-Reply-To: <vpqveasmaqo.fsf@bauges.imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> I'd rather avoid talking about plumbing in the documentation of
> porcelain, so I'd say "silently refreshes the index's stat
> information".
Sounds like a much better wording.
^ 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