* Re: Fw: git-core: SIGSEGV during {peek,ls}-remote on HTTP remotes.
From: Daniel Barkalow @ 2009-11-02 0:59 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Sverre Rabbelier, Samium Gromoff, git,
Tay Ray Chuan, Mike Hommey
In-Reply-To: <20091101230442.GA20264@coredump.intra.peff.net>
On Sun, 1 Nov 2009, Jeff King wrote:
> On Sun, Nov 01, 2009 at 04:19:34PM -0500, Daniel Barkalow wrote:
>
> > > > Do things like git_path() fail cleanly if there was no git directory? If
> > > > not, there should probably be tests of nongit on paths that actually need
> > > > a git directory,...
> > >
> > > I don't know. Again, you tell me ;-)
> >
> > I'm not an expert on that part. But it looks like it misbehaves, returning
> > ".git/foo" even when that path doesn't make sense.
>
> I will not admit to being an expert in that area, but yes, that is what
> I observed before while looking into some of our weird startup problems.
> We really have two systems for setting up the environment:
>
> - setup_git_directory, which tries to do everything at the outset (but
> which we don't necessarily run for all commands, and where "outset"
> is defined as when the git wrapper handles an actual command, which
> means we sometimes do quite a bit of stuff beforehand)
>
> - some lazy magic in environment.c, mostly setup_git_env. If
> setup_git_directory has been run, this does the right thing because
> it reads GIT_DIR from the environment as set previously. But if not,
> then it can quite often do the wrong thing (as you noticed).
>
> I tried simply ditching the lazy magic, but that doesn't work: there are
> many cases where setup_git_directory hasn't been run. Moving it to the
> very beginning doesn't quite work, either. I don't remember the details,
> sadly. It may be that the lazy setup_git_env magic should, rather than
> doing anything itself, call setup_git_directory if it has not been
> initialized. But at that point, you might as well setup_git_directory in
> every program, since just about every one is going to want to look at
> git_path at some point.
>
> Sorry, I know that is vague. Refactoring this area has been on my plate
> for so long that I have forgotten all the details, and it is such a
> messy area that I am continually procrastinating on diving back into it.
> ;)
I've been looking at it, just now, and I might try to clean stuff up. The
problem I'm running into is that, in some cases, you have to call
setup_git_directory_gently(), and it might determine that there is no git
repo, but then the various environment functions don't distinguish between
the situation where you haven't called it at all and the situation where
you called it and determined there to be no answer. Furthermore, a lot of
functions seem to be getting git_path(something), ignoring the fact that
there is no repo, and acting like there is a repo that has simply not got
the file it is looking for.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] commit -c/-C/--amend: acquire authorship and restamp time with --claim
From: Erick Mattos @ 2009-11-02 0:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbpjlycqc.fsf@alter.siamese.dyndns.org>
2009/11/1 Junio C Hamano <gitster@pobox.com>:
> Erick Mattos <erick.mattos@gmail.com> writes:
>
>>> % git commit --claim --author='Erick Mattos <eric@mattos>' -C HEAD
>>>
>>> Should you detect an error? Does your code do so? Do you have a test
>>> that catches this error?
>>
>> It works as intended. Both together.
>
> That does not make any sense. If you are saying this is yours and it is
> his at the same time, there can be no sane way to work "as intended", no?.
I am adding a new option not changing the option --author already in
git. So it does work together.
>>>> + git commit -c HEAD <<EOF
>>>> + "Changed"
>>>> + EOF &&
>>>
>>> What editor is reading this "Changed"?
>>
>> Nobody cares... Just a text to change the file.
>
> I actually care. Who uses that Changed string, and where does it end up
> with? At the end of the log message? At the beginning? What "file"?
>
I didn't get it. -c option does not accept -m option and starts an
editor to change the message. The text "Changed is just a forced
message. I can not use an editor in interactive mode in a script...
What I am losing here??
^ permalink raw reply
* Re: [PATCH] commit -c/-C/--amend: acquire authorship and restamp time with --claim
From: Junio C Hamano @ 2009-11-02 0:47 UTC (permalink / raw)
To: Erick Mattos; +Cc: git
In-Reply-To: <55bacdd30911011257m22ee85f2wc5d51865f7f2aadd@mail.gmail.com>
Erick Mattos <erick.mattos@gmail.com> writes:
>> % git commit --claim --author='Erick Mattos <eric@mattos>' -C HEAD
>>
>> Should you detect an error? Does your code do so? Do you have a test
>> that catches this error?
>
> It works as intended. Both together.
That does not make any sense. If you are saying this is yours and it is
his at the same time, there can be no sane way to work "as intended", no?.
>>> + git commit -c HEAD <<EOF
>>> + "Changed"
>>> + EOF &&
>>
>> What editor is reading this "Changed"?
>
> Nobody cares... Just a text to change the file.
I actually care. Who uses that Changed string, and where does it end up
with? At the end of the log message? At the beginning? What "file"?
^ permalink raw reply
* [PATCH] commit -c/-C/--amend: reset timestamp and authorship to committer with --mine
From: Erick Mattos @ 2009-11-02 0:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Erick Mattos
In-Reply-To: <55bacdd30911011514r1be6e09fobe2751588ed5166e@mail.gmail.com>
When we use one of the options above we are normally trying to do mainly
two things: one is using the source as a template and second is to
recreate a commit with corrections.
When they are used, the authorship and timestamp recorded in the newly
created commit is always taken from the original commit. And they
should not when we are using it as a template.
The new --mine option is meant to solve this need by regenerating the
timestamp and setting as new author the committer or the one specified
on --author option.
Signed-off-by: Erick Mattos <erick.mattos@gmail.com>
---
Documentation/git-commit.txt | 8 +++-
builtin-commit.c | 9 +++-
t/t7509-commit.sh | 98 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 111 insertions(+), 4 deletions(-)
create mode 100755 t/t7509-commit.sh
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0578a40..eae5bf4 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -9,7 +9,7 @@ SYNOPSIS
--------
[verse]
'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
- [(-c | -C) <commit>] [-F <file> | -m <msg>]
+ [(-c | -C) <commit>] [-F <file> | -m <msg>] [--mine]
[--allow-empty] [--no-verify] [-e] [--author=<author>]
[--cleanup=<mode>] [--] [[-i | -o ]<file>...]
@@ -69,6 +69,12 @@ OPTIONS
Like '-C', but with '-c' the editor is invoked, so that
the user can further edit the commit message.
+--mine::
+ When used with -C/-c/--amend options, declare that the
+ authorship of the resulting commit now belongs of the committer.
+ This also renews the author timestamp. Therefore this option
+ sets the use of only the message from the original commit.
+
-F <file>::
--file=<file>::
Take the commit message from the given file. Use '-' to
diff --git a/builtin-commit.c b/builtin-commit.c
index c395cbf..17a6794 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -51,7 +51,7 @@ static const char *template_file;
static char *edit_message, *use_message;
static char *author_name, *author_email, *author_date;
static int all, edit_flag, also, interactive, only, amend, signoff;
-static int quiet, verbose, no_verify, allow_empty, dry_run;
+static int quiet, verbose, no_verify, allow_empty, dry_run, mine;
static char *untracked_files_arg;
/*
* The default commit message cleanup mode will remove the lines
@@ -91,8 +91,9 @@ static struct option builtin_commit_options[] = {
OPT_FILENAME('F', "file", &logfile, "read log from file"),
OPT_STRING(0, "author", &force_author, "AUTHOR", "override author for commit"),
OPT_CALLBACK('m', "message", &message, "MESSAGE", "specify commit message", opt_parse_m),
- OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit "),
+ OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit"),
OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
+ OPT_BOOLEAN(0, "mine", &mine, "reset timestamp and authorship to committer"),
OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
OPT_FILENAME('t', "template", &template_file, "use specified template file"),
OPT_BOOLEAN('e', "edit", &edit_flag, "force edit of commit"),
@@ -381,7 +382,7 @@ static void determine_author_info(void)
email = getenv("GIT_AUTHOR_EMAIL");
date = getenv("GIT_AUTHOR_DATE");
- if (use_message) {
+ if (use_message && !mine) {
const char *a, *lb, *rb, *eol;
a = strstr(use_message_buffer, "\nauthor ");
@@ -778,6 +779,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
use_message = edit_message;
if (amend && !use_message)
use_message = "HEAD";
+ if (!use_message && mine)
+ die("Option --mine is used only with -C/-c/--amend.");
if (use_message) {
unsigned char sha1[20];
static char utf8[] = "UTF-8";
diff --git a/t/t7509-commit.sh b/t/t7509-commit.sh
new file mode 100755
index 0000000..514de6a
--- /dev/null
+++ b/t/t7509-commit.sh
@@ -0,0 +1,98 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Erick Mattos
+#
+
+test_description='git commit
+
+Tests for --mine option on a commit.'
+
+. ./test-lib.sh
+
+TEST_FILE=foo
+
+test_expect_success '-C without --mine uses the author from the old commit' '
+ echo "initial" > "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "Initial Commit" --author "Frigate <flying@over.world>" &&
+ test_tick &&
+ echo "Test 1" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -C HEAD &&
+ git cat-file -p HEAD^ | sed -e "/^parent/d" -e "/^tree/d" \
+ -e "/^committer/d" > commit_1 &&
+ git cat-file -p HEAD | sed -e "/^parent/d" -e "/^tree/d" \
+ -e "/^committer/d" > commit_2 &&
+ test_cmp commit_1 commit_2
+'
+
+test_expect_success '-C with --mine makes me the author' '
+ test_tick &&
+ echo "Test 2" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -C HEAD^ --mine &&
+ git cat-file -p HEAD^ | grep "^author" > commit_1 &&
+ git cat-file -p HEAD | grep "^author" > commit_2 &&
+ test "$(cat commit_1 | sed "s/.*> //")" !=\
+ "$(cat commit_2 | sed "s/.*> //")" &&
+ test "$(cat commit_2 | sed -e "s/author //" -e "s/>.*/>/")" =\
+ "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
+'
+
+test_expect_success '-c without --mine uses the author from the old commit' '
+ echo "Initial" > "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "Initial Commit" --author "Frigate <flying@over.world>" &&
+ test_tick &&
+ echo "Test 3" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -c HEAD <<EOF
+ "Changed"
+ EOF &&
+ git cat-file -p HEAD^ | grep "^author" > commit_1 &&
+ git cat-file -p HEAD | grep "^author" > commit_2 &&
+ test_cmp commit_1 commit_2
+'
+
+test_expect_success '-c with --mine makes me the author' '
+ test_tick &&
+ echo "Test 4" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -c HEAD^ --mine <<EOF
+ "Changed again"
+ EOF &&
+ git cat-file -p HEAD^ | grep "^author" > commit_1 &&
+ git cat-file -p HEAD | grep "^author" > commit_2 &&
+ test "$(cat commit_1 | sed "s/.*> //")" !=\
+ "$(cat commit_2 | sed "s/.*> //")" &&
+ test "$(cat commit_2 | sed -e "s/author //" -e "s/>.*/>/")" =\
+ "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
+'
+
+test_expect_success '--amend without --mine uses the author from the old commit' '
+ echo "Initial" > "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "Initial Commit" --author "Frigate <flying@over.world>" &&
+ echo "Test 5" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "--amend test" &&
+ git cat-file -p HEAD | grep "^author" > commit_1 &&
+ test_tick &&
+ git commit -m "Changed" --amend &&
+ git cat-file -p HEAD | grep "^author" > commit_2 &&
+ test_cmp commit_1 commit_2
+'
+
+test_expect_success '--amend with --mine makes me the author' '
+ test_tick &&
+ echo "Test 6" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "Changed again" --amend --mine &&
+ git cat-file -p HEAD | grep "^author" > commit_1 &&
+ test "$(cat commit_1 | sed "s/.*> //")" !=\
+ "$(cat commit_2 | sed "s/.*> //")" &&
+ test "$(cat commit_2 | sed -e "s/author //" -e "s/>.*/>/")" =\
+ "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
+'
+
+test_done
--
1.6.5.2.102.gdbd78.dirty
^ permalink raw reply related
* [PATCH] Makefile: add compat/bswap.h to LIB_H
From: Dmitry V. Levin @ 2009-11-01 23:09 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Starting with commit 51ea55190b6e72c77c96754c1bf2f149a4714848,
git-compat-util.h includes compat/bswap.h
Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---
Makefile | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index 268aede..3bca8d4 100644
--- a/Makefile
+++ b/Makefile
@@ -412,6 +412,7 @@ LIB_H += builtin.h
LIB_H += cache.h
LIB_H += cache-tree.h
LIB_H += commit.h
+LIB_H += compat/bswap.h
LIB_H += compat/cygwin.h
LIB_H += compat/mingw.h
LIB_H += csum-file.h
--
ldv
^ permalink raw reply related
* [PATCH] Update packfile transfer protocol documentation
From: Scott Chacon @ 2009-11-01 23:18 UTC (permalink / raw)
To: git list; +Cc: Shawn O. Pearce, Junio C Hamano
The technical documentation for the packfile protocol is both sparse and
incorrect. This documents the fetch-pack/upload-pack and send-pack/
receive-pack protocols much more fully.
Add documentation from Shawn's upcoming http-protocol docs that is shared
by the packfile protocol. protocol-common.txt describes ABNF notation
amendments, refname rules and the packet line format.
Add documentation on the various capabilities supported by the
upload-pack and receive-pack protocols. protocol-capabilities.txt describes
multi-ack, thin-pack, side-band[-64k], shallow, no-progress, include-tag,
ofs-delta, delete-refs and report-status.
Signed-Off-By: Scott Chacon <schacon@gmail.com>
---
I've incorporated the comments from Shawn (except for the 'annotated
tag' comment
in the 'include-tag' section of the capabilities doc, which I didn't
understand). I also
fixed the whitespace issues that were in most of the ABNF sections.
Documentation/technical/pack-protocol.txt | 529 +++++++++++++++++++--
Documentation/technical/protocol-capabilities.txt | 188 ++++++++
Documentation/technical/protocol-common.txt | 96 ++++
3 files changed, 772 insertions(+), 41 deletions(-)
create mode 100644 Documentation/technical/protocol-capabilities.txt
create mode 100644 Documentation/technical/protocol-common.txt
diff --git a/Documentation/technical/pack-protocol.txt
b/Documentation/technical/pack-protocol.txt
index 9cd48b4..1cc61d8 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -1,41 +1,488 @@
-Pack transfer protocols
-=======================
-
-There are two Pack push-pull protocols.
-
-upload-pack (S) | fetch/clone-pack (C) protocol:
-
- # Tell the puller what commits we have and what their names are
- S: SHA1 name
- S: ...
- S: SHA1 name
- S: # flush -- it's your turn
- # Tell the pusher what commits we want, and what we have
- C: want name
- C: ..
- C: want name
- C: have SHA1
- C: have SHA1
- C: ...
- C: # flush -- occasionally ask "had enough?"
- S: NAK
- C: have SHA1
- C: ...
- C: have SHA1
- S: ACK
- C: done
- S: XXXXXXX -- packfile contents.
-
-send-pack | receive-pack protocol.
-
- # Tell the pusher what commits we have and what their names are
- C: SHA1 name
- C: ...
- C: SHA1 name
- C: # flush -- it's your turn
- # Tell the puller what the pusher has
- S: old-SHA1 new-SHA1 name
- S: old-SHA1 new-SHA1 name
- S: ...
- S: # flush -- done with the list
- S: XXXXXXX --- packfile contents.
+Packfile transfer protocols
+===========================
+
+Git supports transferring data in packfiles over the ssh://, git:// and
+file:// transports. There exist two sets of protocols, one for pushing
+data from a client to a server and another for fetching data from a
+server to a client. All three transports (ssh, git, file) use the same
+protocol to transfer data.
+
+The processes invoked in the canonical Git implementation are 'upload-pack'
+on the server side and 'fetch-pack' on the client side for fetching data;
+then 'receive-pack' on the server and 'send-pack' on the client for pushing
+data. The protocol functions to have a server tell a client what is
+currently on the server, then for the two to negotiate the smallest amount
+of data to send in order to fully update one or the other.
+
+Transports
+----------
+There are three transports over which the packfile protocol is
+initiated. The Git transport is a simple, unauthenticated server that
+simply takes the command (almost always 'upload-pack', though Git
+servers can be configured to be globally writable, in which 'receive-
+pack' initiation is also allowed) with which the client wishes to
+communicate and executes it and connects it to the requesting
+process.
+
+In the SSH transport, the client basically just runs the 'upload-pack'
+or 'receive-pack' process on the server over the SSH protocol and then
+communicates with that invoked process over the SSH connection.
+
+The file:// transport simply runs the 'upload-pack' or 'receive-pack'
+process locally and communicates with it over a pipe.
+
+Git Protocol
+------------
+
+The Git protocol starts off by sending "git-receive-pack 'repo.git'"
+on the wire using the pkt-line format, followed by a null byte and a
+hostname paramater, terminated by a null byte.
+
+ 0032git-upload-pack /project.git\0host=myserver.com\0
+
+--
+ git-proto-request = request-command SP pathname NUL [ host-parameter NUL ]
+ request-command = "git-upload-pack" / "git-receive-pack" /
+ "git-upload-archive" ; case sensitive
+ pathname = *( %x01-ff ) ; exclude NUL
+ host-parameter = "host=" hostname [ ":" port ]
+--
+
+Only host-parameter is allowed in the git-proto-request. Clients
+MUST NOT attempt to send additional parameters. It is used for the
+git-daemon name based virtual hosting. See --interpolated-path
+option to git daemon, with the %H/%CH format characters.
+
+Basically what the Git client is doing to connect to an 'upload-pack'
+process on the server side over the Git protocol is this:
+
+ $ echo -e -n \
+ "0039git-upload-pack /schacon/gitbook.git\0host=example.com\0" |
+ nc -v example.com 9418
+
+
+SSH Protocol
+------------
+
+Initiating the upload-pack or receive-pack processes over SSH is
+simply executing the binary on the server via SSH remote execution.
+It is basically equivalent to running this:
+
+ $ ssh git.example.com "git-upload-pack '/project.git'"
+
+For a server to support Git pushing and pulling for a given user over
+SSH, that user needs to be able to execute one or both of those
+commands via the SSH shell that they are provided on login. On some
+systems, that shell access is limited to only being able to run those
+two commands, or even just one of them.
+
+In an ssh:// format URI, it's absolute in the URI, so the '/' after
+the host name (or port number) is sent as an argument, which is then
+read by the remote git-upload-pack exactly as is, so it's effectively
+an absolute path in the remote filesystem.
+
+ git clone ssh://user@example.com/project.git
+ |
+ v
+ ssh user@example.com "git-upload-pack '/project.git'"
+
+In a "user@host:path" format URI, its relative to the user's home
+directory, because the Git client will run:
+
+ git clone user@example.com:project.git
+ |
+ v
+ ssh user@example.com "git-upload-pack 'project.git'"
+
+The exception is if a '~' is used, in which case
+we execute it without the leading '/'.
+
+ ssh://user@example.com/~alice/project.git,
+ |
+ v
+ ssh user@example.com "git-upload-pack '~alice/project.git'"
+
+Fetching Data From a Server
+===========================
+
+When one Git repository wants to get all the data that a second
+repository has, the first can 'fetch' from the second. This
+operation determines what data the server has that the client does
+not then streams that data down to the client in packfile format.
+
+The server side binaries need to be executable as 'git-upload-pack'
+for fetching and 'git-receive-pack' for pushing over SSH, since the
+Git clients will connect to the server and attempt to run that command
+directly. Over the Git protocol, one could write their own daemon
+that sees that the client is trying to invoke those commands and
+simply handle the requests.
+
+
+Reference Discovery
+-------------------
+
+When the client initially connects the server will immediately respond
+with a listing of each reference it has (all branches and tags) along
+with the commit SHA that each reference currently points to.
+
+ $ echo -e -n \
+ "0039git-upload-pack /schacon/gitbook.git\0host=example.com\0" |
+ nc -v example.com 9418
+ 00887217a7c7e582c46cec22a130adf4b9d7d950fba0 HEAD\0multi_ack \
+ thin-pack side-band side-band-64k ofs-delta shallow no-progress \
+ include-tag
+ 00441d3fcd5ced445d1abc402225c0b8a1299641f497 refs/heads/integration
+ 003f7217a7c7e582c46cec22a130adf4b9d7d950fba0 refs/heads/master
+ 003cb88d2441cac0977faf98efc80305012112238d9d refs/tags/v0.9
+ 003c525128480b96c89e6418b1e40909bf6c5b2d580f refs/tags/v1.0
+ 003fe92df48743b7bc7d26bcaabfddde0a1e20cae47c refs/tags/v1.0^{}
+ 0000
+
+Server SHOULD terminate each non-flush line
+using LF ("\n") terminator; client MUST NOT complain if there is no
+terminator.
+
+The returned response is a pkt-line stream describing each ref and
+its known value. The stream SHOULD be sorted by name according to
+the C locale ordering. The stream SHOULD include the default ref
+named 'HEAD' as the first ref. The stream MUST include capability
+declarations behind a NUL on the first ref.
+
+----
+ advertised-refs = (no-refs / list-of-refs)
+ flush-pkt
+
+ no-refs = PKT-LINE(zero-id SP "capabilities^{}"
+ NUL capability-list LF)
+
+ list-of-refs = first-ref *other-ref
+ first-ref = PKT-LINE(obj-id SP refname
+ NUL capability-list LF)
+
+ other-ref = PKT-LINE(other-tip / other-peeled)
+ other-tip = obj-id SP refname LF
+ other-peeled = obj-id SP refname "^{}" LF
+
+ capability-list = capability *(SP capability)
+ capability = 1*(ALPHA / DIGIT / "-" / "_")
+----
+
+Server and client SHOULD use lowercase for SHA1, both MUST treat SHA1
+as case-insensitive.
+
+See protocol-capabilities.txt for a list of allowed server capabilities
+and descriptions.
+
+Packfile Negotiation
+--------------------
+After reference and capabilities discovery, the client can decide
+to terminate the connection by sending a flush-pkt, telling the
+server it can now gracefully terminate (as happens with the ls-remote
+command) or it can enter the negotiation phase, where the client and
+server determine what the minimal packfile necessary for transport is.
+
+Once the client has the initial list of references that the server
+has, as well as the list of capabilities, it will begin telling the
+server what objects it wants and what objects it has, so the server
+can make a packfile that only has the objects that the client needs.
+The client will also send a list of the capabilities it supports out
+of what the server said it could do with the first 'want' statement.
+
+----
+ upload-request = want-list
+ have-list
+ compute-end
+
+ want-list = first-want
+ *additional-want
+ flush-pkt
+
+ first-want = PKT-LINE("want" SP obj-id SP capability-list LF)
+ additional-want = PKT-LINE("want" SP obj-id LF)
+
+ have-list = *have-line
+ have-line = PKT-LINE("have" SP obj-id LF)
+ compute-end = flush-pkt / PKT-LINE("done")
+----
+
+Clients MUST send all the SHAs it wants from the reference
+discovery phase as 'want' lines. Clients MUST send at least one
+'want' command in the request body. Clients MUST NOT mention an
+obj-id in a 'want' command which did not appear in the response
+obtained through ref discovery.
+
+If client is requesting a shallow clone, it will now send a 'deepen'
+command with the depth it is requesting.
+
+Once all the "want"s (and optional 'deepen') are transferred,
+clients MUST send a flush. If the client has all the references on
+the server, client simply flushes and disconnects.
+
+TODO: shallow/unshallow response and document the deepen command in the ABNF.
+
+Now the client will send a list of the obj-ids it has. In multi_ack
+mode, the canonical implementation will send up to 32 of these at a
+time, then will send a flush-pkt. The canonical implementation will
+also skip ahead and send the next 32 immediately, so that there is
+always a block of 32 "in-flight on the wire" at a time.
+
+If the client has no objects (as in the case of a non-referencing
+clone), it will skip this phase, just send it's 'done' and wait for
+the packfile.
+
+If the server reads 'have' lines, it then will respond by ACKing any
+of the obj-ids the client said it had that the server also has. The
+server will ACK obj-ids differently depending on which ack mode is
+signaled by the client.
+
+In multi_ack mode:
+
+ * the server will respond with 'ACK obj-id continue' for any common
+ commits.
+
+ * once the server has found an acceptable common base commit and is
+ ready to make a packfile, it will blindly ACK all 'have' obj-ids
+ back to the client.
+
+ * the server will then send a 'NACK' and then wait for another response
+ from the client - either a 'done' or another list of 'have' lines.
+
+In multi_ack_detailed mode:
+
+ * the server will differentiate the ACKs where it is simply signaling
+ that it is ready to send data with 'ACK obj-id ready' lines, and
+ signals the identified common commits with 'ACK obj-id common' lines.
+
+Without multi_ack:
+
+ * upload-pack sends "ACK obj-id" on the first common object it finds.
+ After that it says nothing until the client gives it a "done".
+
+ * upload-pack sends "NAK" on a flush-pkt if no common object
+ has been found yet. If one has been found, and thus an ACK
+ was already sent, its silent on the flush-pkt.
+
+After the client has gotten enough ACK responses that it can determine
+that the server has enough information to send an efficient packfile
+(in the canonical implementation, this is determined when it has received
+enough ACKs that it can color everything left in the --date-order queue
+as common with the server, or the --date-order queue is empty), or the
+client determines that it wants to give up (in the canonical implementation,
+this is determined when the client sends 256 'have' lines without getting
+any of them ACKed by the server - meaning there is nothing in common and
+the server should just send all it's objects), then the client will send
+a 'done' command. The 'done' command signals to the server that the client
+is ready to receive it's packfile data.
+
+However, the 256 limit *only* turns on in the canonical client
+implementation if we have received at least one "ACK %s continue"
+during a prior round. This helps to ensure that at least one common
+ancestor is found before we give up entirely.
+
+Once the 'done' line is read from the client, the server will either
+send a final 'ACK obj-id' or it will send a 'NAK'. The server only sends
+ACK after 'done' if there is at least one common base and multi_ack or
+multi_ack_detailed is enabled. The server always sends NAK after 'done'
+if there is no common base found.
+
+Then the server will start sending it's packfile data.
+
+----
+ server-response = *ack_multi ack / nak
+ ack_multi = PKT-LINE("ACK" SP obj-id ack_status LF)
+ ack_status = "continue" / "common" / "ready"
+ ack = PKT-LINE("ACK SP obj-id LF)
+ nak = PKT-LINE("NAK" LF)
+----
+
+A simple clone may look like this (with no 'have' statements):
+
+----
+ C: 0054want 74730d410fcb6603ace96f1dc55ea6196122532d\0multi_ack \
+ side-band-64k ofs-delta\n
+ C: 0032want 7d1665144a3a975c05f1f43902ddaf084e784dbe\n
+ C: 0032want 5a3f6be755bbb7deae50065988cbfa1ffa9ab68a\n
+ C: 0032want 7e47fe2bd8d01d481f44d7af0531bd93d3b21c01\n
+ C: 0032want 74730d410fcb6603ace96f1dc55ea6196122532d\n
+ C: 0000
+ C: 0009done\n
+
+ S: 0008NAK\n
+ S: [PACKFILE]
+----
+
+An incremental update (fetch) response might look like this:
+
+----
+ C: 0054want 74730d410fcb6603ace96f1dc55ea6196122532d\0multi_ack \
+ side-band-64k ofs-delta\n
+ C: 0032want 7d1665144a3a975c05f1f43902ddaf084e784dbe\n
+ C: 0032want 5a3f6be755bbb7deae50065988cbfa1ffa9ab68a\n
+ C: 0000
+ C: 0032have 7e47fe2bd8d01d481f44d7af0531bd93d3b21c01\n
+ C: [30 more have lines]
+ C: 0032have 74730d410fcb6603ace96f1dc55ea6196122532d\n
+ C: 0000
+
+ S: 003aACK 7e47fe2bd8d01d481f44d7af0531bd93d3b21c01 continue\n
+ S: 003aACK 74730d410fcb6603ace96f1dc55ea6196122532d continue\n
+ S: 0008NAK\n
+
+ C: 0009done\n
+
+ S: 003aACK 74730d410fcb6603ace96f1dc55ea6196122532d\n
+ S: [PACKFILE]
+----
+
+
+Packfile Data
+-------------
+
+Now that the client and server have done some negotiation about what
+the minimal amount of data that can be sent to the client is, the server
+will construct and send the required data in packfile format.
+
+See pack-format.txt for what the packfile itself actually looks like.
+
+If 'side-band' or 'side-band-64k' capabilities have been specified by
+the client, the server will send the packfile data multiplexed.
+
+Each packet starting with the packet-line length of the amount of data
+that follows, followed by a single byte specifying the sideband the
+following data is coming in on.
+
+In 'side-band' mode, it will send 999 data bytes plus 1 control code,
+for a total of 1000 bytes in a pkt-line. In 'side-band-64k' mode it
+will send 65519 data bytes plus 1 control code, for a total of 65520
+bytes in a pkt-line.
+
+The sideband byte will be either a '1' or a '2'. Sideband '1' will contain
+packfile data, sideband '2' will be used for progress information that the
+client will generally print to stderr.
+
+If no 'side-band' capability was specified, the server will simply
+stream the entire packfile without multiplexing.
+
+
+Pushing Data To a Server
+========================
+
+Pushing data to a server will invoke the 'receive-pack' process on the
+server, which will allow the client to tell it which references it should
+update and then send all the data the server will need for those new
+references to be complete. Once all the data is received and validated,
+the server will then update it's references to what the client specified.
+
+Authentication
+--------------
+
+The protocol itself contains no authentication mechanisms. That is to be
+handled by the transport, such as SSH, before the 'receive-pack' process is
+invoked. If 'receive-pack' is configured over the Git transport, those
+repositories will be writable by anyone who can access that port (9418) as
+that transport is unauthenticated.
+
+Reference Discovery
+-------------------
+
+The reference discovery phase is done nearly the same way as it is in the
+fetching protocol. Each reference obj-id and name on the server is sent
+in packet-line format to the client, followed by a flush packet. The only
+real difference is that the capability listing is different - the only
+possible values are 'report-status', 'delete-refs' and 'ofs-delta'.
+
+Reference Update Request and Packfile Transfer
+----------------------------------------------
+
+Once the client knows what references the server is at, it can send a
+list of reference update requests. For each reference on the server
+that it wants to update, it sends a line listing the obj-id currently on
+the server, the obj-id the client would like to update it to and the name
+of the reference.
+
+This list is followed by a flush packet and then the packfile that should
+contain all the objects that the server will need to complete the new
+references.
+
+The pack-file MUST NOT be sent if the only command used is 'delete'.
+
+A pack-file MUST be sent if either create or update command is used.
+An empty pack-file MUST be sent if a create or update command is
+used, and the server already obviously has the object (e.g. the
+SHA-1 is already pointed to by another ref that was listed in the
+advertisement).
+
+----
+ update-request = command-list [pack-file]
+
+ command-list = PKT-LINE(command NUL capability-list LF)
+ *PKT-LINE(command LF)
+ flush-pkt
+
+ command = create / delete / update
+ create = zero-id SP new-id SP name
+ delete = old-id SP zero-id SP name
+ update = old-id SP new-id SP name
+
+ old-id = obj-id
+ new-id = obj-id
+
+ pack-file = "PACK" 24*(OCTET)
+----
+
+The server will receive the packfile, unpack it, then validate each
+reference that is being updated that it hasn't changed while the request
+was being processed (the obj-id is still the same as the old-id), and
+it will run any update hooks to make sure that the update is acceptable.
+If all of that is fine, the server will then update the references.
+
+Report Status
+-------------
+
+If the 'report-status' capability is sent by the client, then the server
+will send a short report of what happened in that update. It will first
+list the status of the packfile unpacking as either 'unpack ok' or
+'unpack [error]'. Then it will list the status for each of the references
+that it tried to update. Each line be either 'ok [refname]' if the
+update was successful, or 'ng [refname] [error]' if the update was not.
+
+----
+ report-status = unpack-status
+ 1*(command-status)
+ flush-pkt
+
+ unpack-status = PKT-LINE("unpack" SP unpack-result LF)
+ unpack-result = "ok" / error-msg
+
+ command-status = command-ok / command-fail
+ command-ok = PKT-LINE("ok" SP refname LF)
+ command-fail = PKT-LINE("ng" SP refname SP error-msg LF)
+
+ error-msg = 1*(OCTECT) ; where not "ok"
+----
+
+Updates can be unsuccessful for a number of reasons. The reference can have
+changed since the reference discovery phase was originally sent, meaning
+someone pushed in the meantime. The reference being pushed could be a
+non-fast-forward reference and the update hooks or configuration could be
+set to not allow that, etc. Also, some references can be updated while others
+can be rejected.
+
+An example client/server communication might look like this:
+
+----
+ S: 007c74730d410fcb6603ace96f1dc55ea6196122532d
refs/heads/local\0report-status delete-refs ofs-delta\n
+ S: 003e7d1665144a3a975c05f1f43902ddaf084e784dbe refs/heads/debug\n
+ S: 003f74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/master\n
+ S: 003f74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/team\n
+ S: 0000
+
+ C: 003e7d1665144a3a975c05f1f43902ddaf084e784dbe
74730d410fcb6603ace96f1dc55ea6196122532d refs/heads/debug\n
+ C: 003e74730d410fcb6603ace96f1dc55ea6196122532d
5a3f6be755bbb7deae50065988cbfa1ffa9ab68a refs/heads/master\n
+ C: 0000
+ C: [PACKDATA]
+
+ S: 000aunpack ok\n
+ S: 0014ok refs/heads/debug\n
+ S: 0026ng refs/heads/master non-fast-forward\n
+----
diff --git a/Documentation/technical/protocol-capabilities.txt
b/Documentation/technical/protocol-capabilities.txt
new file mode 100644
index 0000000..3c86fc3
--- /dev/null
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -0,0 +1,188 @@
+Git Protocol Capabilities
+=========================
+
+Servers SHOULD support all capabilities defined in this document.
+
+On the very first line of the initial server response, the first
+reference is followed by a null byte and then a list of space
+delimited server capabilities. These allow the server to declare
+what it can and cannot do to the client.
+
+Client sends space separated list of capabilities it wants. It
+SHOULD send a subset of server capabilities, i.e do not send
+capabilities served does not advertise. The client SHOULD NOT ask
+for capabilities the server did not say it supports.
+
+Server MUST ignore capabilities it does not understand. Server MUST
+NOT ignore capabilities that client requested and server advertised.
+
+The 'report-status' and 'delete-refs' capabilities are sent and
+recognized by the receive-pack (push to server) process.
+
+The 'ofs-delta' capability is sent and recognized by both upload-pack
+and receive-pack protocols.
+
+All other capabilities are only recognized by the upload-pack (fetch
+from server) process.
+
+multi_ack
+---------
+
+The 'multi_ack' capability allows the server to return "ACK $SHA1
+continue" as soon as it finds a commit that it can use as a common
+base, between the client's wants and the client's have set.
+
+By sending this early, the server can potentially head off the client
+from walking any further down that particular branch of the client's
+repository history. The client may still need to walk down other
+branches, sending have lines for those, until the server has a
+complete cut across the DAG, or the client has said "done".
+
+Without multi_ack, a client sends have lines in --date-order until
+the server has found a common base. That means the client will send
+have lines that are already known by the server to be common, because
+they overlap in time with another branch that the server hasn't found
+a common base on yet.
+
+The client has things in caps that the server doesn't; server has
+things in lower case.
+
+ +---- u ---------------------- x
+ / +----- y
+ / /
+ a -- b -- c -- d -- E -- F
+ \
+ +--- Q -- R -- S
+
+If the client wants x,y and starts out by saying have F,S, the server
+doesn't know what F,S is. Eventually the client says "have d" and
+the server sends "ACK d continue" to let the client know to stop
+walking down that line (so don't send c-b-a), but its not done yet,
+it needs a base for X. The client keeps going with S-R-Q, until a
+gets reached, at which point the server has a clear base and it all
+ends.
+
+Without multi_ack the client would have sent that c-b-a chain anyway,
+interleaved with S-R-Q.
+
+thin-pack
+---------
+
+Server can send thin packs, i.e. packs which do not contain base
+elements, if those base elements are available on clients side.
+Client requests thin-pack capability when it understands how to "thicken"
+them adding required delta bases making them independent.
+
+Client MUST NOT request 'thin-pack' capability if it
+cannot turn thin packs into proper independent packs.
+
+
+side-band, side-band-64k
+------------------------
+
+This means that server can send, and client understand multiplexed
+(muxed) progress reports and error info interleaved with the packfile
+itself.
+
+These two options are mutually exclusive. A client should ask for
+only one of them, and a modern client always favors side-band-64k.
+
+Either mode indicates that the packfile data will be streamed broken
+up into packets of either 1000 bytes in the case of 'side_band', or
+65520 bytes in the case of 'side_band_64k'. Each packet is made up of
+a leading 4-byte pkt-line length of how much data is in the packet,
+followed by a 1-byte stream code, followed by the actual data.
+
+The stream code can be one of:
+
+ 1 - pack data
+ 2 - progress messages
+ 3 - fatal error message just before stream aborts
+
+The "side-band-64k" capability came about as a way for newer clients
+that can handle much larger packets to request packets that are
+actually crammed nearly full, while maintaining backward compatibility
+for the older clients.
+
+Further, with side-band and its 1000 byte messages, it's actually
+999 bytes of payload and 1 byte for the stream code. With side-band-64k,
+same deal, you have 65519 bytes of data and 1 byte for the stream code.
+
+The client MUST send only maximum of one of "side-band" and "side-
+band-64k". Server MUST favor side-band-64k if client requests both.
+
+ofs-delta
+---------
+
+Server can send, and client understand PACKv2 with delta refering to
+its base by position in pack rather than by SHA-1. Its that they can
+send/read OBJ_OFS_DELTA, aka type 6 in a pack file.
+
+shallow
+-------
+
+This capability adds "deepen", "shallow" and "unshallow" commands to
+the fetch-pack/upload-pack protocol so clients can request shallow
+clones.
+
+no-progress
+-----------
+
+The client was started with "git clone -q" or something, and doesn't
+want that side band 2. Basically the client just says "I do not
+wish to receive stream 2 on sideband, so do not send it to me, and if
+you did, I will drop it on the floor anyway". However, the sideband
+channel 3 is still used for error responses.
+
+include-tag
+-----------
+
+The 'include-tag' capability is about sending tags if we are sending
+objects they point to. If we pack an object to the client, and a tag
+points exactly at that object, we pack the tag too. In general this
+allows a client to get all new tags when it fetches a branch, in a
+single network connection.
+
+Clients MAY always send include-tag, hardcoding it into a request.
+The decision for a client to request include-tag only has to do with
+the client's desires for tag data, whether or not a server had
+advertised objects in the refs/tags/* namespace.
+
+Clients SHOULD NOT send include-tag if remote.name.tagopt was set to
+--no-tags, as the client doesn't want tag data.
+
+Servers MUST accept include-tag without error or warning, even if the
+server does not understand or support the option.
+
+Servers SHOULD pack the tags if their referrant is packed and the
+client has requested include-tag.
+
+Clients MUST be prepared for the case where a server has ignored
+include-tag and has not actually sent tags in the pack. In such
+cases the client SHOULD issue a subsequent fetch to acquire the tags
+that include-tag would have otherwise given the client.
+
+The server SHOULD send include-tag, if it supports it, irregardless
+of whether or not there are tags available.
+
+report-status
+-------------
+
+The upload-pack process can receive a 'report-status' capability,
+which tells it that the client wants a report of what happened after
+a packfile upload and reference update. If the pushing client requests
+this capability, after unpacking and updating references the server
+will respond with whether the packfile unpacked successfully and if
+each reference was updated successfully. If any of those were not
+successful, it will send back an error message. See pack-protocol.txt
+for example messages.
+
+delete-refs
+-----------
+
+If the server sends back the 'delete-refs' capability, it means that
+it is capable of accepting an all-zeroed SHA-1 value as the target
+value of a reference update. It is not sent back by the client, it
+simply informs the client that it can be sent zeroed SHA-1 values
+to delete references.
+
diff --git a/Documentation/technical/protocol-common.txt
b/Documentation/technical/protocol-common.txt
new file mode 100644
index 0000000..ddf9912
--- /dev/null
+++ b/Documentation/technical/protocol-common.txt
@@ -0,0 +1,96 @@
+Documentation Common to Pack and Http Protocols
+===============================================
+
+ABNF Notation
+-------------
+
+ABNF notation as described by RFC 5234 is used within the protocol documents,
+except the following replacement core rules are used:
+----
+ HEXDIG = DIGIT / "a" / "b" / "c" / "d" / "e" / "f"
+----
+
+We also define the following common rules:
+----
+ NUL = %x00
+ zero-id = 40*"0"
+ obj-id = 40*(HEXDIGIT)
+
+ refname = "HEAD"
+ refname /= "refs/" <see discussion below>
+----
+
+A refname is a hierarichal octet string beginning with "refs/" and
+not violating the 'git-check-ref-format' command's validation rules.
+More generally, they:
+
+. They can include slash `/` for hierarchical (directory)
+ grouping, but no slash-separated component can begin with a
+ dot `.`.
+
+. They must contain at least one `/`. This enforces the presence of a
+ category like `heads/`, `tags/` etc. but the actual names are not
+ restricted.
+
+. They cannot have two consecutive dots `..` anywhere.
+
+. They cannot have ASCII control characters (i.e. bytes whose
+ values are lower than \040, or \177 `DEL`), space, tilde `~`,
+ caret `{caret}`, colon `:`, question-mark `?`, asterisk `*`,
+ or open bracket `[` anywhere.
+
+. They cannot end with a slash `/` nor a dot `.`.
+
+. They cannot end with the sequence `.lock`.
+
+. They cannot contain a sequence `@{`.
+
+. They cannot contain a `\\`.
+
+
+pkt-line Format
+---------------
+
+Much (but not all) of the payload is described around pkt-lines.
+
+A pkt-line is a variable length binary string. The first four bytes
+of the line, the pkt-len, indicates the total length of the line,
+in hexadecimal. The pkt-len includes the 4 bytes used to contain
+the length's hexadecimal representation.
+
+A pkt-line MAY contain binary data, so implementors MUST ensure
+pkt-line parsing/formatting routines are 8-bit clean.
+
+A non-binary line SHOULD BE terminated by an LF, which if present
+MUST be included in the total length.
+
+The maximum length of a pkt-line's data component is 65520 bytes.
+Implementations MUST NOT send pkt-line whose length exceeds 65524
+(65520 bytes of payload + 4 bytes of length data).
+
+Implementations SHOULD NOT send an empty pkt-line ("0004").
+
+A pkt-line with a length field of 0 ("0000"), called a flush-pkt,
+is a special case and MUST be handled differently than an empty
+pkt-line ("0004").
+
+----
+ pkt-line = data-pkt / flush-pkt
+
+ data-pkt = pkt-len pkt-payload
+ pkt-len = 4*(HEXDIG)
+ pkt-payload = (pkt-len - 4)*(OCTET)
+
+ flush-pkt = "0000"
+----
+
+Examples (as C-style strings):
+
+----
+ pkt-line actual value
+ ---------------------------------
+ "0006a\n" "a\n"
+ "0005a" "a"
+ "000bfoobar\n" "foobar\n"
+ "0004" ""
+----
--
1.6.5.2.340.g000a3e.dirty
^ permalink raw reply related
* Re: [PATCH] commit -c/-C/--amend: acquire authorship and restamp time with --claim
From: Erick Mattos @ 2009-11-01 23:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr5sixbd1.fsf@alter.siamese.dyndns.org>
2009/11/1 Junio C Hamano <gitster@pobox.com>:
> Erick Mattos <erick.mattos@gmail.com> writes:
>
>> + OPT_BOOLEAN(0, "claim", &claim, "acquire authorship and restamp time of resulting commit"),
>
> It is unclear from where it is "acquire"-ing, nor what "restamp" means.
>
> Here are my attempts to come up with better wording:
>
> "ignore author and timestamp of the original commit (used with -C/-c/--amend)"
> "the commit is authored by me now (used with -C/-c/--amend)"
Two new suggestions:
"reset authorship and timestamp to you (-C-c/--amend)"
# On this case "you" can be changed to "commiter" but the text
is going to be bigger
# and wrap.
"make me new author with new timestamp (-C-c/--amend)"
I am just waiting for your answers to this e-mail and to the previous
one so I can send you another patch.
Regards
^ permalink raw reply
* Re: Fw: git-core: SIGSEGV during {peek,ls}-remote on HTTP remotes.
From: Jeff King @ 2009-11-01 23:04 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Junio C Hamano, Sverre Rabbelier, Samium Gromoff, git,
Tay Ray Chuan, Mike Hommey
In-Reply-To: <alpine.LNX.2.00.0911011518590.14365@iabervon.org>
On Sun, Nov 01, 2009 at 04:19:34PM -0500, Daniel Barkalow wrote:
> > > Do things like git_path() fail cleanly if there was no git directory? If
> > > not, there should probably be tests of nongit on paths that actually need
> > > a git directory,...
> >
> > I don't know. Again, you tell me ;-)
>
> I'm not an expert on that part. But it looks like it misbehaves, returning
> ".git/foo" even when that path doesn't make sense.
I will not admit to being an expert in that area, but yes, that is what
I observed before while looking into some of our weird startup problems.
We really have two systems for setting up the environment:
- setup_git_directory, which tries to do everything at the outset (but
which we don't necessarily run for all commands, and where "outset"
is defined as when the git wrapper handles an actual command, which
means we sometimes do quite a bit of stuff beforehand)
- some lazy magic in environment.c, mostly setup_git_env. If
setup_git_directory has been run, this does the right thing because
it reads GIT_DIR from the environment as set previously. But if not,
then it can quite often do the wrong thing (as you noticed).
I tried simply ditching the lazy magic, but that doesn't work: there are
many cases where setup_git_directory hasn't been run. Moving it to the
very beginning doesn't quite work, either. I don't remember the details,
sadly. It may be that the lazy setup_git_env magic should, rather than
doing anything itself, call setup_git_directory if it has not been
initialized. But at that point, you might as well setup_git_directory in
every program, since just about every one is going to want to look at
git_path at some point.
Sorry, I know that is vague. Refactoring this area has been on my plate
for so long that I have forgotten all the details, and it is such a
messy area that I am continually procrastinating on diving back into it.
;)
-Peff
^ permalink raw reply
* Re: Fw: git-core: SIGSEGV during {peek,ls}-remote on HTTP remotes.
From: Daniel Barkalow @ 2009-11-01 21:19 UTC (permalink / raw)
To: Junio C Hamano
Cc: Sverre Rabbelier, Samium Gromoff, git, Tay Ray Chuan, Mike Hommey
In-Reply-To: <7vljiqxapw.fsf@alter.siamese.dyndns.org>
On Sun, 1 Nov 2009, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > On Sat, 31 Oct 2009, Junio C Hamano wrote:
> > ...
> >> Attached is a minimum fix/work around, but this is done without being very
> >> familiar with the current assumptions in the codepaths involved.
> >>
> >> Issues I want area experts to consider before coming up with the final fix
> >> are:
> >> ...
> > I think there's no benefit to allowing NULL for the remote; I think you
> > can always get a struct remote for what you want to access. So it's
> > probably just as well to require it, particularly because, as in the case
> > of cmd_ls_remote() below, you'd need a special case to not get a struct
> > remote.
> >
> > Is there any way in which the intended semantics of "transport_get(NULL,
> > url)" is not the same as "transport_get(remote_get(url), url)"?
> > (And, in the extended series, I make "transport_get(remote_get(url),
> > NULL)" also mean the same thing as above, while "transport_get(NULL,
> > NULL)" is obviously underspecified.)
>
> That was really my question to people who are involved in the transport
> layer code. I didn't know how transport->url and transport->remote->url
> are intended to relate to each other, for example, and that was why you
> were on Cc list. In other words, you are the area expert, you tell me ;-)
transport->remote->url[...] has all of the URLs for a remote;
transport->url has the particular one this struct transport is supposed to
contact. At least, that was my intent, but I think some of the code that
uses struct transports didn't realize that remote_get() will work in any
context where transport_get() will work. (And this might have been broken
at the time).
> Sverre seemed to think slightly differently; perhaps having worked on the
> foreign vcs interface he has some other input.
I think we agree that transport-helper.c ought to be able to deal with
anything transport.c will put together. But I'd also like to tighten what
transport.c will put together and I don't think he considered that
possibility.
> > Do things like git_path() fail cleanly if there was no git directory? If
> > not, there should probably be tests of nongit on paths that actually need
> > a git directory,...
>
> I don't know. Again, you tell me ;-)
I'm not an expert on that part. But it looks like it misbehaves, returning
".git/foo" even when that path doesn't make sense.
> It probably makes sesne as you outlined in the earlier part of your
> response for the caller to give a bit more clue to the helper to help
> making such a decision.
Yeah, it's probably worth getting that in at this point. I'll write up
some patches.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: git-daemon in IPv4 and IPv6 mode
From: Vineet Kumar @ 2009-11-01 21:11 UTC (permalink / raw)
To: git
In-Reply-To: <4AEB5640.1010708@hartwork.org>
* Sebastian Pipping <webmaster <at> hartwork.org> writes:
> it seems like the git daemon cannot bind to both IPv4 and IPv6 at the
> same time, yet.
It doesn't seem that way here. I just tried to run a git-daemon and
it listened by default on 0.0.0.0:9418 (v4) and :::9418 (v6). I could
connect using either v4 or v6. Is yours behaving differently?
Have a look at the output from "strace -ebind,socket,listen ./git-daemon".
Vineet
^ permalink raw reply
* Re: [PATCH] commit -c/-C/--amend: acquire authorship and restamp time with --claim
From: Erick Mattos @ 2009-11-01 20:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr5sixbd1.fsf@alter.siamese.dyndns.org>
2009/11/1 Junio C Hamano <gitster@pobox.com>:
> Erick Mattos <erick.mattos@gmail.com> writes:
>
>> The new --claim option is meant to solve this need by regenerating the
>> timestamp and setting as new author the committer or the one specified
>> on --author option.
>
> Thanks.
>
> I don't think "claim" is a good name for this option. It makes me go
> "huh, I do not get it. What are you claiming? Claiming that this is the
> correct fix?"
>
> Renaming it to "claim-authorship" may avoid that confusion, but it is too
> long.
>
> How about naming this option "mine"?
Makes sense: "mine" then. I felt you haven't like the name.
>> @@ -61,14 +61,19 @@ OPTIONS
>> -C <commit>::
>> --reuse-message=<commit>::
>> Take an existing commit object, and reuse the log message
>> - and the authorship information (including the timestamp)
>> - when creating the commit.
>> + and the authorship information when creating the commit.
>
> I don't think this is a good change.
>
> When you use the new option, the author name, email and timestamp are
> ignored, and when you don't, they are all used.
>
> To new users who are taught to first set user.name and user.email via
> configuration variables, the phrase "authorship information" would mean
> <name, email> pair, and the explanation in the parentheses helps to avoid
> a misunderstanding that these two are the only things that are copied.
>
> I would suggest you keep the original text.
I think it is pointless but if you say so.
>> +--claim::
>> + When used with -C/-c/--amend options, declare that the
>> + authorship of the resulting commit now belongs of the committer.
>> + This also renews the author timestamp. Therefore this option
>> + sets the use of only the message from the original commit.
>
> I don't understand/parse the last sentence; I don't think it is necessary,
> either.
Just trying to clarify. I think people that would be searching for
renewing timestamp as I was would find it easier to identify the
correct option.
>> diff --git a/builtin-commit.c b/builtin-commit.c
>> index c395cbf..919e3fe 100644
>> --- a/builtin-commit.c
>> +++ b/builtin-commit.c
>> @@ -51,7 +51,7 @@ static const char *template_file;
>> static char *edit_message, *use_message;
>> static char *author_name, *author_email, *author_date;
>> static int all, edit_flag, also, interactive, only, amend, signoff;
>> -static int quiet, verbose, no_verify, allow_empty, dry_run;
>> +static int quiet, verbose, no_verify, allow_empty, dry_run, claim;
>
> Even if you name the command option "claim" in order to keep it short, I
> think it is a bad idea to name the variable "claim", because it doesn't
> say _what_ you are claiming and is confusing. Naming it claim_authorship
> would be better.
Changed to mine.
>> + OPT_BOOLEAN(0, "claim", &claim, "acquire authorship and restamp time of resulting commit"),
>
> It is unclear from where it is "acquire"-ing, nor what "restamp" means.
>
> Here are my attempts to come up with better wording:
>
> "ignore author and timestamp of the original commit (used with -C/-c/--amend)"
> "the commit is authored by me now (used with -C/-c/--amend)"
What about "take new timestamp and make me new author (with -C/-c/--amend)"?
> The latter will work well if the option is renamed to "--mine".
>
> What should happen when the user uses --claim without -C/-c/--amend?
>
> % git commit --claim
>
> Should you detect an error? Does your code do so? Do you have a test
> that catches this error?
You say it. In the first patch of mine I was testing for
--(new|old)-timestamp. Now I thought it was unnecessary because the
normal behavior is documented and by comparison with other options
which does not test combinations extensively. But it is just code to
add if you want...
> What should happen when the user uses --author and --claim at the same time?
>
> % git commit --claim --author='Erick Mattos <eric@mattos>' -C HEAD
>
> Should you detect an error? Does your code do so? Do you have a test
> that catches this error?
It works as intended. Both together.
>> diff --git a/t/t7509-commit.sh b/t/t7509-commit.sh
>> new file mode 100755
>> index 0000000..62fb00f
>> --- /dev/null
>> +++ b/t/t7509-commit.sh
>> @@ -0,0 +1,87 @@
>> +#!/bin/sh
>> +#
>> +# Copyright (c) 2009 Erick Mattos
>> +#
>> +
>> +test_description='git commit
>> +
>> +Tests for --claim option on a commit.'
>> +
>> +. ./test-lib.sh
>> +
>> +TEST_FILE="$PWD"/foo
>
> Why does this have to be given as a full path, not just "foo"?
Templating from other scripts. Ask them... :-)
>> +test_expect_success '-C option should be working' '
>
> Every test is about "should be working", so you are wasting 16 letters or
> so without giving any useful information.
>
> Say something like "-C without --claim uses the author from the old commit" here.
>
>> + echo "Initial" > "$TEST_FILE" &&
>> + git add "$TEST_FILE" &&
>> + git commit -m "Initial Commit" --author Frigate\ \<flying@over.world\> &&
>> + sleep 1 &&
>
> If you use "test_tick", you don't have to slow the test down. You use
> "test_tick" before you commit to increment the time. Look at t6036 for an
> example.
Easy change.
>> + echo "Test 1" >> "$TEST_FILE" &&
>> + git add "$TEST_FILE" &&
>> + git commit -C HEAD &&
>> + git cat-file -p HEAD^ | sed -e '/^parent/d' -e '/^tree/d' -e '/^committer/d' > commit_1 &&
>> + git cat-file -p HEAD | sed -e '/^parent/d' -e '/^tree/d' -e '/^committer/d' > commit_2 &&
>> + cmp commit_1 commit_2
>> +'
>
> Use "test_cmp" instead, so that errors can be seen easily when somebody
> breaks this new feature.
Easy change.
>> +test_expect_success '-C option with --claim is working properly' '
>
> Again, "working properly" is a meaningless thing to say because that is
> what all tests check. "-C with --claim makes me the author" would be
> better.
>
>> + sleep 1 &&
>> + echo "Test 2" >> "$TEST_FILE" &&
>> + git add "$TEST_FILE" &&
>> + git commit -C HEAD^ --claim &&
>> + git cat-file -p HEAD^ | grep '^author' > commit_1 &&
>> + git cat-file -p HEAD | grep '^author' > commit_2 &&
>> + test_must_fail cmp commit_1 commit_2
>
> This test shouldn't be happy with any random author information that
> happens to be different from the original. The purpose of --claim option
> is to take the authorship, make it mine (or whoever is specified with
> GIT_AUTHOR_NAME or user.name or uid-to-gecos), so the last cmp (again, it
> should use test_cmp) should make sure that the author is 'A U Thor', not
> just being different from "Frigate" or whatever. It should check email
> and timestamp as well, of course.
Good point but the text imho is a little unnecessary because the
change could only be to user.name, user.email, ...
But if want that it will be an easy change too.
>> +'
>> +
>> +test_expect_success '-c option should be working' '
>> + echo "Initial" > "$TEST_FILE" &&
>> + git add "$TEST_FILE" &&
>> + git commit -m "Initial Commit" --author Frigate\ \<flying@over.world\> &&
>> + sleep 1 &&
>> + echo "Test 3" >> "$TEST_FILE" &&
>> + git add "$TEST_FILE" &&
>> + git commit -c HEAD <<EOF
>> + "Changed"
>> + EOF &&
>
> What editor is reading this "Changed"?
>
Nobody cares... Just a text to change the file.
^ permalink raw reply
* Re: Fw: git-core: SIGSEGV during {peek,ls}-remote on HTTP remotes.
From: Sverre Rabbelier @ 2009-11-01 20:54 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Junio C Hamano, Samium Gromoff, git, Tay Ray Chuan, Mike Hommey
In-Reply-To: <alpine.LNX.2.00.0911011504460.14365@iabervon.org>
Heya,
On Sun, Nov 1, 2009 at 21:16, Daniel Barkalow <barkalow@iabervon.org> wrote:
> If we change the ls-remote.c case, it becomes impossible for a struct
> transport to ever have a NULL remote field. And the change to ls-remote
> removes a special case. I'd go so far as to say that ls-remote.c should
> provide a struct remote, and transport_get should enforce that there's a
> struct remote.
If that is the case (that we can eliminate the only special case), I
agree that we should fix it there, where it will be the least effort.
I got the impression from Junio's original post that there are
multiple places that would have to be fixed, and I figured that we
should fix it where it will be the least amount of effort :).
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: Fw: git-core: SIGSEGV during {peek,ls}-remote on HTTP remotes.
From: Daniel Barkalow @ 2009-11-01 20:16 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Junio C Hamano, Samium Gromoff, git, Tay Ray Chuan, Mike Hommey
In-Reply-To: <fabb9a1e0911010646v2043bdb7l9215f1114e9e8385@mail.gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 620 bytes --]
On Sun, 1 Nov 2009, Sverre Rabbelier wrote:
> Heya,
>
> On Sun, Nov 1, 2009 at 05:27, Junio C Hamano <gitster@pobox.com> wrote:
> > - Should we fix get_helper() in transport-helper.c, instead of touching
> > ls-remote.c like this patch does?
>
> Probably, yes.
If we change the ls-remote.c case, it becomes impossible for a struct
transport to ever have a NULL remote field. And the change to ls-remote
removes a special case. I'd go so far as to say that ls-remote.c should
provide a struct remote, and transport_get should enforce that there's a
struct remote.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: Fw: git-core: SIGSEGV during {peek,ls}-remote on HTTP remotes.
From: Junio C Hamano @ 2009-11-01 20:15 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Junio C Hamano, Sverre Rabbelier, Samium Gromoff, git,
Tay Ray Chuan, Mike Hommey
In-Reply-To: <alpine.LNX.2.00.0911011348190.14365@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> On Sat, 31 Oct 2009, Junio C Hamano wrote:
> ...
>> Attached is a minimum fix/work around, but this is done without being very
>> familiar with the current assumptions in the codepaths involved.
>>
>> Issues I want area experts to consider before coming up with the final fix
>> are:
>> ...
> I think there's no benefit to allowing NULL for the remote; I think you
> can always get a struct remote for what you want to access. So it's
> probably just as well to require it, particularly because, as in the case
> of cmd_ls_remote() below, you'd need a special case to not get a struct
> remote.
>
> Is there any way in which the intended semantics of "transport_get(NULL,
> url)" is not the same as "transport_get(remote_get(url), url)"?
> (And, in the extended series, I make "transport_get(remote_get(url),
> NULL)" also mean the same thing as above, while "transport_get(NULL,
> NULL)" is obviously underspecified.)
That was really my question to people who are involved in the transport
layer code. I didn't know how transport->url and transport->remote->url
are intended to relate to each other, for example, and that was why you
were on Cc list. In other words, you are the area expert, you tell me ;-)
Sverre seemed to think slightly differently; perhaps having worked on the
foreign vcs interface he has some other input.
>> - When helping to handle ls-remote request, there is no need for the
>> helper to know anything about the local state. We probably shouldn't
>> even run setup_git_directory_gently() at all in this case. But when
>> helping other kinds of request, the helper does need to know where our
>> repository is.
>>
>> In general, what should the initial environment for helpers be? Should
>> they assume that they have to figure out where the git repository is
>> themselves (in other words, should they assume they cannot rely on
>> anything the caller does before they are called? Would the caller
>> generally have done the usual repo discovery (including chdir() to the
>> toplevel), and there are some set of assumptions they can make? If so
>> what are they?
>
> Probably, the helper should be run with a predicable initial environment,
> simply because operations that use remote repositories are most often run
> from the toplevel of a repo, so people will fail to notice their bugs
> which trigger on running from subdirectories....
> Perhaps we should actively tell the helper if there is no git repository
> (or, if any git repository we happen to be in is merely coincidental and
> shouldn't affect the helper)? Helpers involving importing will probably
> want to know they don't have a private refs namespace, private state
> directory, etc. even for implementing "list" for ls-remote, and it would
> probably be best to require helper authors to report that they've
> considered this possibility before trying to use it.
I think that is a sane approach.
>> diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
>> index 78a88f7..a8d5613 100644
>> --- a/builtin-ls-remote.c
>> +++ b/builtin-ls-remote.c
>> @@ -86,7 +86,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>> pattern[j - i] = p;
>> }
>> }
>> - remote = nongit ? NULL : remote_get(dest);
>> + remote = remote_get(dest);
>> if (remote && !remote->url_nr)
>> die("remote %s has no configured URL", dest);
>> transport = transport_get(remote, remote ? remote->url[0] : dest);
>
> You can also drop the two checks for remote being non-NULL here, since
> it's now always non-NULL...
You are probably right; I didn't even look when I did the above.
>> diff --git a/remote-curl.c b/remote-curl.c
>> index ad6a163..7c83f77 100644
>> --- a/remote-curl.c
>> +++ b/remote-curl.c
>> @@ -81,8 +81,9 @@ int main(int argc, const char **argv)
>> struct strbuf buf = STRBUF_INIT;
>> const char *url;
>> struct walker *walker = NULL;
>> + int nongit = 0;
>>
>> - setup_git_directory();
>> + setup_git_directory_gently(&nongit);
>> if (argc < 2) {
>> fprintf(stderr, "Remote needed\n");
>> return 1;
>
> Do things like git_path() fail cleanly if there was no git directory? If
> not, there should probably be tests of nongit on paths that actually need
> a git directory,...
I don't know. Again, you tell me ;-)
It probably makes sesne as you outlined in the earlier part of your
response for the caller to give a bit more clue to the helper to help
making such a decision.
^ permalink raw reply
* Re: [PATCH] commit -c/-C/--amend: acquire authorship and restamp time with --claim
From: Junio C Hamano @ 2009-11-01 20:02 UTC (permalink / raw)
To: Erick Mattos; +Cc: git
In-Reply-To: <1257101127-8196-1-git-send-email-erick.mattos@gmail.com>
Erick Mattos <erick.mattos@gmail.com> writes:
> The new --claim option is meant to solve this need by regenerating the
> timestamp and setting as new author the committer or the one specified
> on --author option.
Thanks.
I don't think "claim" is a good name for this option. It makes me go
"huh, I do not get it. What are you claiming? Claiming that this is the
correct fix?"
Renaming it to "claim-authorship" may avoid that confusion, but it is too
long.
How about naming this option "mine"?
> @@ -61,14 +61,19 @@ OPTIONS
> -C <commit>::
> --reuse-message=<commit>::
> Take an existing commit object, and reuse the log message
> - and the authorship information (including the timestamp)
> - when creating the commit.
> + and the authorship information when creating the commit.
I don't think this is a good change.
When you use the new option, the author name, email and timestamp are
ignored, and when you don't, they are all used.
To new users who are taught to first set user.name and user.email via
configuration variables, the phrase "authorship information" would mean
<name, email> pair, and the explanation in the parentheses helps to avoid
a misunderstanding that these two are the only things that are copied.
I would suggest you keep the original text.
> +--claim::
> + When used with -C/-c/--amend options, declare that the
> + authorship of the resulting commit now belongs of the committer.
> + This also renews the author timestamp. Therefore this option
> + sets the use of only the message from the original commit.
I don't understand/parse the last sentence; I don't think it is necessary,
either.
> diff --git a/builtin-commit.c b/builtin-commit.c
> index c395cbf..919e3fe 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -51,7 +51,7 @@ static const char *template_file;
> static char *edit_message, *use_message;
> static char *author_name, *author_email, *author_date;
> static int all, edit_flag, also, interactive, only, amend, signoff;
> -static int quiet, verbose, no_verify, allow_empty, dry_run;
> +static int quiet, verbose, no_verify, allow_empty, dry_run, claim;
Even if you name the command option "claim" in order to keep it short, I
think it is a bad idea to name the variable "claim", because it doesn't
say _what_ you are claiming and is confusing. Naming it claim_authorship
would be better.
> + OPT_BOOLEAN(0, "claim", &claim, "acquire authorship and restamp time of resulting commit"),
It is unclear from where it is "acquire"-ing, nor what "restamp" means.
Here are my attempts to come up with better wording:
"ignore author and timestamp of the original commit (used with -C/-c/--amend)"
"the commit is authored by me now (used with -C/-c/--amend)"
The latter will work well if the option is renamed to "--mine".
What should happen when the user uses --claim without -C/-c/--amend?
% git commit --claim
Should you detect an error? Does your code do so? Do you have a test
that catches this error?
What should happen when the user uses --author and --claim at the same time?
% git commit --claim --author='Erick Mattos <eric@mattos>' -C HEAD
Should you detect an error? Does your code do so? Do you have a test
that catches this error?
> diff --git a/t/t7509-commit.sh b/t/t7509-commit.sh
> new file mode 100755
> index 0000000..62fb00f
> --- /dev/null
> +++ b/t/t7509-commit.sh
> @@ -0,0 +1,87 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2009 Erick Mattos
> +#
> +
> +test_description='git commit
> +
> +Tests for --claim option on a commit.'
> +
> +. ./test-lib.sh
> +
> +TEST_FILE="$PWD"/foo
Why does this have to be given as a full path, not just "foo"?
> +test_expect_success '-C option should be working' '
Every test is about "should be working", so you are wasting 16 letters or
so without giving any useful information.
Say something like "-C without --claim uses the author from the old commit" here.
> + echo "Initial" > "$TEST_FILE" &&
> + git add "$TEST_FILE" &&
> + git commit -m "Initial Commit" --author Frigate\ \<flying@over.world\> &&
> + sleep 1 &&
If you use "test_tick", you don't have to slow the test down. You use
"test_tick" before you commit to increment the time. Look at t6036 for an
example.
> + echo "Test 1" >> "$TEST_FILE" &&
> + git add "$TEST_FILE" &&
> + git commit -C HEAD &&
> + git cat-file -p HEAD^ | sed -e '/^parent/d' -e '/^tree/d' -e '/^committer/d' > commit_1 &&
> + git cat-file -p HEAD | sed -e '/^parent/d' -e '/^tree/d' -e '/^committer/d' > commit_2 &&
> + cmp commit_1 commit_2
> +'
Use "test_cmp" instead, so that errors can be seen easily when somebody
breaks this new feature.
> +test_expect_success '-C option with --claim is working properly' '
Again, "working properly" is a meaningless thing to say because that is
what all tests check. "-C with --claim makes me the author" would be
better.
> + sleep 1 &&
> + echo "Test 2" >> "$TEST_FILE" &&
> + git add "$TEST_FILE" &&
> + git commit -C HEAD^ --claim &&
> + git cat-file -p HEAD^ | grep '^author' > commit_1 &&
> + git cat-file -p HEAD | grep '^author' > commit_2 &&
> + test_must_fail cmp commit_1 commit_2
This test shouldn't be happy with any random author information that
happens to be different from the original. The purpose of --claim option
is to take the authorship, make it mine (or whoever is specified with
GIT_AUTHOR_NAME or user.name or uid-to-gecos), so the last cmp (again, it
should use test_cmp) should make sure that the author is 'A U Thor', not
just being different from "Frigate" or whatever. It should check email
and timestamp as well, of course.
> +'
> +
> +test_expect_success '-c option should be working' '
> + echo "Initial" > "$TEST_FILE" &&
> + git add "$TEST_FILE" &&
> + git commit -m "Initial Commit" --author Frigate\ \<flying@over.world\> &&
> + sleep 1 &&
> + echo "Test 3" >> "$TEST_FILE" &&
> + git add "$TEST_FILE" &&
> + git commit -c HEAD <<EOF
> + "Changed"
> + EOF &&
What editor is reading this "Changed"?
^ permalink raw reply
* Re: Fw: git-core: SIGSEGV during {peek,ls}-remote on HTTP remotes.
From: Daniel Barkalow @ 2009-11-01 19:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Samium Gromoff, git, Tay Ray Chuan, Mike Hommey
In-Reply-To: <7v8weq50pm.fsf@alter.siamese.dyndns.org>
On Sat, 31 Oct 2009, Junio C Hamano wrote:
> Samium Gromoff <_deepfire@feelingofgreen.ru> writes:
>
> > Attached is the SEGV bugreport I sent to debian.
>
> Thanks for a report.
>
> There are two issues.
>
> * transport-helper.c::get_helper() assumes that the transport structure
> always has its remote member filled and it can get name out of it.
> This is the segv in the report.
>
> * Even if we work around the above issue, the helper subprocess (in this
> case, remote-curl helper) insists on being inside a git repository, To
> satisfy ls-remote request, you do not have to be in one.
>
> Attached is a minimum fix/work around, but this is done without being very
> familiar with the current assumptions in the codepaths involved.
>
> Issues I want area experts to consider before coming up with the final fix
> are:
>
> - Should we fix get_helper() in transport-helper.c, instead of touching
> ls-remote.c like this patch does?
>
> This issue really boils down to this question: is it valid for a
> transport to have NULL in its remote field, and should all the code
> that touch transport be prepared to deal with such a transport
> structure?
I think there's no benefit to allowing NULL for the remote; I think you
can always get a struct remote for what you want to access. So it's
probably just as well to require it, particularly because, as in the case
of cmd_ls_remote() below, you'd need a special case to not get a struct
remote.
Is there any way in which the intended semantics of "transport_get(NULL,
url)" is not the same as "transport_get(remote_get(url), url)"?
(And, in the extended series, I make "transport_get(remote_get(url),
NULL)" also mean the same thing as above, while "transport_get(NULL,
NULL)" is obviously underspecified.)
> - When helping to handle ls-remote request, there is no need for the
> helper to know anything about the local state. We probably shouldn't
> even run setup_git_directory_gently() at all in this case. But when
> helping other kinds of request, the helper does need to know where our
> repository is.
>
> In general, what should the initial environment for helpers be? Should
> they assume that they have to figure out where the git repository is
> themselves (in other words, should they assume they cannot rely on
> anything the caller does before they are called? Would the caller
> generally have done the usual repo discovery (including chdir() to the
> toplevel), and there are some set of assumptions they can make? If so
> what are they?
Probably, the helper should be run with a predicable initial environment,
simply because operations that use remote repositories are most often run
from the toplevel of a repo, so people will fail to notice their bugs
which trigger on running from subdirectories. If it's possible for the
core git code to enforce regularity here, these oversights aren't bugs.
That is, I think people working on helpers will assume that they always
run from the environment that they run from the first time they try,
whether or not they're supposed to assume this, and we can save a lot of
trouble by making it okay.
Perhaps we should actively tell the helper if there is no git repository
(or, if any git repository we happen to be in is merely coincidental and
shouldn't affect the helper)? Helpers involving importing will probably
want to know they don't have a private refs namespace, private state
directory, etc. even for implementing "list" for ls-remote, and it would
probably be best to require helper authors to report that they've
considered this possibility before trying to use it.
> builtin-ls-remote.c | 2 +-
> remote-curl.c | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/builtin-ls-remote.c b/builtin-ls-remote.c
> index 78a88f7..a8d5613 100644
> --- a/builtin-ls-remote.c
> +++ b/builtin-ls-remote.c
> @@ -86,7 +86,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
> pattern[j - i] = p;
> }
> }
> - remote = nongit ? NULL : remote_get(dest);
> + remote = remote_get(dest);
> if (remote && !remote->url_nr)
> die("remote %s has no configured URL", dest);
> transport = transport_get(remote, remote ? remote->url[0] : dest);
You can also drop the two checks for remote being non-NULL here, since
it's now always non-NULL...
> diff --git a/remote-curl.c b/remote-curl.c
> index ad6a163..7c83f77 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -81,8 +81,9 @@ int main(int argc, const char **argv)
> struct strbuf buf = STRBUF_INIT;
> const char *url;
> struct walker *walker = NULL;
> + int nongit = 0;
>
> - setup_git_directory();
> + setup_git_directory_gently(&nongit);
> if (argc < 2) {
> fprintf(stderr, "Remote needed\n");
> return 1;
Do things like git_path() fail cleanly if there was no git directory? If
not, there should probably be tests of nongit on paths that actually need
a git directory, rather than relying on the caller not to do anything that
needs a git directory. Even if the core is wise enough not to use "fetch"
if there's nowhere to put the result, this code is likely to be an example
for helpers that won't be able to rely on the same analysis.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* [PATCH] commit -c/-C/--amend: acquire authorship and restamp time with --claim
From: Erick Mattos @ 2009-11-01 18:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Erick Mattos
In-Reply-To: <1257099580-7365-1-git-send-email-erick.mattos@gmail.com>
When we use one of the options above we are normally trying to do mainly
two things: one is using the source as a template and second is to
recreate a commit with corrections.
When they are used, the authorship and timestamp recorded in the newly
created commit is always taken from the original commit. And they
should not when we are using it as a template.
The new --claim option is meant to solve this need by regenerating the
timestamp and setting as new author the committer or the one specified
on --author option.
Signed-off-by: Erick Mattos <erick.mattos@gmail.com>
---
Documentation/git-commit.txt | 11 ++++-
builtin-commit.c | 7 ++-
t/t7509-commit.sh | 87 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 99 insertions(+), 6 deletions(-)
create mode 100755 t/t7509-commit.sh
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0578a40..01eeb3e 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -9,7 +9,7 @@ SYNOPSIS
--------
[verse]
'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
- [(-c | -C) <commit>] [-F <file> | -m <msg>]
+ [(-c | -C) <commit>] [-F <file> | -m <msg>] [--claim]
[--allow-empty] [--no-verify] [-e] [--author=<author>]
[--cleanup=<mode>] [--] [[-i | -o ]<file>...]
@@ -61,14 +61,19 @@ OPTIONS
-C <commit>::
--reuse-message=<commit>::
Take an existing commit object, and reuse the log message
- and the authorship information (including the timestamp)
- when creating the commit.
+ and the authorship information when creating the commit.
-c <commit>::
--reedit-message=<commit>::
Like '-C', but with '-c' the editor is invoked, so that
the user can further edit the commit message.
+--claim::
+ When used with -C/-c/--amend options, declare that the
+ authorship of the resulting commit now belongs of the committer.
+ This also renews the author timestamp. Therefore this option
+ sets the use of only the message from the original commit.
+
-F <file>::
--file=<file>::
Take the commit message from the given file. Use '-' to
diff --git a/builtin-commit.c b/builtin-commit.c
index c395cbf..919e3fe 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -51,7 +51,7 @@ static const char *template_file;
static char *edit_message, *use_message;
static char *author_name, *author_email, *author_date;
static int all, edit_flag, also, interactive, only, amend, signoff;
-static int quiet, verbose, no_verify, allow_empty, dry_run;
+static int quiet, verbose, no_verify, allow_empty, dry_run, claim;
static char *untracked_files_arg;
/*
* The default commit message cleanup mode will remove the lines
@@ -91,8 +91,9 @@ static struct option builtin_commit_options[] = {
OPT_FILENAME('F', "file", &logfile, "read log from file"),
OPT_STRING(0, "author", &force_author, "AUTHOR", "override author for commit"),
OPT_CALLBACK('m', "message", &message, "MESSAGE", "specify commit message", opt_parse_m),
- OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit "),
+ OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit"),
OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
+ OPT_BOOLEAN(0, "claim", &claim, "acquire authorship and restamp time of resulting commit"),
OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
OPT_FILENAME('t', "template", &template_file, "use specified template file"),
OPT_BOOLEAN('e', "edit", &edit_flag, "force edit of commit"),
@@ -381,7 +382,7 @@ static void determine_author_info(void)
email = getenv("GIT_AUTHOR_EMAIL");
date = getenv("GIT_AUTHOR_DATE");
- if (use_message) {
+ if (use_message && !claim) {
const char *a, *lb, *rb, *eol;
a = strstr(use_message_buffer, "\nauthor ");
diff --git a/t/t7509-commit.sh b/t/t7509-commit.sh
new file mode 100755
index 0000000..62fb00f
--- /dev/null
+++ b/t/t7509-commit.sh
@@ -0,0 +1,87 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Erick Mattos
+#
+
+test_description='git commit
+
+Tests for --claim option on a commit.'
+
+. ./test-lib.sh
+
+TEST_FILE="$PWD"/foo
+
+test_expect_success '-C option should be working' '
+ echo "Initial" > "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "Initial Commit" --author Frigate\ \<flying@over.world\> &&
+ sleep 1 &&
+ echo "Test 1" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -C HEAD &&
+ git cat-file -p HEAD^ | sed -e '/^parent/d' -e '/^tree/d' -e '/^committer/d' > commit_1 &&
+ git cat-file -p HEAD | sed -e '/^parent/d' -e '/^tree/d' -e '/^committer/d' > commit_2 &&
+ cmp commit_1 commit_2
+'
+
+test_expect_success '-C option with --claim is working properly' '
+ sleep 1 &&
+ echo "Test 2" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -C HEAD^ --claim &&
+ git cat-file -p HEAD^ | grep '^author' > commit_1 &&
+ git cat-file -p HEAD | grep '^author' > commit_2 &&
+ test_must_fail cmp commit_1 commit_2
+'
+
+test_expect_success '-c option should be working' '
+ echo "Initial" > "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "Initial Commit" --author Frigate\ \<flying@over.world\> &&
+ sleep 1 &&
+ echo "Test 3" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -c HEAD <<EOF
+ "Changed"
+ EOF &&
+ git cat-file -p HEAD^ | grep '^author' > commit_1 &&
+ git cat-file -p HEAD | grep '^author' > commit_2 &&
+ cmp commit_1 commit_2
+'
+
+test_expect_success '-c option with --claim is working properly' '
+ sleep 1 &&
+ echo "Test 4" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -c HEAD^ --claim <<EOF
+ "Changed again"
+ EOF &&
+ git cat-file -p HEAD^ | grep '^author' > commit_1 &&
+ git cat-file -p HEAD | grep '^author' > commit_2 &&
+ test_must_fail cmp commit_1 commit_2
+'
+
+test_expect_success '--amend option should be working' '
+ echo "Initial" > "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "Initial Commit" --author Frigate\ \<flying@over.world\> &&
+ echo "Test 5" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "--amend test" &&
+ git cat-file -p HEAD | grep '^author' > commit_1 &&
+ sleep 1 &&
+ git commit -m "Changed" --amend &&
+ git cat-file -p HEAD | grep '^author' > commit_2 &&
+ cmp commit_1 commit_2
+'
+
+test_expect_success '--amend option with --claim is working properly' '
+ sleep 1 &&
+ echo "Test 6" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "Changed again" --amend --claim &&
+ git cat-file -p HEAD | grep '^author' > commit_1 &&
+ test_must_fail cmp commit_1 commit_2
+'
+
+test_done
--
1.6.5.GIT
^ permalink raw reply related
* [PATCH v2] commit -c/-C/--amend: take over authorship and restamp time with --claim
From: Erick Mattos @ 2009-11-01 18:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Erick Mattos
In-Reply-To: <7v7hub5kam.fsf@alter.siamese.dyndns.org>
When we use one of the options above we are normally trying to do mainly
two things: one is using the source as a template and second is to
recreate a commit with corrections.
When they are used, the authorship and timestamp recorded in the newly
created commit is always taken from the original commit. And they
should not when we are using it as a template.
The new --claim option is meant to solve this need by regenerating the
timestamp and setting as new author the committer or the one specified
on --author option.
Signed-off-by: Erick Mattos <erick.mattos@gmail.com>
---
Documentation/git-commit.txt | 11 ++++-
builtin-commit.c | 7 ++-
t/t7509-commit.sh | 87 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 99 insertions(+), 6 deletions(-)
create mode 100755 t/t7509-commit.sh
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0578a40..27b774e 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -9,7 +9,7 @@ SYNOPSIS
--------
[verse]
'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
- [(-c | -C) <commit>] [-F <file> | -m <msg>]
+ [(-c | -C) <commit>] [-F <file> | -m <msg>] [--claim]
[--allow-empty] [--no-verify] [-e] [--author=<author>]
[--cleanup=<mode>] [--] [[-i | -o ]<file>...]
@@ -61,14 +61,19 @@ OPTIONS
-C <commit>::
--reuse-message=<commit>::
Take an existing commit object, and reuse the log message
- and the authorship information (including the timestamp)
- when creating the commit.
+ and the authorship information when creating the commit.
-c <commit>::
--reedit-message=<commit>::
Like '-C', but with '-c' the editor is invoked, so that
the user can further edit the commit message.
+--claim::
+ When used with -C/-c/--amend options, declare that the
+ authorship of the resulting commit now belongs of the committer.
+ This also renews the author timestamp. Therefore this option
+ sets the use of the message only from the original commit.
+
-F <file>::
--file=<file>::
Take the commit message from the given file. Use '-' to
diff --git a/builtin-commit.c b/builtin-commit.c
index c395cbf..33922df 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -51,7 +51,7 @@ static const char *template_file;
static char *edit_message, *use_message;
static char *author_name, *author_email, *author_date;
static int all, edit_flag, also, interactive, only, amend, signoff;
-static int quiet, verbose, no_verify, allow_empty, dry_run;
+static int quiet, verbose, no_verify, allow_empty, dry_run, claim;
static char *untracked_files_arg;
/*
* The default commit message cleanup mode will remove the lines
@@ -91,8 +91,9 @@ static struct option builtin_commit_options[] = {
OPT_FILENAME('F', "file", &logfile, "read log from file"),
OPT_STRING(0, "author", &force_author, "AUTHOR", "override author for commit"),
OPT_CALLBACK('m', "message", &message, "MESSAGE", "specify commit message", opt_parse_m),
- OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit "),
+ OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit"),
OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
+ OPT_BOOLEAN(0, "claim", &claim, "take over cloned commit authorship and renew timestamp"),
OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
OPT_FILENAME('t', "template", &template_file, "use specified template file"),
OPT_BOOLEAN('e', "edit", &edit_flag, "force edit of commit"),
@@ -381,7 +382,7 @@ static void determine_author_info(void)
email = getenv("GIT_AUTHOR_EMAIL");
date = getenv("GIT_AUTHOR_DATE");
- if (use_message) {
+ if (use_message && !claim) {
const char *a, *lb, *rb, *eol;
a = strstr(use_message_buffer, "\nauthor ");
diff --git a/t/t7509-commit.sh b/t/t7509-commit.sh
new file mode 100755
index 0000000..62fb00f
--- /dev/null
+++ b/t/t7509-commit.sh
@@ -0,0 +1,87 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Erick Mattos
+#
+
+test_description='git commit
+
+Tests for --claim option on a commit.'
+
+. ./test-lib.sh
+
+TEST_FILE="$PWD"/foo
+
+test_expect_success '-C option should be working' '
+ echo "Initial" > "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "Initial Commit" --author Frigate\ \<flying@over.world\> &&
+ sleep 1 &&
+ echo "Test 1" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -C HEAD &&
+ git cat-file -p HEAD^ | sed -e '/^parent/d' -e '/^tree/d' -e '/^committer/d' > commit_1 &&
+ git cat-file -p HEAD | sed -e '/^parent/d' -e '/^tree/d' -e '/^committer/d' > commit_2 &&
+ cmp commit_1 commit_2
+'
+
+test_expect_success '-C option with --claim is working properly' '
+ sleep 1 &&
+ echo "Test 2" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -C HEAD^ --claim &&
+ git cat-file -p HEAD^ | grep '^author' > commit_1 &&
+ git cat-file -p HEAD | grep '^author' > commit_2 &&
+ test_must_fail cmp commit_1 commit_2
+'
+
+test_expect_success '-c option should be working' '
+ echo "Initial" > "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "Initial Commit" --author Frigate\ \<flying@over.world\> &&
+ sleep 1 &&
+ echo "Test 3" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -c HEAD <<EOF
+ "Changed"
+ EOF &&
+ git cat-file -p HEAD^ | grep '^author' > commit_1 &&
+ git cat-file -p HEAD | grep '^author' > commit_2 &&
+ cmp commit_1 commit_2
+'
+
+test_expect_success '-c option with --claim is working properly' '
+ sleep 1 &&
+ echo "Test 4" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -c HEAD^ --claim <<EOF
+ "Changed again"
+ EOF &&
+ git cat-file -p HEAD^ | grep '^author' > commit_1 &&
+ git cat-file -p HEAD | grep '^author' > commit_2 &&
+ test_must_fail cmp commit_1 commit_2
+'
+
+test_expect_success '--amend option should be working' '
+ echo "Initial" > "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "Initial Commit" --author Frigate\ \<flying@over.world\> &&
+ echo "Test 5" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "--amend test" &&
+ git cat-file -p HEAD | grep '^author' > commit_1 &&
+ sleep 1 &&
+ git commit -m "Changed" --amend &&
+ git cat-file -p HEAD | grep '^author' > commit_2 &&
+ cmp commit_1 commit_2
+'
+
+test_expect_success '--amend option with --claim is working properly' '
+ sleep 1 &&
+ echo "Test 6" >> "$TEST_FILE" &&
+ git add "$TEST_FILE" &&
+ git commit -m "Changed again" --amend --claim &&
+ git cat-file -p HEAD | grep '^author' > commit_1 &&
+ test_must_fail cmp commit_1 commit_2
+'
+
+test_done
--
1.6.5.GIT
^ permalink raw reply related
* Re: [PATCH v5 27/28] http tests: use /dumb/ URL prefix
From: Tay Ray Chuan @ 2009-11-01 14:50 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1256950067-27938-29-git-send-email-spearce@spearce.org>
Hi,
On Sat, Oct 31, 2009 at 8:47 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> To clarify what part of the HTTP transprot is being tested we change
> the URLs used by existing tests to include /dumb/ at the start,
> indicating they use the non-Git aware code paths.
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> CC: Tay Ray Chuan <rctay89@gmail.com>
thanks.
Acked-by: Tay Ray Chuan <rctay89@gmail.com>
--
Cheers,
Ray Chuan
^ permalink raw reply
* Re: Fw: git-core: SIGSEGV during {peek,ls}-remote on HTTP remotes.
From: Sverre Rabbelier @ 2009-11-01 14:46 UTC (permalink / raw)
To: Junio C Hamano
Cc: Samium Gromoff, git, Daniel Barkalow, Tay Ray Chuan, Mike Hommey
In-Reply-To: <7v8weq50pm.fsf@alter.siamese.dyndns.org>
Heya,
On Sun, Nov 1, 2009 at 05:27, Junio C Hamano <gitster@pobox.com> wrote:
> - Should we fix get_helper() in transport-helper.c, instead of touching
> ls-remote.c like this patch does?
Probably, yes.
> This issue really boils down to this question: is it valid for a
> transport to have NULL in its remote field, and should all the code
> that touch transport be prepared to deal with such a transport
> structure?
I think the code in transport-helper should be prepared to deal with
such a field appropriately, since it knows beforehand that only a few
operations will be performed on such a remote (I'm guessing just the
'list' command).
> In general, what should the initial environment for helpers be? Should
> they assume that they have to figure out where the git repository is
> themselves (in other words, should they assume they cannot rely on
> anything the caller does before they are called?
Let's not duplicate that logic, if git can figure out where we are, it
should do so, if it can't, then the helper can't either.
> Would the caller
> generally have done the usual repo discovery (including chdir() to the
> toplevel), and there are some set of assumptions they can make? If so
> what are they?
Probably the above, if there is going to be a git repository, we'll
have found it, if there isn't one, we're in 'bare' mode.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 1/2] gitk: Initialize msgcat before first use
From: Bernt Hansen @ 2009-11-01 13:09 UTC (permalink / raw)
To: Pat Thoyts; +Cc: git, Paul Mackerras
In-Reply-To: <87d443xn5u.fsf@users.sourceforge.net>
Pat Thoyts <patthoyts@users.sourceforge.net> writes:
> Bernt Hansen <bernt@norang.ca> writes:
>
>>The error text generated when your version of Tcl is too old is
>>translated with msgcat (mc) before msgcat is initialized. This
>>causes Tcl to abort with:
>>
>> Error in startup script: invalid command name "mc"
>>
>>We now initialize msgcat first before we check the Tcl version. Msgcat
>>is available since Tcl 8.1.
>>
>
> This doesn't quite work. [file normalize] was introduced with Tcl 8.4
> and when I test this by starting it using Tcl 8.3 I get an error:
> "bad option "normalize": must be atime, attributes, channels..."
> from line 11014. It is probably sufficient to just drop the [file
> normalize] here. On Windows $argv0 is fully qualified and
> [file dirname] works ok on it. By removing the [file normalize] I get
> the expected error dialog when testing with 8.3.
> However, on Windows we actually get a better looking result by not
> catching the [package require Tcl 8.4] and just letting Tk bring up a
> standard message box with the version conflict error message.
>
> Well, actually if show_error just used tk_messageBox it would look
> better on Windows.
You're right. Thanks for catching this. When I tested this code I
bumped the version number temporarily to 8.5 instead of downgrading TK
to 8.3.
The problem I was trying to fix was show_error using mc internally
before it was initialized. Maybe it would be better to give show_error
an optional parameter that controls calling mc - so that the call for
the version check can just bypass the mc translation of the text and OK
buttons. With this approach the code I moved around can just stay where
it is and all of the existing calls to show_error will use the default
parameter setting which invokes mc.
Would that be a better approach?
-Bernt
^ permalink raw reply
* Headless tags don't have a follows or precedes?
From: Tim Mazid @ 2009-11-01 9:31 UTC (permalink / raw)
To: git
Hi all,
I've noticed that if I create a headless tag (one that doesn't have a
branch, right?), when I click on that commit, it doesn't have precedes or
follows information. Is this by design? Is there a work-around I can use
without creating a branch there?
Thanks,
Tim.
--
View this message in context: http://n2.nabble.com/Headless-tags-don-t-have-a-follows-or-precedes-tp3926483p3926483.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* Re: Fw: git-core: SIGSEGV during {peek,ls}-remote on HTTP remotes.
From: Mike Hommey @ 2009-11-01 8:19 UTC (permalink / raw)
To: Samium Gromoff; +Cc: git
In-Reply-To: <20091101.010702.527849118592864646._deepfire@feelingofgreen.ru>
On Sun, Nov 01, 2009 at 01:07:02AM +0300, Samium Gromoff wrote:
> Good day folks,
>
> Attached is the SEGV bugreport I sent to debian.
>
> I tried to convince ld to use /usr/lib/debug, via LD_LIBRARY_PATH,
> and run the thing under gdb 7.0, but it won't use debug libraries
> for some reason.
You don't need to point to /usr/lib/debug, for two reasons:
- The files there are empty nutshells as far as binaries are concerned.
They only contain the debugging symbols.
- gdb is going to load files from there automatically for each file it
has loaded and doesn't contain debugging symbols.
Mike
^ permalink raw reply
* Re: [PATCH 7/8] Provide a build time default-editor setting
From: Junio C Hamano @ 2009-11-01 4:29 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091031212144.GA5022@progeny.tock>
Jonathan Nieder <jrnieder@gmail.com> writes:
> +unset EDITOR VISUAL GIT_EDITOR
> +
> +test_expect_success 'determine default editor' '
> +
> + vi=$(TERM=vt100 git var GIT_EDITOR) &&
> + test -n "$vi"
> +
> +'
> +
> +if ! expr "$vi" : '^[a-z]*$' >/dev/null
> +then
> + vi=
> +fi
> +
> +for i in GIT_EDITOR core_editor EDITOR VISUAL $vi
> ...
> -for i in vi EDITOR VISUAL core_editor GIT_EDITOR
> +for i in $vi EDITOR VISUAL core_editor GIT_EDITOR
> do
> echo "Edited by $i" >expect
> unset EDITOR VISUAL GIT_EDITOR
> @@ -78,7 +91,7 @@ done
>
> unset EDITOR VISUAL GIT_EDITOR
> git config --unset-all core.editor
> -for i in vi EDITOR VISUAL core_editor GIT_EDITOR
> +for i in $vi EDITOR VISUAL core_editor GIT_EDITOR
> do
Beautiful ;-)
^ permalink raw reply
* Re: [PATCH v4] Teach git var about GIT_EDITOR
From: Junio C Hamano @ 2009-11-01 4:29 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ben Walton, Johannes Sixt, David Roundy, GIT List
In-Reply-To: <20091031075627.GB635@progeny.tock>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Expose the command used by launch_editor() for scripts to use.
> This should allow one to avoid searching for a proper editor
> separately in each command.
Ok, so the idea is...
* git_editor(void) uses the logic to decide which editor to use that used
to live in launch_editor(). The function returns NULL if there is no
suitable editor; the caller is expected to issue an error message when
appropriate.
* launch_editor() uses git_editor() and gives the error message the same
way as before when EDITOR is not set.
* "git var GIT_EDITOR" gives the editor name, or an error message when
there is no appropriate one.
* "git var -l" gives GIT_EDITOR=name only if there is an appropriate
editor.
The above all look sensible, but IIRC, the true "vi" fell back on "ex"
mode on dumb terminals and was usable as a line editor, so we should be
able to run it even on dumb terminals. I do not know about vi-clones
though.
^ 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