{"thread":{"id":"26385","subject":"[PATCH/RFC 0/2] fast-import: introduce \"feature notes\" command","startedAt":"2011-02-02T04:58:26Z","lastAt":"2011-02-09T23:06:51Z","messageCount":10,"participants":["Jonathan Nieder","Thomas Rast","Sverre Rabbelier"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"160256","messageId":"20110202045826.GC15285@elie","threadId":"26385","inReplyTo":null,"subject":"[PATCH/RFC 0/2] fast-import: introduce \"feature notes\" command","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-02T04:58:26Z","receivedAt":"2011-02-02T04:58:26Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nAs promised, here is a \"feature\" command for streams to use to require\nsupport for the notemodify (N) command.\n\nPatch 1 explains how to delete notes. On first reading, I had thought\nthat feature was missing.\n\nPatch 2 is the title feature. The relevant message explains why\nthis is an rfc.\n\nThoughts welcome, as always.\n\nJonathan Nieder (2):\n Documentation/fast-import: explain how to delete a note\n fast-import: introduce \"feature notes\" command\n\n Documentation/git-fast-import.txt | 18 ++++++++++--------\n fast-import.c | 6 ++++--\n t/t9301-fast-import-notes.sh | 1 +\n 3 files changed, 15 insertions(+), 10 deletions(-)\n"},{"id":"160257","messageId":"20110202050022.GD15285@elie","threadId":"26385","inReplyTo":"20110202045826.GC15285@elie","subject":"[PATCH 1/2] Documentation/fast-import: explain how to remove a note","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-02T05:00:22Z","receivedAt":"2011-02-02T05:00:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"A notemodify (N) command with blob id consisting of 40 zeroes (so\nis_null_sha1 is true) means to not add a note to replace the existing,\nremoved one.\n\nSigned-off-by: Jonathan Nieder \n---\n Documentation/git-fast-import.txt | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 4415e63..086b14f 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -651,9 +651,9 @@ External data format::\n \t'N' SP SP LF\n ....\n +\n-Here `` can be either a mark reference (`:`)\n-set by a prior `blob` command, or a full 40-byte SHA-1 of an\n-existing Git blob object.\n+Here `` can be a mark reference (`:`)\n+set by a prior `blob` command, a full 40-byte SHA-1 of an\n+existing Git blob object, or 40 zeroes, to remove a note.\n \n Inline data format::\n \tThe data content for the note has not been supplied yet.\n-- \n1.7.2.3\n"},{"id":"160258","messageId":"20110202050735.GE15285@elie","threadId":"26385","inReplyTo":"20110202045826.GC15285@elie","subject":"[PATCH 2/2] fast-import: introduce \"feature notes\" command","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-02T05:07:35Z","receivedAt":"2011-02-02T05:07:35Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Here is a \"feature\" command for streams to use to require support for\nthe notemodify (N) command.\n\nSupport for importing notes was added to git fast-import quite a while\nago (v1.6.6-rc0~21^2~8, 2009-10-09), before the 'feature' facility was\nintroduced (v1.7.0-rc0~95^2~4, fast-import: add feature command,\n2009-12-04) so for compatibility with older git versions, authors\nof existing frontends should not start using the \"feature notes\"\ncommand. Most git versions in wide use support notemodify already.\n\nThe purpose of the \"feature notes\" declaration is instead to\ndistinguish between git and fast-import backends that do not support\nnotemodify. In git \"feature notes\" will be a no-op while in other\ncurrent fast-import backends it will error out with a clear error\nmessage.\n\nSigned-off-by: Jonathan Nieder \n---\nStill to do: the documentation does not hint at the point mentioned\nin paragraphs 2 and 3 above. Ideas for wording?\n\nAside from that, I think this is ready. Thanks for reading.\n\n Documentation/git-fast-import.txt | 12 +++++++-----\n fast-import.c | 6 ++++--\n t/t9301-fast-import-notes.sh | 1 +\n 3 files changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 086b14f..2393252 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -942,11 +942,13 @@ import-marks::\n \tany \"feature import-marks\" command in the stream.\n \n cat-blob::\n-\tIgnored. Versions of fast-import not supporting the\n-\t\"cat-blob\" command will exit with a message indicating so.\n-\tThis lets the import error out early with a clear message,\n-\trather than wasting time on the early part of an import\n-\tbefore the unsupported command is detected.\n+\tRequire that the backend support the 'cat-blob' command.\n+\tVersions of fast-import not supporting the 'cat-blob'\n+\tcommand will exit with a message indicating so.\n+\n+notes::\n+\tRequire that the backend support the 'notemodify' (N)\n+\tsubcommand to the 'commit' command.\n \n `option`\n ~~~~~~~~\ndiff --git a/fast-import.c b/fast-import.c\nindex 60f26fe..bf2f9f8 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2983,14 +2983,16 @@ static int parse_one_feature(const char *feature, int from_stream)\n \t\toption_import_marks(feature + 13, from_stream);\n \t} else if (!prefixcmp(feature, \"export-marks=\")) {\n \t\toption_export_marks(feature + 13);\n-\t} else if (!strcmp(feature, \"cat-blob\")) {\n-\t\t; /* Don't die - this feature is supported */\n \t} else if (!prefixcmp(feature, \"relative-marks\")) {\n \t\trelative_marks_paths = 1;\n \t} else if (!prefixcmp(feature, \"no-relative-marks\")) {\n \t\trelative_marks_paths = 0;\n \t} else if (!prefixcmp(feature, \"force\")) {\n \t\tforce_update = 1;\n+\t}\n+\t/* These features are present; don't error out for them. */\n+\telse if (!strcmp(feature, \"cat-blob\")) {\n+\t} else if (!strcmp(feature, \"notes\")) {\n \t} else {\n \t\treturn 0;\n \t}\ndiff --git a/t/t9301-fast-import-notes.sh b/t/t9301-fast-import-notes.sh\nindex 7cf8cd8..463254c 100755\n--- a/t/t9301-fast-import-notes.sh\n+++ b/t/t9301-fast-import-notes.sh\n@@ -120,6 +120,7 @@ test_expect_success 'add notes with simple M command' '\n \n test_tick\n cat >input < $GIT_COMMITTER_DATE\n data < \n> Support for importing notes was added to git fast-import quite a while\n> ago (v1.6.6-rc0~21^2~8, 2009-10-09), before the 'feature' facility was\n> introduced (v1.7.0-rc0~95^2~4, fast-import: add feature command,\n> 2009-12-04) so for compatibility with older git versions, authors\n> of existing frontends should not start using the \"feature notes\"\n> command. Most git versions in wide use support notemodify already.\n> \n> The purpose of the \"feature notes\" declaration is instead to\n> distinguish between git and fast-import backends that do not support\n> notemodify. In git \"feature notes\" will be a no-op while in other\n> current fast-import backends it will error out with a clear error\n> message.\n\nSo in summary, don't use \"feature notes\" because it would fail with\nold gits, but do use \"feature notes\" because it will fail for non-git?\n\nIsn't that a bit backwards? I mean, a tool author would either just\nuse it and say \"if it doesn't read this, upgrade your git\" or run\n\n echo feature notes | scm fast-import\n\nto test, and use notes depending on success. In both cases old gits\nwill be regarded as incompatible. Or am I missing something?\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"160288","messageId":"AANLkTi=bEi6J3wcex8JCowU6PRt3BKix5v67v8Ds-MN2@mail.gmail.com","threadId":"26385","inReplyTo":"201102022047.55152.trast@student.ethz.ch","subject":"Re: [PATCH 2/2] fast-import: introduce \"feature notes\" command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-02-02T19:57:02Z","receivedAt":"2011-02-02T19:57:02Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Feb 2, 2011 at 20:47, Thomas Rast wrote:\n> to test, and use notes depending on success.  In both cases old gits\n> will be regarded as incompatible.  Or am I missing something?\n\nI agree, old gits breaking on \"feature notes\", is/should be intended\nbehavior. Perhaps we can submit a patch to maint to have it (the\noldest git that supports the 'feature' command) recognize 'feature\nnotes' though?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"160293","messageId":"20110202202259.GA21138@elie","threadId":"26385","inReplyTo":"AANLkTi=bEi6J3wcex8JCowU6PRt3BKix5v67v8Ds-MN2@mail.gmail.com","subject":"Re: [PATCH 2/2] fast-import: introduce \"feature notes\" command","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-02T20:22:59Z","receivedAt":"2011-02-02T20:22:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\n> Perhaps we can submit a patch to maint to have it (the\n> oldest git that supports the 'feature' command) recognize 'feature\n> notes' though?\n\nSmart idea. My concerns would evaporate.\n\n[v1.7.0-rc0~95^2~4 (2009-12-04) is the relevant git.]\n"},{"id":"160786","messageId":"20110209214638.GA16716@elie","threadId":"26385","inReplyTo":"AANLkTi=bEi6J3wcex8JCowU6PRt3BKix5v67v8Ds-MN2@mail.gmail.com","subject":"[PATCH maint-1.7.0 v2 0/2] fast-import: introduce \"feature notes\" command","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-09T21:46:38Z","receivedAt":"2011-02-09T21:46:38Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Sverre Rabbelier wrote:\n\n> Perhaps we can submit a patch to maint to have it (the\n> oldest git that supports the 'feature' command) recognize 'feature\n> notes' though?\n\nThanks, both. Here's a series based against v1.7.0.9 to do that\n(since 1.7.0.y is the oldest series with support for 'feature'.)\n\nFor ease of patching, patch 1 backports a related documentation tweak\nfrom v1.7.4-rc0. Patch 2 introduces the \"feature notes\" command\nitself.\n\nJonathan Nieder (2):\n fast-import: clarify documentation of \"feature\" command\n fast-import: introduce \"feature notes\" command\n\n Documentation/git-fast-import.txt | 37 ++++++++++++++++++++-----------------\n fast-import.c | 2 ++\n t/t9301-fast-import-notes.sh | 1 +\n 3 files changed, 23 insertions(+), 17 deletions(-)\n"},{"id":"160792","messageId":"20110209224302.GA17128@elie","threadId":"26385","inReplyTo":"20110209214638.GA16716@elie","subject":"[PATCH 1/2] fast-import: clarify documentation of \"feature\" command","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-09T22:43:02Z","receivedAt":"2011-02-09T22:43:02Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Date: Sun, 28 Nov 2010 13:43:57 -0600\n\nThe \"feature\" command allows streams to specify options for the import\nthat must not be ignored. Logically, they are part of the stream,\neven though technically most supported features are synonyms to\ncommand-line options.\n\nMake this more obvious by being more explicit about how the analogy\nbetween most \"feature\" commands and command-line options works. Treat\nthe feature (import-marks) that does not fit this analogy separately.\n\nSigned-off-by: Jonathan Nieder \nAcked-by: Sverre Rabbelier \nSigned-off-by: Junio C Hamano \nSigned-off-by: Jonathan Nieder \n---\nJust a cherry-pick.\n\n Documentation/git-fast-import.txt | 33 +++++++++++++++------------------\n 1 files changed, 15 insertions(+), 18 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 19082b0..3bf04e3 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -878,28 +878,25 @@ Require that fast-import supports the specified feature, or abort if\n it does not.\n \n ....\n-\t'feature' SP LF\n+\t'feature' SP ('=' )? LF\n ....\n \n-The part of the command may be any string matching\n-^[a-zA-Z][a-zA-Z-]*$ and should be understood by fast-import.\n+The part of the command may be any one of the following:\n \n-Feature work identical as their option counterparts with the\n-exception of the import-marks feature, see below.\n+date-format::\n+export-marks::\n+relative-marks::\n+no-relative-marks::\n+force::\n+\tAct as though the corresponding command-line option with\n+\ta leading '--' was passed on the command line\n+\t(see OPTIONS, above).\n \n-The following features are currently supported:\n-\n-* date-format\n-* import-marks\n-* export-marks\n-* relative-marks\n-* no-relative-marks\n-* force\n-\n-The import-marks behaves differently from when it is specified as\n-commandline option in that only one \"feature import-marks\" is allowed\n-per stream. Also, any --import-marks= specified on the commandline\n-will override those from the stream (if any).\n+import-marks::\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 \n `option`\n ~~~~~~~~\n-- \n1.7.4\n"},{"id":"160793","messageId":"20110209224357.GB17128@elie","threadId":"26385","inReplyTo":"20110209214638.GA16716@elie","subject":"[PATCH 2/2] fast-import: introduce \"feature notes\" command","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-02-09T22:43:57Z","receivedAt":"2011-02-09T22:43:57Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Here is a 'feature' command for streams to use to require support for\nthe notemodify (N) command.\n\nWhen the 'feature' facility was introduced (v1.7.0-rc0~95^2~4,\n2009-12-04), the notes import feature was old news (v1.6.6-rc0~21^2~8,\n2009-10-09) and it was not obvious it deserved to be a named feature.\nBut now that is clear, since all major non-git fast-import backends\nlack support for it.\n\nDetails: on git version with this patch applied, any \"feature notes\"\ncommand in the features/options section at the beginning of a stream\nwill be treated as a no-op. On fast-import implementations without\nthe feature (and older git versions), the command instead errors out\nwith a message like\n\n\tThis version of fast-import does not support feature notes.\n\nSo by declaring use of notes at the beginning of a stream, frontends\ncan avoid wasting time and other resources when the backend does not\nsupport notes. (This would be especially important for backends that\ndo not support rewinding history after a botched import.)\n\nImproved-by: Thomas Rast \nImproved-by: Sverre Rabbelier \nSigned-off-by: Jonathan Nieder \n---\n Documentation/git-fast-import.txt | 6 ++++++\n fast-import.c | 2 ++\n t/t9301-fast-import-notes.sh | 1 +\n 3 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 3bf04e3..becee8b 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -898,6 +898,12 @@ import-marks::\n \tsecond, an --import-marks= command-line option overrides\n \tany \"feature import-marks\" command in the stream.\n \n+notes::\n+\tRequire that the backend support the 'notemodify' (N)\n+\tsubcommand to the 'commit' command.\n+\tVersions of fast-import not supporting notes will exit\n+\twith a message indicating so.\n+\n `option`\n ~~~~~~~~\n Processes the specified option so that git fast-import behaves in a\ndiff --git a/fast-import.c b/fast-import.c\nindex 74f08bd..ff56ea2 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2821,6 +2821,8 @@ static int parse_one_feature(const char *feature, int from_stream)\n \t\trelative_marks_paths = 0;\n \t} else if (!prefixcmp(feature, \"force\")) {\n \t\tforce_update = 1;\n+\t} else if (!strcmp(feature, \"notes\")) {\n+\t\t; /* do nothing; we have the feature */\n \t} else {\n \t\treturn 0;\n \t}\ndiff --git a/t/t9301-fast-import-notes.sh b/t/t9301-fast-import-notes.sh\nindex a5c99d8..164edf0 100755\n--- a/t/t9301-fast-import-notes.sh\n+++ b/t/t9301-fast-import-notes.sh\n@@ -120,6 +120,7 @@ test_expect_success 'add notes with simple M command' '\n \n test_tick\n cat >input < $GIT_COMMITTER_DATE\n data < wrote:\n> Improved-by: Thomas Rast \n> Improved-by: Sverre Rabbelier \n> Signed-off-by: Jonathan Nieder \n\nAcked as well. Thanks for backporting this!\n\n\n-- \nCheers,\n\nSverre Rabbelier\n"}]}