* Re: problem with http clone/pull
From: Junio C Hamano @ 2006-09-12 23:43 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git
In-Reply-To: <17671.16741.995661.664789@cargo.ozlabs.ibm.com>
Paul Mackerras <paulus@samba.org> writes:
> Getting alternates list for http://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/
> Also look at http://git.kernel.or
> error: Couldn't resolve host 'git.kernel.orobjects' (curl_result = 6, http_code = 0, sha1 = c336923b668fdcf0312efbec3b44895d713f4d81)
> Getting pack list for http://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/
> Getting pack list for http://git.kernel.or
I've seen this "last character of hostname dropped" symptom
mentioned on the #git channel long time ago, but I do not
remember if somebody figured out what the problem was. I know
that nobody did a patch to specifically fix it.
Among the changes since v1.3.0 that touches http-fetch.c the
only thing I can see that touches anything related to alternates
handling is this one, but I do not see anything obviously wrong
with it X-<.
commit bfbd0bb6ecbbbf75a5caaff6afaf5a6af8fa518e
Date: Sun Jun 11 14:03:28 2006 +0200
Implement safe_strncpy() as strlcpy() and use it more.
diff --git a/http-fetch.c b/http-fetch.c
index d3602b7..da1a7f5 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -584,10 +584,8 @@ static void process_alternates_response(
// skip 'objects' at end
if (okay) {
target = xmalloc(serverlen + posn - i - 6);
- strncpy(target, base, serverlen);
- strncpy(target + serverlen, data + i,
- posn - i - 7);
- target[serverlen + posn - i - 7] = '\0';
+ safe_strncpy(target, base, serverlen);
+ safe_strncpy(target + serverlen, data + i, posn - i - 6);
if (get_verbosely)
fprintf(stderr,
"Also look at %s\n", target);
^ permalink raw reply related
* problem with http clone/pull
From: Paul Mackerras @ 2006-09-12 23:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio,
Users are having problems doing clones or pulls of my powerpc.git tree
on kernel.org with version 1.4.2 of git. Apparently earlier versions
work OK.
When I do:
git clone http://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git
it produces the following output:
Getting alternates list for http://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/
Also look at http://git.kernel.or
error: Couldn't resolve host 'git.kernel.orobjects' (curl_result = 6, http_code = 0, sha1 = c336923b668fdcf0312efbec3b44895d713f4d81)
Getting pack list for http://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/
Getting pack list for http://git.kernel.or
error: Couldn't resolve host 'git.kernel.or'
error: Unable to find c336923b668fdcf0312efbec3b44895d713f4d81 under http://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/
Cannot obtain needed none c336923b668fdcf0312efbec3b44895d713f4d81
while processing commit 0000000000000000000000000000000000000000.
Now, this repository has a objects/info/alternates file that contains
the single line:
/pub/scm/linux/kernel/git/torvalds/linux-2.6.git/objects
I also have a hooks/post-update with execute permission, that does
exec git-update-server-info.
Is there anything obviously wrong in how I have set this up, or is it
a new bug in git 1.4.2?
Thanks,
Paul.
^ permalink raw reply
* [PATCH] Teach runstatus about --untracked
From: Johannes Schindelin @ 2006-09-12 20:45 UTC (permalink / raw)
To: git, junkio, Jeff King
Actually, teach runstatus what to do if it is not passed; it should not list
the contents of completely untracked directories, but only the name of that
directory (plus a trailing '/').
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
builtin-runstatus.c | 2 ++
git-commit.sh | 3 ++-
wt-status.c | 3 +++
wt-status.h | 1 +
4 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/builtin-runstatus.c b/builtin-runstatus.c
index 7979d61..303c556 100644
--- a/builtin-runstatus.c
+++ b/builtin-runstatus.c
@@ -25,6 +25,8 @@ int cmd_runstatus(int argc, const char *
}
else if (!strcmp(argv[i], "--verbose"))
s.verbose = 1;
+ else if (!strcmp(argv[i], "--untracked"))
+ s.untracked = 1;
else
usage(runstatus_usage);
}
diff --git a/git-commit.sh b/git-commit.sh
index 10c269a..5a4c659 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -82,7 +82,8 @@ run_status () {
esac
git-runstatus ${color} \
${verbose:+--verbose} \
- ${amend:+--amend}
+ ${amend:+--amend} \
+ ${untracked_files:+--untracked}
}
trap '
diff --git a/wt-status.c b/wt-status.c
index ec2c728..4398f9b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -50,6 +50,7 @@ void wt_status_prepare(struct wt_status
s->amend = 0;
s->verbose = 0;
s->commitable = 0;
+ s->untracked = 0;
}
static void wt_status_print_header(const char *main, const char *sub)
@@ -188,6 +189,8 @@ static void wt_status_print_untracked(co
memset(&dir, 0, sizeof(dir));
dir.exclude_per_dir = ".gitignore";
+ if (!s->untracked)
+ dir.show_other_directories = 1;
x = git_path("info/exclude");
if (file_exists(x))
add_excludes_from_file(&dir, x);
diff --git a/wt-status.h b/wt-status.h
index 75d3cfe..0a5a5b7 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -15,6 +15,7 @@ struct wt_status {
int commitable;
int verbose;
int amend;
+ int untracked;
};
int git_status_config(const char *var, const char *value);
--
1.4.2.g1734-dirty
^ permalink raw reply related
* Re: Make ref resolution saner
From: linux @ 2006-09-12 18:57 UTC (permalink / raw)
To: git, torvalds; +Cc: linux
I just noticed as part of that patch that building the path of
$GIT_DIR/HEAD is no longer necessary, which is the middle hunk,
and the other two jumped out at me while looking at the code.
(On top of Linus' patch.)
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 23b7714..f4a6d1f 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -203,7 +203,6 @@ static void create_default_files(const c
* shared-repository settings, we would need to fix them up.
*/
if (shared_repository) {
- path[len] = 0;
adjust_shared_perm(path);
strcpy(path + len, "refs");
adjust_shared_perm(path);
@@ -217,7 +216,6 @@ static void create_default_files(const c
* Create the default symlink from ".git/HEAD" to the "master"
* branch, if it does not exist yet.
*/
- strcpy(path + len, "HEAD");
if (read_ref("HEAD", sha1) < 0) {
if (create_symref("HEAD", "refs/heads/master") < 0)
exit(1);
@@ -227,7 +225,6 @@ static void create_default_files(const c
sprintf(repo_version_string, "%d", GIT_REPO_VERSION);
git_config_set("core.repositoryformatversion", repo_version_string);
- path[len] = 0;
strcpy(path + len, "config");
/* Check filemode trustability */
^ permalink raw reply related
* Re: qgit segfaults after b237b00
From: Marco Costalba @ 2006-09-12 17:39 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Git Mailing List
In-Reply-To: <45068577.2020608@op5.se>
Hi Andreas,
On 9/12/06, Andreas Ericsson <ae@op5.se> wrote:
> What subject says, really. Tried cold cache, hot cache, with and without
> qgit.dat, 3 different repos and 14 different repo-tips. Same result
> every time. A segfault before anything is drawn.
>
Sorry but I am not able to reproduce the bug here. Also from the trace I see
that the segfault happens in a part of code that has not been touched
by b237b00.
And that _should_ not segfault in any way.
The code there is the same of qgit-1.5.1 so perhaps could be some
platform related issue,
please write me Qt and gcc versions and processor used and, in case,
do a complete rebuild with also reconfiguration (autoreconf -i).
What about the current qgit HEAD (6128112f36) , could you reproduce
the bug also there?
>From the trace I see the segfault occurs in main view constructor,
i.e. before to touch anything.
So it is not related to a particular repository.
> c++ is a scary thing to me,
C++ is really scary not because we don't understand the code, but
because we _think_ to understand what the code does ;-)
Principle of least surprise is not exactly one of the design
cornerstones of C++, at least for normal people sense of surprise.
>
> btw, kudos for a great tool.
>
Thanks to you for using it, and reporting bugs.
Any suggestion is very appreciated!
Marco
^ permalink raw reply
* cvsimport fails with cvsps core dump
From: Rajkumar S @ 2006-09-12 16:14 UTC (permalink / raw)
To: git
Hi,
While trying to cvsimport pfsense [1] source [2], cvsps coredumps and
cvsimport aborts with the following error.
root@beastie:# git cvsimport -v -d /usr/local/pfSense/tmp/cvsroot -C
pfSense/ pfSense
Running cvsps...
cvs_direct initialized to CVSROOT /usr/local/pfSense/tmp/cvsroot
cvs rlog: Logging pfSense
cvs rlog: Logging pfSense/bin
cvs rlog: Logging pfSense/boot
cvs rlog: Logging pfSense/boot/kernel
cvs rlog: Logging pfSense/cf
cvs rlog: Logging pfSense/cf/conf
cvs rlog: Logging pfSense/conf.default
cvs rlog: Logging pfSense/etc
cvs rlog: Logging pfSense/etc/ascii-art
cvs rlog: Logging pfSense/etc/inc
DONE; creating master branch
fatal: refs/heads/origin: not a valid SHA1
fatal: master: not a valid SHA1
fatal: ambiguous argument 'HEAD': unknown revision or path not in the
working tree.
Use '--' to separate paths from revisions
fatal: Not a valid object name HEAD
checkout failed: 256
root@beastie:# ls pfSense/
.git cvsps.core
I am trying this in FreeBSD 6.1 with cvsps compiled from master branch
of git repository at http://ydirson.free.fr/soft/git/cvsps.git
To replicate this error get the pfSense cvsroot tarball from [2] and
run the command
$ git cvsimport -v -d /<path>/cvsroot -C pfSense/ pfSense
While this is not a git bug as such, I have written to the cvsps
author about this about one week back with no replies and this seems
to be the only place interested in cvsps.
Thanks and regards,
raj
1 http://www.pfsense.com
2 http://www.pfsense.com/cvs.tgz
^ permalink raw reply
* Re: Make ref resolution saner
From: Linus Torvalds @ 2006-09-12 14:41 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20060912053616.GA6706@coredump.intra.peff.net>
On Tue, 12 Sep 2006, Jeff King wrote:
>
> I assume your patch is against master;
Yeah. Well, master plus my previous patches.
> it looks like there's exactly one call to resolve_ref that's in next but
> not master. One-liner fix below.
Not quite enough. It's the same thing: wt-status.c plays games with the
return value (which _used_ to be a path) in order to turn it back into a
ref. But now that it's all about refs, the games are unnecessary:
> diff --git a/wt-status.c b/wt-status.c
> index ec2c728..e2f49c7 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -41,7 +41,7 @@ void wt_status_prepare(struct wt_status
>
> s->is_initial = get_sha1("HEAD", sha1) ? 1 : 0;
>
> - head = resolve_ref(git_path("HEAD"), sha1, 0);
> + head = resolve_ref("HEAD", sha1, 0);
> s->branch = head ?
> strdup(head + strlen(get_git_dir()) + 1) :
> NULL;
So that "strdup(head + strlen(get_git_dir()) + 1)" should now be just
"strdup(head)".
Linus
^ permalink raw reply
* qgit segfaults after b237b00
From: Andreas Ericsson @ 2006-09-12 10:01 UTC (permalink / raw)
To: Git Mailing List
What subject says, really. Tried cold cache, hot cache, with and without
qgit.dat, 3 different repos and 14 different repo-tips. Same result
every time. A segfault before anything is drawn.
Backtrace isn't very helpful (to me anyways):
#0 0x00000000 in ?? ()
(gdb) bt
#0 0x00000000 in ?? ()
#1 0x458525d2 in QGVector::insert () from /usr/lib/qt-3.3/lib/libqt-mt.so.3
#2 0x080aa19a in MainImpl::setupPixmaps (this=0x8191520, h=18)
at /usr/lib/qt-3.3/include/qptrvector.h:67
#3 0x080b1ad1 in MainImpl::adjustFontSize (this=0x8191520, delta=0)
at mainimpl.cpp:981
#4 0x080bae05 in MainImpl (this=0x8191520, cd=@0xbf92b094, p=0x0, name=0x0)
at mainimpl.cpp:142
#5 0x080c4cdf in main (argc=0, argv=0x0) at qgit.cpp:14
(gdb) quit
c++ is a scary thing to me, and my familiarity with QT is akin to the
common cavemans grasp of piloting fighter-jets. I thus refrain from
guessing where the problem might be.
qgit-1.5.1 works wonderfully, so I've reset to that and am using it
meanwhile.
btw, kudos for a great tool.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Git Survey summary
From: Nguyễn Thái Ngọc Duy @ 2006-09-12 9:05 UTC (permalink / raw)
To: git
I'm summarizing the survey at http://git.or.cz/gitwiki/GitSurvey.
There are some questions I haven't done yet: 14, 20, 21, 23, 24, 25
and 31. However as I won't work on it until next week, if you have
spare time, feel free to edit it (even other questions because I'm
quite certain the result is biased :). We may need some analysis than
just raw numbers I collected.
--
Duy
^ permalink raw reply
* [PATCH] connect.c: finish_connect(): allow null pid parameter
From: Franck Bui-Huu @ 2006-09-12 9:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <450673B7.8050704@innova-card.com>
git_connect() can return 0 if we use git protocol for example.
Users of this function don't know and don't care if a process
had been created or not, and to avoid them to check it before
calling finish_connect() this patch allows finish_connect() to
take a null pid. And in that case return 0.
Signed-off-by: Franck Bui-Huu <vagabon.xyz@gmail.com>
---
Found it when debugging 'git archive --remote=git://...'
command. I noticed that this command always exited with 1 as
status.
connect.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/connect.c b/connect.c
index 1c6429b..e6efff9 100644
--- a/connect.c
+++ b/connect.c
@@ -737,6 +737,9 @@ int git_connect(int fd[2], char *url, co
int finish_connect(pid_t pid)
{
+ if (pid == 0)
+ return 0;
+
while (waitpid(pid, NULL, 0) < 0) {
if (errno != EINTR)
return -1;
--
1.4.2
^ permalink raw reply related
* Re: [PATCH 3/3] Add sideband status report to git-archive protocol
From: Franck Bui-Huu @ 2006-09-12 8:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Junio C Hamano, git
In-Reply-To: <45066CFD.5040202@innova-card.com>
Franck Bui-Huu wrote:
> Junio C Hamano wrote:
>>
>> We should also check when read() returns an error, so how about
>> this on top of your patch?
>>
argh, there's still something broken...doing:
$ while true; do
> git archive --format=tar --remote=git://localhost/repo.git HEAD | tar tf -
> done
stucks somehow. I won't have time to look at it today :(
Franck
^ permalink raw reply
* Re: [PATCH 3/3] Add sideband status report to git-archive protocol
From: Franck Bui-Huu @ 2006-09-12 8:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Franck Bui-Huu, git
In-Reply-To: <7vzmd5eedk.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> Franck Bui-Huu <vagabon.xyz@gmail.com> writes:
>
>> I get a lot of "Hmph, HUP?" messages when testing "git-archive
>> --remote" command. One guess: this can be due to the fact that when
>> the writer process exits, it first closes its fd but do not send a
>> SIGCHLD signal right after to its parent.
>
> It does not reproduce for me, but the code I have is obviously
> bogus in a few places.
>
> - When POLLHUP is set, it goes ahead and reads the file
> descriptor. Worse yet, it does not check the return value of
> read() for errors when it does.
>
The thing is that read() doesn't actually return an error in that
case instead it returns 0 meaning it saw EOF when reading on the
pipe. But I agree we should check for errors. FYI, here's an output
of strace on git-daemon. This trace is realized with the buggy code.
3070 read(5, "[core]\n\trepositoryformatversion "..., 4096) = 53
23070 read(5, "", 4096) = 0
23070 close(5) = 0
23070 munmap(0xb7f92000, 4096) = 0
23070 write(1, "pax_global_header\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 10240) = 10240
23069 <... poll resumed> [{fd=5, events=POLLIN, revents=POLLIN}, {fd=7, events=POLLIN}], 2, -1) = 1
23069 read(5, "pax_global_header\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 16384) = 10240
23069 write(1, "2805\1", 5) = 5
23069 write(1, "pax_global_header\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 10240) = 10240
23069 poll( <unfinished ...>
23070 write(1, "type f -iname \"$c-*\" | sed -e 1q"..., 10240) = 10240
23069 <... poll resumed> [{fd=5, events=POLLIN, revents=POLLIN}, {fd=7, events=POLLIN}], 2, -1) = 1
23069 read(5, "type f -iname \"$c-*\" | sed -e 1q"..., 16384) = 10240
23069 write(1, "2805\1", 5) = 5
23069 write(1, "type f -iname \"$c-*\" | sed -e 1q"..., 10240) = 10240
23069 poll( <unfinished ...>
23070 exit_group(0) = ?
23069 <... poll resumed> [{fd=5, events=POLLIN, revents=POLLHUP}, {fd=7, events=POLLIN}], 2, -1) = 1
23069 read(5, "", 16384) = 0
23069 waitpid(23070, 0xbfef3188, WNOHANG) = 0
23069 write(2, "Hmph, HUP?\n", 11) = 11
23069 poll([{fd=5, events=POLLIN, revents=POLLHUP}, {fd=7, events=POLLIN}], 2, -1) = 1
23069 read(5, "", 16384) = 0
23069 waitpid(23070, 0xbfef3188, WNOHANG) = 0
23069 write(2, "Hmph, HUP?\n", 11) = 11
[...]
23077 poll([{fd=5, events=POLLIN, revents=POLLHUP}, {fd=7, events=POLLIN}], 2, -1) = 1
23077 read(5, "", 16384) = 0
23077 waitpid(23078, 0xbf9bb448, WNOHANG) = 0
23077 --- SIGCHLD (Child exited) @ 0 (0) ---
23077 write(2, "Hmph, HUP?\n", 11) = 11
23077 poll([{fd=5, events=POLLIN, revents=POLLHUP}, {fd=7, events=POLLIN, revents=POLLHUP}], 2, -1) = 2
23077 read(5, "", 16384) = 0
23077 read(7, "", 16384) = 0
23077 waitpid(23078, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], WNOHANG) = 23078
23077 write(1, "0000", 4) = 4
23077 exit_group(0) = ?
23065 <... poll resumed> [{fd=3, events=POLLIN}, {fd=4, events=POLLIN}], 2, -1) = -1 EINTR (Interrupted system call)
23065 --- SIGCHLD (Child exited) @ 0 (0) ---
23065 waitpid(-1, [{WIFEXITED(s) && WEXITSTATUS(s) == 0}], WNOHANG) = 23077
23065 time(NULL) = 1158047440
[...]
> - When we processed one POLLIN, we should just go back and see
> if any more data is available. We can check if the child is
> still there when poll gave control back at us but without any
> actual input as you said.
>
> I was uncomfortable letting waitpid() there to wait forever.
> When does poll() return? (1) we have data ready in which case
> we process; (2) the child somehow closed the pipe but without
> dying, which is an error in the child. In the latter case even
> not hanging in waitpid() and retrying the poll would not give
> any useful input so that would not help either.
>
your case (2) is not totaly right. If you look a the trace above,
for the normal case, you can see that the child close the pipe then
_after_ a while die. So there's a time when the child is not died
but the pipe is closed.
I think it's safe to assume that if the child closes the pipe, either
because it has finished to write or something wrong going on, then
it's going to die pretty soon.
> So I think your patch is a correct fix, except that I think we
> should let the remote side know why we stopped talking to them
> instead of calling die() there.
>
> We should also check when read() returns an error, so how about
> this on top of your patch?
>
> diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
> index 2ebe9a0..a53cfee 100644
> --- a/builtin-upload-archive.c
> +++ b/builtin-upload-archive.c
> @@ -16,6 +16,9 @@ static const char upload_archive_usage[]
> static const char deadchild[] =
> "git-upload-archive: archiver died with error";
>
> +static const char lostchild[] =
> +"git-upload-archive: archiver process was lost";
> +
>
> static int run_upload_archive(int argc, const char **argv, const char *prefix)
> {
> @@ -73,6 +76,31 @@ static int run_upload_archive(int argc,
> return ar.write_archive(&ar.args);
> }
>
> +static void error_clnt(const char *fmt, ...)
> +{
> + char buf[1024];
> + va_list params;
> + int len;
> +
> + va_start(params, fmt);
> + len = vsprintf(buf, fmt, params);
> + va_end(params);
> + send_sideband(1, 3, buf, len, LARGE_PACKET_MAX);
> + die("sent error to the client: %s", buf);
> +}
> +
> +static void process_input(int child_fd, int band)
> +{
> + char buf[16384];
> + ssize_t sz = read(child_fd, buf, sizeof(buf));
> + if (sz < 0) {
> + if (errno != EINTR)
> + error_clnt("read error: %s\n", strerror(errno));
> + }
> + else if (sz)
> + send_sideband(1, band, buf, sz, LARGE_PACKET_MAX);
> +}
> +
I think calling send_sideband() with sz = 0 should be fine, otherwise
this function is buggy, no ? So you can simply write process_input() like
static void process_input(int child_fd, int band)
{
char buf[16384];
ssize_t sz = read(child_fd, buf, sizeof(buf));
if (sz < 0) {
if (errno != EINTR)
error_clnt("read error: %s\n", strerror(errno));
return;
}
send_sideband(1, band, buf, sz, LARGE_PACKET_MAX);
}
Franck
^ permalink raw reply
* Re: [PATCH 3/3] Add sideband status report to git-archive protocol
From: Junio C Hamano @ 2006-09-12 7:24 UTC (permalink / raw)
To: Franck Bui-Huu; +Cc: git
In-Reply-To: <45053BA2.6050502@innova-card.com>
Franck Bui-Huu <vagabon.xyz@gmail.com> writes:
> I get a lot of "Hmph, HUP?" messages when testing "git-archive
> --remote" command. One guess: this can be due to the fact that when
> the writer process exits, it first closes its fd but do not send a
> SIGCHLD signal right after to its parent.
It does not reproduce for me, but the code I have is obviously
bogus in a few places.
- When POLLHUP is set, it goes ahead and reads the file
descriptor. Worse yet, it does not check the return value of
read() for errors when it does.
- When we processed one POLLIN, we should just go back and see
if any more data is available. We can check if the child is
still there when poll gave control back at us but without any
actual input as you said.
I was uncomfortable letting waitpid() there to wait forever.
When does poll() return? (1) we have data ready in which case
we process; (2) the child somehow closed the pipe but without
dying, which is an error in the child. In the latter case even
not hanging in waitpid() and retrying the poll would not give
any useful input so that would not help either.
So I think your patch is a correct fix, except that I think we
should let the remote side know why we stopped talking to them
instead of calling die() there.
We should also check when read() returns an error, so how about
this on top of your patch?
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index 2ebe9a0..a53cfee 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -16,6 +16,9 @@ static const char upload_archive_usage[]
static const char deadchild[] =
"git-upload-archive: archiver died with error";
+static const char lostchild[] =
+"git-upload-archive: archiver process was lost";
+
static int run_upload_archive(int argc, const char **argv, const char *prefix)
{
@@ -73,6 +76,31 @@ static int run_upload_archive(int argc,
return ar.write_archive(&ar.args);
}
+static void error_clnt(const char *fmt, ...)
+{
+ char buf[1024];
+ va_list params;
+ int len;
+
+ va_start(params, fmt);
+ len = vsprintf(buf, fmt, params);
+ va_end(params);
+ send_sideband(1, 3, buf, len, LARGE_PACKET_MAX);
+ die("sent error to the client: %s", buf);
+}
+
+static void process_input(int child_fd, int band)
+{
+ char buf[16384];
+ ssize_t sz = read(child_fd, buf, sizeof(buf));
+ if (sz < 0) {
+ if (errno != EINTR)
+ error_clnt("read error: %s\n", strerror(errno));
+ }
+ else if (sz)
+ send_sideband(1, band, buf, sz, LARGE_PACKET_MAX);
+}
+
int cmd_upload_archive(int argc, const char **argv, const char *prefix)
{
pid_t writer;
@@ -112,8 +140,6 @@ int cmd_upload_archive(int argc, const c
while (1) {
struct pollfd pfd[2];
- char buf[16384];
- ssize_t sz;
int status;
pfd[0].fd = fd1[0];
@@ -128,26 +154,19 @@ int cmd_upload_archive(int argc, const c
}
continue;
}
- if (pfd[0].revents & (POLLIN|POLLHUP)) {
+ if (pfd[0].revents & POLLIN)
/* Data stream ready */
- sz = read(pfd[0].fd, buf, sizeof(buf));
- send_sideband(1, 1, buf, sz, LARGE_PACKET_MAX);
- }
- if (pfd[1].revents & (POLLIN|POLLHUP)) {
+ process_input(pfd[0].fd, 1);
+ if (pfd[1].revents & POLLIN)
/* Status stream ready */
- sz = read(pfd[1].fd, buf, sizeof(buf));
- send_sideband(1, 2, buf, sz, LARGE_PACKET_MAX);
- }
-
+ process_input(pfd[1].fd, 2);
if ((pfd[0].revents | pfd[1].revents) == POLLIN)
continue;
- if (waitpid(writer, &status, 0) < 0) {
- die("waitpid failed: %s", strerror(errno));
- }
- if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
- send_sideband(1, 3, deadchild, strlen(deadchild),
- LARGE_PACKET_MAX);
+ if (waitpid(writer, &status, 0) < 0)
+ error_clnt("%s", lostchild);
+ else if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
+ error_clnt("%s", deadchild);
packet_flush(1);
break;
}
^ permalink raw reply related
* Re: Make ref resolution saner
From: Jeff King @ 2006-09-12 5:36 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0609112010340.27779@g5.osdl.org>
On Mon, Sep 11, 2006 at 08:17:35PM -0700, Linus Torvalds wrote:
> The old code used to totally mix up the notion of a ref-name and the path
> that that ref was associated with. That was not only horribly ugly (a
I assume your patch is against master; it looks like there's exactly one
call to resolve_ref that's in next but not master. One-liner fix below.
-Peff
-- >8 --
wt-status: use simplified resolve_ref to find current branch
---
wt-status.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index ec2c728..e2f49c7 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -41,7 +41,7 @@ void wt_status_prepare(struct wt_status
s->is_initial = get_sha1("HEAD", sha1) ? 1 : 0;
- head = resolve_ref(git_path("HEAD"), sha1, 0);
+ head = resolve_ref("HEAD", sha1, 0);
s->branch = head ?
strdup(head + strlen(get_git_dir()) + 1) :
NULL;
--
1.4.2.g39f1
^ permalink raw reply related
* [PATCH] Fix space in string " false" problem in "trace.c".
From: Christian Couder @ 2006-09-12 4:43 UTC (permalink / raw)
To: Junio Hamano; +Cc: git
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
trace.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/trace.c b/trace.c
index ce01c34..f9efc91 100644
--- a/trace.c
+++ b/trace.c
@@ -55,7 +55,7 @@ static int get_trace_fd(int *need_close)
{
char *trace = getenv("GIT_TRACE");
- if (!trace || !strcmp(trace, "0") || !strcasecmp(trace," false"))
+ if (!trace || !strcmp(trace, "0") || !strcasecmp(trace, "false"))
return 0;
if (!strcmp(trace, "1") || !strcasecmp(trace, "true"))
return STDERR_FILENO;
--
1.4.2.g886e
^ permalink raw reply related
* Make ref resolution saner
From: Linus Torvalds @ 2006-09-12 3:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0609112008500.27779@g5.osdl.org>
The old code used to totally mix up the notion of a ref-name and the path
that that ref was associated with. That was not only horribly ugly (a
number of users got the path, and then wanted to try to turn it back into
a ref-name again), but it fundamnetally doesn't work at all once we do any
setup where a ref doesn't have a 1:1 relationship with a particular
pathname.
This fixes things up so that we use the ref-name throughout, and only
turn it into a pathname once we actually look it up in the filesystem.
That makes a lot of things much clearer and more straightforward.
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
This patch is a bit scary, but look at the first two hunks to
builtin-fmt-merge-msg.c to get an idea of what kind of *crap* it gets rid
of.
There were several places where we used "resolve_ref()" to figure out what
the branch it was on, but then to actually figure out what the branch-name
of that branch was (rather than the cwd-relative _filename_ is), it then
did strange things with the string length of the original ref compared
with the string length of the _filename_ of the original ref etc etc.
Making all the ref-related functions just use the ref-name itself
throughout just gets rid of the crap, but more importantly, it's
absolutely required for just about _any_ ref-packing scheme.
That said, it _is_ a scary patch. It removes more lines that it adds:
builtin-fmt-merge-msg.c | 7 +----
builtin-init-db.c | 4 +--
builtin-show-branch.c | 46 +++++++++++++------------------
builtin-symbolic-ref.c | 14 +++-------
cache.h | 4 +--
refs.c | 69 +++++++++++++++++++++++++----------------------
sha1_name.c | 14 +++++-----
7 files changed, 74 insertions(+), 84 deletions(-)
but if I missed some place where we used a ref as a pathname, or added one
"git_path()" translation too many, it results in problems.
The good news is that our test-suite for this seems to be reasonably
complete. It sure caught a lot of places I had missed the first time
around. So this should be ok.
I'd _really_ like for this to go in, even regardless of the other patches
I have sent (although I think it depends on them in mostly fairly trivial
ways, since the previous patches changed how we do the recursive
readdir() on the refs/ directory).
It's really needed.
diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index c407c03..b93c17c 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -249,7 +249,7 @@ int cmd_fmt_merge_msg(int argc, const ch
FILE *in = stdin;
const char *sep = "";
unsigned char head_sha1[20];
- const char *head, *current_branch;
+ const char *current_branch;
git_config(fmt_merge_msg_config);
@@ -277,10 +277,7 @@ int cmd_fmt_merge_msg(int argc, const ch
usage(fmt_merge_msg_usage);
/* get current branch */
- head = xstrdup(git_path("HEAD"));
- current_branch = resolve_ref(head, head_sha1, 1);
- current_branch += strlen(head) - 4;
- free((char *)head);
+ current_branch = resolve_ref("HEAD", head_sha1, 1);
if (!strncmp(current_branch, "refs/heads/", 11))
current_branch += 11;
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 5085018..23b7714 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -218,8 +218,8 @@ static void create_default_files(const c
* branch, if it does not exist yet.
*/
strcpy(path + len, "HEAD");
- if (read_ref(path, sha1) < 0) {
- if (create_symref(path, "refs/heads/master") < 0)
+ if (read_ref("HEAD", sha1) < 0) {
+ if (create_symref("HEAD", "refs/heads/master") < 0)
exit(1);
}
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 578c9fa..4d8db0c 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -437,21 +437,13 @@ static void snarf_refs(int head, int tag
}
}
-static int rev_is_head(char *head_path, int headlen, char *name,
+static int rev_is_head(char *head, int headlen, char *name,
unsigned char *head_sha1, unsigned char *sha1)
{
- int namelen;
- if ((!head_path[0]) ||
+ if ((!head[0]) ||
(head_sha1 && sha1 && hashcmp(head_sha1, sha1)))
return 0;
- namelen = strlen(name);
- if ((headlen < namelen) ||
- memcmp(head_path + headlen - namelen, name, namelen))
- return 0;
- if (headlen == namelen ||
- head_path[headlen - namelen - 1] == '/')
- return 1;
- return 0;
+ return !strcmp(head, name);
}
static int show_merge_base(struct commit_list *seen, int num_rev)
@@ -559,9 +551,9 @@ int cmd_show_branch(int ac, const char *
int all_heads = 0, all_tags = 0;
int all_mask, all_revs;
int lifo = 1;
- char head_path[128];
- const char *head_path_p;
- int head_path_len;
+ char head[128];
+ const char *head_p;
+ int head_len;
unsigned char head_sha1[20];
int merge_base = 0;
int independent = 0;
@@ -638,31 +630,31 @@ int cmd_show_branch(int ac, const char *
ac--; av++;
}
- head_path_p = resolve_ref(git_path("HEAD"), head_sha1, 1);
- if (head_path_p) {
- head_path_len = strlen(head_path_p);
- memcpy(head_path, head_path_p, head_path_len + 1);
+ head_p = resolve_ref("HEAD", head_sha1, 1);
+ if (head_p) {
+ head_len = strlen(head_p);
+ memcpy(head, head_p, head_len + 1);
}
else {
- head_path_len = 0;
- head_path[0] = 0;
+ head_len = 0;
+ head[0] = 0;
}
- if (with_current_branch && head_path_p) {
+ if (with_current_branch && head_p) {
int has_head = 0;
for (i = 0; !has_head && i < ref_name_cnt; i++) {
/* We are only interested in adding the branch
* HEAD points at.
*/
- if (rev_is_head(head_path,
- head_path_len,
+ if (rev_is_head(head,
+ head_len,
ref_name[i],
head_sha1, NULL))
has_head++;
}
if (!has_head) {
- int pfxlen = strlen(git_path("refs/heads/"));
- append_one_rev(head_path + pfxlen);
+ int pfxlen = strlen("refs/heads/");
+ append_one_rev(head + pfxlen);
}
}
@@ -713,8 +705,8 @@ int cmd_show_branch(int ac, const char *
if (1 < num_rev || extra < 0) {
for (i = 0; i < num_rev; i++) {
int j;
- int is_head = rev_is_head(head_path,
- head_path_len,
+ int is_head = rev_is_head(head,
+ head_len,
ref_name[i],
head_sha1,
rev[i]->object.sha1);
diff --git a/builtin-symbolic-ref.c b/builtin-symbolic-ref.c
index 1d3a5e2..6f18db8 100644
--- a/builtin-symbolic-ref.c
+++ b/builtin-symbolic-ref.c
@@ -7,15 +7,11 @@ static const char git_symbolic_ref_usage
static void check_symref(const char *HEAD)
{
unsigned char sha1[20];
- const char *git_HEAD = xstrdup(git_path("%s", HEAD));
- const char *git_refs_heads_master = resolve_ref(git_HEAD, sha1, 0);
- if (git_refs_heads_master) {
- /* we want to strip the .git/ part */
- int pfxlen = strlen(git_HEAD) - strlen(HEAD);
- puts(git_refs_heads_master + pfxlen);
- }
- else
+ const char *refs_heads_master = resolve_ref("HEAD", sha1, 0);
+
+ if (!refs_heads_master)
die("No such ref: %s", HEAD);
+ puts(refs_heads_master);
}
int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
@@ -26,7 +22,7 @@ int cmd_symbolic_ref(int argc, const cha
check_symref(argv[1]);
break;
case 3:
- create_symref(xstrdup(git_path("%s", argv[1])), argv[2]);
+ create_symref(argv[1], argv[2]);
break;
default:
usage(git_symbolic_ref_usage);
diff --git a/cache.h b/cache.h
index a53204f..5d6c7ee 100644
--- a/cache.h
+++ b/cache.h
@@ -287,8 +287,8 @@ extern int get_sha1_hex(const char *hex,
extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */
extern int read_ref(const char *filename, unsigned char *sha1);
extern const char *resolve_ref(const char *path, unsigned char *sha1, int);
-extern int create_symref(const char *git_HEAD, const char *refs_heads_master);
-extern int validate_symref(const char *git_HEAD);
+extern int create_symref(const char *ref, const char *refs_heads_master);
+extern int validate_symref(const char *ref);
extern int base_name_compare(const char *name1, int len1, int mode1, const char *name2, int len2, int mode2);
extern int cache_name_compare(const char *name1, int len1, const char *name2, int len2);
diff --git a/refs.c b/refs.c
index 72e2283..50c25d3 100644
--- a/refs.c
+++ b/refs.c
@@ -93,11 +93,11 @@ static struct ref_list *get_ref_dir(cons
if (dir) {
struct dirent *de;
int baselen = strlen(base);
- char *path = xmalloc(baselen + 257);
+ char *ref = xmalloc(baselen + 257);
- memcpy(path, base, baselen);
+ memcpy(ref, base, baselen);
if (baselen && base[baselen-1] != '/')
- path[baselen++] = '/';
+ ref[baselen++] = '/';
while ((de = readdir(dir)) != NULL) {
unsigned char sha1[20];
@@ -111,20 +111,20 @@ static struct ref_list *get_ref_dir(cons
continue;
if (has_extension(de->d_name, ".lock"))
continue;
- memcpy(path + baselen, de->d_name, namelen+1);
- if (stat(git_path("%s", path), &st) < 0)
+ memcpy(ref + baselen, de->d_name, namelen+1);
+ if (stat(git_path("%s", ref), &st) < 0)
continue;
if (S_ISDIR(st.st_mode)) {
- list = get_ref_dir(path, list);
+ list = get_ref_dir(ref, list);
continue;
}
- if (read_ref(git_path("%s", path), sha1) < 0) {
- error("%s points nowhere!", path);
+ if (read_ref(ref, sha1) < 0) {
+ error("%s points nowhere!", ref);
continue;
}
- list = add_ref(path, sha1, list);
+ list = add_ref(ref, sha1, list);
}
- free(path);
+ free(ref);
closedir(dir);
}
return list;
@@ -145,12 +145,14 @@ static struct ref_list *get_loose_refs(v
/* We allow "recursive" symbolic refs. Only within reason, though */
#define MAXDEPTH 5
-const char *resolve_ref(const char *path, unsigned char *sha1, int reading)
+const char *resolve_ref(const char *ref, unsigned char *sha1, int reading)
{
int depth = MAXDEPTH, len;
char buffer[256];
+ static char ref_buffer[256];
for (;;) {
+ const char *path = git_path("%s", ref);
struct stat st;
char *buf;
int fd;
@@ -169,14 +171,16 @@ const char *resolve_ref(const char *path
if (reading || errno != ENOENT)
return NULL;
hashclr(sha1);
- return path;
+ return ref;
}
/* Follow "normalized" - ie "refs/.." symlinks by hand */
if (S_ISLNK(st.st_mode)) {
len = readlink(path, buffer, sizeof(buffer)-1);
if (len >= 5 && !memcmp("refs/", buffer, 5)) {
- path = git_path("%.*s", len, buffer);
+ buffer[len] = 0;
+ strcpy(ref_buffer, buffer);
+ ref = ref_buffer;
continue;
}
}
@@ -201,19 +205,22 @@ const char *resolve_ref(const char *path
while (len && isspace(*buf))
buf++, len--;
while (len && isspace(buf[len-1]))
- buf[--len] = 0;
- path = git_path("%.*s", len, buf);
+ len--;
+ buf[len] = 0;
+ memcpy(ref_buffer, buf, len + 1);
+ ref = ref_buffer;
}
if (len < 40 || get_sha1_hex(buffer, sha1))
return NULL;
- return path;
+ return ref;
}
-int create_symref(const char *git_HEAD, const char *refs_heads_master)
+int create_symref(const char *ref_target, const char *refs_heads_master)
{
const char *lockpath;
char ref[1000];
int fd, len, written;
+ const char *git_HEAD = git_path("%s", ref_target);
#ifndef NO_SYMLINK_HEAD
if (prefer_symlink_refs) {
@@ -251,9 +258,9 @@ #endif
return 0;
}
-int read_ref(const char *filename, unsigned char *sha1)
+int read_ref(const char *ref, unsigned char *sha1)
{
- if (resolve_ref(filename, sha1, 1))
+ if (resolve_ref(ref, sha1, 1))
return 0;
return -1;
}
@@ -306,7 +313,7 @@ static int do_for_each_ref(const char *b
int head_ref(int (*fn)(const char *path, const unsigned char *sha1))
{
unsigned char sha1[20];
- if (!read_ref(git_path("HEAD"), sha1))
+ if (!read_ref("HEAD", sha1))
return fn("HEAD", sha1);
return 0;
}
@@ -335,7 +342,7 @@ int get_ref_sha1(const char *ref, unsign
{
if (check_ref_format(ref))
return -1;
- return read_ref(git_path("refs/%s", ref), sha1);
+ return read_ref(mkpath("refs/%s", ref), sha1);
}
/*
@@ -416,31 +423,30 @@ static struct ref_lock *verify_lock(stru
return lock;
}
-static struct ref_lock *lock_ref_sha1_basic(const char *path,
+static struct ref_lock *lock_ref_sha1_basic(const char *ref,
int plen,
const unsigned char *old_sha1, int mustexist)
{
- const char *orig_path = path;
+ const char *orig_ref = ref;
struct ref_lock *lock;
struct stat st;
lock = xcalloc(1, sizeof(struct ref_lock));
lock->lock_fd = -1;
- plen = strlen(path) - plen;
- path = resolve_ref(path, lock->old_sha1, mustexist);
- if (!path) {
+ ref = resolve_ref(ref, lock->old_sha1, mustexist);
+ if (!ref) {
int last_errno = errno;
error("unable to resolve reference %s: %s",
- orig_path, strerror(errno));
+ orig_ref, strerror(errno));
unlock_ref(lock);
errno = last_errno;
return NULL;
}
lock->lk = xcalloc(1, sizeof(struct lock_file));
- lock->ref_file = xstrdup(path);
- lock->log_file = xstrdup(git_path("logs/%s", lock->ref_file + plen));
+ lock->ref_file = xstrdup(git_path("%s", ref));
+ lock->log_file = xstrdup(git_path("logs/%s", ref));
lock->force_write = lstat(lock->ref_file, &st) && errno == ENOENT;
if (safe_create_leading_directories(lock->ref_file))
@@ -455,15 +461,14 @@ struct ref_lock *lock_ref_sha1(const cha
{
if (check_ref_format(ref))
return NULL;
- return lock_ref_sha1_basic(git_path("refs/%s", ref),
+ return lock_ref_sha1_basic(mkpath("refs/%s", ref),
5 + strlen(ref), old_sha1, mustexist);
}
struct ref_lock *lock_any_ref_for_update(const char *ref,
const unsigned char *old_sha1, int mustexist)
{
- return lock_ref_sha1_basic(git_path("%s", ref),
- strlen(ref), old_sha1, mustexist);
+ return lock_ref_sha1_basic(ref, strlen(ref), old_sha1, mustexist);
}
void unlock_ref(struct ref_lock *lock)
diff --git a/sha1_name.c b/sha1_name.c
index 1fbc443..b497528 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -247,8 +247,8 @@ static int get_sha1_basic(const char *st
NULL
};
static const char *warning = "warning: refname '%.*s' is ambiguous.\n";
- const char **p, *pathname;
- char *real_path = NULL;
+ const char **p, *ref;
+ char *real_ref = NULL;
int refs_found = 0, am;
unsigned long at_time = (unsigned long)-1;
unsigned char *this_result;
@@ -276,10 +276,10 @@ static int get_sha1_basic(const char *st
for (p = fmt; *p; p++) {
this_result = refs_found ? sha1_from_ref : sha1;
- pathname = resolve_ref(git_path(*p, len, str), this_result, 1);
- if (pathname) {
+ ref = resolve_ref(mkpath(*p, len, str), this_result, 1);
+ if (ref) {
if (!refs_found++)
- real_path = xstrdup(pathname);
+ real_ref = xstrdup(ref);
if (!warn_ambiguous_refs)
break;
}
@@ -293,12 +293,12 @@ static int get_sha1_basic(const char *st
if (at_time != (unsigned long)-1) {
read_ref_at(
- real_path + strlen(git_path(".")) - 1,
+ real_ref,
at_time,
sha1);
}
- free(real_path);
+ free(real_ref);
return 0;
}
^ permalink raw reply related
* Add support for negative refs
From: Linus Torvalds @ 2006-09-12 3:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0609111632050.27779@g5.osdl.org>
You can remove a ref that is packed two different ways: either simply
repack all the refs without that one, or create a loose ref that has the
magic all-zero SHA1.
This also adds back the test that a ref actually has the object it
points to.
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
This one is trivial, and obviously depends on the previous series.
I'm sending it just because I have a much bigger cleanup that I'd _really_
like to go in, and I did this trivial one before that much more
interesting one.
diff --git a/refs.c b/refs.c
index 5f80a68..72e2283 100644
--- a/refs.c
+++ b/refs.c
@@ -280,6 +280,12 @@ static int do_for_each_ref(const char *b
}
if (strncmp(base, entry->name, trim))
continue;
+ if (is_null_sha1(entry->sha1))
+ continue;
+ if (!has_sha1_file(entry->sha1)) {
+ error("%s does not point to a valid object!", entry->name);
+ continue;
+ }
retval = fn(entry->name + trim, entry->sha1);
if (retval)
return retval;
^ permalink raw reply related
* Re: [PATCH] contrib/vim: add syntax highlighting file for commits
From: Jeff King @ 2006-09-12 2:32 UTC (permalink / raw)
To: git
In-Reply-To: <20060912020813.GD26332@socrates.priv>
On Mon, Sep 11, 2006 at 08:08:13PM -0600, Tom Prince wrote:
> > + 1. Copy commit-syntax.vim to vim's syntax directory:
> > + $ cp commit-syntax.vim $HOME/.vim/syntax/gitcommit.vim
> It would be more obvious if you could do cp gitcomit.vim $HOME/.vim/syntax/
It would be with only one file, but you could just as easily have a
.vim/ftplugin/gitcommit.vim file (I think there are others, too). If you
want to give it the "proper" name, it should probably be
syntax/gitcommit.vim.
-Peff
^ permalink raw reply
* Re: [PATCH] contrib/vim: add syntax highlighting file for commits
From: Tom Prince @ 2006-09-12 2:08 UTC (permalink / raw)
To: git
In-Reply-To: <20060911232249.GB29736@coredump.intra.peff.net>
On Mon, Sep 11, 2006 at 07:22:49PM -0400, Jeff King wrote:
> + 1. Copy commit-syntax.vim to vim's syntax directory:
> + $ cp commit-syntax.vim $HOME/.vim/syntax/gitcommit.vim
...
> diff --git a/contrib/vim/commit-syntax.vim b/contrib/vim/commit-syntax.vim
It would be more obvious if you could do cp gitcomit.vim $HOME/.vim/syntax/
Tom
^ permalink raw reply
* Re: Start handling references internally as a sorted in-memory list
From: Chris Wedgwood @ 2006-09-12 1:09 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0609111632050.27779@g5.osdl.org>
On Mon, Sep 11, 2006 at 04:37:32PM -0700, Linus Torvalds wrote:
> This also adds some very rudimentary support for the notion of packed
> refs.
Neat. Although git makes some repos nice and small the refs take a
lot of space when people use a lot of them:
3.5M glibc/glibc-cvs.git/.git/refs/
11M gcc-git/gcc.git/.git/refs/
^ permalink raw reply
* Re: Start handling references internally as a sorted in-memory list
From: Linus Torvalds @ 2006-09-12 1:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsliyez2m.fsf@assigned-by-dhcp.cox.net>
On Mon, 11 Sep 2006, Junio C Hamano wrote:
>
> I thought what triggered the restructuring of this part was N
> actually being large enough to cause pain to some people, so I
> suspect later might need to be reasonably soon ;-).
Well, the current problems is actually related to the _IO_ load of O(n)
(and to some degree the disk usage), and that is indeed fairly high. The
new O(n**2) is all for a (very) simple CPU loop, so I suspect "n" has to
be in the millions before it even approaches the IO load problem ;)
> I've been wondering what happens if you pack "refs/heads/foo",
> delete it, and create "refs/heads/foo/bar" with your proposal.
> I'll find the answer (which is _not_ "42") in the updated
> do_for_each_ref(), I guess.
Right now, the code in question only contains some preliminary support for
refs at all, and it's technically incorrect. If you ever have a
"packed-refs" file that contains _anything_ outside of the actual loose
refs, it won't do the right thing.
But I'd rather have this as a base, than send one huge patch that does
everything.
For example, right now the code doesn't support negative refs (even the
simple kind) at all, so you don't need to even have the "file turned into
a directory" case for it to "not work".
So right now it's basically just scaffolding. It's hopefully _good_
scaffolding, so that when I send the next few patches, it actually starts
working.. ;)
Linus
^ permalink raw reply
* Re: Change set based shallow clone
From: Jakub Narebski @ 2006-09-12 0:25 UTC (permalink / raw)
To: git
In-Reply-To: <7vodtmey30.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
>> Actually 'git ls-remote .' _is_ useful for gitweb, see the new
>> git_get_references code.
>
> You are missing the point.
>
> We are not discussing if having foo^{} is useful. There is no
> argument about it. Without it the user is forced to ask
> rev-parse.
>
> The point is if it is Ok to assume foo and foo^{} (and nothing
> else) are enough to make Porcelains and visualizers happy, and I
> suspected the answer was no (remember show-ref discussion?).
Well, it is enough for ref markers (this commit is this head or tag), for
dumb http transport (indo/refs output is the same as ls-remote), and not
much else...
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: Change set based shallow clone
From: Junio C Hamano @ 2006-09-12 0:18 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <ee4tmo$mhm$1@sea.gmane.org>
Jakub Narebski <jnareb@gmail.com> writes:
> Actually 'git ls-remote .' _is_ useful for gitweb, see the new
> git_get_references code.
You are missing the point.
We are not discussing if having foo^{} is useful. There is no
argument about it. Without it the user is forced to ask
rev-parse.
The point is if it is Ok to assume foo and foo^{} (and nothing
else) are enough to make Porcelains and visualizers happy, and I
suspected the answer was no (remember show-ref discussion?).
^ permalink raw reply
* Re: Change set based shallow clone
From: Jakub Narebski @ 2006-09-12 0:06 UTC (permalink / raw)
To: git
In-Reply-To: <7v1wqige41.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> Paul Mackerras <paulus@samba.org> writes:
>
>> Junio C Hamano writes:
>>
>>> That's a dubious idea.
>>>
>>> - Why assume a tag points directly at a commit, or if it is
>>> not, why assume "foo^{}" (dereferencing repeatedly until we
>>> get a non-tag) is special?
>>
>> Umm, I'm not sure what you're getting at here - if one shouldn't make
>> those assumptions, why does git ls-remote output both the tag and
>> tag^{} lines?
>
> This was originally done to support Cogito's tag following which
> was in its infancy. So in that sense it is already special (iow
> we know one user that can take advantage of it), but my point
> was that its usefulness for a commit chain fetching application
> (i.e. Cogito) does not automatically mean it is also useful for
> visualizers like gitk and gitweb.
Actually 'git ls-remote .' _is_ useful for gitweb, see the new
git_get_references code.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: Start handling references internally as a sorted in-memory list
From: Junio C Hamano @ 2006-09-11 23:57 UTC (permalink / raw)
To: Linus Torvalds; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0609111632050.27779@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
> This also adds some very rudimentary support for the notion of packed
> And yeah, I know that the "sorting" code is O(n**2) thanks to doing an
> insertion sort into a simple linked list. Tough. I didn't care enough to
> do it well. With "n" usually being a few hundred at most, we really don't
> care, and if we ever do, we _can_ fix it later on to use a heap or
> something.
I thought what triggered the restructuring of this part was N
actually being large enough to cause pain to some people, so I
suspect later might need to be reasonably soon ;-).
> +static const char *parse_ref_line(char *line, unsigned char *sha1)
> +{
> + /*
> + * 42: the answer to everything.
> + *
> + * In this case, it happens to be the answer to
>...
Rof,l.
I've been wondering what happens if you pack "refs/heads/foo",
delete it, and create "refs/heads/foo/bar" with your proposal.
I'll find the answer (which is _not_ "42") in the updated
do_for_each_ref(), I guess.
^ 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