* [PATCH] tests: Set EDITOR=: and VISUAL=: globally
From: Eric Wong @ 2006-07-11 19:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlkr04i4p.fsf@assigned-by-dhcp.cox.net>
This way we don't have to remember to set it for each test; and
if we forget, we won't cause interactive editors to be spawned
for non-interactive tests.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
Junio C Hamano <junkio@cox.net> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
>
> > I have VISUAL set in my environment, and it caused git-commit to
> > spawn my editor during the test.
>
> I think it would be better to remove "EDITOR=: VISUAL=:"
> settings from annotate-tests.sh, t1400-update-ref.sh and
> t4013-diff-various.sh, and move that to test-lib.sh; there is no
> point overriding them differently in each of these automated
> tests.
I've been under the impression this has already been set, but it turns
it that was only the case in my config.mak on a different machine :)
t/annotate-tests.sh | 2 +-
t/t1400-update-ref.sh | 1 -
t/t4013-diff-various.sh | 2 +-
t/test-lib.sh | 3 +++
4 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 1148b02..b6a2edd 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -94,7 +94,7 @@ test_expect_success \
test_expect_success \
'merge-setup part 4' \
'echo "evil merge." >>file &&
- EDITOR=: VISUAL=: git commit -a --amend'
+ git commit -a --amend'
test_expect_success \
'Two lines blamed on A, one on B, two on B1, one on B2, one on A U Thor' \
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 6a3515d..04fab26 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -190,7 +190,6 @@ test_expect_success \
GIT_COMMITTER_DATE="2005-05-26 23:41" git-commit -F M -a &&
h_OTHER=$(git-rev-parse --verify HEAD) &&
echo FIXED >F &&
- EDITOR=true \
GIT_AUTHOR_DATE="2005-05-26 23:44" \
GIT_COMMITTER_DATE="2005-05-26 23:44" git-commit --amend &&
h_FIXED=$(git-rev-parse --verify HEAD) &&
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 06837d1..36658fb 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -70,7 +70,7 @@ test_expect_success setup '
for i in 1 2; do echo $i; done >>dir/sub &&
git update-index file0 dir/sub &&
- EDITOR=: VISUAL=: git commit --amend &&
+ git commit --amend &&
git show-branch
'
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 05f6e79..b0d7990 100755
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -9,6 +9,8 @@ LC_ALL=C
PAGER=cat
TZ=UTC
export LANG LC_ALL PAGER TZ
+EDITOR=:
+VISUAL=:
unset AUTHOR_DATE
unset AUTHOR_EMAIL
unset AUTHOR_NAME
@@ -30,6 +32,7 @@ unset SHA1_FILE_DIRECTORIES
unset SHA1_FILE_DIRECTORY
export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
+export EDITOR VISUAL
# Each test should start with something like this, after copyright notices:
#
--
1.4.1.g710d
^ permalink raw reply related
* Re: [PATCH 2/3] sha1_file: add the ability to parse objects in "pack file format"
From: Linus Torvalds @ 2006-07-11 18:58 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Carl Baldwin, Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.63.0607112031150.29667@wbgn013.biozentrum.uni-wuerzburg.de>
On Tue, 11 Jul 2006, Johannes Schindelin wrote:
>
> Why not just say
>
> return *hdr ? -1 : bytes;
Hey, whatever works. I rewrote more, and edited some of my changes down
again..
> You might want to add a comment saying "since the type is lowercase in
> ascii format, object_type must be 6 or 7, which is an invalid object
> type." It took me a little to figure that out...
It's not even correct in my version - I check the ASCII header _first_, so
by the time it looks at the binary one, it already knows it's not ascii.
The problematic case is actually the other way around: my
"parse_ascii_sha1_header()" isn't strict enough.
Or, more likely, the parse_sha1_header() function should just be changed
to check the binary format first (and then add your comment about why that
is safe).
> > + bits = 4;
> > + while (!(c & 0x80)) {
> > + if (bits >= 8*sizeof(unsigned long))
> > + return -1;
> > + c = *hdr++;
> > + size += (unsigned long) (c & 0x7f) << bits;
> > + bytes++;
> > + bits += 7;
> > + }
>
> Are you not losing the last byte by putting the "while" _before_ instead
> of _after_ the loop?
No. The very first byte can have the 0x80 end marker, when the size was
between 0..15.
> > int sha1_object_info(const unsigned char *sha1, char *type, unsigned long *sizep)
> > {
> > - int status;
> > + int status, hdrlen;
> > unsigned long mapsize, size;
> > void *map;
> > z_stream stream;
>
> This hunk is unnecessary, right?
Yeah, never mind. That function didn't actually need the hdrlen, it only
cared about the SHA1.
Linus
^ permalink raw reply
* Re: [PATCH 2/3] sha1_file: add the ability to parse objects in "pack file format"
From: Johannes Schindelin @ 2006-07-11 19:20 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Carl Baldwin, Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0607111153170.5623@g5.osdl.org>
Hi,
On Tue, 11 Jul 2006, Linus Torvalds wrote:
> On Tue, 11 Jul 2006, Johannes Schindelin wrote:
>
> > You might want to add a comment saying "since the type is lowercase in
> > ascii format, object_type must be 6 or 7, which is an invalid object
> > type." It took me a little to figure that out...
>
> It's not even correct in my version - I check the ASCII header _first_, so
> by the time it looks at the binary one, it already knows it's not ascii.
Just realized it all by myself...
> Or, more likely, the parse_sha1_header() function should just be changed
> to check the binary format first (and then add your comment about why that
> is safe).
Yes, exactly.
> > > + bits = 4;
> > > + while (!(c & 0x80)) {
> > > + if (bits >= 8*sizeof(unsigned long))
> > > + return -1;
> > > + c = *hdr++;
> > > + size += (unsigned long) (c & 0x7f) << bits;
> > > + bytes++;
> > > + bits += 7;
> > > + }
> >
> > Are you not losing the last byte by putting the "while" _before_ instead
> > of _after_ the loop?
>
> No. The very first byte can have the 0x80 end marker, when the size was
> between 0..15.
Yes, I understand now. I was a little confused by the way it is written...
Thanks for the clarification,
Dscho
^ permalink raw reply
* Re: Re : 2 questions on git-send-email usage
From: Junio C Hamano @ 2006-07-11 19:22 UTC (permalink / raw)
To: Franck; +Cc: git
In-Reply-To: <44B37893.5090501@innova-card.com>
Franck Bui-Huu <vagabon.xyz@gmail.com> writes:
> Maybe that patch does what you want.
>
> -- >8 --
>
> Subject: [PATCH] Add a newline before appending "Signed-off-by:"
>
> It looks nicer.
>
> Signed-off-by: Franck Bui-Huu <vagabon.xyz@gmail.com>
Haven't checked the code around the patch yet, but does it work
when the original commit log message ends with a blank line and
existing signed-off-by lines by other people? You do not want
an extra blank lines there.
^ permalink raw reply
* Re: [PATCH] gitweb: Include a site name in page titles
From: Junio C Hamano @ 2006-07-11 19:48 UTC (permalink / raw)
To: Alp Toker; +Cc: git, Martin Langhoff
In-Reply-To: <46a038f90607110448p40fd8984ke3e15639cf5ecf46@mail.gmail.com>
"Martin Langhoff" <martin.langhoff@gmail.com> writes:
>> +# name of your site or organization to appear in page titles
>> +our $site_name = "Untitled";
>
> I generally agree, but as a default, $ENV{SERVER_NAME} should be better.
Sounds like a good suggestion.
^ permalink raw reply
* Re: [PATCH 2/3] sha1_file: add the ability to parse objects in "pack file format"
From: Linus Torvalds @ 2006-07-11 19:48 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Carl Baldwin, Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.63.0607112116270.29667@wbgn013.biozentrum.uni-wuerzburg.de>
On Tue, 11 Jul 2006, Johannes Schindelin wrote:
>
> > Or, more likely, the parse_sha1_header() function should just be changed
> > to check the binary format first (and then add your comment about why that
> > is safe).
>
> Yes, exactly.
Here's a newer verson of [2/3], with these issues fixed. It actually fixes
things twice: (a) by parsing the binary version first (which makes sense
for a totally independent reason - if that is going to be the "default"
version in the long run, we should just test it first anyway) and (b) by
making the ASCII version parser stricter too.
This did, btw, also fix the test failure, so the fact that the ASCII
header parser wasn't careful enough was actually a problem in real life.
So please throw away the old version.
Linus
---
From: Linus Torvalds <torvalds@osdl.org>
Subject: [PATCH 2/3] sha1_file: add the ability to parse objects in "pack file format"
The pack-file format is slightly different from the traditional git
object format, in that it has a much denser binary header encoding.
The traditional format uses an ASCII string with type and length
information, which is somewhat wasteful.
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
sha1_file.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 files changed, 82 insertions(+), 11 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 8734d50..15ccf5e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -697,9 +697,9 @@ static int unpack_sha1_header(z_stream *
return inflate(stream, 0);
}
-static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size)
+static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size, unsigned int hdrlen)
{
- int bytes = strlen(buffer) + 1;
+ int bytes = hdrlen;
unsigned char *buf = xmalloc(1+size);
memcpy(buf, (char *) buffer + bytes, stream->total_out - bytes);
@@ -720,25 +720,40 @@ static void *unpack_sha1_rest(z_stream *
* too permissive for what we want to check. So do an anal
* object header parse by hand.
*/
-static int parse_sha1_header(char *hdr, char *type, unsigned long *sizep)
+static int parse_ascii_sha1_header(char *hdr, char *type, unsigned long *sizep)
{
- int i;
+ int bytes = 0;
unsigned long size;
/*
* The type can be at most ten bytes (including the
* terminating '\0' that we add), and is followed by
* a space.
+ *
+ * We want at least three characters, and they should
+ * all be normal lower-case letters.
*/
- i = 10;
for (;;) {
- char c = *hdr++;
+ unsigned char c = *hdr++;
+ bytes++;
if (c == ' ')
break;
- if (!--i)
+ if (bytes >= 10)
return -1;
*type++ = c;
+
+ /*
+ * The high nybble must be 6 of 7, see
+ * parse_binary_header(). This covers
+ * all ASCII lowercase characters.
+ */
+ if (c < 0x60 || c > 0x7f)
+ return -1;
}
+
+ /* Minimum three letters and the space */
+ if (bytes < 4)
+ return -1;
*type = 0;
/*
@@ -746,6 +761,7 @@ static int parse_sha1_header(char *hdr,
* decimal format (ie "010" is not valid).
*/
size = *hdr++ - '0';
+ bytes++;
if (size > 9)
return -1;
if (size) {
@@ -754,6 +770,7 @@ static int parse_sha1_header(char *hdr,
if (c > 9)
break;
hdr++;
+ bytes++;
size = size * 10 + c;
}
}
@@ -762,20 +779,74 @@ static int parse_sha1_header(char *hdr,
/*
* The length must be followed by a zero byte
*/
- return *hdr ? -1 : 0;
+ bytes++;
+ return *hdr ? -1 : bytes;
+}
+
+/*
+ * We never confuse a binary header with an old ASCII one,
+ * because the ASCII one will always start with a lower-case
+ * letter, meaning that the first byte will be of the form
+ * 0x6? or 0x7?.
+ *
+ * That in turn would be parsed as object type 6 or 7, neither
+ * of which is valid for a unpacked object (object type 7 is
+ * a delta, and can only exist in a pack-file, while object type
+ * 6 is invalid).
+ */
+static int parse_binary_sha1_header(char *hdr, char *type, unsigned long *sizep)
+{
+ unsigned char c;
+ int bytes = 1;
+ unsigned long size;
+ unsigned object_type, bits;
+ static const char *typename[8] = {
+ NULL, /* OBJ_EXT */
+ "commit", "tree", "blob", "tag",
+ NULL, NULL, NULL
+ };
+
+ c = *hdr++;
+ object_type = (c >> 4) & 7;
+ if (!typename[object_type])
+ return -1;
+ strcpy(type, typename[object_type]);
+ size = c & 15;
+ bits = 4;
+ while (!(c & 0x80)) {
+ if (bits >= 8*sizeof(unsigned long))
+ return -1;
+ c = *hdr++;
+ size += (unsigned long) (c & 0x7f) << bits;
+ bytes++;
+ bits += 7;
+ }
+ *sizep = size;
+ return bytes;
+}
+
+static int parse_sha1_header(char *hdr, char *type, unsigned long *sizep)
+{
+ int retval = parse_binary_sha1_header(hdr, type, sizep);
+ if (retval < 0)
+ retval = parse_ascii_sha1_header(hdr, type, sizep);
+ return retval;
}
void * unpack_sha1_file(void *map, unsigned long mapsize, char *type, unsigned long *size)
{
- int ret;
+ int ret, hdrlen;
z_stream stream;
char hdr[8192];
ret = unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr));
- if (ret < Z_OK || parse_sha1_header(hdr, type, size) < 0)
+ if (ret < Z_OK)
+ return NULL;
+ hdrlen = parse_sha1_header(hdr, type, size);
+ if (hdrlen < 0)
return NULL;
- return unpack_sha1_rest(&stream, hdr, *size);
+ return unpack_sha1_rest(&stream, hdr, *size, hdrlen);
}
/* forward declaration for a mutually recursive function */
^ permalink raw reply related
* Re: [PATCH] gitweb: Use the git binary in the search path by default
From: Junio C Hamano @ 2006-07-11 19:56 UTC (permalink / raw)
To: Alp Toker; +Cc: git
In-Reply-To: <11526131791902-git-send-email-alp@atoker.com>
Alp Toker <alp@atoker.com> writes:
> Introduce a sensible default for the location of the git binary used by
> gitweb. This means one less option to configure when deploying gitweb if
> git is in the search path.
While I think the part of the change to make things go through
the single "git" wrapper is a good idea, the comment to "our
$GIT" that says "absolute path is optional" makes this change
more like "assume PATH your webserver process uses is sensible",
not "introduce a sensible default".
So I would prefer to do that part like this:
-# location of the git-core binaries
-our $gitbin = "/usr/bin";
+# core git wrapper -- if your webserver runs with a sensible PATH
+# you can just say "git" without using absolute pathname here.
+our $GIT = "/usr/bin/git";
^ permalink raw reply
* [PATCH] gitweb: Include a site name in page titles
From: Alp Toker @ 2006-07-11 20:10 UTC (permalink / raw)
To: git; +Cc: junkio, martin.langhoff
Tip Of The Day:
<title>: the most important element of a quality Web page.
This helps users tell one 'git' bookmark apart from the other in their
browser and improves the indexing of gitweb sites in Web search engines.
Defaults to the HTTP SERVER_NAME.
Signed-off-by: Alp Toker <alp@atoker.com>
---
gitweb/gitweb.cgi | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.cgi b/gitweb/gitweb.cgi
index 2e87de4..be0a01d 100755
--- a/gitweb/gitweb.cgi
+++ b/gitweb/gitweb.cgi
@@ -46,6 +46,10 @@ if (! -d $git_temp) {
# target of the home link on top of all pages
our $home_link = $my_uri;
+# name of your site or organization to appear in page titles
+# replace this with something more descriptive for clearer bookmarks
+our $site_name = $ENV{'SERVER_NAME'} || "Untitled";
+
# html text to include at home page
our $home_text = "indextext.html";
@@ -280,7 +284,7 @@ sub git_header_html {
my $status = shift || "200 OK";
my $expires = shift;
- my $title = "git";
+ my $title = "$site_name git";
if (defined $project) {
$title .= " - $project";
if (defined $action) {
@@ -1760,7 +1764,7 @@ sub git_opml {
print "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n".
"<opml version=\"1.0\">\n".
"<head>".
- " <title>Git OPML Export</title>\n".
+ " <title>$site_name Git OPML Export</title>\n".
"</head>\n".
"<body>\n".
"<outline text=\"git RSS feeds\">\n";
--
1.4.1.g97c7-dirty
^ permalink raw reply related
* [PATCH] git-fetch: fix a bashism (==)
From: Eric Wong @ 2006-07-11 21:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Eric Wong
From: Eric Wong <normalperson@untitled.(none)>
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
git-fetch.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-fetch.sh b/git-fetch.sh
index c0d256c..ff17699 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -80,7 +80,7 @@ rref=
rsync_slurped_objects=
rloga="$rloga $remote_nick"
-test "$remote_nick" == "$remote" || rloga="$rloga $remote"
+test "$remote_nick" = "$remote" || rloga="$rloga $remote"
if test "" = "$append"
then
--
1.4.1.g2f48
^ permalink raw reply related
* Re: [PATCH] git-fetch: fix a bashism (==)
From: Eric Wong @ 2006-07-11 21:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <11526519991197-git-send-email-normalperson@yhbt.net>
Eric Wong <normalperson@yhbt.net> wrote:
> From: Eric Wong <normalperson@untitled.(none)>
Weird, I didn't have my email address set correctly (yet another
different repo from my usual one) and send-email added this line to it.
I'll look into fixing it later today/tonight.
--
Eric Wong
^ permalink raw reply
* Re: [PATCH 2/3] sha1_file: add the ability to parse objects in "pack file format"
From: Johannes Schindelin @ 2006-07-11 21:25 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0607111241460.5623@g5.osdl.org>
Hi,
On Tue, 11 Jul 2006, Linus Torvalds wrote:
> On Tue, 11 Jul 2006, Johannes Schindelin wrote:
> >
> > > Or, more likely, the parse_sha1_header() function should just be changed
> > > to check the binary format first (and then add your comment about why that
> > > is safe).
> >
> > Yes, exactly.
>
> Here's a newer verson of [2/3], with these issues fixed. It actually fixes
> things twice: (a) by parsing the binary version first (which makes sense
> for a totally independent reason - if that is going to be the "default"
> version in the long run, we should just test it first anyway) and (b) by
> making the ASCII version parser stricter too.
Melikey.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 2/3] sha1_file: add the ability to parse objects in "pack file format"
From: sf @ 2006-07-11 21:24 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.64.0607111010320.5623@g5.osdl.org>
Linus Torvalds wrote:
> The pack-file format is slightly different from the traditional git
> object format, in that it has a much denser binary header encoding.
>
> The traditional format uses an ASCII string with type and length
> information, which is somewhat wasteful.
And in the traditional format type and length are compressed whereas in
the pack-file format they are not.
> This should probably be applied to the main tree asap if we think
> this is at all a worthwhile exercise. But somebody should verify that I
> got the format right first!
Sorry but see above.
Regards
Stephan
^ permalink raw reply
* Re: [RFC]: Pack-file object format for individual objects (Was: Revisiting large binary files issue.)
From: sf @ 2006-07-11 21:45 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.64.0607111053270.5623@g5.osdl.org>
Linus Torvalds wrote:
...
> On Tue, 11 Jul 2006, sf wrote:
...
>> Step 1. When reading individual objects from disk check the first nibble
>> and decode accordingly (see above).
>
> Check more than that, but yes, this should be tightened up in my
> series.
Just look at the first byte of the object file _without doing any
decompression_. It is 0x78 _if and only if_ the object file is in the
traditional format.
>> Step 3. Remove code for (old) individual object disk format.
>
> Well, I'm not sure how necessary that even is. We actually do have to
> generate the old header regardless, if for no other reason than the fact
> that we generate the SHA1 names based on it (even if we then write a
> new-style dense binary header to disk and discard the ASCII header).
>
> Having it there means that you can always just get a new version of git,
> and never worry about how old the archive you're working with is.
>
> (And then doing a "git repack -a -d" will make any archive also work with
> an old-style git, since the pack-file format didn't change, and a "git
> repack" thus ends up always creating something that is readable by
> anybody, including old clients).
Agreed.
Regards
Stephan
^ permalink raw reply
* Re: [PATCH 2/3] sha1_file: add the ability to parse objects in "pack file format"
From: Junio C Hamano @ 2006-07-11 21:47 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Johannes Schindelin, Carl Baldwin, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0607111241460.5623@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> Here's a newer verson of [2/3], with these issues fixed. It actually fixes
> things twice: (a) by parsing the binary version first (which makes sense
> for a totally independent reason - if that is going to be the "default"
> version in the long run, we should just test it first anyway) and (b) by
> making the ASCII version parser stricter too.
Wait a minute.
read-sha1-file maps sha1-file-internal (for unpacked one), and then
calls unpack-sha1-file.
unpack-sha1-file calls unpack-sha1-header to start inflation,
lets parse-sha1-header to read the header in the inflated
buffer, and calls unpack-sha1-rest to inflate the rest.
But in packs, we have binary header not deflated, followed by
deflated payload. If we want to copy things from loose objects
into pack without changing the packfile format, this change
would not help, I suspect.
At least, your updated unpack_sha1_file() needs to check for
binary header first (starting from "map"), and if that starts
with binary header, start inflating after the header to extract
the payload. Otherwise you would do the traditional.
^ permalink raw reply
* Re: [PATCH 2/3] sha1_file: add the ability to parse objects in "pack file format"
From: Linus Torvalds @ 2006-07-11 22:09 UTC (permalink / raw)
To: sf; +Cc: git
In-Reply-To: <44B4172B.3070503@stephan-feder.de>
On Tue, 11 Jul 2006, sf wrote:
>
> And in the traditional format type and length are compressed whereas in
> the pack-file format they are not.
Ahh. Yes.
> > This should probably be applied to the main tree asap if we think
> > this is at all a worthwhile exercise. But somebody should verify that I
> > got the format right first!
>
> Sorry but see above.
Good catch, thanks indeed.
Doing that for unpacked objects would in fact make a lot of things much
simpler, so it would be good to do. The _bad_ part is that this also makes
it a lot harder to see the difference between a "binary header" and a
"compressed ASCII header". The two are not "obviously different" any more.
The common byte sequence for a compressed stream is
78 9c ...
where the first byte is the CMF byte (compression info and method).
But it's not the only possible such sequence according to the zlib format.
(The 16-bit hex number in MSB format, ie 0x789c above, is defined to have
a built-in checksum, so that it must be a multiple of 31 according to the
standard: 0x789c = 996 * 31).
So if we have a uncompressed header, we'd need to add a separate 2-byte
fingerprint to it _before_ the real header that isn't divisible by 31, and
use that as the thing to test.
Ho humm. I'll see what I can come up with.
Linus
^ permalink raw reply
* Re: [RFC]: Pack-file object format for individual objects (Was: Revisiting large binary files issue.)
From: Linus Torvalds @ 2006-07-11 22:17 UTC (permalink / raw)
To: sf; +Cc: git
In-Reply-To: <44B41BFD.8010808@stephan-feder.de>
On Tue, 11 Jul 2006, sf wrote:
>
> Just look at the first byte of the object file _without doing any
> decompression_. It is 0x78 _if and only if_ the object file is in the
> traditional format.
0x78 isn't the only valid flag for a zlib stream, as far as I can tell.
It may be the only one _in_practice_, of course, but the zlib standard
defines the first byte as
- for low bits: CM (compression method):
"This identifies the compression method used in the file. CM = 8
denotes the "deflate" compression method with a window size up
to 32K. This is the method used by gzip and PNG (see
references [1] and [2] in Chapter 3, below, for the reference
documents). CM = 15 is reserved. It might be used in a future
version of this specification to indicate the presence of an
extra field before the compressed data."
- four high bits are CINFO:
"For CM = 8, CINFO is the base-2 logarithm of the LZ77 window
size, minus eight (CINFO=7 indicates a 32K window size). Values
of CINFO above 7 are not allowed in this version of the
specification. CINFO is not defined in this specification for
CM not equal to 8."
so 0x78 means "deflate with 32kB window size", but I don't see anything
guaranteeing that we might not see something else for an object that
cannot be compressed, for example.
Anyway, the good news is that _if_ 0x78 is indeed the only possible value,
then it is also an illegal value for an unpacked object in pack-file
format (type=7 being OBJ_DELTA) and we wouldn't need any other flag for
this.
I just don't know if it's the only possible one..
Linus
^ permalink raw reply
* git-daemon problem
From: Matthias Lederhofer @ 2006-07-11 22:24 UTC (permalink / raw)
To: git
A few weeks ago upgrading from 1.3.x to 1.4.1 I had a problem with
git-daemon. I started git-daemon on a terminal but did not redirect
stdin/stdout/stderr to /dev/null (actually using daemon(8) on freebsd
without -f but just disowning the process and closing the terminal
works fine too, nothing freebsd/daemon(8) specific). After closing
the terminal I was not able to use the git-daemon anymore with some
versions of the git. So now I took some time and tried to find what
was the reason for that.
It seems to be related to the client version too (git without version
appendix is the current next (028cfcba78c3e4).
583b7ea31b7c16~1 (last good):
$ git clone git://host:9419/foo
$ git1.3.2 clone git://host:9419/foo.git
(cloned successfully, both no output)
583b7ea31b7c16 (first bad):
$ git clone git://host:9420/foo
Generating pack...
Done counting 6 objects.
Deltifying 6 objects.
100% (6/6) done
Total 6, written 6 (delta 0), reused 0 (delta 0)
$ git1.3.2 clone git://host:9420/foo.git
fatal: cannot mmap packfile '/somewhere/foo/.git/objects/pack/tmp-VX82qz': Invalid argument
error: git-fetch-pack: unable to read from git-index-pack
error: git-index-pack died with error code 128
fetch-pack from 'git://host:9420/foo.git' failed.
[1] 13267 exit 1 git1.3.2 clone git://host:9420/foo.git
(/somewhere is the cwd on the client)
I tried to find which part of the patch caused the problem and came
out with the patch below. With this I can clone with git1.3.2 again
but then git 1.4.x does not show any statistics about packing, its
just a starting point to look at. Perhaps someone has an idea why
this happens. I've got to sleep now :)
---
upload-pack.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c
index 7b86f69..94f0d85 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -249,7 +249,7 @@ static void create_pack_file(void)
sz = read(pe_pipe[0], progress,
sizeof(progress));
if (0 < sz)
- send_client_data(2, progress, sz);
+ write(2, progress, sz);
else if (sz == 0) {
close(pe_pipe[0]);
pe_pipe[0] = -1;
^ permalink raw reply related
* Re: [RFC]: Pack-file object format for individual objects (Was: Revisiting large binary files issue.)
From: Linus Torvalds @ 2006-07-11 22:26 UTC (permalink / raw)
To: sf; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0607111512420.5623@g5.osdl.org>
On Tue, 11 Jul 2006, Linus Torvalds wrote:
>
> - for low bits: CM (compression method):
>
> "This identifies the compression method used in the file. CM = 8
> denotes the "deflate" compression method with a window size up
> to 32K. This is the method used by gzip and PNG (see
> references [1] and [2] in Chapter 3, below, for the reference
> documents). CM = 15 is reserved. It might be used in a future
> version of this specification to indicate the presence of an
> extra field before the compressed data."
>
> - four high bits are CINFO:
>
> "For CM = 8, CINFO is the base-2 logarithm of the LZ77 window
> size, minus eight (CINFO=7 indicates a 32K window size). Values
> of CINFO above 7 are not allowed in this version of the
> specification. CINFO is not defined in this specification for
> CM not equal to 8."
>
> so 0x78 means "deflate with 32kB window size", but I don't see anything
> guaranteeing that we might not see something else for an object that
> cannot be compressed, for example.
Ahh. Looking at the zlib sources, I see
/* Write the zlib header */
if (s->status == INIT_STATE) {
uInt header = (Z_DEFLATED + ((s->w_bits-8)<<4)) << 8;
uInt level_flags = (s->level-1) >> 1;
if (level_flags > 3) level_flags = 3;
header |= (level_flags << 6);
if (s->strstart != 0) header |= PRESET_DICT;
header += 31 - (header % 31);
s->status = BUSY_STATE;
putShortMSB(s, header);
(which is that first 16-bit word, MSB first). So we'll always have the
Z-DEFLATED (8) there in the low four bits, but the high nybble will be
"s->w_bits-8" where w_bits comes from windowBits, and I think we can
depend on it beign 15:
"The windowBits parameter is the base two logarithm of the window size
(the size of the history buffer). It should be in the range 8..15 for this
version of the library. Larger values of this parameter result in better
compression at the expense of memory usage. The default value is 15 if
deflateInit is used instead."
so since we use deflateInit(), we know the window will be 15.
So I guess we _can_ depend on the first byte being 0x78 for our use.
Goodie.
Linus
^ permalink raw reply
* Re: [PATCH 2/3] sha1_file: add the ability to parse objects in "pack file format"
From: sf @ 2006-07-11 22:25 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.64.0607111449190.5623@g5.osdl.org>
Linus Torvalds wrote:
...
> The common byte sequence for a compressed stream is
>
> 78 9c ...
>
> where the first byte is the CMF byte (compression info and method).
>
> But it's not the only possible such sequence according to the zlib format.
No, but 0x78 is the only first byte ever produced by git; the files are
always deflated (second nibble is 8) with window size 32K (first nibble
is 7).
^ permalink raw reply
* Re: [PATCH] Typo fix
From: Pavel Roskin @ 2006-07-11 22:37 UTC (permalink / raw)
To: Alp Toker; +Cc: git
In-Reply-To: <11526131782190-git-send-email-alp@atoker.com>
Hello Alp,
On Tue, 2006-07-11 at 11:19 +0100, Alp Toker wrote:
> Signed-off-by: Alp Toker <alp@atoker.com>
> ---
> -wrote you about that phantastic commit 33db5f4d9027a10e477ccf054b2c1ab94f74c85a.
> +wrote you about that fantastic commit 33db5f4d9027a10e477ccf054b2c1ab94f74c85a.
You are overreacting. I checked all sources for typos and of course I
saw "phantastic" but I assumed it was intentional. We don't want the
documentation to be devoid of humor. Just in case, British spelling
like "behaviour" was also preserved out of respect to the authors.
--
Regards,
Pavel Roskin
^ permalink raw reply
* Re: [PATCH 2/3] sha1_file: add the ability to parse objects in "pack file format"
From: Junio C Hamano @ 2006-07-11 23:03 UTC (permalink / raw)
To: git
In-Reply-To: <Pine.LNX.4.64.0607111449190.5623@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> So if we have a uncompressed header, we'd need to add a separate 2-byte
> fingerprint to it _before_ the real header that isn't divisible by 31, and
> use that as the thing to test.
>
> Ho humm. I'll see what I can come up with.
I do not like to rely too heavily on what zlib compression's
beginning of stream looks like.
I think the new format can be deflated new header (fully
synched) followed by deflated payload.
So the sequence unpack-sha1-header followed by parse-sha1-header
would notice we are dealing with new format and reinitialize the
deflator at the point where the header deflator left off.
Wouldn't that work?
^ permalink raw reply
* Re: git-daemon problem
From: Junio C Hamano @ 2006-07-11 23:04 UTC (permalink / raw)
To: git
In-Reply-To: <E1G0QeX-0003hG-0I@moooo.ath.cx>
Matthias Lederhofer <matled@gmx.net> writes:
> A few weeks ago upgrading from 1.3.x to 1.4.1 I had a problem with
> git-daemon. I started git-daemon on a terminal but did not redirect
> stdin/stdout/stderr to /dev/null (actually using daemon(8) on freebsd
> without -f but just disowning the process and closing the terminal
> works fine too, nothing freebsd/daemon(8) specific). After closing
> the terminal I was not able to use the git-daemon anymore with some
> versions of the git. So now I took some time and tried to find what
> was the reason for that.
>
> It seems to be related to the client version too (git without version
> appendix is the current next (028cfcba78c3e4).
>
> 583b7ea31b7c16~1 (last good):
> $ git clone git://host:9419/foo
> $ git1.3.2 clone git://host:9419/foo.git
> (cloned successfully, both no output)
>
> 583b7ea31b7c16 (first bad):
> $ git clone git://host:9420/foo
> Generating pack...
> Done counting 6 objects.
> Deltifying 6 objects.
> 100% (6/6) done
> Total 6, written 6 (delta 0), reused 0 (delta 0)
> $ git1.3.2 clone git://host:9420/foo.git
> fatal: cannot mmap packfile '/somewhere/foo/.git/objects/pack/tmp-VX82qz': Invalid argument
> error: git-fetch-pack: unable to read from git-index-pack
> error: git-index-pack died with error code 128
> fetch-pack from 'git://host:9420/foo.git' failed.
> [1] 13267 exit 1 git1.3.2 clone git://host:9420/foo.git
> (/somewhere is the cwd on the client)
>
> I tried to find which part of the patch caused the problem and came
> out with the patch below. With this I can clone with git1.3.2 again
> but then git 1.4.x does not show any statistics about packing, its
> just a starting point to look at. Perhaps someone has an idea why
> this happens. I've got to sleep now :)
>
> ---
> upload-pack.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/upload-pack.c b/upload-pack.c
> index 7b86f69..94f0d85 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -249,7 +249,7 @@ static void create_pack_file(void)
> sz = read(pe_pipe[0], progress,
> sizeof(progress));
> if (0 < sz)
> - send_client_data(2, progress, sz);
> + write(2, progress, sz);
> else if (sz == 0) {
> close(pe_pipe[0]);
> pe_pipe[0] = -1;
This breaks the newer clients that knows how to do side-band
doesn't it?
^ permalink raw reply
* Re: git-daemon problem
From: Junio C Hamano @ 2006-07-11 23:32 UTC (permalink / raw)
To: Matthias Lederhofer; +Cc: git
In-Reply-To: <E1G0QeX-0003hG-0I@moooo.ath.cx>
Matthias Lederhofer <matled@gmx.net> writes:
> A few weeks ago upgrading from 1.3.x to 1.4.1 I had a problem with
> git-daemon. I started git-daemon on a terminal but did not redirect
> stdin/stdout/stderr to /dev/null (actually using daemon(8) on freebsd
> without -f but just disowning the process and closing the terminal
> works fine too, nothing freebsd/daemon(8) specific).
This is because the server side closes fd #2 in such a setup,
and we still wrote using safe_write() into it. Thanks for
spotting.
Would this replacement patch help?
diff --git a/upload-pack.c b/upload-pack.c
index b18eb9b..44038d3 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -51,6 +51,13 @@ static ssize_t send_client_data(int fd,
if (fd == 3)
/* emergency quit */
fd = 2;
+ if (fd == 2) {
+ /* people sometomes close fd 2 on the server
+ * side -- making safe_write() to barf.
+ */
+ write(2, data, sz);
+ return sz;
+ }
return safe_write(fd, data, sz);
}
p = data;
^ permalink raw reply related
* Re: [PATCH] Install built-ins as symlinks
From: Alex Riesen @ 2006-07-12 0:01 UTC (permalink / raw)
To: Petr Baudis; +Cc: Johannes Schindelin, Alp Toker, git
In-Reply-To: <20060711184838.GC13776@pasky.or.cz>
Petr Baudis, Tue, Jul 11, 2006 20:48:38 +0200:
> > And -- just maybe -- I _did_ mention a single reason to keep hard links:
> > It works now. So why change it?
>
> The original patch mentioned why hardlinks are bad, so if you argue
> that the raised points are moot, you should give some substance to your
> argument.
Windows will add that substance, even if it is a dirty one: symlinks
don't work there properly (cygwin makes that very stupid .lnk file).
^ permalink raw reply
* Re: [PATCH 2/3] sha1_file: add the ability to parse objects in "pack file format"
From: Linus Torvalds @ 2006-07-12 0:03 UTC (permalink / raw)
To: Junio C Hamano, sf, Johannes Schindelin; +Cc: Git Mailing List
In-Reply-To: <7vejwr3ftl.fsf@assigned-by-dhcp.cox.net>
On Tue, 11 Jul 2006, Junio C Hamano wrote:
>
> I do not like to rely too heavily on what zlib compression's
> beginning of stream looks like.
Well, I normally would agree with you if it was a "oh, all our zlib
objects seem to start with 0x78" thing, but after having dug into both the
zlib standard (which is actually an RFC, not just some random thing), and
looked at the sources, it's definitely the case that the "0x78" byte isn't
just an implementation detail.
It's actually really part of the specs, and not just happenstance.
> I think the new format can be deflated new header (fully
> synched) followed by deflated payload.
That would work, but on the other hand, one of the advantages of doing the
new format would be that the "check size and type" code wouldn't even need
to call into the zlib code.
Anyway, I think this following patch replaces the old 2/3 and 3/3 (it
still depends on the original [1/3] cleanup.
(It also renames and reverses the meaning of the config file option: it's
now "[core] LegacyHeaders = true" for using legacy headers.)
Not heavily tested, but seems ok.
sf? Dscho? Can you check this thing out?
Linus
----
Documentation/config.txt | 6 +++
cache.h | 1
config.c | 5 ++
environment.c | 1
sha1_file.c | 106 +++++++++++++++++++++++++++++++++++++++++++---
5 files changed, 111 insertions(+), 8 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0b434c1..9780c89 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -97,6 +97,12 @@ core.compression::
compression, and 1..9 are various speed/size tradeoffs, 9 being
slowest.
+core.legacyheaders::
+ A boolean which enables the legacy object header format in case
+ you want to interoperate with old clients accessing the object
+ database directly (where the "http://" and "rsync://" protocols
+ count as direct access).
+
alias.*::
Command aliases for the gitlink:git[1] command wrapper - e.g.
after defining "alias.last = cat-file commit HEAD", the invocation
diff --git a/cache.h b/cache.h
index d433d46..eee5fc9 100644
--- a/cache.h
+++ b/cache.h
@@ -176,6 +176,7 @@ extern int commit_lock_file(struct lock_
extern void rollback_lock_file(struct lock_file *);
/* Environment bits from configuration mechanism */
+extern int use_legacy_headers;
extern int trust_executable_bit;
extern int assume_unchanged;
extern int prefer_symlink_refs;
diff --git a/config.c b/config.c
index 8445f7d..0ac6aeb 100644
--- a/config.c
+++ b/config.c
@@ -279,6 +279,11 @@ int git_default_config(const char *var,
return 0;
}
+ if (!strcmp(var, "core.legacyheaders")) {
+ use_legacy_headers = git_config_bool(var, value);
+ return 0;
+ }
+
if (!strcmp(var, "core.compression")) {
int level = git_config_int(var, value);
if (level == -1)
diff --git a/environment.c b/environment.c
index 97d42b1..d80a39a 100644
--- a/environment.c
+++ b/environment.c
@@ -11,6 +11,7 @@ #include "cache.h"
char git_default_email[MAX_GITNAME];
char git_default_name[MAX_GITNAME];
+int use_legacy_headers = 0;
int trust_executable_bit = 1;
int assume_unchanged = 0;
int prefer_symlink_refs = 0;
diff --git a/sha1_file.c b/sha1_file.c
index 8734d50..475b23d 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -684,26 +684,74 @@ static void *map_sha1_file_internal(cons
return map;
}
-static int unpack_sha1_header(z_stream *stream, void *map, unsigned long mapsize, void *buffer, unsigned long size)
+static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz)
{
+ unsigned char c;
+ unsigned int word, bits;
+ unsigned long size;
+ static const char *typename[8] = {
+ NULL, /* OBJ_EXT */
+ "commit", "tree", "blob", "tag",
+ NULL, NULL, NULL
+ };
+ const char *type;
+
/* Get the data stream */
memset(stream, 0, sizeof(*stream));
stream->next_in = map;
stream->avail_in = mapsize;
stream->next_out = buffer;
- stream->avail_out = size;
+ stream->avail_out = bufsiz;
+
+ /*
+ * Is it a zlib-compressed buffer? If so, the first byte
+ * must be 0x78 (15-bit window size, deflated), and the
+ * first 16-bit word is evenly divisible by 31
+ */
+ word = (map[0] << 8) + map[1];
+ if (map[0] == 0x78 && !(word % 31)) {
+ inflateInit(stream);
+ return inflate(stream, 0);
+ }
+
+ c = *map++;
+ mapsize--;
+ type = typename[(c >> 4) & 7];
+ if (!type)
+ return -1;
+
+ bits = 4;
+ size = c & 0xf;
+ while (!(c & 0x80)) {
+ if (bits >= 8*sizeof(long))
+ return -1;
+ c = *map++;
+ size += (c & 0x7f) << bits;
+ bits += 7;
+ mapsize--;
+ }
+ /* Set up the stream for the rest.. */
+ stream->next_in = map;
+ stream->avail_in = mapsize;
inflateInit(stream);
- return inflate(stream, 0);
+
+ /* And generate the fake traditional header */
+ stream->total_out = 1 + snprintf(buffer, bufsiz, "%s %lu", type, size);
+ return 0;
}
static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size)
{
int bytes = strlen(buffer) + 1;
unsigned char *buf = xmalloc(1+size);
+ unsigned long n;
- memcpy(buf, (char *) buffer + bytes, stream->total_out - bytes);
- bytes = stream->total_out - bytes;
+ n = stream->total_out - bytes;
+ if (n > size)
+ n = size;
+ memcpy(buf, (char *) buffer + bytes, n);
+ bytes = n;
if (bytes < size) {
stream->next_out = buf + bytes;
stream->avail_out = size - bytes;
@@ -1414,6 +1462,49 @@ static int write_buffer(int fd, const vo
return 0;
}
+static int write_binary_header(unsigned char *hdr, enum object_type type, unsigned long len)
+{
+ int hdr_len;
+ unsigned char c;
+
+ c = (type << 4) | (len & 15);
+ len >>= 4;
+ hdr_len = 1;
+ while (len) {
+ *hdr++ = c;
+ hdr_len++;
+ c = (len & 0x7f);
+ len >>= 7;
+ }
+ *hdr = c | 0x80;
+ return hdr_len;
+}
+
+static void setup_object_header(z_stream *stream, const char *type, unsigned long len)
+{
+ int obj_type, hdr;
+
+ if (use_legacy_headers) {
+ while (deflate(stream, 0) == Z_OK)
+ /* nothing */;
+ return;
+ }
+ if (!strcmp(type, blob_type))
+ obj_type = OBJ_BLOB;
+ else if (!strcmp(type, tree_type))
+ obj_type = OBJ_TREE;
+ else if (!strcmp(type, commit_type))
+ obj_type = OBJ_COMMIT;
+ else if (!strcmp(type, tag_type))
+ obj_type = OBJ_TAG;
+ else
+ die("trying to generate bogus object of type '%s'", type);
+ hdr = write_binary_header(stream->next_out, obj_type, len);
+ stream->total_out = hdr;
+ stream->next_out += hdr;
+ stream->avail_out -= hdr;
+}
+
int write_sha1_file(void *buf, unsigned long len, const char *type, unsigned char *returnsha1)
{
int size;
@@ -1459,7 +1550,7 @@ int write_sha1_file(void *buf, unsigned
/* Set it up */
memset(&stream, 0, sizeof(stream));
deflateInit(&stream, zlib_compression_level);
- size = deflateBound(&stream, len+hdrlen);
+ size = 8 + deflateBound(&stream, len+hdrlen);
compressed = xmalloc(size);
/* Compress it */
@@ -1469,8 +1560,7 @@ int write_sha1_file(void *buf, unsigned
/* First header.. */
stream.next_in = hdr;
stream.avail_in = hdrlen;
- while (deflate(&stream, 0) == Z_OK)
- /* nothing */;
+ setup_object_header(&stream, type, len);
/* Then the data itself.. */
stream.next_in = buf;
^ permalink raw reply related
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