Re: [PATCH v3 2/3] fast-import: add option command

["Shawn O. Pearce" <spearce@spearce.org>]

Sverre Rabbelier <srabbelier@gmail.com> wrote:
> +`option`
> +~~~~~~~~
> +Processes the specified option so that git fast-import behaves in a
> +way that suits the frontend's needs.
> +Note that options specified by the frontend are overridden by any
> +options the user may specify to git fast-import itself.

Wha?  This disagrees with the code.

> +static void read_marks(void)
> +{
> +	char line[512];
> +	FILE *f = fopen(input_file, "r");
...
> +static void option_import_marks(const char *marks)
>  {
> -	char line[512];
> -	FILE *f = fopen(input_file, "r");

This is a nasty refactoring, I would prefer to see it done in its
own commit, "move option_import_marks so it can be called during
command processing".

> @@ -2517,9 +2556,16 @@ int main(int argc, const char **argv)
>  			parse_checkpoint();
>  		else if (!prefixcmp(command_buf.buf, "progress "))
>  			parse_progress();
> +		else if (!prefixcmp(command_buf.buf, "option "))
> +			parse_option();
>  		else
>  			die("Unsupported command: %s", command_buf.buf);
>  	}
> +
> +	// argv hasn't been parsed yet, do so
> +	if (!seen_non_option_command)
> +		parse_argv();

This is too late.  Options like --date-format or --max-pack-size or
--depth or --active-branches all influence the command processing
above.  Parsing these at the end means they have no affect on the
import, which is wrong.

Oh, and --active-branches or --max-pack-size or --depth are really
good examples of things that maybe you do want to override on the
command line.  They have impact only on memory usage of the running
import process, and the local disk usage.  Maybe the frontend has
given too many active branches for your physical memory, and you
want a lower threshold.

So yea, I really do think its a good idea for command line options
to override stream options, despite what Dscho may think.  :-)

-- 
Shawn.