* Re: commit type
From: 7rans @ 2008-11-01 4:15 UTC (permalink / raw)
To: git
In-Reply-To: <alpine.DEB.1.00.0811010025570.22125@pacific.mpi-cbg.de.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin <at> gmx.de> writes:
> So you want to force this onto all Git users?
Not at all. It would be a purely optional. You would never even need to know the
feature existed if you didn't want to use it. So I'm not sure how that is
forcing it upon anyone.
Moreover, I suggested it b/c I would find such a feature very useful, and, by
extension, thought others might as well. Perhaps others have done something
similar, in which case it would be interesting the hear their take, or on the
other hand they've never considered it before, but now can consider the
potential utility of just such a feature.
> If yes: that murmur you hear in the background, it might well be the
> collective "thanks, but no thanks" of people who do not want that type of
> distinction between different commits.
There is no need to make one. It's purely annotative.
T.
^ permalink raw reply
* Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client
From: Tom Preston-Werner @ 2008-11-01 3:35 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Nicolas Pitre, git, gitster
In-Reply-To: <alpine.DEB.1.00.0811010334010.22125@pacific.mpi-cbg.de.mpi-cbg.de>
On Fri, Oct 31, 2008 at 7:35 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Fri, 31 Oct 2008, Nicolas Pitre wrote:
>
>> On Sat, 1 Nov 2008, Johannes Schindelin wrote:
>>
>> > On Fri, 31 Oct 2008, Tom Preston-Werner wrote:
>> >
>> > > The current behavior of git-daemon is to simply close the connection
>> > > on any error condition. This leaves the client without any
>> > > information as to the cause of the failed fetch/push/etc.
>> > >
>> > > This patch allows get_remote_heads to accept a line prefixed with
>> > > "ERR" that it can display to the user in an informative fashion.
>> > > Once clients can understand this ERR line, git-daemon can be made to
>> > > properly report "repository not found", "permission denied", or
>> > > other errors.
>> > >
>> > > Example
>> > >
>> > > S: ERR No matching repository.
>> > > C: fatal: remote error: No matching repository.
>> >
>> > Makes sense to me.
>>
>> Note that this behavior of not returning any reason for failure was
>> argued to be a security feature in the past, by Linus I think.
>
> Yes. And it might still be considered one. You do not need to patch
> git-daemon to use that facility (note that Tom's patch was only for the
> client side).
>
> But for hosting sites such as repo.or.cz or GitHub, that security feature
> just does not make sense, but it makes for support requests that could be
> resolved better with a proper error message.
We could also have the error messages sent back conditionally based on
a command line switch. I've begun porting the changes I made in our
Erlang git-daemon back to the C code, so maybe I'll give that a try.
We *definitely* need good error messages for GitHub and I see no
security risk in doing so.
Maybe this is worth asking the question: does anybody use git-daemon
for private code? If so, why are they not using SSH instead? And in
that case, how are informative error messages a security risk?
Tom
^ permalink raw reply
* Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client
From: Johannes Schindelin @ 2008-11-01 2:35 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Tom Preston-Werner, git, gitster
In-Reply-To: <alpine.LFD.2.00.0810312218300.13034@xanadu.home>
Hi,
On Fri, 31 Oct 2008, Nicolas Pitre wrote:
> On Sat, 1 Nov 2008, Johannes Schindelin wrote:
>
> > On Fri, 31 Oct 2008, Tom Preston-Werner wrote:
> >
> > > The current behavior of git-daemon is to simply close the connection
> > > on any error condition. This leaves the client without any
> > > information as to the cause of the failed fetch/push/etc.
> > >
> > > This patch allows get_remote_heads to accept a line prefixed with
> > > "ERR" that it can display to the user in an informative fashion.
> > > Once clients can understand this ERR line, git-daemon can be made to
> > > properly report "repository not found", "permission denied", or
> > > other errors.
> > >
> > > Example
> > >
> > > S: ERR No matching repository.
> > > C: fatal: remote error: No matching repository.
> >
> > Makes sense to me.
>
> Note that this behavior of not returning any reason for failure was
> argued to be a security feature in the past, by Linus I think.
Yes. And it might still be considered one. You do not need to patch
git-daemon to use that facility (note that Tom's patch was only for the
client side).
But for hosting sites such as repo.or.cz or GitHub, that security feature
just does not make sense, but it makes for support requests that could be
resolved better with a proper error message.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/3] git send-email: do not ask questions when --compose is used.
From: Ian Hilt @ 2008-11-01 2:26 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <20081031213803.GB21799@artemis.corp>
On Fri, Oct 31, 2008 at 06:01:49PM -0400, Ian Hilt wrote:
> I've thought something like this would be a good thing. An editor makes
> things easier to fix than the command-line.
Speaking of which, maybe let's add the To field to the list.
--->8---
From: Ian Hilt <ian.hilt@gmx.com>
Date: Fri, 31 Oct 2008 22:15:46 -0400
Subject: [PATCH] git-send-email.perl: add To field in editor
This allows the compose mode to add the To field in the editor. However
it currently will not remove an address.
Signed-off-by: Ian Hilt <ian.hilt@gmx.com>
---
This is on top of the previous patch I sent.
git-send-email.perl | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 0944be7..ed95402 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -450,6 +450,12 @@ if ($compose) {
my $tpl_sender = $sender || $repoauthor || $repocommitter || '';
+ my @tpl_to;
+ if (@to) {
+ foreach my $i (0..$#to) {
+ $tpl_to[$i] = $to[$i];
+ }
+ }
my $tpl_subject = $initial_subject || '';
my $tpl_reply_to = $initial_reply_to || '';
@@ -459,6 +465,7 @@ From $tpl_sender # This line is ignored.
# Consider including an overall diffstat or table of contents
# for the patch you are writing.
From: $tpl_sender
+To: @tpl_to
Subject: $tpl_subject
In-Reply-To: $tpl_reply_to
@@ -514,6 +521,9 @@ EOT
} elsif (/^From:\s*(.+)\s*$/i) {
$sender = $1;
next;
+ } elsif (/^To:\s*(.+)\s*$/i) {
+ push @to, $1;
+ next;
}
$summary_empty = 0;
print C2 $_;
@@ -522,7 +532,7 @@ EOT
close(C2);
if ($summary_empty) {
- print "Summary email is empty, skpping it\n";
+ print "Summary email is empty, skipping it\n";
$compose = -1;
}
} elsif ($annotate) {
--
1.6.0.3.523.g304d0
^ permalink raw reply related
* Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client
From: Nicolas Pitre @ 2008-11-01 2:20 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Tom Preston-Werner, git, gitster
In-Reply-To: <alpine.DEB.1.00.0811010316340.22125@pacific.mpi-cbg.de.mpi-cbg.de>
On Sat, 1 Nov 2008, Johannes Schindelin wrote:
> Hi,
>
> On Fri, 31 Oct 2008, Tom Preston-Werner wrote:
>
> > The current behavior of git-daemon is to simply close the connection on
> > any error condition. This leaves the client without any information as
> > to the cause of the failed fetch/push/etc.
> >
> > This patch allows get_remote_heads to accept a line prefixed with "ERR"
> > that it can display to the user in an informative fashion. Once clients
> > can understand this ERR line, git-daemon can be made to properly report
> > "repository not found", "permission denied", or other errors.
> >
> > Example
> >
> > S: ERR No matching repository.
> > C: fatal: remote error: No matching repository.
>
> Makes sense to me.
Note that this behavior of not returning any reason for failure was
argued to be a security feature in the past, by Linus I think.
Nicolas
^ permalink raw reply
* Re: libgit2 - a true git library
From: Johannes Schindelin @ 2008-11-01 2:26 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Nicolas Pitre, Junio C Hamano, Pieter de Bie, Pierre Habouzit,
git, Scott Chacon
In-Reply-To: <20081101015217.GJ14786@spearce.org>
Hi,
On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
> Nicolas Pitre <nico@cam.org> wrote:
> > On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
> >
> > > > Both the negative code and errno style are lightweight in the
> > > > common "no error" case. The errno style is probably more handy
> > > > for those functions returning a pointer which should be NULL in
> > > > the error case.
Unfortunately, errno would not be thread-safe, unless you can guarantee
that errno is a thread-local variable.
> > >
> > > I'm sticking with return a negative code for now, to the extent that
> > > some functions which return a pointer but also have many common
> > > failure modes (e.g. git_odb_open) use an output parameter as their
> > > first arg, so the error code can be returned as the result of the
> > > function.
> >
> > Actually, the pointer-returning functions can encode error cases into a
> > "negative" pointer. See include/linux/err.h for example.
> >
> > void *ptr = git_alloc_foo(...);
> > if (IS_ERR(ptr))
> > die("git_alloc_foo failed: %s", git_strerr(PTR_ERR(ptr)));
>
> Oh, good point. We could also stagger the errors so they are
> always odd, and never return an odd-alignment pointer from a
> successful function. Thus IS_ERR can be written as:
>
> #define IS_ERR(ptr) (((intptr_t)(ptr)) & 1)
>
> which is quite cheap, and given the (probably required anyway)
> aligned allocation policy means we still have 2^31 possible
> error codes available.
Oh boy, both solutions are ugly as hell. Although the &1 method does not
limit the memory space as much (except if you plan to work in
space-contrained environments, where you do not want to be forced to
word-align structs).
The only pointer game I would remotely consider clean is if you had
const char *errors[] = {
...
};
inline int is_error(void *ptr) {
return ptr >= errors && ptr < errors + ARRAY_SIZE(errors);
}
although that would be less performant.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client
From: Tom Preston-Werner @ 2008-11-01 2:18 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0811010316340.22125@pacific.mpi-cbg.de.mpi-cbg.de>
On Fri, Oct 31, 2008 at 7:19 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> The cute trick with this is that even older clients would be able to
> display the error, albeit with a sort of funny message:
>
> protocol error: expected sha/ref, got 'No matching repository.'
This is exactly what GitHub does right now with our custom Erlang
git-daemon. Try doing:
$ git clone git://github.com/mojombo/foo
and you'll get:
fatal: protocol error: expected sha/ref, got '
*********'
No matching repositories found.
*********'
This has cut down tremendously on the number of support requests we
get from new users. It would be nice to cut out the clutter from the
error message though.
Tom
^ permalink raw reply
* Re: [PATCH] connect.c: add a way for git-daemon to pass an error back to client
From: Johannes Schindelin @ 2008-11-01 2:19 UTC (permalink / raw)
To: Tom Preston-Werner; +Cc: git, gitster
In-Reply-To: <b97024a40810311859t2e5a6102u31ad4480e7c75c03@mail.gmail.com>
Hi,
On Fri, 31 Oct 2008, Tom Preston-Werner wrote:
> The current behavior of git-daemon is to simply close the connection on
> any error condition. This leaves the client without any information as
> to the cause of the failed fetch/push/etc.
>
> This patch allows get_remote_heads to accept a line prefixed with "ERR"
> that it can display to the user in an informative fashion. Once clients
> can understand this ERR line, git-daemon can be made to properly report
> "repository not found", "permission denied", or other errors.
>
> Example
>
> S: ERR No matching repository.
> C: fatal: remote error: No matching repository.
Makes sense to me.
The cute trick with this is that even older clients would be able to
display the error, albeit with a sort of funny message:
protocol error: expected sha/ref, got 'No matching repository.'
Ciao,
Dscho
^ permalink raw reply
* [PATCH] connect.c: add a way for git-daemon to pass an error back to client
From: Tom Preston-Werner @ 2008-11-01 1:59 UTC (permalink / raw)
To: git; +Cc: gitster
The current behavior of git-daemon is to simply close the connection on
any error condition. This leaves the client without any information as
to the cause of the failed fetch/push/etc.
This patch allows get_remote_heads to accept a line prefixed with "ERR"
that it can display to the user in an informative fashion. Once clients
can understand this ERR line, git-daemon can be made to properly report
"repository not found", "permission denied", or other errors.
Example
S: ERR No matching repository.
C: fatal: remote error: No matching repository.
Signed-off-by: Tom Preston-Werner <tom@github.com>
---
connect.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/connect.c b/connect.c
index 0c50d0a..3af91d6 100644
--- a/connect.c
+++ b/connect.c
@@ -70,6 +70,9 @@ struct ref **get_remote_heads(int in, struct ref **list,
if (buffer[len-1] == '\n')
buffer[--len] = 0;
+ if (len > 4 && !memcmp("ERR", buffer, 3))
+ die("remote error: %s", buffer + 4);
+
if (len < 42 || get_sha1_hex(buffer, old_sha1) || buffer[40] != ' ')
die("protocol error: expected sha/ref, got '%s'", buffer);
name = buffer + 41;
--
1.6.0.2
^ permalink raw reply related
* Re: libgit2 - a true git library
From: Nicolas Pitre @ 2008-11-01 1:53 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Pierre Habouzit, david, git
In-Reply-To: <20081101014336.GI14786@spearce.org>
On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
> My take on the consensus for the license part of the discussion is
> that libgit2 should be under the "GPL gcc library" license.
>
> BTW, I can't actually find a copy of that license; the only thing
> I can locate in the GCC SVN tree is a copy of the LGPL.
The exception is usually found at the top of files constituting
libgcc.a. One example is gcc/config/arm/ieee754-df.S. ;-)
Nicolas
^ permalink raw reply
* Re: libgit2 - a true git library
From: Shawn O. Pearce @ 2008-11-01 1:52 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Junio C Hamano, Pieter de Bie, Pierre Habouzit, git, Scott Chacon
In-Reply-To: <alpine.LFD.2.00.0810312135190.13034@xanadu.home>
Nicolas Pitre <nico@cam.org> wrote:
> On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
>
> > > Both the negative code and errno style are lightweight in the common "no
> > > error" case. The errno style is probably more handy for those functions
> > > returning a pointer which should be NULL in the error case.
> >
> > I'm sticking with return a negative code for now, to the extent
> > that some functions which return a pointer but also have many
> > common failure modes (e.g. git_odb_open) use an output parameter
> > as their first arg, so the error code can be returned as the result
> > of the function.
>
> Actually, the pointer-returning functions can encode error cases into a
> "negative" pointer. See include/linux/err.h for example.
>
> void *ptr = git_alloc_foo(...);
> if (IS_ERR(ptr))
> die("git_alloc_foo failed: %s", git_strerr(PTR_ERR(ptr)));
Oh, good point. We could also stagger the errors so they are
always odd, and never return an odd-alignment pointer from a
successful function. Thus IS_ERR can be written as:
#define IS_ERR(ptr) (((intptr_t)(ptr)) & 1)
which is quite cheap, and given the (probably required anyway)
aligned allocation policy means we still have 2^31 possible
error codes available.
--
Shawn.
^ permalink raw reply
* Re: libgit2 - a true git library
From: Nicolas Pitre @ 2008-11-01 1:49 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: david, Shawn O. Pearce, git
In-Reply-To: <20081101013815.GF29036@artemis.corp>
On Sat, 1 Nov 2008, Pierre Habouzit wrote:
> On Sat, Nov 01, 2008 at 01:33:51AM +0000, Nicolas Pitre wrote:
> > OTOH they would certainly come out clean if the license was BSD since
> > the spirit of that license explicitly allows closing up the whole
> > library and adding extra features.
>
> I think it's pretty clear most contributor from git.git would be driven
> away if the license isn't GPLish. So I was basing my answer on a
> supposition it would be.
Sure. My point is the difference in guiltiness. Even if a LGPL license
doesn't prevent all abuses, it at least allows for public denunciations
at the very least.
Nicolas
^ permalink raw reply
* Re: libgit2 - a true git library
From: Nicolas Pitre @ 2008-11-01 1:45 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Junio C Hamano, Pieter de Bie, Pierre Habouzit, git, Scott Chacon
In-Reply-To: <20081101011910.GH14786@spearce.org>
On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
> > Both the negative code and errno style are lightweight in the common "no
> > error" case. The errno style is probably more handy for those functions
> > returning a pointer which should be NULL in the error case.
>
> I'm sticking with return a negative code for now, to the extent
> that some functions which return a pointer but also have many
> common failure modes (e.g. git_odb_open) use an output parameter
> as their first arg, so the error code can be returned as the result
> of the function.
Actually, the pointer-returning functions can encode error cases into a
"negative" pointer. See include/linux/err.h for example.
void *ptr = git_alloc_foo(...);
if (IS_ERR(ptr))
die("git_alloc_foo failed: %s", git_strerr(PTR_ERR(ptr)));
Nicolas
^ permalink raw reply
* Re: libgit2 - a true git library
From: Shawn O. Pearce @ 2008-11-01 1:43 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Pierre Habouzit, david, git
In-Reply-To: <alpine.LFD.2.00.0810312121000.13034@xanadu.home>
Nicolas Pitre <nico@cam.org> wrote:
> On Sat, 1 Nov 2008, Pierre Habouzit wrote:
>
> > See junio's example. It's rather easy to add hooks into the library to
> > implement a feature outside of it. It's even possible to do it while
> > preserving the ABI fully IMHO (by being a strict superset of it).
> >
> > The patch would be so trivial, that I see no reason why they wouldn't
> > provide it. Though the real implementation of the feature that would be
> > delegated through it would be in their closed source stuff.
>
> But at that point it's a matter of public perception. It would clearly
> be against the spirit of the license even though the license itself
> couldn't prevent such tortuous practices. Those people doing such
> things would clearly be identified as bad guys and get bad press, and
> libgit contributors could even attempt law suits based on the derived
> work angle. That might be just enough to prevent such things to happen.
Yes.
> OTOH they would certainly come out clean if the license was BSD since
> the spirit of that license explicitly allows closing up the whole
> library and adding extra features.
My take on the consensus for the license part of the discussion is
that libgit2 should be under the "GPL gcc library" license.
BTW, I can't actually find a copy of that license; the only thing
I can locate in the GCC SVN tree is a copy of the LGPL.
--
Shawn.
^ permalink raw reply
* Re: libgit2 - a true git library
From: Pierre Habouzit @ 2008-11-01 1:38 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: david, Shawn O. Pearce, git
In-Reply-To: <alpine.LFD.2.00.0810312121000.13034@xanadu.home>
[-- Attachment #1: Type: text/plain, Size: 592 bytes --]
On Sat, Nov 01, 2008 at 01:33:51AM +0000, Nicolas Pitre wrote:
> OTOH they would certainly come out clean if the license was BSD since
> the spirit of that license explicitly allows closing up the whole
> library and adding extra features.
I think it's pretty clear most contributor from git.git would be driven
away if the license isn't GPLish. So I was basing my answer on a
supposition it would be.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: libgit2 - a true git library
From: david @ 2008-11-01 1:36 UTC (permalink / raw)
To: Pierre Habouzit
Cc: Shawn O. Pearce, Junio C Hamano, Nicolas Pitre, git, Scott Chacon
In-Reply-To: <20081101010620.GD29036@artemis.corp>
On Sat, 1 Nov 2008, Pierre Habouzit wrote:
> On Sat, Nov 01, 2008 at 12:41:22AM +0000, david@lang.hm wrote:
>> On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
>>
>>> Junio C Hamano <gitster@pobox.com> wrote:
>>>>
>>>>
>>>> I.e. use the supplied custom function to do proprietary magic, such as
>>>> reading the object lazily from elsewhere over the network. And we will
>>>> never get that magic bit back.
>>>
>>> As a maintainer I'd never accept such a patch. I'd ask for the
>>> code under read_object_custom, or toss the patch on the floor.
>>> But that doesn't stop them from distributing the patched sources
>>> like above, keeping the fun bits in the closed source portion of
>>> the executable they distribute.
>>>
>>> Maybe I just think too highly of the other guy, but I'd hope that
>>> anyone patching libgit2 like above would try to avoid it, because
>>> they'd face merge issues in the future.
>>
>> the issue that I see is that libgit2 will be (on most systems) a shared
>> library.
>>
>> what's to stop someone from taking the libgit2 code, adding the magic
>> proprietary piece, and selling a new libgit2 library binary 'just replace
>> your existing shared library with this new one and all your git related
>> programs gain this feature'
>
> Its license. GPL even with GCC exception would not allow you to do that.
> Though they could propose a fork of the library patched, with the patch
> distributed. The downside would be that their code would not be binary
> compatible with the "true" libgit2, so they would probably have to
> change the name to avoid namespace clashes, or overwrite the "real"
> library.
the proposal had been for the library to be BSD, not GPL. and I don't see
any reason why such a thing couldn't be done with a BSE library.
David Lang
> But yes, it's theoretically feasible. I'm not sure it would be worth the
> hassle, and if they respect the license (if they don't they can already
> do that with the current git anyway) then the fact that someone would
> want to do something like that would be known fact, probably not
> avoided, but known.
>
>
^ permalink raw reply
* Re: libgit2 - a true git library
From: Nicolas Pitre @ 2008-11-01 1:33 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: david, Shawn O. Pearce, git
In-Reply-To: <20081101010824.GE29036@artemis.corp>
On Sat, 1 Nov 2008, Pierre Habouzit wrote:
> See junio's example. It's rather easy to add hooks into the library to
> implement a feature outside of it. It's even possible to do it while
> preserving the ABI fully IMHO (by being a strict superset of it).
>
> The patch would be so trivial, that I see no reason why they wouldn't
> provide it. Though the real implementation of the feature that would be
> delegated through it would be in their closed source stuff.
But at that point it's a matter of public perception. It would clearly
be against the spirit of the license even though the license itself
couldn't prevent such tortuous practices. Those people doing such
things would clearly be identified as bad guys and get bad press, and
libgit contributors could even attempt law suits based on the derived
work angle. That might be just enough to prevent such things to happen.
OTOH they would certainly come out clean if the license was BSD since
the spirit of that license explicitly allows closing up the whole
library and adding extra features.
Nicolas
^ permalink raw reply
* Re: libgit2 - a true git library
From: Shawn O. Pearce @ 2008-11-01 1:19 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Junio C Hamano, Pieter de Bie, Pierre Habouzit, git, Scott Chacon
In-Reply-To: <alpine.LFD.2.00.0810312106310.13034@xanadu.home>
Nicolas Pitre <nico@cam.org> wrote:
> On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
>
> > I'm tempted to stick with int for the result type, but I don't
> > want readers to confuse our result type of 0 == success, <0 ==
> > failure with some case where we return a signed integral value as
> > a result of a computation.
>
> Does this actually happen ?
Sometimes, but yea, its not that often. I think we've already
settled that it shall be "int".
I'm updating some stuff like that (and dropping _t) as I write
this email.
> > I'm also debating the error handling. Do we return the error
> > code as the return value from the function, or do we stick it into
> > some sort of thread-global like classic "errno", or do we ask the
> > application to pass in a structure to us?
>
> Passing a structure pointer for errors is adding overhead to the API in
> all cases, please don't do that.
Agreed. Not going there.
> Both the negative code and errno style are lightweight in the common "no
> error" case. The errno style is probably more handy for those functions
> returning a pointer which should be NULL in the error case.
I'm sticking with return a negative code for now, to the extent
that some functions which return a pointer but also have many
common failure modes (e.g. git_odb_open) use an output parameter
as their first arg, so the error code can be returned as the result
of the function.
--
Shawn.
^ permalink raw reply
* Re: libgit2 - a true git library
From: Nicolas Pitre @ 2008-11-01 1:15 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Junio C Hamano, Pieter de Bie, Pierre Habouzit, git, Scott Chacon
In-Reply-To: <20081101001300.GE14786@spearce.org>
On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
> I'm tempted to stick with int for the result type, but I don't
> want readers to confuse our result type of 0 == success, <0 ==
> failure with some case where we return a signed integral value as
> a result of a computation.
Does this actually happen ?
> I'm also debating the error handling. Do we return the error
> code as the return value from the function, or do we stick it into
> some sort of thread-global like classic "errno", or do we ask the
> application to pass in a structure to us?
Passing a structure pointer for errors is adding overhead to the API in
all cases, please don't do that.
Both the negative code and errno style are lightweight in the common "no
error" case. The errno style is probably more handy for those functions
returning a pointer which should be NULL in the error case.
Nicolas
^ permalink raw reply
* Re: libgit2 - a true git library
From: Pierre Habouzit @ 2008-11-01 1:08 UTC (permalink / raw)
To: david; +Cc: Shawn O. Pearce, git
In-Reply-To: <alpine.DEB.1.10.0810311802360.5851@asgard.lang.hm>
[-- Attachment #1: Type: text/plain, Size: 2225 bytes --]
On Sat, Nov 01, 2008 at 01:04:50AM +0000, david@lang.hm wrote:
> On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
>
> >david@lang.hm wrote:
> >>On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
> >>>Junio C Hamano <gitster@pobox.com> wrote:
> >>>>
> >>>>I.e. use the supplied custom function to do proprietary magic, such
> >>>>as
> >>>>reading the object lazily from elsewhere over the network. And we
> >>>>will
> >>>>never get that magic bit back.
> >>>
> >>>Maybe I just think too highly of the other guy, but I'd hope that
> >>>anyone patching libgit2 like above would try to avoid it, because
> >>>they'd face merge issues in the future.
> >>
> >>the issue that I see is that libgit2 will be (on most systems) a shared
> >>library.
> >>
> >>what's to stop someone from taking the libgit2 code, adding the magic
> >>proprietary piece, and selling a new libgit2 library binary 'just
> >>replace
> >>your existing shared library with this new one and all your git related
> >>programs gain this feature'
> >
> >True. The only thing that prevents that is the normal GPL. The
> >LGPL and GPL+"gcc exception" allow this sort of mean behavior.
> >I doubt there's enough of a market for that; replacing a library
> >is something of a pain and if the feature really is interesting or
> >useful someone will write a clean-room re-implementation and submit
> >patches to do the same thing.
>
> how would the LGPL of GPL+gcc extention allow this? if they modify the
> code in the library and then distribute the modified library wouldn't
> they be required to distribute the changes to that library?
See junio's example. It's rather easy to add hooks into the library to
implement a feature outside of it. It's even possible to do it while
preserving the ABI fully IMHO (by being a strict superset of it).
The patch would be so trivial, that I see no reason why they wouldn't
provide it. Though the real implementation of the feature that would be
delegated through it would be in their closed source stuff.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: libgit2 - a true git library
From: Pierre Habouzit @ 2008-11-01 1:06 UTC (permalink / raw)
To: david; +Cc: Shawn O. Pearce, Junio C Hamano, Nicolas Pitre, git, Scott Chacon
In-Reply-To: <alpine.DEB.1.10.0810311738100.5851@asgard.lang.hm>
[-- Attachment #1: Type: text/plain, Size: 2074 bytes --]
On Sat, Nov 01, 2008 at 12:41:22AM +0000, david@lang.hm wrote:
> On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
>
> >Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >>
> >>I.e. use the supplied custom function to do proprietary magic, such as
> >>reading the object lazily from elsewhere over the network. And we will
> >>never get that magic bit back.
> >
> >As a maintainer I'd never accept such a patch. I'd ask for the
> >code under read_object_custom, or toss the patch on the floor.
> >But that doesn't stop them from distributing the patched sources
> >like above, keeping the fun bits in the closed source portion of
> >the executable they distribute.
> >
> >Maybe I just think too highly of the other guy, but I'd hope that
> >anyone patching libgit2 like above would try to avoid it, because
> >they'd face merge issues in the future.
>
> the issue that I see is that libgit2 will be (on most systems) a shared
> library.
>
> what's to stop someone from taking the libgit2 code, adding the magic
> proprietary piece, and selling a new libgit2 library binary 'just replace
> your existing shared library with this new one and all your git related
> programs gain this feature'
Its license. GPL even with GCC exception would not allow you to do that.
Though they could propose a fork of the library patched, with the patch
distributed. The downside would be that their code would not be binary
compatible with the "true" libgit2, so they would probably have to
change the name to avoid namespace clashes, or overwrite the "real"
library.
But yes, it's theoretically feasible. I'm not sure it would be worth the
hassle, and if they respect the license (if they don't they can already
do that with the current git anyway) then the fact that someone would
want to do something like that would be known fact, probably not
avoided, but known.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: libgit2 - a true git library
From: david @ 2008-11-01 1:04 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081101010011.GG14786@spearce.org>
On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
> david@lang.hm wrote:
>> On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
>>> Junio C Hamano <gitster@pobox.com> wrote:
>>>>
>>>> I.e. use the supplied custom function to do proprietary magic, such as
>>>> reading the object lazily from elsewhere over the network. And we will
>>>> never get that magic bit back.
>>>
>>> Maybe I just think too highly of the other guy, but I'd hope that
>>> anyone patching libgit2 like above would try to avoid it, because
>>> they'd face merge issues in the future.
>>
>> the issue that I see is that libgit2 will be (on most systems) a shared
>> library.
>>
>> what's to stop someone from taking the libgit2 code, adding the magic
>> proprietary piece, and selling a new libgit2 library binary 'just replace
>> your existing shared library with this new one and all your git related
>> programs gain this feature'
>
> True. The only thing that prevents that is the normal GPL. The
> LGPL and GPL+"gcc exception" allow this sort of mean behavior.
> I doubt there's enough of a market for that; replacing a library
> is something of a pain and if the feature really is interesting or
> useful someone will write a clean-room re-implementation and submit
> patches to do the same thing.
how would the LGPL of GPL+gcc extention allow this? if they modify the
code in the library and then distribute the modified library wouldn't they
be required to distribute the changes to that library?
they could use the LGPL or GPL+exception library with their propriatary
program, but I don't see how they could get away with modifying the
library.
David Lang
^ permalink raw reply
* Re: libgit2 - a true git library
From: Shawn O. Pearce @ 2008-11-01 1:00 UTC (permalink / raw)
To: david; +Cc: git
In-Reply-To: <alpine.DEB.1.10.0810311738100.5851@asgard.lang.hm>
david@lang.hm wrote:
> On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
>> Junio C Hamano <gitster@pobox.com> wrote:
>>>
>>> I.e. use the supplied custom function to do proprietary magic, such as
>>> reading the object lazily from elsewhere over the network. And we will
>>> never get that magic bit back.
>>
>> Maybe I just think too highly of the other guy, but I'd hope that
>> anyone patching libgit2 like above would try to avoid it, because
>> they'd face merge issues in the future.
>
> the issue that I see is that libgit2 will be (on most systems) a shared
> library.
>
> what's to stop someone from taking the libgit2 code, adding the magic
> proprietary piece, and selling a new libgit2 library binary 'just replace
> your existing shared library with this new one and all your git related
> programs gain this feature'
True. The only thing that prevents that is the normal GPL. The
LGPL and GPL+"gcc exception" allow this sort of mean behavior.
I doubt there's enough of a market for that; replacing a library
is something of a pain and if the feature really is interesting or
useful someone will write a clean-room re-implementation and submit
patches to do the same thing.
> they would only face merge issues if they need to keep up to date with
> you, and git makes it pretty easy to maintain a fork if you only have to
> do one-way merging (rere)
In other words, we're too good for our own good. ;-)
--
Shawn.
^ permalink raw reply
* Re: libgit2 - a true git library
From: Pierre Habouzit @ 2008-11-01 1:02 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Junio C Hamano, Pieter de Bie, Nicolas Pitre, git, Scott Chacon
In-Reply-To: <20081101001926.GF14786@spearce.org>
[-- Attachment #1: Type: text/plain, Size: 5028 bytes --]
On Sat, Nov 01, 2008 at 12:19:26AM +0000, Shawn O. Pearce wrote:
> Pierre Habouzit <madcoder@debian.org> wrote:
> > On Fri, Oct 31, 2008 at 11:49:11PM +0000, Junio C Hamano wrote:
> > >
> > > I understand that the apidocs/ is a very early work-in-progress, but
> > > still, it bothers me that it is unclear to me what lifetime rules are in
> > > effect on the in-core objects. For example, in C-git, commit objects are
> > > not just parsed but are modified in place as history is traversed
> > > (e.g. their flags smudged and their parents simplified). You have "flags"
> > > field in commit, which implies to me that the design shares this same
> > > "modified by processing in-place" assumption.
> >
> > I don't think it's impossible to have something efficient without this
> > kind of hacks. You just need to dissociate the objects from their
> > annotations, though use some kind of allocator that allow numbering of
> > the objects, and use that number as a lookup in an array of annotations.
> > It will require pool allocators for the annotations, but that should
> > work fine and efficientely.
>
> Interesting approach. I don't know why I didn't think of that one.
>
> You'll still need to be able to toss parts of the git graph though.
> If you just pin everything in memory under a single global object
> table you'll run server processes out of memory as they chug through
> large numbers of repositories.
Sure, but for that you just need to reinject the numbers into some kind
of free list (hint a bitmap) to reuse old slots. Of course this is some
kind of take-once never-release approach _BUT_ one can do better and
"defrag" this at times.
E.g. for a server if we take your idea, there are some times we probably
*know* nobody has kept a reference to one of the pointers and we can
reorganize some pointers around to free chunks of data not allocated.
An escape way is to use mmap + madvise for those pools, the former to
allocate the memory and the latter to drop large unused ranges when
needed (remapping with MAP_FIXED is also supposed to work but fails on
Macos it seems). Even win32 has what it takes to do so (just skim
through the code of jemalloc, that is used for mozilla, it's quite
portable on POSIX + Windows).
I was thinking that one should create and register pools of annotations
as such, define the size of the annotation (IOW the size of cell), and
let deal with that. I imagine the stuff as some kind of allocator that
would allocate the first time e.g. 4k of objects, and if you need more
8k, and if need more 16k and so on exponentially. Mapping an integer to
a cell in this kind of ropes is _really_ efficient: you need to know the
first bit set (__builtin_clz is of help with gcc, it can be emulated
with proper asm for many non-gcc platforms also) to give you the number
of the rope component that you intend to address, you clear that bit,
the resulting number is the index in that rope component of the cell
you're interested in. On most machines such an operation would be a few
cycles, which should be fairly little wrt the operation you would do
during a traversal.
Maintaining the bitmap is important, so that we can give back large
chunks of physical memory back to the system, and many malloc
implementations would likely use such kind of implementations, we would
just do our (which is arguably heavy) but gives us control on the cell
number, which is just what we need.
Of _course_ when we have a better / more natural place to put
annotations we need, let's do it ! But the kind of thing I just imagined
is more generic and can help to do arbitrary complex stuff during a
traversal.
We may want to explore a storage that is aware of the objects type also,
as I expect the liveness of objects to depend on the objects type a lot,
and some traversal to not care about some kind of objects at all, which
when combined with lazy allocation in the annotations pools, could end
up with reduced memory footprint. We could e.g. use two bits of the
"object handler" to store the type and dispatch into 3 (commit, tree,
blob) ropes instead of a big one.
The object and annotations store _is_ what makes git efficient, and is
the very service the library will have to support well. We _will_ have
to put some clever code in there and we cannot sacrifice any kind of
performance, this will be the tight loop every time. I don't think
there will be anything else in git that is so central for performance.
On the other hand, it's probably ok for the first versions of the
library to have some things in it that don't perform well in long lived
processes (repacking e.g., if we ever want to do that in the library, as
it's slow and that forking a git-repack away should not be a too
expensive cost anyway).
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: libgit2 - a true git library
From: david @ 2008-11-01 0:41 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Junio C Hamano, Nicolas Pitre, Pierre Habouzit, git, Scott Chacon
In-Reply-To: <20081031234115.GD14786@spearce.org>
On Fri, 31 Oct 2008, Shawn O. Pearce wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
>>
>>
>> I.e. use the supplied custom function to do proprietary magic, such as
>> reading the object lazily from elsewhere over the network. And we will
>> never get that magic bit back.
>
> As a maintainer I'd never accept such a patch. I'd ask for the
> code under read_object_custom, or toss the patch on the floor.
> But that doesn't stop them from distributing the patched sources
> like above, keeping the fun bits in the closed source portion of
> the executable they distribute.
>
> Maybe I just think too highly of the other guy, but I'd hope that
> anyone patching libgit2 like above would try to avoid it, because
> they'd face merge issues in the future.
the issue that I see is that libgit2 will be (on most systems) a shared
library.
what's to stop someone from taking the libgit2 code, adding the magic
proprietary piece, and selling a new libgit2 library binary 'just replace
your existing shared library with this new one and all your git related
programs gain this feature'
they would only face merge issues if they need to keep up to date with
you, and git makes it pretty easy to maintain a fork if you only have to
do one-way merging (rere)
David Lang
^ 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