* What's in git.git
From: Junio C Hamano @ 2006-08-28 7:19 UTC (permalink / raw)
To: git; +Cc: linux-kernel
This is CC'ed to the kernel list as well because the "master"
update is rather large.
On the "maint" front, I've been wanting to cut 1.4.2.1 for some
time, but various time constraints prevented me doing so so far.
I have a vague suspicion that 1.4.3 might happen before that.
Also I have been sort-of waiting for the x86-32 machine at
kernel.org to become available again so that I can do an RPM for
end users, which unfortunately hasn't happened yet.
* The 'maint' branch has these fixes since the last announcement.
Johannes Schindelin:
git-mv: special case destination "."
git-mv: fix off-by-one error
builtin-mv: readability patch
* The 'master' branch has these since the last announcement.
- Johannes's reimplementation of merge-recursive in C is in
'master' for early adopter testing. Currently it is called
'merge-recur', so you either (1) invoke it explicitly with
the -s option to 'git pull' and/or 'git merge', or (2) have
an environment variable GIT_USE_RECUR_FOR_RECURSIVE set to
non-empty string, in which case places that call
'git-merge-recursive' would use 'git-merge-recur' instead.
This has been tested in 'next' for some time, and Johannes
ran tests to reproduce all merges in post 2.6.12-rc2 kernel
history to validate it produces the same result as the
current merge-recursive. The only difference is that it is
about 6x-10x faster and you do not have to have Python
installed.
I intend to retire the current merge-recursive.py and
replace it with merge-recur before 1.4.3 happens.
- Various calls to memcmp/memcpy/memset with length '20' to
compare, copy and clear object names have been abstracted
out to hashcmp/hashcpy/hashclr wrappers, spearheaded by
David Rientjes. This would make it easier to migrate the
code to hashes of other lengths if it is ever needed.
Obviously migrating the existing data is another story.
- Updates to git-svn by Eric Wong.
- git-apply can be given --reject to produce *.rej files,
instead of failing the whole patch atomically. It also can
be given --verbose to report what it is doing.
- Rene Scharfe helped git-tar-tree find its soulmate
git-zip-tree.
- Tilman Sauerbeck taught git-daemon to setuid/setgid before
serving the clients.
- Various small fixes and clean-ups by Haavard Skinnemoen, Jakub
Narebski, Jonas Fonseca, Pierre Habouzit, Rene Scharfe,
Shawn Pearce, and Tilman Sauerbeck.
- Various documentation clean-ups by Jonas Fonseca, and Rene
Scharfe.
- The internal is readied to be able to say "32 hours ago" in
"git log" and friends by Linus; we do not have an UI to
enable it yet.
* The 'next' branch, in addition, has these.
- Various gitweb updates by Jakub Narebski with help from
Aneesh Kumar, Luben Tuikov, and Martin Waitz. The most
attractive thing these updates have is that we finally got
rid of having to use temporary files to show diffs.
I'd like to push this out to "master" soonish. You can get
a taste of how it works at the site Jakub maintains
http://front.fuw.edu.pl/cgi-bin/jnareb/gitweb.cgi
- Git.pm by Pasky with help from Dennis Stosberg, Eric Wong,
Johannes, and Pavel Roskin. During the next round I'd like
to push this out to "master" to see who screams ;-).
- upload-pack has a bit of updates still held back.
- git-daemon is taught to optionally serve git-tar-tree
output.
* In the 'pu' branch, I have my WIP of a library to walk the
index, the working tree, and zero or more tree objects in
parallel. Its test program does something that vaguely looks
like diff-index with and without --cached in parallel, but it
is not polished enough for public testing/consumption yet.
^ permalink raw reply
* Re: [PATCH] full featured formating function of the --{base,user}_path arguments,
From: Pierre Habouzit @ 2006-08-28 6:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd5altob5.fsf@assigned-by-dhcp.cox.net>
[-- Attachment #1: Type: text/plain, Size: 4527 bytes --]
Le lun 28 août 2006 07:35, Junio C Hamano a écrit :
> Pierre Habouzit <madcoder@debian.org> writes:
> > +static struct {
> > + const char *path;
> > + int use_as_fmt;
> > +} base_path;
> >
> > /* If defined, ~user notation is allowed and the string is
> > inserted * after ~user/. E.g. a request to git://host/~alice/frotz
> > would * go to /home/alice/pub_git/frotz with --user-path=pub_git.
> > */
> > -static const char *user_path;
> > +static struct {
> > + const char *path;
> > + int use_as_fmt;
> > +} user_path;
>
> Maybe it does not matter much, but I wonder if we want to keep
> two structs the same type, like:
>
> static struct {
> const char *path;
> int use_as_fmt;
> } base_path, user_path;
>
> I also wondered if we can just extend the semantics of base_path
> and user_path to autodetect the fmt-ness of them, but that means
> we would break existing setups that uses per-cent in the
> pathname. Arguably that would not be so common and we may not
> need to worry about such an installation, though. What do you
> think?
I think that merging the structs is indeed good, but that I do not like
to break (even non common) setups when avoidable. but well, I've no
*very* strong opinion on this, and if the consensus is that extending
the argument with a format is good enough, I'll go for it, it would
simplify the code a bit, and remove two arguments, both are good.
>
> > @@ -174,24 +285,45 @@ static char *path_ok(char *dir)
> > slash = dir + restlen;
> > namlen = slash - dir;
> > restlen -= namlen;
> > +
> > + if (user_path.use_as_fmt) {
> > + loginfo("host <%s>, "
> > + "userpathfmt <%s>, request <%s>, "
> > + "namlen %d, restlen %d, slash <%s>",
> > + vhost,
> > + user_path.path, dir,
> > + namlen, restlen, slash);
> > + dir = git_path_fmt(rpath, user_path.path, vhost,
> > + slash, dir + 1, namlen - 1);
>
> When vhost is NULL you would feed it to "%s", which I think
> glibc works around with (null) fine but other C libraries would
> not like it. git_path_fmt()'s logging does not have this
> problem, though.
uuhhh, I've always thought that passing NULL to a %s format was allowed
by the C standard ... I've no copy here but one at work, I will verify
it (for my peace of mind) but I agree it's not pretty.
> > + else if (base_path.path) {
> > if (*dir != '/') {
> > /* Allow only absolute */
> > logerror("'%s': Non-absolute path denied (base-path active)",
> > dir); return NULL;
> > }
> > +
> > + if (base_path.use_as_fmt) {
> > + dir = git_base_path_fmt(rpath, base_path.path, vhost, dir);
> > + } else {
> > + snprintf(rpath, PATH_MAX, "%s%s", base_path.path, dir);
>
> The level of logging in this branch and in user_path.use_as_fmt
> branch are inconsistent. Maybe the more detailed one above I
> commented about vhost==NULL case was primarily meant for
> debugging and you forgot to remove it?
well, it was here before, so I left it, but I do not liked it much
either, so if it's ok to remove it, I'd be glad to.
> > @@ -274,6 +406,7 @@ static int execute(struct sockaddr *addr
> > @@ -303,15 +436,30 @@ #endif
> > alarm(0);
> >
> > len = strlen(line);
> > +
> > + if (pktlen != len) {
> > + int arg_pos = len + 1;
> > +
> > loginfo("Extended attributes (%d bytes) exist <%.*s>",
> > (int) pktlen - len,
> > + (int) pktlen - len, line + arg_pos);
> > +
> > + while (arg_pos < pktlen) {
> > + int arg_len = strlen(line + arg_pos);
> > +
> > + if (!strncmp("host=", line + arg_pos, 5)) {
> > + vhost = line + arg_pos + 5;
> > + }
> > +
> > + arg_pos += arg_len + 1;
> > + }
> > + }
> > +
>
> I think it is easier to do:
>
> if (!vhost)
> vhost = default_host;
>
> and have git_base_path_fmt() barf if the format calls for %h and
> vhost passed to it is NULL. Lack of "host=" in the request is
> logged here already.
hmmm, lack of host= is only logged in the git_path_fmt function atm. Its
presence is logged though. But I could obviously log that it's missing
while parsing the extending attributes, and then only need to barf when
the %h is actually needed in the format string. That seems indeed
easier. Will do.
--
·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: Packfile can't be mapped
From: Shawn Pearce @ 2006-08-28 6:00 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Nicolas Pitre, git, Jon Smirl
In-Reply-To: <Pine.LNX.4.64.0608272133390.27779@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> wrote:
>
> On Mon, 28 Aug 2006, Nicolas Pitre wrote:
> >
> > Good job indeed. Oh and you probably should not bother trying to
> > deltify commit objects at all since that would be a waste of time.
>
> It might not necessarily always be a waste of time. Especially if you have
> multiple branches tracking a "maintenance" branch, you often end up having
> the same commit message repeated several times in "unrelated" commits
> (they're really the same commit, applied to another branch).
>
> Also, I could imagine that some automated system generates very verbose
> (and possibly very regular) commit messages, so under certain
> circumstances it may well make sense to see if the commits migth delta
> against each other.
>
> But I'll agree that in normal use it's not likely to be a huge saving,
> though. It's probably not worth doing for the fast importer unless it just
> happens to fall out of the code very easily.
Does git-pack-objects attempt to delta commits against each other?
I've been thinking about applying a pack-local but zlib-stream
global dictionary. If we added three global dicationaries to the
front of the pack file, one for commits, one for trees and one
for blobs, and use those as the global dictionaries for the zlib
streams stored within that pack we could probably get a good space
savings for trees and commits.
I'd suspect that for many projects the commit global dictionary
would contain the common required strings such as:
'tree ', 'parent ', 'committer ', 'author ', 'Signed-off-by: '
plus the top author/committer name/email combination strings.
For GIT I'd expect 'Junio C Hamano <junkio@cox.net>' to be way up
there in terms of frequency within commit objects. Finding the most
common authors and committer strings would be trivial, as would
finding the most common 'footer' strings such as 'Signed-off-by: '
and 'Acked-by: '.
I think the same is true of trees, with '10644 ', '10755 ', '40000 '
being way up there, but also file names that commonly appear within
trees, e.g. "Makefile.in\0".
Blobs would be more difficult to generate a reasonable global
dictionary for. But for some projects a crude estimated dictionary
can shave off at least 4% of pack size (true in both GIT and Mozilla
sources it seems).
Of course the major problem with pack-local, stream global
dictionaries is it voids the ability to reuse that zlib'd content
from that pack in another pack without wholesale copying the
dictionary as well. This is an issue for servers which want to
copy out the pack entry without recompressing it but also want the
storage savings from the global dictionaries.
But then again, if we just delta against a commit which uses the
same author and committer, or against the same tree but different
version then there should be a lot of delta copying from the base...
which easily allows entry reuse and should provide similiar space
savings, providing the delta depth is deep enough (or the delta graph
is wide enough) to minimize the number of base objects containing
repeated occurrances of the common strings.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] parse_object: check if buffer is non-NULL before freeing it
From: Junio C Hamano @ 2006-08-28 5:46 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0608272131020.27779@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> On Sun, 27 Aug 2006, Junio C Hamano wrote:
>>
>> Eh, free(NULL) should work just fine. It is "other places" that
>> is misguided and needs to be fixed.
>
> Well, some very old libraries will SIGSEGV on free(NULL).
>
> Admittedly those libraries are either very old or _very_ broken, but if
> you want to be strictly portable, you should not ever pass NULL to free(),
> unless you actually got it from a malloc(0) (and even then, it might be a
> really broken libc that just ran out of memory).
Fair enough, but I think there are many places we already assume
the library handles free(NULL) sensibly.
> I actually suspect we should wrap all free() calls as "xfree()", which may
> also help us some day if we want to do any memory usage statistics.
That sounds sensible.
Another thing I was thinking about was to extend the existing
XMALLOC_POISON debugging to allow also xrealloc()'ed area. That
would unfortunately involve wrapping strdup() and x*alloc() to
make sure all allocations we do go through xmalloc() and then
store the current allocation size somewhere hidden (immediately
before, perhaps) in the area xmalloc() returns, but at that
point running git under valgrind would probably be easier.
^ permalink raw reply
* Re: [PATCH] full featured formating function of the --{base,user}_path arguments,
From: Junio C Hamano @ 2006-08-28 5:35 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <11566787581030-git-send-email-madcoder@debian.org>
Pierre Habouzit <madcoder@debian.org> writes:
> +static struct {
> + const char *path;
> + int use_as_fmt;
> +} base_path;
>
> /* If defined, ~user notation is allowed and the string is inserted
> * after ~user/. E.g. a request to git://host/~alice/frotz would
> * go to /home/alice/pub_git/frotz with --user-path=pub_git.
> */
> -static const char *user_path;
> +static struct {
> + const char *path;
> + int use_as_fmt;
> +} user_path;
Maybe it does not matter much, but I wonder if we want to keep
two structs the same type, like:
static struct {
const char *path;
int use_as_fmt;
} base_path, user_path;
I also wondered if we can just extend the semantics of base_path
and user_path to autodetect the fmt-ness of them, but that means
we would break existing setups that uses per-cent in the
pathname. Arguably that would not be so common and we may not
need to worry about such an installation, though. What do you
think?
> @@ -174,24 +285,45 @@ static char *path_ok(char *dir)
> slash = dir + restlen;
> namlen = slash - dir;
> restlen -= namlen;
> +
> + if (user_path.use_as_fmt) {
> + loginfo("host <%s>, "
> + "userpathfmt <%s>, request <%s>, "
> + "namlen %d, restlen %d, slash <%s>",
> + vhost,
> + user_path.path, dir,
> + namlen, restlen, slash);
> + dir = git_path_fmt(rpath, user_path.path, vhost,
> + slash, dir + 1, namlen - 1);
When vhost is NULL you would feed it to "%s", which I think
glibc works around with (null) fine but other C libraries would
not like it. git_path_fmt()'s logging does not have this
problem, though.
> + else if (base_path.path) {
> if (*dir != '/') {
> /* Allow only absolute */
> logerror("'%s': Non-absolute path denied (base-path active)", dir);
> return NULL;
> }
> +
> + if (base_path.use_as_fmt) {
> + dir = git_base_path_fmt(rpath, base_path.path, vhost, dir);
> + } else {
> + snprintf(rpath, PATH_MAX, "%s%s", base_path.path, dir);
The level of logging in this branch and in user_path.use_as_fmt
branch are inconsistent. Maybe the more detailed one above I
commented about vhost==NULL case was primarily meant for
debugging and you forgot to remove it?
> @@ -274,6 +406,7 @@ static int execute(struct sockaddr *addr
> @@ -303,15 +436,30 @@ #endif
> alarm(0);
>
> len = strlen(line);
> +
> + if (pktlen != len) {
> + int arg_pos = len + 1;
> +
> loginfo("Extended attributes (%d bytes) exist <%.*s>",
> (int) pktlen - len,
> + (int) pktlen - len, line + arg_pos);
> +
> + while (arg_pos < pktlen) {
> + int arg_len = strlen(line + arg_pos);
> +
> + if (!strncmp("host=", line + arg_pos, 5)) {
> + vhost = line + arg_pos + 5;
> + }
> +
> + arg_pos += arg_len + 1;
> + }
> + }
> +
I think it is easier to do:
if (!vhost)
vhost = default_host;
and have git_base_path_fmt() barf if the format calls for %h and
vhost passed to it is NULL. Lack of "host=" in the request is
logged here already.
^ permalink raw reply
* Re: Packfile can't be mapped
From: Shawn Pearce @ 2006-08-28 5:33 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git, Jon Smirl
In-Reply-To: <Pine.LNX.4.64.0608280014190.3683@localhost.localdomain>
Nicolas Pitre <nico@cam.org> wrote:
> On Sun, 27 Aug 2006, Shawn Pearce wrote:
>
> > I'm going to try to get tree deltas written to the pack sometime this
> > week. That should compact this intermediate pack down to something
> > that git-pack-objects would be able to successfully mmap into a
> > 32 bit address space. A complete repack with no delta reuse will
> > hopefully generate a pack closer to 400 MB in size. But I know
> > Jon would like to get that pack even smaller. :)
>
> One thing to consider in your code (if you didn't implement that
> already) is to _not_ attempt any delta on any object whose size is
> smaller than 50 bytes, and then limit the maximum delta size to
> object_size/2 - 20 (use that for the last argument to diff-delta() and
> store the undeltified object when diff-delta returns NULL). This way
> you'll avoid creating delta objects that are most likely to end up being
> _larger_ than the undeltified object.
I haven't tried this. Should be trivial to implement. Thanks for
the suggestion.
> > I should point out that the input stream to fast-import was 20 GB
> > (completely decompressed revisions from RCS) plus all commit data.
> > The original CVS ,v files are around 3 GB. An archive .tar.gz'ing
> > the ,v files is around 550 MB. Going to only 1.7 GB without tree
> > or commit deltas is certainly pretty good. :)
>
> Good job indeed. Oh and you probably should not bother trying to
> deltify commit objects at all since that would be a waste of time.
I wasn't going to bother even trying to delta the commits. In this
import the 200k commits isn't a very large percentage of the data.
As I'm sure you are well aware its pretty much a waste time to try
with the commits, especially with an "intermediate" pack such as
this one.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] Refactor sha1_pack_index_name and sha1_pack_name to use a common backend
From: Junio C Hamano @ 2006-08-28 4:40 UTC (permalink / raw)
To: Jonas Fonseca; +Cc: git
In-Reply-To: <20060828001610.GC20904@diku.dk>
Jonas Fonseca <fonseca@diku.dk> writes:
> This adds another pair of static buffers, if that's a problem and the
> cleanup is still wanted I can change it to use malloc.
To be perfectly honest, I would rather get rid of these two
functions, and also sha1[] member from struct packed_git.
The real core git does not impose any limit to the name of the
packfiles, other than that the name of the datafile ends with
".pack" and that its corresponding pack index file has the same
name ending with ".idx". It is only the packfile support which
was later bolted on to commit walkers by commit bf592c5 that
introduced this silly expectation that they are always of form
"pack-[0-9a-f]{40}.pack" and "pack-[0-9a-f]{40}.idx".
For example, fetch_pack() in local-fetch.c is given an object
name and tries to find the packfile that contains it by looking
at ".idx" files downloaded from the other side. When it finds
one, it fabricates a name for the pack file by what is in
target->sha1 (where target is of type "struct packed_git").
However, struct packed_git already has perfectly good place that
is meant to store the real name of the packfile, so this is
totally unnecessary.
As a clean-up, I'd rather see a patch that removes the need for
these two functions and one struct member, rather than keeping
these two misguided functions and consolidating their
implementations.
^ permalink raw reply
* Re: Packfile can't be mapped
From: Linus Torvalds @ 2006-08-28 4:36 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Shawn Pearce, git, Jon Smirl
In-Reply-To: <Pine.LNX.4.64.0608280014190.3683@localhost.localdomain>
On Mon, 28 Aug 2006, Nicolas Pitre wrote:
>
> Good job indeed. Oh and you probably should not bother trying to
> deltify commit objects at all since that would be a waste of time.
It might not necessarily always be a waste of time. Especially if you have
multiple branches tracking a "maintenance" branch, you often end up having
the same commit message repeated several times in "unrelated" commits
(they're really the same commit, applied to another branch).
Also, I could imagine that some automated system generates very verbose
(and possibly very regular) commit messages, so under certain
circumstances it may well make sense to see if the commits migth delta
against each other.
But I'll agree that in normal use it's not likely to be a huge saving,
though. It's probably not worth doing for the fast importer unless it just
happens to fall out of the code very easily.
Linus
^ permalink raw reply
* Re: [PATCH] parse_object: check if buffer is non-NULL before freeing it
From: Linus Torvalds @ 2006-08-28 4:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonas Fonseca, git
In-Reply-To: <7vsljhtrsv.fsf@assigned-by-dhcp.cox.net>
On Sun, 27 Aug 2006, Junio C Hamano wrote:
>
> Eh, free(NULL) should work just fine. It is "other places" that
> is misguided and needs to be fixed.
Well, some very old libraries will SIGSEGV on free(NULL).
Admittedly those libraries are either very old or _very_ broken, but if
you want to be strictly portable, you should not ever pass NULL to free(),
unless you actually got it from a malloc(0) (and even then, it might be a
really broken libc that just ran out of memory).
I actually suspect we should wrap all free() calls as "xfree()", which may
also help us some day if we want to do any memory usage statistics.
Linus
^ permalink raw reply
* Re: Packfile can't be mapped
From: Nicolas Pitre @ 2006-08-28 4:27 UTC (permalink / raw)
To: Shawn Pearce; +Cc: git, Jon Smirl
In-Reply-To: <20060828024720.GD24204@spearce.org>
On Sun, 27 Aug 2006, Shawn Pearce wrote:
> I'm going to try to get tree deltas written to the pack sometime this
> week. That should compact this intermediate pack down to something
> that git-pack-objects would be able to successfully mmap into a
> 32 bit address space. A complete repack with no delta reuse will
> hopefully generate a pack closer to 400 MB in size. But I know
> Jon would like to get that pack even smaller. :)
One thing to consider in your code (if you didn't implement that
already) is to _not_ attempt any delta on any object whose size is
smaller than 50 bytes, and then limit the maximum delta size to
object_size/2 - 20 (use that for the last argument to diff-delta() and
store the undeltified object when diff-delta returns NULL). This way
you'll avoid creating delta objects that are most likely to end up being
_larger_ than the undeltified object.
> I should point out that the input stream to fast-import was 20 GB
> (completely decompressed revisions from RCS) plus all commit data.
> The original CVS ,v files are around 3 GB. An archive .tar.gz'ing
> the ,v files is around 550 MB. Going to only 1.7 GB without tree
> or commit deltas is certainly pretty good. :)
Good job indeed. Oh and you probably should not bother trying to
deltify commit objects at all since that would be a waste of time.
Nicolas
^ permalink raw reply
* Re: [PATCH] parse_object: check if buffer is non-NULL before freeing it
From: Junio C Hamano @ 2006-08-28 4:20 UTC (permalink / raw)
To: Jonas Fonseca; +Cc: git
In-Reply-To: <20060828003129.GE20904@diku.dk>
Jonas Fonseca <fonseca@diku.dk> writes:
> Two code paths may cause this and other places in the source have the
> courtesy to do the check.
Eh, free(NULL) should work just fine. It is "other places" that
is misguided and needs to be fixed.
- >8 -
[PATCH] free(NULL) is perfectly valid.
Jonas noticed some places say "if (X) free(X)" which is totally
unnecessary.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
builtin-apply.c | 6 ++----
builtin-fmt-merge-msg.c | 3 +--
builtin-repo-config.c | 6 ++----
builtin-rev-list.c | 6 ++----
config.c | 6 ++----
fetch.c | 3 +--
http-fetch.c | 13 ++++---------
http-push.c | 13 ++++---------
path-list.c | 3 +--
refs.c | 6 ++----
10 files changed, 21 insertions(+), 44 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index b47ccac..1a1deaf 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1131,8 +1131,7 @@ static struct fragment *parse_binary_hun
return frag;
corrupt:
- if (data)
- free(data);
+ free(data);
*status_p = -1;
error("corrupt binary patch at line %d: %.*s",
linenr-1, llen-1, buffer);
@@ -1329,8 +1328,7 @@ static void show_stats(struct patch *pat
printf(" %s%-*s |%5d %.*s%.*s\n", prefix,
len, name, patch->lines_added + patch->lines_deleted,
add, pluses, del, minuses);
- if (qname)
- free(qname);
+ free(qname);
}
static int read_old_data(struct stat *st, const char *path, void *buf, unsigned long size)
diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index a5ed8db..76d22b4 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -55,8 +55,7 @@ static void free_list(struct list *list)
for (i = 0; i < list->nr; i++) {
free(list->list[i]);
- if (list->payload[i])
- free(list->payload[i]);
+ free(list->payload[i]);
}
free(list->list);
free(list->payload);
diff --git a/builtin-repo-config.c b/builtin-repo-config.c
index c416480..6560cf1 100644
--- a/builtin-repo-config.c
+++ b/builtin-repo-config.c
@@ -122,10 +122,8 @@ static int get_value(const char* key_, c
ret = (seen == 1) ? 0 : 1;
free_strings:
- if (repo_config)
- free(repo_config);
- if (global)
- free(global);
+ free(repo_config);
+ free(global);
return ret;
}
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index bc48a3e..7f3e1fc 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -93,10 +93,8 @@ static void show_commit(struct commit *c
free_commit_list(commit->parents);
commit->parents = NULL;
}
- if (commit->buffer) {
- free(commit->buffer);
- commit->buffer = NULL;
- }
+ free(commit->buffer);
+ commit->buffer = NULL;
}
static void process_blob(struct blob *blob,
diff --git a/config.c b/config.c
index 82b3562..d9f2b78 100644
--- a/config.c
+++ b/config.c
@@ -361,8 +361,7 @@ int git_config(config_fn_t fn)
}
ret += git_config_from_file(fn, filename);
- if (repo_config)
- free(repo_config);
+ free(repo_config);
return ret;
}
@@ -734,8 +733,7 @@ int git_config_set_multivar(const char*
out_free:
if (0 <= fd)
close(fd);
- if (config_filename)
- free(config_filename);
+ free(config_filename);
if (lock_file) {
unlink(lock_file);
free(lock_file);
diff --git a/fetch.c b/fetch.c
index ef60b04..7d3812c 100644
--- a/fetch.c
+++ b/fetch.c
@@ -302,8 +302,7 @@ int pull(int targets, char **target, con
if (ret)
goto unlock_and_fail;
}
- if (msg)
- free(msg);
+ free(msg);
return 0;
diff --git a/http-fetch.c b/http-fetch.c
index 7619b33..6806f36 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -696,10 +696,8 @@ xml_start_tag(void *userData, const char
strcat(ctx->name, ".");
strcat(ctx->name, c);
- if (ctx->cdata) {
- free(ctx->cdata);
- ctx->cdata = NULL;
- }
+ free(ctx->cdata);
+ ctx->cdata = NULL;
ctx->userFunc(ctx, 0);
}
@@ -726,8 +724,7 @@ static void
xml_cdata(void *userData, const XML_Char *s, int len)
{
struct xml_ctx *ctx = (struct xml_ctx *)userData;
- if (ctx->cdata)
- free(ctx->cdata);
+ free(ctx->cdata);
ctx->cdata = xmalloc(len + 1);
strlcpy(ctx->cdata, s, len + 1);
}
@@ -765,9 +762,7 @@ static void handle_remote_ls_ctx(struct
ls->dentry_flags |= IS_DIR;
}
} else if (!strcmp(ctx->name, DAV_PROPFIND_RESP)) {
- if (ls->dentry_name) {
- free(ls->dentry_name);
- }
+ free(ls->dentry_name);
ls->dentry_name = NULL;
ls->dentry_flags = 0;
}
diff --git a/http-push.c b/http-push.c
index 04cb238..7814666 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1238,10 +1238,8 @@ xml_start_tag(void *userData, const char
strcat(ctx->name, ".");
strcat(ctx->name, c);
- if (ctx->cdata) {
- free(ctx->cdata);
- ctx->cdata = NULL;
- }
+ free(ctx->cdata);
+ ctx->cdata = NULL;
ctx->userFunc(ctx, 0);
}
@@ -1268,8 +1266,7 @@ static void
xml_cdata(void *userData, const XML_Char *s, int len)
{
struct xml_ctx *ctx = (struct xml_ctx *)userData;
- if (ctx->cdata)
- free(ctx->cdata);
+ free(ctx->cdata);
ctx->cdata = xmalloc(len + 1);
strlcpy(ctx->cdata, s, len + 1);
}
@@ -1518,9 +1515,7 @@ static void handle_remote_ls_ctx(struct
ls->dentry_flags |= IS_DIR;
}
} else if (!strcmp(ctx->name, DAV_PROPFIND_RESP)) {
- if (ls->dentry_name) {
- free(ls->dentry_name);
- }
+ free(ls->dentry_name);
ls->dentry_name = NULL;
ls->dentry_flags = 0;
}
diff --git a/path-list.c b/path-list.c
index f15a10d..b1ee72d 100644
--- a/path-list.c
+++ b/path-list.c
@@ -85,8 +85,7 @@ void path_list_clear(struct path_list *l
for (i = 0; i < list->nr; i++) {
if (list->strdup_paths)
free(list->items[i].path);
- if (list->items[i].util)
- free(list->items[i].util);
+ free(list->items[i].util);
}
free(list->items);
}
diff --git a/refs.c b/refs.c
index e70ef0a..aab14fc 100644
--- a/refs.c
+++ b/refs.c
@@ -348,10 +348,8 @@ void unlock_ref(struct ref_lock *lock)
if (lock->lk)
rollback_lock_file(lock->lk);
}
- if (lock->ref_file)
- free(lock->ref_file);
- if (lock->log_file)
- free(lock->log_file);
+ free(lock->ref_file);
+ free(lock->log_file);
free(lock);
}
--
1.4.2.g2f76
^ permalink raw reply related
* Re: Packfile can't be mapped
From: Shawn Pearce @ 2006-08-28 2:47 UTC (permalink / raw)
To: git; +Cc: Jon Smirl
In-Reply-To: <9e4733910608271804j762960a8ud83654c78ebe009a@mail.gmail.com>
Jon Smirl <jonsmirl@gmail.com> wrote:
> git-repack can't handle my 1.75GB pack file. I am running x86 with 3GB
> address space.
>
> -rw-rw-r-- 1 jonsmirl jonsmirl 47221712 Aug 27 20:29 testme.idx
> -rw-rw-r-- 1 jonsmirl jonsmirl 1754317619 Aug 27 20:29 testme.pack
>
> [jonsmirl@jonsmirl t1]$ git-repack -a -f --window=50 --depth=5000
> Generating pack...
> Done counting 1963325 objects.
> fatal: packfile .git/objects/pack/testme.pack cannot be mapped.
> [jonsmirl@jonsmirl t1]$
>
> It is built from Mozilla CVS but it is an intermediate stage of our
> work. The fast-import tool isn't diffing directory tree which makes
> the pack much bigger than it needs to be. Shawn is working on the
> packing code.
I'm going to try to get tree deltas written to the pack sometime this
week. That should compact this intermediate pack down to something
that git-pack-objects would be able to successfully mmap into a
32 bit address space. A complete repack with no delta reuse will
hopefully generate a pack closer to 400 MB in size. But I know
Jon would like to get that pack even smaller. :)
I should point out that the input stream to fast-import was 20 GB
(completely decompressed revisions from RCS) plus all commit data.
The original CVS ,v files are around 3 GB. An archive .tar.gz'ing
the ,v files is around 550 MB. Going to only 1.7 GB without tree
or commit deltas is certainly pretty good. :)
> ---------------------------------------------------
> Alloc'd objects: 1968000 ( 1892000 overflow )
> Total objects: 1967527 ( 41856 duplicates)
> blobs : 633842 ( 0 duplicates)
> trees : 1131208 ( 41856 duplicates)
> commits: 200921 ( 0 duplicates)
> tags : 1556 ( 0 duplicates)
> Total branches: 1600 ( 7985 loads )
> marks: 1048576 ( 200921 unique )
> atoms: 56803
> Memory total: 66908 KiB
> pools: 5408 KiB
> objects: 61500 KiB
> Pack remaps: 9501
> ---------------------------------------------------
> Pack size: 1713200 KiB
> Index size: 46114 KiB
All of that says that aside from the 1.7 GB output file fast-import
ran extremely well. About 1.9 million objects were written into
the output pack file, with 41k duplicate trees (duplicate blobs
were removed by cvs2svn prior to fast-import so they don't appear).
200k commits were created across 1600 branches. And we did it in
only 67 MB of memory.
We also had ~8000 LRU cache misses related to our branch data;
this just means that cvs2svn likes to frequently jump around
between branches rather than import an entire branch at a time.
Boosting the size of the LRU cache (at the expense of needing more
memory) should reduce those cache misses as well as 'Pack remaps'.
I'd also like to clean up that pack remapping code and move it
into sha1_file.c. Its an implementation of partial pack mapping
and it is apparently working quite well for us in fast-import.
It may help GIT deal with very large packs (e.g. 1.7 GB) on smaller
address space systems (e.g. 32 bit).
We're not confident that this import is completely valid yet.
We have a few translation issues we're still working on. But now
that we have a complete pack going from start to finish we can start
to focus on those issues. Especially since this entire process
(,v to .pack) is less than half a day to run.
--
Shawn.
^ permalink raw reply
* Packfile can't be mapped
From: Jon Smirl @ 2006-08-28 1:04 UTC (permalink / raw)
To: git, Shawn Pearce
git-repack can't handle my 1.75GB pack file. I am running x86 with 3GB
address space.
-rw-rw-r-- 1 jonsmirl jonsmirl 47221712 Aug 27 20:29 testme.idx
-rw-rw-r-- 1 jonsmirl jonsmirl 1754317619 Aug 27 20:29 testme.pack
[jonsmirl@jonsmirl t1]$ git-repack -a -f --window=50 --depth=5000
Generating pack...
Done counting 1963325 objects.
fatal: packfile .git/objects/pack/testme.pack cannot be mapped.
[jonsmirl@jonsmirl t1]$
It is built from Mozilla CVS but it is an intermediate stage of our
work. The fast-import tool isn't diffing directory tree which makes
the pack much bigger than it needs to be. Shawn is working on the
packing code.
---------------------------------------------------
Alloc'd objects: 1968000 ( 1892000 overflow )
Total objects: 1967527 ( 41856 duplicates)
blobs : 633842 ( 0 duplicates)
trees : 1131208 ( 41856 duplicates)
commits: 200921 ( 0 duplicates)
tags : 1556 ( 0 duplicates)
Total branches: 1600 ( 7985 loads )
marks: 1048576 ( 200921 unique )
atoms: 56803
Memory total: 66908 KiB
pools: 5408 KiB
objects: 61500 KiB
Pack remaps: 9501
---------------------------------------------------
Pack size: 1713200 KiB
Index size: 46114 KiB
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: File archiver using git
From: Matt McCutchen @ 2006-08-28 0:40 UTC (permalink / raw)
To: Grzegorz Kulewski; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0608271528130.8018@alpha.polcom.net>
On 8/27/06, Grzegorz Kulewski <kangur@polcom.net> wrote:
> Does it (and GIT in general) work ok with file permisions, ownership, soft
> and hard links, named sockets, device files and similar "strange"
> filesystem objects? Do I need any options to GIT to make it work with
> them?
Git tracks data, not filesystems. That means it doesn't handle any of
the things you mentioned except soft links. It doesn't even handle
empty directories, though I don't accept the argument that they do not
constitute data. If you want to capture a filesystem, I would
recommend running "find . -ls" to create a listing file from which
everything you mentioned can be reconstructed and then putting the
filesystem together with the listing file into git.
> Can I for example securely backup or even version-control /etc directory?
With the above technique, you can do both. However, if you just want
to back up /etc, you might be better served by a backup tool. For
example, I use rsnapshot ( http://www.rsnapshot.org/ ).
Matt
^ permalink raw reply
* [PATCH] parse_object: check if buffer is non-NULL before freeing it
From: Jonas Fonseca @ 2006-08-28 0:31 UTC (permalink / raw)
To: git
Two code paths may cause this and other places in the source have the
courtesy to do the check.
Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
---
object.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/object.c b/object.c
index 9281300..c08347f 100644
--- a/object.c
+++ b/object.c
@@ -173,7 +173,8 @@ struct object *parse_object(const unsign
} else {
obj = NULL;
}
- free(buffer);
+ if (buffer)
+ free(buffer);
return obj;
}
return NULL;
--
1.4.2.g2f76-dirty
--
Jonas Fonseca
^ permalink raw reply related
* [PATCH] Use xcalloc instead of calloc
From: Jonas Fonseca @ 2006-08-28 0:26 UTC (permalink / raw)
To: git
Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
---
object-refs.c | 2 +-
object.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/object-refs.c b/object-refs.c
index b1b8065..b0034e4 100644
--- a/object-refs.c
+++ b/object-refs.c
@@ -30,7 +30,7 @@ static void grow_refs_hash(void)
int new_hash_size = (refs_hash_size + 1000) * 3 / 2;
struct object_refs **new_hash;
- new_hash = calloc(new_hash_size, sizeof(struct object_refs *));
+ new_hash = xcalloc(new_hash_size, sizeof(struct object_refs *));
for (i = 0; i < refs_hash_size; i++) {
struct object_refs *ref = refs_hash[i];
if (!ref)
diff --git a/object.c b/object.c
index 60bf16b..9281300 100644
--- a/object.c
+++ b/object.c
@@ -73,7 +73,7 @@ static void grow_object_hash(void)
int new_hash_size = obj_hash_size < 32 ? 32 : 2 * obj_hash_size;
struct object **new_hash;
- new_hash = calloc(new_hash_size, sizeof(struct object *));
+ new_hash = xcalloc(new_hash_size, sizeof(struct object *));
for (i = 0; i < obj_hash_size; i++) {
struct object *obj = obj_hash[i];
if (!obj)
--
1.4.2.g2f76-dirty
--
Jonas Fonseca
^ permalink raw reply related
* [PATCH] Refactor sha1_pack_index_name and sha1_pack_name to use a common backend
From: Jonas Fonseca @ 2006-08-28 0:16 UTC (permalink / raw)
To: git
Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
---
Tested with doing a local cg-clone, since there doesn't seem to be any
tests of users, such as git-local-fetch, under t/.
This adds another pair of static buffers, if that's a problem and the
cleanup is still wanted I can change it to use malloc.
sha1_file.c | 64 +++++++++++++++++++++++------------------------------------
1 files changed, 25 insertions(+), 39 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 01aa745..5a846f5 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -147,65 +147,51 @@ char *sha1_file_name(const unsigned char
return base;
}
-char *sha1_pack_name(const unsigned char *sha1)
+static int fill_sha1_pack_name(const unsigned char *sha1, char base[], size_t baselen,
+ int dir_offset, const char *extension)
{
static const char hex[] = "0123456789abcdef";
- static char *name, *base, *buf;
- static const char *last_objdir;
const char *sha1_file_directory = get_object_directory();
+ char *buf;
int i;
- if (!last_objdir || strcmp(last_objdir, sha1_file_directory)) {
- int len = strlen(sha1_file_directory);
- if (base)
- free(base);
- base = xmalloc(len + 60);
- sprintf(base, "%s/pack/pack-1234567890123456789012345678901234567890.pack", sha1_file_directory);
- name = base + len + 11;
- if (last_objdir)
- free((char *) last_objdir);
- last_objdir = strdup(sha1_file_directory);
+ base[dir_offset] = 0;
+ if (strcmp(base, sha1_file_directory)) {
+ dir_offset = strlen(sha1_file_directory);
+ if (snprintf(base, baselen,
+ "%s/pack/pack-1234567890123456789012345678901234567890.%s",
+ sha1_file_directory, extension) >= baselen)
+ die("pack name too long");
}
+ base[dir_offset] = '/';
- buf = name;
+ buf = base + dir_offset + 11;
for (i = 0; i < 20; i++) {
unsigned int val = *sha1++;
+
*buf++ = hex[val >> 4];
*buf++ = hex[val & 0xf];
}
-
- return base;
+
+ return dir_offset;
}
char *sha1_pack_index_name(const unsigned char *sha1)
{
- static const char hex[] = "0123456789abcdef";
- static char *name, *base, *buf;
- static const char *last_objdir;
- const char *sha1_file_directory = get_object_directory();
- int i;
+ static char base[PATH_MAX + 1];
+ static int offset;
- if (!last_objdir || strcmp(last_objdir, sha1_file_directory)) {
- int len = strlen(sha1_file_directory);
- if (base)
- free(base);
- base = xmalloc(len + 60);
- sprintf(base, "%s/pack/pack-1234567890123456789012345678901234567890.idx", sha1_file_directory);
- name = base + len + 11;
- if (last_objdir)
- free((char *) last_objdir);
- last_objdir = strdup(sha1_file_directory);
- }
+ offset = fill_sha1_pack_name(sha1, base, sizeof(base), offset, "idx");
+ return base;
+}
- buf = name;
+char *sha1_pack_name(const unsigned char *sha1)
+{
+ static char base[PATH_MAX + 1];
+ static int offset;
- for (i = 0; i < 20; i++) {
- unsigned int val = *sha1++;
- *buf++ = hex[val >> 4];
- *buf++ = hex[val & 0xf];
- }
-
+ offset = fill_sha1_pack_name(sha1, base, sizeof(base), offset, "pack");
return base;
}
--
1.4.2.g2f76-dirty
--
Jonas Fonseca
^ permalink raw reply related
* [PATCH] Use fstat instead of fseek
From: Jonas Fonseca @ 2006-08-27 23:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7v4pvz11o6.fsf@assigned-by-dhcp.cox.net>
Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
---
dir.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
Junio C Hamano <junkio@cox.net> wrote Sat, Aug 26, 2006:
> Linus Torvalds <torvalds@osdl.org> writes:
>
> > I really think you'd be better off rewriting that to use "fstat()"
> > instead. I don't know why it uses two lseek's, but it's wrong, and looks
> > like some bad habit Junio picked up at some point.
>
> I think the code was written to avoid getting confused by
> unseekable input (pipes) but was done in early morning before
> the first shot of caffeine.
I take it that you want this change, so here's a little addition to the
"use X instead of Y" series.
diff --git a/dir.c b/dir.c
index d53d48f..5a40d8f 100644
--- a/dir.c
+++ b/dir.c
@@ -112,17 +112,15 @@ static int add_excludes_from_file_1(cons
int baselen,
struct exclude_list *which)
{
+ struct stat st;
int fd, i;
long size;
char *buf, *entry;
fd = open(fname, O_RDONLY);
- if (fd < 0)
+ if (fd < 0 || fstat(fd, &st) < 0)
goto err;
- size = lseek(fd, 0, SEEK_END);
- if (size < 0)
- goto err;
- lseek(fd, 0, SEEK_SET);
+ size = st.st_size;
if (size == 0) {
close(fd);
return 0;
--
1.4.2.g2f76-dirty
--
Jonas Fonseca
^ permalink raw reply related
* Re: [PATCH/RFC] gitweb: Make git_print_log generic; git_print_simplified_log uses it
From: Junio C Hamano @ 2006-08-27 23:11 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <200608272355.07625.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> The RFC is about style of git_print_log function. Is it a good idea
> and good implementation to pass miscelaneus options as hash values
> instead of using fixed order of parameters, and passing 1 or undef?
> Is it a good naming of parameters like '-remove_title', or would
> 'remove_title' or 'remove-title' be better?
I do not personally like the line-noise sub prototypes, since I
have not seen it buys you much in real programs. Although some
cute hacks like rolling your own control structure lookalikes
cannot be done without them, they are just that -- cute hacks --
and tends to obfuscate what is really happening behind the scene
(not that writing anything in Perl is not already obfuscating
things ;-)). But maybe it is just me.
I think you already use CGI.pm and the argument passing style
using things like "-remove_title" is the norm there, so I do not
have objections against it. You might even be able to lift the
code CGI.pm uses to implement the hash-style passing with
defaults.
^ permalink raw reply
* git-apply --reject
From: Junio C Hamano @ 2006-08-27 23:03 UTC (permalink / raw)
To: git
The git-apply program in "next" has had a few additional options
for some time: --reject and --verbose. I've been using this
after a failed "git am" attempt:
git apply --reject --verbose .dotest/patch
This, instead of the usual "patch does not apply so I would not
touch any of your working tree files" behaviour, applies hunks
that are applicable and leaves *.rej files the rejected hunks,
and it reports what it is doing. With --index, files with a
rejected hunk do not get their index entries updated at all, so
"git diff" will show the hunks that successfully got applied.
This seems to be working rather nicely, so I intend to make it
graduate to "master". I made a mistake of adding the man page
that already talks about these options to "master", but with an
incorrect the description for --reject (earlier I planned to
send the rejected hunks to stdout which is still what the
documentation describes), so the documentation will be updated
accordingly.
I initially made --reject without --verbose not talk about
anything that applied cleanly, but as the above example usage
shows, I find myself always using --verbose when using --reject.
Without --verbose to remind the user that the patch updated some
other paths cleanly, it is very easy to lose track of the status
of the working tree. So I'll make the final adjustment to make
it imply --verbose before pushing it out.
^ permalink raw reply
* Re: [PATCH] Add git-zip-tree to .gitignore
From: Junio C Hamano @ 2006-08-27 22:57 UTC (permalink / raw)
To: Rene Scharfe; +Cc: git
In-Reply-To: <44F21E0B.9010600@lsrfire.ath.cx>
Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> Junio C Hamano schrieb:
>> Although it was my fault applying it already to "master" without
>> asking this question first, I started to wonder how many more archive
>> format people would want, and if it might make more sense to
>> consolidate git-*-tree into a single frontend with an option to
>> specify the archiver.
>>
>> We would obviously need to keep git-tar-tree as a backward
>> compatibility alias for "git archive-tree -f tar", but doing things
>> that way we do not have to introduce unbounded number of new
>> git-{zip,rar,...}-tree programs.
>
> That thought occurred to me, too. I guess there are not that many more
> interesting archive formats, though. Can we defer adding
> git-archive-tree until a third archive format command appears? I won't
> submit another one, I promise. ;-)
I was more worried about possibly having to do --remote once in
each for tar-tree and zip-tree in git-daemon. The version of
daemon currently in "pu" can allow git-tar-tree to be accessed
from the client via git:// URL.
^ permalink raw reply
* Re: Starting to think about sha-256?
From: Linus Torvalds @ 2006-08-27 22:35 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Krzysztof Halasa, Jeff Garzik, Git Mailing List
In-Reply-To: <Pine.LNX.4.63.0608272341330.28360@wbgn013.biozentrum.uni-wuerzburg.de>
On Mon, 28 Aug 2006, Johannes Schindelin wrote:
> >
> > Modifying git-convert-objects.c to rewrite the regular sha1 into a sha256
> > should be fairly straightforward. It's never been used since the early
> > days (and has limits like a maximum of a million objects etc that can need
> > fixing), but it shouldn't be "fundamentally hard" per se.
>
> But what about signed tags? (This issue has come up before, but never has
> been adressed.)
Signed tags fundamentally have to be re-signed. That's by design: if
somebody could rewrite an archive and signed tags would still be accepted
to have the right signature, that would be a _serious_ sign of a totally
broken security model.
The git security model isn't broken.
> I also thought about supporting hybrid hashes, i.e. that older objects
> still can be hashed with SHA-1. Alas, a simple thought experiment
> demonstrates how silly that idea is: most of the objects will not change
> between two revisions, and they'd have to be rehashed with SHA-256 (or
> whatever we decide upon) anyway, so hybrids would do no good.
Indeed. Hybrids would not only do no good, but they would actually
_actively_ hurt things, because they'd fundamentally break the notion that
the hash being identical means that the object (blob, tree, subtree) is
the same.
So allowing two names for the same object is very fundamentally wrong in
git-speak.
> A better idea would be to increment the repository version, and expect
> SHA-1 for version 1, SHA-256 for version >= 2.
Yes. It would be reasonably painful for users, though (as Krzysztof
correctly points out). Every client would have to convert when a
repository they track is converted.
> Even if the breakthrough really comes to full SHA-1, you still have to add
> _at least_ 20 bytes of gibberish. Which would be harder to spot, but it
> would be spotted.
Yeah, I don't think this is at all critical, especially since git really
on a security level doesn't _depend_ on the hashes being cryptographically
secure. As I explained early on (ie over a year ago, back when the whole
design of git was being discussed), the _security_ of git actually depends
on not cryptographic hashes, but simply on everybody being able to secure
their own _private_ repository.
So the only thing git really _requires_ is a hash that is _unique_ for the
developer (and there we are talking not of an _attacker_, but a benign
participant).
That said, the cryptographic security of SHA-1 is obviously a real bonus.
So I'd be disappointed if SHA-1 can be broken more easily (and I obviously
already argued against using MD5, exactly because generating duplicates of
that is fairly easy). But it's not "fundamentally required" in git per se.
[ The one exception: the "signed tags" security does depend on the hashes
being cryptographically strong. So again, breaking SHA-1 would not mean
that git stops working, but it _would_ potentially mean that if you
don't trust your own _private_ repository, the signed tag may no longer
protect you entirely ]
> This made me think about the use of hashes in git. Why do we need a hash
> here (in no particular order):
>
> 1) integrity checking,
> 2) fast lookup,
> 3) identifying objects (related to (2)),
> 4) trust.
>
> Except for (4), I do not see why SHA-1 -- even if broken -- should not be
> adequate. It is not like somebody found out that all JPGs tend to have
> similar hashes so that collisions are more likely.
Correct. I'm pretty sure we had exactly this discussion around May 2005,
but I'm too lazy to search ;)
Linus
^ permalink raw reply
* Re: [PATCH] Add git-zip-tree to .gitignore
From: Rene Scharfe @ 2006-08-27 22:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlkp9x2b9.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano schrieb:
> Although it was my fault applying it already to "master" without
> asking this question first, I started to wonder how many more archive
> format people would want, and if it might make more sense to
> consolidate git-*-tree into a single frontend with an option to
> specify the archiver.
>
> We would obviously need to keep git-tar-tree as a backward
> compatibility alias for "git archive-tree -f tar", but doing things
> that way we do not have to introduce unbounded number of new
> git-{zip,rar,...}-tree programs.
That thought occurred to me, too. I guess there are not that many more
interesting archive formats, though. Can we defer adding
git-archive-tree until a third archive format command appears? I won't
submit another one, I promise. ;-)
René
^ permalink raw reply
* Re: [PATCH 0/2] gitweb: Get rid of failed experiments ;-)
From: Junio C Hamano @ 2006-08-27 22:22 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <200608272343.14653.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> Remove unused subroutines or subroutines which don't work correctly from
> gitweb.
I'll apply both, thanks. [2/2] had a fuzz at the very end of
the second hunk (your base version do not have -M to diff-tree
parameter anymore) but that was easy to deal with.
Also thanks for the reminder about "typo in git_patchset_body".
^ permalink raw reply
* Re: [PATCH] Add git-zip-tree to .gitignore
From: Junio C Hamano @ 2006-08-27 22:04 UTC (permalink / raw)
To: Rene Scharfe; +Cc: git
In-Reply-To: <44F17FD5.2030000@lsrfire.ath.cx>
Rene Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
>
> diff --git a/.gitignore b/.gitignore
> index 55cd984..58a7c92 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -124,6 +124,7 @@ git-verify-pack
> git-verify-tag
> git-whatchanged
> git-write-tree
> +git-zip-tree
> git-core-*/?*
> gitweb/gitweb.cgi
> test-date
That is sensible.
Although it was my fault applying it already to "master" without
asking this question first, I started to wonder how many more
archive format people would want, and if it might make more
sense to consolidate git-*-tree into a single frontend with an
option to specify the archiver.
We would obviously need to keep git-tar-tree as a backward
compatibility alias for "git archive-tree -f tar", but doing
things that way we do not have to introduce unbounded number of
new git-{zip,rar,...}-tree programs.
^ 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