{"thread":{"id":"20569","subject":"[PATCH 0/4] fast-import: add a new option command","startedAt":"2009-08-13T05:09:42Z","lastAt":"2009-11-23T20:50:12Z","messageCount":30,"participants":["Sverre Rabbelier","Johannes Schindelin","Shawn O. Pearce","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"120484","messageId":"1250140186-12363-1-git-send-email-srabbelier@gmail.com","threadId":"20569","inReplyTo":null,"subject":"[PATCH 0/4] fast-import: add a new option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T05:09:42Z","receivedAt":"2009-08-13T05:09:42Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Sverre Rabbelier (4):\n      fast-import: put option parsing code in seperate functions\n      fast-import: define a new option command\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                     |  149 +++++++++++++++++++++++++++----------\n t/t9300-fast-import.sh            |   33 ++++++++\n 3 files changed, 165 insertions(+), 40 deletions(-)\n"},{"id":"120485","messageId":"1250140186-12363-2-git-send-email-srabbelier@gmail.com","threadId":"20569","inReplyTo":"1250140186-12363-1-git-send-email-srabbelier@gmail.com","subject":"[PATCH 1/4] fast-import: put option parsing code in seperate functions","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T05:09:43Z","receivedAt":"2009-08-13T05:09:43Z","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---\n\n    This is nearly identical to the RFC, but with parse_one_option\n    also factored out for easy reuse.\n\n fast-import.c |  132 +++++++++++++++++++++++++++++++++++++++-----------------\n 1 files changed, 92 insertions(+), 40 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 7ef9865..17d57ab 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,93 @@ 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+\tstruct strbuf buf = STRBUF_INIT;\n+\tstrbuf_addstr(&buf, marks);\n+\tmark_file = strbuf_detach(&buf, NULL);\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 option_force()\n+{\n+\tforce_update = 1;\n+}\n+\n+static void option_quiet()\n+{\n+\tshow_stats = 0;\n+}\n+\n+static void option_stats()\n+{\n+\tshow_stats = 1;\n+}\n+\n+static void parse_one_option(const char *option)\n+{\n+    if (!prefixcmp(option, \"date-format=\")) {\n+\t\toption_date_format(option + 12);\n+    } else if (!prefixcmp(option, \"max-pack-size=\")) {\n+\t\toption_max_pack_size(option + 14);\n+    } else if (!prefixcmp(option, \"depth=\")) {\n+\t\toption_depth(option + 6);\n+    } else if (!prefixcmp(option, \"active-branches=\")) {\n+\t\toption_active_branches(option + 16);\n+    } else if (!prefixcmp(option, \"import-marks=\")) {\n+\t\toption_import_marks(option + 13);\n+    } else if (!prefixcmp(option, \"export-marks=\")) {\n+\t\toption_export_marks(option + 13);\n+    } else if (!prefixcmp(option, \"export-pack-edges=\")) {\n+\t\toption_export_pack_edges(option + 18);\n+    } else if (!prefixcmp(option, \"force\")) {\n+\t\toption_force();\n+    } else if (!prefixcmp(option, \"quiet\")) {\n+\t\toption_quiet();\n+    } else if (!prefixcmp(option, \"stats\")) {\n+\t\toption_stats();\n+    } else {\n+\t\tdie(\"Unsupported option: %s\", option);\n+    }\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 +2486,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 +2507,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.16.g72c66.dirty\n"},{"id":"120486","messageId":"1250140186-12363-3-git-send-email-srabbelier@gmail.com","threadId":"20569","inReplyTo":"1250140186-12363-2-git-send-email-srabbelier@gmail.com","subject":"[PATCH 2/4] fast-import: define a new option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T05:09:44Z","receivedAt":"2009-08-13T05:09:44Z","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.\n---\n\n    As requested, updated the documentation of the language format\n\n Documentation/git-fast-import.txt |   23 +++++++++++++++++++++++\n 1 files changed, 23 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex c2f483a..6b5bc1b 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 frontends 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 front-ends needs.\n+Note that options specified by the frontend override any options the\n+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\n-- \n1.6.4.16.g72c66.dirty\n"},{"id":"120488","messageId":"1250140186-12363-4-git-send-email-srabbelier@gmail.com","threadId":"20569","inReplyTo":"1250140186-12363-3-git-send-email-srabbelier@gmail.com","subject":"[PATCH 3/4] fast-import: add option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T05:09:45Z","receivedAt":"2009-08-13T05:09:45Z","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---\n\n    This is a lot simpler from the previous version as we can now\n    reuse parse_one_option from 1/4.\n\n fast-import.c |   17 +++++++++++++++++\n 1 files changed, 17 insertions(+), 0 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 17d57ab..2ec804d 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -348,6 +348,7 @@ 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 write_branch_report(FILE *rpt, struct branch *b)\n {\n@@ -1663,6 +1664,10 @@ 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\tseen_non_option_command = 1;\n+\n \t\t\trc = rc_free;\n \t\t\tif (rc)\n \t\t\t\trc_free = rc->next;\n@@ -2460,6 +2465,16 @@ static void parse_one_option(const char *option)\n     }\n }\n \n+static void parse_option(void)\n+{\n+    char* 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+    parse_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@@ -2534,6 +2549,8 @@ 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-- \n1.6.4.16.g72c66.dirty\n"},{"id":"120487","messageId":"1250140186-12363-5-git-send-email-srabbelier@gmail.com","threadId":"20569","inReplyTo":"1250140186-12363-4-git-send-email-srabbelier@gmail.com","subject":"[PATCH 4/4] fast-import: test the new option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T05:09:46Z","receivedAt":"2009-08-13T05:09:46Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"---\n\n    Only difference with the previous version is\n    s/export-marks /export-marks=/\n\n t/t9300-fast-import.sh |   33 +++++++++++++++++++++++++++++++++\n 1 files changed, 33 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 821be7c..4152c3a 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -1088,4 +1088,37 @@ 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 2> output &&\n+    grep :1 git.marks'\n+\n test_done\n-- \n1.6.4.16.g72c66.dirty\n"},{"id":"120500","messageId":"alpine.DEB.1.00.0908131208400.7429@intel-tinevez-2-302","threadId":"20569","inReplyTo":"1250140186-12363-2-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 1/4] fast-import: put option parsing code in seperate functions","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-13T10:19:28Z","receivedAt":"2009-08-13T10:19:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 12 Aug 2009, Sverre Rabbelier wrote:\n\n> +static void option_export_marks(const char *marks)\n> +{\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tstrbuf_addstr(&buf, marks);\n> +\tmark_file = strbuf_detach(&buf, NULL);\n> +}\n\nHeh, this is a pretty convoluted way to write\n\n\tmark_file = xstrdup(marks);\n\n;-)\n\n> +static void option_force()\n> +{\n> +\tforce_update = 1;\n> +}\n\nI'm not sure that I would put these simple assignments in separate \nfunctions, but that's certainly up to you!\n\nThanks,\nDscho\n"},{"id":"120529","messageId":"20090813144007.GJ1033@spearce.org","threadId":"20569","inReplyTo":"alpine.DEB.1.00.0908131208400.7429@intel-tinevez-2-302","subject":"Re: [PATCH 1/4] fast-import: put option parsing code in seperate functions","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-08-13T14:40:07Z","receivedAt":"2009-08-13T14:40:07Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> On Wed, 12 Aug 2009, Sverre Rabbelier wrote:\n> \n> > +static void option_export_marks(const char *marks)\n> > +{\n> > +\tstruct strbuf buf = STRBUF_INIT;\n> > +\tstrbuf_addstr(&buf, marks);\n> > +\tmark_file = strbuf_detach(&buf, NULL);\n> > +}\n> \n> Heh, this is a pretty convoluted way to write\n> \n> \tmark_file = xstrdup(marks);\n> \n> ;-)\n\nAgreed.\n\n> > +static void option_force()\n> > +{\n> > +\tforce_update = 1;\n> > +}\n> \n> I'm not sure that I would put these simple assignments in separate \n> functions, but that's certainly up to you!\n\nOh, good point.  Yes, please don't do that, please just write these\ndirectly into the option parser.\n\nAside from these remarks, this patch looks good to me.\n\n-- \nShawn.\n"},{"id":"120530","messageId":"20090813144327.GK1033@spearce.org","threadId":"20569","inReplyTo":"1250140186-12363-3-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-08-13T14:43:27Z","receivedAt":"2009-08-13T14:43:27Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> This allows the frontend to specify any of the supported options as\n> long as no non-option command has been given.\n> ---\n> \n>     As requested, updated the documentation of the language format\n> \n>  Documentation/git-fast-import.txt |   23 +++++++++++++++++++++++\n>  1 files changed, 23 insertions(+), 0 deletions(-)\n\nWhy isn't this new 'option' command actually implemented by this\npatch?  Please introduce docs and code in the same patch if you can,\nespecially when it is this simple.\n \n> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\n> index c2f483a..6b5bc1b 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 frontends needs. This command\n> +    is optional and is not needed to perform an import.\n\ns/frontends/frontend's/\n\n> +`option`\n> +~~~~~~~~\n> +Processes the specified option so that git fast-import behaves in a\n> +way that suits the front-ends needs.\n\ns/front-ends/frontend's/\n\n> +Note that options specified by the frontend override any options the\n> +user may specify to git fast-import itself.\n\nHmmph.  Do we really want that?  I would think the command line\noptions should override the stream, such that we can then do\nsomething like:\n\n  hg fast-export >foo\n  git fast-import --export-marks=mymarks <foo\n\neven though 'option export-marks=bar' appears in foo.\n\n-- \nShawn.\n"},{"id":"120531","messageId":"20090813144539.GL1033@spearce.org","threadId":"20569","inReplyTo":"1250140186-12363-4-git-send-email-srabbelier@gmail.com","subject":"Re: [PATCH 3/4] fast-import: add option command","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-08-13T14:45:39Z","receivedAt":"2009-08-13T14:45:39Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> This allows the frontend to specify any of the supported options as\n> long as no non-option command has been given. This way the\n> user does not have to include any frontend-specific options, but\n> instead she can rely on the frontend to tell fast-import what it\n> needs.\n> ---\n\nMissing Signed-off-by.\n \n> @@ -2460,6 +2465,16 @@ static void parse_one_option(const char *option)\n>      }\n>  }\n>  \n> +static void parse_option(void)\n> +{\n> +    char* option = command_buf.buf + 7;\n\nGit style is \"char *option\", isn't it?\n\n> +\n> +\tif (seen_non_option_command)\n> +\t\tdie(\"Got option command '%s' after non-option command\", option);\n\nIndentation is messed up here.  1 tab per level, please.\n\n-- \nShawn.\n"},{"id":"120532","messageId":"alpine.DEB.1.00.0908131652190.7429@intel-tinevez-2-302","threadId":"20569","inReplyTo":"20090813144327.GK1033@spearce.org","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-13T14:56:21Z","receivedAt":"2009-08-13T14:56:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 13 Aug 2009, Shawn O. Pearce wrote:\n\n> Sverre Rabbelier <srabbelier@gmail.com> wrote:\n>\n> > +Note that options specified by the frontend override any options the\n> > +user may specify to git fast-import itself.\n> \n> Hmmph.  Do we really want that?  I would think the command line options \n> should override the stream, such that we can then do something like:\n> \n>   hg fast-export >foo\n>   git fast-import --export-marks=mymarks <foo\n> \n> even though 'option export-marks=bar' appears in foo.\n\nI guess the reason is that this is harder to implement.  The problem is \nthat you _have_ to parse the command line options first.  So you need to \nrecord with every option you set that it has been set by the command line, \nand must not be overridden by the in-stream options.\n\nOTOH,\n\n\thg fast-export |\n\tsed '1,/^blob/s/^option export-marks=.*$//' > foo\n\nis relatively easy.\n\nCiao,\nDscho\n"},{"id":"120533","messageId":"20090813150446.GM1033@spearce.org","threadId":"20569","inReplyTo":"alpine.DEB.1.00.0908131652190.7429@intel-tinevez-2-302","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-08-13T15:04:46Z","receivedAt":"2009-08-13T15:04:46Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> On Thu, 13 Aug 2009, Shawn O. Pearce wrote:\n> > Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> >\n> > > +Note that options specified by the frontend override any options the\n> > > +user may specify to git fast-import itself.\n> > \n> > Hmmph.  Do we really want that?  I would think the command line options \n> > should override the stream, such that we can then do something like:\n> > \n> >   hg fast-export >foo\n> >   git fast-import --export-marks=mymarks <foo\n> > \n> > even though 'option export-marks=bar' appears in foo.\n> \n> I guess the reason is that this is harder to implement.  The problem is \n> that you _have_ to parse the command line options first.  So you need to \n> record with every option you set that it has been set by the command line, \n> and must not be overridden by the in-stream options.\n\nIt might be easy enough to just save the argv into a global, and\nreparse the argv immediately before the first non-option command.\n\n> OTOH,\n> \n> \thg fast-export |\n> \tsed '1,/^blob/s/^option export-marks=.*$//' > foo\n> \n> is relatively easy.\n\nJust for the archives, but that sed expression won't catch inline\ndata under a file command.  Its legal for a stream to not use the\nblob command at all.  I guess it would need to be:\n\n \thg fast-export |\n \tsed '1,/^(blob|commit)/s/^option export-marks=.*$//' > foo\n\n-- \nShawn.\n"},{"id":"120534","messageId":"20090813152419.GN1033@spearce.org","threadId":"20569","inReplyTo":"fabb9a1e0908130812s297ccfc6vd6b746daf1dcc69a@mail.gmail.com","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-08-13T15:24:19Z","receivedAt":"2009-08-13T15:24:19Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> Actually I was thinking that the frontend should allow the user to override\n> these options. Assuming it is, say, specifying a marks file, if the user\n> wishes to change that, the frontend would probably need to know about the\n> new location too.\n> \n> So not only is it easier to implement, it makes sense from an ui\n> perspective. No?\n\nMy point is, most of the options are about paths to local files.\n\nIf I export something from hg on machine A, then copy the\nfast-import stream to machine B, the marks file on B might not be\nin the same path.  I might not even control machine A, maybe you\nemailed me that stream file.  I may not be able to use an option\non the source tool to alter the path.\n\nWhy shouldn't I be able to (easily) override the marks location\nlocally, without resorting to stream editing?\n\nSame with --force.  Which probably now demands a counter option,\n--no-force.  If you send me a stream with \"option force\\n\" in the\nheader, why can't I try to import it with \"--no-force\" first?\n\n-- \nShawn.\n"},{"id":"120537","messageId":"fabb9a1e0908130926qdc6cdf1ka7f2442421ce12ce@mail.gmail.com","threadId":"20569","inReplyTo":"20090813152419.GN1033@spearce.org","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T16:26:41Z","receivedAt":"2009-08-13T16:26:41Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Aug 13, 2009 at 08:24, Shawn O. Pearce<spearce@spearce.org> wrote:\n> Why shouldn't I be able to (easily) override the marks location\n> locally, without resorting to stream editing?\n\nYou convinced me, I'll store argv and argc in global_argv/global_argc\nand move the option parsing to parse_argv(), which is then called when\nthe first non_option command is parsed.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"120546","messageId":"alpine.DEB.1.00.0908131907080.7429@intel-tinevez-2-302","threadId":"20569","inReplyTo":"fabb9a1e0908130926qdc6cdf1ka7f2442421ce12ce@mail.gmail.com","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-13T17:07:24Z","receivedAt":"2009-08-13T17:07:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 13 Aug 2009, Sverre Rabbelier wrote:\n\n> On Thu, Aug 13, 2009 at 08:24, Shawn O. Pearce<spearce@spearce.org> \n> wrote:\n> > Why shouldn't I be able to (easily) override the marks location \n> > locally, without resorting to stream editing?\n> \n> You convinced me, I'll store argv and argc in global_argv/global_argc\n> and move the option parsing to parse_argv(), which is then called when\n> the first non_option command is parsed.\n\n... and will import the marks twice?\n\nCiao,\nDscho\n"},{"id":"120547","messageId":"fabb9a1e0908131009j51c54cacp3f837f9b8525061@mail.gmail.com","threadId":"20569","inReplyTo":"alpine.DEB.1.00.0908131907080.7429@intel-tinevez-2-302","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T17:09:41Z","receivedAt":"2009-08-13T17:09:41Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Aug 13, 2009 at 10:07, Johannes\nSchindelin<Johannes.Schindelin@gmx.de> wrote:\n> ... and will import the marks twice?\n\nAh, you're right :(. What's the best way to do this? Should we dump\nany previous marks when importing new ones?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"120550","messageId":"20090813172508.GO1033@spearce.org","threadId":"20569","inReplyTo":"fabb9a1e0908131009j51c54cacp3f837f9b8525061@mail.gmail.com","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-08-13T17:25:08Z","receivedAt":"2009-08-13T17:25:08Z","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 Thu, Aug 13, 2009 at 10:07, Johannes\n> Schindelin<Johannes.Schindelin@gmx.de> wrote:\n> > ... and will import the marks twice?\n> \n> Ah, you're right :(. What's the best way to do this? Should we dump\n> any previous marks when importing new ones?\n\nUh, well, yes.  We shouldn't define :5 if it was in the file that\nappeared in the stream, but isn't in the file on the command line.\n\nWorse, what happens if we do this:\n\n  echo \"option import-marks=/not/found\" \\\n  | git fast-import --import-marks=my.marks\n\nI want this to work, even though /not/found does not exist, but\nmy.marks does.  So that does complicate things...\n\n-- \nShawn.\n"},{"id":"120551","messageId":"fabb9a1e0908131028t438509d2m180293ca95daad74@mail.gmail.com","threadId":"20569","inReplyTo":"20090813172508.GO1033@spearce.org","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T17:28:55Z","receivedAt":"2009-08-13T17:28:55Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Aug 13, 2009 at 10:25, Shawn O. Pearce<spearce@spearce.org> wrote:\n> I want this to work, even though /not/found does not exist, but\n> my.marks does.  So that does complicate things...\n\nShould we pass an option to parse_marks to make it ignore a\nnon-existing file, and set that option when parsing the stream\ncommands?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"120554","messageId":"20090813174119.GP1033@spearce.org","threadId":"20569","inReplyTo":"fabb9a1e0908131028t438509d2m180293ca95daad74@mail.gmail.com","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-08-13T17:41:19Z","receivedAt":"2009-08-13T17:41:19Z","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 Thu, Aug 13, 2009 at 10:25, Shawn O. Pearce<spearce@spearce.org> wrote:\n> > I want this to work, even though /not/found does not exist, but\n> > my.marks does. ?So that does complicate things...\n> \n> Should we pass an option to parse_marks to make it ignore a\n> non-existing file, and set that option when parsing the stream\n> commands?\n\nUh, no, if we have \"option import-marks=...\" and we can't find the\nfile \"...\" and we have no --import-marks command line flag that\nwould have overridden it, we need to abort with an error.\n\n-- \nShawn.\n"},{"id":"120555","messageId":"fabb9a1e0908131044g583f126dm6a3818b4b295eaf5@mail.gmail.com","threadId":"20569","inReplyTo":"20090813174119.GP1033@spearce.org","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T17:44:51Z","receivedAt":"2009-08-13T17:44:51Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Aug 13, 2009 at 10:41, Shawn O. Pearce<spearce@spearce.org> wrote:\n> Uh, no, if we have \"option import-marks=...\" and we can't find the\n> file \"...\" and we have no --import-marks command line flag that\n> would have overridden it, we need to abort with an error.\n\nAh, then how about in option_import_marks() we only store the name of\nthe file, like in option_export_marks, and at the end, when we reach\nthe first non-option command (and we've parsed argv), we read the\nfile. That way it's only read once, and it deals with the above\nscenario.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"120556","messageId":"20090813175211.GQ1033@spearce.org","threadId":"20569","inReplyTo":"fabb9a1e0908131044g583f126dm6a3818b4b295eaf5@mail.gmail.com","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-08-13T17:52:11Z","receivedAt":"2009-08-13T17:52:11Z","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 Thu, Aug 13, 2009 at 10:41, Shawn O. Pearce<spearce@spearce.org> wrote:\n> > Uh, no, if we have \"option import-marks=...\" and we can't find the\n> > file \"...\" and we have no --import-marks command line flag that\n> > would have overridden it, we need to abort with an error.\n> \n> Ah, then how about in option_import_marks() we only store the name of\n> the file, like in option_export_marks, and at the end, when we reach\n> the first non-option command (and we've parsed argv), we read the\n> file. That way it's only read once, and it deals with the above\n> scenario.\n\nThat's better.  :-)\n\n-- \nShawn.\n"},{"id":"120562","messageId":"7vzla3bjn5.fsf@alter.siamese.dyndns.org","threadId":"20569","inReplyTo":"20090813172508.GO1033@spearce.org","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-13T19:26:06Z","receivedAt":"2009-08-13T19:26:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" <spearce@spearce.org> writes:\n\n> Sverre Rabbelier <srabbelier@gmail.com> wrote:\n>> On Thu, Aug 13, 2009 at 10:07, Johannes\n>> Schindelin<Johannes.Schindelin@gmx.de> wrote:\n>> > ... and will import the marks twice?\n>> \n>> Ah, you're right :(. What's the best way to do this? Should we dump\n>> any previous marks when importing new ones?\n>\n> Uh, well, yes.  We shouldn't define :5 if it was in the file that\n> appeared in the stream, but isn't in the file on the command line.\n>\n> Worse, what happens if we do this:\n>\n>   echo \"option import-marks=/not/found\" \\\n>   | git fast-import --import-marks=my.marks\n>\n> I want this to work, even though /not/found does not exist, but\n> my.marks does.  So that does complicate things...\n\nHow about making the option parser get and keep the _name_ of the file\nuntil option parsing session (i.e. read the stream until initial run of\n\"option\" command runs out and then parse the command line to override),\nand then finally open the file and read it?\n"},{"id":"120570","messageId":"fabb9a1e0908131301g4361a06es98fbf3c256c25300@mail.gmail.com","threadId":"20569","inReplyTo":"7vzla3bjn5.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T20:01:16Z","receivedAt":"2009-08-13T20:01:16Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Aug 13, 2009 at 12:26, Junio C Hamano<gitster@pobox.com> wrote:\n> How about making the option parser get and keep the _name_ of the file\n> until option parsing session (i.e. read the stream until initial run of\n> \"option\" command runs out and then parse the command line to override),\n> and then finally open the file and read it?\n\nOn Thu, Aug 13, 2009 at 10:44, Sverre Rabbelier<srabbelier@gmail.com> wrote:\n> Ah, then how about in option_import_marks() we only store the name of\n> the file, like in option_export_marks, and at the end, when we reach\n> the first non-option command (and we've parsed argv), we read the\n> file. That way it's only read once, and it deals with the above\n> scenario.\n\nWhich is exactly what the latest version does :).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"120577","messageId":"7vhbwb5tul.fsf@alter.siamese.dyndns.org","threadId":"20569","inReplyTo":"fabb9a1e0908131301g4361a06es98fbf3c256c25300@mail.gmail.com","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-13T20:42:10Z","receivedAt":"2009-08-13T20:42:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sverre Rabbelier <srabbelier@gmail.com> writes:\n\n> Heya,\n>\n> On Thu, Aug 13, 2009 at 12:26, Junio C Hamano<gitster@pobox.com> wrote:\n>> How about making the option parser get and keep the _name_ of the file\n>> until option parsing session (i.e. read the stream until initial run of\n>> \"option\" command runs out and then parse the command line to override),\n>> and then finally open the file and read it?\n>\n> On Thu, Aug 13, 2009 at 10:44, Sverre Rabbelier<srabbelier@gmail.com> wrote:\n>> Ah, then how about in option_import_marks() we only store the name of\n>> the file, like in option_export_marks, and at the end, when we reach\n>> the first non-option command (and we've parsed argv), we read the\n>> file. That way it's only read once, and it deals with the above\n>> scenario.\n>\n> Which is exactly what the latest version does :).\n\nHeh, thanks; it appears I lagged behind by about 2 hours?\n"},{"id":"120584","messageId":"fabb9a1e0908131414n7f9394c7r13032a6bd346923f@mail.gmail.com","threadId":"20569","inReplyTo":"7vhbwb5tul.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T21:14:40Z","receivedAt":"2009-08-13T21:14:40Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Aug 13, 2009 at 13:42, Junio C Hamano<gitster@pobox.com> wrote:\n> Heh, thanks; it appears I lagged behind by about 2 hours?\n\nSo it would seem, not a bad latency at all though.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"120587","messageId":"alpine.DEB.1.00.0908132350190.8306@pacific.mpi-cbg.de","threadId":"20569","inReplyTo":"20090813175211.GQ1033@spearce.org","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-13T21:51:55Z","receivedAt":"2009-08-13T21:51:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 13 Aug 2009, Shawn O. Pearce wrote:\n\n> Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> > On Thu, Aug 13, 2009 at 10:41, Shawn O. Pearce<spearce@spearce.org> wrote:\n> > > Uh, no, if we have \"option import-marks=...\" and we can't find the\n> > > file \"...\" and we have no --import-marks command line flag that\n> > > would have overridden it, we need to abort with an error.\n> > \n> > Ah, then how about in option_import_marks() we only store the name of\n> > the file, like in option_export_marks, and at the end, when we reach\n> > the first non-option command (and we've parsed argv), we read the\n> > file. That way it's only read once, and it deals with the above\n> > scenario.\n> \n> That's better.  :-)\n\nSorry if I spoil the party, but maybe if things get so complicated, it may \nbe a sign that the original version (stream overrides command line, since \nit knows better) is to be preferred?  After all, if hg fast-export says \nthat the marks should be imported from a certain file, it may be for a \n_very good_ reason...\n\nCiao,\nDscho\n"},{"id":"120592","messageId":"fabb9a1e0908131501o73807f81mb230530c284ad123@mail.gmail.com","threadId":"20569","inReplyTo":"alpine.DEB.1.00.0908132350190.8306@pacific.mpi-cbg.de","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T22:01:02Z","receivedAt":"2009-08-13T22:01:02Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Aug 13, 2009 at 14:51, Johannes\nSchindelin<Johannes.Schindelin@gmx.de> wrote:\n> Sorry if I spoil the party, but maybe if things get so complicated, it may\n> be a sign that the original version (stream overrides command line, since\n> it knows better) is to be preferred?  After all, if hg fast-export says\n> that the marks should be imported from a certain file, it may be for a\n> _very good_ reason...\n\nYes, and that should Just Work (which it does). Also, I'm not sure how\noften one would output a stream on one computer, then move it to\nanother and import it there, but I'll methinks Shawn brought it up for\na reason ;). However, I do think it's better design to only store the\nname of the import file and then do the actual import later on (to\nprevent double imports).\n\nI don't have a preference either way (both patches are already written\nafter all). Shawn?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"120593","messageId":"20090813221213.GR1033@spearce.org","threadId":"20569","inReplyTo":"fabb9a1e0908131501o73807f81mb230530c284ad123@mail.gmail.com","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-08-13T22:12:13Z","receivedAt":"2009-08-13T22:12:13Z","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 Thu, Aug 13, 2009 at 14:51, Johannes\n> Schindelin<Johannes.Schindelin@gmx.de> wrote:\n> > Sorry if I spoil the party, but maybe if things get so complicated, it may\n> > be a sign that the original version (stream overrides command line, since\n> > it knows better) is to be preferred? ?After all, if hg fast-export says\n> > that the marks should be imported from a certain file, it may be for a\n> > _very good_ reason...\n> \n> Yes, and that should Just Work (which it does). Also, I'm not sure how\n> often one would output a stream on one computer, then move it to\n> another and import it there, but I'll methinks Shawn brought it up for\n> a reason ;). However, I do think it's better design to only store the\n> name of the import file and then do the actual import later on (to\n> prevent double imports).\n> \n> I don't have a preference either way (both patches are already written\n> after all). Shawn?\n\nNo, I don't have a really good reason for the command line overrides\nthe file other than this simple rule:\n\n  If the file is likely to be several hundred MiB, or bigger; thou\n  shall never try to open it with vi, *especially* vi on a Solaris\n  system, as at least one line is likely to be too long.\n\n  If the file header contains paths to other files, it is likely\n  one will want to modify that header sometime, because you moved\n  the file between systems.\n\n  Given the size of the file above, you can't just fix it with vi.\n\n  Lacking a tool that *can* do this edit safely (and Dscho's simple\n  sed wasn't enough, as I already pointed out, oh and Solaris sed\n  also fails on long lines), we *should* be able to override this on\n  the command line, *especially* since we already have the command\n  line option standardized!\n\nWhat is this, gang up on Shawn's words-of-wisdom week?  Both this\nthread and my intern this week have been argueing with me about\nwhat seem to me to be fairly trivial things.  Maybe I just need to\ntake vacation.  Good thing I have one coming in 5 weeks.\n\nI say use the version where we store the values (e.g. file names)\nduring option parsing, and then actually apply those saved values\njust before the first non-option command.  Which I think only has\nan impact on the import-marks option, the rest are all just simple\nvariable updates whose values aren't read until after the first\nnon-option command anyway.\n\n-- \nShawn.\n"},{"id":"120594","messageId":"fabb9a1e0908131517k334e5215mae565549768a1dde@mail.gmail.com","threadId":"20569","inReplyTo":"20090813221213.GR1033@spearce.org","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-13T22:17:59Z","receivedAt":"2009-08-13T22:17:59Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Thu, Aug 13, 2009 at 15:12, Shawn O. Pearce<spearce@spearce.org> wrote:\n> No, I don't have a really good reason for the command line overrides\n> the file other than this simple rule:\n\nNice anecdote :P.\n\n> What is this, gang up on Shawn's words-of-wisdom week?  Both this\n> thread and my intern this week have been argueing with me about\n> what seem to me to be fairly trivial things.  Maybe I just need to\n> take vacation.  Good thing I have one coming in 5 weeks.\n\nDidn't you get the memo, you could be crashing an airplane [0] like that ;).\n\n> I say use the version where we store the values (e.g. file names)\n> during option parsing, and then actually apply those saved values\n> just before the first non-option command.  Which I think only has\n> an impact on the import-marks option, the rest are all just simple\n> variable updates whose values aren't read until after the first\n> non-option command anyway.\n\nOk, that's v3, speaking of which, does it look good enough for inclusion?\n\n[0] http://www.thehackerchickblog.com/2009/05/plane-crashes-software-failures-and.html\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"128182","messageId":"fabb9a1e0911230939m56e20812o939456c41becf5fd@mail.gmail.com","threadId":"20569","inReplyTo":"20090813172508.GO1033@spearce.org","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-11-23T17:39:46Z","receivedAt":"2009-11-23T17:39:46Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\n[picking up a topic on what to do if there's a import-marks from the\nstream and the commandline]\n\nOn Thu, Aug 13, 2009 at 18:25, Shawn O. Pearce <spearce@spearce.org> wrote:\n> Sverre Rabbelier <srabbelier@gmail.com> wrote:\n>> On Thu, Aug 13, 2009 at 10:07, Johannes\n>> Schindelin<Johannes.Schindelin@gmx.de> wrote:\n>> > ... and will import the marks twice?\n>>\n>> Ah, you're right :(. What's the best way to do this? Should we dump\n>> any previous marks when importing new ones?\n>\n> Uh, well, yes.  We shouldn't define :5 if it was in the file that\n> appeared in the stream, but isn't in the file on the command line.\n>\n> Worse, what happens if we do this:\n>\n>  echo \"option import-marks=/not/found\" \\\n>  | git fast-import --import-marks=my.marks\n>\n> I want this to work, even though /not/found does not exist, but\n> my.marks does.  So that does complicate things...\n\nAccording to the docs it is allowed to specify import-marks multiple\ntimes (the latest series does not honor that, and there's apparently\nno test to verify it). What should the behavior be wrt\nstream/commandline arguments? Load from stream if present and valid,\nand also load from commandline? In that case, how do you override\nwhat's in the stream?\n\nI think the simplest is to allow the stream to specify a marks file\nexactly once, and commandline arguments override what's in the stream.\nListing import-marks on the commandline is still valid and keeps the\nold behavior.\n\nSensible?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"128203","messageId":"20091123205012.GG11919@spearce.org","threadId":"20569","inReplyTo":"fabb9a1e0911230939m56e20812o939456c41becf5fd@mail.gmail.com","subject":"Re: [PATCH 2/4] fast-import: define a new option command","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-11-23T20:50:12Z","receivedAt":"2009-11-23T20:50:12Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> I think the simplest is to allow the stream to specify a marks file\n> exactly once, and commandline arguments override what's in the stream.\n> Listing import-marks on the commandline is still valid and keeps the\n> old behavior.\n> \n> Sensible?\n\nYes.\n\n-- \nShawn.\n"}]}