* Re: [PATCH] Documentation/git-commit: rewrite to make it more end-user friendly.
From: Junio C Hamano @ 2006-12-10 23:05 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git, J. Bruce Fields
In-Reply-To: <Pine.LNX.4.64.0612101704390.2630@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
>> Man pages are reference documentation, so I figure it's OK to sacrifice
>> a little newbie-friendliness for accuracy and concision.
>
> I disagree. Clarity should be the first goal. And the fact that even
> modified files have to be specified is something worth enphasizing,
> especially since this is not something other systems do.
Let's keep plumbing documentation technical reference material,
but allow more newbie friendliness in Porcelain-ish
documentation.
^ permalink raw reply
* Re: Collection of stgit issues and wishes
From: Jakub Narebski @ 2006-12-10 23:02 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0612101426l763d46cahe15683410cb4398d@mail.gmail.com>
Catalin Marinas wrote:
> On 10/12/06, Jakub Narebski <jnareb@gmail.com> wrote:
>> The git commands StGit uses to perform operations automatically record
>> changes in branches in reflog. What StGit does not provide is the "reason".
>> You do use git-update-ref?
>
> Yes, only for updating HEAD. The refs in refs/patches/<branch>/ are
> written directly. I initialy wanted to add patch history support using
> reflogs and added "git-update-ref -m ..." for the patch commits but I
> found slow the pushing operation a bit. Do you only want to track the
> reflogs for HEAD?
Yes, I want for StGit to provide reasons when updating HEAD. I know that
StGit manages it's own versioning of patches not using reflog -- fine.
What matters for me is reflog for HEAD after "stg commit; stg clean".
--
Jakub Narebski
^ permalink raw reply
* Re: kernel.org mirroring (Re: [GIT PULL] MMC update)
From: Jakub Narebski @ 2006-12-10 22:59 UTC (permalink / raw)
To: Jeff Garzik
Cc: Martin Langhoff, Git Mailing List, Linus Torvalds, H. Peter Anvin,
Rogan Dawes, Kernel Org Admin
In-Reply-To: <457C84AC.7060105@garzik.org>
Jeff Garzik wrote:
> Jakub Narebski wrote:
>>
>> And in CGI standard there is a way to access additional HTTP headers
>> info from CGI script: the envirionmental variables are HTTP_HEADER,
>> for example if browser sent If-Modified-Since: header it's value
>> can be found in HTTP_IF_MODIFIED_SINCE environmental variable.
>
> The CGI spec does not at all guarantee that the CGI environment will
> contain all the HTTP headers sent by the client. That was the point of
> the environment dump script -- you can see exactly which headers are,
> and are not, passed through to CGI.
>
> CGI only /guarantees/ a bare minimum (things like QUERY_STRING,
> PATH_INFO, etc.)
>
> Even basic server info environment variables are optional.
I have checked that at least Apache 2.0.54 passes HTTP_IF_MODIFIED_SINCE
when getting If-Modified-Since: header (my own script + netcat/nc).
>> It is ETag, not E-tag. Besides, I don't see what the attached script is
>> meant to do: it does not output the sample file anyway.
>
> It's not meant to output the sample file. It outputs the server
> metadata sent to the CGI script (the environment variables). The sample
> file was simply a way to play around with etag and last-modified metadata.
Ah.
>> The idea is of course to stop processing in CGI script / mod_perl script
>> as soon as possible if cache validates.
>
> Certainly. That should help cut down on I/O. FWIW though the projects
> list is particularly painful, with its File::Find call, which you'll
> need to do in order to return 304-not-modified.
First, it is better to use $projects_list which is projects index file
in the format:
<project path> SPC <project owner>
where <project path> is relative to $projectroot and is URI encoded; well
at least SPC has to be URI (percent) encoded. <project owner> is owner
of given project, and is also URI encoded (one would usually use '+' in
the place of SPC here).
Gitweb now can generate projects list in above format, by using
"project_index" action ("a=project_index" query string), or by clicking
'TXT' link at the bottom of the projects list page in new gitweb: see
http://repo.or.cz by Petr Baudis. The problem is that it generates
projects list from the list of projects it sees, so to generate it from
scratch from the filesystem you have for generating "project_index"
to have $projects_list a directory (changing it to something that
evals to false, e.g. undef or "" makes gitweb use $projectroot for
$projects_list). I have posted how to do this.
The project list changes rarely, only on addition/removal of project,
and on changing owner of project; so it can be generated on demand.
Second, even with $projects_list being set to projects index file
as of now gitweb runs git-for-each-ref (which scans refs and access
pack file for commit date), checks for description file and reads it;
for $projects_list being directory it also checks project directory
owner. I plan to make it configurable to read last activity from
all heads (all branches) as it is now, from HEAD (current branch)
as it was before, or given branch (for example 'master').
Assuming that gitweb is configured to read last activity from single
defined branch, generating ETag = checksum(sha1 of heads of projects)
needs at least read one file from each project.
>> I don't know if Apache intercepts and remembers ETag and Last-Modified
>> headers, adds 304 Not Modified HTTP response on finding that cache validates
>> and cuts out CGI script output. I.e. if browser provided If-Modified-Since:,
>> script wrote Last-Modified: header, If-Modified-Since: is no earlier than
>> Last-Modified: (usually is equal in the case of cache validation), then
>> Apache provides 304 Not Modified response instead of CGI script output.
>
> This wanders into the realm of mod_cache configuration, I think. (which
> I have tried to get working as reverse proxy, and failed serveral times)
> If you are not using mod_*_cache, then Apache must execute the CGI
> script every time AFAICS, regardless of etag/[if-]last-mod headers.
No, it wanders into realm of header parsing by Apache, and NPH (No Parse
Headers) option.
Even if Apache does execute CGI script to completion every time, it might
not send the output of the script, but HTTP 304 Not Modified reply. Might.
I don't know if it does.
--
Jakub Narebski
^ permalink raw reply
* Re: [PATCH 2/3] merge-recursive: make empty tree a known object
From: Linus Torvalds @ 2006-12-10 22:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Shawn O. Pearce, git
In-Reply-To: <7vlklf2xgd.fsf@assigned-by-dhcp.cox.net>
On Sun, 10 Dec 2006, Junio C Hamano wrote:
> > In contrast, the empty tree clearly ends up being an interesting special
> > case that actually gets used occasionally, ie here we had two independent
> > uses for the same thing..
>
> Two?
merge-recursive and "diff against root"
^ permalink raw reply
* Re: [RFC \ WISH] Add -o option to git-rev-list
From: Linus Torvalds @ 2006-12-10 22:53 UTC (permalink / raw)
To: Marco Costalba
Cc: Git Mailing List, Junio C Hamano, Alex Riesen, Shawn Pearce
In-Reply-To: <e5bfff550612101435o6bc938acmac28ad6adf0c8844@mail.gmail.com>
On Sun, 10 Dec 2006, Marco Costalba wrote:
>
> I consistently have len == 65536 bytes until the last fread() where
> it's less.
That's because fread() will block until it gets all data. Did you actually
ever try this with a uncached tree and did you compare what you got with a
plain "read()".
On older kernels, I guarantee that you get 4kB at a time for reads, even
for a blocking pipe. Because we have bigger pipe buffers these days, it
_may_ return 64kB at a time every time, but only if the writer is much
faster than the reader.
Based on the fact that you say that "read()" was ten times slower than
fread(), I very much suspect you got 4kB at a time, and then slept 100ms
each time or something.
Anyway, you should seriously also check the case when "git rev-list" is
slow because you have cold caches and unpacked objects. You can't wait for
it to synchronously write a thousand lines or so - that could take
seconds.
> How can open in nonblocking mode with popen() ?
>
> FILE *popen(const char *command, const char *type);
something like
fcntl(fileno(file), F_SETFL, O_NONBLOCK);
will do it, and then your loop should look something like
for (;;) {
int count = read(fileno(file), buf, BUFSIZE);
if (!count) {
/* All done, no more to read */
return 0;
}
if (count < 0) {
if (errno == EAGAIN)
break;
if (errno == EINTR)
continue;
/* Anything else is fatal - report error */
return -1;
}
... handle 'count' new bytes here ...
}
.. this is the EAGAIN case ..
either set polling on the file descriptor, or use a
timer here yo get back to this loop eventually.
looks about right.
^ permalink raw reply
* Re: [PATCH 1/3] diff_tree_sha1(): avoid rereading trees if possible
From: Johannes Schindelin @ 2006-12-10 22:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, torvalds, git
In-Reply-To: <7vvekk5xpc.fsf@assigned-by-dhcp.cox.net>
Hi,
On Sat, 9 Dec 2006, Junio C Hamano wrote:
> I think this is overkill that only helps a very narrow "empty tree"
> special case that [PATCH 2/3] addresses, and can be easily and
> incorrectly abused. We do not want people to expect that reading many
> trees from different revisions as "struct tree" objects and keeping all
> of them in memory would magically speed up diff-tree, for example.
>
> I'd prefer write_sha1_file() approach in Shawn's patch for its
> simplicity at least for now.
Okay, after thinking about it, I agree. merge-recursive is really the only
user for such a diff. So, I do not think EMPTY_TREE would be useful.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] shortlog: fix segfault on empty authorname
From: Johannes Schindelin @ 2006-12-10 22:43 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Nicolas Pitre, git
In-Reply-To: <20061210034524.GA11819@coredump.intra.peff.net>
Hi,
On Sat, 9 Dec 2006, Jeff King wrote:
> On Sun, Dec 10, 2006 at 12:21:03AM +0100, Johannes Schindelin wrote:
>
> > > Instead, remove at most one space between name and address.
> >
> > Why? We can fix it properly: Instead of
> >
> > > - while (isspace(bob[-1]))
> > > + if (isspace(bob[-1]))
> >
> > do something like
> >
> > while (bob - 1 != buffer + 7 &&
> > isspace(bob[-1]))
>
> It doesn't look like there are ever extra spaces to get soaked up in the
> kernel or git repositories, but if there is a reason to expect
> Full Name <user@domain>
> then we should probably replace my fix with yours.
This is exactly the kind of assumption which led to the bug to begin with.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Documentation/git-commit: rewrite to make it more end-user friendly.
From: J. Bruce Fields @ 2006-12-10 22:41 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Josef Weidendorfer, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0612101704390.2630@xanadu.home>
On Sun, Dec 10, 2006 at 05:07:40PM -0500, Nicolas Pitre wrote:
> On Sun, 10 Dec 2006, J. Bruce Fields wrote:
>
> > Or maybe: "by using gitlink:git-add[1] to add new content (of either new
> > or newly modified files) to the next commit."
> >
> > Man pages are reference documentation, so I figure it's OK to sacrifice
> > a little newbie-friendliness for accuracy and concision.
>
> I disagree. Clarity should be the first goal. And the fact that even
> modified files have to be specified is something worth enphasizing,
> especially since this is not something other systems do.
OK, OK. Shortest I can manage then is:
by using gitlink:git-add[1] to add new or modified files to the
next commit. This adds a file's current contents only; run
gitlink:git-add[1] again to include any subsequent changes.
which may not be any improvement.
^ permalink raw reply
* Re: [PATCH] remove "[PATCH]" prefix from shortlog output
From: Johannes Schindelin @ 2006-12-10 22:40 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0612091954010.2630@xanadu.home>
Hi,
On Sat, 9 Dec 2006, Nicolas Pitre wrote:
> This doesn't convey much information in the shortlog context.
> And the perl version did strip it as well.
From the current builtin-shortlog.c:128ff:
if (!strncmp(oneline, "[PATCH", 6)) {
char *eob = strchr(oneline, ']');
if (eob) {
while (isspace(eob[1]) && eob[1] != '\n')
eob++;
if (eob - oneline < onelinelen) {
onelinelen -= eob - oneline;
oneline = eob;
}
}
}
It tries not only to strip "[PATCH]", but also "[PATCH 0/n]" and basically
every prefix beginning with "[PATCH" and ending in "]". I do not remember
if I really tested that code, but it should work.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC \ WISH] Add -o option to git-rev-list
From: Marco Costalba @ 2006-12-10 22:35 UTC (permalink / raw)
To: Linus Torvalds
Cc: Git Mailing List, Junio C Hamano, Alex Riesen, Shawn Pearce
In-Reply-To: <Pine.LNX.4.64.0612101410220.12500@woody.osdl.org>
On 12/10/06, Linus Torvalds <torvalds@osdl.org> wrote:
>
>
> On Sun, 10 Dec 2006, Marco Costalba wrote:
> >
> > Ok. Perhaps I'm doing something wrong but the following code it's
> > always 10% slower then the temporary file one (4.7s against 4.3s for
> > linux tree)
>
> Why do you seem to be doing a "new" on every iteration inside the loop?
>
Becuase it's there where I store the file content.
Function parseSingleBuffer(ba) does only the indexing. But the file
content is stored in QByteArray objects (little wrappers around a
const char* []). So the fread() in the byte array object is the _only_
data copy operation in whole qgit.
> Also, why do you have that strange FILE_BLOCK_SIZE thing, and in
> particular the "if (len < FILE_BLOCK_SIZE)" check? One thing that pipes vs
> files do is the blocking factor.
>
> Especially with older kernels, I _guarantee_ you that you'll only ever get
> 4kB at a time, so because of that "if (len < 64kB) break" thing, the only
> thing you're doing is to make sure things suck performance-wise, and you
> won't be reading the rest of the data until 100ms later.
>
I consistently have len == 65536 bytes until the last fread() where
it's less. See below run against qgit own repository with 'len'
printed inside while loop.
$ ./qgit
Found GNU source-highlight 2.5
len is <65536>
len is <65536>
len is <65536>
len is <65536>
len is <65536>
len is <65536>
len is <65536>
len is <54479>
bash-3.1$
> IOW, your code is written for a file, and makes no sense there either
> (checking "feof(file)" is wrong, since you may well have hit the EOF
> *at*that*time*, but the file may GROW since you are doing it all in the
> background, so you can't rely on feof() anyway).
>
Yes. feof() it's difficult to handle correctly.
> For a pipe, what you should do is to make sure it's in nonblocking mode,
> and just continue reading until it gets no more. And instead of using a
> timeout, you should use poll() or something to get notified when there is
> more data.
>
How can open in nonblocking mode with popen() ?
FILE *popen(const char *command, const char *type);
> IOW, the reason it's slow is because you're doing the wrong thing.
>
That's for sure :-) My problem is to guess what's is wrong.
^ permalink raw reply
* Re: [PATCH 2/3] merge-recursive: make empty tree a known object
From: Junio C Hamano @ 2006-12-10 22:33 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Johannes Schindelin, Shawn O. Pearce, git
In-Reply-To: <Pine.LNX.4.64.0612101326420.12500@woody.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> On Sun, 10 Dec 2006, Junio C Hamano wrote:
>>
>> That is fine by me. We would benefit from an empty blob and an
>> empty tree.
>
> I was wondering if we ever had any special case where we wanted the empty
> blob, and couldn't come up with any.
After saying that I was thinking about the same. Empty blob is
not all that useful (and let's not bring up the "empty commit"
in this discussion pretty please).
> In contrast, the empty tree clearly ends up being an interesting special
> case that actually gets used occasionally, ie here we had two independent
> uses for the same thing..
Two?
I sent out a patch with comment about an issue I worry about.
I do not think the empty-tree case matters in practice because
we do not allow an empty tree to be committed, but if we add
empty blob to the set of "well known objects", the backward
compatibility issue becomes real.
^ permalink raw reply
* Re: [PATCH 2/3] merge-recursive: make empty tree a known object
From: Junio C Hamano @ 2006-12-10 22:28 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git, Shawn O. Pearce, Johannes Schindelin
In-Reply-To: <7vpsar4fcu.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
> Linus Torvalds <torvalds@osdl.org> writes:
>
>> So you could make "read_sha1_file()" just have a special case for known
>> objects at the end. If the pack entry fails, the loose file case fails,
>> then rather than returning NULL at the end, you could have a list of known
>> fixed objects..
>
> That is fine by me. We would benefit from an empty blob and an
> empty tree.
I think this might be fragile in the presense of older
implementation of git.
A new git may operate fine without having "well known" objects
in the repository, and would happily write result that points at
them; an older git would find the resulting repository corrupt.
We somehow need to force "well known" objects to be instantiated
in the object database when something else points at them. I am
not absolutely sure if keeping has_sha1_file() unaware of this
magic is enough.
-- >8 --
Implement "well known" built-in objects.
Make read_sha1_file() aware of a handful "well known" objects
and use built-in copy when they are not found in the object
database.
Note that we do not hook this into has_sha1_file(), so it is
possible that has_sha1_file() says you do not have an empty tree
object in your repository but read_sha1_file() successfully
gives you an empty tree object back.
Otherwise write_sha1_file() would not create "well known"
objects in the repository, leaving the repository corrupt for
older implementations of git.
---
diff --git a/cache.h b/cache.h
index f2ec5c8..7ee40e8 100644
--- a/cache.h
+++ b/cache.h
@@ -241,6 +241,10 @@ int adjust_shared_perm(const char *path);
int safe_create_leading_directories(char *path);
char *enter_repo(char *path, int strict);
+/* Well known object names */
+extern unsigned const char EMPTY_TREE_NAME[];
+extern unsigned const char EMPTY_BLOB_NAME[];
+
/* Read and unpack a sha1 file into memory, write memory to a sha1 file */
extern int sha1_object_info(const unsigned char *, char *, unsigned long *);
extern void * unpack_sha1_file(void *map, unsigned long mapsize, char *type, unsigned long *size);
diff --git a/merge-recursive.c b/merge-recursive.c
index 32e186c..1c56fd3 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1234,11 +1234,8 @@ static int merge(struct commit *h1,
merged_common_ancestors = pop_commit(&ca);
if (merged_common_ancestors == NULL) {
/* if there is no common ancestor, make an empty tree */
- struct tree *tree = xcalloc(1, sizeof(struct tree));
-
- tree->object.parsed = 1;
- tree->object.type = OBJ_TREE;
- write_sha1_file(NULL, 0, tree_type, tree->object.sha1);
+ struct tree *tree = lookup_tree(EMPTY_TREE_NAME);
+ parse_tree(tree);
merged_common_ancestors = make_virtual_commit(tree, "ancestor");
}
diff --git a/sha1_file.c b/sha1_file.c
index 63f416b..c6efe88 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1315,11 +1315,32 @@ static void *read_packed_sha1(const unsigned char *sha1, char *type, unsigned lo
return unpack_entry(&e, type, size);
}
-void * read_sha1_file(const unsigned char *sha1, char *type, unsigned long *size)
+unsigned const char EMPTY_TREE_NAME[20] = {
+ 0x4b, 0x82, 0x5d, 0xc6, 0x42, 0xcb, 0x6e, 0xb9, 0xa0, 0x60,
+ 0xe5, 0x4b, 0xf8, 0xd6, 0x92, 0x88, 0xfb, 0xee, 0x49, 0x04,
+};
+
+unsigned const char EMPTY_BLOB_NAME[20] = {
+ 0xe6, 0x9d, 0xe2, 0x9b, 0xb2, 0xd1, 0xd6, 0x43, 0x4b, 0x8b,
+ 0x29, 0xae, 0x77, 0x5a, 0xd8, 0xc2, 0xe4, 0x8c, 0x53, 0x91,
+};
+
+static struct well_known_objects {
+ char *type;
+ unsigned const char *sha1;
+ unsigned long size;
+ const char *data;
+} well_known_objects[] = {
+ { "tree", EMPTY_TREE_NAME, 0UL, "" },
+ { "blob", EMPTY_BLOB_NAME, 0UL, "" },
+};
+
+void *read_sha1_file(const unsigned char *sha1, char *type, unsigned long *size)
{
unsigned long mapsize;
void *map, *buf;
struct pack_entry e;
+ int i;
if (find_pack_entry(sha1, &e, NULL))
return read_packed_sha1(sha1, type, size);
@@ -1332,6 +1353,17 @@ void * read_sha1_file(const unsigned char *sha1, char *type, unsigned long *size
reprepare_packed_git();
if (find_pack_entry(sha1, &e, NULL))
return read_packed_sha1(sha1, type, size);
+
+ /* "well known" objects */
+ for (i = 0; i < ARRAY_SIZE(well_known_objects); i++) {
+ if (!hashcmp(well_known_objects[i].sha1, sha1)) {
+ *size = well_known_objects[i].size;
+ buf = xmalloc(*size);
+ strcpy(type, well_known_objects[i].type);
+ memcpy(buf, well_known_objects[i].data, *size);
+ return buf;
+ }
+ }
return NULL;
}
^ permalink raw reply related
* Re: Collection of stgit issues and wishes
From: Catalin Marinas @ 2006-12-10 22:26 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <200612101801.58247.jnareb@gmail.com>
On 10/12/06, Jakub Narebski <jnareb@gmail.com> wrote:
> Catalin Marinas wrote:
> > On 10/12/06, Jakub Narebski <jnareb@gmail.com> wrote:
> >>>> Here are some issues which are a bit annoying for me:
> >>>> - make "stg help" (without command name) equivalent to "stg --help"
> >
> > There was a patch in this area. Doesn't it work correctly now?
>
> I use stgit 0.11
I was in general referring to the latest HEAD in the StGIT repository.
>
> 1056:[gitweb/web!git]$ stg help
> usage: stg help <command>
>
> while "stg --help" lists all commands.
Works correctly in the latest snapshot.
> >> And as far as I can see it doe not use git credentials (user.name and
> >> user.email).
> >
> > StGIT now uses the GIT credentials (and config files).
>
> Hmmm... in stgit 0.11 "stg refresh --sign" once gave me Signed-off-by:
> Nobody line instead of using git user.name and user.email.
Again, it is different in the latest snapshot (feature added post 0.11).
> The git commands StGit uses to perform operations automatically record
> changes in branches in reflog. What StGit does not provide is the "reason".
> You do use git-update-ref?
Yes, only for updating HEAD. The refs in refs/patches/<branch>/ are
written directly. I initialy wanted to add patch history support using
reflogs and added "git-update-ref -m ..." for the patch commits but I
found slow the pushing operation a bit. Do you only want to track the
reflogs for HEAD?
--
^ permalink raw reply
* Re: [RFC \ WISH] Add -o option to git-rev-list
From: Linus Torvalds @ 2006-12-10 22:16 UTC (permalink / raw)
To: Marco Costalba
Cc: Git Mailing List, Junio C Hamano, Alex Riesen, Shawn Pearce
In-Reply-To: <e5bfff550612101405h507cc877k87491264d11b0aa8@mail.gmail.com>
On Sun, 10 Dec 2006, Marco Costalba wrote:
>
> Ok. Perhaps I'm doing something wrong but the following code it's
> always 10% slower then the temporary file one (4.7s against 4.3s for
> linux tree)
Why do you seem to be doing a "new" on every iteration inside the loop?
Also, why do you have that strange FILE_BLOCK_SIZE thing, and in
particular the "if (len < FILE_BLOCK_SIZE)" check? One thing that pipes vs
files do is the blocking factor.
Especially with older kernels, I _guarantee_ you that you'll only ever get
4kB at a time, so because of that "if (len < 64kB) break" thing, the only
thing you're doing is to make sure things suck performance-wise, and you
won't be reading the rest of the data until 100ms later.
IOW, your code is written for a file, and makes no sense there either
(checking "feof(file)" is wrong, since you may well have hit the EOF
*at*that*time*, but the file may GROW since you are doing it all in the
background, so you can't rely on feof() anyway).
For a pipe, what you should do is to make sure it's in nonblocking mode,
and just continue reading until it gets no more. And instead of using a
timeout, you should use poll() or something to get notified when there is
more data.
IOW, the reason it's slow is because you're doing the wrong thing.
^ permalink raw reply
* Re: kernel.org mirroring (Re: [GIT PULL] MMC update)
From: Jeff Garzik @ 2006-12-10 22:14 UTC (permalink / raw)
To: Martin Langhoff
Cc: Linus Torvalds, Jakub Narebski, Git Mailing List, H. Peter Anvin,
Rogan Dawes, Kernel Org Admin
In-Reply-To: <46a038f90612101401m5f65aefbh78f7adf84725ade4@mail.gmail.com>
Martin Langhoff wrote:
> On 12/11/06, Linus Torvalds <torvalds@osdl.org> wrote:
>> Sure, if the proxies actually do the rigth thing (which they may or may
>> not do)
>
> For a high-traffic setup like kernel.org, you can setup a local
> reverse proxy -- it's a pretty standard practice. That allows you to
> control a well-behaved and locally tuned caching engine just by
> emitting good headers.
>
> It beats writing and maintaining an internal caching mechanism for
> each CGI script out there by a long mile. It means there'll be no
> further tunables or complexity for administrators of other gitweb
> installs.
If gitweb produced cache-friendly headers, squid could definitely serve
as an HTTP front-end ("HTTP accelerator" mode in squid talk).
In fact, given kernel.org's slave1/slave2<->master setup, that's a
pretty natural fit for caching files and/or cache-aware CGI output.
You could even replace rsync to the slaves, if squid was serving as the
front-end accelerator running on the slaves, communicating to the master.
squid is smart enough to hold off a thundering herd, and only pulls
single cacheable copies of files as needed.
Jeff
^ permalink raw reply
* Re: [RFC \ WISH] Add -o option to git-rev-list
From: Marco Costalba @ 2006-12-10 22:09 UTC (permalink / raw)
To: Linus Torvalds
Cc: Git Mailing List, Junio C Hamano, Alex Riesen, Shawn Pearce
In-Reply-To: <e5bfff550612101405h507cc877k87491264d11b0aa8@mail.gmail.com>
On 12/10/06, Marco Costalba <mcostalba@gmail.com> wrote:
> On 12/10/06, Linus Torvalds <torvalds@osdl.org> wrote:
> >
> void DataLoader::on_timeout() {
>
> if (canceling)
> deleteLater();
>
Of course is
if (canceling) {
deleteLater();
return;
}
^ permalink raw reply
* Re: kernel.org mirroring (Re: [GIT PULL] MMC update)
From: Jeff Garzik @ 2006-12-10 22:08 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jakub Narebski, Martin Langhoff, Git Mailing List, H. Peter Anvin,
Rogan Dawes, Kernel Org Admin
In-Reply-To: <Pine.LNX.4.64.0612101228590.12500@woody.osdl.org>
Linus Torvalds wrote:
>
> On Sun, 10 Dec 2006, Jakub Narebski wrote:
>> Well, the idea (perhaps stupid idea: I don't know how caching engines
>> / reverse proxy works) was that there would be caching engine / reverse
>> proxy in the front (Squid for example) would cache results and serve it
>> to rampaging hordes.
>
> Sure, if the proxies actually do the rigth thing (which they may or may
> not do)
squid seems to work well as an HTTP accelerator (reverse proxy).
Apache's mem|disk cache stuff fails miserably.
Unfortunately squid development seems to have slowed in recent years.
Jeff
^ permalink raw reply
* Re: [PATCH] Documentation/git-commit: rewrite to make it more end-user friendly.
From: Nicolas Pitre @ 2006-12-10 22:07 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: Josef Weidendorfer, Junio C Hamano, git
In-Reply-To: <20061210210057.GB23387@fieldses.org>
On Sun, 10 Dec 2006, J. Bruce Fields wrote:
> Or maybe: "by using gitlink:git-add[1] to add new content (of either new
> or newly modified files) to the next commit."
>
> Man pages are reference documentation, so I figure it's OK to sacrifice
> a little newbie-friendliness for accuracy and concision.
I disagree. Clarity should be the first goal. And the fact that even
modified files have to be specified is something worth enphasizing,
especially since this is not something other systems do.
^ permalink raw reply
* Re: kernel.org mirroring (Re: [GIT PULL] MMC update)
From: Jeff Garzik @ 2006-12-10 22:05 UTC (permalink / raw)
To: Jakub Narebski
Cc: Martin Langhoff, Git Mailing List, Linus Torvalds, H. Peter Anvin,
Rogan Dawes, Kernel Org Admin
In-Reply-To: <200612102011.52589.jnareb@gmail.com>
Jakub Narebski wrote:
> Adn in CGI standard there is a way to access additional HTTP headers
> info from CGI script: the envirionmental variables are HTTP_HEADER,
> for example if browser sent If-Modified-Since: header it's value
> can be found in HTTP_IF_MODIFIED_SINCE environmental variable.
The CGI spec does not at all guarantee that the CGI environment will
contain all the HTTP headers sent by the client. That was the point of
the environment dump script -- you can see exactly which headers are,
and are not, passed through to CGI.
CGI only /guarantees/ a bare minimum (things like QUERY_STRING,
PATH_INFO, etc.)
Even basic server info environment variables are optional.
> It is ETag, not E-tag. Besides, I don't see what the attached script is
> meant to do: it does not output the sample file anyway.
It's not meant to output the sample file. It outputs the server
metadata sent to the CGI script (the environment variables). The sample
file was simply a way to play around with etag and last-modified metadata.
> The idea is of course to stop processing in CGI script / mod_perl script
> as soon as possible if cache validates.
Certainly. That should help cut down on I/O. FWIW though the projects
list is particularly painful, with its File::Find call, which you'll
need to do in order to return 304-not-modified.
> I don't know if Apache intercepts and remembers ETag and Last-Modified
> headers, adds 304 Not Modified HTTP response on finding that cache validates
> and cuts out CGI script output. I.e. if browser provided If-Modified-Since:,
> script wrote Last-Modified: header, If-Modified-Since: is no earlier than
> Last-Modified: (usually is equal in the case of cache validation), then
> Apache provides 304 Not Modified response instead of CGI script output.
This wanders into the realm of mod_cache configuration, I think. (which
I have tried to get working as reverse proxy, and failed serveral times)
If you are not using mod_*_cache, then Apache must execute the CGI
script every time AFAICS, regardless of etag/[if-]last-mod headers.
Jeff
^ permalink raw reply
* Re: [RFC \ WISH] Add -o option to git-rev-list
From: Marco Costalba @ 2006-12-10 22:05 UTC (permalink / raw)
To: Linus Torvalds
Cc: Git Mailing List, Junio C Hamano, Alex Riesen, Shawn Pearce
In-Reply-To: <Pine.LNX.4.64.0612101213520.12500@woody.osdl.org>
On 12/10/06, Linus Torvalds <torvalds@osdl.org> wrote:
>
> data into ASCII etc. So temp-files are almost never a better solution than
> keeping things in memory (unless you use those temp-files to truly
> _share_ data between processes, ie you do a shared mmap and they can
> re-use the same pages actively in a way they couldn't otherwise).
>
Ok. Perhaps I'm doing something wrong but the following code it's
always 10% slower then the temporary file one (4.7s against 4.3s for
linux tree)
bool DataLoader::start(const QStringList& args, const QString& workDir) {
QDir::setCurrent(workDir);
_file = popen(args.join(" ").ascii(), "r");
if (!_file)
return false;
loadTime.start();
guiUpdateTimer.start(10, true); // will call on_timeout() in 10ms
return true;
}
void DataLoader::on_timeout() {
if (canceling)
deleteLater();
// int fd = fileno(_file); // read() case
ssize_t len = 0;
while (1) {
QByteArray* ba = new QByteArray(FILE_BLOCK_SIZE); // 64KB
// len = read(fd, ba->data(), ba->size()); // read() case
len = fread(ba->data(), 1, ba->size(), _file); // fread() case
if (len <= 0) {
delete ba;
break;
} else if (len < (ssize_t)ba->size()) // very rare, 4 out of 40000
on Linux tree
ba->resize(len);
loadedBytes += len;
fh->rowData.append(ba); // fh->rowData it's a pointer's list
parseSingleBuffer(ba);
// avoid reading small chunks if data producer is still running
if (len < FILE_BLOCK_SIZE)
break;
}
// if (len == 0) { // read() case
if (feof(_file)) { // fread() case
emit loaded(fh, loadedBytes, loadTime.elapsed(), true, "", ""); //
show some stats
pclose(_file);
_file = NULL;
deleteLater();
} else
guiUpdateTimer.start(100, true); // next read in 100ms
}
Uncomment 'read() case' lines and comment out the 'fread case()' ones
and you have a way slooooower code, about 10 times slower!
^ permalink raw reply
* Re: kernel.org mirroring (Re: [GIT PULL] MMC update)
From: Martin Langhoff @ 2006-12-10 22:01 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jakub Narebski, Jeff Garzik, Git Mailing List, H. Peter Anvin,
Rogan Dawes, Kernel Org Admin
In-Reply-To: <Pine.LNX.4.64.0612101228590.12500@woody.osdl.org>
On 12/11/06, Linus Torvalds <torvalds@osdl.org> wrote:
> Sure, if the proxies actually do the rigth thing (which they may or may
> not do)
For a high-traffic setup like kernel.org, you can setup a local
reverse proxy -- it's a pretty standard practice. That allows you to
control a well-behaved and locally tuned caching engine just by
emitting good headers.
It beats writing and maintaining an internal caching mechanism for
each CGI script out there by a long mile. It means there'll be no
further tunables or complexity for administrators of other gitweb
installs.
cheers,
^ permalink raw reply
* Re: reexporting git repository via git-daemon
From: Junio C Hamano @ 2006-12-10 21:39 UTC (permalink / raw)
To: Arkadiusz Miskiewicz; +Cc: git
In-Reply-To: <200612101742.44564.arekm@maven.pl>
Arkadiusz Miskiewicz <arekm@maven.pl> writes:
>> 100% repeatable at this moment. Any ideas?
>
> after:
> $ git repack -a -d
> Generating pack...
> Done counting 33587 objects.
> Deltifying 33587 objects.
> 100% (33587/33587) done
> Writing 33587 objects.
> 100% (33587/33587) done
> Total 33587, written 33587 (delta 23205), reused 33587 (delta 23205)
> Pack pack-bc830a5d1c2efa3b727ef9af8faba13c3e719395 created.
>
> the problem disapears and I can safely reexport git repository.
>
> Does anyone care about this bug in git?
I do. I wanted to see if it reproduced but it didn't for me.
^ permalink raw reply
* Re: [PATCH 2/3] merge-recursive: make empty tree a known object
From: Linus Torvalds @ 2006-12-10 21:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Shawn O. Pearce, git
In-Reply-To: <7vpsar4fcu.fsf@assigned-by-dhcp.cox.net>
On Sun, 10 Dec 2006, Junio C Hamano wrote:
>
> That is fine by me. We would benefit from an empty blob and an
> empty tree.
I was wondering if we ever had any special case where we wanted the empty
blob, and couldn't come up with any. Unlike the "tree diff" case, a "blob
diff" will not actually do a "real diff" with a non-existing object, it
special-cases it (and, for performance reasons, I think it really should).
And all the blob-diff routines generally want to be able to take explicit
data anyway, not just diffing two SHA1's (since we often don't have a SHA1
anyway - the working tree case).
So while I think it would make sense to have both the "empty tree" and the
"empty blob" as special cases, off-hand I can't actually see where we'd
ever use the empty blob SHA1.
In contrast, the empty tree clearly ends up being an interesting special
case that actually gets used occasionally, ie here we had two independent
uses for the same thing..
But maybe somebody can point to a case where we actually would want to
internally have an easy representation of "empty blob".
^ permalink raw reply
* Re: [PATCH/RFC 4/3] gitweb: SHA-1 in commit log message links to "object" view
From: Junio C Hamano @ 2006-12-10 21:29 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <11657536473295-git-send-email-jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> Instead of checking if explicit SHA-1 in commit log message is sha1 of
> commit and making link to "commit" view, make [fragment of] explicit
> SHA-1 in commit log message link to "object" view. While at it allow
> to hyperlink also shortened SHA-1, from 8 characters up to full SHA-1,
> instead of requiring full 40 characters of SHA-1.
>
> This makes the following changes:
>
> * SHA-1 of objects which no longer exists, for example in commit
> cherry-picked from no longer existing temporary branch, or revert
> of commit in rebased branch, are no longer marked as such by not
> being made into hyperlink (and not having default hyperlink view:
> being underlined among others). On the other hand it makes gitweb
> to not write error messages when object is not found to web serwer
> log; it also moves cost of getting type and SHA-1 validation to
> when link is clicked, and not only viewed.
>
> * SHA-1 of other objects: blobs, trees, tags are also hyperlinked
> and lead to appropriate view (although in the case of tags it is
> more natural to just use tag name).
>
> * You can put shortened SHA-1 of commit in the commit message, and it
> would be hyperlinked; it would be checked on clicking if abbrev is
> unique.
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> This follows the "lazy hyperlink" idea of symbolic link support in the
> "tree" view.
>
> It is an RFC (Requests For Comments) because I'm not sure if it
> wouldn't be better to make dead SHA-1 marked in commit log message,
> instead of finfing it out after clicking...
I am certainly in favor of the approach. Will look at the code
later. Thanks.
^ permalink raw reply
* Re: [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.
From: Junio C Hamano @ 2006-12-10 21:27 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <200612101639.22397.robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> writes:
>> Well, have you tried this?
>>
>> git diff-tree -p --binary fe142b3a | git apply --summary --numstat
> of course not. I didn't understand it. Why can't it tell me about removed
> binary files, so I could remove the git-diff-tree invocation to find out about
> added/removed files?
Maybe I am missing something. It tells you about added or
removed files (either binary or non-binary).
I'd prepare a pair of practice patch files. "forward" has
creation, "reverse" has addition.
$ git diff-tree -p --binary fe142b3a >forward.patch
$ git diff-tree -R -p --binary fe142b3a >reverse.patch
$ git apply --summary --numstat forward.patch
50 20 git-cvsexportcommit.perl
145 0 t/t9200-git-cvsexportcommit.sh
- - t/test9200a.png
- - t/test9200b.png
create mode 100755 t/t9200-git-cvsexportcommit.sh
create mode 100644 t/test9200a.png
create mode 100644 t/test9200b.png
$ git apply --summary --numstat reverse.patch
20 50 git-cvsexportcommit.perl
0 145 t/t9200-git-cvsexportcommit.sh
- - t/test9200a.png
- - t/test9200b.png
delete mode 100755 t/t9200-git-cvsexportcommit.sh
delete mode 100644 t/test9200a.png
delete mode 100644 t/test9200b.png
> This fuzz gives some, but not much slack. Reverting the option could be a
> good idea.
>
> Update follows.
Ok.
^ 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