* [PATCH/RFC 0/2] fast-import: introduce "feature notes" command @ 2011-02-02 4:58 Jonathan Nieder 2011-02-02 5:00 ` [PATCH 1/2] Documentation/fast-import: explain how to remove a note Jonathan Nieder 2011-02-02 5:07 ` [PATCH 2/2] fast-import: introduce "feature notes" command Jonathan Nieder 0 siblings, 2 replies; 10+ messages in thread From: Jonathan Nieder @ 2011-02-02 4:58 UTC (permalink / raw) To: git Cc: Johan Herland, Sverre Rabbelier, Ramkumar Ramachandra, Shawn O. Pearce, Sam Vilain Hi, As promised, here is a "feature" command for streams to use to require support for the notemodify (N) command. Patch 1 explains how to delete notes. On first reading, I had thought that feature was missing. Patch 2 is the title feature. The relevant message explains why this is an rfc. Thoughts welcome, as always. Jonathan Nieder (2): Documentation/fast-import: explain how to delete a note fast-import: introduce "feature notes" command Documentation/git-fast-import.txt | 18 ++++++++++-------- fast-import.c | 6 ++++-- t/t9301-fast-import-notes.sh | 1 + 3 files changed, 15 insertions(+), 10 deletions(-) ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] Documentation/fast-import: explain how to remove a note 2011-02-02 4:58 [PATCH/RFC 0/2] fast-import: introduce "feature notes" command Jonathan Nieder @ 2011-02-02 5:00 ` Jonathan Nieder 2011-02-02 5:07 ` [PATCH 2/2] fast-import: introduce "feature notes" command Jonathan Nieder 1 sibling, 0 replies; 10+ messages in thread From: Jonathan Nieder @ 2011-02-02 5:00 UTC (permalink / raw) To: git Cc: Johan Herland, Sverre Rabbelier, Ramkumar Ramachandra, Shawn O. Pearce, Sam Vilain A notemodify (N) command with blob id consisting of 40 zeroes (so is_null_sha1 is true) means to not add a note to replace the existing, removed one. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Documentation/git-fast-import.txt | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt index 4415e63..086b14f 100644 --- a/Documentation/git-fast-import.txt +++ b/Documentation/git-fast-import.txt @@ -651,9 +651,9 @@ External data format:: 'N' SP <dataref> SP <committish> LF .... + -Here `<dataref>` can be either a mark reference (`:<idnum>`) -set by a prior `blob` command, or a full 40-byte SHA-1 of an -existing Git blob object. +Here `<dataref>` can be a mark reference (`:<idnum>`) +set by a prior `blob` command, a full 40-byte SHA-1 of an +existing Git blob object, or 40 zeroes, to remove a note. Inline data format:: The data content for the note has not been supplied yet. -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] fast-import: introduce "feature notes" command 2011-02-02 4:58 [PATCH/RFC 0/2] fast-import: introduce "feature notes" command Jonathan Nieder 2011-02-02 5:00 ` [PATCH 1/2] Documentation/fast-import: explain how to remove a note Jonathan Nieder @ 2011-02-02 5:07 ` Jonathan Nieder 2011-02-02 19:47 ` Thomas Rast 1 sibling, 1 reply; 10+ messages in thread From: Jonathan Nieder @ 2011-02-02 5:07 UTC (permalink / raw) To: git Cc: Johan Herland, Sverre Rabbelier, Ramkumar Ramachandra, Shawn O. Pearce, Sam Vilain Here is a "feature" command for streams to use to require support for the notemodify (N) command. Support for importing notes was added to git fast-import quite a while ago (v1.6.6-rc0~21^2~8, 2009-10-09), before the 'feature' facility was introduced (v1.7.0-rc0~95^2~4, fast-import: add feature command, 2009-12-04) so for compatibility with older git versions, authors of existing frontends should not start using the "feature notes" command. Most git versions in wide use support notemodify already. The purpose of the "feature notes" declaration is instead to distinguish between git and fast-import backends that do not support notemodify. In git "feature notes" will be a no-op while in other current fast-import backends it will error out with a clear error message. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Still to do: the documentation does not hint at the point mentioned in paragraphs 2 and 3 above. Ideas for wording? Aside from that, I think this is ready. Thanks for reading. Documentation/git-fast-import.txt | 12 +++++++----- fast-import.c | 6 ++++-- t/t9301-fast-import-notes.sh | 1 + 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt index 086b14f..2393252 100644 --- a/Documentation/git-fast-import.txt +++ b/Documentation/git-fast-import.txt @@ -942,11 +942,13 @@ import-marks:: any "feature import-marks" command in the stream. cat-blob:: - Ignored. Versions of fast-import not supporting the - "cat-blob" command will exit with a message indicating so. - This lets the import error out early with a clear message, - rather than wasting time on the early part of an import - before the unsupported command is detected. + Require that the backend support the 'cat-blob' command. + Versions of fast-import not supporting the 'cat-blob' + command will exit with a message indicating so. + +notes:: + Require that the backend support the 'notemodify' (N) + subcommand to the 'commit' command. `option` ~~~~~~~~ diff --git a/fast-import.c b/fast-import.c index 60f26fe..bf2f9f8 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2983,14 +2983,16 @@ static int parse_one_feature(const char *feature, int from_stream) option_import_marks(feature + 13, from_stream); } else if (!prefixcmp(feature, "export-marks=")) { option_export_marks(feature + 13); - } else if (!strcmp(feature, "cat-blob")) { - ; /* Don't die - this feature is supported */ } else if (!prefixcmp(feature, "relative-marks")) { relative_marks_paths = 1; } else if (!prefixcmp(feature, "no-relative-marks")) { relative_marks_paths = 0; } else if (!prefixcmp(feature, "force")) { force_update = 1; + } + /* These features are present; don't error out for them. */ + else if (!strcmp(feature, "cat-blob")) { + } else if (!strcmp(feature, "notes")) { } else { return 0; } diff --git a/t/t9301-fast-import-notes.sh b/t/t9301-fast-import-notes.sh index 7cf8cd8..463254c 100755 --- a/t/t9301-fast-import-notes.sh +++ b/t/t9301-fast-import-notes.sh @@ -120,6 +120,7 @@ test_expect_success 'add notes with simple M command' ' test_tick cat >input <<INPUT_END +feature notes commit refs/notes/test committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE data <<COMMIT -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] fast-import: introduce "feature notes" command 2011-02-02 5:07 ` [PATCH 2/2] fast-import: introduce "feature notes" command Jonathan Nieder @ 2011-02-02 19:47 ` Thomas Rast 2011-02-02 19:57 ` Sverre Rabbelier 0 siblings, 1 reply; 10+ messages in thread From: Thomas Rast @ 2011-02-02 19:47 UTC (permalink / raw) To: Jonathan Nieder Cc: git, Johan Herland, Sverre Rabbelier, Ramkumar Ramachandra, Shawn O. Pearce, Sam Vilain Jonathan Nieder wrote: > > Support for importing notes was added to git fast-import quite a while > ago (v1.6.6-rc0~21^2~8, 2009-10-09), before the 'feature' facility was > introduced (v1.7.0-rc0~95^2~4, fast-import: add feature command, > 2009-12-04) so for compatibility with older git versions, authors > of existing frontends should not start using the "feature notes" > command. Most git versions in wide use support notemodify already. > > The purpose of the "feature notes" declaration is instead to > distinguish between git and fast-import backends that do not support > notemodify. In git "feature notes" will be a no-op while in other > current fast-import backends it will error out with a clear error > message. So in summary, don't use "feature notes" because it would fail with old gits, but do use "feature notes" because it will fail for non-git? Isn't that a bit backwards? I mean, a tool author would either just use it and say "if it doesn't read this, upgrade your git" or run echo feature notes | scm fast-import to test, and use notes depending on success. In both cases old gits will be regarded as incompatible. Or am I missing something? -- Thomas Rast trast@{inf,student}.ethz.ch ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] fast-import: introduce "feature notes" command 2011-02-02 19:47 ` Thomas Rast @ 2011-02-02 19:57 ` Sverre Rabbelier 2011-02-02 20:22 ` Jonathan Nieder 2011-02-09 21:46 ` [PATCH maint-1.7.0 v2 0/2] " Jonathan Nieder 0 siblings, 2 replies; 10+ messages in thread From: Sverre Rabbelier @ 2011-02-02 19:57 UTC (permalink / raw) To: Thomas Rast Cc: Jonathan Nieder, git, Johan Herland, Ramkumar Ramachandra, Shawn O. Pearce, Sam Vilain Heya, On Wed, Feb 2, 2011 at 20:47, Thomas Rast <trast@student.ethz.ch> wrote: > to test, and use notes depending on success. In both cases old gits > will be regarded as incompatible. Or am I missing something? I agree, old gits breaking on "feature notes", is/should be intended behavior. Perhaps we can submit a patch to maint to have it (the oldest git that supports the 'feature' command) recognize 'feature notes' though? -- Cheers, Sverre Rabbelier ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] fast-import: introduce "feature notes" command 2011-02-02 19:57 ` Sverre Rabbelier @ 2011-02-02 20:22 ` Jonathan Nieder 2011-02-09 21:46 ` [PATCH maint-1.7.0 v2 0/2] " Jonathan Nieder 1 sibling, 0 replies; 10+ messages in thread From: Jonathan Nieder @ 2011-02-02 20:22 UTC (permalink / raw) To: Sverre Rabbelier Cc: Thomas Rast, git, Johan Herland, Ramkumar Ramachandra, Shawn O. Pearce, Sam Vilain Sverre Rabbelier wrote: > Perhaps we can submit a patch to maint to have it (the > oldest git that supports the 'feature' command) recognize 'feature > notes' though? Smart idea. My concerns would evaporate. [v1.7.0-rc0~95^2~4 (2009-12-04) is the relevant git.] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH maint-1.7.0 v2 0/2] fast-import: introduce "feature notes" command 2011-02-02 19:57 ` Sverre Rabbelier 2011-02-02 20:22 ` Jonathan Nieder @ 2011-02-09 21:46 ` Jonathan Nieder 2011-02-09 22:43 ` [PATCH 1/2] fast-import: clarify documentation of "feature" command Jonathan Nieder 2011-02-09 22:43 ` [PATCH 2/2] fast-import: introduce "feature notes" command Jonathan Nieder 1 sibling, 2 replies; 10+ messages in thread From: Jonathan Nieder @ 2011-02-09 21:46 UTC (permalink / raw) To: Sverre Rabbelier Cc: Thomas Rast, git, Johan Herland, Ramkumar Ramachandra, Shawn O. Pearce, Sam Vilain, Junio C Hamano Sverre Rabbelier wrote: > Perhaps we can submit a patch to maint to have it (the > oldest git that supports the 'feature' command) recognize 'feature > notes' though? Thanks, both. Here's a series based against v1.7.0.9 to do that (since 1.7.0.y is the oldest series with support for 'feature'.) For ease of patching, patch 1 backports a related documentation tweak from v1.7.4-rc0. Patch 2 introduces the "feature notes" command itself. Jonathan Nieder (2): fast-import: clarify documentation of "feature" command fast-import: introduce "feature notes" command Documentation/git-fast-import.txt | 37 ++++++++++++++++++++----------------- fast-import.c | 2 ++ t/t9301-fast-import-notes.sh | 1 + 3 files changed, 23 insertions(+), 17 deletions(-) ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] fast-import: clarify documentation of "feature" command 2011-02-09 21:46 ` [PATCH maint-1.7.0 v2 0/2] " Jonathan Nieder @ 2011-02-09 22:43 ` Jonathan Nieder 2011-02-09 22:43 ` [PATCH 2/2] fast-import: introduce "feature notes" command Jonathan Nieder 1 sibling, 0 replies; 10+ messages in thread From: Jonathan Nieder @ 2011-02-09 22:43 UTC (permalink / raw) To: Sverre Rabbelier Cc: Thomas Rast, git, Johan Herland, Ramkumar Ramachandra, Shawn O. Pearce, Sam Vilain, Junio C Hamano Date: Sun, 28 Nov 2010 13:43:57 -0600 The "feature" command allows streams to specify options for the import that must not be ignored. Logically, they are part of the stream, even though technically most supported features are synonyms to command-line options. Make this more obvious by being more explicit about how the analogy between most "feature" commands and command-line options works. Treat the feature (import-marks) that does not fit this analogy separately. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Acked-by: Sverre Rabbelier <srabbelier@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Just a cherry-pick. Documentation/git-fast-import.txt | 33 +++++++++++++++------------------ 1 files changed, 15 insertions(+), 18 deletions(-) diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt index 19082b0..3bf04e3 100644 --- a/Documentation/git-fast-import.txt +++ b/Documentation/git-fast-import.txt @@ -878,28 +878,25 @@ Require that fast-import supports the specified feature, or abort if it does not. .... - 'feature' SP <feature> LF + 'feature' SP <feature> ('=' <argument>)? LF .... -The <feature> part of the command may be any string matching -^[a-zA-Z][a-zA-Z-]*$ and should be understood by fast-import. +The <feature> part of the command may be any one of the following: -Feature work identical as their option counterparts with the -exception of the import-marks feature, see below. +date-format:: +export-marks:: +relative-marks:: +no-relative-marks:: +force:: + Act as though the corresponding command-line option with + a leading '--' was passed on the command line + (see OPTIONS, above). -The following features are currently supported: - -* date-format -* import-marks -* export-marks -* relative-marks -* no-relative-marks -* force - -The import-marks behaves differently from when it is specified as -commandline option in that only one "feature import-marks" is allowed -per stream. Also, any --import-marks= specified on the commandline -will override those from the stream (if any). +import-marks:: + 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. `option` ~~~~~~~~ -- 1.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] fast-import: introduce "feature notes" command 2011-02-09 21:46 ` [PATCH maint-1.7.0 v2 0/2] " Jonathan Nieder 2011-02-09 22:43 ` [PATCH 1/2] fast-import: clarify documentation of "feature" command Jonathan Nieder @ 2011-02-09 22:43 ` Jonathan Nieder 2011-02-09 23:06 ` Sverre Rabbelier 1 sibling, 1 reply; 10+ messages in thread From: Jonathan Nieder @ 2011-02-09 22:43 UTC (permalink / raw) To: Sverre Rabbelier Cc: Thomas Rast, git, Johan Herland, Ramkumar Ramachandra, Shawn O. Pearce, Sam Vilain, Junio C Hamano Here is a 'feature' command for streams to use to require support for the notemodify (N) command. When the 'feature' facility was introduced (v1.7.0-rc0~95^2~4, 2009-12-04), the notes import feature was old news (v1.6.6-rc0~21^2~8, 2009-10-09) and it was not obvious it deserved to be a named feature. But now that is clear, since all major non-git fast-import backends lack support for it. Details: on git version with this patch applied, any "feature notes" command in the features/options section at the beginning of a stream will be treated as a no-op. On fast-import implementations without the feature (and older git versions), the command instead errors out with a message like This version of fast-import does not support feature notes. So by declaring use of notes at the beginning of a stream, frontends can avoid wasting time and other resources when the backend does not support notes. (This would be especially important for backends that do not support rewinding history after a botched import.) Improved-by: Thomas Rast <trast@student.ethz.ch> Improved-by: Sverre Rabbelier <srabbelier@gmail.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Documentation/git-fast-import.txt | 6 ++++++ fast-import.c | 2 ++ t/t9301-fast-import-notes.sh | 1 + 3 files changed, 9 insertions(+), 0 deletions(-) diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt index 3bf04e3..becee8b 100644 --- a/Documentation/git-fast-import.txt +++ b/Documentation/git-fast-import.txt @@ -898,6 +898,12 @@ import-marks:: second, an --import-marks= command-line option overrides any "feature import-marks" command in the stream. +notes:: + Require that the backend support the 'notemodify' (N) + subcommand to the 'commit' command. + Versions of fast-import not supporting notes will exit + with a message indicating so. + `option` ~~~~~~~~ Processes the specified option so that git fast-import behaves in a diff --git a/fast-import.c b/fast-import.c index 74f08bd..ff56ea2 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2821,6 +2821,8 @@ static int parse_one_feature(const char *feature, int from_stream) relative_marks_paths = 0; } else if (!prefixcmp(feature, "force")) { force_update = 1; + } else if (!strcmp(feature, "notes")) { + ; /* do nothing; we have the feature */ } else { return 0; } diff --git a/t/t9301-fast-import-notes.sh b/t/t9301-fast-import-notes.sh index a5c99d8..164edf0 100755 --- a/t/t9301-fast-import-notes.sh +++ b/t/t9301-fast-import-notes.sh @@ -120,6 +120,7 @@ test_expect_success 'add notes with simple M command' ' test_tick cat >input <<INPUT_END +feature notes commit refs/notes/test committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE data <<COMMIT -- 1.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] fast-import: introduce "feature notes" command 2011-02-09 22:43 ` [PATCH 2/2] fast-import: introduce "feature notes" command Jonathan Nieder @ 2011-02-09 23:06 ` Sverre Rabbelier 0 siblings, 0 replies; 10+ messages in thread From: Sverre Rabbelier @ 2011-02-09 23:06 UTC (permalink / raw) To: Jonathan Nieder Cc: Thomas Rast, git, Johan Herland, Ramkumar Ramachandra, Shawn O. Pearce, Sam Vilain, Junio C Hamano Heya, On Wed, Feb 9, 2011 at 23:43, Jonathan Nieder <jrnieder@gmail.com> wrote: > Improved-by: Thomas Rast <trast@student.ethz.ch> > Improved-by: Sverre Rabbelier <srabbelier@gmail.com> > Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Acked as well. Thanks for backporting this! -- Cheers, Sverre Rabbelier ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-02-09 23:07 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-02-02 4:58 [PATCH/RFC 0/2] fast-import: introduce "feature notes" command Jonathan Nieder 2011-02-02 5:00 ` [PATCH 1/2] Documentation/fast-import: explain how to remove a note Jonathan Nieder 2011-02-02 5:07 ` [PATCH 2/2] fast-import: introduce "feature notes" command Jonathan Nieder 2011-02-02 19:47 ` Thomas Rast 2011-02-02 19:57 ` Sverre Rabbelier 2011-02-02 20:22 ` Jonathan Nieder 2011-02-09 21:46 ` [PATCH maint-1.7.0 v2 0/2] " Jonathan Nieder 2011-02-09 22:43 ` [PATCH 1/2] fast-import: clarify documentation of "feature" command Jonathan Nieder 2011-02-09 22:43 ` [PATCH 2/2] fast-import: introduce "feature notes" command Jonathan Nieder 2011-02-09 23:06 ` Sverre Rabbelier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).