{"thread":{"id":"53716","subject":"[PATCH 0/3] fast-export: allow dumping anonymization mappings","startedAt":"2020-06-19T13:23:10Z","lastAt":"2020-06-30T19:35:51Z","messageCount":64,"participants":["Jeff King","Eric Sunshine","Junio C Hamano","Johannes Schindelin","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"400114","messageId":"20200619132304.GA2540657@coredump.intra.peff.net","threadId":"53716","inReplyTo":null,"subject":"[PATCH 0/3] fast-export: allow dumping anonymization mappings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-19T13:23:04Z","receivedAt":"2020-06-19T13:23:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The \"master\" branch is special in fast-export in that we don't anonymize\nit. And the reason is that it helps to have some known reference point\nbetween the original and anonymized repo, so you can find the same\ncommits.\n\nThis series gives an alternate way to achieve the same effect, but much\nbetter in that it works for _any_ ref (so if you are trying to reproduce\nthe effect of \"rev-list origin/foo..bar\" in the anonymized repo, you can\neasily do so). Ditto for paths, so that \"rev-list -- foo.c\" can be\nreproduced in the anonymized repo.\n\nAnd then we can drop the specialness of \"master\", which in turn is one\nless thing to worry about in Dscho's series to make the default branch\nconfigurable.\n\n [1/3]: fast-export: allow dumping the refname mapping\n [2/3]: fast-export: anonymize \"master\" refname\n [3/3]: fast-export: allow dumping the path mapping\n\n Documentation/git-fast-export.txt | 32 +++++++++++++++++\n builtin/fast-export.c | 58 +++++++++++++++++++++++++++----\n t/t9351-fast-export-anonymize.sh | 30 ++++++++++++----\n 3 files changed, 106 insertions(+), 14 deletions(-)\n\n-Peff\n"},{"id":"400115","messageId":"20200619132546.GA2540774@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200619132304.GA2540657@coredump.intra.peff.net","subject":"[PATCH 1/3] fast-export: allow dumping the refname mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-19T13:25:46Z","receivedAt":"2020-06-19T13:25:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"After you anonymize a repository, it can be hard to find which commits\ncorrespond between the original and the result, and thus hard to\nreproduce commands that triggered bugs in the original.\n\nLet's make it possible to dump the mapping separate from the output\nstream. This can be used by a bug reporter to modify their reproduction\nrecipe without revealing the original names (see the example in the\ndocumentation).\n\nThe implementation is slightly non-obvious. There's no point in the\nprogram where we know the complete set of refs we're going to anonymize.\nNor do we have a complete set of anonymized refs after finishing (we\nhave a set of anonymized ref path components, but no knowledge of how\nthose are assembled into complete refs). So we lazily write to the dump\nfile as we anonymize each name, and keep a list of ones that we've\noutput in order to avoid duplicates.\n\nSome possible alternatives:\n\n - we could just output the mapping of anonymized components (e.g.,\n that \"foo\" became \"ref123\"). That works OK when you have short\n refnames (e.g., \"refs/heads/foo\" becomes \"refs/heads/ref123\"), but\n longer names would require the user to look up each component to\n assemble the result. For example, \"refs/remotes/origin/jk/foo\" might\n become \"refs/remotes/refs37/refs56/refs102\".\n\n - instead of dumping the mapping, the same problem could be solved by\n allowing the user to leave some refs alone. So if you want to\n reproduce \"git rev-list branch~17..HEAD\" in the anonymized repo, we\n could allow something like:\n\n git tag anon-one branch\n git tag anon-two HEAD\n git fast-export --anonymize --all \\\n --no-anonymize-ref=anon-one \\\n\t\t --no-anonymize-ref=anon-two \\\n\t\t >stream\n\n and then presumably \"git rev-list anon-one~17..anon-two\" would\n behave the same in the re-imported repository. This is more\n convenient in some ways, but it does require modifying the\n original repository. And the concept doesn't easily extend to\n other fields (e.g., pathnames, which will be addressed in a\n subsequent patch).\n\n - we could dump before/after commit hashes; combined with rev-parse,\n that could convert these cases (as well as ones using raw hashes).\n But we don't actually know the anonymized commit hashes; we're just\n generating a stream that will produce them in the anonymized repo.\n\n - likewise, we probably could insert object names or other markers\n into commit messages, blob contents, etc, in order to let a user\n with the original repo figure out which parts correspond. But using\n this gets complicated (I have to find my commits in the result with\n \"git log --all --grep\" or similar). It also makes it less clear that\n the anonymized repo didn't leak any information (because we are\n relying on object ids being unguessable).\n\nSigned-off-by: Jeff King \n---\n Documentation/git-fast-export.txt | 22 +++++++++++++++++\n builtin/fast-export.c | 39 +++++++++++++++++++++++++++++++\n t/t9351-fast-export-anonymize.sh | 12 ++++++++++\n 3 files changed, 73 insertions(+)\n\ndiff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\nindex e8950de3ba..e809bb3f18 100644\n--- a/Documentation/git-fast-export.txt\n+++ b/Documentation/git-fast-export.txt\n@@ -119,6 +119,12 @@ by keeping the marks the same across runs.\n \tthe shape of the history and stored tree. See the section on\n \t`ANONYMIZING` below.\n \n+--dump-anonymized-refnames=::\n+\tOutput the mapping of real refnames to anonymized refnames to\n+\t. The output will contain one line per ref that appears in\n+\tthe output stream, with the original refname, a space, and its\n+\tanonymized counterpart. See the section on `ANONYMIZING` below.\n+\n --reference-excluded-parents::\n \tBy default, running a command such as `git fast-export\n \tmaster~5..master` will not include the commit master{tilde}5\n@@ -238,6 +244,22 @@ collapse \"User 0\", \"User 1\", etc into \"User X\"). This produces a much\n smaller output, and it is usually easy to quickly confirm that there is\n no private data in the stream.\n \n+Reproducing some bugs may require referencing particular commits, which\n+becomes challenging after the refnames have all been anonymized. You can\n+use `--dump-anonymized-refnames` to output the mapping, and then alter\n+your reproduction recipe to use the anonymized names. E.g., if you find\n+a bug with `git rev-list v1.0..v2.0` in the private repository, you can\n+run:\n+\n+---------------------------------------------------\n+$ git fast-export --anonymize --all --dump-anonymized-refnames=refs.out >stream\n+$ grep '^refs/tags/v[12].0' refs.out\n+refs/tags/v1.0 refs/tags/ref31\n+refs/tags/v2.0 refs/tags/ref50\n+---------------------------------------------------\n+\n+which tells you that `git rev-list ref31..ref50` may produce the same\n+bug in the re-imported anonymous repository.\n \n LIMITATIONS\n -----------\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 85868162ee..6caea6f290 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -24,6 +24,7 @@\n #include \"remote.h\"\n #include \"blob.h\"\n #include \"commit-slab.h\"\n+#include \"khash.h\"\n \n static const char *fast_export_usage[] = {\n \tN_(\"git fast-export [rev-list-opts]\"),\n@@ -45,6 +46,7 @@ static struct string_list extra_refs = STRING_LIST_INIT_NODUP;\n static struct string_list tag_refs = STRING_LIST_INIT_NODUP;\n static struct refspec refspecs = REFSPEC_INIT_FETCH;\n static int anonymize;\n+static FILE *anonymized_refnames_handle;\n static struct revision_sources revision_sources;\n \n static int parse_opt_signed_tag_mode(const struct option *opt,\n@@ -118,6 +120,32 @@ static int has_unshown_parent(struct commit *commit)\n \treturn 0;\n }\n \n+KHASH_INIT(strset, const char *, int, 0, kh_str_hash_func, kh_str_hash_equal);\n+\n+struct seen_set {\n+\tkh_strset_t *set;\n+};\n+\n+static int check_and_mark_seen(struct seen_set *seen, const char *str)\n+{\n+\tint hashret;\n+\tif (!seen->set)\n+\t\tseen->set = kh_init_strset();\n+\tif (kh_get_strset(seen->set, str) < kh_end(seen->set))\n+\t\treturn 1;\n+\tkh_put_strset(seen->set, xstrdup(str), &hashret);\n+\treturn 0;\n+}\n+\n+static void maybe_dump_anon(FILE *out, struct seen_set *seen,\n+\t\t\t const char *orig, const char *anon)\n+{\n+\tif (!out)\n+\t\treturn;\n+\tif (!check_and_mark_seen(seen, orig))\n+\t\tfprintf(out, \"%s %s\\n\", orig, anon);\n+}\n+\n struct anonymized_entry {\n \tstruct hashmap_entry hash;\n \tconst char *orig;\n@@ -515,6 +543,8 @@ static const char *anonymize_refname(const char *refname)\n \t};\n \tstatic struct hashmap refs;\n \tstatic struct strbuf anon = STRBUF_INIT;\n+\tstatic struct seen_set seen;\n+\tconst char *full_refname = refname;\n \tint i;\n \n \t/*\n@@ -533,6 +563,8 @@ static const char *anonymize_refname(const char *refname)\n \t}\n \n \tanonymize_path(&anon, refname, &refs, anonymize_ref_component);\n+\tmaybe_dump_anon(anonymized_refnames_handle, &seen,\n+\t\t\tfull_refname, anon.buf);\n \treturn anon.buf;\n }\n \n@@ -1144,6 +1176,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \tchar *export_filename = NULL,\n \t *import_filename = NULL,\n \t *import_filename_if_exists = NULL;\n+\tconst char *anonymized_refnames_file = NULL;\n \tuint32_t lastimportid;\n \tstruct string_list refspecs_list = STRING_LIST_INIT_NODUP;\n \tstruct string_list paths_of_changed_objects = STRING_LIST_INIT_DUP;\n@@ -1177,6 +1210,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING_LIST(0, \"refspec\", &refspecs_list, N_(\"refspec\"),\n \t\t\t N_(\"Apply refspec to exported refs\")),\n \t\tOPT_BOOL(0, \"anonymize\", &anonymize, N_(\"anonymize output\")),\n+\t\tOPT_STRING(0, \"dump-anonymized-refnames\",\n+\t\t\t &anonymized_refnames_file, N_(\"file\"),\n+\t\t\t N_(\"output anonymized refname mapping to \")),\n \t\tOPT_BOOL(0, \"reference-excluded-parents\",\n \t\t\t &reference_excluded_commits, N_(\"Reference parents which are not in fast-export stream by object id\")),\n \t\tOPT_BOOL(0, \"show-original-ids\", &show_original_ids,\n@@ -1213,6 +1249,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\tstring_list_clear(&refspecs_list, 1);\n \t}\n \n+\tif (anonymized_refnames_file)\n+\t\tanonymized_refnames_handle = xfopen(anonymized_refnames_file, \"w\");\n+\n \tif (use_done_feature)\n \t\tprintf(\"feature done\\n\");\n \ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex 897dc50907..0c5dd2a4fb 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -46,6 +46,18 @@ test_expect_success 'stream omits tag message' '\n \t! grep \"annotated tag\" stream\n '\n \n+test_expect_success 'refname mapping can be dumped' '\n+\tgit fast-export --anonymize --all \\\n+\t\t--dump-anonymized-refnames=refs.out >/dev/null &&\n+\t# we make no guarantees of the exact anonymized names,\n+\t# so just check that we have the right number and\n+\t# that a sample line looks sane.\n+\t# Note that master is not anonymized, and so not included\n+\t# in the mapping.\n+\ttest_line_count = 6 refs.out &&\n+\tgrep \"^refs/heads/other refs/heads/\" refs.out\n+'\n+\n # NOTE: we chdir to the new, anonymized repository\n # after this. All further tests should assume this.\n test_expect_success 'import stream to new repository' '\n-- \n2.27.0.480.g4f98dbcb10\n\n"},{"id":"400116","messageId":"20200619132640.GB2540774@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200619132304.GA2540657@coredump.intra.peff.net","subject":"[PATCH 2/3] fast-export: anonymize \"master\" refname","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-19T13:26:40Z","receivedAt":"2020-06-19T13:26:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Running \"fast-export --anonymize\" will leave \"refs/heads/master\"\nuntouched in the output, for two reasons:\n\n - it helped to have some known reference point between the original\n and anonymized repository\n\n - since it's historically the default branch name, it doesn't leak any\n information\n\nNow that we can ask fast-export to dump the anonymized ref mapping, we\nhave a much better tool for the first one (because it works for _any_\nref, not just master).\n\nFor the second, the notion of \"default branch name\" is likely to become\nconfigurable soon, at which point the name _does_ leak information.\nLet's drop this special case in preparation.\n\nNote that we have to adjust the test a bit, since it relied on using the\nname \"master\" in the anonymized repos. But this gives us a good\nopportunity to further test the new dumping feature.\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 7 -------\n t/t9351-fast-export-anonymize.sh | 16 ++++++----------\n 2 files changed, 6 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 6caea6f290..cd0174d514 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -547,13 +547,6 @@ static const char *anonymize_refname(const char *refname)\n \tconst char *full_refname = refname;\n \tint i;\n \n-\t/*\n-\t * We also leave \"master\" as a special case, since it does not reveal\n-\t * anything interesting.\n-\t */\n-\tif (!strcmp(refname, \"refs/heads/master\"))\n-\t\treturn refname;\n-\n \tstrbuf_reset(&anon);\n \tfor (i = 0; i < ARRAY_SIZE(prefixes); i++) {\n \t\tif (skip_prefix(refname, prefixes[i], &refname)) {\ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex 0c5dd2a4fb..88847b0f60 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -26,11 +26,8 @@ test_expect_success 'stream omits path names' '\n \t! grep xyzzy stream\n '\n \n-test_expect_success 'stream allows master as refname' '\n-\tgrep master stream\n-'\n-\n-test_expect_success 'stream omits other refnames' '\n+test_expect_success 'stream omits refnames' '\n+\t! grep master stream &&\n \t! grep other stream &&\n \t! grep mytag stream\n '\n@@ -52,9 +49,7 @@ test_expect_success 'refname mapping can be dumped' '\n \t# we make no guarantees of the exact anonymized names,\n \t# so just check that we have the right number and\n \t# that a sample line looks sane.\n-\t# Note that master is not anonymized, and so not included\n-\t# in the mapping.\n-\ttest_line_count = 6 refs.out &&\n+\ttest_line_count = 7 refs.out &&\n \tgrep \"^refs/heads/other refs/heads/\" refs.out\n '\n \n@@ -69,15 +64,16 @@ test_expect_success 'import stream to new repository' '\n test_expect_success 'result has two branches' '\n \tgit for-each-ref --format=\"%(refname)\" refs/heads >branches &&\n \ttest_line_count = 2 branches &&\n-\tother_branch=$(grep -v refs/heads/master branches)\n+\tmain_branch=$(sed -ne \"s,refs/heads/master ,,p\" ../refs.out) &&\n+\tother_branch=$(sed -ne \"s,refs/heads/other ,,p\" ../refs.out)\n '\n \n test_expect_success 'repo has original shape and timestamps' '\n \tshape () {\n \t\tgit log --format=\"%m %ct\" --left-right --boundary \"$@\"\n \t} &&\n \t(cd .. && shape master...other) >expect &&\n-\tshape master...$other_branch >actual &&\n+\tshape $main_branch...$other_branch >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.27.0.480.g4f98dbcb10\n\n"},{"id":"400118","messageId":"20200619132923.GA2540897@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200619132304.GA2540657@coredump.intra.peff.net","subject":"[PATCH 3/3] fast-export: allow dumping the path mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-19T13:29:23Z","receivedAt":"2020-06-19T13:29:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When working with an anonymized repo, it can be useful to be able to\nrefer to particular paths. E.g., reproducing a bug with \"git rev-list --\nfoo.c\" in the original repo would need to replace \"foo.c\" with its\nanonymized counterpart to produce the same effect.\n\nWe recently taught fast-export to dump the refname mapping. Let's do the\nsame thing for paths, which can reuse most of the same infrastructure.\nNote that the output format isn't unambiguous here (because paths could\ncontain spaces). That's OK because this is meant to be examined by a\nhuman.\n\nWe could also just introduce a \"dump mapping\" file that shows every\nmapping we make. But it would be a bit more awkward to work with, as the\nuser would have to sort through more data to find the parts they're\ninterested in (and there are likely to be many more paths than refnames,\nmaking it annoying for people who just want to dump the refnames).\n\nSigned-off-by: Jeff King \n---\n Documentation/git-fast-export.txt | 10 ++++++++++\n builtin/fast-export.c | 12 ++++++++++++\n t/t9351-fast-export-anonymize.sh | 8 ++++++++\n 3 files changed, 30 insertions(+)\n\ndiff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\nindex e809bb3f18..c63f109f1d 100644\n--- a/Documentation/git-fast-export.txt\n+++ b/Documentation/git-fast-export.txt\n@@ -125,6 +125,12 @@ by keeping the marks the same across runs.\n \tthe output stream, with the original refname, a space, and its\n \tanonymized counterpart. See the section on `ANONYMIZING` below.\n \n+--dump-anonymized-paths=::\n+\tOutput the mapping of real paths to anonymized paths to .\n+\tThe output will contain one line per path that appears in the\n+\toutput stream, with the original path, a space, and its\n+\tanonymized counterpart. See the section on `ANONYMIZING` below.\n+\n --reference-excluded-parents::\n \tBy default, running a command such as `git fast-export\n \tmaster~5..master` will not include the commit master{tilde}5\n@@ -261,6 +267,10 @@ refs/tags/v2.0 refs/tags/ref50\n which tells you that `git rev-list ref31..ref50` may produce the same\n bug in the re-imported anonymous repository.\n \n+Likewise, `--dump-anonymized-paths` may be useful for a bug that\n+involves pathspecs. E.g., `git rev-list v1.0..v2.0 -- foo.c` requires\n+knowing the path corresponding to `foo.c` in the result.\n+\n LIMITATIONS\n -----------\n \ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex cd0174d514..ed1f8daa7f 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -47,6 +47,7 @@ static struct string_list tag_refs = STRING_LIST_INIT_NODUP;\n static struct refspec refspecs = REFSPEC_INIT_FETCH;\n static int anonymize;\n static FILE *anonymized_refnames_handle;\n+static FILE *anonymized_paths_handle;\n static struct revision_sources revision_sources;\n \n static int parse_opt_signed_tag_mode(const struct option *opt,\n@@ -211,6 +212,9 @@ static void anonymize_path(struct strbuf *out, const char *path,\n \t\t\t struct hashmap *map,\n \t\t\t void *(*generate)(const void *, size_t *))\n {\n+\tstatic struct seen_set seen;\n+\tconst char *full_path = path;\n+\n \twhile (*path) {\n \t\tconst char *end_of_component = strchrnul(path, '/');\n \t\tsize_t len = end_of_component - path;\n@@ -220,6 +224,8 @@ static void anonymize_path(struct strbuf *out, const char *path,\n \t\tif (*path)\n \t\t\tstrbuf_addch(out, *path++);\n \t}\n+\n+\tmaybe_dump_anon(anonymized_paths_handle, &seen, full_path, out->buf);\n }\n \n static inline void *mark_to_ptr(uint32_t mark)\n@@ -1170,6 +1176,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t *import_filename = NULL,\n \t *import_filename_if_exists = NULL;\n \tconst char *anonymized_refnames_file = NULL;\n+\tconst char *anonymized_paths_file = NULL;\n \tuint32_t lastimportid;\n \tstruct string_list refspecs_list = STRING_LIST_INIT_NODUP;\n \tstruct string_list paths_of_changed_objects = STRING_LIST_INIT_DUP;\n@@ -1206,6 +1213,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"dump-anonymized-refnames\",\n \t\t\t &anonymized_refnames_file, N_(\"file\"),\n \t\t\t N_(\"output anonymized refname mapping to \")),\n+\t\tOPT_STRING(0, \"dump-anonymized-paths\",\n+\t\t\t &anonymized_paths_file, N_(\"file\"),\n+\t\t\t N_(\"output anonymized path mapping to \")),\n \t\tOPT_BOOL(0, \"reference-excluded-parents\",\n \t\t\t &reference_excluded_commits, N_(\"Reference parents which are not in fast-export stream by object id\")),\n \t\tOPT_BOOL(0, \"show-original-ids\", &show_original_ids,\n@@ -1244,6 +1254,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \n \tif (anonymized_refnames_file)\n \t\tanonymized_refnames_handle = xfopen(anonymized_refnames_file, \"w\");\n+\tif (anonymized_paths_file)\n+\t\tanonymized_paths_handle = xfopen(anonymized_paths_file, \"w\");\n \n \tif (use_done_feature)\n \t\tprintf(\"feature done\\n\");\ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex 88847b0f60..3607b9b972 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -53,6 +53,14 @@ test_expect_success 'refname mapping can be dumped' '\n \tgrep \"^refs/heads/other refs/heads/\" refs.out\n '\n \n+test_expect_success 'path mapping can be dumped' '\n+\tgit fast-export --anonymize --all \\\n+\t\t--dump-anonymized-paths=paths.out >/dev/null &&\n+\t# do not assume a particular anonymization scheme or order;\n+\t# just sanity check that a sample line looks sensible.\n+\tgrep \"^foo \" paths.out\n+'\n+\n # NOTE: we chdir to the new, anonymized repository\n # after this. All further tests should assume this.\n test_expect_success 'import stream to new repository' '\n-- \n2.27.0.480.g4f98dbcb10\n"},{"id":"400127","messageId":"CAPig+cRNem-S5LGX=v=1Tid64sXWBxNyWH4ffgLgF0o1yN=mtw@mail.gmail.com","threadId":"53716","inReplyTo":"20200619132546.GA2540774@coredump.intra.peff.net","subject":"Re: [PATCH 1/3] fast-export: allow dumping the refname mapping","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-06-19T15:51:06Z","receivedAt":"2020-06-19T15:51:26Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jun 19, 2020 at 9:25 AM Jeff King wrote:\n> [...]\n> Let's make it possible to dump the mapping separate from the output\n> stream. This can be used by a bug reporter to modify their reproduction\n> recipe without revealing the original names (see the example in the\n> documentation).\n> [...]\n> Signed-off-by: Jeff King \n> ---\n> diff --git a/builtin/fast-export.c b/builtin/fast-export.c\n> @@ -118,6 +120,32 @@ static int has_unshown_parent(struct commit *commit)\n> +static void maybe_dump_anon(FILE *out, struct seen_set *seen,\n> + const char *orig, const char *anon)\n> +{\n> + if (!out)\n> + return;\n> + if (!check_and_mark_seen(seen, orig))\n> + fprintf(out, \"%s %s\\n\", orig, anon);\n> +}\n\nNit: For a function which has only a single caller in this patch and\none more caller added in 3/3, I wonder if the, um, convenience of this\nfunction short-circuiting as the very first thing it does outweighs\nthe loss of clarity of having the callers make the check themselves.\nThat is, have the caller do this instead:\n\n if (anonymized_refnames_handle)\n dump_anon(anonymized_refnames_handle, ...);\n\n> @@ -1213,6 +1249,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n> + if (anonymized_refnames_file)\n> + anonymized_refnames_handle = xfopen(anonymized_refnames_file, \"w\");\n\nFor completeness, do you want to close this file at some point?\n\n> diff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\n> @@ -46,6 +46,18 @@ test_expect_success 'stream omits tag message' '\n> +test_expect_success 'refname mapping can be dumped' '\n> + git fast-export --anonymize --all \\\n> + --dump-anonymized-refnames=refs.out >/dev/null &&\n> + # we make no guarantees of the exact anonymized names,\n> + # so just check that we have the right number and\n> + # that a sample line looks sane.\n> + # Note that master is not anonymized, and so not included\n> + # in the mapping.\n> + test_line_count = 6 refs.out &&\n\nThis hard-coded \"6\" seems rather fragile, causing the test to break if\nother refs are ever added or removed. Perhaps just count the refs\ndynamically?\n\n git show-ref >refs &&\n nrefs=$(wc -l refs) &&\n # Note that master is not anonymized, and so not included\n # in the mapping.\n nrefs=$((nrefs - 1))\n test_line_count = $nrefs refs.out &&\n\nand then drop the 'nrefs=$((nrefs - 1))' line and associated comment\nin patch 2/3 which removes the \"master\" special case.\n\n> + grep \"^refs/heads/other refs/heads/\" refs.out\n> +'\n"},{"id":"400133","messageId":"CAPig+cSkmGABkfK6MaSTYsX-okrrkk_SpGgztMvzBCT0bLjGmA@mail.gmail.com","threadId":"53716","inReplyTo":"20200619132923.GA2540897@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] fast-export: allow dumping the path mapping","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-06-19T16:00:40Z","receivedAt":"2020-06-19T16:00:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jun 19, 2020 at 9:29 AM Jeff King wrote:\n> [...]\n> We recently taught fast-export to dump the refname mapping. Let's do the\n> same thing for paths, which can reuse most of the same infrastructure.\n> Note that the output format isn't unambiguous here (because paths could\n> contain spaces). That's OK because this is meant to be examined by a\n> human.\n> [...]\n> Signed-off-by: Jeff King \n> ---\n> diff --git a/builtin/fast-export.c b/builtin/fast-export.c\n> @@ -1244,6 +1254,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n> if (anonymized_refnames_file)\n> anonymized_refnames_handle = xfopen(anonymized_refnames_file, \"w\");\n> + if (anonymized_paths_file)\n> + anonymized_paths_handle = xfopen(anonymized_paths_file, \"w\");\n\nSame question as in my patch 1/1 review: Do you want to close this\nfile at some point?\n"},{"id":"400134","messageId":"20200619160129.GA1843858@coredump.intra.peff.net","threadId":"53716","inReplyTo":"CAPig+cRNem-S5LGX=v=1Tid64sXWBxNyWH4ffgLgF0o1yN=mtw@mail.gmail.com","subject":"Re: [PATCH 1/3] fast-export: allow dumping the refname mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-19T16:01:29Z","receivedAt":"2020-06-19T16:01:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 19, 2020 at 11:51:06AM -0400, Eric Sunshine wrote:\n\n> > +static void maybe_dump_anon(FILE *out, struct seen_set *seen,\n> > + const char *orig, const char *anon)\n> > +{\n> > + if (!out)\n> > + return;\n> > + if (!check_and_mark_seen(seen, orig))\n> > + fprintf(out, \"%s %s\\n\", orig, anon);\n> > +}\n> \n> Nit: For a function which has only a single caller in this patch and\n> one more caller added in 3/3, I wonder if the, um, convenience of this\n> function short-circuiting as the very first thing it does outweighs\n> the loss of clarity of having the callers make the check themselves.\n> That is, have the caller do this instead:\n> \n> if (anonymized_refnames_handle)\n> dump_anon(anonymized_refnames_handle, ...);\n\n The names were long enough that I thought it was more clear not\nto repeat myself. I actually wrote originally:\n\n if (anonymized_refnames_handle &&\n check_and_mark_seen(&seen, full_refname))\n fprintf(anonymized_refnames_handle, \"%s %s\\n\",\n\t full_refname, anon.buf));\n\nbut I wasn't sure if it was better not to duplicate the output format.\nOTOH, we _could_ be using quote_path() for the path dumping, in which\ncase the formats would start to differ.\n\n> > @@ -1213,6 +1249,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n> > + if (anonymized_refnames_file)\n> > + anonymized_refnames_handle = xfopen(anonymized_refnames_file, \"w\");\n> \n> For completeness, do you want to close this file at some point?\n\nMy thought was to just let it auto-close at exit() time, since it's\neffectively process-length.\n\n> > diff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\n> > @@ -46,6 +46,18 @@ test_expect_success 'stream omits tag message' '\n> > +test_expect_success 'refname mapping can be dumped' '\n> > + git fast-export --anonymize --all \\\n> > + --dump-anonymized-refnames=refs.out >/dev/null &&\n> > + # we make no guarantees of the exact anonymized names,\n> > + # so just check that we have the right number and\n> > + # that a sample line looks sane.\n> > + # Note that master is not anonymized, and so not included\n> > + # in the mapping.\n> > + test_line_count = 6 refs.out &&\n> \n> This hard-coded \"6\" seems rather fragile, causing the test to break if\n> other refs are ever added or removed. Perhaps just count the refs\n> dynamically?\n> \n> git show-ref >refs &&\n> nrefs=$(wc -l refs) &&\n> # Note that master is not anonymized, and so not included\n> # in the mapping.\n> nrefs=$((nrefs - 1))\n> test_line_count = $nrefs refs.out &&\n> \n> and then drop the 'nrefs=$((nrefs - 1))' line and associated comment\n> in patch 2/3 which removes the \"master\" special case.\n\nThat's exactly what I wrote originally, but it failed on macos due to\nextra spaces in the \"wc\" output. There are other tests nearby that also\ncount the refs (e.g., exactly 2 branches), so I didn't think it was\nworth trying to deal with the portability issue.\n\n-Peff\n"},{"id":"400143","messageId":"20200619161816.GA9205@flurp.local","threadId":"53716","inReplyTo":"20200619160129.GA1843858@coredump.intra.peff.net","subject":"Re: [PATCH 1/3] fast-export: allow dumping the refname mapping","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-06-19T16:18:16Z","receivedAt":"2020-06-19T16:19:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jun 19, 2020 at 12:01 PM Jeff King wrote:\n> On Fri, Jun 19, 2020 at 11:51:06AM -0400, Eric Sunshine wrote:\n> > That is, have the caller do this instead:\n> >\n> > if (anonymized_refnames_handle)\n> > dump_anon(anonymized_refnames_handle, ...);\n>\n> The names were long enough that I thought it was more clear not\n> to repeat myself. [...]\n\nIndeed, it's a minor point and subjective. Certainly not worth a\nre-roll or even worrying about it.\n\n> > This hard-coded \"6\" seems rather fragile, causing the test to break if\n> > other refs are ever added or removed. Perhaps just count the refs\n> > dynamically?\n> >\n> > git show-ref >refs &&\n> > nrefs=$(wc -l refs) &&\n> > # Note that master is not anonymized, and so not included\n> > # in the mapping.\n> > nrefs=$((nrefs - 1))\n> > test_line_count = $nrefs refs.out &&\n> >\n> That's exactly what I wrote originally, but it failed on macos due to\n> extra spaces in the \"wc\" output.\n\nHmph, that shouldn't have failed. Did you quote the $(wc -l refs)\ninvocation? Quoting it would cause it to fail.\n\nI just applied this atop your patch and it worked fine on Mac OS:\n\ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex 0c5dd2a4fb..849192b1b0 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -52,9 +52,12 @@ test_expect_success 'refname mapping can be dumped' '\n \t# we make no guarantees of the exact anonymized names,\n \t# so just check that we have the right number and\n \t# that a sample line looks sane.\n+\tgit show-ref >refs &&\n+\tnrefs=$(wc -l > That's exactly what I wrote originally, but it failed on macos due to\n> > extra spaces in the \"wc\" output.\n> \n> Hmph, that shouldn't have failed. Did you quote the $(wc -l refs)\n> invocation? Quoting it would cause it to fail.\n\nNope (and indeed, I was wary of the issue and made sure I didn't use\nquotes). My original was:\n\ntest_expect_success 'refname mapping can be dumped' '\n git fast-export --anonymize --all \\\n --dump-anonymized-refnames=refs.out >/dev/null &&\n # we make no guarantees of the exact anonymized names,\n # so just check that we have the right number and\n # that a sample line looks sane.\n expected_count=$(git for-each-ref | wc -l) &&\n # Note that master is not anonymized, and so not included\n # in the mapping.\n expected_count=$((expected_count - 1)) &&\n test_line_count = \"$expected_count\" refs.out &&\n grep \"^refs/heads/other refs/heads/\" refs.out\n'\n\nSo I guess I did quote the variable later. It works fine on Linux, but\none of the osx ci jobs failed:\n\n https://github.com/peff/git/runs/787911270\n\nThe relevant log is:\n\n++ git fast-export --anonymize --all --dump-anonymized-refnames=refs.out\n+++ git for-each-ref\n+++ wc -l\n++ expected_count=' 7'\n++ test_line_count = ' 7' refs.out\n++ test 3 '!=' 3\n+++ wc -l\n++ test 7 = ' 7'\n++ echo 'test_line_count: line count for refs.out != 7'\ntest_line_count: line count for refs.out != 7\n\nso the whitespace is eaten not when \"wc\" is run, but rather when the\nvariable is expanded.\n\n-Peff\n"},{"id":"400202","messageId":"CAPig+cQxoz1AcS51qhtBr6oaTs9KTDamofCyN3qy_EARUi7oFQ@mail.gmail.com","threadId":"53716","inReplyTo":"20200619174551.GA2123813@coredump.intra.peff.net","subject":"Re: [PATCH 1/3] fast-export: allow dumping the refname mapping","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-06-19T18:00:47Z","receivedAt":"2020-06-19T18:01:02Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jun 19, 2020 at 1:45 PM Jeff King wrote:\n> On Fri, Jun 19, 2020 at 12:18:16PM -0400, Eric Sunshine wrote:\n> > Hmph, that shouldn't have failed. Did you quote the $(wc -l refs)\n> > invocation? Quoting it would cause it to fail.\n>\n> Nope (and indeed, I was wary of the issue and made sure I didn't use\n> quotes). My original was:\n>\n> test_expect_success 'refname mapping can be dumped' '\n> [...]\n> expected_count=$(git for-each-ref | wc -l) &&\n> expected_count=$((expected_count - 1)) &&\n> test_line_count = \"$expected_count\" refs.out &&\n>\n> So I guess I did quote the variable later. It works fine on Linux, but\n> one of the osx ci jobs failed:\n>\n> ++ expected_count=' 7'\n> ++ test_line_count = ' 7' refs.out\n> ++ test 7 = ' 7'\n> ++ echo 'test_line_count: line count for refs.out != 7'\n>\n> so the whitespace is eaten not when \"wc\" is run, but rather when the\n> variable is expanded.\n\nNot something that should be done by this series (more a\nleft-over-bitty thing, perhaps), but this almost suggests that\ntest_line_count() deserves a tweak to make it more robust against that\nsort of thing:\n\n test_line_count () {\n if test $# != 3\n then\n BUG \"not 3 parameters to test_line_count\"\n elif ! test $(wc -l <\"$3\") \"$1\" \"$2\"\n then\n echo \"test_line_count: line count for $3 !$1 $2\"\n cat \"$3\"\n return 1\n fi\n }\n\nIf we drop the quotes around $2 from the 'test':\n\n elif ! test $(wc -l <\"$3\") \"$1\" $2\n\nthen your code would have worked as expected.\n\nMy only worry about that is that a poorly written caller would get a\nweird and unhelpful error message:\n\n test_line_count = 4 4\n --> sh: test: too many arguments\n"},{"id":"400206","messageId":"xmqqv9jmhng9.fsf@gitster.c.googlers.com","threadId":"53716","inReplyTo":"20200619161816.GA9205@flurp.local","subject":"Re: [PATCH 1/3] fast-export: allow dumping the refname mapping","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-19T19:20:38Z","receivedAt":"2020-06-19T19:20:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Fri, Jun 19, 2020 at 12:01 PM Jeff King wrote:\n>> On Fri, Jun 19, 2020 at 11:51:06AM -0400, Eric Sunshine wrote:\n>> > That is, have the caller do this instead:\n>> >\n>> > if (anonymized_refnames_handle)\n>> > dump_anon(anonymized_refnames_handle, ...);\n>>\n>> The names were long enough that I thought it was more clear not\n>> to repeat myself. [...]\n>\n> Indeed, it's a minor point and subjective. Certainly not worth a\n> re-roll or even worrying about it.\n\nIt probably is subjective but fwiw I too find yours easier to\nfollow.\n\n>> > This hard-coded \"6\" seems rather fragile, causing the test to break if\n>> > other refs are ever added or removed. Perhaps just count the refs\n>> > dynamically?\n>> >\n>> > git show-ref >refs &&\n>> > nrefs=$(wc -l refs) &&\n>> > # Note that master is not anonymized, and so not included\n>> > # in the mapping.\n>> > nrefs=$((nrefs - 1))\n>> > test_line_count = $nrefs refs.out &&\n>> >\n>> That's exactly what I wrote originally, but it failed on macos due to\n>> extra spaces in the \"wc\" output.\n>\n> Hmph, that shouldn't have failed. Did you quote the $(wc -l refs)\n> invocation? Quoting it would cause it to fail.\n\n> +\tgit show-ref >refs &&\n> +\tnrefs=$(wc -l \t# Note that master is not anonymized, and so not included\n> \t# in the mapping.\n> -\ttest_line_count = 6 refs.out &&\n> +\tnrefs=$((nrefs - 1)) &&\n> +\ttest_line_count = $nrefs refs.out &&\n> \tgrep \"^refs/heads/other refs/heads/\" refs.out\n> '\n"},{"id":"400207","messageId":"xmqqr1uahn9t.fsf@gitster.c.googlers.com","threadId":"53716","inReplyTo":"20200619132923.GA2540897@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] fast-export: allow dumping the path mapping","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-19T19:24:30Z","receivedAt":"2020-06-19T19:24:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> When working with an anonymized repo, it can be useful to be able to\n> refer to particular paths. E.g., reproducing a bug with \"git rev-list --\n> foo.c\" in the original repo would need to replace \"foo.c\" with its\n> anonymized counterpart to produce the same effect.\n\nThat would not work with \"git rev-list -- 'foo*'\", where there is no\ncounterpart, no?\n\nIt almost feels as if we want a separate knob to disable\nanonymization for paths and perhaps refs while still anonymizing the\nblob contents, or something like that.\n\n"},{"id":"400277","messageId":"nycvar.QRO.7.76.6.2006191550260.54@tvgsbejvaqbjf.bet","threadId":"53716","inReplyTo":"20200619132304.GA2540657@coredump.intra.peff.net","subject":"Re: [PATCH 0/3] fast-export: allow dumping anonymization mappings","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-06-19T13:51:55Z","receivedAt":"2020-06-22T12:12:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Fri, 19 Jun 2020, Jeff King wrote:\n\n> The \"master\" branch is special in fast-export in that we don't anonymize\n> it. And the reason is that it helps to have some known reference point\n> between the original and anonymized repo, so you can find the same\n> commits.\n>\n> This series gives an alternate way to achieve the same effect, but much\n> better in that it works for _any_ ref (so if you are trying to reproduce\n> the effect of \"rev-list origin/foo..bar\" in the anonymized repo, you can\n> easily do so). Ditto for paths, so that \"rev-list -- foo.c\" can be\n> reproduced in the anonymized repo.\n>\n> And then we can drop the specialness of \"master\", which in turn is one\n> less thing to worry about in Dscho's series to make the default branch\n> configurable.\n\nThank you for working on this!\n\nI don't have any suggestions on top of Eric's and Junio's.\n\nI'll drop the `fast-export` patches from the next round of my patch\nseries.\n\nCiao,\nDscho\n\n>\n> [1/3]: fast-export: allow dumping the refname mapping\n> [2/3]: fast-export: anonymize \"master\" refname\n> [3/3]: fast-export: allow dumping the path mapping\n>\n> Documentation/git-fast-export.txt | 32 +++++++++++++++++\n> builtin/fast-export.c | 58 +++++++++++++++++++++++++++----\n> t/t9351-fast-export-anonymize.sh | 30 ++++++++++++----\n> 3 files changed, 106 insertions(+), 14 deletions(-)\n>\n> -Peff\n>\n"},{"id":"400286","messageId":"xmqqftandpn6.fsf@gitster.c.googlers.com","threadId":"53716","inReplyTo":"nycvar.QRO.7.76.6.2006191550260.54@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 0/3] fast-export: allow dumping anonymization mappings","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-22T16:35:57Z","receivedAt":"2020-06-22T16:36:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi Peff,\n>\n> On Fri, 19 Jun 2020, Jeff King wrote:\n>\n>> The \"master\" branch is special in fast-export in that we don't anonymize\n>> it. And the reason is that it helps to have some known reference point\n>> between the original and anonymized repo, so you can find the same\n>> commits.\n>>\n>> This series gives an alternate way to achieve the same effect, but much\n>> better in that it works for _any_ ref (so if you are trying to reproduce\n>> the effect of \"rev-list origin/foo..bar\" in the anonymized repo, you can\n>> easily do so). Ditto for paths, so that \"rev-list -- foo.c\" can be\n>> reproduced in the anonymized repo.\n>>\n>> And then we can drop the specialness of \"master\", which in turn is one\n>> less thing to worry about in Dscho's series to make the default branch\n>> configurable.\n>\n> Thank you for working on this!\n>\n> I don't have any suggestions on top of Eric's and Junio's.\n>\n> I'll drop the `fast-export` patches from the next round of my patch\n> series.\n\nUnderstood. Thanks, all.\n"},{"id":"400315","messageId":"20200622213050.GA1376217@coredump.intra.peff.net","threadId":"53716","inReplyTo":"CAPig+cQxoz1AcS51qhtBr6oaTs9KTDamofCyN3qy_EARUi7oFQ@mail.gmail.com","subject":"Re: [PATCH 1/3] fast-export: allow dumping the refname mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-22T21:30:50Z","receivedAt":"2020-06-22T21:30:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 19, 2020 at 02:00:47PM -0400, Eric Sunshine wrote:\n\n> > so the whitespace is eaten not when \"wc\" is run, but rather when the\n> > variable is expanded.\n> \n> Not something that should be done by this series (more a\n> left-over-bitty thing, perhaps), but this almost suggests that\n> test_line_count() deserves a tweak to make it more robust against that\n> sort of thing:\n> \n> test_line_count () {\n> if test $# != 3\n> then\n> BUG \"not 3 parameters to test_line_count\"\n> elif ! test $(wc -l <\"$3\") \"$1\" \"$2\"\n> then\n> echo \"test_line_count: line count for $3 !$1 $2\"\n> cat \"$3\"\n> return 1\n> fi\n> }\n> \n> If we drop the quotes around $2 from the 'test':\n> \n> elif ! test $(wc -l <\"$3\") \"$1\" $2\n> \n> then your code would have worked as expected.\n> \n> My only worry about that is that a poorly written caller would get a\n> weird and unhelpful error message:\n> \n> test_line_count = 4 4\n> --> sh: test: too many arguments\n\nI think your unhelpful-error-message case would happen only if the\nlength argument contains two non-whitespace tokens separated by a\nwhitespace (so the shell splits them into two arguments), _and_ the\ncaller passed that argument in quotes (otherwise the shell would split\nit at the function call and we'd hit the BUG message). In which case,\nwhat are they trying to do with passing \"4 4\" to \"test\"? And since we're\nusing \"=\" and not \"-eq\", I think \"test\" would be complaining about that.\n\nSo it seems like it might be a reasonable change to make things more\nfriendly.\n\n-Peff\n"},{"id":"400316","messageId":"20200622213241.GB1376217@coredump.intra.peff.net","threadId":"53716","inReplyTo":"xmqqv9jmhng9.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 1/3] fast-export: allow dumping the refname mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-22T21:32:41Z","receivedAt":"2020-06-22T21:32:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 19, 2020 at 12:20:38PM -0700, Junio C Hamano wrote:\n\n> >> The names were long enough that I thought it was more clear not\n> >> to repeat myself. [...]\n> >\n> > Indeed, it's a minor point and subjective. Certainly not worth a\n> > re-roll or even worrying about it.\n> \n> It probably is subjective but fwiw I too find yours easier to\n> follow.\n\nI ended up doing it this way, since I decided to quote the pathnames\n(and thus they needed their own fprintf, at which point there was\nreally nothing left for the two to share).\n\n> > Hmph, that shouldn't have failed. Did you quote the $(wc -l refs)\n> > invocation? Quoting it would cause it to fail.\n> \n> > +\tgit show-ref >refs &&\n> > +\tnrefs=$(wc -l \n> Yup, I've seen that workaround for macs too many times and it should\n> work well.\n\nI switched to this style in the re-roll (and ditto for the path output,\nwhich actually turned up a bug).\n\n-Peff\n"},{"id":"400317","messageId":"20200622213813.GC1376217@coredump.intra.peff.net","threadId":"53716","inReplyTo":"xmqqr1uahn9t.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 3/3] fast-export: allow dumping the path mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-22T21:38:13Z","receivedAt":"2020-06-22T21:38:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 19, 2020 at 12:24:30PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > When working with an anonymized repo, it can be useful to be able to\n> > refer to particular paths. E.g., reproducing a bug with \"git rev-list --\n> > foo.c\" in the original repo would need to replace \"foo.c\" with its\n> > anonymized counterpart to produce the same effect.\n> \n> That would not work with \"git rev-list -- 'foo*'\", where there is no\n> counterpart, no?\n\nCorrect. There's no way that can work without treating the \"foo\" prefix\nas a token and anonymizing it consistently. We currently only treat full\npath components as tokens. So \"foo/\" will always be \"ref123/\", but\n\"foobar\" will have no relation.\n\n> It almost feels as if we want a separate knob to disable\n> anonymization for paths and perhaps refs while still anonymizing the\n> blob contents, or something like that.\n\nI think such a knob could be useful for some situations, but I don't\nthink it's a complete replacement for a mapping dump, since you may not\nwant to reveal your pathnames (or refnames). Unlike refnames, where I\ncan easily point a tag \"foo\" at a commit of interest and then ask that\n\"foo\" not be anonymized, pathnames are harder to change.\n\nAn alternative would be to let users seed the mapping (e.g., to ask that\n\"foo1\" and \"foo2\" become \"anon1\" and \"anon2\"). That requires a little\nmore forethought on the part of the user, but it also avoids them having\nto dig through the mapping to rewrite their commands (instead they use\nthe mappings they already invented).\n\n-Peff\n"},{"id":"400318","messageId":"20200622214745.GA3302779@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200619132304.GA2540657@coredump.intra.peff.net","subject":"[PATCH v2 0/4] fast-export: allow dumping anonymization mappings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-22T21:47:45Z","receivedAt":"2020-06-22T21:47:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 19, 2020 at 09:23:04AM -0400, Jeff King wrote:\n\n> This series gives an alternate way to achieve the same effect, but much\n> better in that it works for _any_ ref (so if you are trying to reproduce\n> the effect of \"rev-list origin/foo..bar\" in the anonymized repo, you can\n> easily do so). Ditto for paths, so that \"rev-list -- foo.c\" can be\n> reproduced in the anonymized repo.\n\nHere's a v2 which I think addresses all of the comments. I have to admit\nthat after writing my last email to Junio, I am wondering whether it\nwould be sufficient and simpler to let the user specify a static mapping\nof tokens (that could just be applied anywhere).\n\nI'll take a look at that, but since I worked up this version, here it is\nin the meantime.\n\nThe interesting changes are:\n\n - path output is now quoted, making it unambiguous. The intent is for\n humans to look at it, but it's not much extra work to make it\n machine readable, too.\n\n - the path dumping was in the wrong spot. It was happening in the\n generic function that's used for \"path-like\" things, including\n refnames. So the path mapping dump had extra cruft in it.\n\n - got rid of the maybe_dump_anon() helper\n\n - tests now avoid hard-coding expected counts\n\n - the path-dump test now checks the expected count\n\n [1/4]: fast-export: allow dumping the refname mapping\n [2/4]: fast-export: anonymize \"master\" refname\n [3/4]: fast-export: refactor path printing to not rely on stdout\n [4/4]: fast-export: allow dumping the path mapping\n\n Documentation/git-fast-export.txt | 34 +++++++++++++++\n builtin/fast-export.c | 69 +++++++++++++++++++++++++------\n t/t9351-fast-export-anonymize.sh | 44 ++++++++++++++++----\n 3 files changed, 125 insertions(+), 22 deletions(-)\n\nRange-diff from v1:\n\n1: 82a17ae976 ! 1: 7ba5582d66 fast-export: allow dumping the refname mapping\n @@ builtin/fast-export.c: static int has_unshown_parent(struct commit *commit)\n +\tkh_put_strset(seen->set, xstrdup(str), &hashret);\n +\treturn 0;\n +}\n -+\n -+static void maybe_dump_anon(FILE *out, struct seen_set *seen,\n -+\t\t\t const char *orig, const char *anon)\n -+{\n -+\tif (!out)\n -+\t\treturn;\n -+\tif (!check_and_mark_seen(seen, orig))\n -+\t\tfprintf(out, \"%s %s\\n\", orig, anon);\n -+}\n +\n struct anonymized_entry {\n \tstruct hashmap_entry hash;\n @@ builtin/fast-export.c: static const char *anonymize_refname(const char *refname)\n \t}\n \n \tanonymize_path(&anon, refname, &refs, anonymize_ref_component);\n -+\tmaybe_dump_anon(anonymized_refnames_handle, &seen,\n ++\n ++\tif (anonymized_refnames_handle &&\n ++\t !check_and_mark_seen(&seen, full_refname))\n ++\t\tfprintf(anonymized_refnames_handle, \"%s %s\\n\",\n +\t\t\tfull_refname, anon.buf);\n ++\n \treturn anon.buf;\n }\n \n @@ t/t9351-fast-export-anonymize.sh: test_expect_success 'stream omits tag message'\n +\t# we make no guarantees of the exact anonymized names,\n +\t# so just check that we have the right number and\n +\t# that a sample line looks sane.\n ++\texpected_count=$(git for-each-ref | wc -l) &&\n +\t# Note that master is not anonymized, and so not included\n +\t# in the mapping.\n -+\ttest_line_count = 6 refs.out &&\n ++\texpected_count=$((expected_count - 1)) &&\n ++\ttest_line_count = $expected_count refs.out &&\n +\tgrep \"^refs/heads/other refs/heads/\" refs.out\n +'\n +\n2: be56b375cc ! 2: d88f7c83a5 fast-export: anonymize \"master\" refname\n @@ t/t9351-fast-export-anonymize.sh: test_expect_success 'stream omits path names'\n \t! grep mytag stream\n '\n @@ t/t9351-fast-export-anonymize.sh: test_expect_success 'refname mapping can be dumped' '\n - \t# we make no guarantees of the exact anonymized names,\n \t# so just check that we have the right number and\n \t# that a sample line looks sane.\n + \texpected_count=$(git for-each-ref | wc -l) &&\n -\t# Note that master is not anonymized, and so not included\n -\t# in the mapping.\n --\ttest_line_count = 6 refs.out &&\n -+\ttest_line_count = 7 refs.out &&\n +-\texpected_count=$((expected_count - 1)) &&\n + \ttest_line_count = $expected_count refs.out &&\n \tgrep \"^refs/heads/other refs/heads/\" refs.out\n '\n - \n @@ t/t9351-fast-export-anonymize.sh: test_expect_success 'import stream to new repository' '\n test_expect_success 'result has two branches' '\n \tgit for-each-ref --format=\"%(refname)\" refs/heads >branches &&\n-: ---------- > 3: 164f1e1eab fast-export: refactor path printing to not rely on stdout\n3: a4e9f1f2ac ! 4: b0aa59f07e fast-export: allow dumping the path mapping\n @@ Commit message\n \n We recently taught fast-export to dump the refname mapping. Let's do the\n same thing for paths, which can reuse most of the same infrastructure.\n - Note that the output format isn't unambiguous here (because paths could\n - contain spaces). That's OK because this is meant to be examined by a\n - human.\n \n We could also just introduce a \"dump mapping\" file that shows every\n mapping we make. But it would be a bit more awkward to work with, as the\n @@ Documentation/git-fast-export.txt: by keeping the marks the same across runs.\n +\tOutput the mapping of real paths to anonymized paths to .\n +\tThe output will contain one line per path that appears in the\n +\toutput stream, with the original path, a space, and its\n -+\tanonymized counterpart. See the section on `ANONYMIZING` below.\n ++\tanonymized counterpart. Paths may be quoted if they contain a\n ++\tspace, or unusual characters; see `core.quotePath` in\n ++\tlinkgit:git-config(1). See also `ANONYMIZING` below.\n +\n --reference-excluded-parents::\n \tBy default, running a command such as `git fast-export\n @@ builtin/fast-export.c: static struct string_list tag_refs = STRING_LIST_INIT_NOD\n static struct revision_sources revision_sources;\n \n static int parse_opt_signed_tag_mode(const struct option *opt,\n -@@ builtin/fast-export.c: static void anonymize_path(struct strbuf *out, const char *path,\n - \t\t\t struct hashmap *map,\n - \t\t\t void *(*generate)(const void *, size_t *))\n - {\n -+\tstatic struct seen_set seen;\n -+\tconst char *full_path = path;\n +@@ builtin/fast-export.c: static void print_path(const char *path)\n + \t\tprint_path_1(stdout, path);\n + \telse {\n + \t\tstatic struct hashmap paths;\n ++\t\tstatic struct seen_set seen;\n + \t\tstatic struct strbuf anon = STRBUF_INIT;\n + \n + \t\tanonymize_path(&anon, path, &paths, anonymize_path_component);\n ++\t\tif (anonymized_paths_handle &&\n ++\t\t !check_and_mark_seen(&seen, path)) {\n ++\t\t\tprint_path_1(anonymized_paths_handle, path);\n ++\t\t\tfputc(' ', anonymized_paths_handle);\n ++\t\t\tprint_path_1(anonymized_paths_handle, anon.buf);\n ++\t\t\tfputc('\\n', anonymized_paths_handle);\n ++\t\t}\n +\n - \twhile (*path) {\n - \t\tconst char *end_of_component = strchrnul(path, '/');\n - \t\tsize_t len = end_of_component - path;\n -@@ builtin/fast-export.c: static void anonymize_path(struct strbuf *out, const char *path,\n - \t\tif (*path)\n - \t\t\tstrbuf_addch(out, *path++);\n + \t\tprint_path_1(stdout, anon.buf);\n + \t\tstrbuf_reset(&anon);\n \t}\n -+\n -+\tmaybe_dump_anon(anonymized_paths_handle, &seen, full_path, out->buf);\n - }\n - \n - static inline void *mark_to_ptr(uint32_t mark)\n @@ builtin/fast-export.c: int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t *import_filename = NULL,\n \t *import_filename_if_exists = NULL;\n @@ builtin/fast-export.c: int cmd_fast_export(int argc, const char **argv, const ch\n \t\tprintf(\"feature done\\n\");\n \n ## t/t9351-fast-export-anonymize.sh ##\n +@@ t/t9351-fast-export-anonymize.sh: test_expect_success 'setup simple repo' '\n + \tgit checkout -b other HEAD^ &&\n + \tmkdir subdir &&\n + \ttest_commit subdir/bar &&\n +-\ttest_commit subdir/xyzzy &&\n ++\ttest_commit quoting \"subdir/this needs quoting\" &&\n + \tgit tag -m \"annotated tag\" mytag\n + '\n + \n +@@ t/t9351-fast-export-anonymize.sh: test_expect_success 'stream omits path names' '\n + \t! grep foo stream &&\n + \t! grep subdir stream &&\n + \t! grep bar stream &&\n +-\t! grep xyzzy stream\n ++\t! grep quoting stream\n + '\n + \n + test_expect_success 'stream omits refnames' '\n @@ t/t9351-fast-export-anonymize.sh: test_expect_success 'refname mapping can be dumped' '\n \tgrep \"^refs/heads/other refs/heads/\" refs.out\n '\n \n +test_expect_success 'path mapping can be dumped' '\n +\tgit fast-export --anonymize --all \\\n +\t\t--dump-anonymized-paths=paths.out >/dev/null &&\n -+\t# do not assume a particular anonymization scheme or order;\n -+\t# just sanity check that a sample line looks sensible.\n -+\tgrep \"^foo \" paths.out\n ++\t# as above, avoid depending on the exact scheme, but\n ++\t# but check that we have the right number of mappings,\n ++\t# and spot-check one sample.\n ++\texpected_count=$(\n ++\t\tgit rev-list --objects --all |\n ++\t\tgit cat-file --batch-check=\"%(objecttype) %(rest)\" |\n ++\t\tsed -ne \"s/^blob //p\" |\n ++\t\tsort -u |\n ++\t\twc -l\n ++\t) &&\n ++\ttest_line_count = $expected_count paths.out &&\n ++\tgrep \"^\\\"subdir/this needs quoting\\\" \" paths.out\n +'\n +\n # NOTE: we chdir to the new, anonymized repository\n"},{"id":"400319","messageId":"20200622214759.GA3303964@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200622214745.GA3302779@coredump.intra.peff.net","subject":"[PATCH v2 1/4] fast-export: allow dumping the refname mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-22T21:47:59Z","receivedAt":"2020-06-22T21:48:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"After you anonymize a repository, it can be hard to find which commits\ncorrespond between the original and the result, and thus hard to\nreproduce commands that triggered bugs in the original.\n\nLet's make it possible to dump the mapping separate from the output\nstream. This can be used by a bug reporter to modify their reproduction\nrecipe without revealing the original names (see the example in the\ndocumentation).\n\nThe implementation is slightly non-obvious. There's no point in the\nprogram where we know the complete set of refs we're going to anonymize.\nNor do we have a complete set of anonymized refs after finishing (we\nhave a set of anonymized ref path components, but no knowledge of how\nthose are assembled into complete refs). So we lazily write to the dump\nfile as we anonymize each name, and keep a list of ones that we've\noutput in order to avoid duplicates.\n\nSome possible alternatives:\n\n - we could just output the mapping of anonymized components (e.g.,\n that \"foo\" became \"ref123\"). That works OK when you have short\n refnames (e.g., \"refs/heads/foo\" becomes \"refs/heads/ref123\"), but\n longer names would require the user to look up each component to\n assemble the result. For example, \"refs/remotes/origin/jk/foo\" might\n become \"refs/remotes/refs37/refs56/refs102\".\n\n - instead of dumping the mapping, the same problem could be solved by\n allowing the user to leave some refs alone. So if you want to\n reproduce \"git rev-list branch~17..HEAD\" in the anonymized repo, we\n could allow something like:\n\n git tag anon-one branch\n git tag anon-two HEAD\n git fast-export --anonymize --all \\\n --no-anonymize-ref=anon-one \\\n\t\t --no-anonymize-ref=anon-two \\\n\t\t >stream\n\n and then presumably \"git rev-list anon-one~17..anon-two\" would\n behave the same in the re-imported repository. This is more\n convenient in some ways, but it does require modifying the\n original repository. And the concept doesn't easily extend to\n other fields (e.g., pathnames, which will be addressed in a\n subsequent patch).\n\n - we could dump before/after commit hashes; combined with rev-parse,\n that could convert these cases (as well as ones using raw hashes).\n But we don't actually know the anonymized commit hashes; we're just\n generating a stream that will produce them in the anonymized repo.\n\n - likewise, we probably could insert object names or other markers\n into commit messages, blob contents, etc, in order to let a user\n with the original repo figure out which parts correspond. But using\n this gets complicated (I have to find my commits in the result with\n \"git log --all --grep\" or similar). It also makes it less clear that\n the anonymized repo didn't leak any information (because we are\n relying on object ids being unguessable).\n\nSigned-off-by: Jeff King \n---\n Documentation/git-fast-export.txt | 22 ++++++++++++++++++++\n builtin/fast-export.c | 34 +++++++++++++++++++++++++++++++\n t/t9351-fast-export-anonymize.sh | 14 +++++++++++++\n 3 files changed, 70 insertions(+)\n\ndiff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\nindex e8950de3ba..e809bb3f18 100644\n--- a/Documentation/git-fast-export.txt\n+++ b/Documentation/git-fast-export.txt\n@@ -119,6 +119,12 @@ by keeping the marks the same across runs.\n \tthe shape of the history and stored tree. See the section on\n \t`ANONYMIZING` below.\n \n+--dump-anonymized-refnames=::\n+\tOutput the mapping of real refnames to anonymized refnames to\n+\t. The output will contain one line per ref that appears in\n+\tthe output stream, with the original refname, a space, and its\n+\tanonymized counterpart. See the section on `ANONYMIZING` below.\n+\n --reference-excluded-parents::\n \tBy default, running a command such as `git fast-export\n \tmaster~5..master` will not include the commit master{tilde}5\n@@ -238,6 +244,22 @@ collapse \"User 0\", \"User 1\", etc into \"User X\"). This produces a much\n smaller output, and it is usually easy to quickly confirm that there is\n no private data in the stream.\n \n+Reproducing some bugs may require referencing particular commits, which\n+becomes challenging after the refnames have all been anonymized. You can\n+use `--dump-anonymized-refnames` to output the mapping, and then alter\n+your reproduction recipe to use the anonymized names. E.g., if you find\n+a bug with `git rev-list v1.0..v2.0` in the private repository, you can\n+run:\n+\n+---------------------------------------------------\n+$ git fast-export --anonymize --all --dump-anonymized-refnames=refs.out >stream\n+$ grep '^refs/tags/v[12].0' refs.out\n+refs/tags/v1.0 refs/tags/ref31\n+refs/tags/v2.0 refs/tags/ref50\n+---------------------------------------------------\n+\n+which tells you that `git rev-list ref31..ref50` may produce the same\n+bug in the re-imported anonymous repository.\n \n LIMITATIONS\n -----------\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 85868162ee..844726d45a 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -24,6 +24,7 @@\n #include \"remote.h\"\n #include \"blob.h\"\n #include \"commit-slab.h\"\n+#include \"khash.h\"\n \n static const char *fast_export_usage[] = {\n \tN_(\"git fast-export [rev-list-opts]\"),\n@@ -45,6 +46,7 @@ static struct string_list extra_refs = STRING_LIST_INIT_NODUP;\n static struct string_list tag_refs = STRING_LIST_INIT_NODUP;\n static struct refspec refspecs = REFSPEC_INIT_FETCH;\n static int anonymize;\n+static FILE *anonymized_refnames_handle;\n static struct revision_sources revision_sources;\n \n static int parse_opt_signed_tag_mode(const struct option *opt,\n@@ -118,6 +120,23 @@ static int has_unshown_parent(struct commit *commit)\n \treturn 0;\n }\n \n+KHASH_INIT(strset, const char *, int, 0, kh_str_hash_func, kh_str_hash_equal);\n+\n+struct seen_set {\n+\tkh_strset_t *set;\n+};\n+\n+static int check_and_mark_seen(struct seen_set *seen, const char *str)\n+{\n+\tint hashret;\n+\tif (!seen->set)\n+\t\tseen->set = kh_init_strset();\n+\tif (kh_get_strset(seen->set, str) < kh_end(seen->set))\n+\t\treturn 1;\n+\tkh_put_strset(seen->set, xstrdup(str), &hashret);\n+\treturn 0;\n+}\n+\n struct anonymized_entry {\n \tstruct hashmap_entry hash;\n \tconst char *orig;\n@@ -515,6 +534,8 @@ static const char *anonymize_refname(const char *refname)\n \t};\n \tstatic struct hashmap refs;\n \tstatic struct strbuf anon = STRBUF_INIT;\n+\tstatic struct seen_set seen;\n+\tconst char *full_refname = refname;\n \tint i;\n \n \t/*\n@@ -533,6 +554,12 @@ static const char *anonymize_refname(const char *refname)\n \t}\n \n \tanonymize_path(&anon, refname, &refs, anonymize_ref_component);\n+\n+\tif (anonymized_refnames_handle &&\n+\t !check_and_mark_seen(&seen, full_refname))\n+\t\tfprintf(anonymized_refnames_handle, \"%s %s\\n\",\n+\t\t\tfull_refname, anon.buf);\n+\n \treturn anon.buf;\n }\n \n@@ -1144,6 +1171,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \tchar *export_filename = NULL,\n \t *import_filename = NULL,\n \t *import_filename_if_exists = NULL;\n+\tconst char *anonymized_refnames_file = NULL;\n \tuint32_t lastimportid;\n \tstruct string_list refspecs_list = STRING_LIST_INIT_NODUP;\n \tstruct string_list paths_of_changed_objects = STRING_LIST_INIT_DUP;\n@@ -1177,6 +1205,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING_LIST(0, \"refspec\", &refspecs_list, N_(\"refspec\"),\n \t\t\t N_(\"Apply refspec to exported refs\")),\n \t\tOPT_BOOL(0, \"anonymize\", &anonymize, N_(\"anonymize output\")),\n+\t\tOPT_STRING(0, \"dump-anonymized-refnames\",\n+\t\t\t &anonymized_refnames_file, N_(\"file\"),\n+\t\t\t N_(\"output anonymized refname mapping to \")),\n \t\tOPT_BOOL(0, \"reference-excluded-parents\",\n \t\t\t &reference_excluded_commits, N_(\"Reference parents which are not in fast-export stream by object id\")),\n \t\tOPT_BOOL(0, \"show-original-ids\", &show_original_ids,\n@@ -1213,6 +1244,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\tstring_list_clear(&refspecs_list, 1);\n \t}\n \n+\tif (anonymized_refnames_file)\n+\t\tanonymized_refnames_handle = xfopen(anonymized_refnames_file, \"w\");\n+\n \tif (use_done_feature)\n \t\tprintf(\"feature done\\n\");\n \ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex 897dc50907..75cbc7b329 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -46,6 +46,20 @@ test_expect_success 'stream omits tag message' '\n \t! grep \"annotated tag\" stream\n '\n \n+test_expect_success 'refname mapping can be dumped' '\n+\tgit fast-export --anonymize --all \\\n+\t\t--dump-anonymized-refnames=refs.out >/dev/null &&\n+\t# we make no guarantees of the exact anonymized names,\n+\t# so just check that we have the right number and\n+\t# that a sample line looks sane.\n+\texpected_count=$(git for-each-ref | wc -l) &&\n+\t# Note that master is not anonymized, and so not included\n+\t# in the mapping.\n+\texpected_count=$((expected_count - 1)) &&\n+\ttest_line_count = $expected_count refs.out &&\n+\tgrep \"^refs/heads/other refs/heads/\" refs.out\n+'\n+\n # NOTE: we chdir to the new, anonymized repository\n # after this. All further tests should assume this.\n test_expect_success 'import stream to new repository' '\n-- \n2.27.0.517.gbc32778fa3\n\n"},{"id":"400320","messageId":"20200622214802.GB3303964@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200622214745.GA3302779@coredump.intra.peff.net","subject":"[PATCH v2 2/4] fast-export: anonymize \"master\" refname","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-22T21:48:02Z","receivedAt":"2020-06-22T21:48:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Running \"fast-export --anonymize\" will leave \"refs/heads/master\"\nuntouched in the output, for two reasons:\n\n - it helped to have some known reference point between the original\n and anonymized repository\n\n - since it's historically the default branch name, it doesn't leak any\n information\n\nNow that we can ask fast-export to dump the anonymized ref mapping, we\nhave a much better tool for the first one (because it works for _any_\nref, not just master).\n\nFor the second, the notion of \"default branch name\" is likely to become\nconfigurable soon, at which point the name _does_ leak information.\nLet's drop this special case in preparation.\n\nNote that we have to adjust the test a bit, since it relied on using the\nname \"master\" in the anonymized repos. But this gives us a good\nopportunity to further test the new dumping feature.\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 7 -------\n t/t9351-fast-export-anonymize.sh | 15 +++++----------\n 2 files changed, 5 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 844726d45a..faaab6c7e9 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -538,13 +538,6 @@ static const char *anonymize_refname(const char *refname)\n \tconst char *full_refname = refname;\n \tint i;\n \n-\t/*\n-\t * We also leave \"master\" as a special case, since it does not reveal\n-\t * anything interesting.\n-\t */\n-\tif (!strcmp(refname, \"refs/heads/master\"))\n-\t\treturn refname;\n-\n \tstrbuf_reset(&anon);\n \tfor (i = 0; i < ARRAY_SIZE(prefixes); i++) {\n \t\tif (skip_prefix(refname, prefixes[i], &refname)) {\ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex 75cbc7b329..c726306c4d 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -26,11 +26,8 @@ test_expect_success 'stream omits path names' '\n \t! grep xyzzy stream\n '\n \n-test_expect_success 'stream allows master as refname' '\n-\tgrep master stream\n-'\n-\n-test_expect_success 'stream omits other refnames' '\n+test_expect_success 'stream omits refnames' '\n+\t! grep master stream &&\n \t! grep other stream &&\n \t! grep mytag stream\n '\n@@ -53,9 +50,6 @@ test_expect_success 'refname mapping can be dumped' '\n \t# so just check that we have the right number and\n \t# that a sample line looks sane.\n \texpected_count=$(git for-each-ref | wc -l) &&\n-\t# Note that master is not anonymized, and so not included\n-\t# in the mapping.\n-\texpected_count=$((expected_count - 1)) &&\n \ttest_line_count = $expected_count refs.out &&\n \tgrep \"^refs/heads/other refs/heads/\" refs.out\n '\n@@ -71,15 +65,16 @@ test_expect_success 'import stream to new repository' '\n test_expect_success 'result has two branches' '\n \tgit for-each-ref --format=\"%(refname)\" refs/heads >branches &&\n \ttest_line_count = 2 branches &&\n-\tother_branch=$(grep -v refs/heads/master branches)\n+\tmain_branch=$(sed -ne \"s,refs/heads/master ,,p\" ../refs.out) &&\n+\tother_branch=$(sed -ne \"s,refs/heads/other ,,p\" ../refs.out)\n '\n \n test_expect_success 'repo has original shape and timestamps' '\n \tshape () {\n \t\tgit log --format=\"%m %ct\" --left-right --boundary \"$@\"\n \t} &&\n \t(cd .. && shape master...other) >expect &&\n-\tshape master...$other_branch >actual &&\n+\tshape $main_branch...$other_branch >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.27.0.517.gbc32778fa3\n\n"},{"id":"400321","messageId":"20200622214805.GC3303964@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200622214745.GA3302779@coredump.intra.peff.net","subject":"[PATCH v2 3/4] fast-export: refactor path printing to not rely on stdout","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-22T21:48:05Z","receivedAt":"2020-06-22T21:48:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We'll be using print_path_1() in more places in a subsequent patch, so\nlet's teach it to take the output handle as a parameter.\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex faaab6c7e9..aa7ac9761d 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -369,15 +369,15 @@ static int depth_first(const void *a_, const void *b_)\n \treturn (a->status == 'R') - (b->status == 'R');\n }\n \n-static void print_path_1(const char *path)\n+static void print_path_1(FILE *out, const char *path)\n {\n \tint need_quote = quote_c_style(path, NULL, NULL, 0);\n \tif (need_quote)\n-\t\tquote_c_style(path, NULL, stdout, 0);\n+\t\tquote_c_style(path, NULL, out, 0);\n \telse if (strchr(path, ' '))\n-\t\tprintf(\"\\\"%s\\\"\", path);\n+\t\tfprintf(out, \"\\\"%s\\\"\", path);\n \telse\n-\t\tprintf(\"%s\", path);\n+\t\tfprintf(out, \"%s\", path);\n }\n \n static void *anonymize_path_component(const void *path, size_t *len)\n@@ -391,13 +391,13 @@ static void *anonymize_path_component(const void *path, size_t *len)\n static void print_path(const char *path)\n {\n \tif (!anonymize)\n-\t\tprint_path_1(path);\n+\t\tprint_path_1(stdout, path);\n \telse {\n \t\tstatic struct hashmap paths;\n \t\tstatic struct strbuf anon = STRBUF_INIT;\n \n \t\tanonymize_path(&anon, path, &paths, anonymize_path_component);\n-\t\tprint_path_1(anon.buf);\n+\t\tprint_path_1(stdout, anon.buf);\n \t\tstrbuf_reset(&anon);\n \t}\n }\n-- \n2.27.0.517.gbc32778fa3\n\n"},{"id":"400322","messageId":"20200622214809.GD3303964@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200622214745.GA3302779@coredump.intra.peff.net","subject":"[PATCH v2 4/4] fast-export: allow dumping the path mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-22T21:48:09Z","receivedAt":"2020-06-22T21:48:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When working with an anonymized repo, it can be useful to be able to\nrefer to particular paths. E.g., reproducing a bug with \"git rev-list --\nfoo.c\" in the original repo would need to replace \"foo.c\" with its\nanonymized counterpart to produce the same effect.\n\nWe recently taught fast-export to dump the refname mapping. Let's do the\nsame thing for paths, which can reuse most of the same infrastructure.\n\nWe could also just introduce a \"dump mapping\" file that shows every\nmapping we make. But it would be a bit more awkward to work with, as the\nuser would have to sort through more data to find the parts they're\ninterested in (and there are likely to be many more paths than refnames,\nmaking it annoying for people who just want to dump the refnames).\n\nSigned-off-by: Jeff King \n---\n Documentation/git-fast-export.txt | 12 ++++++++++++\n builtin/fast-export.c | 16 ++++++++++++++++\n t/t9351-fast-export-anonymize.sh | 21 +++++++++++++++++++--\n 3 files changed, 47 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\nindex e809bb3f18..342e34fd89 100644\n--- a/Documentation/git-fast-export.txt\n+++ b/Documentation/git-fast-export.txt\n@@ -125,6 +125,14 @@ by keeping the marks the same across runs.\n \tthe output stream, with the original refname, a space, and its\n \tanonymized counterpart. See the section on `ANONYMIZING` below.\n \n+--dump-anonymized-paths=::\n+\tOutput the mapping of real paths to anonymized paths to .\n+\tThe output will contain one line per path that appears in the\n+\toutput stream, with the original path, a space, and its\n+\tanonymized counterpart. Paths may be quoted if they contain a\n+\tspace, or unusual characters; see `core.quotePath` in\n+\tlinkgit:git-config(1). See also `ANONYMIZING` below.\n+\n --reference-excluded-parents::\n \tBy default, running a command such as `git fast-export\n \tmaster~5..master` will not include the commit master{tilde}5\n@@ -261,6 +269,10 @@ refs/tags/v2.0 refs/tags/ref50\n which tells you that `git rev-list ref31..ref50` may produce the same\n bug in the re-imported anonymous repository.\n \n+Likewise, `--dump-anonymized-paths` may be useful for a bug that\n+involves pathspecs. E.g., `git rev-list v1.0..v2.0 -- foo.c` requires\n+knowing the path corresponding to `foo.c` in the result.\n+\n LIMITATIONS\n -----------\n \ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex aa7ac9761d..080ded92e4 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -47,6 +47,7 @@ static struct string_list tag_refs = STRING_LIST_INIT_NODUP;\n static struct refspec refspecs = REFSPEC_INIT_FETCH;\n static int anonymize;\n static FILE *anonymized_refnames_handle;\n+static FILE *anonymized_paths_handle;\n static struct revision_sources revision_sources;\n \n static int parse_opt_signed_tag_mode(const struct option *opt,\n@@ -394,9 +395,18 @@ static void print_path(const char *path)\n \t\tprint_path_1(stdout, path);\n \telse {\n \t\tstatic struct hashmap paths;\n+\t\tstatic struct seen_set seen;\n \t\tstatic struct strbuf anon = STRBUF_INIT;\n \n \t\tanonymize_path(&anon, path, &paths, anonymize_path_component);\n+\t\tif (anonymized_paths_handle &&\n+\t\t !check_and_mark_seen(&seen, path)) {\n+\t\t\tprint_path_1(anonymized_paths_handle, path);\n+\t\t\tfputc(' ', anonymized_paths_handle);\n+\t\t\tprint_path_1(anonymized_paths_handle, anon.buf);\n+\t\t\tfputc('\\n', anonymized_paths_handle);\n+\t\t}\n+\n \t\tprint_path_1(stdout, anon.buf);\n \t\tstrbuf_reset(&anon);\n \t}\n@@ -1165,6 +1175,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t *import_filename = NULL,\n \t *import_filename_if_exists = NULL;\n \tconst char *anonymized_refnames_file = NULL;\n+\tconst char *anonymized_paths_file = NULL;\n \tuint32_t lastimportid;\n \tstruct string_list refspecs_list = STRING_LIST_INIT_NODUP;\n \tstruct string_list paths_of_changed_objects = STRING_LIST_INIT_DUP;\n@@ -1201,6 +1212,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"dump-anonymized-refnames\",\n \t\t\t &anonymized_refnames_file, N_(\"file\"),\n \t\t\t N_(\"output anonymized refname mapping to \")),\n+\t\tOPT_STRING(0, \"dump-anonymized-paths\",\n+\t\t\t &anonymized_paths_file, N_(\"file\"),\n+\t\t\t N_(\"output anonymized path mapping to \")),\n \t\tOPT_BOOL(0, \"reference-excluded-parents\",\n \t\t\t &reference_excluded_commits, N_(\"Reference parents which are not in fast-export stream by object id\")),\n \t\tOPT_BOOL(0, \"show-original-ids\", &show_original_ids,\n@@ -1239,6 +1253,8 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \n \tif (anonymized_refnames_file)\n \t\tanonymized_refnames_handle = xfopen(anonymized_refnames_file, \"w\");\n+\tif (anonymized_paths_file)\n+\t\tanonymized_paths_handle = xfopen(anonymized_paths_file, \"w\");\n \n \tif (use_done_feature)\n \t\tprintf(\"feature done\\n\");\ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex c726306c4d..ef72b1ec95 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -9,7 +9,7 @@ test_expect_success 'setup simple repo' '\n \tgit checkout -b other HEAD^ &&\n \tmkdir subdir &&\n \ttest_commit subdir/bar &&\n-\ttest_commit subdir/xyzzy &&\n+\ttest_commit quoting \"subdir/this needs quoting\" &&\n \tgit tag -m \"annotated tag\" mytag\n '\n \n@@ -23,7 +23,7 @@ test_expect_success 'stream omits path names' '\n \t! grep foo stream &&\n \t! grep subdir stream &&\n \t! grep bar stream &&\n-\t! grep xyzzy stream\n+\t! grep quoting stream\n '\n \n test_expect_success 'stream omits refnames' '\n@@ -54,6 +54,23 @@ test_expect_success 'refname mapping can be dumped' '\n \tgrep \"^refs/heads/other refs/heads/\" refs.out\n '\n \n+test_expect_success 'path mapping can be dumped' '\n+\tgit fast-export --anonymize --all \\\n+\t\t--dump-anonymized-paths=paths.out >/dev/null &&\n+\t# as above, avoid depending on the exact scheme, but\n+\t# but check that we have the right number of mappings,\n+\t# and spot-check one sample.\n+\texpected_count=$(\n+\t\tgit rev-list --objects --all |\n+\t\tgit cat-file --batch-check=\"%(objecttype) %(rest)\" |\n+\t\tsed -ne \"s/^blob //p\" |\n+\t\tsort -u |\n+\t\twc -l\n+\t) &&\n+\ttest_line_count = $expected_count paths.out &&\n+\tgrep \"^\\\"subdir/this needs quoting\\\" \" paths.out\n+'\n+\n # NOTE: we chdir to the new, anonymized repository\n # after this. All further tests should assume this.\n test_expect_success 'import stream to new repository' '\n-- \n2.27.0.517.gbc32778fa3\n"},{"id":"400377","messageId":"20200623152436.GA50925@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200622214745.GA3302779@coredump.intra.peff.net","subject":"[alternative 0/10] fast-export: allow seeding the anonymized mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-23T15:24:36Z","receivedAt":"2020-06-23T15:24:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 22, 2020 at 05:47:46PM -0400, Jeff King wrote:\n\n> Here's a v2 which I think addresses all of the comments. I have to admit\n> that after writing my last email to Junio, I am wondering whether it\n> would be sufficient and simpler to let the user specify a static mapping\n> of tokens (that could just be applied anywhere).\n> \n> I'll take a look at that, but since I worked up this version, here it is\n> in the meantime.\n\nSo here's that alternative. I think the result is actually a bit nicer\nto work with, _and_ it's a lot less code. Don't let the number of\npatches fool you. Most of it is cleanup that would be worth doing even\nwithout the final patches.\n\nBoth of these techniques _could_ live side-by-side within fast-export,\nas they have slightly different strengths and weaknesses. But I'd prefer\nto just go with one (this one) in the name of simplicity, and I strongly\nsuspect nobody will ever ask for the other.\n\n [01/10]: t9351: derive anonymized tree checks from original repo\n [02/10]: fast-export: use xmemdupz() for anonymizing oids\n\n These first two are actually a bug-fix that I noticed while writing\n it.\n\n [03/10]: fast-export: store anonymized oids as hex strings\n [04/10]: fast-export: tighten anonymize_mem() interface to handle only strings\n [05/10]: fast-export: stop storing lengths in anonymized hashmaps\n [06/10]: fast-export: use a flex array to store anonymized entries\n [07/10]: fast-export: move global \"idents\" anonymize hashmap into function\n [08/10]: fast-export: add a \"data\" callback parameter to anonymize_str()\n\n This is all cleanup and prep. More than is strictly necessary for\n this series, but it does simplify things and reduce the memory\n footprint (only a few megabytes in git.git, but more in larger\n repos).\n\n [09/10]: fast-export: allow seeding the anonymized mapping\n\n And then this is the actual feature...\n\n [10/10]: fast-export: anonymize \"master\" refname\n\n ...which finally lets us drop the special name rule.\n\n Documentation/git-fast-export.txt | 24 +++++\n builtin/fast-export.c | 161 +++++++++++++++++++-----------\n t/t9351-fast-export-anonymize.sh | 54 +++++++---\n 3 files changed, 167 insertions(+), 72 deletions(-)\n\n"},{"id":"400378","messageId":"20200623152447.GA1435482@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200623152436.GA50925@coredump.intra.peff.net","subject":"[PATCH 01/10] t9351: derive anonymized tree checks from original repo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-23T15:24:47Z","receivedAt":"2020-06-23T15:24:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Our tests of the anonymized repo just hard-code the expected set of\nobjects in the root and subdirectory trees. This makes them brittle to\nthe test setup changing (e.g., adding new paths that need tested).\n\nLet's look at the original repo to compute our expected set of objects.\nNote that this isn't completely perfect (e.g., we still rely on there\nbeing only one tree in the root), but it does simplify later patches.\n\nSigned-off-by: Jeff King \n---\n t/t9351-fast-export-anonymize.sh | 16 ++++++----------\n 1 file changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex 897dc50907..e772cf9930 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -71,22 +71,18 @@ test_expect_success 'repo has original shape and timestamps' '\n \n test_expect_success 'root tree has original shape' '\n \t# the output entries are not necessarily in the same\n-\t# order, but we know at least that we will have one tree\n-\t# and one blob, so just check the sorted order\n-\tcat >expect <<-\\EOF &&\n-\tblob\n-\ttree\n-\tEOF\n+\t# order, but we should at least have the same set of\n+\t# object types.\n+\tgit -C .. ls-tree HEAD >orig-root &&\n+\tcut -d\" \" -f2 expect &&\n \tgit ls-tree $other_branch >root &&\n \tcut -d\" \" -f2 actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'paths in subdir ended up in one tree' '\n-\tcat >expect <<-\\EOF &&\n-\tblob\n-\tblob\n-\tEOF\n+\tgit -C .. ls-tree other:subdir >orig-subdir &&\n+\tcut -d\" \" -f2 expect &&\n \ttree=$(grep tree root | cut -f2) &&\n \tgit ls-tree $other_branch:$tree >tree &&\n \tcut -d\" \" -f2 actual &&\n-- \n2.27.0.517.gbc32778fa3\n\n"},{"id":"400379","messageId":"20200623152449.GB1435482@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200623152436.GA50925@coredump.intra.peff.net","subject":"[PATCH 02/10] fast-export: use xmemdupz() for anonymizing oids","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-23T15:24:49Z","receivedAt":"2020-06-23T15:24:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Our anonymize_mem() function is careful to take a ptr/len pair to allow\nstoring binary tokens like object ids, as well as partial strings (e.g.,\njust \"foo\" of \"foo/bar\"). But it duplicates the hash key using\nxstrdup()! That means that:\n\n - for a partial string, we'd store all bytes up to the NUL, even\n though we'd never look at anything past \"len\". This didn't produce\n wrong behavior, but was wasteful.\n\n - for a binary oid that doesn't contain a zero byte, we'd copy garbage\n bytes off the end of the array (though as long as nothing complained\n about reading uninitialized bytes, further reads would be limited by\n \"len\", and we'd produce the correct results)\n\n - for a binary oid that does contain a zero byte, we'd copy _fewer_\n bytes than intended into the hashmap struct. When we later try to\n look up a value, we'd access uninitialized memory and potentially\n falsely claim that a particular oid is not present.\n\nThe most common reason to store an oid is an anonymized gitlink, but our\ntest case doesn't have any gitlinks at all. So let's add one whose oid\ncontains a NUL and is present at two different paths. ASan catches the\nmemory error, but even without it we can detect the bug because the oid\nis not anonymized the same way for both paths.\n\nAnd of course the fix is to copy the correct number of bytes. We don't\ntechnically need the appended NUL from xmemdupz(), but it doesn't hurt\nas an extra protection against anybody treating it like a string (plus a\nfuture patch will push us more in that direction).\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 2 +-\n t/t9351-fast-export-anonymize.sh | 15 +++++++++++++++\n 2 files changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 85868162ee..289395a131 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -162,7 +162,7 @@ static const void *anonymize_mem(struct hashmap *map,\n \tif (!ret) {\n \t\tret = xmalloc(sizeof(*ret));\n \t\thashmap_entry_init(&ret->hash, key.hash.hash);\n-\t\tret->orig = xstrdup(orig);\n+\t\tret->orig = xmemdupz(orig, *len);\n \t\tret->orig_len = *len;\n \t\tret->anon = generate(orig, len);\n \t\tret->anon_len = *len;\ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex e772cf9930..dc5d75cd19 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -10,6 +10,10 @@ test_expect_success 'setup simple repo' '\n \tmkdir subdir &&\n \ttest_commit subdir/bar &&\n \ttest_commit subdir/xyzzy &&\n+\tfake_commit=$(echo $ZERO_OID | sed s/0/a/) &&\n+\tgit update-index --add --cacheinfo 160000,$fake_commit,link1 &&\n+\tgit update-index --add --cacheinfo 160000,$fake_commit,link2 &&\n+\tgit commit -m \"add gitlink\" &&\n \tgit tag -m \"annotated tag\" mytag\n '\n \n@@ -26,6 +30,12 @@ test_expect_success 'stream omits path names' '\n \t! grep xyzzy stream\n '\n \n+test_expect_success 'stream omits gitlink oids' '\n+\t# avoid relying on the whole oid to remain hash-agnostic; this is\n+\t# plenty to be unique within our test case\n+\t! grep a000000000000000000 stream\n+'\n+\n test_expect_success 'stream allows master as refname' '\n \tgrep master stream\n '\n@@ -89,6 +99,11 @@ test_expect_success 'paths in subdir ended up in one tree' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'identical gitlinks got identical oid' '\n+\tawk \"/commit/ { print \\$3 }\" commits &&\n+\ttest_line_count = 1 commits\n+'\n+\n test_expect_success 'tag points to branch tip' '\n \tgit rev-parse $other_branch >expect &&\n \tgit for-each-ref --format=\"%(*objectname)\" | grep . >actual &&\n-- \n2.27.0.517.gbc32778fa3\n\n"},{"id":"400380","messageId":"20200623152451.GC1435482@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200623152436.GA50925@coredump.intra.peff.net","subject":"[PATCH 03/10] fast-export: store anonymized oids as hex strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-23T15:24:51Z","receivedAt":"2020-06-23T15:24:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When fast-export stores anonymized oids, it does so as binary strings.\nAnd while the anonymous mapping storage is binary-clean (at least as of\nthe previous commit), this will become awkward when we start exposing\nmore of it to the user. In particular, if we allow a method for\nretaining token \"foo\", then users may want to specify a hex oid as such\na token.\n\nLet's just switch to storing the hex strings. The difference in memory\nusage is negligible (especially considering how infrequently we'd\ngenerally store an oid compared to, say, path components).\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 28 ++++++++++++++++------------\n 1 file changed, 16 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 289395a131..4a3a4c933e 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -387,16 +387,19 @@ static void *generate_fake_oid(const void *old, size_t *len)\n {\n \tstatic uint32_t counter = 1; /* avoid null oid */\n \tconst unsigned hashsz = the_hash_algo->rawsz;\n-\tunsigned char *out = xcalloc(hashsz, 1);\n-\tput_be32(out + hashsz - 4, counter++);\n-\treturn out;\n+\tstruct object_id oid;\n+\tchar *hex = xmallocz(GIT_MAX_HEXSZ);\n+\n+\toidclr(&oid);\n+\tput_be32(oid.hash + hashsz - 4, counter++);\n+\treturn oid_to_hex_r(hex, &oid);\n }\n \n-static const struct object_id *anonymize_oid(const struct object_id *oid)\n+static const char *anonymize_oid(const char *oid_hex)\n {\n \tstatic struct hashmap objs;\n-\tsize_t len = the_hash_algo->rawsz;\n-\treturn anonymize_mem(&objs, generate_fake_oid, oid, &len);\n+\tsize_t len = strlen(oid_hex);\n+\treturn anonymize_mem(&objs, generate_fake_oid, oid_hex, &len);\n }\n \n static void show_filemodify(struct diff_queue_struct *q,\n@@ -455,9 +458,9 @@ static void show_filemodify(struct diff_queue_struct *q,\n \t\t\t */\n \t\t\tif (no_data || S_ISGITLINK(spec->mode))\n \t\t\t\tprintf(\"M %06o %s \", spec->mode,\n-\t\t\t\t oid_to_hex(anonymize ?\n-\t\t\t\t\t\t anonymize_oid(&spec->oid) :\n-\t\t\t\t\t\t &spec->oid));\n+\t\t\t\t anonymize ?\n+\t\t\t\t anonymize_oid(oid_to_hex(&spec->oid)) :\n+\t\t\t\t oid_to_hex(&spec->oid));\n \t\t\telse {\n \t\t\t\tstruct object *object = lookup_object(the_repository,\n \t\t\t\t\t\t\t\t &spec->oid);\n@@ -712,9 +715,10 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \t\tif (mark)\n \t\t\tprintf(\":%d\\n\", mark);\n \t\telse\n-\t\t\tprintf(\"%s\\n\", oid_to_hex(anonymize ?\n-\t\t\t\t\t\t anonymize_oid(&obj->oid) :\n-\t\t\t\t\t\t &obj->oid));\n+\t\t\tprintf(\"%s\\n\",\n+\t\t\t anonymize ?\n+\t\t\t anonymize_oid(oid_to_hex(&obj->oid)) :\n+\t\t\t oid_to_hex(&obj->oid));\n \t\ti++;\n \t}\n \n-- \n2.27.0.517.gbc32778fa3\n\n"},{"id":"400381","messageId":"20200623152454.GD1435482@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200623152436.GA50925@coredump.intra.peff.net","subject":"[PATCH 04/10] fast-export: tighten anonymize_mem() interface to handle only strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-23T15:24:54Z","receivedAt":"2020-06-23T15:24:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"While the anonymize_mem() interface _can_ store arbitrary byte\nsequences, none of the callers uses this feature (as of the previous\ncommit). We'd like to keep it that way, as we'll be exposing the\nstring-like nature of the anonymization routines to the user. So let's\ntighten up the interface a bit:\n\n - don't treat \"len\" as an out-parameter from anonymize_mem(); this\n ensures callers treat the pointer result as a NUL-terminated string\n\n - likewise, don't treat \"len\" as an out-parameter from generator\n functions\n\n - swap out \"void *\" for \"char *\" as appropriate to signal that we\n don't handle arbitrary memory\n\n - rename the function to anonymize_str()\n\nThis will also open up some optimization opportunities in a future\npatch.\n\nNote that we can't drop the \"len\" parameter entirely. Some callers do\npass in partial strings (e.g., \"foo/bar\", len=3) to avoid copying, and\nwe need to handle those still.\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 53 +++++++++++++++++++++----------------------\n 1 file changed, 26 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 4a3a4c933e..d8ea067630 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -145,31 +145,30 @@ static int anonymized_entry_cmp(const void *unused_cmp_data,\n * the same anonymized string with another. The actual generation\n * is farmed out to the generate function.\n */\n-static const void *anonymize_mem(struct hashmap *map,\n-\t\t\t\t void *(*generate)(const void *, size_t *),\n-\t\t\t\t const void *orig, size_t *len)\n+static const char *anonymize_str(struct hashmap *map,\n+\t\t\t\t char *(*generate)(const char *, size_t),\n+\t\t\t\t const char *orig, size_t len)\n {\n \tstruct anonymized_entry key, *ret;\n \n \tif (!map->cmpfn)\n \t\thashmap_init(map, anonymized_entry_cmp, NULL, 0);\n \n-\thashmap_entry_init(&key.hash, memhash(orig, *len));\n+\thashmap_entry_init(&key.hash, memhash(orig, len));\n \tkey.orig = orig;\n-\tkey.orig_len = *len;\n+\tkey.orig_len = len;\n \tret = hashmap_get_entry(map, &key, hash, NULL);\n \n \tif (!ret) {\n \t\tret = xmalloc(sizeof(*ret));\n \t\thashmap_entry_init(&ret->hash, key.hash.hash);\n-\t\tret->orig = xmemdupz(orig, *len);\n-\t\tret->orig_len = *len;\n+\t\tret->orig = xmemdupz(orig, len);\n+\t\tret->orig_len = len;\n \t\tret->anon = generate(orig, len);\n-\t\tret->anon_len = *len;\n+\t\tret->anon_len = strlen(ret->anon);\n \t\thashmap_put(map, &ret->hash);\n \t}\n \n-\t*len = ret->anon_len;\n \treturn ret->anon;\n }\n \n@@ -181,13 +180,13 @@ static const void *anonymize_mem(struct hashmap *map,\n */\n static void anonymize_path(struct strbuf *out, const char *path,\n \t\t\t struct hashmap *map,\n-\t\t\t void *(*generate)(const void *, size_t *))\n+\t\t\t char *(*generate)(const char *, size_t))\n {\n \twhile (*path) {\n \t\tconst char *end_of_component = strchrnul(path, '/');\n \t\tsize_t len = end_of_component - path;\n-\t\tconst char *c = anonymize_mem(map, generate, path, &len);\n-\t\tstrbuf_add(out, c, len);\n+\t\tconst char *c = anonymize_str(map, generate, path, len);\n+\t\tstrbuf_addstr(out, c);\n \t\tpath = end_of_component;\n \t\tif (*path)\n \t\t\tstrbuf_addch(out, *path++);\n@@ -361,12 +360,12 @@ static void print_path_1(const char *path)\n \t\tprintf(\"%s\", path);\n }\n \n-static void *anonymize_path_component(const void *path, size_t *len)\n+static char *anonymize_path_component(const char *path, size_t len)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n \tstrbuf_addf(&out, \"path%d\", counter++);\n-\treturn strbuf_detach(&out, len);\n+\treturn strbuf_detach(&out, NULL);\n }\n \n static void print_path(const char *path)\n@@ -383,7 +382,7 @@ static void print_path(const char *path)\n \t}\n }\n \n-static void *generate_fake_oid(const void *old, size_t *len)\n+static char *generate_fake_oid(const char *old, size_t len)\n {\n \tstatic uint32_t counter = 1; /* avoid null oid */\n \tconst unsigned hashsz = the_hash_algo->rawsz;\n@@ -399,7 +398,7 @@ static const char *anonymize_oid(const char *oid_hex)\n {\n \tstatic struct hashmap objs;\n \tsize_t len = strlen(oid_hex);\n-\treturn anonymize_mem(&objs, generate_fake_oid, oid_hex, &len);\n+\treturn anonymize_str(&objs, generate_fake_oid, oid_hex, len);\n }\n \n static void show_filemodify(struct diff_queue_struct *q,\n@@ -496,12 +495,12 @@ static const char *find_encoding(const char *begin, const char *end)\n \treturn bol;\n }\n \n-static void *anonymize_ref_component(const void *old, size_t *len)\n+static char *anonymize_ref_component(const char *old, size_t len)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n \tstrbuf_addf(&out, \"ref%d\", counter++);\n-\treturn strbuf_detach(&out, len);\n+\treturn strbuf_detach(&out, NULL);\n }\n \n static const char *anonymize_refname(const char *refname)\n@@ -550,13 +549,13 @@ static char *anonymize_commit_message(const char *old)\n }\n \n static struct hashmap idents;\n-static void *anonymize_ident(const void *old, size_t *len)\n+static char *anonymize_ident(const char *old, size_t len)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n \tstrbuf_addf(&out, \"User %d \", counter, counter);\n \tcounter++;\n-\treturn strbuf_detach(&out, len);\n+\treturn strbuf_detach(&out, NULL);\n }\n \n /*\n@@ -591,9 +590,9 @@ static void anonymize_ident_line(const char **beg, const char **end)\n \t\tsize_t len;\n \n \t\tlen = split.mail_end - split.name_begin;\n-\t\tident = anonymize_mem(&idents, anonymize_ident,\n-\t\t\t\t split.name_begin, &len);\n-\t\tstrbuf_add(out, ident, len);\n+\t\tident = anonymize_str(&idents, anonymize_ident,\n+\t\t\t\t split.name_begin, len);\n+\t\tstrbuf_addstr(out, ident);\n \t\tstrbuf_addch(out, ' ');\n \t\tstrbuf_add(out, split.date_begin, split.tz_end - split.date_begin);\n \t} else {\n@@ -733,12 +732,12 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \tshow_progress();\n }\n \n-static void *anonymize_tag(const void *old, size_t *len)\n+static char *anonymize_tag(const char *old, size_t len)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n \tstrbuf_addf(&out, \"tag message %d\", counter++);\n-\treturn strbuf_detach(&out, len);\n+\treturn strbuf_detach(&out, NULL);\n }\n \n static void handle_tail(struct object_array *commits, struct rev_info *revs,\n@@ -808,8 +807,8 @@ static void handle_tag(const char *name, struct tag *tag)\n \t\tname = anonymize_refname(name);\n \t\tif (message) {\n \t\t\tstatic struct hashmap tags;\n-\t\t\tmessage = anonymize_mem(&tags, anonymize_tag,\n-\t\t\t\t\t\tmessage, &message_size);\n+\t\t\tmessage = anonymize_str(&tags, anonymize_tag,\n+\t\t\t\t\t\tmessage, message_size);\n \t\t}\n \t}\n \n-- \n2.27.0.517.gbc32778fa3\n\n"},{"id":"400382","messageId":"20200623152456.GE1435482@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200623152436.GA50925@coredump.intra.peff.net","subject":"[PATCH 05/10] fast-export: stop storing lengths in anonymized hashmaps","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-23T15:24:56Z","receivedAt":"2020-06-23T15:24:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Now that the anonymize_str() interface is restricted to NUL-terminated\nstrings, there's no need for us to keep track of the length of each\nentry in the hashmap. This simplifies the code and saves a bit of\nmemory.\n\nNote that we do still need to compare the stored results to partial\nstrings passed in by the callers. We can do that by using hashmap's\nkeydata feature to get the ptr/len pair into the comparison function,\nand then using strncmp().\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 28 ++++++++++++++++++----------\n 1 file changed, 18 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex d8ea067630..5df2ada47d 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -121,23 +121,32 @@ static int has_unshown_parent(struct commit *commit)\n struct anonymized_entry {\n \tstruct hashmap_entry hash;\n \tconst char *orig;\n-\tsize_t orig_len;\n \tconst char *anon;\n-\tsize_t anon_len;\n+};\n+\n+struct anonymized_entry_key {\n+\tstruct hashmap_entry hash;\n+\tconst char *orig;\n+\tsize_t orig_len;\n };\n \n static int anonymized_entry_cmp(const void *unused_cmp_data,\n \t\t\t\tconst struct hashmap_entry *eptr,\n \t\t\t\tconst struct hashmap_entry *entry_or_key,\n-\t\t\t\tconst void *unused_keydata)\n+\t\t\t\tconst void *keydata)\n {\n \tconst struct anonymized_entry *a, *b;\n \n \ta = container_of(eptr, const struct anonymized_entry, hash);\n-\tb = container_of(entry_or_key, const struct anonymized_entry, hash);\n+\tif (keydata) {\n+\t\tconst struct anonymized_entry_key *key = keydata;\n+\t\tint equal = !strncmp(a->orig, key->orig, key->orig_len) &&\n+\t\t\t !a->orig[key->orig_len];\n+\t\treturn !equal;\n+\t}\n \n-\treturn a->orig_len != b->orig_len ||\n-\t\tmemcmp(a->orig, b->orig, a->orig_len);\n+\tb = container_of(entry_or_key, const struct anonymized_entry, hash);\n+\treturn strcmp(a->orig, b->orig);\n }\n \n /*\n@@ -149,23 +158,22 @@ static const char *anonymize_str(struct hashmap *map,\n \t\t\t\t char *(*generate)(const char *, size_t),\n \t\t\t\t const char *orig, size_t len)\n {\n-\tstruct anonymized_entry key, *ret;\n+\tstruct anonymized_entry_key key;\n+\tstruct anonymized_entry *ret;\n \n \tif (!map->cmpfn)\n \t\thashmap_init(map, anonymized_entry_cmp, NULL, 0);\n \n \thashmap_entry_init(&key.hash, memhash(orig, len));\n \tkey.orig = orig;\n \tkey.orig_len = len;\n-\tret = hashmap_get_entry(map, &key, hash, NULL);\n+\tret = hashmap_get_entry(map, &key, hash, &key);\n \n \tif (!ret) {\n \t\tret = xmalloc(sizeof(*ret));\n \t\thashmap_entry_init(&ret->hash, key.hash.hash);\n \t\tret->orig = xmemdupz(orig, len);\n-\t\tret->orig_len = len;\n \t\tret->anon = generate(orig, len);\n-\t\tret->anon_len = strlen(ret->anon);\n \t\thashmap_put(map, &ret->hash);\n \t}\n \n-- \n2.27.0.517.gbc32778fa3\n\n"},{"id":"400383","messageId":"20200623152458.GF1435482@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200623152436.GA50925@coredump.intra.peff.net","subject":"[PATCH 06/10] fast-export: use a flex array to store anonymized entries","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-23T15:24:58Z","receivedAt":"2020-06-23T15:25:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Now that we're using a separate keydata struct for hash lookups, we have\nmore flexibility in how we allocate anonymized_entry structs. Let's push\nthe \"orig\" key into a flex member within the struct. That should save us\na few bytes of memory per entry (a pointer plus any malloc overhead),\nand may make lookups a little faster (since it's one less pointer to\nchase in the comparison function).\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 5df2ada47d..99d4a2b404 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -120,8 +120,8 @@ static int has_unshown_parent(struct commit *commit)\n \n struct anonymized_entry {\n \tstruct hashmap_entry hash;\n-\tconst char *orig;\n \tconst char *anon;\n+\tconst char orig[FLEX_ARRAY];\n };\n \n struct anonymized_entry_key {\n@@ -170,9 +170,8 @@ static const char *anonymize_str(struct hashmap *map,\n \tret = hashmap_get_entry(map, &key, hash, &key);\n \n \tif (!ret) {\n-\t\tret = xmalloc(sizeof(*ret));\n+\t\tFLEX_ALLOC_MEM(ret, orig, orig, len);\n \t\thashmap_entry_init(&ret->hash, key.hash.hash);\n-\t\tret->orig = xmemdupz(orig, len);\n \t\tret->anon = generate(orig, len);\n \t\thashmap_put(map, &ret->hash);\n \t}\n-- \n2.27.0.517.gbc32778fa3\n\n"},{"id":"400384","messageId":"20200623152501.GG1435482@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200623152436.GA50925@coredump.intra.peff.net","subject":"[PATCH 07/10] fast-export: move global \"idents\" anonymize hashmap into function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-23T15:25:01Z","receivedAt":"2020-06-23T15:25:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"All of the other anonymization functions keep their static mappings\ninside the function to avoid polluting the global namespace. Let's do\nthe same for \"idents\", as nobody needs it outside of\nanonymize_ident_line().\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 99d4a2b404..16a1563e49 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -555,7 +555,6 @@ static char *anonymize_commit_message(const char *old)\n \treturn xstrfmt(\"subject %d\\n\\nbody\\n\", counter++);\n }\n \n-static struct hashmap idents;\n static char *anonymize_ident(const char *old, size_t len)\n {\n \tstatic int counter;\n@@ -572,6 +571,7 @@ static char *anonymize_ident(const char *old, size_t len)\n */\n static void anonymize_ident_line(const char **beg, const char **end)\n {\n+\tstatic struct hashmap idents;\n \tstatic struct strbuf buffers[] = { STRBUF_INIT, STRBUF_INIT };\n \tstatic unsigned which_buffer;\n \n-- \n2.27.0.517.gbc32778fa3\n\n"},{"id":"400385","messageId":"20200623152503.GH1435482@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200623152436.GA50925@coredump.intra.peff.net","subject":"[PATCH 08/10] fast-export: add a \"data\" callback parameter to anonymize_str()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-23T15:25:03Z","receivedAt":"2020-06-23T15:25:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The anonymize_str() function takes a generator callback, but there's no\nway to pass extra context to it. Let's add the usual \"void *data\"\nparameter to the generator interface and pass it along.\n\nThis is mildly annoying for existing callers, all of which pass NULL,\nbut is necessary to avoid extra globals in some cases we'll add in a\nsubsequent patch.\n\nWhile we're touching each of these callbacks, we can further observe\nthat none of them use the existing orig/len parameters at all. This\nmakes sense, since the point is for their output to have no discernable\nbasis in the original (my original version had some notion that we might\nuse a one-way function to obfuscate the names, but it was never\nimplemented). So let's drop those extra parameters. If a caller really\nwants to do something with them, it can pass a struct through the new\ndata parameter.\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 27 ++++++++++++++-------------\n 1 file changed, 14 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 16a1563e49..1cbca5b4b4 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -155,8 +155,9 @@ static int anonymized_entry_cmp(const void *unused_cmp_data,\n * is farmed out to the generate function.\n */\n static const char *anonymize_str(struct hashmap *map,\n-\t\t\t\t char *(*generate)(const char *, size_t),\n-\t\t\t\t const char *orig, size_t len)\n+\t\t\t\t char *(*generate)(void *),\n+\t\t\t\t const char *orig, size_t len,\n+\t\t\t\t void *data)\n {\n \tstruct anonymized_entry_key key;\n \tstruct anonymized_entry *ret;\n@@ -172,7 +173,7 @@ static const char *anonymize_str(struct hashmap *map,\n \tif (!ret) {\n \t\tFLEX_ALLOC_MEM(ret, orig, orig, len);\n \t\thashmap_entry_init(&ret->hash, key.hash.hash);\n-\t\tret->anon = generate(orig, len);\n+\t\tret->anon = generate(data);\n \t\thashmap_put(map, &ret->hash);\n \t}\n \n@@ -187,12 +188,12 @@ static const char *anonymize_str(struct hashmap *map,\n */\n static void anonymize_path(struct strbuf *out, const char *path,\n \t\t\t struct hashmap *map,\n-\t\t\t char *(*generate)(const char *, size_t))\n+\t\t\t char *(*generate)(void *))\n {\n \twhile (*path) {\n \t\tconst char *end_of_component = strchrnul(path, '/');\n \t\tsize_t len = end_of_component - path;\n-\t\tconst char *c = anonymize_str(map, generate, path, len);\n+\t\tconst char *c = anonymize_str(map, generate, path, len, NULL);\n \t\tstrbuf_addstr(out, c);\n \t\tpath = end_of_component;\n \t\tif (*path)\n@@ -367,7 +368,7 @@ static void print_path_1(const char *path)\n \t\tprintf(\"%s\", path);\n }\n \n-static char *anonymize_path_component(const char *path, size_t len)\n+static char *anonymize_path_component(void *data)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n@@ -389,7 +390,7 @@ static void print_path(const char *path)\n \t}\n }\n \n-static char *generate_fake_oid(const char *old, size_t len)\n+static char *generate_fake_oid(void *data)\n {\n \tstatic uint32_t counter = 1; /* avoid null oid */\n \tconst unsigned hashsz = the_hash_algo->rawsz;\n@@ -405,7 +406,7 @@ static const char *anonymize_oid(const char *oid_hex)\n {\n \tstatic struct hashmap objs;\n \tsize_t len = strlen(oid_hex);\n-\treturn anonymize_str(&objs, generate_fake_oid, oid_hex, len);\n+\treturn anonymize_str(&objs, generate_fake_oid, oid_hex, len, NULL);\n }\n \n static void show_filemodify(struct diff_queue_struct *q,\n@@ -502,7 +503,7 @@ static const char *find_encoding(const char *begin, const char *end)\n \treturn bol;\n }\n \n-static char *anonymize_ref_component(const char *old, size_t len)\n+static char *anonymize_ref_component(void *data)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n@@ -555,7 +556,7 @@ static char *anonymize_commit_message(const char *old)\n \treturn xstrfmt(\"subject %d\\n\\nbody\\n\", counter++);\n }\n \n-static char *anonymize_ident(const char *old, size_t len)\n+static char *anonymize_ident(void *data)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n@@ -598,7 +599,7 @@ static void anonymize_ident_line(const char **beg, const char **end)\n \n \t\tlen = split.mail_end - split.name_begin;\n \t\tident = anonymize_str(&idents, anonymize_ident,\n-\t\t\t\t split.name_begin, len);\n+\t\t\t\t split.name_begin, len, NULL);\n \t\tstrbuf_addstr(out, ident);\n \t\tstrbuf_addch(out, ' ');\n \t\tstrbuf_add(out, split.date_begin, split.tz_end - split.date_begin);\n@@ -739,7 +740,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \tshow_progress();\n }\n \n-static char *anonymize_tag(const char *old, size_t len)\n+static char *anonymize_tag(void *data)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n@@ -815,7 +816,7 @@ static void handle_tag(const char *name, struct tag *tag)\n \t\tif (message) {\n \t\t\tstatic struct hashmap tags;\n \t\t\tmessage = anonymize_str(&tags, anonymize_tag,\n-\t\t\t\t\t\tmessage, message_size);\n+\t\t\t\t\t\tmessage, message_size, NULL);\n \t\t}\n \t}\n \n-- \n2.27.0.517.gbc32778fa3\n\n"},{"id":"400386","messageId":"20200623152505.GI1435482@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200623152436.GA50925@coredump.intra.peff.net","subject":"[PATCH 09/10] fast-export: allow seeding the anonymized mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-23T15:25:05Z","receivedAt":"2020-06-23T15:25:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"After you anonymize a repository, it can be hard to find which commits\ncorrespond between the original and the result, and thus hard to\nreproduce commands that triggered bugs in the original.\n\nLet's make it possible to seed the anonymization map. This lets users\neither:\n\n - mark names to be retained as-is, if they don't consider them secret\n (in which case their original commands would just work)\n\n - map names to new values, which lets them adapt the reproduction\n recipe to the new names without revealing the originals\n\nThe implementation is fairly straight-forward. We already store each\nanonymized token in a hashmap (so that the same token appearing twice is\nconverted to the same result). We can just introduce a new \"seed\"\nhashmap which is consulted first.\n\nThis does make a few more promises to the user about how we'll anonymize\nthings (e.g., token-splitting pathnames). But it's unlikely that we'd\nwant to change those rules, even if the actual anonymization of a single\ntoken changes. And it makes things much easier for the user, who can\nunblind only a directory name without having to specify each path within\nit.\n\nOne alternative to this approach would be to anonymize as we see fit,\nand then dump the whole refname and pathname mappings to a file. This\ndoes work, but it's a bit awkward to use (you have to manually dig the\nitems you care about out of the mapping).\n\nSigned-off-by: Jeff King \n---\n Documentation/git-fast-export.txt | 24 ++++++++++++++++\n builtin/fast-export.c | 47 ++++++++++++++++++++++++++++++-\n t/t9351-fast-export-anonymize.sh | 11 +++++++-\n 3 files changed, 80 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\nindex e8950de3ba..2d7b62e835 100644\n--- a/Documentation/git-fast-export.txt\n+++ b/Documentation/git-fast-export.txt\n@@ -119,6 +119,11 @@ by keeping the marks the same across runs.\n \tthe shape of the history and stored tree. See the section on\n \t`ANONYMIZING` below.\n \n+--seed-anonymized=[:]::\n+\tConvert token `` to `` in the anonymized output. If\n+\t`` is omitted, map `` to itself (i.e., do not\n+\tanonymize it). See the section on `ANONYMIZING` below.\n+\n --reference-excluded-parents::\n \tBy default, running a command such as `git fast-export\n \tmaster~5..master` will not include the commit master{tilde}5\n@@ -238,6 +243,25 @@ collapse \"User 0\", \"User 1\", etc into \"User X\"). This produces a much\n smaller output, and it is usually easy to quickly confirm that there is\n no private data in the stream.\n \n+Reproducing some bugs may require referencing particular commits or\n+paths, which becomes challenging after refnames and paths have been\n+anonymized. You can ask for a particular token to be left as-is or\n+mapped to a new value. For example, if you have a bug which reproduces\n+with `git rev-list mybranch -- foo.c`, you can run:\n+\n+---------------------------------------------------\n+$ git fast-export --anonymize --all \\\n+ --seed-anonymized=foo.c:secret.c \\\n+ --seed-anonymized=mybranch \\\n+ >stream\n+---------------------------------------------------\n+\n+After importing the stream, you can then run `git rev-list mybranch --\n+secret.c` in the anonymized repository.\n+\n+Note that paths and refnames are split into tokens at slash boundaries.\n+The command above would anonymize `subdir/foo.c` as something like\n+`path123/secret.c`.\n \n LIMITATIONS\n -----------\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 1cbca5b4b4..ef82497bbf 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -45,6 +45,7 @@ static struct string_list extra_refs = STRING_LIST_INIT_NODUP;\n static struct string_list tag_refs = STRING_LIST_INIT_NODUP;\n static struct refspec refspecs = REFSPEC_INIT_FETCH;\n static int anonymize;\n+static struct hashmap anonymized_seeds;\n static struct revision_sources revision_sources;\n \n static int parse_opt_signed_tag_mode(const struct option *opt,\n@@ -168,8 +169,18 @@ static const char *anonymize_str(struct hashmap *map,\n \thashmap_entry_init(&key.hash, memhash(orig, len));\n \tkey.orig = orig;\n \tkey.orig_len = len;\n-\tret = hashmap_get_entry(map, &key, hash, &key);\n \n+\t/* First check if it's a token the user configured manually... */\n+\tif (anonymized_seeds.cmpfn)\n+\t\tret = hashmap_get_entry(&anonymized_seeds, &key, hash, &key);\n+\telse\n+\t\tret = NULL;\n+\n+\t/* ...otherwise check if we've already seen it in this context... */\n+\tif (!ret)\n+\t\tret = hashmap_get_entry(map, &key, hash, &key);\n+\n+\t/* ...and finally generate a new mapping if necessary */\n \tif (!ret) {\n \t\tFLEX_ALLOC_MEM(ret, orig, orig, len);\n \t\thashmap_entry_init(&ret->hash, key.hash.hash);\n@@ -1147,6 +1158,37 @@ static void handle_deletes(void)\n \t}\n }\n \n+static char *anonymize_seed(void *data)\n+{\n+\treturn xstrdup(data);\n+}\n+\n+static int parse_opt_seed_anonymized(const struct option *opt,\n+\t\t\t\t const char *arg, int unset)\n+{\n+\tstruct hashmap *map = opt->value;\n+\tconst char *delim, *value;\n+\tsize_t keylen;\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\n+\tdelim = strchr(arg, ':');\n+\tif (delim) {\n+\t\tkeylen = delim - arg;\n+\t\tvalue = delim + 1;\n+\t} else {\n+\t\tkeylen = strlen(arg);\n+\t\tvalue = arg;\n+\t}\n+\n+\tif (!keylen || !*value)\n+\t\treturn error(_(\"--seed-anonymized token cannot be empty\"));\n+\n+\tanonymize_str(map, anonymize_seed, arg, keylen, (void *)value);\n+\n+\treturn 0;\n+}\n+\n int cmd_fast_export(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info revs;\n@@ -1188,6 +1230,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING_LIST(0, \"refspec\", &refspecs_list, N_(\"refspec\"),\n \t\t\t N_(\"Apply refspec to exported refs\")),\n \t\tOPT_BOOL(0, \"anonymize\", &anonymize, N_(\"anonymize output\")),\n+\t\tOPT_CALLBACK_F(0, \"seed-anonymized\", &anonymized_seeds, N_(\"from:to\"),\n+\t\t\t N_(\"convert to in anonymized output\"),\n+\t\t\t PARSE_OPT_NONEG, parse_opt_seed_anonymized),\n \t\tOPT_BOOL(0, \"reference-excluded-parents\",\n \t\t\t &reference_excluded_commits, N_(\"Reference parents which are not in fast-export stream by object id\")),\n \t\tOPT_BOOL(0, \"show-original-ids\", &show_original_ids,\ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex dc5d75cd19..d84eec9bab 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -6,6 +6,7 @@ test_description='basic tests for fast-export --anonymize'\n test_expect_success 'setup simple repo' '\n \ttest_commit base &&\n \ttest_commit foo &&\n+\ttest_commit retain-me &&\n \tgit checkout -b other HEAD^ &&\n \tmkdir subdir &&\n \ttest_commit subdir/bar &&\n@@ -18,7 +19,10 @@ test_expect_success 'setup simple repo' '\n '\n \n test_expect_success 'export anonymized stream' '\n-\tgit fast-export --anonymize --all >stream\n+\tgit fast-export --anonymize --all \\\n+\t\t--seed-anonymized=retain-me \\\n+\t\t--seed-anonymized=xyzzy:custom-name \\\n+\t\t>stream\n '\n \n # this also covers commit messages\n@@ -30,6 +34,11 @@ test_expect_success 'stream omits path names' '\n \t! grep xyzzy stream\n '\n \n+test_expect_success 'stream contains user-specified names' '\n+\tgrep retain-me stream &&\n+\tgrep custom-name stream\n+'\n+\n test_expect_success 'stream omits gitlink oids' '\n \t# avoid relying on the whole oid to remain hash-agnostic; this is\n \t# plenty to be unique within our test case\n-- \n2.27.0.517.gbc32778fa3\n\n"},{"id":"400387","messageId":"20200623152507.GJ1435482@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200623152436.GA50925@coredump.intra.peff.net","subject":"[PATCH 10/10] fast-export: anonymize \"master\" refname","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-23T15:25:07Z","receivedAt":"2020-06-23T15:25:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Running \"fast-export --anonymize\" will leave \"refs/heads/master\"\nuntouched in the output, for two reasons:\n\n - it helped to have some known reference point between the original\n and anonymized repository\n\n - since it's historically the default branch name, it doesn't leak any\n information\n\nNow that we can ask fast-export to retain particular tokens, we have a\nmuch better tool for the first one (because it works for any ref, not\njust master).\n\nFor the second, the notion of \"default branch name\" is likely to become\nconfigurable soon, at which point the name _does_ leak information.\nLet's drop this special case in preparation.\n\nNote that we have to adjust the test a bit, since it relied on using the\nname \"master\" in the anonymized repos. We could just use\n--seed-anonymized=master to keep the same output, but then we wouldn't\nknow if it works because of our hard-coded master or because of the\nseeding.\n\nSo let's flip the test a bit, and confirm that we anonymize \"master\",\nbut keep \"other\" in the output.\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 7 -------\n t/t9351-fast-export-anonymize.sh | 12 +++++++-----\n 2 files changed, 7 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex ef82497bbf..7e0e1770cf 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -538,13 +538,6 @@ static const char *anonymize_refname(const char *refname)\n \tstatic struct strbuf anon = STRBUF_INIT;\n \tint i;\n \n-\t/*\n-\t * We also leave \"master\" as a special case, since it does not reveal\n-\t * anything interesting.\n-\t */\n-\tif (!strcmp(refname, \"refs/heads/master\"))\n-\t\treturn refname;\n-\n \tstrbuf_reset(&anon);\n \tfor (i = 0; i < ARRAY_SIZE(prefixes); i++) {\n \t\tif (skip_prefix(refname, prefixes[i], &refname)) {\ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex d84eec9bab..6e2041865c 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -22,6 +22,7 @@ test_expect_success 'export anonymized stream' '\n \tgit fast-export --anonymize --all \\\n \t\t--seed-anonymized=retain-me \\\n \t\t--seed-anonymized=xyzzy:custom-name \\\n+\t\t--seed-anonymized=other \\\n \t\t>stream\n '\n \n@@ -45,12 +46,12 @@ test_expect_success 'stream omits gitlink oids' '\n \t! grep a000000000000000000 stream\n '\n \n-test_expect_success 'stream allows master as refname' '\n-\tgrep master stream\n+test_expect_success 'stream retains other as refname' '\n+\tgrep other stream\n '\n \n test_expect_success 'stream omits other refnames' '\n-\t! grep other stream &&\n+\t! grep master stream &&\n \t! grep mytag stream\n '\n \n@@ -76,15 +77,16 @@ test_expect_success 'import stream to new repository' '\n test_expect_success 'result has two branches' '\n \tgit for-each-ref --format=\"%(refname)\" refs/heads >branches &&\n \ttest_line_count = 2 branches &&\n-\tother_branch=$(grep -v refs/heads/master branches)\n+\tother_branch=refs/heads/other &&\n+\tmain_branch=$(grep -v $other_branch branches)\n '\n \n test_expect_success 'repo has original shape and timestamps' '\n \tshape () {\n \t\tgit log --format=\"%m %ct\" --left-right --boundary \"$@\"\n \t} &&\n \t(cd .. && shape master...other) >expect &&\n-\tshape master...$other_branch >actual &&\n+\tshape $main_branch...$other_branch >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.27.0.517.gbc32778fa3\n"},{"id":"400396","messageId":"CAPig+cTMu3SW5UE59Eva=jh-07hOT8Dz5A5trWyEugKd_2HaKg@mail.gmail.com","threadId":"53716","inReplyTo":"20200623152505.GI1435482@coredump.intra.peff.net","subject":"Re: [PATCH 09/10] fast-export: allow seeding the anonymized mapping","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-06-23T17:16:05Z","receivedAt":"2020-06-23T17:16:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 23, 2020 at 11:25 AM Jeff King wrote:\n> diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\n> @@ -238,6 +243,25 @@ collapse \"User 0\", \"User 1\", etc into \"User X\"). This produces a much\n> +[...] For example, if you have a bug which reproduces\n> +with `git rev-list mybranch -- foo.c`, you can run:\n> +\n> +---------------------------------------------------\n> +$ git fast-export --anonymize --all \\\n> + --seed-anonymized=foo.c:secret.c \\\n> + --seed-anonymized=mybranch \\\n> + >stream\n> +---------------------------------------------------\n> +\n> +After importing the stream, you can then run `git rev-list mybranch --\n> +secret.c` in the anonymized repository.\n\nI understand that your intention here is to demonstrate both forms of\n--seed-anonymized, but I'm slightly concerned that people may\ninterpret this example as meaning that you are not allowed to\nanonymize the refname when anonymizing a pathname. It might be less\nambiguous to avoid the \"short form\" in the example; people who have\nread the description of --seed-anonymized will know that the short\nform can be used without having to see it in an example.\n\n> +Note that paths and refnames are split into tokens at slash boundaries.\n> +The command above would anonymize `subdir/foo.c` as something like\n> +`path123/secret.c`.\n\nConfusing. This seems to be saying that anonymizing filenames in\nsubdirectories is pointless because you can't know how the leading\ndirectory names will be anonymized. That leaves the reader wondering\nhow to deal with the situation. Does it require using\n--seed-anonymized for each path component leading up to the filename?\nOr can --seed-anonymized take an full pathname (leading directory\ncomponents and filename) in one shot?\n\n> @@ -168,8 +169,18 @@ static const char *anonymize_str(struct hashmap *map,\n> - ret = hashmap_get_entry(map, &key, hash, &key);\n>\n> + /* First check if it's a token the user configured manually... */\n> + if (anonymized_seeds.cmpfn)\n> + ret = hashmap_get_entry(&anonymized_seeds, &key, hash, &key);\n> + else\n> + ret = NULL;\n> +\n> + /* ...otherwise check if we've already seen it in this context... */\n> + if (!ret)\n> + ret = hashmap_get_entry(map, &key, hash, &key);\n> +\n> + /* ...and finally generate a new mapping if necessary */\n\nI was a bit surprised to see that --seed-anonymized values are stored\nin a separate hash map rather than simply being used to (literally)\nseed the existing anonymization hash map. I guess there's a good\ntechnical reason for doing it this way, such as the normal\nanonymization hash map not yet being in existence at the time the\n--seed-anonymized option is processed? (I haven't checked because I'm\ntoo lazy, so it may not be worth spending time answering me.)\n\n> @@ -1188,6 +1230,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n> OPT_BOOL(0, \"anonymize\", &anonymize, N_(\"anonymize output\")),\n> + OPT_CALLBACK_F(0, \"seed-anonymized\", &anonymized_seeds, N_(\"from:to\"),\n> + N_(\"convert to in anonymized output\"),\n> + PARSE_OPT_NONEG, parse_opt_seed_anonymized),\n\nWould it be worthwhile to add a check somewhere after the\nparse_options() invocation and complain if --seed-anonymized was used\nwithout --anonymize? (Or should --seed-anonymized perhaps imply\n--anonymize?)\n"},{"id":"400415","messageId":"CAPig+cQ1EGR6_8NJ+U3MQEseU1yT9k_AwdL8Mg7qZAB8Ev4y3g@mail.gmail.com","threadId":"53716","inReplyTo":"20200623152505.GI1435482@coredump.intra.peff.net","subject":"Re: [PATCH 09/10] fast-export: allow seeding the anonymized mapping","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-06-23T18:11:51Z","receivedAt":"2020-06-23T18:12:06Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 23, 2020 at 11:25 AM Jeff King wrote:\n> Let's make it possible to seed the anonymization map. This lets users\n> either:\n> [...]\n> Signed-off-by: Jeff King \n> ---\n> diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\n> @@ -119,6 +119,11 @@ by keeping the marks the same across runs.\n> +--seed-anonymized=[:]::\n> + Convert token `` to `` in the anonymized output. If\n> + `` is omitted, map `` to itself (i.e., do not\n> + anonymize it). See the section on `ANONYMIZING` below.\n\nBy the way (possible bikeshedding ahead), \"seed anonymous\" seems\noverly technical. I wonder if a name such as\n'--anonymize-to=[:]' might be clearer and easier for people\nto understand.\n\nIn fact, in an earlier email, I asked whether --seed-anonymized should\nimply --anonymize. Thinking further on this, I wonder if we even need\nthe second option name. It should be possible to overload the existing\n--anonymize to handle all functions. For instance:\n\n '--anonymize' would anonymize everything\n\n '--anonymize=[:]' would anonymize and map to \n\nSo, the example you give in the documentation would become:\n\n git fast-export --all \\\n --anonymize=foo.c:secret.c \\\n --anonymize=mybranch >stream\n\nOr is that too cryptic?\n"},{"id":"400417","messageId":"20200623183053.GA1444619@coredump.intra.peff.net","threadId":"53716","inReplyTo":"CAPig+cTMu3SW5UE59Eva=jh-07hOT8Dz5A5trWyEugKd_2HaKg@mail.gmail.com","subject":"Re: [PATCH 09/10] fast-export: allow seeding the anonymized mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-23T18:30:53Z","receivedAt":"2020-06-23T18:30:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 23, 2020 at 01:16:05PM -0400, Eric Sunshine wrote:\n\n> On Tue, Jun 23, 2020 at 11:25 AM Jeff King wrote:\n> > diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\n> > @@ -238,6 +243,25 @@ collapse \"User 0\", \"User 1\", etc into \"User X\"). This produces a much\n> > +[...] For example, if you have a bug which reproduces\n> > +with `git rev-list mybranch -- foo.c`, you can run:\n> > +\n> > +---------------------------------------------------\n> > +$ git fast-export --anonymize --all \\\n> > + --seed-anonymized=foo.c:secret.c \\\n> > + --seed-anonymized=mybranch \\\n> > + >stream\n> > +---------------------------------------------------\n> > +\n> > +After importing the stream, you can then run `git rev-list mybranch --\n> > +secret.c` in the anonymized repository.\n> \n> I understand that your intention here is to demonstrate both forms of\n> --seed-anonymized, but I'm slightly concerned that people may\n> interpret this example as meaning that you are not allowed to\n> anonymize the refname when anonymizing a pathname. It might be less\n> ambiguous to avoid the \"short form\" in the example; people who have\n> read the description of --seed-anonymized will know that the short\n> form can be used without having to see it in an example.\n\nI'm not sure what you'd write, then. You can't mention \"mybranch\"\nanymore if it was anonymized. Are you suggesting to make the example:\n\n git rev-list -- foo.c\n\nby itself?\n\n> > +Note that paths and refnames are split into tokens at slash boundaries.\n> > +The command above would anonymize `subdir/foo.c` as something like\n> > +`path123/secret.c`.\n> \n> Confusing. This seems to be saying that anonymizing filenames in\n> subdirectories is pointless because you can't know how the leading\n> directory names will be anonymized. That leaves the reader wondering\n> how to deal with the situation. Does it require using\n> --seed-anonymized for each path component leading up to the filename?\n\nYou can do that, but I think it would be simpler to just find \"secret.c\"\nin the anonymized repo (either in the checkout, or just \"git ls-tree\n-r\").\n\n> Or can --seed-anonymized take an full pathname (leading directory\n> components and filename) in one shot?\n\nNo, it can't. Suggested wording? That's what I was trying to say with\nthe above sentence.\n\n> > + /* First check if it's a token the user configured manually... */\n> > + if (anonymized_seeds.cmpfn)\n> > + ret = hashmap_get_entry(&anonymized_seeds, &key, hash, &key);\n> > + else\n> > + ret = NULL;\n> > +\n> > + /* ...otherwise check if we've already seen it in this context... */\n> > + if (!ret)\n> > + ret = hashmap_get_entry(map, &key, hash, &key);\n> > +\n> > + /* ...and finally generate a new mapping if necessary */\n> \n> I was a bit surprised to see that --seed-anonymized values are stored\n> in a separate hash map rather than simply being used to (literally)\n> seed the existing anonymization hash map. I guess there's a good\n> technical reason for doing it this way, such as the normal\n> anonymization hash map not yet being in existence at the time the\n> --seed-anonymized option is processed? (I haven't checked because I'm\n> too lazy, so it may not be worth spending time answering me.)\n\nThe reason is that there isn't one anonymization hash map. There's a\nseparate one for each generator (so refs become \"refs/heads/ref123\" and\npaths become \"path123/path456\").\n\n> > @@ -1188,6 +1230,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n> > OPT_BOOL(0, \"anonymize\", &anonymize, N_(\"anonymize output\")),\n> > + OPT_CALLBACK_F(0, \"seed-anonymized\", &anonymized_seeds, N_(\"from:to\"),\n> > + N_(\"convert to in anonymized output\"),\n> > + PARSE_OPT_NONEG, parse_opt_seed_anonymized),\n> \n> Would it be worthwhile to add a check somewhere after the\n> parse_options() invocation and complain if --seed-anonymized was used\n> without --anonymize? (Or should --seed-anonymized perhaps imply\n> --anonymize?)\n\nI thought about implying, but I have a slight preference to err on the\nside of making things less magical. I don't mind triggering a warning or\nerror, but it's not like anything _bad_ happens if you don't say\n--anonymize. It just doesn't do anything, which seems like a perfectly\nlogical outcome.\n\n-Peff\n"},{"id":"400418","messageId":"20200623183525.GB1444619@coredump.intra.peff.net","threadId":"53716","inReplyTo":"CAPig+cQ1EGR6_8NJ+U3MQEseU1yT9k_AwdL8Mg7qZAB8Ev4y3g@mail.gmail.com","subject":"Re: [PATCH 09/10] fast-export: allow seeding the anonymized mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-23T18:35:25Z","receivedAt":"2020-06-23T18:35:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 23, 2020 at 02:11:51PM -0400, Eric Sunshine wrote:\n\n> On Tue, Jun 23, 2020 at 11:25 AM Jeff King wrote:\n> > Let's make it possible to seed the anonymization map. This lets users\n> > either:\n> > [...]\n> > Signed-off-by: Jeff King \n> > ---\n> > diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\n> > @@ -119,6 +119,11 @@ by keeping the marks the same across runs.\n> > +--seed-anonymized=[:]::\n> > + Convert token `` to `` in the anonymized output. If\n> > + `` is omitted, map `` to itself (i.e., do not\n> > + anonymize it). See the section on `ANONYMIZING` below.\n> \n> By the way (possible bikeshedding ahead), \"seed anonymous\" seems\n> overly technical. I wonder if a name such as\n> '--anonymize-to=[:]' might be clearer and easier for people\n> to understand.\n\nI wrestled with the name, and I agree \"seed\" is overly technical. And I\ncame up with many similar variations of \"anonymize-to\", but they all\nseemed ambiguous (e.g., it could be \"to\" a file that we're storing the\ndata in).\n\nPerhaps \"--anonymize-map\" would be less technical?\n\n> In fact, in an earlier email, I asked whether --seed-anonymized should\n> imply --anonymize. Thinking further on this, I wonder if we even need\n> the second option name. It should be possible to overload the existing\n> --anonymize to handle all functions. For instance:\n> \n> '--anonymize' would anonymize everything\n> \n> '--anonymize=[:]' would anonymize and map to \n> \n> So, the example you give in the documentation would become:\n> \n> git fast-export --all \\\n> --anonymize=foo.c:secret.c \\\n> --anonymize=mybranch >stream\n> \n> Or is that too cryptic?\n\nYeah, that was another one I considered, but it both seemed cryptic\n(after all, we're saying what _not_ to anonymize), and it squats on the\n\"anonymize\" option. So imagine we had another option later, like\n\"anonymize blobs and paths, but not refs\", that could easily be\n\"--anonymize=blobs,path\" or \"--anonymize=!refs\". I'd rather not paint\nourselves in a corner.\n\n-Peff\n"},{"id":"400430","messageId":"xmqq1rm5d19k.fsf@gitster.c.googlers.com","threadId":"53716","inReplyTo":"20200623152436.GA50925@coredump.intra.peff.net","subject":"Re: [alternative 0/10] fast-export: allow seeding the anonymized mapping","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-23T19:34:47Z","receivedAt":"2020-06-23T19:34:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Jun 22, 2020 at 05:47:46PM -0400, Jeff King wrote:\n>\n>> Here's a v2 which I think addresses all of the comments. I have to admit\n>> that after writing my last email to Junio, I am wondering whether it\n>> would be sufficient and simpler to let the user specify a static mapping\n>> of tokens (that could just be applied anywhere).\n\nYeah, dumping the random mapping created and telling the user to\nfigure out what the tool did is less nice than allowing the user\nto enumerate what tokens are sensitible and need to be replaced.\n\n> Both of these techniques _could_ live side-by-side within fast-export,\n> as they have slightly different strengths and weaknesses. But I'd prefer\n> to just go with one (this one) in the name of simplicity, and I strongly\n> suspect nobody will ever ask for the other.\n\nOK. So should we revert the merge of the other one into 'next'?\nThat is easy enough ;-)\n\nThanks.\n\n> [01/10]: t9351: derive anonymized tree checks from original repo\n> [02/10]: fast-export: use xmemdupz() for anonymizing oids\n>\n> These first two are actually a bug-fix that I noticed while writing\n> it.\n>\n> [03/10]: fast-export: store anonymized oids as hex strings\n> [04/10]: fast-export: tighten anonymize_mem() interface to handle only strings\n> [05/10]: fast-export: stop storing lengths in anonymized hashmaps\n> [06/10]: fast-export: use a flex array to store anonymized entries\n> [07/10]: fast-export: move global \"idents\" anonymize hashmap into function\n> [08/10]: fast-export: add a \"data\" callback parameter to anonymize_str()\n>\n> This is all cleanup and prep. More than is strictly necessary for\n> this series, but it does simplify things and reduce the memory\n> footprint (only a few megabytes in git.git, but more in larger\n> repos).\n>\n> [09/10]: fast-export: allow seeding the anonymized mapping\n>\n> And then this is the actual feature...\n>\n> [10/10]: fast-export: anonymize \"master\" refname\n>\n> ...which finally lets us drop the special name rule.\n>\n> Documentation/git-fast-export.txt | 24 +++++\n> builtin/fast-export.c | 161 +++++++++++++++++++-----------\n> t/t9351-fast-export-anonymize.sh | 54 +++++++---\n> 3 files changed, 167 insertions(+), 72 deletions(-)\n"},{"id":"400432","messageId":"20200623194456.GA2068874@coredump.intra.peff.net","threadId":"53716","inReplyTo":"xmqq1rm5d19k.fsf@gitster.c.googlers.com","subject":"Re: [alternative 0/10] fast-export: allow seeding the anonymized mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-23T19:44:56Z","receivedAt":"2020-06-23T19:44:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 23, 2020 at 12:34:47PM -0700, Junio C Hamano wrote:\n\n> > Both of these techniques _could_ live side-by-side within fast-export,\n> > as they have slightly different strengths and weaknesses. But I'd prefer\n> > to just go with one (this one) in the name of simplicity, and I strongly\n> > suspect nobody will ever ask for the other.\n> \n> OK. So should we revert the merge of the other one into 'next'?\n> That is easy enough ;-)\n\nYes, please. :)\n\n-Peff\n"},{"id":"400440","messageId":"CAPig+cTBwZJnUYqeEjXp-hAOjHb783B+9kyy+h+HtiOTy_c_hg@mail.gmail.com","threadId":"53716","inReplyTo":"20200623183525.GB1444619@coredump.intra.peff.net","subject":"Re: [PATCH 09/10] fast-export: allow seeding the anonymized mapping","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-06-23T20:35:37Z","receivedAt":"2020-06-23T21:06:32Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 23, 2020 at 2:35 PM Jeff King wrote:\n> On Tue, Jun 23, 2020 at 02:11:51PM -0400, Eric Sunshine wrote:\n> > By the way (possible bikeshedding ahead), \"seed anonymous\" seems\n> > overly technical. I wonder if a name such as\n> > '--anonymize-to=[:]' might be clearer and easier for people\n> > to understand.\n>\n> I wrestled with the name, and I agree \"seed\" is overly technical. And I\n> came up with many similar variations of \"anonymize-to\", but they all\n> seemed ambiguous (e.g., it could be \"to\" a file that we're storing the\n> data in).\n>\n> Perhaps \"--anonymize-map\" would be less technical?\n\nThat's not too bad. It is better than --seed-anonymized. I haven't\ncome up with any name which improves upon it.\n\n> > In fact, in an earlier email, I asked whether --seed-anonymized should\n> > imply --anonymize. Thinking further on this, I wonder if we even need\n> > the second option name. It should be possible to overload the existing\n> > --anonymize to handle all functions. For instance:\n> >\n> > git fast-export --all \\\n> > --anonymize=foo.c:secret.c \\\n> > --anonymize=mybranch >stream\n> >\n> > Or is that too cryptic?\n>\n> Yeah, that was another one I considered, but it both seemed cryptic\n> (after all, we're saying what _not_ to anonymize), and it squats on the\n> \"anonymize\" option. So imagine we had another option later, like\n> \"anonymize blobs and paths, but not refs\", that could easily be\n> \"--anonymize=blobs,path\" or \"--anonymize=!refs\". I'd rather not paint\n> ourselves in a corner.\n\nOkay, makes sense.\n"},{"id":"400445","messageId":"CAPig+cTFkAOyLG1Sm_p11GgH9Ms87_7zs-7kFbEYZ-uXg1yrYw@mail.gmail.com","threadId":"53716","inReplyTo":"20200623183053.GA1444619@coredump.intra.peff.net","subject":"Re: [PATCH 09/10] fast-export: allow seeding the anonymized mapping","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-06-23T20:30:23Z","receivedAt":"2020-06-23T21:13:19Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 23, 2020 at 2:31 PM Jeff King wrote:\n> On Tue, Jun 23, 2020 at 01:16:05PM -0400, Eric Sunshine wrote:\n> > On Tue, Jun 23, 2020 at 11:25 AM Jeff King wrote:\n> > I understand that your intention here is to demonstrate both forms of\n> > --seed-anonymized, but I'm slightly concerned that people may\n> > interpret this example as meaning that you are not allowed to\n> > anonymize the refname when anonymizing a pathname. It might be less\n> > ambiguous to avoid the \"short form\" in the example; people who have\n> > read the description of --seed-anonymized will know that the short\n> > form can be used without having to see it in an example.\n>\n> I'm not sure what you'd write, then. You can't mention \"mybranch\"\n> anymore if it was anonymized. Are you suggesting to make the example:\n>\n> git rev-list -- foo.c\n>\n> by itself?\n\nSorry, I meant to provide an example like this:\n\n For example, if you have a bug which reproduces with `git rev-list\n sensitive -- secret.c`, you can run:\n\n $ git fast-export --anonymize --all \\\n --seed-anonymized=sensitive:foo \\\n --seed-anonymized=secret.c:bar.c \\\n >stream\n\n After importing the stream, you can then run `git rev-list foo --\n bar.c` in the anonymized repository.\n\n> > > +Note that paths and refnames are split into tokens at slash boundaries.\n> > > +The command above would anonymize `subdir/foo.c` as something like\n> > > +`path123/secret.c`.\n> >\n> > Confusing. This seems to be saying that anonymizing filenames in\n> > subdirectories is pointless because you can't know how the leading\n> > directory names will be anonymized. That leaves the reader wondering\n> > how to deal with the situation. Does it require using\n> > --seed-anonymized for each path component leading up to the filename?\n>\n> You can do that, but I think it would be simpler to just find \"secret.c\"\n> in the anonymized repo (either in the checkout, or just \"git ls-tree\n> -r\").\n>\n> > Or can --seed-anonymized take an full pathname (leading directory\n> > components and filename) in one shot?\n>\n> No, it can't. Suggested wording? That's what I was trying to say with\n> the above sentence.\n\nHmm, perhaps your original attempt can be extended slightly to state\nit more explicitly?\n\n Note that paths and refnames are split into tokens at slash\n boundaries. The command above would anonymize `subdir/foo.c` as\n something like `path123/secret.c`; you could then search for\n `secret.c` in the anonymized repository to determine the final\n pathname.\n\n To make referencing the final pathname simpler, you can seed\n anonymization for each path component; so, if you also anonymize\n `subdir` to `publicdir`, then the final pathname would be\n `publicdir/secret.c`.\n\nThis makes me wonder if --seed-anonymized should do its own\ntokenization so that --seed-anonymized=subdir/foo:public/bar is\nautomatically understood as anonymizing \"subdir\" to \"public\" _and_\n\"foo\" to \"bar\". But that potentially gets weird if you say:\n\n --seed-anonymized=a/b:q/p --seed-anonymized=a/c:y/z\n\nin which case you've given conflicting replacements for \"a\". (I\nsuppose it could issue a warning message in that case.)\n\n> > Would it be worthwhile to add a check somewhere after the\n> > parse_options() invocation and complain if --seed-anonymized was used\n> > without --anonymize? (Or should --seed-anonymized perhaps imply\n> > --anonymize?)\n>\n> I thought about implying, but I have a slight preference to err on the\n> side of making things less magical. I don't mind triggering a warning or\n> error, but it's not like anything _bad_ happens if you don't say\n> --anonymize. It just doesn't do anything, which seems like a perfectly\n> logical outcome.\n\nLack of a warning or error could be kind of bad if the person doesn't\ncheck the fast-export file before sending it out and only discovers\nlater that:\n\n git fast-export --seed-anonymized=foo:bar\n\ndidn't perform _any_ anonymization at all.\n"},{"id":"400493","messageId":"20200624114313.GJ2898@szeder.dev","threadId":"53716","inReplyTo":"20200623152451.GC1435482@coredump.intra.peff.net","subject":"Re: [PATCH 03/10] fast-export: store anonymized oids as hex strings","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-06-24T11:43:13Z","receivedAt":"2020-06-24T11:43:22Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Jun 23, 2020 at 11:24:51AM -0400, Jeff King wrote:\n\n> diff --git a/builtin/fast-export.c b/builtin/fast-export.c\n> index 289395a131..4a3a4c933e 100644\n> --- a/builtin/fast-export.c\n> +++ b/builtin/fast-export.c\n> @@ -387,16 +387,19 @@ static void *generate_fake_oid(const void *old, size_t *len)\n> {\n> \tstatic uint32_t counter = 1; /* avoid null oid */\n> \tconst unsigned hashsz = the_hash_algo->rawsz;\n> -\tunsigned char *out = xcalloc(hashsz, 1);\n> -\tput_be32(out + hashsz - 4, counter++);\n> -\treturn out;\n> +\tstruct object_id oid;\n> +\tchar *hex = xmallocz(GIT_MAX_HEXSZ);\n> +\n> +\toidclr(&oid);\n> +\tput_be32(oid.hash + hashsz - 4, counter++);\n> +\treturn oid_to_hex_r(hex, &oid);\n> }\n\nGCC 4.8 complains about this change in our CI job:\n\n $ make CC=gcc-4.8 builtin/fast-export.o\n CC builtin/fast-export.o\n builtin/fast-export.c: In function ‘generate_fake_oid’:\n builtin/fast-export.c:394:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]\n put_be32(oid.hash + hashsz - 4, counter++);\n ^\n cc1: all warnings being treated as errors\n\n\n"},{"id":"400530","messageId":"20200624154740.GA2088459@coredump.intra.peff.net","threadId":"53716","inReplyTo":"CAPig+cTFkAOyLG1Sm_p11GgH9Ms87_7zs-7kFbEYZ-uXg1yrYw@mail.gmail.com","subject":"Re: [PATCH 09/10] fast-export: allow seeding the anonymized mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-24T15:47:40Z","receivedAt":"2020-06-24T15:47:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 23, 2020 at 04:30:23PM -0400, Eric Sunshine wrote:\n\n> > I'm not sure what you'd write, then. You can't mention \"mybranch\"\n> > anymore if it was anonymized. Are you suggesting to make the example:\n> >\n> > git rev-list -- foo.c\n> >\n> > by itself?\n> \n> Sorry, I meant to provide an example like this:\n> \n> For example, if you have a bug which reproduces with `git rev-list\n> sensitive -- secret.c`, you can run:\n> \n> $ git fast-export --anonymize --all \\\n> --seed-anonymized=sensitive:foo \\\n> --seed-anonymized=secret.c:bar.c \\\n> >stream\n> \n> After importing the stream, you can then run `git rev-list foo --\n> bar.c` in the anonymized repository.\n\nThanks, that makes sense. I took this as-is for my reroll (modulo the\nchange of option name discussed elsewhere).\n\n> Hmm, perhaps your original attempt can be extended slightly to state\n> it more explicitly?\n> \n> Note that paths and refnames are split into tokens at slash\n> boundaries. The command above would anonymize `subdir/foo.c` as\n> something like `path123/secret.c`; you could then search for\n> `secret.c` in the anonymized repository to determine the final\n> pathname.\n> \n> To make referencing the final pathname simpler, you can seed\n> anonymization for each path component; so, if you also anonymize\n> `subdir` to `publicdir`, then the final pathname would be\n> `publicdir/secret.c`.\n\nThanks, I took this modulo some fixups to match the example above, and\nto avoid the use of the word \"seed\" based on our other discussion.\n\n> This makes me wonder if --seed-anonymized should do its own\n> tokenization so that --seed-anonymized=subdir/foo:public/bar is\n> automatically understood as anonymizing \"subdir\" to \"public\" _and_\n> \"foo\" to \"bar\". But that potentially gets weird if you say:\n> \n> --seed-anonymized=a/b:q/p --seed-anonymized=a/c:y/z\n> \n> in which case you've given conflicting replacements for \"a\". (I\n> suppose it could issue a warning message in that case.)\n\nRight, I think you get into weird corner cases. Another issue is that\nnot all items are tokenized (e.g., if your author name was foo/bar,\nyou'd want that replaced as a whole). Probably you could add both the\nbroken-down and full inputs. Yet another issue is that you can't add a\ntoken with a \":\" due to the syntax.\n\nThis is an infrequently-enough-used feature that I think it's worth\nkeeping things simple, even if they're a little less convenient to\ninvoke.\n\n> Lack of a warning or error could be kind of bad if the person doesn't\n> check the fast-export file before sending it out and only discovers\n> later that:\n> \n> git fast-export --seed-anonymized=foo:bar\n> \n> didn't perform _any_ anonymization at all.\n\nGood point. I'd hope people would glance at the output before sending it\nout, but given that it's a potential safety issue, it probably is worth\ndetecting this case. I'll add it to my re-roll.\n\n-Peff\n"},{"id":"400531","messageId":"20200624154853.GB2088459@coredump.intra.peff.net","threadId":"53716","inReplyTo":"CAPig+cTBwZJnUYqeEjXp-hAOjHb783B+9kyy+h+HtiOTy_c_hg@mail.gmail.com","subject":"Re: [PATCH 09/10] fast-export: allow seeding the anonymized mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-24T15:48:53Z","receivedAt":"2020-06-24T15:48:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 23, 2020 at 04:35:37PM -0400, Eric Sunshine wrote:\n\n> > I wrestled with the name, and I agree \"seed\" is overly technical. And I\n> > came up with many similar variations of \"anonymize-to\", but they all\n> > seemed ambiguous (e.g., it could be \"to\" a file that we're storing the\n> > data in).\n> >\n> > Perhaps \"--anonymize-map\" would be less technical?\n> \n> That's not too bad. It is better than --seed-anonymized. I haven't\n> come up with any name which improves upon it.\n\nI went with that in my reroll, and avoided using the word \"seed\" at all\nin the documentation (I did keep the name \"anonymized_seeds\" as the\ninternal variable name for the hashmap, since just calling it \"map\"\nthere is ambiguous with all of the other maps).\n\n-Peff\n"},{"id":"400534","messageId":"20200624155420.GC2088459@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200624114313.GJ2898@szeder.dev","subject":"Re: [PATCH 03/10] fast-export: store anonymized oids as hex strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-24T15:54:20Z","receivedAt":"2020-06-24T15:54:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 24, 2020 at 01:43:13PM +0200, SZEDER Gábor wrote:\n\n> > \tstatic uint32_t counter = 1; /* avoid null oid */\n> > \tconst unsigned hashsz = the_hash_algo->rawsz;\n> > -\tunsigned char *out = xcalloc(hashsz, 1);\n> > -\tput_be32(out + hashsz - 4, counter++);\n> > -\treturn out;\n> > +\tstruct object_id oid;\n> > +\tchar *hex = xmallocz(GIT_MAX_HEXSZ);\n> > +\n> > +\toidclr(&oid);\n> > +\tput_be32(oid.hash + hashsz - 4, counter++);\n> > +\treturn oid_to_hex_r(hex, &oid);\n> > }\n> \n> GCC 4.8 complains about this change in our CI job:\n> \n> $ make CC=gcc-4.8 builtin/fast-export.o\n> CC builtin/fast-export.o\n> builtin/fast-export.c: In function ‘generate_fake_oid’:\n> builtin/fast-export.c:394:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]\n> put_be32(oid.hash + hashsz - 4, counter++);\n> ^\n> cc1: all warnings being treated as errors\n\nInteresting. The only change on this line is swapping out the local\n\"unsigned char *\" for an object_id, which also stores an \"unsigned\nchar\" (though as an array). And while put_be32() takes a void pointer,\nit's inlined and treats it the argument an \"unsigned char *\" internally.\nSo I'm not sure I see that any type punning is going on at all.\n\nI'll see if I can appease the compiler, though.\n\n-Peff\n"},{"id":"400556","messageId":"xmqqzh8s5j7u.fsf@gitster.c.googlers.com","threadId":"53716","inReplyTo":"20200623152503.GH1435482@coredump.intra.peff.net","subject":"Re: [PATCH 08/10] fast-export: add a \"data\" callback parameter to anonymize_str()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-24T19:58:45Z","receivedAt":"2020-06-24T19:58:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> The anonymize_str() function takes a generator callback, but there's no\n> way to pass extra context to it. Let's add the usual \"void *data\"\n> parameter to the generator interface and pass it along.\n>\n> This is mildly annoying for existing callers, all of which pass NULL,\n> but is necessary to avoid extra globals in some cases we'll add in a\n> subsequent patch.\n\n> While we're touching each of these callbacks, we can further observe\n> that none of them use the existing orig/len parameters at all. This\n> makes sense, since the point is for their output to have no discernable\n> basis in the original (my original version had some notion that we might\n> use a one-way function to obfuscate the names, but it was never\n> implemented). So let's drop those extra parameters. If a caller really\n> wants to do something with them, it can pass a struct through the new\n> data parameter.\n\nI guess it was giving a perfect proof that the anonymization is\ngood---you cannot leak the info in data you did not even look at\n;-).\n\nAnd we must keep passing the pair to the anonymize API,\nof course, because that would be used as a look-up key for the\ncustomized/seeded mapping, which makes sense. But of course it is\nnot necessary to pass them to the lower-level \"generate\" callbacks.\n\n"},{"id":"400613","messageId":"20200625154948.GA3968780@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200624155420.GC2088459@coredump.intra.peff.net","subject":"Re: [PATCH 03/10] fast-export: store anonymized oids as hex strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-25T15:49:48Z","receivedAt":"2020-06-25T15:49:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 24, 2020 at 11:54:20AM -0400, Jeff King wrote:\n\n> > GCC 4.8 complains about this change in our CI job:\n> > \n> > $ make CC=gcc-4.8 builtin/fast-export.o\n> > CC builtin/fast-export.o\n> > builtin/fast-export.c: In function ‘generate_fake_oid’:\n> > builtin/fast-export.c:394:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]\n> > put_be32(oid.hash + hashsz - 4, counter++);\n> > ^\n> > cc1: all warnings being treated as errors\n> \n> Interesting. The only change on this line is swapping out the local\n> \"unsigned char *\" for an object_id, which also stores an \"unsigned\n> char\" (though as an array). And while put_be32() takes a void pointer,\n> it's inlined and treats it the argument an \"unsigned char *\" internally.\n> So I'm not sure I see that any type punning is going on at all.\n> \n> I'll see if I can appease the compiler, though.\n\nHmm. Switching to a local array makes the warning go away:\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 4a3a4c93..eae2ec5 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -387,12 +387,12 @@ static void *generate_fake_oid(const void *old, size_t *len)\n {\n \tstatic uint32_t counter = 1; /* avoid null oid */\n \tconst unsigned hashsz = the_hash_algo->rawsz;\n-\tstruct object_id oid;\n+\tunsigned char out[GIT_MAX_RAWSZ];\n \tchar *hex = xmallocz(GIT_MAX_HEXSZ);\n \n-\toidclr(&oid);\n-\tput_be32(oid.hash + hashsz - 4, counter++);\n-\treturn oid_to_hex_r(hex, &oid);\n+\thashclr(out);\n+\tput_be32(out + hashsz - 4, counter++);\n+\treturn hash_to_hex_algop_r(hex, out, the_hash_algo);\n }\n \n static const char *anonymize_oid(const char *oid_hex)\n\nat the cost of some uglier use of the_hash_algo and RAWSZ. But\ncuriously, even with the array, if I adjust the write only slightly to\nwrite at the begining instead of the end (we don't really care where our\ncounter appears in the hash, since the point is to anonymize):\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex eae2ec5..1f52247 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -386,12 +386,11 @@ static void print_path(const char *path)\n static void *generate_fake_oid(const void *old, size_t *len)\n {\n \tstatic uint32_t counter = 1; /* avoid null oid */\n-\tconst unsigned hashsz = the_hash_algo->rawsz;\n \tunsigned char out[GIT_MAX_RAWSZ];\n \tchar *hex = xmallocz(GIT_MAX_HEXSZ);\n \n \thashclr(out);\n-\tput_be32(out + hashsz - 4, counter++);\n+\tput_be32(out, counter++);\n \treturn hash_to_hex_algop_r(hex, out, the_hash_algo);\n }\n \n\n...then the warning comes back. I tried that because I was wondering\nwhether the same thing might make the warning go away on the object_id\nversion, but it doesn't.\n\nSo this really seems like a pointless false positive from the compiler,\nand it's a rather old compiler (the warning no longer triggers in gcc 6\nand up). Is it worth caring about? Ubuntu Trusty is out of standard\nsupport but not fully EOL'd until 2022. Debian jessie has gcc 4.9\nwhich triggers the warning, but will hit EOL in 5 days. If it were an\nactual breakage I'd be more concerned, but keeping such an old compiler\n-Werror clean doesn't seem that important.\n\nI'd note also that none of the Actions CI jobs run with this compiler\nversion. If we _do_ want to care about it, it might be worth covering it\nthere.\n\n-Peff\n"},{"id":"400628","messageId":"20200625194802.GA4028913@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200623152436.GA50925@coredump.intra.peff.net","subject":"[PATCH v2 0/11] fast-export: allow seeding the anonymized mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-25T19:48:02Z","receivedAt":"2020-06-25T19:48:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Here's a re-roll of the \"--seed-anonymize\" feature, incorporating\nchanges from Eric's review.\n\nHere's a summary of the changes. Patches 1-8 are unchanged:\n\n [01/11]: t9351: derive anonymized tree checks from original repo\n [02/11]: fast-export: use xmemdupz() for anonymizing oids\n [03/11]: fast-export: store anonymized oids as hex strings\n [04/11]: fast-export: tighten anonymize_mem() interface to handle only strings\n [05/11]: fast-export: stop storing lengths in anonymized hashmaps\n [06/11]: fast-export: use a flex array to store anonymized entries\n [07/11]: fast-export: move global \"idents\" anonymize hashmap into function\n [08/11]: fast-export: add a \"data\" callback parameter to anonymize_str()\n\n [09/11]: fast-export: allow seeding the anonymized mapping\n\n - the option is now called \"--anonymize-map\", and the word \"seed\"\n isn't used in any user-facing documentation\n\n - incorporated Eric's documentation suggestions\n\n - --anonymize-map without --anonymize now triggers an error\n\n [10/11]: fast-export: anonymize \"master\" refname\n\n - minor adjustments to handle change of option name\n\n [11/11]: fast-export: use local array to store anonymized oid\n\n - new in this iteration; this address the gcc-4 warning that Gábor\n mentioned. I prepared it on top since I think we'd eventually want\n to revert it once we decide that compiler is too old (and I'd be\n perfectly fine to declare that it's so now, and just never apply\n it at all).\n\nFull range diff is below.\n\n 1: 65dca0c684 = 1: dd2668d7fe t9351: derive anonymized tree checks from original repo\n 2: bc79b92850 = 2: 46a07c62e1 fast-export: use xmemdupz() for anonymizing oids\n 3: f5b34b3730 = 3: a7be6c4703 fast-export: store anonymized oids as hex strings\n 4: 5982a5c764 = 4: 712e481e4e fast-export: tighten anonymize_mem() interface to handle only strings\n 5: d6b7a75a13 = 5: 1ad28f560a fast-export: stop storing lengths in anonymized hashmaps\n 6: f897a949b1 = 6: b6456c0c7b fast-export: use a flex array to store anonymized entries\n 7: e5465fb8d8 = 7: 81d478c12b fast-export: move global \"idents\" anonymize hashmap into function\n 8: 8dd74b95b7 = 8: 0d34f57a94 fast-export: add a \"data\" callback parameter to anonymize_str()\n 9: c7aec82ba5 ! 9: 64e3afe46f fast-export: allow seeding the anonymized mapping\n @@ Commit message\n does work, but it's a bit awkward to use (you have to manually dig the\n items you care about out of the mapping).\n \n + Helped-by: Eric Sunshine \n Signed-off-by: Jeff King \n \n ## Documentation/git-fast-export.txt ##\n @@ Documentation/git-fast-export.txt: by keeping the marks the same across runs.\n \tthe shape of the history and stored tree. See the section on\n \t`ANONYMIZING` below.\n \n -+--seed-anonymized=[:]::\n ++--anonymize-map=[:]::\n +\tConvert token `` to `` in the anonymized output. If\n +\t`` is omitted, map `` to itself (i.e., do not\n +\tanonymize it). See the section on `ANONYMIZING` below.\n @@ Documentation/git-fast-export.txt: collapse \"User 0\", \"User 1\", etc into \"User X\n +paths, which becomes challenging after refnames and paths have been\n +anonymized. You can ask for a particular token to be left as-is or\n +mapped to a new value. For example, if you have a bug which reproduces\n -+with `git rev-list mybranch -- foo.c`, you can run:\n ++with `git rev-list sensitive -- secret.c`, you can run:\n +\n +---------------------------------------------------\n +$ git fast-export --anonymize --all \\\n -+ --seed-anonymized=foo.c:secret.c \\\n -+ --seed-anonymized=mybranch \\\n ++ --anonymize-map=sensitive:foo \\\n ++ --anonymize-map=secret.c:bar.c \\\n + >stream\n +---------------------------------------------------\n +\n -+After importing the stream, you can then run `git rev-list mybranch --\n -+secret.c` in the anonymized repository.\n ++After importing the stream, you can then run `git rev-list foo -- bar.c`\n ++in the anonymized repository.\n +\n +Note that paths and refnames are split into tokens at slash boundaries.\n -+The command above would anonymize `subdir/foo.c` as something like\n -+`path123/secret.c`.\n ++The command above would anonymize `subdir/secret.c` as something like\n ++`path123/bar.c`; you could then search for `bar.c` in the anonymized\n ++repository to determine the final pathname.\n ++\n ++To make referencing the final pathname simpler, you can map each path\n ++component; so if you also anonymize `subdir` to `publicdir`, then the\n ++final pathname would be `publicdir/bar.c`.\n \n LIMITATIONS\n -----------\n @@ builtin/fast-export.c: static void handle_deletes(void)\n +\treturn xstrdup(data);\n +}\n +\n -+static int parse_opt_seed_anonymized(const struct option *opt,\n -+\t\t\t\t const char *arg, int unset)\n ++static int parse_opt_anonymize_map(const struct option *opt,\n ++\t\t\t\t const char *arg, int unset)\n +{\n +\tstruct hashmap *map = opt->value;\n +\tconst char *delim, *value;\n @@ builtin/fast-export.c: static void handle_deletes(void)\n +\t}\n +\n +\tif (!keylen || !*value)\n -+\t\treturn error(_(\"--seed-anonymized token cannot be empty\"));\n ++\t\treturn error(_(\"--anonymize-map token cannot be empty\"));\n +\n +\tanonymize_str(map, anonymize_seed, arg, keylen, (void *)value);\n +\n @@ builtin/fast-export.c: int cmd_fast_export(int argc, const char **argv, const ch\n \t\tOPT_STRING_LIST(0, \"refspec\", &refspecs_list, N_(\"refspec\"),\n \t\t\t N_(\"Apply refspec to exported refs\")),\n \t\tOPT_BOOL(0, \"anonymize\", &anonymize, N_(\"anonymize output\")),\n -+\t\tOPT_CALLBACK_F(0, \"seed-anonymized\", &anonymized_seeds, N_(\"from:to\"),\n ++\t\tOPT_CALLBACK_F(0, \"anonymize-map\", &anonymized_seeds, N_(\"from:to\"),\n +\t\t\t N_(\"convert to in anonymized output\"),\n -+\t\t\t PARSE_OPT_NONEG, parse_opt_seed_anonymized),\n ++\t\t\t PARSE_OPT_NONEG, parse_opt_anonymize_map),\n \t\tOPT_BOOL(0, \"reference-excluded-parents\",\n \t\t\t &reference_excluded_commits, N_(\"Reference parents which are not in fast-export stream by object id\")),\n \t\tOPT_BOOL(0, \"show-original-ids\", &show_original_ids,\n +@@ builtin/fast-export.c: int cmd_fast_export(int argc, const char **argv, const char *prefix)\n + \tif (argc > 1)\n + \t\tusage_with_options (fast_export_usage, options);\n + \n ++\tif (anonymized_seeds.cmpfn && !anonymize)\n ++\t\tdie(_(\"--anonymize-map without --anonymize does not make sense\"));\n ++\n + \tif (refspecs_list.nr) {\n + \t\tint i;\n + \n \n ## t/t9351-fast-export-anonymize.sh ##\n @@ t/t9351-fast-export-anonymize.sh: test_description='basic tests for fast-export --anonymize'\n @@ t/t9351-fast-export-anonymize.sh: test_expect_success 'setup simple repo' '\n test_expect_success 'export anonymized stream' '\n -\tgit fast-export --anonymize --all >stream\n +\tgit fast-export --anonymize --all \\\n -+\t\t--seed-anonymized=retain-me \\\n -+\t\t--seed-anonymized=xyzzy:custom-name \\\n ++\t\t--anonymize-map=retain-me \\\n ++\t\t--anonymize-map=xyzzy:custom-name \\\n +\t\t>stream\n '\n \n10: 59737b97de ! 10: 103f5bcd65 fast-export: anonymize \"master\" refname\n @@ Commit message\n \n Note that we have to adjust the test a bit, since it relied on using the\n name \"master\" in the anonymized repos. We could just use\n - --seed-anonymized=master to keep the same output, but then we wouldn't\n + --anonymize-map=master to keep the same output, but then we wouldn't\n know if it works because of our hard-coded master or because of the\n - seeding.\n + explicit map.\n \n So let's flip the test a bit, and confirm that we anonymize \"master\",\n but keep \"other\" in the output.\n @@ builtin/fast-export.c: static const char *anonymize_refname(const char *refname)\n ## t/t9351-fast-export-anonymize.sh ##\n @@ t/t9351-fast-export-anonymize.sh: test_expect_success 'export anonymized stream' '\n \tgit fast-export --anonymize --all \\\n - \t\t--seed-anonymized=retain-me \\\n - \t\t--seed-anonymized=xyzzy:custom-name \\\n -+\t\t--seed-anonymized=other \\\n + \t\t--anonymize-map=retain-me \\\n + \t\t--anonymize-map=xyzzy:custom-name \\\n ++\t\t--anonymize-map=other \\\n \t\t>stream\n '\n \n -: ---------- > 11: f2640407c5 fast-export: use local array to store anonymized oid\n"},{"id":"400629","messageId":"20200625194814.GA4029374@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200625194802.GA4028913@coredump.intra.peff.net","subject":"[PATCH v2 01/11] t9351: derive anonymized tree checks from original repo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-25T19:48:14Z","receivedAt":"2020-06-25T19:48:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Our tests of the anonymized repo just hard-code the expected set of\nobjects in the root and subdirectory trees. This makes them brittle to\nthe test setup changing (e.g., adding new paths that need tested).\n\nLet's look at the original repo to compute our expected set of objects.\nNote that this isn't completely perfect (e.g., we still rely on there\nbeing only one tree in the root), but it does simplify later patches.\n\nSigned-off-by: Jeff King \n---\n t/t9351-fast-export-anonymize.sh | 16 ++++++----------\n 1 file changed, 6 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex 897dc50907..e772cf9930 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -71,22 +71,18 @@ test_expect_success 'repo has original shape and timestamps' '\n \n test_expect_success 'root tree has original shape' '\n \t# the output entries are not necessarily in the same\n-\t# order, but we know at least that we will have one tree\n-\t# and one blob, so just check the sorted order\n-\tcat >expect <<-\\EOF &&\n-\tblob\n-\ttree\n-\tEOF\n+\t# order, but we should at least have the same set of\n+\t# object types.\n+\tgit -C .. ls-tree HEAD >orig-root &&\n+\tcut -d\" \" -f2 expect &&\n \tgit ls-tree $other_branch >root &&\n \tcut -d\" \" -f2 actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'paths in subdir ended up in one tree' '\n-\tcat >expect <<-\\EOF &&\n-\tblob\n-\tblob\n-\tEOF\n+\tgit -C .. ls-tree other:subdir >orig-subdir &&\n+\tcut -d\" \" -f2 expect &&\n \ttree=$(grep tree root | cut -f2) &&\n \tgit ls-tree $other_branch:$tree >tree &&\n \tcut -d\" \" -f2 actual &&\n-- \n2.27.0.593.gb3082a2aaf\n\n"},{"id":"400630","messageId":"20200625194817.GB4029374@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200625194802.GA4028913@coredump.intra.peff.net","subject":"[PATCH v2 02/11] fast-export: use xmemdupz() for anonymizing oids","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-25T19:48:17Z","receivedAt":"2020-06-25T19:48:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Our anonymize_mem() function is careful to take a ptr/len pair to allow\nstoring binary tokens like object ids, as well as partial strings (e.g.,\njust \"foo\" of \"foo/bar\"). But it duplicates the hash key using\nxstrdup()! That means that:\n\n - for a partial string, we'd store all bytes up to the NUL, even\n though we'd never look at anything past \"len\". This didn't produce\n wrong behavior, but was wasteful.\n\n - for a binary oid that doesn't contain a zero byte, we'd copy garbage\n bytes off the end of the array (though as long as nothing complained\n about reading uninitialized bytes, further reads would be limited by\n \"len\", and we'd produce the correct results)\n\n - for a binary oid that does contain a zero byte, we'd copy _fewer_\n bytes than intended into the hashmap struct. When we later try to\n look up a value, we'd access uninitialized memory and potentially\n falsely claim that a particular oid is not present.\n\nThe most common reason to store an oid is an anonymized gitlink, but our\ntest case doesn't have any gitlinks at all. So let's add one whose oid\ncontains a NUL and is present at two different paths. ASan catches the\nmemory error, but even without it we can detect the bug because the oid\nis not anonymized the same way for both paths.\n\nAnd of course the fix is to copy the correct number of bytes. We don't\ntechnically need the appended NUL from xmemdupz(), but it doesn't hurt\nas an extra protection against anybody treating it like a string (plus a\nfuture patch will push us more in that direction).\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 2 +-\n t/t9351-fast-export-anonymize.sh | 15 +++++++++++++++\n 2 files changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 85868162ee..289395a131 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -162,7 +162,7 @@ static const void *anonymize_mem(struct hashmap *map,\n \tif (!ret) {\n \t\tret = xmalloc(sizeof(*ret));\n \t\thashmap_entry_init(&ret->hash, key.hash.hash);\n-\t\tret->orig = xstrdup(orig);\n+\t\tret->orig = xmemdupz(orig, *len);\n \t\tret->orig_len = *len;\n \t\tret->anon = generate(orig, len);\n \t\tret->anon_len = *len;\ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex e772cf9930..dc5d75cd19 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -10,6 +10,10 @@ test_expect_success 'setup simple repo' '\n \tmkdir subdir &&\n \ttest_commit subdir/bar &&\n \ttest_commit subdir/xyzzy &&\n+\tfake_commit=$(echo $ZERO_OID | sed s/0/a/) &&\n+\tgit update-index --add --cacheinfo 160000,$fake_commit,link1 &&\n+\tgit update-index --add --cacheinfo 160000,$fake_commit,link2 &&\n+\tgit commit -m \"add gitlink\" &&\n \tgit tag -m \"annotated tag\" mytag\n '\n \n@@ -26,6 +30,12 @@ test_expect_success 'stream omits path names' '\n \t! grep xyzzy stream\n '\n \n+test_expect_success 'stream omits gitlink oids' '\n+\t# avoid relying on the whole oid to remain hash-agnostic; this is\n+\t# plenty to be unique within our test case\n+\t! grep a000000000000000000 stream\n+'\n+\n test_expect_success 'stream allows master as refname' '\n \tgrep master stream\n '\n@@ -89,6 +99,11 @@ test_expect_success 'paths in subdir ended up in one tree' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'identical gitlinks got identical oid' '\n+\tawk \"/commit/ { print \\$3 }\" commits &&\n+\ttest_line_count = 1 commits\n+'\n+\n test_expect_success 'tag points to branch tip' '\n \tgit rev-parse $other_branch >expect &&\n \tgit for-each-ref --format=\"%(*objectname)\" | grep . >actual &&\n-- \n2.27.0.593.gb3082a2aaf\n\n"},{"id":"400631","messageId":"20200625194819.GC4029374@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200625194802.GA4028913@coredump.intra.peff.net","subject":"[PATCH v2 03/11] fast-export: store anonymized oids as hex strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-25T19:48:19Z","receivedAt":"2020-06-25T19:48:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When fast-export stores anonymized oids, it does so as binary strings.\nAnd while the anonymous mapping storage is binary-clean (at least as of\nthe previous commit), this will become awkward when we start exposing\nmore of it to the user. In particular, if we allow a method for\nretaining token \"foo\", then users may want to specify a hex oid as such\na token.\n\nLet's just switch to storing the hex strings. The difference in memory\nusage is negligible (especially considering how infrequently we'd\ngenerally store an oid compared to, say, path components).\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 28 ++++++++++++++++------------\n 1 file changed, 16 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 289395a131..4a3a4c933e 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -387,16 +387,19 @@ static void *generate_fake_oid(const void *old, size_t *len)\n {\n \tstatic uint32_t counter = 1; /* avoid null oid */\n \tconst unsigned hashsz = the_hash_algo->rawsz;\n-\tunsigned char *out = xcalloc(hashsz, 1);\n-\tput_be32(out + hashsz - 4, counter++);\n-\treturn out;\n+\tstruct object_id oid;\n+\tchar *hex = xmallocz(GIT_MAX_HEXSZ);\n+\n+\toidclr(&oid);\n+\tput_be32(oid.hash + hashsz - 4, counter++);\n+\treturn oid_to_hex_r(hex, &oid);\n }\n \n-static const struct object_id *anonymize_oid(const struct object_id *oid)\n+static const char *anonymize_oid(const char *oid_hex)\n {\n \tstatic struct hashmap objs;\n-\tsize_t len = the_hash_algo->rawsz;\n-\treturn anonymize_mem(&objs, generate_fake_oid, oid, &len);\n+\tsize_t len = strlen(oid_hex);\n+\treturn anonymize_mem(&objs, generate_fake_oid, oid_hex, &len);\n }\n \n static void show_filemodify(struct diff_queue_struct *q,\n@@ -455,9 +458,9 @@ static void show_filemodify(struct diff_queue_struct *q,\n \t\t\t */\n \t\t\tif (no_data || S_ISGITLINK(spec->mode))\n \t\t\t\tprintf(\"M %06o %s \", spec->mode,\n-\t\t\t\t oid_to_hex(anonymize ?\n-\t\t\t\t\t\t anonymize_oid(&spec->oid) :\n-\t\t\t\t\t\t &spec->oid));\n+\t\t\t\t anonymize ?\n+\t\t\t\t anonymize_oid(oid_to_hex(&spec->oid)) :\n+\t\t\t\t oid_to_hex(&spec->oid));\n \t\t\telse {\n \t\t\t\tstruct object *object = lookup_object(the_repository,\n \t\t\t\t\t\t\t\t &spec->oid);\n@@ -712,9 +715,10 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \t\tif (mark)\n \t\t\tprintf(\":%d\\n\", mark);\n \t\telse\n-\t\t\tprintf(\"%s\\n\", oid_to_hex(anonymize ?\n-\t\t\t\t\t\t anonymize_oid(&obj->oid) :\n-\t\t\t\t\t\t &obj->oid));\n+\t\t\tprintf(\"%s\\n\",\n+\t\t\t anonymize ?\n+\t\t\t anonymize_oid(oid_to_hex(&obj->oid)) :\n+\t\t\t oid_to_hex(&obj->oid));\n \t\ti++;\n \t}\n \n-- \n2.27.0.593.gb3082a2aaf\n\n"},{"id":"400632","messageId":"20200625194823.GE4029374@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200625194802.GA4028913@coredump.intra.peff.net","subject":"[PATCH v2 05/11] fast-export: stop storing lengths in anonymized hashmaps","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-25T19:48:23Z","receivedAt":"2020-06-25T19:48:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Now that the anonymize_str() interface is restricted to NUL-terminated\nstrings, there's no need for us to keep track of the length of each\nentry in the hashmap. This simplifies the code and saves a bit of\nmemory.\n\nNote that we do still need to compare the stored results to partial\nstrings passed in by the callers. We can do that by using hashmap's\nkeydata feature to get the ptr/len pair into the comparison function,\nand then using strncmp().\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 28 ++++++++++++++++++----------\n 1 file changed, 18 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex d8ea067630..5df2ada47d 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -121,23 +121,32 @@ static int has_unshown_parent(struct commit *commit)\n struct anonymized_entry {\n \tstruct hashmap_entry hash;\n \tconst char *orig;\n-\tsize_t orig_len;\n \tconst char *anon;\n-\tsize_t anon_len;\n+};\n+\n+struct anonymized_entry_key {\n+\tstruct hashmap_entry hash;\n+\tconst char *orig;\n+\tsize_t orig_len;\n };\n \n static int anonymized_entry_cmp(const void *unused_cmp_data,\n \t\t\t\tconst struct hashmap_entry *eptr,\n \t\t\t\tconst struct hashmap_entry *entry_or_key,\n-\t\t\t\tconst void *unused_keydata)\n+\t\t\t\tconst void *keydata)\n {\n \tconst struct anonymized_entry *a, *b;\n \n \ta = container_of(eptr, const struct anonymized_entry, hash);\n-\tb = container_of(entry_or_key, const struct anonymized_entry, hash);\n+\tif (keydata) {\n+\t\tconst struct anonymized_entry_key *key = keydata;\n+\t\tint equal = !strncmp(a->orig, key->orig, key->orig_len) &&\n+\t\t\t !a->orig[key->orig_len];\n+\t\treturn !equal;\n+\t}\n \n-\treturn a->orig_len != b->orig_len ||\n-\t\tmemcmp(a->orig, b->orig, a->orig_len);\n+\tb = container_of(entry_or_key, const struct anonymized_entry, hash);\n+\treturn strcmp(a->orig, b->orig);\n }\n \n /*\n@@ -149,23 +158,22 @@ static const char *anonymize_str(struct hashmap *map,\n \t\t\t\t char *(*generate)(const char *, size_t),\n \t\t\t\t const char *orig, size_t len)\n {\n-\tstruct anonymized_entry key, *ret;\n+\tstruct anonymized_entry_key key;\n+\tstruct anonymized_entry *ret;\n \n \tif (!map->cmpfn)\n \t\thashmap_init(map, anonymized_entry_cmp, NULL, 0);\n \n \thashmap_entry_init(&key.hash, memhash(orig, len));\n \tkey.orig = orig;\n \tkey.orig_len = len;\n-\tret = hashmap_get_entry(map, &key, hash, NULL);\n+\tret = hashmap_get_entry(map, &key, hash, &key);\n \n \tif (!ret) {\n \t\tret = xmalloc(sizeof(*ret));\n \t\thashmap_entry_init(&ret->hash, key.hash.hash);\n \t\tret->orig = xmemdupz(orig, len);\n-\t\tret->orig_len = len;\n \t\tret->anon = generate(orig, len);\n-\t\tret->anon_len = strlen(ret->anon);\n \t\thashmap_put(map, &ret->hash);\n \t}\n \n-- \n2.27.0.593.gb3082a2aaf\n\n"},{"id":"400633","messageId":"20200625194825.GF4029374@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200625194802.GA4028913@coredump.intra.peff.net","subject":"[PATCH v2 06/11] fast-export: use a flex array to store anonymized entries","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-25T19:48:25Z","receivedAt":"2020-06-25T19:48:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Now that we're using a separate keydata struct for hash lookups, we have\nmore flexibility in how we allocate anonymized_entry structs. Let's push\nthe \"orig\" key into a flex member within the struct. That should save us\na few bytes of memory per entry (a pointer plus any malloc overhead),\nand may make lookups a little faster (since it's one less pointer to\nchase in the comparison function).\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 5df2ada47d..99d4a2b404 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -120,8 +120,8 @@ static int has_unshown_parent(struct commit *commit)\n \n struct anonymized_entry {\n \tstruct hashmap_entry hash;\n-\tconst char *orig;\n \tconst char *anon;\n+\tconst char orig[FLEX_ARRAY];\n };\n \n struct anonymized_entry_key {\n@@ -170,9 +170,8 @@ static const char *anonymize_str(struct hashmap *map,\n \tret = hashmap_get_entry(map, &key, hash, &key);\n \n \tif (!ret) {\n-\t\tret = xmalloc(sizeof(*ret));\n+\t\tFLEX_ALLOC_MEM(ret, orig, orig, len);\n \t\thashmap_entry_init(&ret->hash, key.hash.hash);\n-\t\tret->orig = xmemdupz(orig, len);\n \t\tret->anon = generate(orig, len);\n \t\thashmap_put(map, &ret->hash);\n \t}\n-- \n2.27.0.593.gb3082a2aaf\n\n"},{"id":"400634","messageId":"20200625194821.GD4029374@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200625194802.GA4028913@coredump.intra.peff.net","subject":"[PATCH v2 04/11] fast-export: tighten anonymize_mem() interface to handle only strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-25T19:48:21Z","receivedAt":"2020-06-25T19:48:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"While the anonymize_mem() interface _can_ store arbitrary byte\nsequences, none of the callers uses this feature (as of the previous\ncommit). We'd like to keep it that way, as we'll be exposing the\nstring-like nature of the anonymization routines to the user. So let's\ntighten up the interface a bit:\n\n - don't treat \"len\" as an out-parameter from anonymize_mem(); this\n ensures callers treat the pointer result as a NUL-terminated string\n\n - likewise, don't treat \"len\" as an out-parameter from generator\n functions\n\n - swap out \"void *\" for \"char *\" as appropriate to signal that we\n don't handle arbitrary memory\n\n - rename the function to anonymize_str()\n\nThis will also open up some optimization opportunities in a future\npatch.\n\nNote that we can't drop the \"len\" parameter entirely. Some callers do\npass in partial strings (e.g., \"foo/bar\", len=3) to avoid copying, and\nwe need to handle those still.\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 53 +++++++++++++++++++++----------------------\n 1 file changed, 26 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 4a3a4c933e..d8ea067630 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -145,31 +145,30 @@ static int anonymized_entry_cmp(const void *unused_cmp_data,\n * the same anonymized string with another. The actual generation\n * is farmed out to the generate function.\n */\n-static const void *anonymize_mem(struct hashmap *map,\n-\t\t\t\t void *(*generate)(const void *, size_t *),\n-\t\t\t\t const void *orig, size_t *len)\n+static const char *anonymize_str(struct hashmap *map,\n+\t\t\t\t char *(*generate)(const char *, size_t),\n+\t\t\t\t const char *orig, size_t len)\n {\n \tstruct anonymized_entry key, *ret;\n \n \tif (!map->cmpfn)\n \t\thashmap_init(map, anonymized_entry_cmp, NULL, 0);\n \n-\thashmap_entry_init(&key.hash, memhash(orig, *len));\n+\thashmap_entry_init(&key.hash, memhash(orig, len));\n \tkey.orig = orig;\n-\tkey.orig_len = *len;\n+\tkey.orig_len = len;\n \tret = hashmap_get_entry(map, &key, hash, NULL);\n \n \tif (!ret) {\n \t\tret = xmalloc(sizeof(*ret));\n \t\thashmap_entry_init(&ret->hash, key.hash.hash);\n-\t\tret->orig = xmemdupz(orig, *len);\n-\t\tret->orig_len = *len;\n+\t\tret->orig = xmemdupz(orig, len);\n+\t\tret->orig_len = len;\n \t\tret->anon = generate(orig, len);\n-\t\tret->anon_len = *len;\n+\t\tret->anon_len = strlen(ret->anon);\n \t\thashmap_put(map, &ret->hash);\n \t}\n \n-\t*len = ret->anon_len;\n \treturn ret->anon;\n }\n \n@@ -181,13 +180,13 @@ static const void *anonymize_mem(struct hashmap *map,\n */\n static void anonymize_path(struct strbuf *out, const char *path,\n \t\t\t struct hashmap *map,\n-\t\t\t void *(*generate)(const void *, size_t *))\n+\t\t\t char *(*generate)(const char *, size_t))\n {\n \twhile (*path) {\n \t\tconst char *end_of_component = strchrnul(path, '/');\n \t\tsize_t len = end_of_component - path;\n-\t\tconst char *c = anonymize_mem(map, generate, path, &len);\n-\t\tstrbuf_add(out, c, len);\n+\t\tconst char *c = anonymize_str(map, generate, path, len);\n+\t\tstrbuf_addstr(out, c);\n \t\tpath = end_of_component;\n \t\tif (*path)\n \t\t\tstrbuf_addch(out, *path++);\n@@ -361,12 +360,12 @@ static void print_path_1(const char *path)\n \t\tprintf(\"%s\", path);\n }\n \n-static void *anonymize_path_component(const void *path, size_t *len)\n+static char *anonymize_path_component(const char *path, size_t len)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n \tstrbuf_addf(&out, \"path%d\", counter++);\n-\treturn strbuf_detach(&out, len);\n+\treturn strbuf_detach(&out, NULL);\n }\n \n static void print_path(const char *path)\n@@ -383,7 +382,7 @@ static void print_path(const char *path)\n \t}\n }\n \n-static void *generate_fake_oid(const void *old, size_t *len)\n+static char *generate_fake_oid(const char *old, size_t len)\n {\n \tstatic uint32_t counter = 1; /* avoid null oid */\n \tconst unsigned hashsz = the_hash_algo->rawsz;\n@@ -399,7 +398,7 @@ static const char *anonymize_oid(const char *oid_hex)\n {\n \tstatic struct hashmap objs;\n \tsize_t len = strlen(oid_hex);\n-\treturn anonymize_mem(&objs, generate_fake_oid, oid_hex, &len);\n+\treturn anonymize_str(&objs, generate_fake_oid, oid_hex, len);\n }\n \n static void show_filemodify(struct diff_queue_struct *q,\n@@ -496,12 +495,12 @@ static const char *find_encoding(const char *begin, const char *end)\n \treturn bol;\n }\n \n-static void *anonymize_ref_component(const void *old, size_t *len)\n+static char *anonymize_ref_component(const char *old, size_t len)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n \tstrbuf_addf(&out, \"ref%d\", counter++);\n-\treturn strbuf_detach(&out, len);\n+\treturn strbuf_detach(&out, NULL);\n }\n \n static const char *anonymize_refname(const char *refname)\n@@ -550,13 +549,13 @@ static char *anonymize_commit_message(const char *old)\n }\n \n static struct hashmap idents;\n-static void *anonymize_ident(const void *old, size_t *len)\n+static char *anonymize_ident(const char *old, size_t len)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n \tstrbuf_addf(&out, \"User %d \", counter, counter);\n \tcounter++;\n-\treturn strbuf_detach(&out, len);\n+\treturn strbuf_detach(&out, NULL);\n }\n \n /*\n@@ -591,9 +590,9 @@ static void anonymize_ident_line(const char **beg, const char **end)\n \t\tsize_t len;\n \n \t\tlen = split.mail_end - split.name_begin;\n-\t\tident = anonymize_mem(&idents, anonymize_ident,\n-\t\t\t\t split.name_begin, &len);\n-\t\tstrbuf_add(out, ident, len);\n+\t\tident = anonymize_str(&idents, anonymize_ident,\n+\t\t\t\t split.name_begin, len);\n+\t\tstrbuf_addstr(out, ident);\n \t\tstrbuf_addch(out, ' ');\n \t\tstrbuf_add(out, split.date_begin, split.tz_end - split.date_begin);\n \t} else {\n@@ -733,12 +732,12 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \tshow_progress();\n }\n \n-static void *anonymize_tag(const void *old, size_t *len)\n+static char *anonymize_tag(const char *old, size_t len)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n \tstrbuf_addf(&out, \"tag message %d\", counter++);\n-\treturn strbuf_detach(&out, len);\n+\treturn strbuf_detach(&out, NULL);\n }\n \n static void handle_tail(struct object_array *commits, struct rev_info *revs,\n@@ -808,8 +807,8 @@ static void handle_tag(const char *name, struct tag *tag)\n \t\tname = anonymize_refname(name);\n \t\tif (message) {\n \t\t\tstatic struct hashmap tags;\n-\t\t\tmessage = anonymize_mem(&tags, anonymize_tag,\n-\t\t\t\t\t\tmessage, &message_size);\n+\t\t\tmessage = anonymize_str(&tags, anonymize_tag,\n+\t\t\t\t\t\tmessage, message_size);\n \t\t}\n \t}\n \n-- \n2.27.0.593.gb3082a2aaf\n\n"},{"id":"400635","messageId":"20200625194828.GG4029374@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200625194802.GA4028913@coredump.intra.peff.net","subject":"[PATCH v2 07/11] fast-export: move global \"idents\" anonymize hashmap into function","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-25T19:48:28Z","receivedAt":"2020-06-25T19:48:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"All of the other anonymization functions keep their static mappings\ninside the function to avoid polluting the global namespace. Let's do\nthe same for \"idents\", as nobody needs it outside of\nanonymize_ident_line().\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 99d4a2b404..16a1563e49 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -555,7 +555,6 @@ static char *anonymize_commit_message(const char *old)\n \treturn xstrfmt(\"subject %d\\n\\nbody\\n\", counter++);\n }\n \n-static struct hashmap idents;\n static char *anonymize_ident(const char *old, size_t len)\n {\n \tstatic int counter;\n@@ -572,6 +571,7 @@ static char *anonymize_ident(const char *old, size_t len)\n */\n static void anonymize_ident_line(const char **beg, const char **end)\n {\n+\tstatic struct hashmap idents;\n \tstatic struct strbuf buffers[] = { STRBUF_INIT, STRBUF_INIT };\n \tstatic unsigned which_buffer;\n \n-- \n2.27.0.593.gb3082a2aaf\n\n"},{"id":"400636","messageId":"20200625194830.GH4029374@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200625194802.GA4028913@coredump.intra.peff.net","subject":"[PATCH v2 08/11] fast-export: add a \"data\" callback parameter to anonymize_str()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-25T19:48:30Z","receivedAt":"2020-06-25T19:48:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The anonymize_str() function takes a generator callback, but there's no\nway to pass extra context to it. Let's add the usual \"void *data\"\nparameter to the generator interface and pass it along.\n\nThis is mildly annoying for existing callers, all of which pass NULL,\nbut is necessary to avoid extra globals in some cases we'll add in a\nsubsequent patch.\n\nWhile we're touching each of these callbacks, we can further observe\nthat none of them use the existing orig/len parameters at all. This\nmakes sense, since the point is for their output to have no discernable\nbasis in the original (my original version had some notion that we might\nuse a one-way function to obfuscate the names, but it was never\nimplemented). So let's drop those extra parameters. If a caller really\nwants to do something with them, it can pass a struct through the new\ndata parameter.\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 27 ++++++++++++++-------------\n 1 file changed, 14 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 16a1563e49..1cbca5b4b4 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -155,8 +155,9 @@ static int anonymized_entry_cmp(const void *unused_cmp_data,\n * is farmed out to the generate function.\n */\n static const char *anonymize_str(struct hashmap *map,\n-\t\t\t\t char *(*generate)(const char *, size_t),\n-\t\t\t\t const char *orig, size_t len)\n+\t\t\t\t char *(*generate)(void *),\n+\t\t\t\t const char *orig, size_t len,\n+\t\t\t\t void *data)\n {\n \tstruct anonymized_entry_key key;\n \tstruct anonymized_entry *ret;\n@@ -172,7 +173,7 @@ static const char *anonymize_str(struct hashmap *map,\n \tif (!ret) {\n \t\tFLEX_ALLOC_MEM(ret, orig, orig, len);\n \t\thashmap_entry_init(&ret->hash, key.hash.hash);\n-\t\tret->anon = generate(orig, len);\n+\t\tret->anon = generate(data);\n \t\thashmap_put(map, &ret->hash);\n \t}\n \n@@ -187,12 +188,12 @@ static const char *anonymize_str(struct hashmap *map,\n */\n static void anonymize_path(struct strbuf *out, const char *path,\n \t\t\t struct hashmap *map,\n-\t\t\t char *(*generate)(const char *, size_t))\n+\t\t\t char *(*generate)(void *))\n {\n \twhile (*path) {\n \t\tconst char *end_of_component = strchrnul(path, '/');\n \t\tsize_t len = end_of_component - path;\n-\t\tconst char *c = anonymize_str(map, generate, path, len);\n+\t\tconst char *c = anonymize_str(map, generate, path, len, NULL);\n \t\tstrbuf_addstr(out, c);\n \t\tpath = end_of_component;\n \t\tif (*path)\n@@ -367,7 +368,7 @@ static void print_path_1(const char *path)\n \t\tprintf(\"%s\", path);\n }\n \n-static char *anonymize_path_component(const char *path, size_t len)\n+static char *anonymize_path_component(void *data)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n@@ -389,7 +390,7 @@ static void print_path(const char *path)\n \t}\n }\n \n-static char *generate_fake_oid(const char *old, size_t len)\n+static char *generate_fake_oid(void *data)\n {\n \tstatic uint32_t counter = 1; /* avoid null oid */\n \tconst unsigned hashsz = the_hash_algo->rawsz;\n@@ -405,7 +406,7 @@ static const char *anonymize_oid(const char *oid_hex)\n {\n \tstatic struct hashmap objs;\n \tsize_t len = strlen(oid_hex);\n-\treturn anonymize_str(&objs, generate_fake_oid, oid_hex, len);\n+\treturn anonymize_str(&objs, generate_fake_oid, oid_hex, len, NULL);\n }\n \n static void show_filemodify(struct diff_queue_struct *q,\n@@ -502,7 +503,7 @@ static const char *find_encoding(const char *begin, const char *end)\n \treturn bol;\n }\n \n-static char *anonymize_ref_component(const char *old, size_t len)\n+static char *anonymize_ref_component(void *data)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n@@ -555,7 +556,7 @@ static char *anonymize_commit_message(const char *old)\n \treturn xstrfmt(\"subject %d\\n\\nbody\\n\", counter++);\n }\n \n-static char *anonymize_ident(const char *old, size_t len)\n+static char *anonymize_ident(void *data)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n@@ -598,7 +599,7 @@ static void anonymize_ident_line(const char **beg, const char **end)\n \n \t\tlen = split.mail_end - split.name_begin;\n \t\tident = anonymize_str(&idents, anonymize_ident,\n-\t\t\t\t split.name_begin, len);\n+\t\t\t\t split.name_begin, len, NULL);\n \t\tstrbuf_addstr(out, ident);\n \t\tstrbuf_addch(out, ' ');\n \t\tstrbuf_add(out, split.date_begin, split.tz_end - split.date_begin);\n@@ -739,7 +740,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \tshow_progress();\n }\n \n-static char *anonymize_tag(const char *old, size_t len)\n+static char *anonymize_tag(void *data)\n {\n \tstatic int counter;\n \tstruct strbuf out = STRBUF_INIT;\n@@ -815,7 +816,7 @@ static void handle_tag(const char *name, struct tag *tag)\n \t\tif (message) {\n \t\t\tstatic struct hashmap tags;\n \t\t\tmessage = anonymize_str(&tags, anonymize_tag,\n-\t\t\t\t\t\tmessage, message_size);\n+\t\t\t\t\t\tmessage, message_size, NULL);\n \t\t}\n \t}\n \n-- \n2.27.0.593.gb3082a2aaf\n\n"},{"id":"400637","messageId":"20200625194832.GI4029374@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200625194802.GA4028913@coredump.intra.peff.net","subject":"[PATCH v2 09/11] fast-export: allow seeding the anonymized mapping","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-25T19:48:32Z","receivedAt":"2020-06-25T19:48:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"After you anonymize a repository, it can be hard to find which commits\ncorrespond between the original and the result, and thus hard to\nreproduce commands that triggered bugs in the original.\n\nLet's make it possible to seed the anonymization map. This lets users\neither:\n\n - mark names to be retained as-is, if they don't consider them secret\n (in which case their original commands would just work)\n\n - map names to new values, which lets them adapt the reproduction\n recipe to the new names without revealing the originals\n\nThe implementation is fairly straight-forward. We already store each\nanonymized token in a hashmap (so that the same token appearing twice is\nconverted to the same result). We can just introduce a new \"seed\"\nhashmap which is consulted first.\n\nThis does make a few more promises to the user about how we'll anonymize\nthings (e.g., token-splitting pathnames). But it's unlikely that we'd\nwant to change those rules, even if the actual anonymization of a single\ntoken changes. And it makes things much easier for the user, who can\nunblind only a directory name without having to specify each path within\nit.\n\nOne alternative to this approach would be to anonymize as we see fit,\nand then dump the whole refname and pathname mappings to a file. This\ndoes work, but it's a bit awkward to use (you have to manually dig the\nitems you care about out of the mapping).\n\nHelped-by: Eric Sunshine \nSigned-off-by: Jeff King \n---\n Documentation/git-fast-export.txt | 29 ++++++++++++++++++\n builtin/fast-export.c | 50 ++++++++++++++++++++++++++++++-\n t/t9351-fast-export-anonymize.sh | 11 ++++++-\n 3 files changed, 88 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\nindex e8950de3ba..1978dbdc6a 100644\n--- a/Documentation/git-fast-export.txt\n+++ b/Documentation/git-fast-export.txt\n@@ -119,6 +119,11 @@ by keeping the marks the same across runs.\n \tthe shape of the history and stored tree. See the section on\n \t`ANONYMIZING` below.\n \n+--anonymize-map=[:]::\n+\tConvert token `` to `` in the anonymized output. If\n+\t`` is omitted, map `` to itself (i.e., do not\n+\tanonymize it). See the section on `ANONYMIZING` below.\n+\n --reference-excluded-parents::\n \tBy default, running a command such as `git fast-export\n \tmaster~5..master` will not include the commit master{tilde}5\n@@ -238,6 +243,30 @@ collapse \"User 0\", \"User 1\", etc into \"User X\"). This produces a much\n smaller output, and it is usually easy to quickly confirm that there is\n no private data in the stream.\n \n+Reproducing some bugs may require referencing particular commits or\n+paths, which becomes challenging after refnames and paths have been\n+anonymized. You can ask for a particular token to be left as-is or\n+mapped to a new value. For example, if you have a bug which reproduces\n+with `git rev-list sensitive -- secret.c`, you can run:\n+\n+---------------------------------------------------\n+$ git fast-export --anonymize --all \\\n+ --anonymize-map=sensitive:foo \\\n+ --anonymize-map=secret.c:bar.c \\\n+ >stream\n+---------------------------------------------------\n+\n+After importing the stream, you can then run `git rev-list foo -- bar.c`\n+in the anonymized repository.\n+\n+Note that paths and refnames are split into tokens at slash boundaries.\n+The command above would anonymize `subdir/secret.c` as something like\n+`path123/bar.c`; you could then search for `bar.c` in the anonymized\n+repository to determine the final pathname.\n+\n+To make referencing the final pathname simpler, you can map each path\n+component; so if you also anonymize `subdir` to `publicdir`, then the\n+final pathname would be `publicdir/bar.c`.\n \n LIMITATIONS\n -----------\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 1cbca5b4b4..b0b09bca30 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -45,6 +45,7 @@ static struct string_list extra_refs = STRING_LIST_INIT_NODUP;\n static struct string_list tag_refs = STRING_LIST_INIT_NODUP;\n static struct refspec refspecs = REFSPEC_INIT_FETCH;\n static int anonymize;\n+static struct hashmap anonymized_seeds;\n static struct revision_sources revision_sources;\n \n static int parse_opt_signed_tag_mode(const struct option *opt,\n@@ -168,8 +169,18 @@ static const char *anonymize_str(struct hashmap *map,\n \thashmap_entry_init(&key.hash, memhash(orig, len));\n \tkey.orig = orig;\n \tkey.orig_len = len;\n-\tret = hashmap_get_entry(map, &key, hash, &key);\n \n+\t/* First check if it's a token the user configured manually... */\n+\tif (anonymized_seeds.cmpfn)\n+\t\tret = hashmap_get_entry(&anonymized_seeds, &key, hash, &key);\n+\telse\n+\t\tret = NULL;\n+\n+\t/* ...otherwise check if we've already seen it in this context... */\n+\tif (!ret)\n+\t\tret = hashmap_get_entry(map, &key, hash, &key);\n+\n+\t/* ...and finally generate a new mapping if necessary */\n \tif (!ret) {\n \t\tFLEX_ALLOC_MEM(ret, orig, orig, len);\n \t\thashmap_entry_init(&ret->hash, key.hash.hash);\n@@ -1147,6 +1158,37 @@ static void handle_deletes(void)\n \t}\n }\n \n+static char *anonymize_seed(void *data)\n+{\n+\treturn xstrdup(data);\n+}\n+\n+static int parse_opt_anonymize_map(const struct option *opt,\n+\t\t\t\t const char *arg, int unset)\n+{\n+\tstruct hashmap *map = opt->value;\n+\tconst char *delim, *value;\n+\tsize_t keylen;\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\n+\tdelim = strchr(arg, ':');\n+\tif (delim) {\n+\t\tkeylen = delim - arg;\n+\t\tvalue = delim + 1;\n+\t} else {\n+\t\tkeylen = strlen(arg);\n+\t\tvalue = arg;\n+\t}\n+\n+\tif (!keylen || !*value)\n+\t\treturn error(_(\"--anonymize-map token cannot be empty\"));\n+\n+\tanonymize_str(map, anonymize_seed, arg, keylen, (void *)value);\n+\n+\treturn 0;\n+}\n+\n int cmd_fast_export(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info revs;\n@@ -1188,6 +1230,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING_LIST(0, \"refspec\", &refspecs_list, N_(\"refspec\"),\n \t\t\t N_(\"Apply refspec to exported refs\")),\n \t\tOPT_BOOL(0, \"anonymize\", &anonymize, N_(\"anonymize output\")),\n+\t\tOPT_CALLBACK_F(0, \"anonymize-map\", &anonymized_seeds, N_(\"from:to\"),\n+\t\t\t N_(\"convert to in anonymized output\"),\n+\t\t\t PARSE_OPT_NONEG, parse_opt_anonymize_map),\n \t\tOPT_BOOL(0, \"reference-excluded-parents\",\n \t\t\t &reference_excluded_commits, N_(\"Reference parents which are not in fast-export stream by object id\")),\n \t\tOPT_BOOL(0, \"show-original-ids\", &show_original_ids,\n@@ -1215,6 +1260,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \tif (argc > 1)\n \t\tusage_with_options (fast_export_usage, options);\n \n+\tif (anonymized_seeds.cmpfn && !anonymize)\n+\t\tdie(_(\"--anonymize-map without --anonymize does not make sense\"));\n+\n \tif (refspecs_list.nr) {\n \t\tint i;\n \ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex dc5d75cd19..5a21c71568 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -6,6 +6,7 @@ test_description='basic tests for fast-export --anonymize'\n test_expect_success 'setup simple repo' '\n \ttest_commit base &&\n \ttest_commit foo &&\n+\ttest_commit retain-me &&\n \tgit checkout -b other HEAD^ &&\n \tmkdir subdir &&\n \ttest_commit subdir/bar &&\n@@ -18,7 +19,10 @@ test_expect_success 'setup simple repo' '\n '\n \n test_expect_success 'export anonymized stream' '\n-\tgit fast-export --anonymize --all >stream\n+\tgit fast-export --anonymize --all \\\n+\t\t--anonymize-map=retain-me \\\n+\t\t--anonymize-map=xyzzy:custom-name \\\n+\t\t>stream\n '\n \n # this also covers commit messages\n@@ -30,6 +34,11 @@ test_expect_success 'stream omits path names' '\n \t! grep xyzzy stream\n '\n \n+test_expect_success 'stream contains user-specified names' '\n+\tgrep retain-me stream &&\n+\tgrep custom-name stream\n+'\n+\n test_expect_success 'stream omits gitlink oids' '\n \t# avoid relying on the whole oid to remain hash-agnostic; this is\n \t# plenty to be unique within our test case\n-- \n2.27.0.593.gb3082a2aaf\n\n"},{"id":"400639","messageId":"20200625194837.GK4029374@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200625194802.GA4028913@coredump.intra.peff.net","subject":"[PATCH v2 11/11] fast-export: use local array to store anonymized oid","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-25T19:48:37Z","receivedAt":"2020-06-25T19:48:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Some older versions of gcc complain about this line:\n\n builtin/fast-export.c:412:2: error: dereferencing type-punned pointer\n will break strict-aliasing rules [-Werror=strict-aliasing]\n put_be32(oid.hash + hashsz - 4, counter++);\n ^\n\nThis seems to be a false positive, as there's no type-punning at all\nhere. oid.hash is an array of unsigned char; when we pass it to a\nfunction it decays to a pointer to unsigned char. We do take a void\npointer in put_be32(), but it's immediately aliased with another pointer\nto unsigned char (and clearly the compiler is looking inside the inlined\nput_be32(), since the warning doesn't happen with -O0).\n\nThis happens on gcc 4.8 and 4.9, but not later versions (I tested gcc 6,\n7, 8, and 9).\n\nWe can work around it by using a local array instead of an object_id\nstruct. This is a little more intimate with the details of object_id,\nbut for whatever reason doesn't seem to trigger the compiler warning.\nWe can revert this patch once we decide that those gcc versions are too\nold to care about for a warning like this (gcc 4.8 is the default\ncompiler for Ubuntu Trusty, which is out-of-support but not fully\nend-of-life'd until April 2022).\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex c6ecf404d7..9f37895d4c 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -405,12 +405,12 @@ static char *generate_fake_oid(void *data)\n {\n \tstatic uint32_t counter = 1; /* avoid null oid */\n \tconst unsigned hashsz = the_hash_algo->rawsz;\n-\tstruct object_id oid;\n+\tunsigned char out[GIT_MAX_RAWSZ];\n \tchar *hex = xmallocz(GIT_MAX_HEXSZ);\n \n-\toidclr(&oid);\n-\tput_be32(oid.hash + hashsz - 4, counter++);\n-\treturn oid_to_hex_r(hex, &oid);\n+\thashclr(out);\n+\tput_be32(out + hashsz - 4, counter++);\n+\treturn hash_to_hex_algop_r(hex, out, the_hash_algo);\n }\n \n static const char *anonymize_oid(const char *oid_hex)\n-- \n2.27.0.593.gb3082a2aaf\n"},{"id":"400638","messageId":"20200625194835.GJ4029374@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200625194802.GA4028913@coredump.intra.peff.net","subject":"[PATCH v2 10/11] fast-export: anonymize \"master\" refname","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-25T19:48:35Z","receivedAt":"2020-06-25T19:48:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Running \"fast-export --anonymize\" will leave \"refs/heads/master\"\nuntouched in the output, for two reasons:\n\n - it helped to have some known reference point between the original\n and anonymized repository\n\n - since it's historically the default branch name, it doesn't leak any\n information\n\nNow that we can ask fast-export to retain particular tokens, we have a\nmuch better tool for the first one (because it works for any ref, not\njust master).\n\nFor the second, the notion of \"default branch name\" is likely to become\nconfigurable soon, at which point the name _does_ leak information.\nLet's drop this special case in preparation.\n\nNote that we have to adjust the test a bit, since it relied on using the\nname \"master\" in the anonymized repos. We could just use\n--anonymize-map=master to keep the same output, but then we wouldn't\nknow if it works because of our hard-coded master or because of the\nexplicit map.\n\nSo let's flip the test a bit, and confirm that we anonymize \"master\",\nbut keep \"other\" in the output.\n\nSigned-off-by: Jeff King \n---\n builtin/fast-export.c | 7 -------\n t/t9351-fast-export-anonymize.sh | 12 +++++++-----\n 2 files changed, 7 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex b0b09bca30..c6ecf404d7 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -538,13 +538,6 @@ static const char *anonymize_refname(const char *refname)\n \tstatic struct strbuf anon = STRBUF_INIT;\n \tint i;\n \n-\t/*\n-\t * We also leave \"master\" as a special case, since it does not reveal\n-\t * anything interesting.\n-\t */\n-\tif (!strcmp(refname, \"refs/heads/master\"))\n-\t\treturn refname;\n-\n \tstrbuf_reset(&anon);\n \tfor (i = 0; i < ARRAY_SIZE(prefixes); i++) {\n \t\tif (skip_prefix(refname, prefixes[i], &refname)) {\ndiff --git a/t/t9351-fast-export-anonymize.sh b/t/t9351-fast-export-anonymize.sh\nindex 5a21c71568..5ac2c3b5ee 100755\n--- a/t/t9351-fast-export-anonymize.sh\n+++ b/t/t9351-fast-export-anonymize.sh\n@@ -22,6 +22,7 @@ test_expect_success 'export anonymized stream' '\n \tgit fast-export --anonymize --all \\\n \t\t--anonymize-map=retain-me \\\n \t\t--anonymize-map=xyzzy:custom-name \\\n+\t\t--anonymize-map=other \\\n \t\t>stream\n '\n \n@@ -45,12 +46,12 @@ test_expect_success 'stream omits gitlink oids' '\n \t! grep a000000000000000000 stream\n '\n \n-test_expect_success 'stream allows master as refname' '\n-\tgrep master stream\n+test_expect_success 'stream retains other as refname' '\n+\tgrep other stream\n '\n \n test_expect_success 'stream omits other refnames' '\n-\t! grep other stream &&\n+\t! grep master stream &&\n \t! grep mytag stream\n '\n \n@@ -76,15 +77,16 @@ test_expect_success 'import stream to new repository' '\n test_expect_success 'result has two branches' '\n \tgit for-each-ref --format=\"%(refname)\" refs/heads >branches &&\n \ttest_line_count = 2 branches &&\n-\tother_branch=$(grep -v refs/heads/master branches)\n+\tother_branch=refs/heads/other &&\n+\tmain_branch=$(grep -v $other_branch branches)\n '\n \n test_expect_success 'repo has original shape and timestamps' '\n \tshape () {\n \t\tgit log --format=\"%m %ct\" --left-right --boundary \"$@\"\n \t} &&\n \t(cd .. && shape master...other) >expect &&\n-\tshape master...$other_branch >actual &&\n+\tshape $main_branch...$other_branch >actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.27.0.593.gb3082a2aaf\n\n"},{"id":"400643","messageId":"20200625204532.GK2898@szeder.dev","threadId":"53716","inReplyTo":"20200625154948.GA3968780@coredump.intra.peff.net","subject":"Re: [PATCH 03/10] fast-export: store anonymized oids as hex strings","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-06-25T20:45:32Z","receivedAt":"2020-06-25T20:45:39Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Jun 25, 2020 at 11:49:48AM -0400, Jeff King wrote:\n> On Wed, Jun 24, 2020 at 11:54:20AM -0400, Jeff King wrote:\n> \n> > > GCC 4.8 complains about this change in our CI job:\n> > > \n> > > $ make CC=gcc-4.8 builtin/fast-export.o\n> > > CC builtin/fast-export.o\n> > > builtin/fast-export.c: In function ‘generate_fake_oid’:\n> > > builtin/fast-export.c:394:2: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]\n> > > put_be32(oid.hash + hashsz - 4, counter++);\n> > > ^\n> > > cc1: all warnings being treated as errors\n\n\n> So this really seems like a pointless false positive from the compiler,\n> and it's a rather old compiler (the warning no longer triggers in gcc 6\n> and up). Is it worth caring about? Ubuntu Trusty is out of standard\n> support but not fully EOL'd until 2022. Debian jessie has gcc 4.9\n> which triggers the warning, but will hit EOL in 5 days. If it were an\n> actual breakage I'd be more concerned, but keeping such an old compiler\n> -Werror clean doesn't seem that important.\n> \n> I'd note also that none of the Actions CI jobs run with this compiler\n> version. If we _do_ want to care about it, it might be worth covering it\n> there.\n\nC99 style 'for' loop initial declarations are still frowned upon in\nGit's codebase, and as far as we know it GCC 4.8 is the the most\nrecent compiler that can reasonably detect it; see fb9d7431cf\n(travis-ci: build with GCC 4.8 as well, 2019-07-18).\n\n"},{"id":"400644","messageId":"20200625211522.GA4030664@coredump.intra.peff.net","threadId":"53716","inReplyTo":"20200625204532.GK2898@szeder.dev","subject":"Re: [PATCH 03/10] fast-export: store anonymized oids as hex strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-25T21:15:22Z","receivedAt":"2020-06-25T21:15:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 25, 2020 at 10:45:32PM +0200, SZEDER Gábor wrote:\n\n> > So this really seems like a pointless false positive from the compiler,\n> > and it's a rather old compiler (the warning no longer triggers in gcc 6\n> > and up). Is it worth caring about? Ubuntu Trusty is out of standard\n> > support but not fully EOL'd until 2022. Debian jessie has gcc 4.9\n> > which triggers the warning, but will hit EOL in 5 days. If it were an\n> > actual breakage I'd be more concerned, but keeping such an old compiler\n> > -Werror clean doesn't seem that important.\n> > \n> > I'd note also that none of the Actions CI jobs run with this compiler\n> > version. If we _do_ want to care about it, it might be worth covering it\n> > there.\n> \n> C99 style 'for' loop initial declarations are still frowned upon in\n> Git's codebase, and as far as we know it GCC 4.8 is the the most\n> recent compiler that can reasonably detect it; see fb9d7431cf\n> (travis-ci: build with GCC 4.8 as well, 2019-07-18).\n\nTBH, that does not seem like that compelling a reason to me to keep it\naround. If no compiler is complaining of C99 for-loop declarations, then\nmaybe we should consider loosening our style. Or are we trying to be\nkind of some unknown set of platform-specific compilers that we can't\nfeasibly hit in our CI?\n\nI also note that fb9d7431cf mentions that -std=c90 doesn't work, because\nthere are other spots where we violate the standard (looks like \"inline\"\nis a big one). But gcc with -std=gnu89 seems to compile just fine for\nme, and does notice for-loop declarations. That's obviously totally\nunportable, but it would be sufficient for a CI test.\n\n-Peff\n"},{"id":"400645","messageId":"xmqqa70q4z8i.fsf@gitster.c.googlers.com","threadId":"53716","inReplyTo":"20200625194802.GA4028913@coredump.intra.peff.net","subject":"Re: [PATCH v2 0/11] fast-export: allow seeding the anonymized mapping","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-25T21:22:37Z","receivedAt":"2020-06-25T21:22:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> [09/11]: fast-export: allow seeding the anonymized mapping\n>\n> - the option is now called \"--anonymize-map\", and the word \"seed\"\n> isn't used in any user-facing documentation\n>\n> - incorporated Eric's documentation suggestions\n>\n> - --anonymize-map without --anonymize now triggers an error\n\nThe changes compared to the v1 here looked sensible.\n\n> [10/11]: fast-export: anonymize \"master\" refname\n>\n> - minor adjustments to handle change of option name\n>\n> [11/11]: fast-export: use local array to store anonymized oid\n>\n> - new in this iteration; this address the gcc-4 warning that Gábor\n> mentioned. I prepared it on top since I think we'd eventually want\n> to revert it once we decide that compiler is too old (and I'd be\n> perfectly fine to declare that it's so now, and just never apply\n> it at all).\n\nSounds like a sensible way.\n\nLet's merge it to 'next' soonish.\n\nThanks, all.\n"},{"id":"400808","messageId":"nycvar.QRO.7.76.6.2006291513330.56@tvgsbejvaqbjf.bet","threadId":"53716","inReplyTo":"20200625211522.GA4030664@coredump.intra.peff.net","subject":"Re: [PATCH 03/10] fast-export: store anonymized oids as hex strings","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-06-29T13:17:00Z","receivedAt":"2020-06-30T13:06:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff & Gábor,\n\nOn Thu, 25 Jun 2020, Jeff King wrote:\n\n> On Thu, Jun 25, 2020 at 10:45:32PM +0200, SZEDER Gábor wrote:\n>\n> > > So this really seems like a pointless false positive from the compiler,\n> > > and it's a rather old compiler (the warning no longer triggers in gcc 6\n> > > and up). Is it worth caring about? Ubuntu Trusty is out of standard\n> > > support but not fully EOL'd until 2022. Debian jessie has gcc 4.9\n> > > which triggers the warning, but will hit EOL in 5 days. If it were an\n> > > actual breakage I'd be more concerned, but keeping such an old compiler\n> > > -Werror clean doesn't seem that important.\n> > >\n> > > I'd note also that none of the Actions CI jobs run with this compiler\n> > > version. If we _do_ want to care about it, it might be worth covering it\n> > > there.\n> >\n> > C99 style 'for' loop initial declarations are still frowned upon in\n> > Git's codebase, and as far as we know it GCC 4.8 is the the most\n> > recent compiler that can reasonably detect it; see fb9d7431cf\n> > (travis-ci: build with GCC 4.8 as well, 2019-07-18).\n>\n> TBH, that does not seem like that compelling a reason to me to keep it\n> around. If no compiler is complaining of C99 for-loop declarations, then\n> maybe we should consider loosening our style. Or are we trying to be\n> kind of some unknown set of platform-specific compilers that we can't\n> feasibly hit in our CI?\n\nFWIW _iff_ we decide to loosen our style, I would like to propose doing it\nin one place first, and keep it that way for two or three major versions.\n\nTraditionally, people stuck on platforms such as IRIX or HP/UX with\nproprietary C compilers (and remember: I once was one of those people)\noften lack the luxury of upgrading frequently. And if it turns out that we\nwant to revert the style-loosening, it is much easier to do it in one\nplace than in many.\n\nCiao,\nDscho\n"},{"id":"400850","messageId":"20200630193548.GB1889576@coredump.intra.peff.net","threadId":"53716","inReplyTo":"nycvar.QRO.7.76.6.2006291513330.56@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 03/10] fast-export: store anonymized oids as hex strings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-30T19:35:48Z","receivedAt":"2020-06-30T19:35:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 29, 2020 at 03:17:00PM +0200, Johannes Schindelin wrote:\n\n> > TBH, that does not seem like that compelling a reason to me to keep it\n> > around. If no compiler is complaining of C99 for-loop declarations, then\n> > maybe we should consider loosening our style. Or are we trying to be\n> > kind of some unknown set of platform-specific compilers that we can't\n> > feasibly hit in our CI?\n> \n> FWIW _iff_ we decide to loosen our style, I would like to propose doing it\n> in one place first, and keep it that way for two or three major versions.\n> \n> Traditionally, people stuck on platforms such as IRIX or HP/UX with\n> proprietary C compilers (and remember: I once was one of those people)\n> often lack the luxury of upgrading frequently. And if it turns out that we\n> want to revert the style-loosening, it is much easier to do it in one\n> place than in many.\n\nYeah, I definitely agree that would be the way to do it. I admit I don't\neven _really_ care that much about allowing the feature. More that it\nmight not be worth trying to crack down on it so aggressively, and\njust letting it get picked up by review (or if it slips through, letting\nthe poor souls stuck on HP/UX complain).\n\n(And I say \"worth\" because now the use of gcc 4.8 as the checking tool\nhas demonstrably cost a bunch of human time, so it comes with a cost).\n\n-Peff\n\n"}]}