[PATCH 0/3] doc/{fast-import,remote-helpers}: few clarifications

[PATCH 0/3] doc/{fast-import,remote-helpers}: few clarifications

[Dmitry Ivankov]

doc/fast-import part seems pretty clear.
doc/remote-helpers is related to a recent remote-helpers series [1] and 
may need some discussion. For example, should we mention a bug [2] 
addressed as a part of [1], should we wait for [1] to document the 
fact that gitdir response can be read in the middle of capabilities 
reporting.

[1] http://thread.gmane.org/gmane.comp.version-control.git/176002
[2] http://thread.gmane.org/gmane.comp.version-control.git/176002/focus=176020

Dmitry Ivankov (3):
  doc/fast-import: clarify notemodify command
  doc/fast-import: document feature import-marks-if-exists
  doc/remote-helpers: document the gitdir feature

 Documentation/git-fast-import.txt    |   16 +++++++++++++---
 Documentation/git-remote-helpers.txt |   15 +++++++++++++++
 t/t9300-fast-import.sh               |   15 +++++++++++++++
 3 files changed, 43 insertions(+), 3 deletions(-)

-- 
1.7.3.4

[PATCH 1/3] doc/fast-import: clarify notemodify command

[Dmitry Ivankov]

The "notemodify" fast-import command was introduced in commit a8dd2..
The commit log has slightly different description than the added
documentation. The latter is somewhat confusing. "notemodify" is a
subcommand of "commit" command used to add a note for some commit.
Does this note annotate the commit produced by the "commit" command
or a commit given by it's committish parameter? Which notes tree
does it write notes to?

The exact meaning could be deduced with old description and some
notes machinery knowledge. But let's make it more obvious. This
command is used in a context like "commit refs/notes/test" to
add or rewrite an annotation for a committish parameter. So the
advised way to add notes in a fast-import stream is:
1) import some commits (optional)
2) prepare a "commit" to the notes tree:
2.1) choose notes ref, committer, log message, etc.
2.2) create annotations with "notemodify", where each can refer to
a commit being annotated via a branch name, import mark reference,
sha1 and other expressions specified in the Documentation.

Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
 Documentation/git-fast-import.txt |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 249249a..3f5b912 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -648,9 +648,14 @@ paths for a commit are encouraged to do so.
 
 `notemodify`
 ^^^^^^^^^^^^
-Included in a `commit` command to add a new note (annotating a given
-commit) or change the content of an existing note.  This command has
-two different means of specifying the content of the note.
+Included in a `commit` `<notes_ref>` command to add a new note
+annotating a `<committish>` or change this annotation contents.
+Internally it is similar to filemodify 100644 on `<committish>`
+path (maybe split into subdirectories). It's not advised to
+use any other commands to write to the `<notes_ref>` tree except
+`filedeleteall` to delete all existing notes in this tree.
+This command has two different means of specifying the content
+of the note.
 
 External data format::
 	The data content for the note was already supplied by a prior
-- 
1.7.3.4

[PATCH 2/3] doc/fast-import: document feature import-marks-if-exists

[Dmitry Ivankov]

fast-import parameter --import-marks-if-exists was introduced in
commit dded4.. --import-marks can be set via a "feature" command in
a fast-import stream and --import-marks-if-exists has support for
such specification too, for free.

Document "feature import-marks-if-exists=<file>" command and add a
minimalistic test for it.

Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
 Documentation/git-fast-import.txt |    5 +++++
 t/t9300-fast-import.sh            |   15 +++++++++++++++
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 3f5b912..bdcc81c 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -1005,6 +1005,11 @@ import-marks::
 	second, an --import-marks= command-line option overrides
 	any "feature import-marks" command in the stream.
 
+import-marks-if-exists::
+	Like import-marks but instead of erroring out, silently
+	skips the file if it does not exist. Differences from --
+	version are the same as with import-marks feature above.
+
 cat-blob::
 ls::
 	Require that the backend support the 'cat-blob' or 'ls' command.
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 2a53640..eed57df 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1882,6 +1882,21 @@ test_expect_success 'R: --import-marks-if-exists' '
 	test_cmp expect io.marks
 '
 
+test_expect_success 'R: feature import-marks-if-exists' '
+	rm -f io.marks &&
+	blob=$(echo hi | git hash-object --stdin) &&
+	echo ":1 $blob" >expect &&
+	git fast-import --import-marks-if-exists=io.marks --export-marks=io.marks <<-\EOF &&
+	feature import-marks-if-exists=io.marks
+	blob
+	mark :1
+	data 3
+	hi
+
+	EOF
+	test_cmp expect io.marks
+'
+
 cat >input << EOF
 feature import-marks=marks.out
 feature export-marks=marks.new
-- 
1.7.3.4

Re: [PATCH 2/3] doc/fast-import: document feature import-marks-if-exists

[Junio C Hamano]

Dmitry Ivankov <divanorama@gmail.com> writes:

> fast-import parameter --import-marks-if-exists was introduced in
> commit dded4.. --import-marks can be set via a "feature" command in
> a fast-import stream and --import-marks-if-exists has support for
> such specification too, for free.
>
> Document "feature import-marks-if-exists=<file>" command and add a
> minimalistic test for it.
>
> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>

Thanks.

>  Documentation/git-fast-import.txt |    5 +++++
>  t/t9300-fast-import.sh            |   15 +++++++++++++++
>  2 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
> index 3f5b912..bdcc81c 100644
> --- a/Documentation/git-fast-import.txt
> +++ b/Documentation/git-fast-import.txt
> @@ -1005,6 +1005,11 @@ import-marks::
>  	second, an --import-marks= command-line option overrides
>  	any "feature import-marks" command in the stream.
>  
> +import-marks-if-exists::
> +	Like import-marks but instead of erroring out, silently
> +	skips the file if it does not exist. Differences from --
> +	version are the same as with import-marks feature above.

Wouldn't it be more concise and complete to do it this way instead?  It
avoids duplication and also clarifies you cannot mix these two.

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 249249a..e939e74 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -995,10 +995,14 @@ force::
 	(see OPTIONS, above).
 
 import-marks::
+import-marks-if-exists::
 	Like --import-marks except in two respects: first, only one
-	"feature import-marks" command is allowed per stream;
+	"feature import-marks" command or "feature import-marks-if-exists"
+	is allowed per stream;
 	second, an --import-marks= command-line option overrides
-	any "feature import-marks" command in the stream.
+	any "feature import-marks" command in the stream. Similarly,
+	import-marks-if-exists corresponds to --import-marks-if-exists
+	from the command line.
 
 cat-blob::
 ls::

>  
> +test_expect_success 'R: feature import-marks-if-exists' '
> +	rm -f io.marks &&
> +	blob=$(echo hi | git hash-object --stdin) &&
> +	echo ":1 $blob" >expect &&
> +	git fast-import --import-marks-if-exists=io.marks --export-marks=io.marks <<-\EOF &&
> +	feature import-marks-if-exists=io.marks
> +	blob
> +	mark :1
> +	data 3
> +	hi
> +
> +	EOF
> +	test_cmp expect io.marks
> +'

Do you really want both the command line option and feature in this test?

There are at least four cases [*1*] you would want to test (a part of
blame lies in the original test you copied this from).

 - "if exists" should not fail if the marks file does not exist;
 - "if exists" reads marks if exists (not tested in this patch);
 - having "feature import-marks-if-exists" will trigger this feature
   without command line option (not tested in this patch); and
 - having "feature import-marks-if-exists" will be overridden by
   the command line option (not tested in this patch).




[Footnote]

*1* Ideally, the test should also cover these 16 combinations:

  io.marks file (2 cases: exists or missing)

times

  --import-mark-if-exists (2 cases: exists or missing)

times

 feature import-marks-if-exists (4 cases: missing,
 				same as command line,
                                different from command line,
				given more than one)

Re: [PATCH 2/3] doc/fast-import: document feature import-marks-if-exists

[Ramkumar Ramachandra]

Hi Dmitry,

In addition to Junio's comments, I have a few things to add:

Dmitry Ivankov writes:
> fast-import parameter --import-marks-if-exists was introduced in

It's a command-line option, not a parameter.

> commit dded4.. --import-marks can be set via a "feature" command in

Please more SHA-1 hex characters while abbreviating, and write a
little more information about the commit:
dded4f1 (fast-import: Introduce --import-marks-if-exists, 2011-01-15)

> a fast-import stream and --import-marks-if-exists has support for
> such specification too, for free.
>
> Document "feature import-marks-if-exists=<file>" command and add a
> minimalistic test for it.

Perhaps you can make it clearer that this feature was available back
in dded4f1, and was supposed to have been documented then?

Thanks for working on this.  I'm to blame for not documenting/ testing
the feature in the original patch.

-- Ram

[PATCH 3/3] doc/remote-helpers: document the gitdir feature

[Dmitry Ivankov]

The gitdir capability is recognized by git and can be used to tell
the helper where the .git directory is. But it is not mentioned in
the documentation.

Document the gitdir feature. Upon reading this capability
transport-helper sends out "gitdir path_to_dot_git" command to the
helper. Which looks exactly like sending a command. So let's call
this a command.

Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
 Documentation/git-remote-helpers.txt |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 58f6ad4..a16a5e5 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -159,6 +159,14 @@ Supported if the helper has the "import" capability.
 +
 Supported if the helper has the "connect" capability.
 
+'gitdir' <path>::
+	Tells helper the location of current repository .git
+	directory. The path is absolute. The command is issued
+	immediately after "gitdir" capability line is read by
+	the caller.
++
+Supported if the helper has the "gitdir" capability.
+
 If a fatal error occurs, the program writes the error message to
 stderr and exits. The caller should expect that a suitable error
 message has been printed if the child closes the connection without
@@ -175,6 +183,7 @@ CAPABILITIES
 'push'::
 'import'::
 'connect'::
+'gitdir'::
 	This helper supports the corresponding command with the same name.
 
 'refspec' 'spec'::
@@ -187,6 +196,12 @@ CAPABILITIES
 	all, it must cover all refs reported by the list command; if
 	it is not used, it is effectively "{asterisk}:{asterisk}"
 
+'gitdir'::
+	The helper wants to know .git directory location. The gitdir
+	command is sent immediately after reading this capability.
+	The helper can read it immediately too or it can proceed with
+	reporting other capabilities and read it as a next command.
+
 REF LIST ATTRIBUTES
 -------------------
 
-- 
1.7.3.4

Re: [PATCH 3/3] doc/remote-helpers: document the gitdir feature

[Sverre Rabbelier]

Heya,

On Tue, Jul 12, 2011 at 17:10, Dmitry Ivankov <divanorama@gmail.com> wrote:
> The gitdir capability is recognized by git and can be used to tell
> the helper where the .git directory is. But it is not mentioned in
> the documentation.
>
> Document the gitdir feature. Upon reading this capability
> transport-helper sends out "gitdir path_to_dot_git" command to the
> helper. Which looks exactly like sending a command. So let's call
> this a command.

Johannes suggested that we should remove this feature and instead
always export an GITDIR environmental variable. I'm inclined to agree.

-- 
Cheers,

Sverre Rabbelier

[PATCH v2 1/3] doc/fast-import: clarify notemodify command

[Dmitry Ivankov]

The "notemodify" fast-import command was introduced in commit a8dd2e7
(fast-import: Add support for importing commit notes, 2009-10-09)
The commit log has slightly different description than the added
documentation. The latter is somewhat confusing. "notemodify" is a
subcommand of "commit" command used to add a note for some commit.
Does this note annotate the commit produced by the "commit" command
or a commit given by it's committish parameter? Which notes tree
does it write notes to?

The exact meaning could be deduced with old description and some
notes machinery knowledge. But let's make it more obvious. This
command is used in a context like "commit refs/notes/test" to
add or rewrite an annotation for a committish parameter. So the
advised way to add notes in a fast-import stream is:
1) import some commits (optional)
2) prepare a "commit" to the notes tree:
2.1) choose notes ref, committer, log message, etc.
2.2) create annotations with "notemodify", where each can refer to
a commit being annotated via a branch name, import mark reference,
sha1 and other expressions specified in the Documentation.

Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
Just added a bit more metadata of a8dd2e7.

 Documentation/git-fast-import.txt |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 249249a..3f5b912 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -648,9 +648,14 @@ paths for a commit are encouraged to do so.
 
 `notemodify`
 ^^^^^^^^^^^^
-Included in a `commit` command to add a new note (annotating a given
-commit) or change the content of an existing note.  This command has
-two different means of specifying the content of the note.
+Included in a `commit` `<notes_ref>` command to add a new note
+annotating a `<committish>` or change this annotation contents.
+Internally it is similar to filemodify 100644 on `<committish>`
+path (maybe split into subdirectories). It's not advised to
+use any other commands to write to the `<notes_ref>` tree except
+`filedeleteall` to delete all existing notes in this tree.
+This command has two different means of specifying the content
+of the note.
 
 External data format::
 	The data content for the note was already supplied by a prior
-- 
1.7.3.4

Re: [PATCH v2 1/3] doc/fast-import: clarify notemodify command

[Johan Herland]

On Wednesday 13 July 2011, Dmitry Ivankov wrote:
> The "notemodify" fast-import command was introduced in commit a8dd2e7
> (fast-import: Add support for importing commit notes, 2009-10-09)
> The commit log has slightly different description than the added
> documentation. The latter is somewhat confusing. "notemodify" is a
> subcommand of "commit" command used to add a note for some commit.
> Does this note annotate the commit produced by the "commit" command
> or a commit given by it's committish parameter? Which notes tree
> does it write notes to?
> 
> The exact meaning could be deduced with old description and some
> notes machinery knowledge. But let's make it more obvious. This
> command is used in a context like "commit refs/notes/test" to
> add or rewrite an annotation for a committish parameter. So the
> advised way to add notes in a fast-import stream is:
> 1) import some commits (optional)
> 2) prepare a "commit" to the notes tree:
> 2.1) choose notes ref, committer, log message, etc.
> 2.2) create annotations with "notemodify", where each can refer to
> a commit being annotated via a branch name, import mark reference,
> sha1 and other expressions specified in the Documentation.
> 
> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>

Acked-by: Johan Herland <johan@herland.net>


-- 
Johan Herland, <johan@herland.net>
www.herland.net

[PATCH v2 2/3] doc/fast-import: document feature import-marks-if-exists

[Dmitry Ivankov]

fast-import command-line option --import-marks-if-exists was introduced
in commit dded4f1 (fast-import: Introduce --import-marks-if-exists, 2011-01-15)

--import-marks option can be set via a "feature" command in a fast-import
stream and --import-marks-if-exists had support for such specification
from the very beginning too due to some shared codebase. Though the
documentation for this feature wasn't written in dded4f1.

Add the documentation for "feature import-marks-if-exists=<file>". Also add
a minimalistic test for it.

Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
Reworded a bit, united import-marks* descriptions. One more case to test:
--import-marks-if-exists= overrides feature import-marks=. And "fixed" the
test, I left the --import-marks accidentally, though it was a fine test too.
Also not feeling like adding a whole ton of coverage tests yet.

 Documentation/git-fast-import.txt |   10 +++++++---
 t/t9300-fast-import.sh            |   15 +++++++++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 3f5b912..3a5aa55 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -1000,10 +1000,14 @@ force::
 	(see OPTIONS, above).
 
 import-marks::
+import-marks-if-exists::
 	Like --import-marks except in two respects: first, only one
-	"feature import-marks" command is allowed per stream;
-	second, an --import-marks= command-line option overrides
-	any "feature import-marks" command in the stream.
+	"feature import-marks" or "feature import-marks-if-exists"
+	command is allowed per stream; second, an --import-marks=
+	or --import-marks-if-exists command-line option overrides
+	any of these "feature" commands in the stream; third,
+	"feature import-marks-if-exists" like a corresponding
+	command-line option silently skips a nonexistent file.
 
 cat-blob::
 ls::
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 2a53640..9af1a78 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1882,6 +1882,21 @@ test_expect_success 'R: --import-marks-if-exists' '
 	test_cmp expect io.marks
 '
 
+test_expect_success 'R: feature import-marks-if-exists' '
+	rm -f io.marks &&
+	blob=$(echo hi | git hash-object --stdin) &&
+	echo ":1 $blob" >expect &&
+	git fast-import --export-marks=io.marks <<-\EOF &&
+	feature import-marks-if-exists=io.marks
+	blob
+	mark :1
+	data 3
+	hi
+
+	EOF
+	test_cmp expect io.marks
+'
+
 cat >input << EOF
 feature import-marks=marks.out
 feature export-marks=marks.new
-- 
1.7.3.4

[PATCH v2 3/3] remote-helpers: export GIT_DIR variable to helpers

[Dmitry Ivankov]

The gitdir capability is recognized by git and can be used to tell
the helper where the .git directory is. But it is not mentioned in
the documentation and considered worse than if gitdir was passed
via GIT_DIR environment variable.

Do export GIT_DIR for a remote helper. Switch testgit to use env
instead of less favoured capability gitdir.

Mention it's possible uses in documentation: data storage, auxiliary
git commands needed by a helper.

Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
Instead of documenting capability gitdir export GIT_DIR. Turned out
to be quite straightforward.

 Documentation/git-remote-helpers.txt |   18 ++++++++++++++++++
 git-remote-testgit.py                |   14 +-------------
 transport-helper.c                   |   10 ++++++++++
 3 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 58f6ad4..394fc8e 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -47,6 +47,9 @@ arguments. The first argument specifies a remote repository as in git;
 it is either the name of a configured remote or a URL. The second
 argument specifies a URL; it is usually of the form
 '<transport>://<address>', but any arbitrary string is possible.
+'GIT_DIR' environment variable is set up for the remote helper and
+can be used to store some additional data or to invoke auxiliary git
+commands.
 
 When git encounters a URL of the form '<transport>://<address>', where
 '<transport>' is a protocol that it cannot handle natively, it
@@ -159,6 +162,14 @@ Supported if the helper has the "import" capability.
 +
 Supported if the helper has the "connect" capability.
 
+'gitdir' <path>::
+	Tells helper the location of current repository .git
+	directory. The path is absolute. The command is issued
+	immediately after "gitdir" capability line is read by
+	the caller.
++
+Supported if the helper has the "gitdir" capability.
+
 If a fatal error occurs, the program writes the error message to
 stderr and exits. The caller should expect that a suitable error
 message has been printed if the child closes the connection without
@@ -175,6 +186,7 @@ CAPABILITIES
 'push'::
 'import'::
 'connect'::
+'gitdir'::
 	This helper supports the corresponding command with the same name.
 
 'refspec' 'spec'::
@@ -187,6 +199,12 @@ CAPABILITIES
 	all, it must cover all refs reported by the list command; if
 	it is not used, it is effectively "{asterisk}:{asterisk}"
 
+'gitdir'::
+	The helper wants to know .git directory location. The gitdir
+	command is sent immediately after reading this capability.
+	The helper can read it immediately too or it can proceed with
+	reporting other capabilities and read it as a next command.
+
 REF LIST ATTRIBUTES
 -------------------
 
diff --git a/git-remote-testgit.py b/git-remote-testgit.py
index df9d512..df3e37f 100644
--- a/git-remote-testgit.py
+++ b/git-remote-testgit.py
@@ -35,7 +35,7 @@ def get_repo(alias, url):
     prefix = 'refs/testgit/%s/' % alias
     debug("prefix: '%s'", prefix)
 
-    repo.gitdir = ""
+    repo.gitdir = os.environ["GIT_DIR"]
     repo.alias = alias
     repo.prefix = prefix
 
@@ -70,7 +70,6 @@ def do_capabilities(repo, args):
 
     print "import"
     print "export"
-    print "gitdir"
     print "refspec refs/heads/*:%s*" % repo.prefix
 
     print # end capabilities
@@ -150,22 +149,11 @@ def do_export(repo, args):
     repo.non_local.push(repo.gitdir)
 
 
-def do_gitdir(repo, args):
-    """Stores the location of the gitdir.
-    """
-
-    if not args:
-        die("gitdir needs an argument")
-
-    repo.gitdir = ' '.join(args)
-
-
 COMMANDS = {
     'capabilities': do_capabilities,
     'list': do_list,
     'import': do_import,
     'export': do_export,
-    'gitdir': do_gitdir,
 }
 
 
diff --git a/transport-helper.c b/transport-helper.c
index 660147f..3282693 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -105,6 +105,12 @@ static struct child_process *get_helper(struct transport *transport)
 	int refspec_alloc = 0;
 	int duped;
 	int code;
+	char git_dir_buf[sizeof(GIT_DIR_ENVIRONMENT) + PATH_MAX + 1];
+	const char *helper_env[] = {
+		git_dir_buf,
+		NULL
+	};
+
 
 	if (data->helper)
 		return data->helper;
@@ -120,6 +126,10 @@ static struct child_process *get_helper(struct transport *transport)
 	helper->argv[2] = remove_ext_force(transport->url);
 	helper->git_cmd = 0;
 	helper->silent_exec_failure = 1;
+
+	snprintf(git_dir_buf, sizeof(git_dir_buf), "%s=%s", GIT_DIR_ENVIRONMENT, get_git_dir());
+	helper->env = helper_env;
+
 	code = start_command(helper);
 	if (code < 0 && errno == ENOENT)
 		die("Unable to find remote helper for '%s'", data->name);
-- 
1.7.3.4

Re: [PATCH v2 3/3] remote-helpers: export GIT_DIR variable to helpers

[Dmitry Ivankov]

+Sverre
Sorry for the noise.

On Wed, Jul 13, 2011 at 11:10 PM, Dmitry Ivankov <divanorama@gmail.com> wrote:
> The gitdir capability is recognized by git and can be used to tell
> the helper where the .git directory is. But it is not mentioned in
> the documentation and considered worse than if gitdir was passed
> via GIT_DIR environment variable.
>
> Do export GIT_DIR for a remote helper. Switch testgit to use env
> instead of less favoured capability gitdir.
>
> Mention it's possible uses in documentation: data storage, auxiliary
> git commands needed by a helper.
>
> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
> ---
> Instead of documenting capability gitdir export GIT_DIR. Turned out
> to be quite straightforward.
>
>  Documentation/git-remote-helpers.txt |   18 ++++++++++++++++++
>  git-remote-testgit.py                |   14 +-------------
>  transport-helper.c                   |   10 ++++++++++
>  3 files changed, 29 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
> index 58f6ad4..394fc8e 100644
> --- a/Documentation/git-remote-helpers.txt
> +++ b/Documentation/git-remote-helpers.txt
> @@ -47,6 +47,9 @@ arguments. The first argument specifies a remote repository as in git;
>  it is either the name of a configured remote or a URL. The second
>  argument specifies a URL; it is usually of the form
>  '<transport>://<address>', but any arbitrary string is possible.
> +'GIT_DIR' environment variable is set up for the remote helper and
> +can be used to store some additional data or to invoke auxiliary git
> +commands.
>
>  When git encounters a URL of the form '<transport>://<address>', where
>  '<transport>' is a protocol that it cannot handle natively, it
> @@ -159,6 +162,14 @@ Supported if the helper has the "import" capability.
>  +
>  Supported if the helper has the "connect" capability.
>
> +'gitdir' <path>::
> +       Tells helper the location of current repository .git
> +       directory. The path is absolute. The command is issued
> +       immediately after "gitdir" capability line is read by
> +       the caller.
> ++
> +Supported if the helper has the "gitdir" capability.
> +
>  If a fatal error occurs, the program writes the error message to
>  stderr and exits. The caller should expect that a suitable error
>  message has been printed if the child closes the connection without
> @@ -175,6 +186,7 @@ CAPABILITIES
>  'push'::
>  'import'::
>  'connect'::
> +'gitdir'::
>        This helper supports the corresponding command with the same name.
>
>  'refspec' 'spec'::
> @@ -187,6 +199,12 @@ CAPABILITIES
>        all, it must cover all refs reported by the list command; if
>        it is not used, it is effectively "{asterisk}:{asterisk}"
>
> +'gitdir'::
> +       The helper wants to know .git directory location. The gitdir
> +       command is sent immediately after reading this capability.
> +       The helper can read it immediately too or it can proceed with
> +       reporting other capabilities and read it as a next command.
> +
>  REF LIST ATTRIBUTES
>  -------------------
>
> diff --git a/git-remote-testgit.py b/git-remote-testgit.py
> index df9d512..df3e37f 100644
> --- a/git-remote-testgit.py
> +++ b/git-remote-testgit.py
> @@ -35,7 +35,7 @@ def get_repo(alias, url):
>     prefix = 'refs/testgit/%s/' % alias
>     debug("prefix: '%s'", prefix)
>
> -    repo.gitdir = ""
> +    repo.gitdir = os.environ["GIT_DIR"]
>     repo.alias = alias
>     repo.prefix = prefix
>
> @@ -70,7 +70,6 @@ def do_capabilities(repo, args):
>
>     print "import"
>     print "export"
> -    print "gitdir"
>     print "refspec refs/heads/*:%s*" % repo.prefix
>
>     print # end capabilities
> @@ -150,22 +149,11 @@ def do_export(repo, args):
>     repo.non_local.push(repo.gitdir)
>
>
> -def do_gitdir(repo, args):
> -    """Stores the location of the gitdir.
> -    """
> -
> -    if not args:
> -        die("gitdir needs an argument")
> -
> -    repo.gitdir = ' '.join(args)
> -
> -
>  COMMANDS = {
>     'capabilities': do_capabilities,
>     'list': do_list,
>     'import': do_import,
>     'export': do_export,
> -    'gitdir': do_gitdir,
>  }
>
>
> diff --git a/transport-helper.c b/transport-helper.c
> index 660147f..3282693 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -105,6 +105,12 @@ static struct child_process *get_helper(struct transport *transport)
>        int refspec_alloc = 0;
>        int duped;
>        int code;
> +       char git_dir_buf[sizeof(GIT_DIR_ENVIRONMENT) + PATH_MAX + 1];
> +       const char *helper_env[] = {
> +               git_dir_buf,
> +               NULL
> +       };
> +
>
>        if (data->helper)
>                return data->helper;
> @@ -120,6 +126,10 @@ static struct child_process *get_helper(struct transport *transport)
>        helper->argv[2] = remove_ext_force(transport->url);
>        helper->git_cmd = 0;
>        helper->silent_exec_failure = 1;
> +
> +       snprintf(git_dir_buf, sizeof(git_dir_buf), "%s=%s", GIT_DIR_ENVIRONMENT, get_git_dir());
> +       helper->env = helper_env;
> +
>        code = start_command(helper);
>        if (code < 0 && errno == ENOENT)
>                die("Unable to find remote helper for '%s'", data->name);
> --
> 1.7.3.4
>
>

Re: [PATCH v2 3/3] remote-helpers: export GIT_DIR variable to helpers

[Sverre Rabbelier]

Heya,

On Wed, Jul 13, 2011 at 19:10, Dmitry Ivankov <divanorama@gmail.com> wrote:
> Instead of documenting capability gitdir export GIT_DIR. Turned out
> to be quite straightforward.

But you _also_ documented the gitdir capability? I'm confused.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH v2 3/3] remote-helpers: export GIT_DIR variable to helpers

[Dmitry Ivankov]

Hi!

On Wed, Jul 13, 2011 at 11:36 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> Heya,
>
> On Wed, Jul 13, 2011 at 19:10, Dmitry Ivankov <divanorama@gmail.com> wrote:
>> Instead of documenting capability gitdir export GIT_DIR. Turned out
>> to be quite straightforward.
>
> But you _also_ documented the gitdir capability? I'm confused.
Oww, it was a bad idea to amend previous commit in a hurry and not to
read the whole diff after.
Of course Documentation/git-remote-helpers.txt should have only a GIT_DIR hunk:

diff --git a/Documentation/git-remote-helpers.txt
b/Documentation/git-remote-helpers.txt
index 58f6ad4..feda03e 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -47,6 +47,9 @@ arguments. The first argument specifies a remote
repository as in git;
 it is either the name of a configured remote or a URL. The second
 argument specifies a URL; it is usually of the form
 '<transport>://<address>', but any arbitrary string is possible.
+'GIT_DIR' environment variable is set up for the remote helper and
+can be used to store some additional data or to invoke auxiliary git
+commands.

 When git encounters a URL of the form '<transport>://<address>', where
 '<transport>' is a protocol that it cannot handle natively, it



>
> --
> Cheers,
>
> Sverre Rabbelier
>