* [PATCH] git-upload-archive: add config option to allow only specified formats
From: Rene Scharfe @ 2006-09-10 15:58 UTC (permalink / raw)
To: Junio C Hamano, Franck Bui-Huu; +Cc: git
In-Reply-To: <7v1wqkt2v4.fsf_-_@assigned-by-dhcp.cox.net>
Documentation/config.txt | 5 +++++
builtin-upload-archive.c | 39 +++++++++++++++++++++++++++++++++++++++
daemon.c | 2 ++
3 files changed, 46 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ce722a2..5c3c6c7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -236,6 +236,11 @@ tar.umask::
the same permissions as gitlink:git-checkout[1] would use. The default
value remains 0, which means world read-write.
+uploadarchive.daemonformats::
+ A comma-separated list of the git-archive formats allowed for upload
+ via git-daemon. If this parameter is missing all formats are allowed
+ for upload.
+
user.email::
Your email address to be recorded in any newly created commits.
Can be overridden by the 'GIT_AUTHOR_EMAIL' and 'GIT_COMMITTER_EMAIL'
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index 96f96bd..6a5245a 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -16,6 +16,37 @@ static const char upload_archive_usage[]
static const char deadchild[] =
"git-upload-archive: archiver died with error";
+static char *daemon_formats;
+
+static int upload_format_config(const char *var, const char *value)
+{
+ if (!strcmp(var, "uploadarchive.daemonformats"))
+ daemon_formats = xstrdup(value);
+ return 0;
+}
+
+static int is_in(const char *needle, const char *haystack, const char *delim)
+{
+ int len = strlen(needle);
+ const char *search = haystack;
+
+ for (;;) {
+ char *pos = strstr(search, needle);
+ if (!pos)
+ return 0;
+ search++;
+ if ((pos == haystack || strchr(delim, pos[-1])) &&
+ (pos[len] == '\0' || strchr(delim, pos[len])))
+ return 1;
+ }
+}
+
+static int upload_format_allowed(const char *fmt)
+{
+ if (getenv("GIT_DAEMON"))
+ return daemon_formats ? is_in(fmt, daemon_formats, " \t,") : 1;
+ return 1;
+}
static int run_upload_archive(int argc, const char **argv, const char *prefix)
{
@@ -38,6 +69,8 @@ static int run_upload_archive(int argc,
if (!enter_repo(buf, 0))
die("not a git archive");
+ git_config(upload_format_config);
+
/* put received options in sent_argv[] */
sent_argc = 1;
sent_argv[0] = "git-upload-archive";
@@ -67,6 +100,12 @@ static int run_upload_archive(int argc,
/* parse all options sent by the client */
treeish_idx = parse_archive_args(sent_argc, sent_argv, &ar);
+ if (!upload_format_allowed(ar.name)) {
+ free(daemon_formats);
+ die("upload of %s format forbidden\n", ar.name);
+ }
+ free(daemon_formats);
+
parse_treeish_arg(sent_argv + treeish_idx, &ar.args, prefix);
parse_pathspec_arg(sent_argv + treeish_idx + 1, &ar.args);
diff --git a/daemon.c b/daemon.c
index a2954a0..2d58abe 100644
--- a/daemon.c
+++ b/daemon.c
@@ -304,6 +304,8 @@ static int run_service(char *dir, struct
return -1;
}
+ setenv("GIT_DAEMON", "I am your father.", 1);
+
/*
* We'll ignore SIGTERM from now on, we have a
* good client.
^ permalink raw reply related
* Re: Change set based shallow clone
From: linux @ 2006-09-10 15:21 UTC (permalink / raw)
To: junkio, mcostalba; +Cc: git, jonsmirl, linux, paulus, torvalds
In-Reply-To: <e5bfff550609092214t4f8e195eib28e302f4d284aa@mail.gmail.com>
This conversation is going in so many directions at once that it's
getting annoying.
The first level decision requires input from the point of view of gitk
and qgit internals as to what's easy to implement.
I'm trying to figure out the gitk code, but I'm not fluent in tcl, and
it has 39 non-boilerplate comment lines in 229 functions and 6308 lines
of source, so it requires fairly intensive grokking.
Still, while it obviously doesn't render bitmaps until the data
appears in the window, it appears as though at least part of the
layout (the "layoutmore" function) code is performed eagerly as
soon as new data arrives via the getcommitlines callback.
(Indeed, it appears that Tk does not inform it of window scroll
events, so it can't wait any longer to decide on the layout.)
Case 1: The visualizer is NOT CAPABLE of accepting out-of-order
input. Without very sophisticated cacheing in git-rev-list,
it must process all of the data before outputting any in
order to make an absolute guarantee of no out-of-order data
despite possibly messed-up timestamps.
It is possible to notice that the date-first traversal only
has one active chain and flush the queued data at that point,
but that situation is unlikely to arise in repositories of
non-trivial size, which are exactly the ones for which
the batch-sorting delay is annoying.
Case 2: The visualizer IS CAPABLE of accepting out-of-order input.
Regardless of whether the layout is done eagerly or lazily,
this requires the visualizer to potentially undo part of
its layout and re-do it, so has a UI implementation cost.
The re-doing does not need to be highly efficient; any number
of heuristics and exception caches can reduce the occurrence of
this in git-rev-list output to very low levels. It's just
absolutely excluding it, without losing incremental output,
that is difficult.
Heuristic: I suspect the most common wrong-timestamp case is
a time zone misconfiguration, so holding back output until the
tree traversal has advanced 24 hours will eliminate most of the
problems. Atypically large time jumps (like more than a year)
could also trigger special "gross clock error" handling.
Cache: whenever a child timestamped older than an ancestor is
encountered, this can be entered in a persistent cache that can
be used to give the child a different sorting priority next time.
The simplest implementation would propagate this information up a
chain of bad timestamps by one commit per git-rev-list invocation,
but even that's probably okay.
(A study of timestamp ordering problems in existing repositories
would be helpful for tuning these.)
In case 2, I utterly fail to see how delaying emitting the out-of-order
commit is of the slightest help to the UI. The simplest way to merge
out-of-order data is with an insertion sort (a.k.a. roll back and
reprocess forward), and the cost of that is minimized if the distance
to back up is minimized.
Some "oops!" annotation on the git-rev-list output may be helpful to
tell the UI that it needs to search back, but it already has an internal
index of commits, so perhaps even that isn't worth bothering with.
Fancier output formats are also more work to process.
With sufficient cacheing of exceptions in git-rev-list, it may be
practical to just have a single "oops, I screwed up; let's start again"
output line, which very rarely triggers.
But can we stop designing git-rev-list output formats until we've figured
out if and how to implement it in the visualizer? Or, more to the point,
visualizers plural. That's the hard part. Then we can see what sort
of git-rev-list output would be most convenient.
For example, is fixing a small number of out-of-place commits practical,
or is it better to purge and restart? The former avoids deleting
already-existing objects, while the latter avoids moving them.
The original problem is that the long delay between starting a git history
browser and being able to browse them is annoying. The visualizer UIs
already support browsing while history is flowing in from git-rev-list,
but git-rev-list is reading and sorting the entire history before
outputting the first line.
^ permalink raw reply
* Re: Change set based shallow clone
From: Jon Smirl @ 2006-09-10 14:56 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Linus Torvalds, linux@horizon.com, Git Mailing List
In-Reply-To: <17668.2019.732961.855446@cargo.ozlabs.ibm.com>
On 9/10/06, Paul Mackerras <paulus@samba.org> wrote:
> Jon Smirl writes:
>
> > gitk takes about a minute to come up on the Mozilla repo when
> > everything is in cache. It takes about twice as long when things are
> > cold. It's enough of delay that I don't use the tool.
>
> I've been doing some timing measurements with Jon's repo. The results
> are interesting.
Using the Mozilla repo you downloaded is not a normal situation since
it is 100% packed. Most people are going to have a few thousand loose
objects floating around too. Loose objects really slow things down.
You noticed too that forks of small apps are relatively slow. The
first pass of the import tools used fork for everything and took a
week to run with 60% of the time spent in the kernel. There may be
some work to do on fork in the kernel. Does mapping the kernel into
the top 1G slow down fork of these small apps? Or are they dynamically
linked to something that is bringing in millions of pages? When I was
doing oprofile all of the time was in the actual fork call and page
table copying.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: Change set based shallow clone
From: Paul Mackerras @ 2006-09-10 12:41 UTC (permalink / raw)
To: Jon Smirl; +Cc: Linus Torvalds, linux@horizon.com, Git Mailing List
In-Reply-To: <9e4733910609081628w2a59551foc28c689d0538a984@mail.gmail.com>
Jon Smirl writes:
> gitk takes about a minute to come up on the Mozilla repo when
> everything is in cache. It takes about twice as long when things are
> cold. It's enough of delay that I don't use the tool.
I've been doing some timing measurements with Jon's repo. The results
are interesting.
It turns out that a lot of the initial delay is in reading all the
references. With ~1600 heads and almost as many tags, readrefs was
taking about 30 seconds (on my 2.5GHz quad G5), largely because it was
doing two execs for each tag, a git rev-parse and a git cat-file,
which was a bit stupid. With that fixed, it's about 5 or 6 seconds
from starting gitk to seeing a window with commits listed in it in the
hot cache case, even still using --topo-order. Without --topo-order
it's about 2-3 seconds to seeing the window with commits listed, but
the overall process takes longer (I still need to figure out why).
In the cold cache case, it takes about 32 seconds to read all the
references, even with the fixed version of readrefs, since that's
about how long git ls-remote .git takes. Also, if you do gitk --all
(as I often do), then gitk does a git rev-parse, which takes about 20
seconds (but then readrefs only takes 10 seconds since the heads are
in cache).
The bottom line is that I can speed up the startup for the hot-cache
case quite a lot. The cold-cache case is going to take about 20-30
seconds whatever I do unless Linus or Junio can come up with a way to
pack the heads and tags. I could read the refs asynchronously but
there will still be a long delay in git rev-parse if you give
arguments such as --all.
Paul.
^ permalink raw reply
* Re: [PATCH 1/2] archive: allow remote to have more formats than we understand.
From: Rene Scharfe @ 2006-09-10 12:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Franck Bui-Huu, git
In-Reply-To: <7vpse4tcyc.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano schrieb:
> This fixes git-archive --remote not to parse archiver arguments;
> otherwise if the remote end implements formats other than the
> one known locally we will not be able to access that format.
Yes! That's the right separation of work between the two parties.
And git-archive --remote=somewhere --list starts to magically work. =)
Thanks,
René
^ permalink raw reply
* [PATCH 3/3] Add sideband status report to git-archive protocol
From: Junio C Hamano @ 2006-09-10 10:47 UTC (permalink / raw)
To: Franck Bui-Huu; +Cc: git, Rene Scharfe
In-Reply-To: <7vk64ctctv.fsf@assigned-by-dhcp.cox.net>
Using the refactored sideband code from existing upload-pack protocol,
this lets the error condition and status output sent from the remote
process to be shown locally.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
* This does it in a "stupid" way. Ideally, write(2) to fd 1
and 2 in each archiver backend could be wrapped with
something like upload-pack.c::send_client_data() that does
straight write(2) to the original destination when it is not
driven by upload-pack, or use send_sideband() when it is, and
that way we can lose two pipes and the multiplexer process.
The sending side of the upload-pack protocol also does this
in the stupid way, but it has an excuse. It needs to set
up a complex pipe and spawn a subprocess (pack-objects) that
does not even want to know about send_client_data().
What is driven by upload-archive() does not have to be a
subprocess (it is just a single ar.write_archive() function
call away), so having to do an extra fork for the multiplexer
is a bit sad...
builtin-archive.c | 24 ++++++++++--
builtin-upload-archive.c | 92 +++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 108 insertions(+), 8 deletions(-)
diff --git a/builtin-archive.c b/builtin-archive.c
index b78d6d8..0c2ad49 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -10,6 +10,7 @@ #include "commit.h"
#include "tree-walk.h"
#include "exec_cmd.h"
#include "pkt-line.h"
+#include "sideband.h"
static const char archive_usage[] = \
"git-archive --format=<fmt> [--prefix=<prefix>/] [--verbose] [<extra>] <tree-ish> [path...]";
@@ -32,16 +33,30 @@ static int run_remote_archiver(const cha
char *url, buf[1024];
int fd[2], i, len, rv;
pid_t pid;
+ const char *exec = "git-upload-archive";
+ int exec_at = 0;
- sprintf(buf, "git-upload-archive");
+ for (i = 1; i < argc; i++) {
+ const char *arg = argv[i];
+ if (!strncmp("--exec=", arg, 7)) {
+ if (exec_at)
+ die("multiple --exec specified");
+ exec = arg + 7;
+ exec_at = i;
+ break;
+ }
+ }
url = xstrdup(remote);
- pid = git_connect(fd, url, buf);
+ pid = git_connect(fd, url, exec);
if (pid < 0)
return pid;
- for (i = 1; i < argc; i++)
+ for (i = 1; i < argc; i++) {
+ if (i == exec_at)
+ continue;
packet_write(fd[1], "argument %s\n", argv[i]);
+ }
packet_flush(fd[1]);
len = packet_read_line(fd[0], buf, sizeof(buf));
@@ -60,8 +75,7 @@ static int run_remote_archiver(const cha
die("git-archive: expected a flush");
/* Now, start reading from fd[0] and spit it out to stdout */
- rv = copy_fd(fd[0], 1);
-
+ rv = recv_sideband("archive", fd[0], 1, 2, buf, sizeof(buf));
close(fd[0]);
rv |= finish_connect(pid);
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index 3bdb607..96f96bd 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -6,12 +6,18 @@ #include "cache.h"
#include "builtin.h"
#include "archive.h"
#include "pkt-line.h"
+#include "sideband.h"
+#include <sys/wait.h>
+#include <sys/poll.h>
static const char upload_archive_usage[] =
"git-upload-archive <repo>";
+static const char deadchild[] =
+"git-upload-archive: archiver died with error";
-int cmd_upload_archive(int argc, const char **argv, const char *prefix)
+
+static int run_upload_archive(int argc, const char **argv, const char *prefix)
{
struct archiver ar;
const char *sent_argv[MAX_ARGS];
@@ -64,9 +70,89 @@ int cmd_upload_archive(int argc, const c
parse_treeish_arg(sent_argv + treeish_idx, &ar.args, prefix);
parse_pathspec_arg(sent_argv + treeish_idx + 1, &ar.args);
+ return ar.write_archive(&ar.args);
+}
+
+int cmd_upload_archive(int argc, const char **argv, const char *prefix)
+{
+ pid_t writer;
+ int fd1[2], fd2[2];
+ /*
+ * Set up sideband subprocess.
+ *
+ * We (parent) monitor and read from child, sending its fd#1 and fd#2
+ * multiplexed out to our fd#1. If the child dies, we tell the other
+ * end over channel #3.
+ */
+ if (pipe(fd1) < 0 || pipe(fd2) < 0) {
+ int err = errno;
+ packet_write(1, "NACK pipe failed on the remote side\n");
+ die("upload-archive: %s", strerror(err));
+ }
+ writer = fork();
+ if (writer < 0) {
+ int err = errno;
+ packet_write(1, "NACK fork failed on the remote side\n");
+ die("upload-archive: %s", strerror(err));
+ }
+ if (!writer) {
+ /* child - connect fd#1 and fd#2 to the pipe */
+ dup2(fd1[1], 1);
+ dup2(fd2[1], 2);
+ close(fd1[1]); close(fd2[1]);
+ close(fd1[0]); close(fd2[0]); /* we do not read from pipe */
+
+ exit(run_upload_archive(argc, argv, prefix));
+ }
+
+ /* parent - read from child, multiplex and send out to fd#1 */
+ close(fd1[1]); close(fd2[1]); /* we do not write to pipe */
packet_write(1, "ACK\n");
packet_flush(1);
- return ar.write_archive(&ar.args);
-}
+ while (1) {
+ struct pollfd pfd[2];
+ char buf[16384];
+ ssize_t sz;
+ pid_t pid;
+ int status;
+
+ pfd[0].fd = fd1[0];
+ pfd[0].events = POLLIN;
+ pfd[1].fd = fd2[0];
+ pfd[1].events = POLLIN;
+ if (poll(pfd, 2, -1) < 0) {
+ if (errno != EINTR) {
+ error("poll failed resuming: %s",
+ strerror(errno));
+ sleep(1);
+ }
+ continue;
+ }
+ if (pfd[0].revents & (POLLIN|POLLHUP)) {
+ /* Data stream ready */
+ sz = read(pfd[0].fd, buf, sizeof(buf));
+ send_sideband(1, 1, buf, sz, DEFAULT_PACKET_MAX);
+ }
+ if (pfd[1].revents & (POLLIN|POLLHUP)) {
+ /* Status stream ready */
+ sz = read(pfd[1].fd, buf, sizeof(buf));
+ send_sideband(1, 2, buf, sz, DEFAULT_PACKET_MAX);
+ }
+ if (((pfd[0].revents | pfd[1].revents) & POLLHUP) == 0)
+ continue;
+ /* did it die? */
+ pid = waitpid(writer, &status, WNOHANG);
+ if (!pid) {
+ fprintf(stderr, "Hmph, HUP?\n");
+ continue;
+ }
+ if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
+ send_sideband(1, 3, deadchild, strlen(deadchild),
+ DEFAULT_PACKET_MAX);
+ packet_flush(1);
+ break;
+ }
+ return 0;
+}
--
1.4.2.gc52f
^ permalink raw reply related
* [PATCH] Move sideband server side support into reusable form.
From: Junio C Hamano @ 2006-09-10 10:37 UTC (permalink / raw)
To: Franck Bui-Huu; +Cc: git, Rene Scharfe
In-Reply-To: <7vk64ctctv.fsf@assigned-by-dhcp.cox.net>
The server side support; this is just the very low level, and the
caller needs to know which band it wants to send things out.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
* With the previous downloader-side support, this should make
it easier to add the status notification from git-archive
--remote over git native transport (aka git-daemon).
sideband.c | 26 ++++++++++++++++++++++++++
sideband.h | 1 +
upload-pack.c | 50 +++++++++++++-------------------------------------
3 files changed, 40 insertions(+), 37 deletions(-)
diff --git a/sideband.c b/sideband.c
index 861f621..1b14ff8 100644
--- a/sideband.c
+++ b/sideband.c
@@ -46,3 +46,29 @@ int recv_sideband(const char *me, int in
}
return 0;
}
+
+/*
+ * fd is connected to the remote side; send the sideband data
+ * over multiplexed packet stream.
+ */
+ssize_t send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_max)
+{
+ ssize_t ssz = sz;
+ const char *p = data;
+
+ while (sz) {
+ unsigned n;
+ char hdr[5];
+
+ n = sz;
+ if (packet_max - 5 < n)
+ n = packet_max - 5;
+ sprintf(hdr, "%04x", n + 5);
+ hdr[4] = band;
+ safe_write(fd, hdr, 5);
+ safe_write(fd, p, n);
+ p += n;
+ sz -= n;
+ }
+ return ssz;
+}
diff --git a/sideband.h b/sideband.h
index 90b3855..c645cf2 100644
--- a/sideband.h
+++ b/sideband.h
@@ -7,5 +7,6 @@ #define SIDEBAND_REMOTE_ERROR -1
#define DEFAULT_PACKET_MAX 1000
int recv_sideband(const char *me, int in_stream, int out, int err, char *, int);
+ssize_t send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_max);
#endif
diff --git a/upload-pack.c b/upload-pack.c
index 51ce936..1f2f7f7 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -4,6 +4,7 @@ #include <sys/poll.h>
#include "cache.h"
#include "refs.h"
#include "pkt-line.h"
+#include "sideband.h"
#include "tag.h"
#include "object.h"
#include "commit.h"
@@ -33,45 +34,19 @@ static int strip(char *line, int len)
return len;
}
-#define PACKET_MAX 1000
static ssize_t send_client_data(int fd, const char *data, ssize_t sz)
{
- ssize_t ssz;
- const char *p;
-
- if (!data) {
- if (!use_sideband)
- return 0;
- packet_flush(1);
- }
-
- if (!use_sideband) {
- if (fd == 3)
- /* emergency quit */
- fd = 2;
- if (fd == 2) {
- xwrite(fd, data, sz);
- return sz;
- }
- return safe_write(fd, data, sz);
- }
- p = data;
- ssz = sz;
- while (sz) {
- unsigned n;
- char hdr[5];
-
- n = sz;
- if (PACKET_MAX - 5 < n)
- n = PACKET_MAX - 5;
- sprintf(hdr, "%04x", n + 5);
- hdr[4] = fd;
- safe_write(1, hdr, 5);
- safe_write(1, p, n);
- p += n;
- sz -= n;
+ if (use_sideband)
+ return send_sideband(1, fd, data, sz, DEFAULT_PACKET_MAX);
+
+ if (fd == 3)
+ /* emergency quit */
+ fd = 2;
+ if (fd == 2) {
+ xwrite(fd, data, sz);
+ return sz;
}
- return ssz;
+ return safe_write(fd, data, sz);
}
static void create_pack_file(void)
@@ -308,7 +283,8 @@ static void create_pack_file(void)
goto fail;
fprintf(stderr, "flushed.\n");
}
- send_client_data(1, NULL, 0);
+ if (use_sideband)
+ packet_flush(1);
return;
}
fail:
--
1.4.2.gc52f
^ permalink raw reply related
* [PATCH 1/3] Move sideband client side support into reusable form.
From: Junio C Hamano @ 2006-09-10 10:36 UTC (permalink / raw)
To: Franck Bui-Huu; +Cc: git, Rene Scharfe
In-Reply-To: <7vk64ctctv.fsf@assigned-by-dhcp.cox.net>
This moves the receiver side of the sideband support from
fetch-clone.c to sideband.c and its header file, so that
archiver protocol can use it.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
* The next one refactors the upload side.
Makefile | 4 ++--
fetch-clone.c | 32 +++++---------------------------
sideband.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
sideband.h | 11 +++++++++++
4 files changed, 66 insertions(+), 29 deletions(-)
diff --git a/Makefile b/Makefile
index 4ac85fd..c724b48 100644
--- a/Makefile
+++ b/Makefile
@@ -233,7 +233,7 @@ XDIFF_LIB=xdiff/lib.a
LIB_H = \
archive.h blob.h cache.h commit.h csum-file.h delta.h \
- diff.h object.h pack.h pkt-line.h quote.h refs.h \
+ diff.h object.h pack.h pkt-line.h quote.h refs.h sideband.h \
run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h
@@ -245,7 +245,7 @@ DIFF_OBJS = \
LIB_OBJS = \
blob.o commit.o connect.o csum-file.o cache-tree.o base85.o \
date.o diff-delta.o entry.o exec_cmd.o ident.o lockfile.o \
- object.o pack-check.o patch-delta.o path.o pkt-line.o \
+ object.o pack-check.o patch-delta.o path.o pkt-line.o sideband.o \
quote.o read-cache.o refs.o run-command.o dir.o object-refs.o \
server-info.o setup.o sha1_file.o sha1_name.o strbuf.o \
tag.o tree.o usage.o config.o environment.o ctype.o copy.o \
diff --git a/fetch-clone.c b/fetch-clone.c
index c5cf477..b62feac 100644
--- a/fetch-clone.c
+++ b/fetch-clone.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "exec_cmd.h"
#include "pkt-line.h"
+#include "sideband.h"
#include <sys/wait.h>
#include <sys/time.h>
@@ -114,36 +115,13 @@ static pid_t setup_sideband(int sideband
die("%s: unable to fork off sideband demultiplexer", me);
if (!side_pid) {
/* subprocess */
+ char buf[DEFAULT_PACKET_MAX];
+
close(fd[0]);
if (xd[0] != xd[1])
close(xd[1]);
- while (1) {
- char buf[1024];
- int len = packet_read_line(xd[0], buf, sizeof(buf));
- if (len == 0)
- break;
- if (len < 1)
- die("%s: protocol error: no band designator",
- me);
- len--;
- switch (buf[0] & 0xFF) {
- case 3:
- safe_write(2, "remote: ", 8);
- safe_write(2, buf+1, len);
- safe_write(2, "\n", 1);
- exit(1);
- case 2:
- safe_write(2, "remote: ", 8);
- safe_write(2, buf+1, len);
- continue;
- case 1:
- safe_write(fd[1], buf+1, len);
- continue;
- default:
- die("%s: protocol error: bad band #%d",
- me, (buf[0] & 0xFF));
- }
- }
+ if (recv_sideband(me, xd[0], fd[1], 2, buf, sizeof(buf)))
+ exit(1);
exit(0);
}
close(xd[0]);
diff --git a/sideband.c b/sideband.c
new file mode 100644
index 0000000..861f621
--- /dev/null
+++ b/sideband.c
@@ -0,0 +1,48 @@
+#include "pkt-line.h"
+#include "sideband.h"
+
+/*
+ * Receive multiplexed output stream over git native protocol.
+ * in_stream is the input stream from the remote, which carries data
+ * in pkt_line format with band designator. Demultiplex it into out
+ * and err and return error appropriately. Band #1 carries the
+ * primary payload. Things coming over band #2 is not necessarily
+ * error; they are usually informative message on the standard error
+ * stream, aka "verbose"). A message over band #3 is a signal that
+ * the remote died unexpectedly. A flush() concludes the stream.
+ */
+int recv_sideband(const char *me, int in_stream, int out, int err, char *buf, int bufsz)
+{
+ while (1) {
+ int len = packet_read_line(in_stream, buf, bufsz);
+ if (len == 0)
+ break;
+ if (len < 1) {
+ len = sprintf(buf, "%s: protocol error: no band designator\n", me);
+ safe_write(err, buf, len);
+ return SIDEBAND_PROTOCOL_ERROR;
+ }
+ len--;
+ switch (buf[0] & 0xFF) {
+ case 3:
+ safe_write(err, "remote: ", 8);
+ safe_write(err, buf+1, len);
+ safe_write(err, "\n", 1);
+ return SIDEBAND_REMOTE_ERROR;
+ case 2:
+ safe_write(err, "remote: ", 8);
+ safe_write(err, buf+1, len);
+ continue;
+ case 1:
+ safe_write(out, buf+1, len);
+ continue;
+ default:
+ len = sprintf(buf + 1,
+ "%s: protocol error: bad band #%d\n",
+ me, buf[0] & 0xFF);
+ safe_write(err, buf+1, len);
+ return SIDEBAND_PROTOCOL_ERROR;
+ }
+ }
+ return 0;
+}
diff --git a/sideband.h b/sideband.h
new file mode 100644
index 0000000..90b3855
--- /dev/null
+++ b/sideband.h
@@ -0,0 +1,11 @@
+#ifndef SIDEBAND_H
+#define SIDEBAND_H
+
+#define SIDEBAND_PROTOCOL_ERROR -2
+#define SIDEBAND_REMOTE_ERROR -1
+
+#define DEFAULT_PACKET_MAX 1000
+
+int recv_sideband(const char *me, int in_stream, int out, int err, char *, int);
+
+#endif
--
1.4.2.gc52f
^ permalink raw reply related
* Re: Change set based shallow clone
From: Josef Weidendorfer @ 2006-09-10 10:28 UTC (permalink / raw)
To: Junio C Hamano
Cc: Marco Costalba, Linus Torvalds, Paul Mackerras, Jon Smirl,
linux@horizon.com, Git Mailing List
In-Reply-To: <7virjwuxrz.fsf@assigned-by-dhcp.cox.net>
On Sunday 10 September 2006 06:54, Junio C Hamano wrote:
> I am starting to suspect that introducing "generation" header to
> the commit objects might actually be a very good thing. For
> one thing, rev-list will automatically get the topology always
> right if we did so.
I do not think that any commits need to be changed, but only
"rev-list --topo-order" to build its own private cache of
sequence numbers for commits.
AFAICS, a generation number is pure redundant information, as a single
run of "rev-list --topo-order" can recalculate it. If it can not find
the number in its private per-repository cache of sequence numbers,
it calculates it the hard way (same as currently) and puts the found
numbers into the cache afterwards.
The cache should be similar in size to a pack index of a fully packed repository,
but probably could be way smaller, by storing only one sequence number for
a linear sequence of commits with only one parent each.
Josef
^ permalink raw reply
* Re: Change set based shallow clone
From: Jakub Narebski @ 2006-09-10 9:49 UTC (permalink / raw)
To: git
In-Reply-To: <7virjwuxrz.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> I am starting to suspect that introducing "generation" header to
> the commit objects might actually be a very good thing. For
> one thing, rev-list will automatically get the topology always
> right if we did so.
>
> We obviously need to update 'convert-objects' and tell everybody
> that they need to rewrite their history if we take this route.
> That kind of flag-day conversion used to be an Ok thing to do,
> but it is getting harder and harder these days for obvious
> reasons, though.
Wouldn't it be possible to have not that more complex code if some
of the commit objects (newer) would have "generation" header, and some
of them (older) wouldn't have it? Git would use generation header if it is
present, and current heuristic timestamp based code if it is not present.
It would be better of course if the new commits have correct "generation"
header, so insertion of "new" commit after "old" commit would have some
extra overhead...
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: Some issues with current qgit on exit ( aka "Crash this!" )
From: Pavel Roskin @ 2006-09-10 7:39 UTC (permalink / raw)
To: Marco Costalba; +Cc: git
In-Reply-To: <e5bfff550609090116waa5d4a1y7de1132fd82f08cf@mail.gmail.com>
On Sat, 2006-09-09 at 10:16 +0200, Marco Costalba wrote:
> It was a little regression from recent changes. Luckily much more easy
> then previous one.
> Logic fixed and patch pushed.
qgit is working perfectly now. I'm glad to see qgit 1.5 released.
Thank you very much!
--
Regards,
Pavel Roskin
^ permalink raw reply
* [PATCH 2/2] Add --verbose to git-archive
From: Junio C Hamano @ 2006-09-10 7:12 UTC (permalink / raw)
To: Franck Bui-Huu; +Cc: git, Rene Scharfe
In-Reply-To: <7vpse4tcyc.fsf@assigned-by-dhcp.cox.net>
And teach backends about it.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
* This is not interesting yet, and the output is discarded if
you are running the remote archiver against git-daemon, but
would be a useful progress indicator when we implement the
side-band in git-archive protocol, which is probably a
requirement before this "git-archive" series can graduate to
"master" branch.
archive.h | 1 +
builtin-archive.c | 8 +++++++-
builtin-tar-tree.c | 4 ++++
builtin-zip-tree.c | 4 ++++
4 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/archive.h b/archive.h
index e0782b9..16dcdb8 100644
--- a/archive.h
+++ b/archive.h
@@ -10,6 +10,7 @@ struct archiver_args {
const unsigned char *commit_sha1;
time_t time;
const char **pathspec;
+ unsigned int verbose : 1;
void *extra;
};
diff --git a/builtin-archive.c b/builtin-archive.c
index c70488c..b78d6d8 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -12,7 +12,7 @@ #include "exec_cmd.h"
#include "pkt-line.h"
static const char archive_usage[] = \
-"git-archive --format=<fmt> [--prefix=<prefix>/] [<extra>] <tree-ish> [path...]";
+"git-archive --format=<fmt> [--prefix=<prefix>/] [--verbose] [<extra>] <tree-ish> [path...]";
struct archiver archivers[] = {
{
@@ -148,6 +148,7 @@ int parse_archive_args(int argc, const c
int extra_argc = 0;
const char *format = NULL; /* might want to default to "tar" */
const char *base = "";
+ int verbose = 0;
int i;
for (i = 1; i < argc; i++) {
@@ -158,6 +159,10 @@ int parse_archive_args(int argc, const c
printf("%s\n", archivers[i].name);
exit(0);
}
+ if (!strcmp(arg, "--verbose") || !strcmp(arg, "-v")) {
+ verbose = 1;
+ continue;
+ }
if (!strncmp(arg, "--format=", 9)) {
format = arg + 9;
continue;
@@ -192,6 +197,7 @@ int parse_archive_args(int argc, const c
die("%s", default_parse_extra(ar, extra_argv));
ar->args.extra = ar->parse_extra(extra_argc, extra_argv);
}
+ ar->args.verbose = verbose;
ar->args.base = base;
return i;
diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index c20eb0e..fae2c0b 100644
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -22,6 +22,7 @@ static unsigned long offset;
static time_t archive_time;
static int tar_umask;
+static int verbose;
/* writes out the whole block, but only if it is full */
static void write_if_needed(void)
@@ -169,6 +170,8 @@ static void write_entry(const unsigned c
mode = 0100666;
sprintf(header.name, "%s.paxheader", sha1_to_hex(sha1));
} else {
+ if (verbose)
+ fprintf(stderr, "%.*s\n", path->len, path->buf);
if (S_ISDIR(mode)) {
*header.typeflag = TYPEFLAG_DIR;
mode = (mode | 0777) & ~tar_umask;
@@ -385,6 +388,7 @@ int write_tar_archive(struct archiver_ar
git_config(git_tar_config);
archive_time = args->time;
+ verbose = args->verbose;
if (args->commit_sha1)
write_global_extended_header(args->commit_sha1);
diff --git a/builtin-zip-tree.c b/builtin-zip-tree.c
index 4e79633..0ebd547 100644
--- a/builtin-zip-tree.c
+++ b/builtin-zip-tree.c
@@ -13,6 +13,7 @@ #include "archive.h"
static const char zip_tree_usage[] =
"git-zip-tree [-0|...|-9] <tree-ish> [ <base> ]";
+static int verbose;
static int zip_date;
static int zip_time;
@@ -164,6 +165,8 @@ static int write_zip_entry(const unsigne
crc = crc32(0, Z_NULL, 0);
path = construct_path(base, baselen, filename, S_ISDIR(mode), &pathlen);
+ if (verbose)
+ fprintf(stderr, "%s\n", path);
if (pathlen > 0xffff) {
error("path too long (%d chars, SHA1: %s): %s", pathlen,
sha1_to_hex(sha1), path);
@@ -361,6 +364,7 @@ int write_zip_archive(struct archiver_ar
zip_dir = xmalloc(ZIP_DIRECTORY_MIN_SIZE);
zip_dir_size = ZIP_DIRECTORY_MIN_SIZE;
+ verbose = args->verbose;
if (args->base && plen > 0 && args->base[plen - 1] == '/') {
char *base = strdup(args->base);
--
1.4.2.gc52f
^ permalink raw reply related
* [PATCH 1/2] archive: allow remote to have more formats than we understand.
From: Junio C Hamano @ 2006-09-10 7:09 UTC (permalink / raw)
To: Franck Bui-Huu; +Cc: git, Rene Scharfe
This fixes git-archive --remote not to parse archiver arguments;
otherwise if the remote end implements formats other than the
one known locally we will not be able to access that format.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
* At first sight, this should not matter that much, but (1) we
do not really parse and validate the arguments when dealing
with remote site, and (2) we have no way validating them if
we wanted to, given that the remote end might be running
different version of git.
Having said that, you would realize that once we start
refactoring things this way, "git archive --remote=foo" is
not archive driver anymore. There is nothing that prevents
us saying "git archive --remote=foo --command=rev-list HEAD",
other than that the remote archive protocol insists the
command invoked at the remote end to be "git archive" itself.
archive.h | 1 -
builtin-archive.c | 79 ++++++++++++++++++++++++++++++++---------------------
2 files changed, 47 insertions(+), 33 deletions(-)
diff --git a/archive.h b/archive.h
index d8cca73..e0782b9 100644
--- a/archive.h
+++ b/archive.h
@@ -19,7 +19,6 @@ typedef void *(*parse_extra_args_fn_t)(i
struct archiver {
const char *name;
- const char *remote;
struct archiver_args args;
write_archive_fn_t write_archive;
parse_extra_args_fn_t parse_extra;
diff --git a/builtin-archive.c b/builtin-archive.c
index b944737..c70488c 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -26,7 +26,7 @@ struct archiver archivers[] = {
},
};
-static int run_remote_archiver(struct archiver *ar, int argc,
+static int run_remote_archiver(const char *remote, int argc,
const char **argv)
{
char *url, buf[1024];
@@ -35,16 +35,13 @@ static int run_remote_archiver(struct ar
sprintf(buf, "git-upload-archive");
- url = xstrdup(ar->remote);
+ url = xstrdup(remote);
pid = git_connect(fd, url, buf);
if (pid < 0)
return pid;
- for (i = 1; i < argc; i++) {
- if (!strncmp(argv[i], "--remote=", 9))
- continue;
+ for (i = 1; i < argc; i++)
packet_write(fd[1], "argument %s\n", argv[i]);
- }
packet_flush(fd[1]);
len = packet_read_line(fd[0], buf, sizeof(buf));
@@ -150,17 +147,16 @@ int parse_archive_args(int argc, const c
const char *extra_argv[MAX_EXTRA_ARGS];
int extra_argc = 0;
const char *format = NULL; /* might want to default to "tar" */
- const char *remote = NULL;
const char *base = "";
- int list = 0;
int i;
for (i = 1; i < argc; i++) {
const char *arg = argv[i];
if (!strcmp(arg, "--list") || !strcmp(arg, "-l")) {
- list = 1;
- continue;
+ for (i = 0; i < ARRAY_SIZE(archivers); i++)
+ printf("%s\n", archivers[i].name);
+ exit(0);
}
if (!strncmp(arg, "--format=", 9)) {
format = arg + 9;
@@ -170,10 +166,6 @@ int parse_archive_args(int argc, const c
base = arg + 9;
continue;
}
- if (!strncmp(arg, "--remote=", 9)) {
- remote = arg + 9;
- continue;
- }
if (!strcmp(arg, "--")) {
i++;
break;
@@ -187,44 +179,67 @@ int parse_archive_args(int argc, const c
break;
}
- if (list) {
- if (!remote) {
- for (i = 0; i < ARRAY_SIZE(archivers); i++)
- printf("%s\n", archivers[i].name);
- exit(0);
- }
- die("--list and --remote are mutually exclusive");
- }
-
- if (argc - i < 1)
+ /* We need at least one parameter -- tree-ish */
+ if (argc - 1 < i)
usage(archive_usage);
if (!format)
die("You must specify an archive format");
if (init_archiver(format, ar) < 0)
die("Unknown archive format '%s'", format);
- if (extra_argc && !remote) {
- if (!ar->parse_extra) {
+ if (extra_argc) {
+ if (!ar->parse_extra)
die("%s", default_parse_extra(ar, extra_argv));
- }
ar->args.extra = ar->parse_extra(extra_argc, extra_argv);
}
- ar->remote = remote;
ar->args.base = base;
return i;
}
+static const char *remote_request(int *ac, const char **av)
+{
+ int ix, iy, cnt = *ac;
+ int no_more_options = 0;
+ const char *remote = NULL;
+
+ for (ix = iy = 1; ix < cnt; ix++) {
+ const char *arg = av[ix];
+ if (!strcmp(arg, "--"))
+ no_more_options = 1;
+ if (!no_more_options) {
+ if (!strncmp(arg, "--remote=", 9)) {
+ if (remote)
+ die("Multiple --remote specified");
+ remote = arg + 9;
+ continue;
+ }
+ if (arg[0] != '-')
+ no_more_options = 1;
+ }
+ if (ix != iy)
+ av[iy] = arg;
+ iy++;
+ }
+ if (remote) {
+ av[--cnt] = NULL;
+ *ac = cnt;
+ }
+ return remote;
+}
+
int cmd_archive(int argc, const char **argv, const char *prefix)
{
struct archiver ar;
int tree_idx;
+ const char *remote = NULL;
- tree_idx = parse_archive_args(argc, argv, &ar);
-
- if (ar.remote)
- return run_remote_archiver(&ar, argc, argv);
+ remote = remote_request(&argc, argv);
+ if (remote)
+ return run_remote_archiver(remote, argc, argv);
+ memset(&ar, 0, sizeof(ar));
+ tree_idx = parse_archive_args(argc, argv, &ar);
if (prefix == NULL)
prefix = setup_git_directory();
--
1.4.2.gc52f
^ permalink raw reply related
* Re: Change set based shallow clone
From: Junio C Hamano @ 2006-09-10 5:46 UTC (permalink / raw)
To: git
In-Reply-To: <e5bfff550609092214t4f8e195eib28e302f4d284aa@mail.gmail.com>
"Marco Costalba" <mcostalba@gmail.com> writes:
> On 9/10/06, Junio C Hamano <junkio@cox.net> wrote:
>> "Marco Costalba" <mcostalba@gmail.com> writes:
>>
>> > >>
>> > >> A <--- tip of branch
>> > >> / \
>> > >> B E
>> > >> | |
>> > >> | F
>> > >> | /
>> > >> C
>> > >> |
>> > >> D
>> > >> ...
>> > >>
>> >
>> > Ok. What about something like this?
>> > A, B, C, D, E, (-3, 1)F
>> >
>> > where -3 is the correct position in sequence and 1 is the number of
>> > revisions before F to whom apply the -3 rule.
>>
>> That means F knows who its descendants are, and commit objects
>> do not have that information, so while traversing you need to
>> keep track of all the descendants yourself, doesn't it?
>>
>
> Yes! it is, but you do it in git instead of in the visualizer ;-)
> because I think in any case someone defenitly needs to keep track of
> it.
Not really.
To git-rev-list, which does not know how the visualizer uses the
data, what you are saying essentially means that it needs to
hold onto the data it parsed forever. That's where my earlier
suggestion for visualizers to take advantage of the "windowing"
information comes from. Since the chain depicted in the above
between E..F can be arbitrary long (and you would need to keep
track of information for B and C too, well, essentially
everything), that is unacceptable for rev-list if you do not
give it additional clue when it can discard that information.
^ permalink raw reply
* Re: Change set based shallow clone
From: Marco Costalba @ 2006-09-10 5:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: Linus Torvalds, Paul Mackerras, Jon Smirl, linux@horizon.com,
Git Mailing List
In-Reply-To: <7virjwuxrz.fsf@assigned-by-dhcp.cox.net>
On 9/10/06, Junio C Hamano <junkio@cox.net> wrote:
> "Marco Costalba" <mcostalba@gmail.com> writes:
>
> > >>
> > >> A <--- tip of branch
> > >> / \
> > >> B E
> > >> | |
> > >> | F
> > >> | /
> > >> C
> > >> |
> > >> D
> > >> ...
> > >>
> >
> > Ok. What about something like this?
> > A, B, C, D, E, (-3, 1)F
> >
> > where -3 is the correct position in sequence and 1 is the number of
> > revisions before F to whom apply the -3 rule.
>
> That means F knows who its descendants are, and commit objects
> do not have that information, so while traversing you need to
> keep track of all the descendants yourself, doesn't it?
>
Yes! it is, but you do it in git instead of in the visualizer ;-)
because I think in any case someone defenitly needs to keep track of
it.
> And how does that fix-up information help the user of the stream
> anyway? If I understand your model correctly, the model does
> not synchronously draw nodes as they are received,
Visualizers draw only what is on screen so when you start them they
draw the first 20/30 lines only. And noramally git output it's faster
then user scrolling so when user scrolls down revisions are already
arrived.
> track of what it has seen so far. When it sees F it can notice
> that its parent, C, is something it has seen, so it can tell F
> is wrong. It also knows that it has seen E and E's parent is F
> (which turned out to be at a wrong spot), so it can suspect E is
> also in the wrong spot (now, it is fuzzy to me how that chain
> of suspicion ends at E and does not propagate up to A, but let's
> think about that later).
>
Is it possible, in that case flicker will be involved, but it is very
uncommon, so in the big amount of all other cases we have no
flickering and early graph drawing.
Becasue the worst that can happen on visualizer side is flickering we
can accept a worst rare case to have a big adavantage (no latency and
no flickering) on the (very) common case: "user scrolls to a given
page when the corresponding revisions are already arrived and, in
case, fixed up".
Marco
^ permalink raw reply
* Re: Change set based shallow clone
From: Junio C Hamano @ 2006-09-10 4:54 UTC (permalink / raw)
To: Marco Costalba
Cc: Linus Torvalds, Paul Mackerras, Jon Smirl, linux@horizon.com,
Git Mailing List
In-Reply-To: <e5bfff550609092123t1d8b6c70s5750fbb787534812@mail.gmail.com>
"Marco Costalba" <mcostalba@gmail.com> writes:
> >>
> >> A <--- tip of branch
> >> / \
> >> B E
> >> | |
> >> | F
> >> | /
> >> C
> >> |
> >> D
> >> ...
> >>
>
> Ok. What about something like this?
> A, B, C, D, E, (-3, 1)F
>
> where -3 is the correct position in sequence and 1 is the number of
> revisions before F to whom apply the -3 rule.
That means F knows who its descendants are, and commit objects
do not have that information, so while traversing you need to
keep track of all the descendants yourself, doesn't it?
And how does that fix-up information help the user of the stream
anyway? If I understand your model correctly, the model does
not synchronously draw nodes as they are received, so it keeps
track of what it has seen so far. When it sees F it can notice
that its parent, C, is something it has seen, so it can tell F
is wrong. It also knows that it has seen E and E's parent is F
(which turned out to be at a wrong spot), so it can suspect E is
also in the wrong spot (now, it is fuzzy to me how that chain
of suspicion ends at E and does not propagate up to A, but let's
think about that later).
There is one thing that the user knows a lot better than the
generic rev-list output part. It is the size of the output
window (how tall the window is). I wonder if there is a way to
exploit it, either in the user, or telling the size of the
window (and perhaps where the display region at the top begins
at) to rev-list...
If we are dealing in a 3-item-tall window, we will traverse A B
C D, notice they form a single strand of pearls, and can make a
mental note that we do not have to worry about ancestors of D
for now, because D's ancestors will be drawn below C, which is
already the putative bottom edge of the window (when oddballs
like E and F comes later, they can only push things down at the
bottom edge of the window). Then rev-list can postpone
traversing D and go on to traverse other nodes that are in the
"active" list (in this case, E).
I am starting to suspect that introducing "generation" header to
the commit objects might actually be a very good thing. For
one thing, rev-list will automatically get the topology always
right if we did so.
We obviously need to update 'convert-objects' and tell everybody
that they need to rewrite their history if we take this route.
That kind of flag-day conversion used to be an Ok thing to do,
but it is getting harder and harder these days for obvious
reasons, though.
^ permalink raw reply
* Re: Change set based shallow clone
From: Marco Costalba @ 2006-09-10 4:46 UTC (permalink / raw)
To: Junio C Hamano
Cc: Linus Torvalds, Paul Mackerras, Jon Smirl, linux@horizon.com,
Git Mailing List
In-Reply-To: <e5bfff550609092123t1d8b6c70s5750fbb787534812@mail.gmail.com>
On 9/10/06, Marco Costalba <mcostalba@gmail.com> wrote:
> On 9/10/06, Junio C Hamano <junkio@cox.net> wrote:
> > "Marco Costalba" <mcostalba@gmail.com> writes:
> >
> > > On 9/9/06, Linus Torvalds <torvalds@osdl.org> wrote:
> > >>
> > >> The example is
> > >>
> > >> A <--- tip of branch
> > >> / \
> > >> B E
> > >> | |
> > >> | F
> > >> | /
> > >> C
> > >> |
> > >> D
> > >> ...
> > >>
> > >
> > > Ok now it' clear, thanks. But anyhow I think that it should be
> > > possible to avoid the check and reordering on the receiver side.
> > >
> > > Suppose for a moment to split the graph drawing from the sequence
> > > reordering problem, suppose for a moment that receiver does do not
> > > draw the graph immediately.
> > >
> > > As you described, in our case git-rev-list sends the following sequence:
> > > A, B, C, D, E, F
> > >
> > > instead git-rev-list --topo-order would have sent something like
> > > A, E, F, B, C, D
> > >
> > > Now I ask, is it possible to have a sequence (without latency) like
> > > A, B, C, D, (-3)E, (-3)F
> > >
> > > where, in case of not topological correct revisions, git-rev-list
> > > gives the hint on the correct position in sequence (3 revs before in
> > > our case) where the revision would have been if the sequence would
> > > have been --topo-order ?
> >
> > When rev-list is writing E out, it does not know it is a
> > descendant of something it already emitted (i.e. C) because it
> > hasn't looked at F nor checked its parent yet. So asking for
> > (-3)F may be fine but I think (-3)E is just a fantasy.
> >
> Ok. What about something like this?
> A, B, C, D, E, (-3, 1)F
>
> where -3 is the correct position in sequence and 1 is the number of
> revisions before F to whom apply the -3 rule.
>
Suppose git-rev-list gives:
A, B, H, C, D, E, F, G, I, L, M, N
Suppose git-rev-list --topo-order would have sent
A, B, C, D, E, F, G, H, I, L, M, N
Perhaps a more general fixup syntax could be:
A,B, H, C, D, E, F, G, I(6, C, D, E, F, G, H), L, M, N
Where 6 is the number of previous revisions to rearrange with the
corresponding correct sequence(C, D, E, F, G, H).
Note that the same subset could be rearranged multiple times if it
helps git-rev-list parsing, as example it should be possible to have
something like:
A,B, H, C, D, E, F, G, I(6, C, D, E, F, G, H), L, M(3, L, G, H), N
Marco
^ permalink raw reply
* Re: Change set based shallow clone
From: Marco Costalba @ 2006-09-10 4:23 UTC (permalink / raw)
To: Junio C Hamano
Cc: Linus Torvalds, Paul Mackerras, Jon Smirl, linux@horizon.com,
Git Mailing List
In-Reply-To: <7vpse4uzos.fsf@assigned-by-dhcp.cox.net>
On 9/10/06, Junio C Hamano <junkio@cox.net> wrote:
> "Marco Costalba" <mcostalba@gmail.com> writes:
>
> > On 9/9/06, Linus Torvalds <torvalds@osdl.org> wrote:
> >>
> >> The example is
> >>
> >> A <--- tip of branch
> >> / \
> >> B E
> >> | |
> >> | F
> >> | /
> >> C
> >> |
> >> D
> >> ...
> >>
> >
> > Ok now it' clear, thanks. But anyhow I think that it should be
> > possible to avoid the check and reordering on the receiver side.
> >
> > Suppose for a moment to split the graph drawing from the sequence
> > reordering problem, suppose for a moment that receiver does do not
> > draw the graph immediately.
> >
> > As you described, in our case git-rev-list sends the following sequence:
> > A, B, C, D, E, F
> >
> > instead git-rev-list --topo-order would have sent something like
> > A, E, F, B, C, D
> >
> > Now I ask, is it possible to have a sequence (without latency) like
> > A, B, C, D, (-3)E, (-3)F
> >
> > where, in case of not topological correct revisions, git-rev-list
> > gives the hint on the correct position in sequence (3 revs before in
> > our case) where the revision would have been if the sequence would
> > have been --topo-order ?
>
> When rev-list is writing E out, it does not know it is a
> descendant of something it already emitted (i.e. C) because it
> hasn't looked at F nor checked its parent yet. So asking for
> (-3)F may be fine but I think (-3)E is just a fantasy.
>
Ok. What about something like this?
A, B, C, D, E, (-3, 1)F
where -3 is the correct position in sequence and 1 is the number of
revisions before F to whom apply the -3 rule.
>
^ permalink raw reply
* Re: Change set based shallow clone
From: Junio C Hamano @ 2006-09-10 4:13 UTC (permalink / raw)
To: Marco Costalba
Cc: Linus Torvalds, Paul Mackerras, Jon Smirl, linux@horizon.com,
Git Mailing List
In-Reply-To: <e5bfff550609092049t5e016cacr2502ce81bbb6489e@mail.gmail.com>
"Marco Costalba" <mcostalba@gmail.com> writes:
> On 9/9/06, Linus Torvalds <torvalds@osdl.org> wrote:
>>
>> The example is
>>
>> A <--- tip of branch
>> / \
>> B E
>> | |
>> | F
>> | /
>> C
>> |
>> D
>> ...
>>
>
> Ok now it' clear, thanks. But anyhow I think that it should be
> possible to avoid the check and reordering on the receiver side.
>
> Suppose for a moment to split the graph drawing from the sequence
> reordering problem, suppose for a moment that receiver does do not
> draw the graph immediately.
>
> As you described, in our case git-rev-list sends the following sequence:
> A, B, C, D, E, F
>
> instead git-rev-list --topo-order would have sent something like
> A, E, F, B, C, D
>
> Now I ask, is it possible to have a sequence (without latency) like
> A, B, C, D, (-3)E, (-3)F
>
> where, in case of not topological correct revisions, git-rev-list
> gives the hint on the correct position in sequence (3 revs before in
> our case) where the revision would have been if the sequence would
> have been --topo-order ?
When rev-list is writing E out, it does not know it is a
descendant of something it already emitted (i.e. C) because it
hasn't looked at F nor checked its parent yet. So asking for
(-3)F may be fine but I think (-3)E is just a fantasy.
^ permalink raw reply
* Re: Change set based shallow clone
From: Marco Costalba @ 2006-09-10 3:49 UTC (permalink / raw)
To: Linus Torvalds
Cc: Paul Mackerras, Jon Smirl, linux@horizon.com, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0609091256110.27779@g5.osdl.org>
On 9/9/06, Linus Torvalds <torvalds@osdl.org> wrote:
>
>
>
> The example is
>
> A <--- tip of branch
> / \
> B E
> | |
> | F
> | /
> C
> |
> D
> ...
>
Ok now it' clear, thanks. But anyhow I think that it should be
possible to avoid the check and reordering on the receiver side.
Suppose for a moment to split the graph drawing from the sequence
reordering problem, suppose for a moment that receiver does do not
draw the graph immediately.
As you described, in our case git-rev-list sends the following sequence:
A, B, C, D, E, F
instead git-rev-list --topo-order would have sent something like
A, E, F, B, C, D
Now I ask, is it possible to have a sequence (without latency) like
A, B, C, D, (-3)E, (-3)F
where, in case of not topological correct revisions, git-rev-list
gives the hint on the correct position in sequence (3 revs before in
our case) where the revision would have been if the sequence would
have been --topo-order ?
This saves all the checking and reordering on the receiver side and
guarantees consistent results on different implementations of git
visualizers because the --topo-order algorithm logic is computed in
git rev-list only.
The visualizers could be sequence order agnostic, i.e. receivers can
handle --topo-order or --date-order sequences or any other kind of
sequence with the same code, simply they recreate the sequence in the
way git-rev-list tells them.
Marco
^ permalink raw reply
* Re: Change set based shallow clone
From: Junio C Hamano @ 2006-09-10 1:22 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0609091708340.27779@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> But hey, maybe somebody can come up with some operation that becomes
> _much_ cheaper with the above kind of sequence number. It shouldn't be
> hard to test..
Sometimes I get a feeling that we are saying the same thing in
the same thread but in parallel without reading what the other
is saying. This is such a thread ;-).
^ permalink raw reply
* Re: {RFC/PATCH] micro-optimize get_sha1_hex()
From: Linus Torvalds @ 2006-09-10 0:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0609091639110.27779@g5.osdl.org>
On Sat, 9 Sep 2006, Linus Torvalds wrote:
>
> I _suspect_ that you profiled using "gprof" and the "-pg" flag to
> the compiler?
Btw, in the absense of oprofile (which is very useful, and has the
advantage that you can run many programs multiple times and get a
"combined data" output from it all), what you can do (and which is more
reliable) is:
- the "minor pagefault" count that /usr/bin/time prints out is very
useful. It gives a pretty much direct view into what the total memory
use of the program was over its lifetime, in a way that very few other
things do.
- using "gprof" to try to find _potential_ hotspots, but then not
trusting the profiling numbers at all for actual improvement, but
simply recompiling (witout profiling) and timing it for real after any
change. The real timings will almost certainly not match what you
thought you'd get from profiling, but now they'll be real numbers.
- using "-pg" to link in the profiling code, BUT ONLY AT THE FINAL LINK
TIME! This will give you the "% time" and "cumulative seconds" part,
but it will mean that you will _not_ get the call-graph, because now
the actual code generation is not affected, and the compiler won't be
inserting all the call-graph-generation code.
Note that the last use makes gprof much more accurate, but it also means
that it won't _work_ very well for things that are fast. You usually have
a 1/100th of a second profiling setup, so anything that is less than a
second won't have much of a profile. So this only works for longer-running
things.
Linus
^ permalink raw reply
* Re: Change set based shallow clone
From: Linus Torvalds @ 2006-09-10 0:18 UTC (permalink / raw)
To: Jon Smirl
Cc: Jeff King, Marco Costalba, Paul Mackerras, linux@horizon.com,
Git Mailing List
In-Reply-To: <9e4733910609091554m2a8a001axd575defff9a0e7a9@mail.gmail.com>
On Sat, 9 Sep 2006, Jon Smirl wrote:
>
> When a merge happens could git fix things up in the database by adding
> a corrected, hidden time stamp that would keep things from having an
> out of order time sequence? That way you wouldn't need to rediscover
> the out of order commit each time the tree is generated.
I don't think any such algorithm exists, and it's too late now ;)
I was thinking of some "sequence number" algorithm, but the thing is, it's
not _merges_ that are the problem per se, it's the "branch points". And
you don't actually know that a commit is a branch point in advance: it's
a branch point not because you branched, but because somebody else
happened to use the same parent as the basis of their independent work.
So we could make the rule be something like "when we commit, the new
commit must have a datestamp that is equal to or in the future from the
parent commits". However, that results in some really strange problems in
a distributed environment, where _if_ somebody has their clock set wrong,
you'll end up either unable to merge with them, or you have to generate a
totally bogus time yourself (which then moves up the chain).
It might be simpler to then not think of the thing as a "date" at all, but
as a pure sequence number. Then the rule would be: a new commit has to
have a sequence number that is "max(of-the-parent-sequence-numbers) + 1".
That _may_ or may not actually help.
It was discussed (a bit) early on - summer-05 - but I wasn't convinced
enough that it would help that I was willing to change the format and add
that sequence number. I still am not. I'm not even sure it would really
work.
In other words, it's probably just not worth it. It's better to just say
"dates don't really _matter_, and all that matters is the parenthood
information". Which is really what git does - the dates aren't _important_
for any real operation, they are literally just a heuristic.
But hey, maybe somebody can come up with some operation that becomes
_much_ cheaper with the above kind of sequence number. It shouldn't be
hard to test..
Linus
^ permalink raw reply
* Re: {RFC/PATCH] micro-optimize get_sha1_hex()
From: Linus Torvalds @ 2006-09-10 0:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzmd8vh6q.fsf@assigned-by-dhcp.cox.net>
On Sat, 9 Sep 2006, Junio C Hamano wrote:
>
> I was profiling 'git-rev-list v2.16.12..', because I suspected
> insert_by_date() might be expensive (the function inserts into
> singly-linked ordered list, so the data structure has to become
> array based to allow optimization).
I _suspect_ that you profiled using "gprof" and the "-pg" flag to
the compiler?
The thing is, that generates almost totally bogus profiles. It makes
simple function calls look extremely - and unrealistically - expensive,
because the profiling code itself adds lots of overhead, and it then ends
up showing on the tick-based profile too in a way that it wouldn't show
in real life.
It tends to be much better to profile unmodified binaries using
"oprofile", which doesn't end up giving the exact number of calls, but
where the time-based profiling is a _lot_ more reliable.
That said, clearly "hexval()" can be optimized, and it's quite possible
that it would show up even in oprofile data too.
> The attached brings get_sha1_hex() down from 15.19% to 5.41%,
> but I feel we should be able to do better.
I doubt it's 5% in real life, but your optimization looks fine to me.
You can make it even _more_ optimized by not even bothering to test at
each point, but just inlining hexval(), and making it a single array
lookup. The point being that if you get -1 for _either_ of the two
lookups, since we shift them and or them together, the end result will be
invalid, and you only need to _test_ once.
So this patch should generate much better code, where the inner loop is
something like
<get_sha1_hex+16>:
add $0x1,%esi # count of result bytes..
cmp $0x14,%esi # did we have 20 already?
mov %dl,(%ebx) # save the "last loop" result
je 0x807630b <get_sha1_hex+75> # break out
add $0x1,%ebx # update dest pointer (one byte at a time)
add $0x2,%ecx # update source pointer (two hex chars at a time)
movsbl (%ecx),%eax # get first character
movsbl 0x80989e0(%eax),%edx # get the hexval for it
movsbl 0x1(%ecx),%eax # get the second character
shl $0x4,%edx # shift first character hexval up by 4
movsbl 0x80989e0(%eax),%eax # get second character hexval
or %eax,%edx # get the byte value
test $0xffffff00,%edx # was it ok (0x00 - 0xff)?
je <get_sha1_hex+16> # yeah, continue
which should perform very well (just two comparisons - one for the "wrong
bits set" at the end, and one for the 20-byte-counter test at the
beginning). Small and sweet.
In fact, I don't see how you can much improve on that code even if you
were to hand-code it, so gcc actually generates good code once the
"hexval()" function has been written like this.
Not very much tested, but it looks obvious enough.. And it's actually
conceptually a smaller diff than yours (ie it doesn't change anything but
the "hexval()" function, and all the bulk of it is the stupid array
initializer).
Linus
---
diff --git a/sha1_file.c b/sha1_file.c
index 428d791..b64b92d 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -26,15 +26,43 @@ const unsigned char null_sha1[20];
static unsigned int sha1_file_open_flag = O_NOATIME;
-static unsigned hexval(char c)
-{
- if (c >= '0' && c <= '9')
- return c - '0';
- if (c >= 'a' && c <= 'f')
- return c - 'a' + 10;
- if (c >= 'A' && c <= 'F')
- return c - 'A' + 10;
- return ~0;
+static inline unsigned int hexval(unsigned int c)
+{
+ static signed char val[256] = {
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 00-07 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 08-0f */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 10-17 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 18-1f */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 20-27 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 28-2f */
+ 0, 1, 2, 3, 4, 5, 6, 7, /* 30-37 */
+ 8, 9, -1, -1, -1, -1, -1, -1, /* 38-3f */
+ -1, 10, 11, 12, 13, 14, 15, -1, /* 40-47 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 48-4f */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 50-57 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 58-5f */
+ -1, 10, 11, 12, 13, 14, 15, -1, /* 60-67 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 68-67 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 70-77 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 78-7f */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 80-87 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 88-8f */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 90-97 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* 98-9f */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* a0-a7 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* a8-af */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* b0-b7 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* b8-bf */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* c0-c7 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* c8-cf */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* d0-d7 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* d8-df */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* e0-e7 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* e8-ef */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* f0-f7 */
+ -1, -1, -1, -1, -1, -1, -1, -1, /* f8-ff */
+ };
+ return val[c];
}
int get_sha1_hex(const char *hex, unsigned char *sha1)
^ permalink raw reply related
* Re: Change set based shallow clone
From: Jon Smirl @ 2006-09-09 22:54 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jeff King, Marco Costalba, Paul Mackerras, linux@horizon.com,
Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0609091433460.27779@g5.osdl.org>
On 9/9/06, Linus Torvalds <torvalds@osdl.org> wrote:
> However, it's just a heuristic. "Most recent date" is not well-defined in
> a distributed environment that doesn't have a global clock. If somebody
> does commits on a machine that has the clock just set wrong, "most recent"
> may well not be _really_ recent.
When a merge happens could git fix things up in the database by adding
a corrected, hidden time stamp that would keep things from having an
out of order time sequence? That way you wouldn't need to rediscover
the out of order commit each time the tree is generated.
--
Jon Smirl
jonsmirl@gmail.com
^ 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