Sverre Rabbelier <srabbelier@xxxxxxxxx> writes:
> Main difference with v5 is that the syntax is now 'option git ...'
> as per a discussion with the other fast-import devs. Other options,
> e.g. 'option hg' are ignored. Also fixed the docs to say that
> feature commands are allowed before git option commands.
Perhaps the other people have discussed and thought about this much deeper
than I have after seeing the above description, but what should the
semantics be when you see unknown options?
If "option git something-unknown" is given, it is clear that the tool that
generated the stream assumed that such an option exists in the importer;
it might appear prudent to abort the operation.
But what about "option hg something"?
It is an indication that the stream is meant to be used with the named
option if fed to Hg, but it does not say anything about what should happen
when used with other systems. If older versions of Hg that do not grok
the given option is expected to abort because they won't be able to change
the behaviour to obey the optional semantics demanded by the "option hg
something", what should the other VCS do?
Worrying about the above would be unnecessary, if you declare that it is
*entirely* optional to understand and obey "option", and ignoring them
does not result in a corrupt import at all. I think that is the intent
behind "option", as opposed to "feature", and is consistent with the fact
that the command line options can override the in-stream settings. In
other words, any in-stream instruction that changes the semantics of
stream to render it dangerous to be processed by older version of tools
would be expressed with "feature", not with "option".
If that is the sensible thing to do, then we obviously should ignore
"option hg anything", but at the same time we should ignore "option git
we-do-not-know-what-it-does".
But then, the call to die("Unsupported option: %s", option) at the end of
parse_one_option() is wrong, isn't it?
I think at least the function should be made conditional to die() if it
was called from parse_argv() but simply ignore unknown if it was called
from the input stream.
> diff --git a/fast-import.c b/fast-import.c
> index 9bf06a4..bad93dc 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -2456,11 +2468,31 @@ static void parse_feature(void)
>
> if (!prefixcmp(feature, "date-format=")) {
> option_date_format(feature + 12);
> + } else if (!strcmp("git-options", feature)) {
> + options_enabled = 1;
> } else {
> die("This version of fast-import does not support feature %s.", feature);
> }
> }
>
> +static void parse_option(void)
> +{
> + char* option = command_buf.buf + 11;
ERROR: "foo* bar" should be "foo *bar"
> +
> + if (!options_enabled)
> + die("Got option command '%s' before options feature'", option);
> +
> + if (seen_non_option_command)
> + die("Got option command '%s' after non-option command", option);
> +
> + parse_one_option(option);
> +}
> +
> +static void parse_nongit_option(void)
> +{
> + // do nothing
ERROR: do not use C99 // comments
> @@ -2539,9 +2581,18 @@ int main(int argc, const char **argv)
> parse_progress();
> else if (!prefixcmp(command_buf.buf, "feature "))
> parse_feature();
> + else if (!prefixcmp(command_buf.buf, "option git "))
> + parse_option();
> + else if (!prefixcmp(command_buf.buf, "option "))
> + parse_nongit_option();
> else
> die("Unsupported command: %s", command_buf.buf);
> }
> +
> + // argv hasn't been parsed yet, do so
ERROR: do not use C99 // comments
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html