{"thread":{"id":"20585","subject":"[PATCH v3 3/3] fast-import: test the new option command","startedAt":"2009-08-13T19:02:33Z","lastAt":"2009-08-14T17:43:21Z","messageCount":8,"participants":["Sverre Rabbelier","Shawn O. Pearce"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"120558","messageId":"1250190156-4752-1-git-send-email-srabbelier@gmail.com","threadId":"20585","inReplyTo":null,"subject":"[PATCH v3 0/3] fast-import: add a new option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T19:02:33Z","receivedAt":"2009-08-13T19:02:33Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Now we delay reading the marks file till option parsing is done.\n\nSverre Rabbelier (3):\n      fast-import: put option parsing code in seperate functions\n      fast-import: add option command\n      fast-import: test the new option command\n\n Documentation/git-fast-import.txt |   23 ++++\n fast-import.c                     |  235 +++++++++++++++++++++++++------------\n t/t9300-fast-import.sh            |   58 +++++++++\n 3 files changed, 239 insertions(+), 77 deletions(-)\n"},{"id":"120559","messageId":"1250190156-4752-2-git-send-email-srabbelier@gmail.com","threadId":"20585","inReplyTo":"1250190156-4752-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH v3 1/3] fast-import: put option parsing code in seperate functions","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T19:02:34Z","receivedAt":"2009-08-13T19:02:34Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Putting the options in their own functions increases readability of\nthe option parsing block and makes it easier to reuse the option\nparsing code later on.\n\nSigned-off-by: Sverre Rabbelier <srabbelier@gmail.com>\n---\n\n\tUnchanged from v2.\n\n fast-import.c |  115 +++++++++++++++++++++++++++++++++++++--------------------\n 1 files changed, 75 insertions(+), 40 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 7ef9865..b904f20 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -291,6 +291,7 @@ static unsigned long branch_count;\n static unsigned long branch_load_count;\n static int failure;\n static FILE *pack_edges;\n+static unsigned int show_stats = 1;\n \n /* Memory pools */\n static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);\n@@ -2337,7 +2338,7 @@ static void parse_progress(void)\n \tskip_optional_lf();\n }\n \n-static void import_marks(const char *input_file)\n+static void option_import_marks(const char *input_file)\n {\n \tchar line[512];\n \tFILE *f = fopen(input_file, \"r\");\n@@ -2372,6 +2373,76 @@ static void import_marks(const char *input_file)\n \tfclose(f);\n }\n \n+static void option_date_format(const char *fmt)\n+{\n+\tif (!strcmp(fmt, \"raw\"))\n+\t\twhenspec = WHENSPEC_RAW;\n+\telse if (!strcmp(fmt, \"rfc2822\"))\n+\t\twhenspec = WHENSPEC_RFC2822;\n+\telse if (!strcmp(fmt, \"now\"))\n+\t\twhenspec = WHENSPEC_NOW;\n+\telse\n+\t\tdie(\"unknown --date-format argument %s\", fmt);\n+}\n+\n+static void option_max_pack_size(const char *packsize)\n+{\n+\tmax_packsize = strtoumax(packsize, NULL, 0) * 1024 * 1024;\n+}\n+\n+static void option_depth(const char *depth)\n+{\n+\tmax_depth = strtoul(depth, NULL, 0);\n+\tif (max_depth > MAX_DEPTH)\n+\t\tdie(\"--depth cannot exceed %u\", MAX_DEPTH);\n+}\n+\n+static void option_active_branches(const char *branches)\n+{\n+\tmax_active_branches = strtoul(branches, NULL, 0);\n+}\n+\n+static void option_export_marks(const char *marks)\n+{\n+\tmark_file = xstrdup(marks);\n+}\n+\n+static void option_export_pack_edges(const char *edges)\n+{\n+\tif (pack_edges)\n+\t\tfclose(pack_edges);\n+\tpack_edges = fopen(edges, \"a\");\n+\tif (!pack_edges)\n+\t\tdie_errno(\"Cannot open '%s'\", edges);\n+}\n+\n+static void parse_one_option(const char *option)\n+{\n+\tif (!prefixcmp(option, \"date-format=\")) {\n+\t\toption_date_format(option + 12);\n+\t} else if (!prefixcmp(option, \"max-pack-size=\")) {\n+\t\toption_max_pack_size(option + 14);\n+\t} else if (!prefixcmp(option, \"depth=\")) {\n+\t\toption_depth(option + 6);\n+\t} else if (!prefixcmp(option, \"active-branches=\")) {\n+\t\toption_active_branches(option + 16);\n+\t} else if (!prefixcmp(option, \"import-marks=\")) {\n+\t\toption_import_marks(option + 13);\n+\t} else if (!prefixcmp(option, \"export-marks=\")) {\n+\t\toption_export_marks(option + 13);\n+\t} else if (!prefixcmp(option, \"export-pack-edges=\")) {\n+\t\toption_export_pack_edges(option + 18);\n+\t} else if (!prefixcmp(option, \"force\")) {\n+\t\tforce_update = 1;\n+\t} else if (!prefixcmp(option, \"quiet\")) {\n+\t\tshow_stats = 0;\n+\t} else if (!prefixcmp(option, \"stats\")) {\n+\t\tshow_stats = 1;\n+\t} else {\n+\t\tdie(\"Unsupported option: %s\", option);\n+\t}\n+}\n+\n static int git_pack_config(const char *k, const char *v, void *cb)\n {\n \tif (!strcmp(k, \"pack.depth\")) {\n@@ -2398,7 +2469,7 @@ static const char fast_import_usage[] =\n \n int main(int argc, const char **argv)\n {\n-\tunsigned int i, show_stats = 1;\n+\tunsigned int i;\n \n \tgit_extract_argv0_path(argv[0]);\n \n@@ -2419,44 +2490,8 @@ int main(int argc, const char **argv)\n \n \t\tif (*a != '-' || !strcmp(a, \"--\"))\n \t\t\tbreak;\n-\t\telse if (!prefixcmp(a, \"--date-format=\")) {\n-\t\t\tconst char *fmt = a + 14;\n-\t\t\tif (!strcmp(fmt, \"raw\"))\n-\t\t\t\twhenspec = WHENSPEC_RAW;\n-\t\t\telse if (!strcmp(fmt, \"rfc2822\"))\n-\t\t\t\twhenspec = WHENSPEC_RFC2822;\n-\t\t\telse if (!strcmp(fmt, \"now\"))\n-\t\t\t\twhenspec = WHENSPEC_NOW;\n-\t\t\telse\n-\t\t\t\tdie(\"unknown --date-format argument %s\", fmt);\n-\t\t}\n-\t\telse if (!prefixcmp(a, \"--max-pack-size=\"))\n-\t\t\tmax_packsize = strtoumax(a + 16, NULL, 0) * 1024 * 1024;\n-\t\telse if (!prefixcmp(a, \"--depth=\")) {\n-\t\t\tmax_depth = strtoul(a + 8, NULL, 0);\n-\t\t\tif (max_depth > MAX_DEPTH)\n-\t\t\t\tdie(\"--depth cannot exceed %u\", MAX_DEPTH);\n-\t\t}\n-\t\telse if (!prefixcmp(a, \"--active-branches=\"))\n-\t\t\tmax_active_branches = strtoul(a + 18, NULL, 0);\n-\t\telse if (!prefixcmp(a, \"--import-marks=\"))\n-\t\t\timport_marks(a + 15);\n-\t\telse if (!prefixcmp(a, \"--export-marks=\"))\n-\t\t\tmark_file = a + 15;\n-\t\telse if (!prefixcmp(a, \"--export-pack-edges=\")) {\n-\t\t\tif (pack_edges)\n-\t\t\t\tfclose(pack_edges);\n-\t\t\tpack_edges = fopen(a + 20, \"a\");\n-\t\t\tif (!pack_edges)\n-\t\t\t\tdie_errno(\"Cannot open '%s'\", a + 20);\n-\t\t} else if (!strcmp(a, \"--force\"))\n-\t\t\tforce_update = 1;\n-\t\telse if (!strcmp(a, \"--quiet\"))\n-\t\t\tshow_stats = 0;\n-\t\telse if (!strcmp(a, \"--stats\"))\n-\t\t\tshow_stats = 1;\n-\t\telse\n-\t\t\tdie(\"unknown option %s\", a);\n+\n+\t\tparse_one_option(a + 2);\n \t}\n \tif (i != argc)\n \t\tusage(fast_import_usage);\n-- \n1.6.4.122.g6ffd7\n"},{"id":"120560","messageId":"1250190156-4752-3-git-send-email-srabbelier@gmail.com","threadId":"20585","inReplyTo":"1250190156-4752-2-git-send-email-srabbelier@gmail.com","subject":"[PATCH v3 2/3] fast-import: add option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T19:02:35Z","receivedAt":"2009-08-13T19:02:35Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"This allows the frontend to specify any of the supported options as\nlong as no non-option command has been given. This way the\nuser does not have to include any frontend-specific options, but\ninstead she can rely on the frontend to tell fast-import what it\nneeds.\n\nAlso factor out parsing of argv and have it execute when we reach the\nfirst non-option command, or after all commands have been read and\nno non-option command has been encountered.\n\nLastly do not read the marks file till after all options have been\nparsed, instead of when receiving the option.\n\nSigned-off-by: Sverre Rabbelier <srabbelier@gmail.com>\n---\n\n\tNow we delay reading the marks file till option parsing is done.\n\n Documentation/git-fast-import.txt |   23 +++++++\n fast-import.c                     |  130 +++++++++++++++++++++++++------------\n 2 files changed, 111 insertions(+), 42 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex c2f483a..ed8bd0d 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -303,6 +303,11 @@ and control the current import process.  More detailed discussion\n \tstandard output.  This command is optional and is not needed\n \tto perform an import.\n \n+`option`::\n+    Specify any of the options listed under OPTIONS to change\n+    fast-import's behavior to suit the frontend's needs. This command\n+    is optional and is not needed to perform an import.\n+\n `commit`\n ~~~~~~~~\n Create or update a branch with a new commit, recording one logical\n@@ -813,6 +818,24 @@ Placing a `progress` command immediately after a `checkpoint` will\n inform the reader when the `checkpoint` has been completed and it\n can safely access the refs that fast-import updated.\n \n+`option`\n+~~~~~~~~\n+Processes the specified option so that git fast-import behaves in a\n+way that suits the frontend's needs.\n+Note that options specified by the frontend are overridden by any\n+options the user may specify to git fast-import itself.\n+\n+....\n+    'option' SP <option> LF\n+....\n+\n+The `<option>` part of the command may contain any of the options\n+listed in the OPTIONS section, without the leading '--' and is\n+treated in the same way.\n+\n+Option commands must be the first commands on the input, to give an\n+option command after any non-option command is an error.\n+\n Crash Reports\n -------------\n If fast-import is supplied invalid input it will terminate with a\ndiff --git a/fast-import.c b/fast-import.c\nindex b904f20..dff2937 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -292,6 +292,8 @@ static unsigned long branch_load_count;\n static int failure;\n static FILE *pack_edges;\n static unsigned int show_stats = 1;\n+static int global_argc;\n+static const char **global_argv;\n \n /* Memory pools */\n static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);\n@@ -315,6 +317,7 @@ static struct object_entry_pool *blocks;\n static struct object_entry *object_table[1 << 16];\n static struct mark_set *marks;\n static const char *mark_file;\n+static const char *input_file;\n \n /* Our last blob */\n static struct last_object last_blob = { STRBUF_INIT, 0, 0, 0 };\n@@ -348,6 +351,9 @@ static struct recent_command *rc_free;\n static unsigned int cmd_save = 100;\n static uintmax_t next_mark;\n static struct strbuf new_data = STRBUF_INIT;\n+static int seen_non_option_command;\n+\n+static void parse_argv(void);\n \n static void write_branch_report(FILE *rpt, struct branch *b)\n {\n@@ -1643,6 +1649,42 @@ static void dump_marks(void)\n \t}\n }\n \n+static void read_marks(void)\n+{\n+\tchar line[512];\n+\tFILE *f = fopen(input_file, \"r\");\n+\tif (!f)\n+\t\tdie_errno(\"cannot read '%s'\", input_file);\n+\twhile (fgets(line, sizeof(line), f)) {\n+\t\tuintmax_t mark;\n+\t\tchar *end;\n+\t\tunsigned char sha1[20];\n+\t\tstruct object_entry *e;\n+\n+\t\tend = strchr(line, '\\n');\n+\t\tif (line[0] != ':' || !end)\n+\t\t\tdie(\"corrupt mark line: %s\", line);\n+\t\t*end = 0;\n+\t\tmark = strtoumax(line + 1, &end, 10);\n+\t\tif (!mark || end == line + 1\n+\t\t\t|| *end != ' ' || get_sha1(end + 1, sha1))\n+\t\t\tdie(\"corrupt mark line: %s\", line);\n+\t\te = find_object(sha1);\n+\t\tif (!e) {\n+\t\t\tenum object_type type = sha1_object_info(sha1, NULL);\n+\t\t\tif (type < 0)\n+\t\t\t\tdie(\"object not found: %s\", sha1_to_hex(sha1));\n+\t\t\te = insert_object(sha1);\n+\t\t\te->type = type;\n+\t\t\te->pack_id = MAX_PACK_ID;\n+\t\t\te->offset = 1; /* just not zero! */\n+\t\t}\n+\t\tinsert_mark(mark, e);\n+\t}\n+\tfclose(f);\n+}\n+\n+\n static int read_next_command(void)\n {\n \tstatic int stdin_eof = 0;\n@@ -1663,6 +1705,11 @@ static int read_next_command(void)\n \t\t\tif (stdin_eof)\n \t\t\t\treturn EOF;\n \n+\t\t\tif (!seen_non_option_command\n+\t\t\t\t&& prefixcmp(command_buf.buf, \"option \")) {\n+\t\t\t\tparse_argv();\n+\t\t\t}\n+\n \t\t\trc = rc_free;\n \t\t\tif (rc)\n \t\t\t\trc_free = rc->next;\n@@ -2338,39 +2385,9 @@ static void parse_progress(void)\n \tskip_optional_lf();\n }\n \n-static void option_import_marks(const char *input_file)\n+static void option_import_marks(const char *marks)\n {\n-\tchar line[512];\n-\tFILE *f = fopen(input_file, \"r\");\n-\tif (!f)\n-\t\tdie_errno(\"cannot read '%s'\", input_file);\n-\twhile (fgets(line, sizeof(line), f)) {\n-\t\tuintmax_t mark;\n-\t\tchar *end;\n-\t\tunsigned char sha1[20];\n-\t\tstruct object_entry *e;\n-\n-\t\tend = strchr(line, '\\n');\n-\t\tif (line[0] != ':' || !end)\n-\t\t\tdie(\"corrupt mark line: %s\", line);\n-\t\t*end = 0;\n-\t\tmark = strtoumax(line + 1, &end, 10);\n-\t\tif (!mark || end == line + 1\n-\t\t\t|| *end != ' ' || get_sha1(end + 1, sha1))\n-\t\t\tdie(\"corrupt mark line: %s\", line);\n-\t\te = find_object(sha1);\n-\t\tif (!e) {\n-\t\t\tenum object_type type = sha1_object_info(sha1, NULL);\n-\t\t\tif (type < 0)\n-\t\t\t\tdie(\"object not found: %s\", sha1_to_hex(sha1));\n-\t\t\te = insert_object(sha1);\n-\t\t\te->type = type;\n-\t\t\te->pack_id = MAX_PACK_ID;\n-\t\t\te->offset = 1; /* just not zero! */\n-\t\t}\n-\t\tinsert_mark(mark, e);\n-\t}\n-\tfclose(f);\n+\tinput_file = xstrdup(marks);\n }\n \n static void option_date_format(const char *fmt)\n@@ -2443,6 +2460,16 @@ static void parse_one_option(const char *option)\n \t}\n }\n \n+static void parse_option(void)\n+{\n+\tchar* option = command_buf.buf + 7;\n+\n+\tif (seen_non_option_command)\n+\t\tdie(\"Got option command '%s' after non-option command\", option);\n+\n+\tparse_one_option(option);\n+}\n+\n static int git_pack_config(const char *k, const char *v, void *cb)\n {\n \tif (!strcmp(k, \"pack.depth\")) {\n@@ -2467,6 +2494,26 @@ static int git_pack_config(const char *k, const char *v, void *cb)\n static const char fast_import_usage[] =\n \"git fast-import [--date-format=f] [--max-pack-size=n] [--depth=n] [--active-branches=n] [--export-marks=marks.file]\";\n \n+static void parse_argv(void)\n+{\n+\tunsigned int i;\n+\n+\tfor (i = 1; i < global_argc; i++) {\n+\t\tconst char *a = global_argv[i];\n+\n+\t\tif (*a != '-' || !strcmp(a, \"--\"))\n+\t\t\tbreak;\n+\n+\t\tparse_one_option(a + 2);\n+\t}\n+\tif (i != global_argc)\n+\t\tusage(fast_import_usage);\n+\n+\tseen_non_option_command = 1;\n+\tif (input_file)\n+\t\tread_marks();\n+}\n+\n int main(int argc, const char **argv)\n {\n \tunsigned int i;\n@@ -2485,16 +2532,8 @@ int main(int argc, const char **argv)\n \tavail_tree_table = xcalloc(avail_tree_table_sz, sizeof(struct avail_tree_content*));\n \tmarks = pool_calloc(1, sizeof(struct mark_set));\n \n-\tfor (i = 1; i < argc; i++) {\n-\t\tconst char *a = argv[i];\n-\n-\t\tif (*a != '-' || !strcmp(a, \"--\"))\n-\t\t\tbreak;\n-\n-\t\tparse_one_option(a + 2);\n-\t}\n-\tif (i != argc)\n-\t\tusage(fast_import_usage);\n+\tglobal_argc = argc;\n+\tglobal_argv = argv;\n \n \trc_free = pool_alloc(cmd_save * sizeof(*rc_free));\n \tfor (i = 0; i < (cmd_save - 1); i++)\n@@ -2517,9 +2556,16 @@ int main(int argc, const char **argv)\n \t\t\tparse_checkpoint();\n \t\telse if (!prefixcmp(command_buf.buf, \"progress \"))\n \t\t\tparse_progress();\n+\t\telse if (!prefixcmp(command_buf.buf, \"option \"))\n+\t\t\tparse_option();\n \t\telse\n \t\t\tdie(\"Unsupported command: %s\", command_buf.buf);\n \t}\n+\n+\t// argv hasn't been parsed yet, do so\n+\tif (!seen_non_option_command)\n+\t\tparse_argv();\n+\n \tend_packfile();\n \n \tdump_branches();\n-- \n1.6.4.122.g6ffd7\n"},{"id":"120557","messageId":"1250190156-4752-4-git-send-email-srabbelier@gmail.com","threadId":"20585","inReplyTo":"1250190156-4752-3-git-send-email-srabbelier@gmail.com","subject":"[PATCH v3 3/3] fast-import: test the new option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T19:02:36Z","receivedAt":"2009-08-13T19:02:36Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Test three options (quiet and import/export-marks) and verify that the\ncommandline options override these.\n\nSigned-off-by: Sverre Rabbelier <srabbelier@gmail.com>\n---\n\n\tAdded some tests to verify that the marks file is handled\n\tproperly as suggested by Dscho and Shawn.\n\n t/t9300-fast-import.sh |   58 ++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 58 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 821be7c..62369e5 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -1088,4 +1088,62 @@ INPUT_END\n test_expect_success 'P: fail on blob mark in gitlink' '\n     test_must_fail git fast-import <input'\n \n+###\n+### series Q (options)\n+###\n+\n+cat >input << EOF\n+option quiet\n+blob\n+data 3\n+hi\n+\n+EOF\n+\n+touch empty\n+\n+test_expect_success 'Q: quiet option results in no stats being output' '\n+    cat input | git fast-import 2> output &&\n+    test_cmp empty output\n+'\n+\n+cat >input << EOF\n+option export-marks=git.marks\n+blob\n+mark :1\n+data 3\n+hi\n+\n+EOF\n+\n+test_expect_success \\\n+    'Q: export-marks option results in a marks file being created' \\\n+    'cat input | git fast-import &&\n+    grep :1 git.marks'\n+\n+test_expect_success \\\n+    'Q: export-marks options can be overriden by commandline options' \\\n+    'cat input | git fast-import --export-marks=other.marks &&\n+    grep :1 other.marks'\n+\n+cat >input << EOF\n+option import-marks=marks.out\n+option export-marks=marks.new\n+EOF\n+\n+test_expect_success \\\n+    'Q: import to output marks works without any content' \\\n+    'cat input | git fast-import &&\n+    test_cmp marks.out marks.new'\n+\n+cat >input <<EOF\n+option import-marks=nonexistant.marks\n+option export-marks=marks.new\n+EOF\n+\n+test_expect_success \\\n+    'Q: import marks uses the commandline marks file when the stream specifies one' \\\n+    'cat input | git fast-import --import-marks=marks.out &&\n+    test_cmp marks.out marks.new'\n+\n test_done\n-- \n1.6.4.122.g6ffd7\n"},{"id":"120639","messageId":"20090814153707.GT1033@spearce.org","threadId":"20585","inReplyTo":"1250190156-4752-3-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH v3 2/3] fast-import: add option command","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-08-14T15:37:07Z","receivedAt":"2009-08-14T15:37:07Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> +`option`\n> +~~~~~~~~\n> +Processes the specified option so that git fast-import behaves in a\n> +way that suits the frontend's needs.\n> +Note that options specified by the frontend are overridden by any\n> +options the user may specify to git fast-import itself.\n\nWha?  This disagrees with the code.\n\n> +static void read_marks(void)\n> +{\n> +\tchar line[512];\n> +\tFILE *f = fopen(input_file, \"r\");\n...\n> +static void option_import_marks(const char *marks)\n>  {\n> -\tchar line[512];\n> -\tFILE *f = fopen(input_file, \"r\");\n\nThis is a nasty refactoring, I would prefer to see it done in its\nown commit, \"move option_import_marks so it can be called during\ncommand processing\".\n\n> @@ -2517,9 +2556,16 @@ int main(int argc, const char **argv)\n>  \t\t\tparse_checkpoint();\n>  \t\telse if (!prefixcmp(command_buf.buf, \"progress \"))\n>  \t\t\tparse_progress();\n> +\t\telse if (!prefixcmp(command_buf.buf, \"option \"))\n> +\t\t\tparse_option();\n>  \t\telse\n>  \t\t\tdie(\"Unsupported command: %s\", command_buf.buf);\n>  \t}\n> +\n> +\t// argv hasn't been parsed yet, do so\n> +\tif (!seen_non_option_command)\n> +\t\tparse_argv();\n\nThis is too late.  Options like --date-format or --max-pack-size or\n--depth or --active-branches all influence the command processing\nabove.  Parsing these at the end means they have no affect on the\nimport, which is wrong.\n\nOh, and --active-branches or --max-pack-size or --depth are really\ngood examples of things that maybe you do want to override on the\ncommand line.  They have impact only on memory usage of the running\nimport process, and the local disk usage.  Maybe the frontend has\ngiven too many active branches for your physical memory, and you\nwant a lower threshold.\n\nSo yea, I really do think its a good idea for command line options\nto override stream options, despite what Dscho may think.  :-)\n\n-- \nShawn.\n"},{"id":"120644","messageId":"fabb9a1e0908140937h32a2eac7ka88f76aa417fd631@mail.gmail.com","threadId":"20585","inReplyTo":"20090814153707.GT1033@spearce.org","subject":"Re: [PATCH v3 2/3] fast-import: add option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-14T16:37:57Z","receivedAt":"2009-08-14T16:37:57Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Aug 14, 2009 at 08:37, Shawn O. Pearce<spearce@spearce.org> wrote:\n> Sverre Rabbelier <srabbelier@gmail.com> wrote:\n>> +`option`\n>> +~~~~~~~~\n>> +Processes the specified option so that git fast-import behaves in a\n>> +way that suits the frontend's needs.\n>> +Note that options specified by the frontend are overridden by any\n>> +options the user may specify to git fast-import itself.\n>\n> Wha?  This disagrees with the code.\n\nIt does? It's saying that the user my override what the frontend\nspecifies, which is what the current version does.\n\n> This is a nasty refactoring, I would prefer to see it done in its\n> own commit, \"move option_import_marks so it can be called during\n> command processing\".\n\nOk, will do.\n\n>> +     // argv hasn't been parsed yet, do so\n>> +     if (!seen_non_option_command)\n>> +             parse_argv();\n>\n> This is too late.\n\nNo it's not. Earlier in the patch, at the other\n'seen_non_option_command', we call parse_argv() as well (which happens\nwhen a non-option command is issued). This statement is here to deal\nwith options that affect an empty stream, such as 'git format-patch\n--import-marks=marks.old --export-marks=marks.new < /dev/null &&\ntest_cmp marks.old marks.new'.\n\n> So yea, I really do think its a good idea for command line options\n> to override stream options, despite what Dscho may think.  :-)\n\nWhich is what the current version does, yes?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"120645","messageId":"20090814173929.GU1033@spearce.org","threadId":"20585","inReplyTo":"fabb9a1e0908140937h32a2eac7ka88f76aa417fd631@mail.gmail.com","subject":"Re: [PATCH v3 2/3] fast-import: add option command","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-08-14T17:39:29Z","receivedAt":"2009-08-14T17:39:29Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> On Fri, Aug 14, 2009 at 08:37, Shawn O. Pearce<spearce@spearce.org> wrote:\n> > Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> >> +`option`\n> >> +~~~~~~~~\n> >> +Processes the specified option so that git fast-import behaves in a\n> >> +way that suits the frontend's needs.\n> >> +Note that options specified by the frontend are overridden by any\n> >> +options the user may specify to git fast-import itself.\n> >\n> > Wha? ?This disagrees with the code.\n> \n> It does? It's saying that the user my override what the frontend\n> specifies, which is what the current version does.\n\nSorry, lack of caffeine.  Rereading the docs makes sense.\n \n> >> + ? ? if (!seen_non_option_command)\n> >> + ? ? ? ? ? ? parse_argv();\n> >\n> > This is too late.\n> \n> No it's not. Earlier in the patch, at the other\n> 'seen_non_option_command', we call parse_argv() as well (which happens\n> when a non-option command is issued). This statement is here to deal\n> with options that affect an empty stream, such as 'git format-patch\n> --import-marks=marks.old --export-marks=marks.new < /dev/null &&\n> test_cmp marks.old marks.new'.\n\nOy, I missed that call.  I looked for it but gave up too soon I guess.\n \n> > So yea, I really do think its a good idea for command line options\n> > to override stream options, despite what Dscho may think. ?:-)\n> \n> Which is what the current version does, yes?\n\nYes.  :-)\n\n-- \nShawn.\n"},{"id":"120646","messageId":"fabb9a1e0908141043l35d2070bxdadae96a7dd8a232@mail.gmail.com","threadId":"20585","inReplyTo":"20090814173929.GU1033@spearce.org","subject":"Re: [PATCH v3 2/3] fast-import: add option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-14T17:43:21Z","receivedAt":"2009-08-14T17:43:21Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Fri, Aug 14, 2009 at 10:39, Shawn O. Pearce<spearce@spearce.org> wrote:\n> Sorry, lack of caffeine.  Rereading the docs makes sense.\n\nPerfectly acceptable, I totally though the 'break' in a 'switch'\nstatement would break out of the outer for loop the other day.\n\n>> > So yea, I really do think its a good idea for command line options\n>> > to override stream options, despite what Dscho may think. ?:-)\n>>\n>> Which is what the current version does, yes?\n>\n> Yes.  :-)\n\nOk, I think perhaps we can wait 'nother day to see if anyone notices\nanything (possibly after more caffeine) and then ask Junio to apply?\n\n-- \nCheers,\n\nSverre Rabbelier\n"}]}