{"thread":{"id":"26246","subject":"[PATCH] fast-import: Create import-marks file when necessary","startedAt":"2011-01-09T19:33:57Z","lastAt":"2011-01-15T06:31:46Z","messageCount":6,"participants":["Ramkumar Ramachandra","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"159252","messageId":"1294601637-18416-1-git-send-email-artagnon@gmail.com","threadId":"26246","inReplyTo":null,"subject":"[PATCH] fast-import: Create import-marks file when necessary","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-01-09T19:33:57Z","receivedAt":"2011-01-09T19:33:57Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"When both --import-marks and --export-marks are given, and specify the\nsame file, that file should be created if it doesn't exist. Without\nthis patch, the caller would have to check for the existence of a\nmarks file and vary its fast-import invocation accordingly.\n\nRequested-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n fast-import.c |   17 ++++++++++++++++-\n 1 files changed, 16 insertions(+), 1 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 5055504..cb7a9c9 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -3247,8 +3247,23 @@ static void parse_argv(void)\n \t\tusage(fast_import_usage);\n \n \tseen_data_command = 1;\n-\tif (import_marks_file)\n+\tif (import_marks_file) {\n+\t\tFILE *f = fopen(import_marks_file, \"r\");\n+\t\tif (!f) {\n+\t\t\tif (errno == ENOENT &&\n+\t\t\t\t!strcmp(import_marks_file, export_marks_file)) {\n+\t\t\t\tFILE *d = fopen(import_marks_file, \"w\");\n+\t\t\t\tif (!d)\n+\t\t\t\t\tdie_errno(\"cannot create '%s'\", import_marks_file);\n+\t\t\t\tfclose(d);\n+\t\t\t}\n+\t\t\telse\n+\t\t\t\tdie_errno(\"cannot read '%s'\", import_marks_file);\n+\t\t}\n+\t\telse\n+\t\t\tfclose(f);\n \t\tread_marks();\n+\t}\n }\n \n int main(int argc, const char **argv)\n-- \n1.7.4.rc1.8.g98938a.dirty\n"},{"id":"159257","messageId":"7vwrmd7kxe.fsf@alter.siamese.dyndns.org","threadId":"26246","inReplyTo":"1294601637-18416-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH] fast-import: Create import-marks file when necessary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-01-09T22:48:45Z","receivedAt":"2011-01-09T22:48:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> When both --import-marks and --export-marks are given, and specify the\n> same file, that file should be created if it doesn't exist. Without\n> this patch, the caller would have to check for the existence of a\n> marks file and vary its fast-import invocation accordingly.\n>\n> Requested-by: Jonathan Nieder <jrnieder@gmail.com>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n\nDo you mean that you are solving a problem for bootstrapping a script that\nsays \"read from this file if exists, do work, and then write to the same\nfile for later invocation\"?\n\nAssuming that is what you are solving, why do you create the file in your\npatch in the import codepath?  I would imagine a sane logic flow for \"read\nif exists, do work and then write to the same\" would be more like:\n\n\tif (import_marks) {\n\t\tFILE = *f = fopen(import_marks);\n                if (f)\n                \tread_marks();\n\t\telse if (errno == ENOENT && !strcmp(import_marks, export_marks))\n                \t; /* Bootstrapping with \"read if exists\" */\n\t\telse\n                \tbarf();\n\t}\n\nno?\n\nAlso I find it a bad taste to use \"The user gave the same file to import\nand export\" as a hint for \"read if exists\".\n\nThe first two lines of the proposed commit message is way suboptimal, if\nthe problem is \"Not allowing read-if-exists-but-do-not-barf-if-missing\nmakes it hard to bootstrap repeated invocations that use a single\nimport/export marks file as the state-keeping mechanism\".\n\nIt does not have to be created if it doesn't exist---the program only\nneeds not to barf.  The condition does not have to be \"if they are the\nsame file\".  The caller may want to script\n\n\tcommand --import-if-exists=state --export=new-state+ &&\n        mv new-state+ state\n\nto make sure that when the underlying command fails, the previous state is\nstill kept intact without getting overwritten.\n"},{"id":"159348","messageId":"1294766058-29739-1-git-send-email-artagnon@gmail.com","threadId":"26246","inReplyTo":"7vwrmd7kxe.fsf@alter.siamese.dyndns.org","subject":"[PATCH] fast-import: Don't barf if import-marks file is missing","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-01-11T17:14:18Z","receivedAt":"2011-01-11T17:14:18Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The --import-marks option used to barf when the specified marks file\ndoesn't exist. Change its meaning to \"read marks file if it exists\" so\nthat callers don't have to handle bootstraping as a special case.\n\nRequested-by: Jonathan Nieder <jrnieder@gmail.com>\nThanks-to: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Documentation/git-fast-import.txt |    2 +-\n fast-import.c                     |    6 +++++-\n 2 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex f56dfca..94fbe54 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -72,7 +72,7 @@ OPTIONS\n \n --import-marks=<file>::\n \tBefore processing any input, load the marks specified in\n-\t<file>.  The input file must exist, must be readable, and\n+\t<file>, if it exists.  The input file must be readable, and\n \tmust use the same format as produced by \\--export-marks.\n \tMultiple options may be supplied to import more than one\n \tset of marks.  If a mark is defined to different values,\ndiff --git a/fast-import.c b/fast-import.c\nindex 7857760..cbd5124 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1795,7 +1795,11 @@ static void read_marks(void)\n {\n \tchar line[512];\n \tFILE *f = fopen(import_marks_file, \"r\");\n-\tif (!f)\n+\tif (f)\n+\t\t;\n+\telse if (errno == ENOENT)\n+\t\treturn; /* Marks file does not exist */\n+\telse\n \t\tdie_errno(\"cannot read '%s'\", import_marks_file);\n \twhile (fgets(line, sizeof(line), f)) {\n \t\tuintmax_t mark;\n-- \n1.7.2.2.409.gdbb11.dirty\n"},{"id":"159353","messageId":"20110111175102.GB15133@burratino","threadId":"26246","inReplyTo":"1294766058-29739-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH] fast-import: Don't barf if import-marks file is missing","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-11T17:51:02Z","receivedAt":"2011-01-11T17:51:02Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> The --import-marks option used to barf when the specified marks file\n> doesn't exist.\n\nMicronit: change descriptions should contrast present behavior\n\n\tThe --import-marks options errors out when the marks file is\n\tmissing.\n\nwith user requirements\n\n\tBut if a frontend is just using a marks file to ensure its state\n\tpersists between runs, it would be simpler if missing marks\n\tfiles were tolerated.  In some cases the frontend cannot even\n\tcheck for the existence of the marks file itself because with\n\t--relative-marks, the location of the marks file is\n\tbackend-dependent.\n\nand the proposed solution\n\n\tChange the meaning of --import-marks to \"read marks file if it\n\texists so such frontends don't have to use a different command\n\tline when bootstrapping.\n\nI.e., the preferred style is to use present tense for the description\nof present behavior, so the patch can be read as a bug report.\n\n> --- a/Documentation/git-fast-import.txt\n> +++ b/Documentation/git-fast-import.txt\n> @@ -72,7 +72,7 @@ OPTIONS\n>  \n>  --import-marks=<file>::\n>  \tBefore processing any input, load the marks specified in\n> -\t<file>.  The input file must exist, must be readable, and\n> +\t<file>, if it exists.\n\nThis means removing a typo detection facility.\n\n\tgit fast-import --import-marks=something-like-a-real-marks-file.marks\n\nwould be accepted and result in an empty marks table instead of an\nerror message.  Is that considered worth it?  (My hunch is no, but\neither way, I think it ought to be mentioned in the change\ndescription.)\n\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -1795,7 +1795,11 @@ static void read_marks(void)\n\nNice implementation.  Maybe something like this on top?\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n Documentation/git-fast-import.txt |    6 +++-\n fast-import.c                     |   12 ++++++--\n t/t9300-fast-import.sh            |   55 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 69 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 94fbe54..32a062c 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -72,12 +72,16 @@ OPTIONS\n \n --import-marks=<file>::\n \tBefore processing any input, load the marks specified in\n-\t<file>, if it exists.  The input file must be readable, and\n+\t<file>.  The input file must exist, must be readable, and\n \tmust use the same format as produced by \\--export-marks.\n \tMultiple options may be supplied to import more than one\n \tset of marks.  If a mark is defined to different values,\n \tthe last file wins.\n \n+--import-marks-if-exists=<file>::\n+\tLike --import-marks but instead of erroring out, silently\n+\tskips the file if it does not exist.\n+\n --relative-marks::\n \tAfter specifying --relative-marks= the paths specified\n \twith --import-marks= and --export-marks= are relative\ndiff --git a/fast-import.c b/fast-import.c\nindex cbd5124..c525fda 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -329,6 +329,7 @@ static struct mark_set *marks;\n static const char *export_marks_file;\n static const char *import_marks_file;\n static int import_marks_file_from_stream;\n+static int import_marks_file_ignore_missing;\n static int relative_marks_paths;\n \n /* Our last blob */\n@@ -1797,7 +1798,7 @@ static void read_marks(void)\n \tFILE *f = fopen(import_marks_file, \"r\");\n \tif (f)\n \t\t;\n-\telse if (errno == ENOENT)\n+\telse if (import_marks_file_ignore_missing && errno == ENOENT)\n \t\treturn; /* Marks file does not exist */\n \telse\n \t\tdie_errno(\"cannot read '%s'\", import_marks_file);\n@@ -2865,7 +2866,8 @@ static char* make_fast_import_path(const char *path)\n \treturn strbuf_detach(&abs_path, NULL);\n }\n \n-static void option_import_marks(const char *marks, int from_stream)\n+static void option_import_marks(const char *marks,\n+\t\t\t\t\tint from_stream, int ignore_missing)\n {\n \tif (import_marks_file) {\n \t\tif (from_stream)\n@@ -2879,6 +2881,7 @@ static void option_import_marks(const char *marks, int from_stream)\n \timport_marks_file = make_fast_import_path(marks);\n \tsafe_create_leading_directories_const(import_marks_file);\n \timport_marks_file_from_stream = from_stream;\n+\timport_marks_file_ignore_missing = ignore_missing;\n }\n \n static void option_date_format(const char *fmt)\n@@ -2978,7 +2981,10 @@ static int parse_one_feature(const char *feature, int from_stream)\n \tif (!prefixcmp(feature, \"date-format=\")) {\n \t\toption_date_format(feature + 12);\n \t} else if (!prefixcmp(feature, \"import-marks=\")) {\n-\t\toption_import_marks(feature + 13, from_stream);\n+\t\toption_import_marks(feature + 13, from_stream, 0);\n+\t} else if (!prefixcmp(feature, \"import-marks-if-exists=\")) {\n+\t\toption_import_marks(feature + strlen(\"import-marks-if-exists=\"),\n+\t\t\t\t\tfrom_stream, 1);\n \t} else if (!prefixcmp(feature, \"export-marks=\")) {\n \t\toption_export_marks(feature + 13);\n \t} else if (!strcmp(feature, \"cat-blob\")) {\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 222d105..870e55b 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -1706,6 +1706,61 @@ test_expect_success \\\n     'cat input | git fast-import --export-marks=other.marks &&\n     grep :1 other.marks'\n \n+test_expect_success 'R: catch typo in marks file name' '\n+\ttest_must_fail git fast-import --import-marks=nonexistent.marks </dev/null &&\n+\techo \"feature import-marks=nonexistent.marks\" |\n+\ttest_must_fail git fast-import\n+'\n+\n+test_expect_success 'R: import and output marks can be the same file' '\n+\trm -f io.marks &&\n+\tblob=$(echo hi | git hash-object --stdin) &&\n+\tcat >expect <<-EOF &&\n+\t:1 $blob\n+\t:2 $blob\n+\tEOF\n+\tgit fast-import --export-marks=io.marks <<-\\EOF &&\n+\tblob\n+\tmark :1\n+\tdata 3\n+\thi\n+\n+\tEOF\n+\tgit fast-import --import-marks=io.marks --export-marks=io.marks <<-\\EOF &&\n+\tblob\n+\tmark :2\n+\tdata 3\n+\thi\n+\n+\tEOF\n+\ttest_cmp expect io.marks\n+'\n+\n+test_expect_success 'R: --import-marks=foo --output-marks=foo to create foo fails' '\n+\trm -f io.marks &&\n+\ttest_must_fail git fast-import --import-marks=io.marks --export-marks=io.marks <<-\\EOF\n+\tblob\n+\tmark :1\n+\tdata 3\n+\thi\n+\n+\tEOF\n+'\n+\n+test_expect_success 'R: --import-marks-if-exists' '\n+\trm -f io.marks &&\n+\tblob=$(echo hi | git hash-object --stdin) &&\n+\techo \":1 $blob\" >expect &&\n+\tgit fast-import --import-marks-if-exists=io.marks --export-marks=io.marks <<-\\EOF &&\n+\tblob\n+\tmark :1\n+\tdata 3\n+\thi\n+\n+\tEOF\n+\ttest_cmp expect io.marks\n+'\n+\n cat >input << EOF\n feature import-marks=marks.out\n feature export-marks=marks.new\n-- \n1.7.4.rc1\n"},{"id":"159378","messageId":"7vei8j435f.fsf@alter.siamese.dyndns.org","threadId":"26246","inReplyTo":"1294766058-29739-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH] fast-import: Don't barf if import-marks file is missing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-01-11T20:05:32Z","receivedAt":"2011-01-11T20:05:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> The --import-marks option used to barf when the specified marks file\n> doesn't exist. Change its meaning to \"read marks file if it exists\" so\n> that callers don't have to handle bootstraping as a special case.\n\nIt is very plausible that people other than you are relying on \"barf if it\ndoes not exist\" semantics, and the above does not justify breaking their\nworkflow.\n\nI'd suggest making this an optional feature.\n"},{"id":"159519","messageId":"1295073106-5779-1-git-send-email-artagnon@gmail.com","threadId":"26246","inReplyTo":"20110111175102.GB15133@burratino","subject":"[PATCH] fast-import: Introduce --import-marks-if-exists","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-01-15T06:31:46Z","receivedAt":"2011-01-15T06:31:46Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The --import-marks option errors out when the specified marks file\ndoesn't exist. If a frontend is just using a marks file to ensure its\nstate persists between runs, it would be simpler if missing marks\nfiles were tolerated.  In some cases, the frontend cannot even check\nfor the existence of the marks file because with --relative-marks, the\nlocation of the marks file is backend-dependent. This can be solved by\nchanging the meaning of --import-marks, but will result in trading off\nfunctionality that some frontends might desire. So, add new\ncommand-line option --import-marks-if-exists that's like\n--import-marks, except that it tolerates missing marks files.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nHelped-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Changes since last time: Junio suggested making this an optional\n feature, and Jonathan wrote a fixup patch for the same.\n\n Documentation/git-fast-import.txt |    4 +++\n fast-import.c                     |   16 +++++++++--\n t/t9300-fast-import.sh            |   55 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 72 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex f56dfca..32a062c 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -78,6 +78,10 @@ OPTIONS\n \tset of marks.  If a mark is defined to different values,\n \tthe last file wins.\n \n+--import-marks-if-exists=<file>::\n+\tLike --import-marks but instead of erroring out, silently\n+\tskips the file if it does not exist.\n+\n --relative-marks::\n \tAfter specifying --relative-marks= the paths specified\n \twith --import-marks= and --export-marks= are relative\ndiff --git a/fast-import.c b/fast-import.c\nindex 7857760..c525fda 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -329,6 +329,7 @@ static struct mark_set *marks;\n static const char *export_marks_file;\n static const char *import_marks_file;\n static int import_marks_file_from_stream;\n+static int import_marks_file_ignore_missing;\n static int relative_marks_paths;\n \n /* Our last blob */\n@@ -1795,7 +1796,11 @@ static void read_marks(void)\n {\n \tchar line[512];\n \tFILE *f = fopen(import_marks_file, \"r\");\n-\tif (!f)\n+\tif (f)\n+\t\t;\n+\telse if (import_marks_file_ignore_missing && errno == ENOENT)\n+\t\treturn; /* Marks file does not exist */\n+\telse\n \t\tdie_errno(\"cannot read '%s'\", import_marks_file);\n \twhile (fgets(line, sizeof(line), f)) {\n \t\tuintmax_t mark;\n@@ -2861,7 +2866,8 @@ static char* make_fast_import_path(const char *path)\n \treturn strbuf_detach(&abs_path, NULL);\n }\n \n-static void option_import_marks(const char *marks, int from_stream)\n+static void option_import_marks(const char *marks,\n+\t\t\t\t\tint from_stream, int ignore_missing)\n {\n \tif (import_marks_file) {\n \t\tif (from_stream)\n@@ -2875,6 +2881,7 @@ static void option_import_marks(const char *marks, int from_stream)\n \timport_marks_file = make_fast_import_path(marks);\n \tsafe_create_leading_directories_const(import_marks_file);\n \timport_marks_file_from_stream = from_stream;\n+\timport_marks_file_ignore_missing = ignore_missing;\n }\n \n static void option_date_format(const char *fmt)\n@@ -2974,7 +2981,10 @@ static int parse_one_feature(const char *feature, int from_stream)\n \tif (!prefixcmp(feature, \"date-format=\")) {\n \t\toption_date_format(feature + 12);\n \t} else if (!prefixcmp(feature, \"import-marks=\")) {\n-\t\toption_import_marks(feature + 13, from_stream);\n+\t\toption_import_marks(feature + 13, from_stream, 0);\n+\t} else if (!prefixcmp(feature, \"import-marks-if-exists=\")) {\n+\t\toption_import_marks(feature + strlen(\"import-marks-if-exists=\"),\n+\t\t\t\t\tfrom_stream, 1);\n \t} else if (!prefixcmp(feature, \"export-marks=\")) {\n \t\toption_export_marks(feature + 13);\n \t} else if (!strcmp(feature, \"cat-blob\")) {\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 222d105..870e55b 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -1706,6 +1706,61 @@ test_expect_success \\\n     'cat input | git fast-import --export-marks=other.marks &&\n     grep :1 other.marks'\n \n+test_expect_success 'R: catch typo in marks file name' '\n+\ttest_must_fail git fast-import --import-marks=nonexistent.marks </dev/null &&\n+\techo \"feature import-marks=nonexistent.marks\" |\n+\ttest_must_fail git fast-import\n+'\n+\n+test_expect_success 'R: import and output marks can be the same file' '\n+\trm -f io.marks &&\n+\tblob=$(echo hi | git hash-object --stdin) &&\n+\tcat >expect <<-EOF &&\n+\t:1 $blob\n+\t:2 $blob\n+\tEOF\n+\tgit fast-import --export-marks=io.marks <<-\\EOF &&\n+\tblob\n+\tmark :1\n+\tdata 3\n+\thi\n+\n+\tEOF\n+\tgit fast-import --import-marks=io.marks --export-marks=io.marks <<-\\EOF &&\n+\tblob\n+\tmark :2\n+\tdata 3\n+\thi\n+\n+\tEOF\n+\ttest_cmp expect io.marks\n+'\n+\n+test_expect_success 'R: --import-marks=foo --output-marks=foo to create foo fails' '\n+\trm -f io.marks &&\n+\ttest_must_fail git fast-import --import-marks=io.marks --export-marks=io.marks <<-\\EOF\n+\tblob\n+\tmark :1\n+\tdata 3\n+\thi\n+\n+\tEOF\n+'\n+\n+test_expect_success 'R: --import-marks-if-exists' '\n+\trm -f io.marks &&\n+\tblob=$(echo hi | git hash-object --stdin) &&\n+\techo \":1 $blob\" >expect &&\n+\tgit fast-import --import-marks-if-exists=io.marks --export-marks=io.marks <<-\\EOF &&\n+\tblob\n+\tmark :1\n+\tdata 3\n+\thi\n+\n+\tEOF\n+\ttest_cmp expect io.marks\n+'\n+\n cat >input << EOF\n feature import-marks=marks.out\n feature export-marks=marks.new\n-- \n1.7.4.rc1.7.g2cf08.dirty\n"}]}