{"thread":{"id":"27795","subject":"[PATCH 0/3] doc/{fast-import,remote-helpers}: few clarifications","startedAt":"2011-07-12T15:10:25Z","lastAt":"2011-07-22T19:32:58Z","messageCount":14,"participants":["Dmitry Ivankov","Junio C Hamano","Ramkumar Ramachandra","Sverre Rabbelier","Johan Herland"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"171150","messageId":"1310483428-29833-1-git-send-email-divanorama@gmail.com","threadId":"27795","inReplyTo":null,"subject":"[PATCH 0/3] doc/{fast-import,remote-helpers}: few clarifications","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-07-12T15:10:25Z","receivedAt":"2011-07-12T15:10:25Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"doc/fast-import part seems pretty clear.\ndoc/remote-helpers is related to a recent remote-helpers series [1] and \nmay need some discussion. For example, should we mention a bug [2] \naddressed as a part of [1], should we wait for [1] to document the \nfact that gitdir response can be read in the middle of capabilities \nreporting.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/176002\n[2] http://thread.gmane.org/gmane.comp.version-control.git/176002/focus=176020\n\nDmitry Ivankov (3):\n  doc/fast-import: clarify notemodify command\n  doc/fast-import: document feature import-marks-if-exists\n  doc/remote-helpers: document the gitdir feature\n\n Documentation/git-fast-import.txt    |   16 +++++++++++++---\n Documentation/git-remote-helpers.txt |   15 +++++++++++++++\n t/t9300-fast-import.sh               |   15 +++++++++++++++\n 3 files changed, 43 insertions(+), 3 deletions(-)\n\n-- \n1.7.3.4\n"},{"id":"171152","messageId":"1310483428-29833-2-git-send-email-divanorama@gmail.com","threadId":"27795","inReplyTo":"1310483428-29833-1-git-send-email-divanorama@gmail.com","subject":"[PATCH 1/3] doc/fast-import: clarify notemodify command","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-07-12T15:10:26Z","receivedAt":"2011-07-12T15:10:26Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"The \"notemodify\" fast-import command was introduced in commit a8dd2..\nThe commit log has slightly different description than the added\ndocumentation. The latter is somewhat confusing. \"notemodify\" is a\nsubcommand of \"commit\" command used to add a note for some commit.\nDoes this note annotate the commit produced by the \"commit\" command\nor a commit given by it's committish parameter? Which notes tree\ndoes it write notes to?\n\nThe exact meaning could be deduced with old description and some\nnotes machinery knowledge. But let's make it more obvious. This\ncommand is used in a context like \"commit refs/notes/test\" to\nadd or rewrite an annotation for a committish parameter. So the\nadvised way to add notes in a fast-import stream is:\n1) import some commits (optional)\n2) prepare a \"commit\" to the notes tree:\n2.1) choose notes ref, committer, log message, etc.\n2.2) create annotations with \"notemodify\", where each can refer to\na commit being annotated via a branch name, import mark reference,\nsha1 and other expressions specified in the Documentation.\n\nSigned-off-by: Dmitry Ivankov <divanorama@gmail.com>\n---\n Documentation/git-fast-import.txt |   11 ++++++++---\n 1 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 249249a..3f5b912 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -648,9 +648,14 @@ paths for a commit are encouraged to do so.\n \n `notemodify`\n ^^^^^^^^^^^^\n-Included in a `commit` command to add a new note (annotating a given\n-commit) or change the content of an existing note.  This command has\n-two different means of specifying the content of the note.\n+Included in a `commit` `<notes_ref>` command to add a new note\n+annotating a `<committish>` or change this annotation contents.\n+Internally it is similar to filemodify 100644 on `<committish>`\n+path (maybe split into subdirectories). It's not advised to\n+use any other commands to write to the `<notes_ref>` tree except\n+`filedeleteall` to delete all existing notes in this tree.\n+This command has two different means of specifying the content\n+of the note.\n \n External data format::\n \tThe data content for the note was already supplied by a prior\n-- \n1.7.3.4\n"},{"id":"171151","messageId":"1310483428-29833-3-git-send-email-divanorama@gmail.com","threadId":"27795","inReplyTo":"1310483428-29833-1-git-send-email-divanorama@gmail.com","subject":"[PATCH 2/3] doc/fast-import: document feature import-marks-if-exists","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-07-12T15:10:27Z","receivedAt":"2011-07-12T15:10:27Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"fast-import parameter --import-marks-if-exists was introduced in\ncommit dded4.. --import-marks can be set via a \"feature\" command in\na fast-import stream and --import-marks-if-exists has support for\nsuch specification too, for free.\n\nDocument \"feature import-marks-if-exists=<file>\" command and add a\nminimalistic test for it.\n\nSigned-off-by: Dmitry Ivankov <divanorama@gmail.com>\n---\n Documentation/git-fast-import.txt |    5 +++++\n t/t9300-fast-import.sh            |   15 +++++++++++++++\n 2 files changed, 20 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 3f5b912..bdcc81c 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -1005,6 +1005,11 @@ import-marks::\n \tsecond, an --import-marks= command-line option overrides\n \tany \"feature import-marks\" command in the stream.\n \n+import-marks-if-exists::\n+\tLike import-marks but instead of erroring out, silently\n+\tskips the file if it does not exist. Differences from --\n+\tversion are the same as with import-marks feature above.\n+\n cat-blob::\n ls::\n \tRequire that the backend support the 'cat-blob' or 'ls' command.\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 2a53640..eed57df 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -1882,6 +1882,21 @@ test_expect_success 'R: --import-marks-if-exists' '\n \ttest_cmp expect io.marks\n '\n \n+test_expect_success 'R: feature import-marks-if-exists' '\n+\trm -f io.marks &&\n+\tblob=$(echo hi | git hash-object --stdin) &&\n+\techo \":1 $blob\" >expect &&\n+\tgit fast-import --import-marks-if-exists=io.marks --export-marks=io.marks <<-\\EOF &&\n+\tfeature import-marks-if-exists=io.marks\n+\tblob\n+\tmark :1\n+\tdata 3\n+\thi\n+\n+\tEOF\n+\ttest_cmp expect io.marks\n+'\n+\n cat >input << EOF\n feature import-marks=marks.out\n feature export-marks=marks.new\n-- \n1.7.3.4\n"},{"id":"171153","messageId":"1310483428-29833-4-git-send-email-divanorama@gmail.com","threadId":"27795","inReplyTo":"1310483428-29833-1-git-send-email-divanorama@gmail.com","subject":"[PATCH 3/3] doc/remote-helpers: document the gitdir feature","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-07-12T15:10:28Z","receivedAt":"2011-07-12T15:10:28Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"The gitdir capability is recognized by git and can be used to tell\nthe helper where the .git directory is. But it is not mentioned in\nthe documentation.\n\nDocument the gitdir feature. Upon reading this capability\ntransport-helper sends out \"gitdir path_to_dot_git\" command to the\nhelper. Which looks exactly like sending a command. So let's call\nthis a command.\n\nSigned-off-by: Dmitry Ivankov <divanorama@gmail.com>\n---\n Documentation/git-remote-helpers.txt |   15 +++++++++++++++\n 1 files changed, 15 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt\nindex 58f6ad4..a16a5e5 100644\n--- a/Documentation/git-remote-helpers.txt\n+++ b/Documentation/git-remote-helpers.txt\n@@ -159,6 +159,14 @@ Supported if the helper has the \"import\" capability.\n +\n Supported if the helper has the \"connect\" capability.\n \n+'gitdir' <path>::\n+\tTells helper the location of current repository .git\n+\tdirectory. The path is absolute. The command is issued\n+\timmediately after \"gitdir\" capability line is read by\n+\tthe caller.\n++\n+Supported if the helper has the \"gitdir\" capability.\n+\n If a fatal error occurs, the program writes the error message to\n stderr and exits. The caller should expect that a suitable error\n message has been printed if the child closes the connection without\n@@ -175,6 +183,7 @@ CAPABILITIES\n 'push'::\n 'import'::\n 'connect'::\n+'gitdir'::\n \tThis helper supports the corresponding command with the same name.\n \n 'refspec' 'spec'::\n@@ -187,6 +196,12 @@ CAPABILITIES\n \tall, it must cover all refs reported by the list command; if\n \tit is not used, it is effectively \"{asterisk}:{asterisk}\"\n \n+'gitdir'::\n+\tThe helper wants to know .git directory location. The gitdir\n+\tcommand is sent immediately after reading this capability.\n+\tThe helper can read it immediately too or it can proceed with\n+\treporting other capabilities and read it as a next command.\n+\n REF LIST ATTRIBUTES\n -------------------\n \n-- \n1.7.3.4\n"},{"id":"171160","messageId":"7voc0z8mdd.fsf@alter.siamese.dyndns.org","threadId":"27795","inReplyTo":"1310483428-29833-3-git-send-email-divanorama@gmail.com","subject":"Re: [PATCH 2/3] doc/fast-import: document feature import-marks-if-exists","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-07-12T17:00:46Z","receivedAt":"2011-07-12T17:00:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dmitry Ivankov <divanorama@gmail.com> writes:\n\n> fast-import parameter --import-marks-if-exists was introduced in\n> commit dded4.. --import-marks can be set via a \"feature\" command in\n> a fast-import stream and --import-marks-if-exists has support for\n> such specification too, for free.\n>\n> Document \"feature import-marks-if-exists=<file>\" command and add a\n> minimalistic test for it.\n>\n> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>\n\nThanks.\n\n>  Documentation/git-fast-import.txt |    5 +++++\n>  t/t9300-fast-import.sh            |   15 +++++++++++++++\n>  2 files changed, 20 insertions(+), 0 deletions(-)\n>\n> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\n> index 3f5b912..bdcc81c 100644\n> --- a/Documentation/git-fast-import.txt\n> +++ b/Documentation/git-fast-import.txt\n> @@ -1005,6 +1005,11 @@ import-marks::\n>  \tsecond, an --import-marks= command-line option overrides\n>  \tany \"feature import-marks\" command in the stream.\n>  \n> +import-marks-if-exists::\n> +\tLike import-marks but instead of erroring out, silently\n> +\tskips the file if it does not exist. Differences from --\n> +\tversion are the same as with import-marks feature above.\n\nWouldn't it be more concise and complete to do it this way instead?  It\navoids duplication and also clarifies you cannot mix these two.\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 249249a..e939e74 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -995,10 +995,14 @@ force::\n \t(see OPTIONS, above).\n \n import-marks::\n+import-marks-if-exists::\n \tLike --import-marks except in two respects: first, only one\n-\t\"feature import-marks\" command is allowed per stream;\n+\t\"feature import-marks\" command or \"feature import-marks-if-exists\"\n+\tis allowed per stream;\n \tsecond, an --import-marks= command-line option overrides\n-\tany \"feature import-marks\" command in the stream.\n+\tany \"feature import-marks\" command in the stream. Similarly,\n+\timport-marks-if-exists corresponds to --import-marks-if-exists\n+\tfrom the command line.\n \n cat-blob::\n ls::\n\n>  \n> +test_expect_success 'R: feature import-marks-if-exists' '\n> +\trm -f io.marks &&\n> +\tblob=$(echo hi | git hash-object --stdin) &&\n> +\techo \":1 $blob\" >expect &&\n> +\tgit fast-import --import-marks-if-exists=io.marks --export-marks=io.marks <<-\\EOF &&\n> +\tfeature import-marks-if-exists=io.marks\n> +\tblob\n> +\tmark :1\n> +\tdata 3\n> +\thi\n> +\n> +\tEOF\n> +\ttest_cmp expect io.marks\n> +'\n\nDo you really want both the command line option and feature in this test?\n\nThere are at least four cases [*1*] you would want to test (a part of\nblame lies in the original test you copied this from).\n\n - \"if exists\" should not fail if the marks file does not exist;\n - \"if exists\" reads marks if exists (not tested in this patch);\n - having \"feature import-marks-if-exists\" will trigger this feature\n   without command line option (not tested in this patch); and\n - having \"feature import-marks-if-exists\" will be overridden by\n   the command line option (not tested in this patch).\n\n\n\n\n[Footnote]\n\n*1* Ideally, the test should also cover these 16 combinations:\n\n  io.marks file (2 cases: exists or missing)\n\ntimes\n\n  --import-mark-if-exists (2 cases: exists or missing)\n\ntimes\n\n feature import-marks-if-exists (4 cases: missing,\n \t\t\t\tsame as command line,\n                                different from command line,\n\t\t\t\tgiven more than one)\n"},{"id":"171230","messageId":"CALkWK0mkiVVAWpq2WMFZuSks4rJhrYA07Z0XyTFZUzxxmnUa-w@mail.gmail.com","threadId":"27795","inReplyTo":"1310483428-29833-3-git-send-email-divanorama@gmail.com","subject":"Re: [PATCH 2/3] doc/fast-import: document feature import-marks-if-exists","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-07-13T10:27:56Z","receivedAt":"2011-07-13T10:27:56Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Dmitry,\n\nIn addition to Junio's comments, I have a few things to add:\n\nDmitry Ivankov writes:\n> fast-import parameter --import-marks-if-exists was introduced in\n\nIt's a command-line option, not a parameter.\n\n> commit dded4.. --import-marks can be set via a \"feature\" command in\n\nPlease more SHA-1 hex characters while abbreviating, and write a\nlittle more information about the commit:\ndded4f1 (fast-import: Introduce --import-marks-if-exists, 2011-01-15)\n\n> a fast-import stream and --import-marks-if-exists has support for\n> such specification too, for free.\n>\n> Document \"feature import-marks-if-exists=<file>\" command and add a\n> minimalistic test for it.\n\nPerhaps you can make it clearer that this feature was available back\nin dded4f1, and was supposed to have been documented then?\n\nThanks for working on this.  I'm to blame for not documenting/ testing\nthe feature in the original patch.\n\n-- Ram\n"},{"id":"171231","messageId":"CAGdFq_igyPMfhhKb-3v5Vnob-DQ9GLR0ONLEc+xjgTwZ1R1sRg@mail.gmail.com","threadId":"27795","inReplyTo":"1310483428-29833-4-git-send-email-divanorama@gmail.com","subject":"Re: [PATCH 3/3] doc/remote-helpers: document the gitdir feature","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-07-13T12:04:24Z","receivedAt":"2011-07-13T12:04:24Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Tue, Jul 12, 2011 at 17:10, Dmitry Ivankov <divanorama@gmail.com> wrote:\n> The gitdir capability is recognized by git and can be used to tell\n> the helper where the .git directory is. But it is not mentioned in\n> the documentation.\n>\n> Document the gitdir feature. Upon reading this capability\n> transport-helper sends out \"gitdir path_to_dot_git\" command to the\n> helper. Which looks exactly like sending a command. So let's call\n> this a command.\n\nJohannes suggested that we should remove this feature and instead\nalways export an GITDIR environmental variable. I'm inclined to agree.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"171269","messageId":"1310577055-6347-1-git-send-email-divanorama@gmail.com","threadId":"27795","inReplyTo":"1310483428-29833-1-git-send-email-divanorama@gmail.com","subject":"[PATCH v2 1/3] doc/fast-import: clarify notemodify command","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-07-13T17:10:53Z","receivedAt":"2011-07-13T17:10:53Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"The \"notemodify\" fast-import command was introduced in commit a8dd2e7\n(fast-import: Add support for importing commit notes, 2009-10-09)\nThe commit log has slightly different description than the added\ndocumentation. The latter is somewhat confusing. \"notemodify\" is a\nsubcommand of \"commit\" command used to add a note for some commit.\nDoes this note annotate the commit produced by the \"commit\" command\nor a commit given by it's committish parameter? Which notes tree\ndoes it write notes to?\n\nThe exact meaning could be deduced with old description and some\nnotes machinery knowledge. But let's make it more obvious. This\ncommand is used in a context like \"commit refs/notes/test\" to\nadd or rewrite an annotation for a committish parameter. So the\nadvised way to add notes in a fast-import stream is:\n1) import some commits (optional)\n2) prepare a \"commit\" to the notes tree:\n2.1) choose notes ref, committer, log message, etc.\n2.2) create annotations with \"notemodify\", where each can refer to\na commit being annotated via a branch name, import mark reference,\nsha1 and other expressions specified in the Documentation.\n\nSigned-off-by: Dmitry Ivankov <divanorama@gmail.com>\n---\nJust added a bit more metadata of a8dd2e7.\n\n Documentation/git-fast-import.txt |   11 ++++++++---\n 1 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 249249a..3f5b912 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -648,9 +648,14 @@ paths for a commit are encouraged to do so.\n \n `notemodify`\n ^^^^^^^^^^^^\n-Included in a `commit` command to add a new note (annotating a given\n-commit) or change the content of an existing note.  This command has\n-two different means of specifying the content of the note.\n+Included in a `commit` `<notes_ref>` command to add a new note\n+annotating a `<committish>` or change this annotation contents.\n+Internally it is similar to filemodify 100644 on `<committish>`\n+path (maybe split into subdirectories). It's not advised to\n+use any other commands to write to the `<notes_ref>` tree except\n+`filedeleteall` to delete all existing notes in this tree.\n+This command has two different means of specifying the content\n+of the note.\n \n External data format::\n \tThe data content for the note was already supplied by a prior\n-- \n1.7.3.4\n"},{"id":"171268","messageId":"1310577055-6347-2-git-send-email-divanorama@gmail.com","threadId":"27795","inReplyTo":"1310483428-29833-1-git-send-email-divanorama@gmail.com","subject":"[PATCH v2 2/3] doc/fast-import: document feature import-marks-if-exists","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-07-13T17:10:54Z","receivedAt":"2011-07-13T17:10:54Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"fast-import command-line option --import-marks-if-exists was introduced\nin commit dded4f1 (fast-import: Introduce --import-marks-if-exists, 2011-01-15)\n\n--import-marks option can be set via a \"feature\" command in a fast-import\nstream and --import-marks-if-exists had support for such specification\nfrom the very beginning too due to some shared codebase. Though the\ndocumentation for this feature wasn't written in dded4f1.\n\nAdd the documentation for \"feature import-marks-if-exists=<file>\". Also add\na minimalistic test for it.\n\nSigned-off-by: Dmitry Ivankov <divanorama@gmail.com>\n---\nReworded a bit, united import-marks* descriptions. One more case to test:\n--import-marks-if-exists= overrides feature import-marks=. And \"fixed\" the\ntest, I left the --import-marks accidentally, though it was a fine test too.\nAlso not feeling like adding a whole ton of coverage tests yet.\n\n Documentation/git-fast-import.txt |   10 +++++++---\n t/t9300-fast-import.sh            |   15 +++++++++++++++\n 2 files changed, 22 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 3f5b912..3a5aa55 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -1000,10 +1000,14 @@ force::\n \t(see OPTIONS, above).\n \n import-marks::\n+import-marks-if-exists::\n \tLike --import-marks except in two respects: first, only one\n-\t\"feature import-marks\" command is allowed per stream;\n-\tsecond, an --import-marks= command-line option overrides\n-\tany \"feature import-marks\" command in the stream.\n+\t\"feature import-marks\" or \"feature import-marks-if-exists\"\n+\tcommand is allowed per stream; second, an --import-marks=\n+\tor --import-marks-if-exists command-line option overrides\n+\tany of these \"feature\" commands in the stream; third,\n+\t\"feature import-marks-if-exists\" like a corresponding\n+\tcommand-line option silently skips a nonexistent file.\n \n cat-blob::\n ls::\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 2a53640..9af1a78 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -1882,6 +1882,21 @@ test_expect_success 'R: --import-marks-if-exists' '\n \ttest_cmp expect io.marks\n '\n \n+test_expect_success 'R: feature import-marks-if-exists' '\n+\trm -f io.marks &&\n+\tblob=$(echo hi | git hash-object --stdin) &&\n+\techo \":1 $blob\" >expect &&\n+\tgit fast-import --export-marks=io.marks <<-\\EOF &&\n+\tfeature import-marks-if-exists=io.marks\n+\tblob\n+\tmark :1\n+\tdata 3\n+\thi\n+\n+\tEOF\n+\ttest_cmp expect io.marks\n+'\n+\n cat >input << EOF\n feature import-marks=marks.out\n feature export-marks=marks.new\n-- \n1.7.3.4\n"},{"id":"171270","messageId":"1310577055-6347-3-git-send-email-divanorama@gmail.com","threadId":"27795","inReplyTo":"1310483428-29833-1-git-send-email-divanorama@gmail.com","subject":"[PATCH v2 3/3] remote-helpers: export GIT_DIR variable to helpers","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-07-13T17:10:55Z","receivedAt":"2011-07-13T17:10:55Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"The gitdir capability is recognized by git and can be used to tell\nthe helper where the .git directory is. But it is not mentioned in\nthe documentation and considered worse than if gitdir was passed\nvia GIT_DIR environment variable.\n\nDo export GIT_DIR for a remote helper. Switch testgit to use env\ninstead of less favoured capability gitdir.\n\nMention it's possible uses in documentation: data storage, auxiliary\ngit commands needed by a helper.\n\nSigned-off-by: Dmitry Ivankov <divanorama@gmail.com>\n---\nInstead of documenting capability gitdir export GIT_DIR. Turned out\nto be quite straightforward.\n\n Documentation/git-remote-helpers.txt |   18 ++++++++++++++++++\n git-remote-testgit.py                |   14 +-------------\n transport-helper.c                   |   10 ++++++++++\n 3 files changed, 29 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt\nindex 58f6ad4..394fc8e 100644\n--- a/Documentation/git-remote-helpers.txt\n+++ b/Documentation/git-remote-helpers.txt\n@@ -47,6 +47,9 @@ arguments. The first argument specifies a remote repository as in git;\n it is either the name of a configured remote or a URL. The second\n argument specifies a URL; it is usually of the form\n '<transport>://<address>', but any arbitrary string is possible.\n+'GIT_DIR' environment variable is set up for the remote helper and\n+can be used to store some additional data or to invoke auxiliary git\n+commands.\n \n When git encounters a URL of the form '<transport>://<address>', where\n '<transport>' is a protocol that it cannot handle natively, it\n@@ -159,6 +162,14 @@ Supported if the helper has the \"import\" capability.\n +\n Supported if the helper has the \"connect\" capability.\n \n+'gitdir' <path>::\n+\tTells helper the location of current repository .git\n+\tdirectory. The path is absolute. The command is issued\n+\timmediately after \"gitdir\" capability line is read by\n+\tthe caller.\n++\n+Supported if the helper has the \"gitdir\" capability.\n+\n If a fatal error occurs, the program writes the error message to\n stderr and exits. The caller should expect that a suitable error\n message has been printed if the child closes the connection without\n@@ -175,6 +186,7 @@ CAPABILITIES\n 'push'::\n 'import'::\n 'connect'::\n+'gitdir'::\n \tThis helper supports the corresponding command with the same name.\n \n 'refspec' 'spec'::\n@@ -187,6 +199,12 @@ CAPABILITIES\n \tall, it must cover all refs reported by the list command; if\n \tit is not used, it is effectively \"{asterisk}:{asterisk}\"\n \n+'gitdir'::\n+\tThe helper wants to know .git directory location. The gitdir\n+\tcommand is sent immediately after reading this capability.\n+\tThe helper can read it immediately too or it can proceed with\n+\treporting other capabilities and read it as a next command.\n+\n REF LIST ATTRIBUTES\n -------------------\n \ndiff --git a/git-remote-testgit.py b/git-remote-testgit.py\nindex df9d512..df3e37f 100644\n--- a/git-remote-testgit.py\n+++ b/git-remote-testgit.py\n@@ -35,7 +35,7 @@ def get_repo(alias, url):\n     prefix = 'refs/testgit/%s/' % alias\n     debug(\"prefix: '%s'\", prefix)\n \n-    repo.gitdir = \"\"\n+    repo.gitdir = os.environ[\"GIT_DIR\"]\n     repo.alias = alias\n     repo.prefix = prefix\n \n@@ -70,7 +70,6 @@ def do_capabilities(repo, args):\n \n     print \"import\"\n     print \"export\"\n-    print \"gitdir\"\n     print \"refspec refs/heads/*:%s*\" % repo.prefix\n \n     print # end capabilities\n@@ -150,22 +149,11 @@ def do_export(repo, args):\n     repo.non_local.push(repo.gitdir)\n \n \n-def do_gitdir(repo, args):\n-    \"\"\"Stores the location of the gitdir.\n-    \"\"\"\n-\n-    if not args:\n-        die(\"gitdir needs an argument\")\n-\n-    repo.gitdir = ' '.join(args)\n-\n-\n COMMANDS = {\n     'capabilities': do_capabilities,\n     'list': do_list,\n     'import': do_import,\n     'export': do_export,\n-    'gitdir': do_gitdir,\n }\n \n \ndiff --git a/transport-helper.c b/transport-helper.c\nindex 660147f..3282693 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -105,6 +105,12 @@ static struct child_process *get_helper(struct transport *transport)\n \tint refspec_alloc = 0;\n \tint duped;\n \tint code;\n+\tchar git_dir_buf[sizeof(GIT_DIR_ENVIRONMENT) + PATH_MAX + 1];\n+\tconst char *helper_env[] = {\n+\t\tgit_dir_buf,\n+\t\tNULL\n+\t};\n+\n \n \tif (data->helper)\n \t\treturn data->helper;\n@@ -120,6 +126,10 @@ static struct child_process *get_helper(struct transport *transport)\n \thelper->argv[2] = remove_ext_force(transport->url);\n \thelper->git_cmd = 0;\n \thelper->silent_exec_failure = 1;\n+\n+\tsnprintf(git_dir_buf, sizeof(git_dir_buf), \"%s=%s\", GIT_DIR_ENVIRONMENT, get_git_dir());\n+\thelper->env = helper_env;\n+\n \tcode = start_command(helper);\n \tif (code < 0 && errno == ENOENT)\n \t\tdie(\"Unable to find remote helper for '%s'\", data->name);\n-- \n1.7.3.4\n"},{"id":"171271","messageId":"CA+gfSn8=kLvYzFcdB-caNLfK3RQtOP8V=-jdFtDgQ41KG0ngOQ@mail.gmail.com","threadId":"27795","inReplyTo":"1310577055-6347-3-git-send-email-divanorama@gmail.com","subject":"Re: [PATCH v2 3/3] remote-helpers: export GIT_DIR variable to helpers","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-07-13T17:14:31Z","receivedAt":"2011-07-13T17:14:31Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"+Sverre\nSorry for the noise.\n\nOn Wed, Jul 13, 2011 at 11:10 PM, Dmitry Ivankov <divanorama@gmail.com> wrote:\n> The gitdir capability is recognized by git and can be used to tell\n> the helper where the .git directory is. But it is not mentioned in\n> the documentation and considered worse than if gitdir was passed\n> via GIT_DIR environment variable.\n>\n> Do export GIT_DIR for a remote helper. Switch testgit to use env\n> instead of less favoured capability gitdir.\n>\n> Mention it's possible uses in documentation: data storage, auxiliary\n> git commands needed by a helper.\n>\n> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>\n> ---\n> Instead of documenting capability gitdir export GIT_DIR. Turned out\n> to be quite straightforward.\n>\n>  Documentation/git-remote-helpers.txt |   18 ++++++++++++++++++\n>  git-remote-testgit.py                |   14 +-------------\n>  transport-helper.c                   |   10 ++++++++++\n>  3 files changed, 29 insertions(+), 13 deletions(-)\n>\n> diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt\n> index 58f6ad4..394fc8e 100644\n> --- a/Documentation/git-remote-helpers.txt\n> +++ b/Documentation/git-remote-helpers.txt\n> @@ -47,6 +47,9 @@ arguments. The first argument specifies a remote repository as in git;\n>  it is either the name of a configured remote or a URL. The second\n>  argument specifies a URL; it is usually of the form\n>  '<transport>://<address>', but any arbitrary string is possible.\n> +'GIT_DIR' environment variable is set up for the remote helper and\n> +can be used to store some additional data or to invoke auxiliary git\n> +commands.\n>\n>  When git encounters a URL of the form '<transport>://<address>', where\n>  '<transport>' is a protocol that it cannot handle natively, it\n> @@ -159,6 +162,14 @@ Supported if the helper has the \"import\" capability.\n>  +\n>  Supported if the helper has the \"connect\" capability.\n>\n> +'gitdir' <path>::\n> +       Tells helper the location of current repository .git\n> +       directory. The path is absolute. The command is issued\n> +       immediately after \"gitdir\" capability line is read by\n> +       the caller.\n> ++\n> +Supported if the helper has the \"gitdir\" capability.\n> +\n>  If a fatal error occurs, the program writes the error message to\n>  stderr and exits. The caller should expect that a suitable error\n>  message has been printed if the child closes the connection without\n> @@ -175,6 +186,7 @@ CAPABILITIES\n>  'push'::\n>  'import'::\n>  'connect'::\n> +'gitdir'::\n>        This helper supports the corresponding command with the same name.\n>\n>  'refspec' 'spec'::\n> @@ -187,6 +199,12 @@ CAPABILITIES\n>        all, it must cover all refs reported by the list command; if\n>        it is not used, it is effectively \"{asterisk}:{asterisk}\"\n>\n> +'gitdir'::\n> +       The helper wants to know .git directory location. The gitdir\n> +       command is sent immediately after reading this capability.\n> +       The helper can read it immediately too or it can proceed with\n> +       reporting other capabilities and read it as a next command.\n> +\n>  REF LIST ATTRIBUTES\n>  -------------------\n>\n> diff --git a/git-remote-testgit.py b/git-remote-testgit.py\n> index df9d512..df3e37f 100644\n> --- a/git-remote-testgit.py\n> +++ b/git-remote-testgit.py\n> @@ -35,7 +35,7 @@ def get_repo(alias, url):\n>     prefix = 'refs/testgit/%s/' % alias\n>     debug(\"prefix: '%s'\", prefix)\n>\n> -    repo.gitdir = \"\"\n> +    repo.gitdir = os.environ[\"GIT_DIR\"]\n>     repo.alias = alias\n>     repo.prefix = prefix\n>\n> @@ -70,7 +70,6 @@ def do_capabilities(repo, args):\n>\n>     print \"import\"\n>     print \"export\"\n> -    print \"gitdir\"\n>     print \"refspec refs/heads/*:%s*\" % repo.prefix\n>\n>     print # end capabilities\n> @@ -150,22 +149,11 @@ def do_export(repo, args):\n>     repo.non_local.push(repo.gitdir)\n>\n>\n> -def do_gitdir(repo, args):\n> -    \"\"\"Stores the location of the gitdir.\n> -    \"\"\"\n> -\n> -    if not args:\n> -        die(\"gitdir needs an argument\")\n> -\n> -    repo.gitdir = ' '.join(args)\n> -\n> -\n>  COMMANDS = {\n>     'capabilities': do_capabilities,\n>     'list': do_list,\n>     'import': do_import,\n>     'export': do_export,\n> -    'gitdir': do_gitdir,\n>  }\n>\n>\n> diff --git a/transport-helper.c b/transport-helper.c\n> index 660147f..3282693 100644\n> --- a/transport-helper.c\n> +++ b/transport-helper.c\n> @@ -105,6 +105,12 @@ static struct child_process *get_helper(struct transport *transport)\n>        int refspec_alloc = 0;\n>        int duped;\n>        int code;\n> +       char git_dir_buf[sizeof(GIT_DIR_ENVIRONMENT) + PATH_MAX + 1];\n> +       const char *helper_env[] = {\n> +               git_dir_buf,\n> +               NULL\n> +       };\n> +\n>\n>        if (data->helper)\n>                return data->helper;\n> @@ -120,6 +126,10 @@ static struct child_process *get_helper(struct transport *transport)\n>        helper->argv[2] = remove_ext_force(transport->url);\n>        helper->git_cmd = 0;\n>        helper->silent_exec_failure = 1;\n> +\n> +       snprintf(git_dir_buf, sizeof(git_dir_buf), \"%s=%s\", GIT_DIR_ENVIRONMENT, get_git_dir());\n> +       helper->env = helper_env;\n> +\n>        code = start_command(helper);\n>        if (code < 0 && errno == ENOENT)\n>                die(\"Unable to find remote helper for '%s'\", data->name);\n> --\n> 1.7.3.4\n>\n>\n"},{"id":"171272","messageId":"CAGdFq_gAyjz+WCpK5qW7xcyGxSx4gg-A7Ju-AaHQ2o9mM_=pvQ@mail.gmail.com","threadId":"27795","inReplyTo":"1310577055-6347-3-git-send-email-divanorama@gmail.com","subject":"Re: [PATCH v2 3/3] remote-helpers: export GIT_DIR variable to helpers","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-07-13T17:36:16Z","receivedAt":"2011-07-13T17:36:16Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Jul 13, 2011 at 19:10, Dmitry Ivankov <divanorama@gmail.com> wrote:\n> Instead of documenting capability gitdir export GIT_DIR. Turned out\n> to be quite straightforward.\n\nBut you _also_ documented the gitdir capability? I'm confused.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"171275","messageId":"CA+gfSn9QeK84xVWrxYBGOwfWbeuS7cU_=D10Lrn8S__iygx+mg@mail.gmail.com","threadId":"27795","inReplyTo":"CAGdFq_gAyjz+WCpK5qW7xcyGxSx4gg-A7Ju-AaHQ2o9mM_=pvQ@mail.gmail.com","subject":"Re: [PATCH v2 3/3] remote-helpers: export GIT_DIR variable to helpers","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-07-13T18:10:40Z","receivedAt":"2011-07-13T18:10:40Z","isPatch":true,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"Hi!\n\nOn Wed, Jul 13, 2011 at 11:36 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> Heya,\n>\n> On Wed, Jul 13, 2011 at 19:10, Dmitry Ivankov <divanorama@gmail.com> wrote:\n>> Instead of documenting capability gitdir export GIT_DIR. Turned out\n>> to be quite straightforward.\n>\n> But you _also_ documented the gitdir capability? I'm confused.\nOww, it was a bad idea to amend previous commit in a hurry and not to\nread the whole diff after.\nOf course Documentation/git-remote-helpers.txt should have only a GIT_DIR hunk:\n\ndiff --git a/Documentation/git-remote-helpers.txt\nb/Documentation/git-remote-helpers.txt\nindex 58f6ad4..feda03e 100644\n--- a/Documentation/git-remote-helpers.txt\n+++ b/Documentation/git-remote-helpers.txt\n@@ -47,6 +47,9 @@ arguments. The first argument specifies a remote\nrepository as in git;\n it is either the name of a configured remote or a URL. The second\n argument specifies a URL; it is usually of the form\n '<transport>://<address>', but any arbitrary string is possible.\n+'GIT_DIR' environment variable is set up for the remote helper and\n+can be used to store some additional data or to invoke auxiliary git\n+commands.\n\n When git encounters a URL of the form '<transport>://<address>', where\n '<transport>' is a protocol that it cannot handle natively, it\n\n\n\n>\n> --\n> Cheers,\n>\n> Sverre Rabbelier\n>\n"},{"id":"171857","messageId":"201107222132.59145.johan@herland.net","threadId":"27795","inReplyTo":"1310577055-6347-1-git-send-email-divanorama@gmail.com","subject":"Re: [PATCH v2 1/3] doc/fast-import: clarify notemodify command","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2011-07-22T19:32:58Z","receivedAt":"2011-07-22T19:32:58Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wednesday 13 July 2011, Dmitry Ivankov wrote:\n> The \"notemodify\" fast-import command was introduced in commit a8dd2e7\n> (fast-import: Add support for importing commit notes, 2009-10-09)\n> The commit log has slightly different description than the added\n> documentation. The latter is somewhat confusing. \"notemodify\" is a\n> subcommand of \"commit\" command used to add a note for some commit.\n> Does this note annotate the commit produced by the \"commit\" command\n> or a commit given by it's committish parameter? Which notes tree\n> does it write notes to?\n> \n> The exact meaning could be deduced with old description and some\n> notes machinery knowledge. But let's make it more obvious. This\n> command is used in a context like \"commit refs/notes/test\" to\n> add or rewrite an annotation for a committish parameter. So the\n> advised way to add notes in a fast-import stream is:\n> 1) import some commits (optional)\n> 2) prepare a \"commit\" to the notes tree:\n> 2.1) choose notes ref, committer, log message, etc.\n> 2.2) create annotations with \"notemodify\", where each can refer to\n> a commit being annotated via a branch name, import mark reference,\n> sha1 and other expressions specified in the Documentation.\n> \n> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>\n\nAcked-by: Johan Herland <johan@herland.net>\n\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"}]}