{"thread":{"id":"36895","subject":"[PATCH v4 0/1] receive-pack: optionally deny case clone refs","startedAt":"2014-06-11T22:30:37Z","lastAt":"2014-08-13T19:28:28Z","messageCount":18,"participants":["David Turner","Junio C Hamano","Torsten Bögershausen","Ronnie Sahlberg","Michael Haggerty"],"isPatch":true,"patchVersion":4,"patchTotal":1},"messages":[{"id":"243928","messageId":"1402525838-31975-1-git-send-email-dturner@twitter.com","threadId":"36895","inReplyTo":null,"subject":"[PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-06-11T22:30:37Z","receivedAt":"2014-06-11T22:30:37Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"This issue bit us again recently.\n\nIn talking with some colleagues, I realized that the previous version\nof this patch, in addition to being potentially slow, was incomplete.\nSpecifically, it didn't handle the case of refs/heads/case/one vs\nrefs/heads/CASE/two; these are case clones even though they strcasecmp\ndifferent.\n\nThis new version contains code to prevent this, as well as tests for\nthis case.\n\nAlso it uses a hashmap to make lookups constant-time.\n"},{"id":"243929","messageId":"1402525838-31975-2-git-send-email-dturner@twitter.com","threadId":"36895","inReplyTo":"1402525838-31975-1-git-send-email-dturner@twitter.com","subject":"[PATCH v4 1/1] receive-pack: optionally deny case clone refs","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-06-11T22:30:38Z","receivedAt":"2014-06-11T22:30:38Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"It is possible to have two refs which are the same but for case.\nThis works great on the case-sensitive filesystems, but not so well on\ncase-insensitive filesystems.  It is fairly typical to have\ncase-insensitive clients (Macs, say) with a case-sensitive server\n(GNU/Linux).\n\nShould a user attempt to pull on a Mac when there are case clone refs\nwith differing contents, they'll get an error message containing\nsomething like \"Ref refs/remotes/origin/lower is at\n[sha-of-lowercase-ref] but expected [sha-of-uppercase-ref]....\n(unable to update local ref)\".\n\nWith a case-insensitive git server, if a branch called capital-M\nMaster (that differs from lowercase-m-master) is pushed, nobody else\ncan push to (lowercase-m) master until the branch is removed.\n\nCreate the option receive.denycaseclonerefs, which checks pushed\nrefs to ensure that they are not case clones of an existing\nref.  This setting is turned on by default if core.ignorecase is\nset, but not otherwise.\n\nSigned-off-by: David Turner <dturner@twitter.com>\n---\n Documentation/config.txt           |   6 ++\n Documentation/git-push.txt         |   5 +-\n Documentation/glossary-content.txt |   9 +++\n builtin/receive-pack.c             | 147 ++++++++++++++++++++++++++++++++++++-\n refs.c                             |   7 +-\n refs.h                             |   6 ++\n t/t5400-send-pack.sh               |  90 +++++++++++++++++++++++\n 7 files changed, 261 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1932e9b..b24b117 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2053,6 +2053,12 @@ receive.unpackLimit::\n \tespecially on slow filesystems.  If not set, the value of\n \t`transfer.unpackLimit` is used instead.\n \n+receive.denyCaseCloneRefs::\n+\tIf set to true, git-receive-pack will deny a ref update that creates\n+\ta ref which is the same but for case as an existing ref.  This is\n+\tuseful when clients are on a case-insensitive filesystem, which\n+\twill cause errors when given refs which differ only in case.\n+\n receive.denyDeletes::\n \tIf set to true, git-receive-pack will deny a ref update that deletes\n \tthe ref. Use this to prevent such a ref deletion via a push.\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 21cd455..c92c3a6 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -323,8 +323,9 @@ remote rejected::\n \tof the following safety options in effect:\n \t`receive.denyCurrentBranch` (for pushes to the checked out\n \tbranch), `receive.denyNonFastForwards` (for forced\n-\tnon-fast-forward updates), `receive.denyDeletes` or\n-\t`receive.denyDeleteCurrent`.  See linkgit:git-config[1].\n+\tnon-fast-forward updates), `receive.denyDeletes`,\n+\t`receive.denyCaseCloneRefs` or `receive.denyDeleteCurrent`.\n+\tSee linkgit:git-config[1].\n \n remote failure::\n \tThe remote end did not report the successful update of the ref,\ndiff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt\nindex be0858c..d1f24bf 100644\n--- a/Documentation/glossary-content.txt\n+++ b/Documentation/glossary-content.txt\n@@ -31,6 +31,15 @@\n [[def_cache]]cache::\n \tObsolete for: <<def_index,index>>.\n \n+[[def_case_clone]]case clone::\n+\tTwo entities (e.g. filenames or refs) that differ only in case.\n+\tThese can cause problems on case-insensitive filesystems, and\n+\tGit has machinery to prevent these problems in various cases.\n+\tCase clone refs also include the situation where one ref has\n+\ta path component which is a case-clone of a different ref's path\n+\tcomponent in the same directory e.g. refs/heads/case/a is a case\n+\tclone of refs/heads/CASE/b\n+\n [[def_chain]]chain::\n \tA list of objects, where each <<def_object,object>> in the list contains\n \ta reference to its successor (for example, the successor of a\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex c323081..6431758 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -27,6 +27,7 @@ enum deny_action {\n \n static int deny_deletes;\n static int deny_non_fast_forwards;\n+static int deny_case_clone_refs = DENY_UNCONFIGURED;\n static enum deny_action deny_current_branch = DENY_UNCONFIGURED;\n static enum deny_action deny_delete_current = DENY_UNCONFIGURED;\n static int receive_fsck_objects = -1;\n@@ -69,6 +70,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n \tif (status)\n \t\treturn status;\n \n+\tif (strcmp(var, \"receive.denycaseclonerefs\") == 0) {\n+\t\tdeny_case_clone_refs = parse_deny_action(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (strcmp(var, \"receive.denydeletes\") == 0) {\n \t\tdeny_deletes = git_config_bool(var, value);\n \t\treturn 0;\n@@ -468,6 +474,138 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)\n \treturn 0;\n }\n \n+/*\n+ * This contains not just refs, but ref prefixes -- i.e. not just\n+ * refs/heads/foo/bar, but refs, refs/heads, and refs/heads/foo\n+ */\n+struct ref_cache_entry {\n+\tstruct hashmap_entry ent;\n+\tunsigned int count; /* count of refs having this as a component */\n+\tchar ref[FLEX_ARRAY];\n+};\n+\n+static struct hashmap *ref_case_clone_cache;\n+\n+static int ref_cache_entry_cmp(const struct ref_cache_entry *e1,\n+\t\t\t       const struct ref_cache_entry *e2, const char *ref)\n+{\n+\treturn strcasecmp(e1->ref, ref ? ref : e2->ref);\n+}\n+\n+/*\n+ * Insert a ref into the ref cache, as well as all of its ancestor\n+ * directory names -- so if we insert refs/heads/something/other,\n+ * refs, refs/heads, refs/heads/something/other will be included.\n+ */\n+static int ref_cache_insert(const char *refname, struct hashmap *map)\n+{\n+\tint total_len = 0, comp_len;\n+\n+\twhile ((comp_len = check_refname_component(refname + total_len, 0)) >= 0) {\n+\t\tstruct ref_cache_entry *old;\n+\t\tstruct ref_cache_entry *entry = xmalloc(sizeof(*entry) + total_len + comp_len + 1);\n+\t\ttotal_len += comp_len;\n+\t\tmemcpy(entry->ref, refname, total_len);\n+\t\tentry->ref[total_len] = 0;\n+\t\tentry->count = 1;\n+\t\thashmap_entry_init(entry, memihash(entry->ref, total_len));\n+\t\told = hashmap_get(map, entry, entry->ref);\n+\t\tif (old) {\n+\t\t\told->count ++;\n+\t\t\tfree(entry);\n+\t\t} else\n+\t\t\thashmap_add(map, entry);\n+\t\ttotal_len ++;\n+\t}\n+}\n+\n+/*\n+ * Remove a ref from the ref cache, as well as any of its ancestor\n+ * directory names that no longer contain any refs.\n+ */\n+static int ref_cache_delete(const char *refname, struct hashmap *map)\n+{\n+\tint total_len = 0, comp_len;\n+\n+\tstruct ref_cache_entry *entry = xmalloc(sizeof(*entry) + strlen(refname));\n+\n+\twhile ((comp_len = check_refname_component(refname + total_len, 0)) >= 0) {\n+\t\tstruct ref_cache_entry *old;\n+\t\ttotal_len += comp_len;\n+\t\tmemcpy(entry->ref, refname, total_len);\n+\t\tentry->ref[total_len] = 0;\n+\t\thashmap_entry_init(entry, memihash(entry->ref, total_len));\n+\t\told = hashmap_get(map, entry, entry->ref);\n+\t\tif (old) {\n+\t\t\told->count --;\n+\t\t\tif (old->count == 0) {\n+\t\t\t\thashmap_remove(map, old, old->ref);\n+\t\t\t\tfree(old);\n+\t\t\t}\n+\t\t} else {\n+\t\t\twarn(\"Ref cache coherency failure: %s from %s\", entry->ref, refname);\n+\t\t\tbreak;\n+\t\t}\n+\t\ttotal_len ++;\n+\t}\n+\tfree(entry);\n+}\n+\n+\n+static int ref_cache_insert_cb(const char *refname, const unsigned char *sha1,\n+\t\t\t       int flags, void *cb_data)\n+{\n+\tref_cache_insert(refname, cb_data);\n+}\n+\n+static void ensure_ref_case_clone_cache(void)\n+{\n+\tif (ref_case_clone_cache)\n+\t\treturn;\n+\tref_case_clone_cache = xmalloc(sizeof(*ref_case_clone_cache));\n+\thashmap_init(ref_case_clone_cache,\n+\t\t     (hashmap_cmp_fn)ref_cache_entry_cmp, 1000);\n+\n+\tfor_each_ref(ref_cache_insert_cb, (void *)ref_case_clone_cache);\n+}\n+\n+/*\n+ * Search the ref cache for a ref that is a case clone of this\n+ * incoming ref; this includes prefix case clones so that\n+ * refs/heads/case/clone will conflict with refs/heads/CASE/other\n+ */\n+static int ref_is_case_clone(const char *name) {\n+\tstruct ref_cache_entry key;\n+\tstruct ref_cache_entry *existing;\n+\tint total_len = 0, comp_len;\n+\tchar *name_so_far = strdup(name);\n+\n+\twhile ((comp_len = check_refname_component(name + total_len, 0)) >= 0) {\n+\t\ttotal_len += comp_len;\n+\t\tname_so_far[total_len] = 0;\n+\t\thashmap_entry_init(&key, memihash(name_so_far, total_len));\n+\t\texisting = hashmap_get(ref_case_clone_cache, &key, name_so_far);\n+\t\tif (!existing)\n+\t\t\treturn 0;\n+\t\tif (memcmp(existing->ref, name_so_far, total_len))\n+\t\t\treturn 1;\n+\t\tname_so_far[total_len] = '/';\n+\t\ttotal_len ++;\n+\t}\n+\n+\tfree(name_so_far);\n+\treturn 0;\n+}\n+\n+static int ref_is_denied_case_clone(const char *name)\n+{\n+\tif (!deny_case_clone_refs)\n+\t\treturn 0;\n+\tensure_ref_case_clone_cache();\n+\n+\treturn ref_is_case_clone(name);\n+}\n+\n static const char *update(struct command *cmd, struct shallow_info *si)\n {\n \tconst char *name = cmd->ref_name;\n@@ -478,7 +616,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \tstruct ref_lock *lock;\n \n \t/* only refs/... are allowed */\n-\tif (!starts_with(name, \"refs/\") || check_refname_format(name + 5, 0)) {\n+\tif (!starts_with(name, \"refs/\") || check_refname_format(name + 5, 0) ||\n+\t    ref_is_denied_case_clone(name)) {\n \t\trp_error(\"refusing to create funny ref '%s' remotely\", name);\n \t\treturn \"funny refname\";\n \t}\n@@ -573,6 +712,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\t\trp_error(\"failed to delete %s\", name);\n \t\t\treturn \"failed to delete\";\n \t\t}\n+\t\tif (deny_case_clone_refs)\n+\t\t\tref_cache_delete(name, ref_case_clone_cache);\n \t\treturn NULL; /* good */\n \t}\n \telse {\n@@ -589,6 +730,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n \t\tif (write_ref_sha1(lock, new_sha1, \"push\")) {\n \t\t\treturn \"failed to write\"; /* error() already called */\n \t\t}\n+\t\tif (deny_case_clone_refs)\n+\t\t\tref_cache_insert(name, ref_case_clone_cache);\n \t\treturn NULL; /* good */\n \t}\n }\n@@ -1171,6 +1314,8 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t\tdie(\"'%s' does not appear to be a git repository\", dir);\n \n \tgit_config(receive_pack_config, NULL);\n+\tif (deny_case_clone_refs == DENY_UNCONFIGURED)\n+\t\tdeny_case_clone_refs = ignore_case;\n \n \tif (0 <= transfer_unpack_limit)\n \t\tunpack_limit = transfer_unpack_limit;\ndiff --git a/refs.c b/refs.c\nindex 28d5eca..7d534cc 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -29,12 +29,7 @@ static inline int bad_ref_char(int ch)\n \treturn 0;\n }\n \n-/*\n- * Try to read one refname component from the front of refname.  Return\n- * the length of the component found, or -1 if the component is not\n- * legal.\n- */\n-static int check_refname_component(const char *refname, int flags)\n+int check_refname_component(const char *refname, int flags)\n {\n \tconst char *cp;\n \tchar last = '\\0';\ndiff --git a/refs.h b/refs.h\nindex 87a1a79..38f3272 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -200,6 +200,12 @@ extern int for_each_reflog(each_ref_fn, void *);\n  * \".\" or \"..\").\n  */\n extern int check_refname_format(const char *refname, int flags);\n+/*\n+ * Try to read one refname component from the front of refname.  Return\n+ * the length of the component found, or -1 if the component is not\n+ * legal.\n+ */\n+extern int check_refname_component(const char *refname, int flags);\n \n extern const char *prettify_refname(const char *refname);\n extern char *shorten_unambiguous_ref(const char *refname, int strict);\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex 0736bcb..de0a88d 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -129,6 +129,96 @@ test_expect_success 'denyNonFastforwards trumps --force' '\n \ttest \"$victim_orig\" = \"$victim_head\"\n '\n \n+test_expect_success 'denyCaseCloneRefs works' '\n+\t(\n+\t    cd victim &&\n+\t    git config receive.denyCaseCloneRefs true &&\n+\t    git config receive.denyDeletes false\n+\t) &&\n+\tgit send-pack ./victim HEAD:refs/heads/case/clone &&\n+\torig_ver=$(git rev-parse HEAD) &&\n+\ttest_must_fail git send-pack ./victim HEAD^:refs/heads/Case/Clone &&\n+\t# confirm that this had no effect upstream\n+\t(\n+\t    cd victim &&\n+\t    ref=$(git for-each-ref --format=\"%(refname)\" refs/heads/Case/Clone) &&\n+\t    echo \"$ref\" | test_must_fail grep -q Case/Clone &&\n+\t    remote_ver=$(git rev-parse case/clone) &&\n+\t    test \"$orig_ver\" = \"$remote_ver\"\n+\t) &&\n+\tgit send-pack ./victim HEAD^:refs/heads/notacase/clone &&\n+\ttest_must_fail git send-pack ./victim :Case/Clone &&\n+\t# confirm that this had no effect upstream\n+\t(\n+\t    cd victim &&\n+\t    ref=$(git for-each-ref --format=\"%(refname)\" refs/heads/Case/Clone) &&\n+\t    echo \"$ref\" | test_must_fail grep -q Case/Clone &&\n+\t    remote_ver=$(git rev-parse case/clone) &&\n+\t    test \"$orig_ver\" = \"$remote_ver\"\n+\t) &&\n+\tgit send-pack ./victim :case/clone &&\n+\t# confirm that this took effect upstream\n+\t(\n+\t    cd victim &&\n+\t    test_must_fail git rev-parse case/clone\n+\t) &&\n+\t# check that we can recreate a branch after deleting a\n+\t# case-clone of it\n+\tcase_clone_ver=$(git rev-parse HEAD^) &&\n+\tgit send-pack ./victim HEAD^:refs/heads/Case/Clone &&\n+\t(\n+\t    cd victim &&\n+\t    ref=$(git for-each-ref --format=\"%(refname)\" refs/heads/case/clone) &&\n+\t    echo \"$ref\" | test_must_fail grep -q case/clone  &&\n+\t    remote_ver=$(git rev-parse Case/Clone) &&\n+\t    test \"$case_clone_ver\" = \"$remote_ver\"\n+\t) &&\n+\t# check subdirectory\n+\ttest_must_fail git send-pack ./victim HEAD:refs/heads/case/clone/morx &&\n+\t# confirm that this had no effect upstream\n+\t(\n+\t    cd victim &&\n+\t    ref=$(git for-each-ref --format=\"%(refname)\" refs/heads/case/clone/morx) &&\n+\t    echo \"$ref\" | test_must_fail grep -q case/clone/morx &&\n+\t    remote_ver=$(git rev-parse Case/Clone) &&\n+\t    test \"$case_clone_ver\" = \"$remote_ver\"\n+\t) &&\n+\t#check parent directory\n+\ttest_must_fail git send-pack ./victim HEAD:refs/heads/case/other &&\n+\t# confirm that this had no effect upstream\n+\t(\n+\t    cd victim &&\n+\t    ref=$(git for-each-ref --format=\"%(refname)\" refs/heads/case/other) &&\n+\t    echo \"$ref\" | test_must_fail grep -q case/other &&\n+\t    remote_ver=$(git rev-parse Case/Clone) &&\n+\t    test \"$case_clone_ver\" = \"$remote_ver\"\n+\t)\n+'\n+test_expect_success 'denyCaseCloneRefs works in same send-pack case' '\n+\t(\n+\t    cd victim &&\n+\t    git config receive.denyCaseCloneRefs true &&\n+\t    git config receive.denyDeletes false\n+\t) &&\n+\ttest_must_fail git send-pack ./victim HEAD:refs/heads/case2 HEAD~:refs/heads/CASE2 &&\n+\t(\n+\t    cd victim &&\n+\t    ref=$(git for-each-ref --format=\"%(refname)\" refs/heads/CASE2) &&\n+\t    echo \"$ref\" | test_must_fail grep -q CASE2 &&\n+\t    remote_ver=$(git rev-parse case2) &&\n+\t    test \"$orig_ver\" = \"$remote_ver\"\n+\t) &&\n+\tcase_clone_ver=$(git rev-parse HEAD^) &&\n+\tgit send-pack ./victim :refs/heads/case2 HEAD~:refs/heads/CASE2 &&\n+\t(\n+\t    cd victim &&\n+\t    ref=$(git for-each-ref --format=\"%(refname)\" refs/heads/case2) &&\n+\t    echo \"$ref\" | test_must_fail grep -q case2 &&\n+\t    remote_ver=$(git rev-parse CASE2) &&\n+\t    test \"$case_clone_ver\" = \"$remote_ver\"\n+\t)\n+'\n+\n test_expect_success 'push --all excludes remote-tracking hierarchy' '\n \tmkdir parent &&\n \t(\n-- \n2.0.0.rc1.24.g0588c94.dirty\n"},{"id":"244028","messageId":"xmqqa99h6hbh.fsf@gitster.dls.corp.google.com","threadId":"36895","inReplyTo":"1402525838-31975-1-git-send-email-dturner@twitter.com","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-12T19:47:30Z","receivedAt":"2014-06-12T19:47:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> This issue bit us again recently.\n>\n> In talking with some colleagues, I realized that the previous version\n> of this patch, in addition to being potentially slow, was incomplete.\n> Specifically, it didn't handle the case of refs/heads/case/one vs\n> refs/heads/CASE/two; these are case clones even though they strcasecmp\n> different.\n\nGood catch to realize that two refs that share leading paths that\nare the same except for cases are also problematic, but that makes\nthis feature even less about \"case clones\", doesn't it?\n\nAlso it somehow feels that the patch attempts to solve the issue at\na wrong level.  On a platform that cannot represent two refs like\nthese (e.g. trying to create \"FOO\" when \"foo\" already exists, or\ntrying to create \"a/c\" when \"A/b\" already exists---ending up with\n\"A/c\" instead, which is not what the user wanted to create), would\nit be more sensible to fail the ref creation without touching the\nusers of ref API such as receive-pack?  That way, you would also\ncatch other uses of refs that are not supported on your system,\ne.g. \"git branch a/c\" when there already is a branch called \"A/b\",\nno?\n\nCC'ing those who are more active in the ref API area recently than I\nam for their inputs.\n"},{"id":"244043","messageId":"1402615835.5629.34.camel@stross","threadId":"36895","inReplyTo":"xmqqa99h6hbh.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-06-12T23:30:35Z","receivedAt":"2014-06-12T23:30:35Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Thu, 2014-06-12 at 12:47 -0700, Junio C Hamano wrote:\n> David Turner <dturner@twopensource.com> writes:\n> \n> > This issue bit us again recently.\n> >\n> > In talking with some colleagues, I realized that the previous version\n> > of this patch, in addition to being potentially slow, was incomplete.\n> > Specifically, it didn't handle the case of refs/heads/case/one vs\n> > refs/heads/CASE/two; these are case clones even though they strcasecmp\n> > different.\n> \n> Good catch to realize that two refs that share leading paths that\n> are the same except for cases are also problematic, but that makes\n> this feature even less about \"case clones\", doesn't it?\n\nI agree: word \"clone\" is less good now.  Maybe \"case conflicts\"?\n\n> Also it somehow feels that the patch attempts to solve the issue at\n> a wrong level.  On a platform that cannot represent two refs like\n> these (e.g. trying to create \"FOO\" when \"foo\" already exists, or\n> trying to create \"a/c\" when \"A/b\" already exists---ending up with\n> \"A/c\" instead, which is not what the user wanted to create), would\n> it be more sensible to fail the ref creation without touching the\n> users of ref API such as receive-pack?  That way, you would also\n> catch other uses of refs that are not supported on your system,\n> e.g. \"git branch a/c\" when there already is a branch called \"A/b\",\n> no?\n\nSo we would change is_refname_available?  And to do this, we would\nchange the ref_dir functions to take case into account?\n\nThis might be somewhat complicated because we could be starting from a\nrepo which already has case conflicts.  What should we do about that?  I\nthink this is even possible on a case-insensitive filesystem with\npacked-refs, although I have not checked.  The simplest idea is probably\nto pretend that the first conflicting refname component we find is the\none true one, and reject new refs containing versions which are case\nconflicting with this one until the user cleans up their repo.  In other\nwords, if the user has A/b and a/c already, and we find A/b first, then\nwe reject a/d but allow A/d.   This is arbitrary, but workable.  We\ncould warn about this situation when we load up the refs, too.  \n\nDoes this match what you are suggesting? \n\nIf so, I think it is possible, and if I don't hear anything back from\nthe other ref folks, I'll see if I have time to implement it.\n"},{"id":"244050","messageId":"539A7802.9090007@web.de","threadId":"36895","inReplyTo":"1402525838-31975-2-git-send-email-dturner@twitter.com","subject":"Re: [PATCH v4 1/1] receive-pack: optionally deny case clone refs","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-06-13T04:03:14Z","receivedAt":"2014-06-13T04:03:14Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 12.06.14 00:30, David Turner wrote:\n[]\nJust a general question:\n\n> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> index c323081..6431758 100644\n> --- a/builtin/receive-pack.c\n> +++ b/builtin/receive-pack.c\n> @@ -27,6 +27,7 @@ enum deny_action {\n>  \n>  static int deny_deletes;\n>  static int deny_non_fast_forwards;\n> +static int deny_case_clone_refs = DENY_UNCONFIGURED;\n>  static enum deny_action deny_current_branch = DENY_UNCONFIGURED;\n>  static enum deny_action deny_delete_current = DENY_UNCONFIGURED;\n>  static int receive_fsck_objects = -1;\n> @@ -69,6 +70,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n>  \tif (status)\n>  \t\treturn status;\n>  \n> +\tif (strcmp(var, \"receive.denycaseclonerefs\") == 0) {\n> +\t\tdeny_case_clone_refs = parse_deny_action(var, value);\n                                       _action() : Which action ?\nMay be this is a better name:\nparse_deny_case_clone_refs()\n> +\t\treturn 0;\n> +\t}\n> +\n>  \tif (strcmp(var, \"receive.denydeletes\") == 0) {\n>  \t\tdeny_deletes = git_config_bool(var, value);\n>  \t\treturn 0;\n> @@ -468,6 +474,138 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)\n>  \treturn 0;\n>  }\n>  \n> +/*\n> + * This contains not just refs, but ref prefixes -- i.e. not just\n> + * refs/heads/foo/bar, but refs, refs/heads, and refs/heads/foo\n> + */\n> +struct ref_cache_entry {\n> +\tstruct hashmap_entry ent;\n> +\tunsigned int count; /* count of refs having this as a component */\n> +\tchar ref[FLEX_ARRAY];\n> +};\n> +\n> +static struct hashmap *ref_case_clone_cache;\n> +\n> +static int ref_cache_entry_cmp(const struct ref_cache_entry *e1,\n> +\t\t\t       const struct ref_cache_entry *e2, const char *ref)\n> +{\n> +\treturn strcasecmp(e1->ref, ref ? ref : e2->ref);\n> +}\n> +\n> +/*\n> + * Insert a ref into the ref cache, as well as all of its ancestor\n> + * directory names -- so if we insert refs/heads/something/other,\n> + * refs, refs/heads, refs/heads/something/other will be included.\n> + */\n> +static int ref_cache_insert(const char *refname, struct hashmap *map)\n> +{\n> +\tint total_len = 0, comp_len;\n> +\n> +\twhile ((comp_len = check_refname_component(refname + total_len, 0)) >= 0) {\n> +\t\tstruct ref_cache_entry *old;\n> +\t\tstruct ref_cache_entry *entry = xmalloc(sizeof(*entry) + total_len + comp_len + 1);\n> +\t\ttotal_len += comp_len;\nCould the total length can be calculated first and already used in xmalloc() ?\nThat will give 3 lines of code, but the reader is sure we are allocating the right length.\n> +\t\tstruct ref_cache_entry;\n> +\t\ttotal_len += comp_len;\n> +\t\tentry = xmalloc(sizeof(*entry) + total_len  + 1);\n\n\n\n> +\t\tmemcpy(entry->ref, refname, total_len);\n> +\t\tentry->ref[total_len] = 0;\n> +\t\tentry->count = 1;\n> +\t\thashmap_entry_init(entry, memihash(entry->ref, total_len));\n> +\t\told = hashmap_get(map, entry, entry->ref);\n> +\t\tif (old) {\n> +\t\t\told->count ++;\n> +\t\t\tfree(entry);\nI'm not sure if I read it right:\nIf there is an old entry \"old\", we anyway create a new one and delete it immediately ?\n> +\t\t} else\n> +\t\t\thashmap_add(map, entry);\n> +\t\ttotal_len ++;\n> +\t}\n> +}\n> +\n> +/*\n> + * Remove a ref from the ref cache, as well as any of its ancestor\n> + * directory names that no longer contain any refs.\n> + */\n> +static int ref_cache_delete(const char *refname, struct hashmap *map)\n> +{\n> +\tint total_len = 0, comp_len;\n> +\n> +\tstruct ref_cache_entry *entry = xmalloc(sizeof(*entry) + strlen(refname));\n> +\n> +\twhile ((comp_len = check_refname_component(refname + total_len, 0)) >= 0) {\n> +\t\tstruct ref_cache_entry *old;\n> +\t\ttotal_len += comp_len;\n> +\t\tmemcpy(entry->ref, refname, total_len);\n> +\t\tentry->ref[total_len] = 0;\n> +\t\thashmap_entry_init(entry, memihash(entry->ref, total_len));\n> +\t\told = hashmap_get(map, entry, entry->ref);\n> +\t\tif (old) {\n> +\t\t\told->count --;\n> +\t\t\tif (old->count == 0) {\n> +\t\t\t\thashmap_remove(map, old, old->ref);\n> +\t\t\t\tfree(old);\n> +\t\t\t}\n> +\t\t} else {\n> +\t\t\twarn(\"Ref cache coherency failure: %s from %s\", entry->ref, refname);\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\ttotal_len ++;\n> +\t}\n> +\tfree(entry);\n> +}\n> +\n> +\n> +static int ref_cache_insert_cb(const char *refname, const unsigned char *sha1,\n> +\t\t\t       int flags, void *cb_data)\n> +{\n> +\tref_cache_insert(refname, cb_data);\n> +}\n> +\n> +static void ensure_ref_case_clone_cache(void)\n> +{\n> +\tif (ref_case_clone_cache)\n> +\t\treturn;\n> +\tref_case_clone_cache = xmalloc(sizeof(*ref_case_clone_cache));\n> +\thashmap_init(ref_case_clone_cache,\n> +\t\t     (hashmap_cmp_fn)ref_cache_entry_cmp, 1000);\n> +\n> +\tfor_each_ref(ref_cache_insert_cb, (void *)ref_case_clone_cache);\n> +}\n> +\n> +/*\n> + * Search the ref cache for a ref that is a case clone of this\n> + * incoming ref; this includes prefix case clones so that\n> + * refs/heads/case/clone will conflict with refs/heads/CASE/other\n> + */\n> +static int ref_is_case_clone(const char *name) {\n> +\tstruct ref_cache_entry key;\n> +\tstruct ref_cache_entry *existing;\n> +\tint total_len = 0, comp_len;\n> +\tchar *name_so_far = strdup(name);\n> +\n> +\twhile ((comp_len = check_refname_component(name + total_len, 0)) >= 0) {\n> +\t\ttotal_len += comp_len;\n> +\t\tname_so_far[total_len] = 0;\n> +\t\thashmap_entry_init(&key, memihash(name_so_far, total_len));\n> +\t\texisting = hashmap_get(ref_case_clone_cache, &key, name_so_far);\n> +\t\tif (!existing)\n> +\t\t\treturn 0;\n> +\t\tif (memcmp(existing->ref, name_so_far, total_len))\n> +\t\t\treturn 1;\n> +\t\tname_so_far[total_len] = '/';\n> +\t\ttotal_len ++;\n> +\t}\n> +\n> +\tfree(name_so_far);\n> +\treturn 0;\n> +}\n> +\n> +static int ref_is_denied_case_clone(const char *name)\n> +{\n> +\tif (!deny_case_clone_refs)\n> +\t\treturn 0;\n> +\tensure_ref_case_clone_cache();\n> +\n> +\treturn ref_is_case_clone(name);\n> +}\n> +\n>  static const char *update(struct command *cmd, struct shallow_info *si)\n>  {\n>  \tconst char *name = cmd->ref_name;\n> @@ -478,7 +616,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n>  \tstruct ref_lock *lock;\n>  \n>  \t/* only refs/... are allowed */\n> -\tif (!starts_with(name, \"refs/\") || check_refname_format(name + 5, 0)) {\n> +\tif (!starts_with(name, \"refs/\") || check_refname_format(name + 5, 0) ||\n> +\t    ref_is_denied_case_clone(name)) {\n>  \t\trp_error(\"refusing to create funny ref '%s' remotely\", name);\n>  \t\treturn \"funny refname\";\nNot related to this patch: the word \"funny\" may be not so funny.\n\nBut related to the patch:\nIf the update is denied because of ref_is_denied_case_clone() says so,\nthe user wants to know this.  So that she/he is able to understand it better.\n\nHere we may want something like\n\t\trp_error(\"refusing to create ref '%s' remotely because of a case insensitive duplicate\", name);\n>  \n>  \t}\n> @@ -573,6 +712,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n>  \t\t\trp_error(\"failed to delete %s\", name);\n>  \t\t\treturn \"failed to delete\";\n>  \t\t}\n> +\t\tif (deny_case_clone_refs)\n> +\t\t\tref_cache_delete(name, ref_case_clone_cache);\n>  \t\treturn NULL; /* good */\n>  \t}\n>  \telse {\n> @@ -589,6 +730,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)\n>  \t\tif (write_ref_sha1(lock, new_sha1, \"push\")) {\n>  \t\t\treturn \"failed to write\"; /* error() already called */\n>  \t\t}\n> +\t\tif (deny_case_clone_refs)\n> +\t\t\tref_cache_insert(name, ref_case_clone_cache);\n>  \t\treturn NULL; /* good */\n>  \t}\n>  }\n> @@ -1171,6 +1314,8 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n>  \t\tdie(\"'%s' does not appear to be a git repository\", dir);\n>  \n>  \tgit_config(receive_pack_config, NULL);\n> +\tif (deny_case_clone_refs == DENY_UNCONFIGURED)\n> +\t\tdeny_case_clone_refs = ignore_case;\n>  \n>  \tif (0 <= transfer_unpack_limit)\n>  \t\tunpack_limit = transfer_unpack_limit;\n> diff --git a/refs.c b/refs.c\n> index 28d5eca..7d534cc 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -29,12 +29,7 @@ static inline int bad_ref_char(int ch)\n>  \treturn 0;\n>  }\n>  \n> -/*\n> - * Try to read one refname component from the front of refname.  Return\n> - * the length of the component found, or -1 if the component is not\n> - * legal.\n> - */\n> -static int check_refname_component(const char *refname, int flags)\n> +int check_refname_component(const char *refname, int flags)\n>  {\n>  \tconst char *cp;\n>  \tchar last = '\\0';\n> diff --git a/refs.h b/refs.h\n> index 87a1a79..38f3272 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -200,6 +200,12 @@ extern int for_each_reflog(each_ref_fn, void *);\n>   * \".\" or \"..\").\n>   */\n>  extern int check_refname_format(const char *refname, int flags);\n> +/*\n> + * Try to read one refname component from the front of refname.  Return\n> + * the length of the component found, or -1 if the component is not\n> + * legal.\n> + */\n> +extern int check_refname_component(const char *refname, int flags);\n>  \n>  extern const char *prettify_refname(const char *refname);\n>  extern char *shorten_unambiguous_ref(const char *refname, int strict);\n> diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\n> index 0736bcb..de0a88d 100755\n> --- a/t/t5400-send-pack.sh\n> +++ b/t/t5400-send-pack.sh\n> @@ -129,6 +129,96 @@ test_expect_success 'denyNonFastforwards trumps --force' '\n>  \ttest \"$victim_orig\" = \"$victim_head\"\n>  '\n>  \n> +test_expect_success 'denyCaseCloneRefs works' '\n> +\t(\n> +\t    cd victim &&\n> +\t    git config receive.denyCaseCloneRefs true &&\n> +\t    git config receive.denyDeletes false\n> +\t) &&\n> +\tgit send-pack ./victim HEAD:refs/heads/case/clone &&\n> +\torig_ver=$(git rev-parse HEAD) &&\n> +\ttest_must_fail git send-pack ./victim HEAD^:refs/heads/Case/Clone &&\n> +\t# confirm that this had no effect upstream\n> +\t(\n> +\t    cd victim &&\n> +\t    ref=$(git for-each-ref --format=\"%(refname)\" refs/heads/Case/Clone) &&\n> +\t    echo \"$ref\" | test_must_fail grep -q Case/Clone &&\n> +\t    remote_ver=$(git rev-parse case/clone) &&\n> +\t    test \"$orig_ver\" = \"$remote_ver\"\n> +\t) &&\n> +\tgit send-pack ./victim HEAD^:refs/heads/notacase/clone &&\n> +\ttest_must_fail git send-pack ./victim :Case/Clone &&\n> +\t# confirm that this had no effect upstream\n> +\t(\n> +\t    cd victim &&\n> +\t    ref=$(git for-each-ref --format=\"%(refname)\" refs/heads/Case/Clone) &&\n> +\t    echo \"$ref\" | test_must_fail grep -q Case/Clone &&\nI'm not sure if this is the ideal combination:\nCollect information in a shell variable, echo that to stdout and use grep -q\nto find out that something is NOT there-\nespecially as I have in the back of my mind the warning \"grep -q\" is not portable...\nBut grep -q is in POSIX, so it may be that versions of grep being part of busybox\nhave this restriction.\nI don't have a better suggestion either, just a loose idea:\n\n\tcd victim &&\n\tcat \"this_or_that\" >expected &&\t\n\tgit for-each-ref --format=\"%(refname)\" refs/heads/Case/Clone | sort >actual &&\n\ttest_cmp expect actual\n"},{"id":"244051","messageId":"539A7821.906@web.de","threadId":"36895","inReplyTo":"1402615835.5629.34.camel@stross","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2014-06-13T04:03:45Z","receivedAt":"2014-06-13T04:03:45Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2014-06-13 01.30, David Turner wrote:\n> On Thu, 2014-06-12 at 12:47 -0700, Junio C Hamano wrote:\n>> David Turner <dturner@twopensource.com> writes:\n>>\n>>> This issue bit us again recently.\n>>>\n>>> In talking with some colleagues, I realized that the previous version\n>>> of this patch, in addition to being potentially slow, was incomplete.\n>>> Specifically, it didn't handle the case of refs/heads/case/one vs\n>>> refs/heads/CASE/two; these are case clones even though they strcasecmp\n>>> different.\n>>\n>> Good catch to realize that two refs that share leading paths that\n>> are the same except for cases are also problematic, but that makes\n>> this feature even less about \"case clones\", doesn't it?\n> \n> I agree: word \"clone\" is less good now.  Maybe \"case conflicts\"?\n> \n>> Also it somehow feels that the patch attempts to solve the issue at\n>> a wrong level.  On a platform that cannot represent two refs like\n>> these (e.g. trying to create \"FOO\" when \"foo\" already exists, or\n>> trying to create \"a/c\" when \"A/b\" already exists---ending up with\n>> \"A/c\" instead, which is not what the user wanted to create), would\n>> it be more sensible to fail the ref creation without touching the\n>> users of ref API such as receive-pack?  That way, you would also\n>> catch other uses of refs that are not supported on your system,\n>> e.g. \"git branch a/c\" when there already is a branch called \"A/b\",\n>> no?\n> \n> So we would change is_refname_available?  And to do this, we would\n> change the ref_dir functions to take case into account?\n> \n> This might be somewhat complicated because we could be starting from a\n> repo which already has case conflicts.  What should we do about that?  I\n> think this is even possible on a case-insensitive filesystem with\n> packed-refs, although I have not checked. \nIt is:\n\nuser@mac:~/test> git init\nInitialized empty Git repository in /Users/tb/test/.git/\nuser@mac:~/test> git checkout -b Branch\nSwitched to a new branch 'Branch'\n\nuser@mac:~/test> echo a>a && git add a && git commit -m \"Add a\"\n[Branch (root-commit) e27e99c] Add a\n 1 file changed, 1 insertion(+)\n create mode 100644 a\n\nuser@mac:~/test> ls .git/refs/heads/\nBranch\n\nuser@mac:~/test> git pack-refs --all\n\nuser@mac:~/test> ls .git/refs/heads/ \n\nuser@mac:~/test> git checkout -b BRANCH\n\nSwitched to a new branch 'BRANCH'\n\nuser@mac:~/test> ls .git/refs/heads/\nBRANCH\nuser@mac:~/test> git branch\n* BRANCH\n  Branch\nuser@mac:~/test> \n\n\n> The simplest idea is probably\n> to pretend that the first conflicting refname component we find is the\n> one true one, and reject new refs containing versions which are case\n> conflicting with this one until the user cleans up their repo.  In other\n> words, if the user has A/b and a/c already, and we find A/b first, then\n> we reject a/d but allow A/d.   This is arbitrary, but workable.  We\n> could warn about this situation when we load up the refs, too.  \n> \n> Does this match what you are suggesting? \n> \n> If so, I think it is possible, and if I don't hear anything back from\n> the other ref folks, I'll see if I have time to implement it.\n\n\n\nI can see 2 ways \"forward\": Either to try to avoid case insensitve refs at all.\nIf you do that carefully in a project, it may work with the patches you suggest.\n\nOr try to have a functionality to always use packed refs, and have a configuration\nfor it:\n\nThe advantage can be that branch names like \"Branch\" and \"BRANCH\" can live together\nin a project, regardless if you have a case sensitive or insensitve file system.\nThis assumes that e.g. \"core.packedrefs\" is default true whenever core.ignorecase\nis true, which we can easily do in init_db.c\n\n\nAnother advantage with the \"always packed refs\" is that you can have branches\nbugfix and bugfix/next-bug side by side.\nToday when you create a branch bugfix theh you have a file bugfix,\nand can not create a directory called bugfix/ to store the file bugfix/next-bug.\n\nI don't know if this is scaling well for many refs, some reports say it does not.\nBut I can imagine to mmap .git/packed-refs into memory, throw every line\ninto a hash table and do a lazy check if a ref is \"good\" whenever it is needed.\nOr do it in git fsck ?\n\nThe disadvantege with packed refs is that you need to carefully lock one central\nfile, and that may be a bottleneck on a server.\n\nI don't have that many refs in my projects, so I only can test on dummy repos,\nmore or less close to reality.\n"},{"id":"244151","messageId":"xmqq7g4k4tzv.fsf@gitster.dls.corp.google.com","threadId":"36895","inReplyTo":"1402615835.5629.34.camel@stross","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-13T17:08:52Z","receivedAt":"2014-06-13T17:08:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> On Thu, 2014-06-12 at 12:47 -0700, Junio C Hamano wrote:\n>> David Turner <dturner@twopensource.com> writes:\n>> \n>> > This issue bit us again recently.\n>> >\n>> > In talking with some colleagues, I realized that the previous version\n>> > of this patch, in addition to being potentially slow, was incomplete.\n>> > Specifically, it didn't handle the case of refs/heads/case/one vs\n>> > refs/heads/CASE/two; these are case clones even though they strcasecmp\n>> > different.\n>> \n>> Good catch to realize that two refs that share leading paths that\n>> are the same except for cases are also problematic, but that makes\n>> this feature even less about \"case clones\", doesn't it?\n>\n> I agree: word \"clone\" is less good now.  Maybe \"case conflicts\"?\n\nSounds better but I'd like to hear from the ref people first, as\nthey have thought about it longer than I have ;-)\n\n>> Also it somehow feels that the patch attempts to solve the issue at\n>> a wrong level.  On a platform that cannot represent two refs like\n>> these (e.g. trying to create \"FOO\" when \"foo\" already exists, or\n>> trying to create \"a/c\" when \"A/b\" already exists---ending up with\n>> \"A/c\" instead, which is not what the user wanted to create), would\n>> it be more sensible to fail the ref creation without touching the\n>> users of ref API such as receive-pack?  That way, you would also\n>> catch other uses of refs that are not supported on your system,\n>> e.g. \"git branch a/c\" when there already is a branch called \"A/b\",\n>> no?\n>\n> So we would change is_refname_available?  And to do this, we would\n> change the ref_dir functions to take case into account?\n> ...\n> In other\n> words, if the user has A/b and a/c already, and we find A/b first, then\n> we reject a/d but allow A/d.   This is arbitrary, but workable.  We\n> could warn about this situation when we load up the refs, too.  \n>\n> Does this match what you are suggesting? \n\nYes.  But again, I'd like to hear from the ref people first.  They\nmay have better ideas.\n"},{"id":"244152","messageId":"xmqq38f84tt3.fsf@gitster.dls.corp.google.com","threadId":"36895","inReplyTo":"539A7821.906@web.de","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-13T17:12:56Z","receivedAt":"2014-06-13T17:12:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> Or try to have a functionality to always use packed refs, and have a configuration\n> for it:\n>\n> The advantage can be that branch names like \"Branch\" and \"BRANCH\" can live together\n> in a project, regardless if you have a case sensitive or insensitve file system.\n\nThat is unfortunately *FALSE*.  You forgot about how reflogs are\nstored for these branches.\n\n> Another advantage with the \"always packed refs\" is that you can have branches\n> bugfix and bugfix/next-bug side by side.\n\nAgain, you forgot about reflogs.\n\nFor this case I do not think it even is unfortunate.  Having two\n\"bugfix\" and \"bugfix/a\" branches, if you are planning to use the\nhierarchical names to group things together, does not make much\nsense, and if you are not using these hierarchical names to group\nthings together you can always use \"bugfix\" vs \"bugfix-a\" instead.\n"},{"id":"244162","messageId":"CAL=YDW=xn0OG5vu=9fnP0nycKV0F9bDJLrkYiwmL9P9q79LJSw@mail.gmail.com","threadId":"36895","inReplyTo":"xmqqa99h6hbh.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-13T18:20:19Z","receivedAt":"2014-06-13T18:20:19Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"On Thu, Jun 12, 2014 at 12:47 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> David Turner <dturner@twopensource.com> writes:\n>\n>> This issue bit us again recently.\n>>\n>> In talking with some colleagues, I realized that the previous version\n>> of this patch, in addition to being potentially slow, was incomplete.\n>> Specifically, it didn't handle the case of refs/heads/case/one vs\n>> refs/heads/CASE/two; these are case clones even though they strcasecmp\n>> different.\n>\n> Good catch to realize that two refs that share leading paths that\n> are the same except for cases are also problematic, but that makes\n> this feature even less about \"case clones\", doesn't it?\n>\n> Also it somehow feels that the patch attempts to solve the issue at\n> a wrong level.\n>  On a platform that cannot represent two refs like\n> these (e.g. trying to create \"FOO\" when \"foo\" already exists, or\n> trying to create \"a/c\" when \"A/b\" already exists---ending up with\n> \"A/c\" instead, which is not what the user wanted to create), would\n> it be more sensible to fail the ref creation without touching the\n> users of ref API such as receive-pack?  That way, you would also\n> catch other uses of refs that are not supported on your system,\n> e.g. \"git branch a/c\" when there already is a branch called \"A/b\",\n> no?\n\nIt gets even more hairy :\nIf the server has A/a and a/b and you clone it it becomes A/a and A/b\nlocally. Then you push back to the server and you end up with three\nrefs on the server:  A/a A/b and a/b.\nOr what we end up with locally could either be a/a  and a/b or A/a and\nA/b depending on which order the server sends the refs back.\nSince the ordering I think is not formally defined, is it?, this could\nmean that we would end up with something but it would be difficult to\ndeteministically decide what the outcome would be.\nAs the refs handling is pretty complex as is I think we want to avoid\nadding undeterministic residuals after a clone/fetch/* has failed.\n\nThat then IMHO means that we should wait with implementing something\nuntil we have finished the ref API rewrite. If nothing else, having\natomic transactions would mean that\nwhen things fail due to case collissions we would have a deterministic\noutcome : transaction failed so nothing was created locally.\n\n\n\nBut I think this is the wrong level to solve this issue at.\nreceive-pack.c is only one out of many place where I think this would\ncome into effect.\nYou also have reflogs, remotes and all sorts of other things that\nwould have to be fixed up.\n\nIf we want to add this kind check-fails, we should wait until after we\nhave ref transactions because then we will have one single path,\ntransaction_update_sha1(), through which every ref creation/update\nwill have to pass. and thus then we have one single place where such a\ncheck-fail could cover all cases of ref creations.\n\n\n\n\n>\n> CC'ing those who are more active in the ref API area recently than I\n> am for their inputs.\n"},{"id":"244165","messageId":"CAL=YDWnHubbC3eOUjHtbiddG0HiaNUW13=GRMXKfyxB+Yomq_g@mail.gmail.com","threadId":"36895","inReplyTo":"CAL=YDW=xn0OG5vu=9fnP0nycKV0F9bDJLrkYiwmL9P9q79LJSw@mail.gmail.com","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-13T19:05:09Z","receivedAt":"2014-06-13T19:05:09Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"Thinking about it more.\n\nI think we want to wait until the ref transaction API work is\nfinished. The ref transactions API is in progress and it aims to add\ntransactions for ref updates as a first step but then it aims to\ndefine a public API for all public ref functions. As part of that I\nwill also develop support for pluggable ref backends. The first\nbackend will be the current files based structure but I also will add\nan optional backend using a TDB database.\n\nThat means that on mac you could just use \"core.refs_backend = tdb\"\nand you would have full support for case sensitive refs. With that\nbackend \"refs/heads/A/foo\" would no longer be a file but a key to a\nrow in the TDB database. I.e. no need to do anything at all, just wait\nuntil my refs work is finished and yourt problem is gone :-)\n\n\nAlternatively, if you don't want a TDB database and you REALLY want to\nkeep storing the refs as files, just as they are today, that would be\nvery easy to do too once the refs work is finished.\n\nEach refs backend will be defined by a set of methods.\nstruct ref_be refs_files = {\n   .transaction_create_sha1 = files_transaction_create_sha1,\n   ...\n\nYou could then very easily create a new backend, say 'struct refs_be\nrefs_files_case_insensitive' where the methods would just convert any\nrefnames to/from a case insensitive encoding before invoking the\ndefault methods from the files backend.\nPerhaps something as simple as converting any upper case characters\nto/from '%xx' representation when accessing the actual files.\n\n\n\nOn Fri, Jun 13, 2014 at 11:20 AM, Ronnie Sahlberg <sahlberg@google.com> wrote:\n> On Thu, Jun 12, 2014 at 12:47 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> David Turner <dturner@twopensource.com> writes:\n>>\n>>> This issue bit us again recently.\n>>>\n>>> In talking with some colleagues, I realized that the previous version\n>>> of this patch, in addition to being potentially slow, was incomplete.\n>>> Specifically, it didn't handle the case of refs/heads/case/one vs\n>>> refs/heads/CASE/two; these are case clones even though they strcasecmp\n>>> different.\n>>\n>> Good catch to realize that two refs that share leading paths that\n>> are the same except for cases are also problematic, but that makes\n>> this feature even less about \"case clones\", doesn't it?\n>>\n>> Also it somehow feels that the patch attempts to solve the issue at\n>> a wrong level.\n>>  On a platform that cannot represent two refs like\n>> these (e.g. trying to create \"FOO\" when \"foo\" already exists, or\n>> trying to create \"a/c\" when \"A/b\" already exists---ending up with\n>> \"A/c\" instead, which is not what the user wanted to create), would\n>> it be more sensible to fail the ref creation without touching the\n>> users of ref API such as receive-pack?  That way, you would also\n>> catch other uses of refs that are not supported on your system,\n>> e.g. \"git branch a/c\" when there already is a branch called \"A/b\",\n>> no?\n>\n> It gets even more hairy :\n> If the server has A/a and a/b and you clone it it becomes A/a and A/b\n> locally. Then you push back to the server and you end up with three\n> refs on the server:  A/a A/b and a/b.\n> Or what we end up with locally could either be a/a  and a/b or A/a and\n> A/b depending on which order the server sends the refs back.\n> Since the ordering I think is not formally defined, is it?, this could\n> mean that we would end up with something but it would be difficult to\n> deteministically decide what the outcome would be.\n> As the refs handling is pretty complex as is I think we want to avoid\n> adding undeterministic residuals after a clone/fetch/* has failed.\n>\n> That then IMHO means that we should wait with implementing something\n> until we have finished the ref API rewrite. If nothing else, having\n> atomic transactions would mean that\n> when things fail due to case collissions we would have a deterministic\n> outcome : transaction failed so nothing was created locally.\n>\n>\n>\n> But I think this is the wrong level to solve this issue at.\n> receive-pack.c is only one out of many place where I think this would\n> come into effect.\n> You also have reflogs, remotes and all sorts of other things that\n> would have to be fixed up.\n>\n> If we want to add this kind check-fails, we should wait until after we\n> have ref transactions because then we will have one single path,\n> transaction_update_sha1(), through which every ref creation/update\n> will have to pass. and thus then we have one single place where such a\n> check-fail could cover all cases of ref creations.\n>\n>\n>\n>\n>>\n>> CC'ing those who are more active in the ref API area recently than I\n>> am for their inputs.\n"},{"id":"244172","messageId":"xmqqk38k1pmv.fsf@gitster.dls.corp.google.com","threadId":"36895","inReplyTo":"CAL=YDWnHubbC3eOUjHtbiddG0HiaNUW13=GRMXKfyxB+Yomq_g@mail.gmail.com","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-13T21:11:20Z","receivedAt":"2014-06-13T21:11:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ronnie Sahlberg <sahlberg@google.com> writes:\n\n> ... The first\n> backend will be the current files based structure but I also will add\n> an optional backend using a TDB database.\n\nI am assuming that as part of the transactions work, accesses to\nreflogs will also have their own backends?\n\n> You could then very easily create a new backend, say 'struct refs_be\n> refs_files_case_insensitive' where the methods would just convert any\n> refnames to/from a case insensitive encoding before invoking the\n> default methods from the files backend.\n> Perhaps something as simple as converting any upper case characters\n> to/from '%xx' representation when accessing the actual files.\n\nHmm... that would work only when the new implementation of Git is\nthe only one that accesses the repository.  Other implementations\n(e.g. Eclipse via egit, Gerrit via jgit, etc.)  peeking into the\nsame repository wouldn't know what to do with these encoded\nrefnames.\n"},{"id":"244175","messageId":"xmqqfvj81oym.fsf@gitster.dls.corp.google.com","threadId":"36895","inReplyTo":"CAL=YDW=xn0OG5vu=9fnP0nycKV0F9bDJLrkYiwmL9P9q79LJSw@mail.gmail.com","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-13T21:25:53Z","receivedAt":"2014-06-13T21:25:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ronnie Sahlberg <sahlberg@google.com> writes:\n\n> It gets even more hairy :\n> If the server has A/a and a/b and you clone it it becomes A/a and A/b\n> locally. Then you push back to the server and you end up with three\n> refs on the server:  A/a A/b and a/b.\n\nThat is part of the transition in deployment.  David who wants to\nforbid A/a and a/b mixed in his project will surely correct the\nsituation at the server end so \"somebody fetches A/a and a/b and\nends up with A/a and A/b\" will not happen.  They will fetch A/a and\nA/b.\n\nIf a user is with an older Git and he has his own a/c, fetching A/a\nand A/b from a corrected central repository will still give the user\na/a and a/b, but then pushing it back will be forbidden.  The user's\nrepository needs to be fixed and installation of Git needs to be\nupdated to the version with an equivalent of David's \"optionally\ndeny\" feature implemented for the fetching side, so that the user\nnotices the local a/c is bad and not allowed within the context of\nhis project, deletes it and recreates it as A/c before he can fetch\nA/a and A/b from the central repository.\n\nI agree that the transition may be painful, but as long as the\ndesired semantics is \"If you have A/a, you are not allowed to have\na/a or a/b\", it cannot be avoided---in that sense, I view it as a\nlower priority issue.\n\nHaving said that, it may indicate that the desired semantics above\nmay not be the optimal one.  Perhaps the flag might want to be \"on\nthis platform, we cannot do case sensitive refs, so pretend as if\nall refs are lowercase\" instead.  I suspect that such a flag may\nallow smoother transition than what has been proposed.\n\nUnderlying refs \"A/a\" and \"a/b\" can stay as they are in David's\ncentral repository, but ref enumeration with the flag enabled will\nreturn a/a and a/b, and these are the names that will be fetched by\nthe users.  If the user had an existing A/c, then fetching these\nwill still create A/a and A/b locally, but pushing them back will,\nunder that flag enabled, be turned into updates to a/a, a/b, and a/c\non the central server side with updated Git.\n\nI dunno.\n"},{"id":"244181","messageId":"CAL=YDWnGBrgHFamc0ZmmGJSFL3Z_6ZaWn4AWOMhJTPQ5UCKD_A@mail.gmail.com","threadId":"36895","inReplyTo":"xmqqk38k1pmv.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-13T22:24:52Z","receivedAt":"2014-06-13T22:24:52Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"On Fri, Jun 13, 2014 at 2:11 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Ronnie Sahlberg <sahlberg@google.com> writes:\n>\n>> ... The first\n>> backend will be the current files based structure but I also will add\n>> an optional backend using a TDB database.\n>\n> I am assuming that as part of the transactions work, accesses to\n> reflogs will also have their own backends?\n\nYes. They will be stored in the same database so that updates to refs\nand reflogs will become truly atomic.\n\n>\n>> You could then very easily create a new backend, say 'struct refs_be\n>> refs_files_case_insensitive' where the methods would just convert any\n>> refnames to/from a case insensitive encoding before invoking the\n>> default methods from the files backend.\n>> Perhaps something as simple as converting any upper case characters\n>> to/from '%xx' representation when accessing the actual files.\n>\n> Hmm... that would work only when the new implementation of Git is\n> the only one that accesses the repository.  Other implementations\n> (e.g. Eclipse via egit, Gerrit via jgit, etc.)  peeking into the\n> same repository wouldn't know what to do with these encoded\n> refnames.\n"},{"id":"244205","messageId":"1402816240.5629.83.camel@stross","threadId":"36895","inReplyTo":"CAL=YDWnHubbC3eOUjHtbiddG0HiaNUW13=GRMXKfyxB+Yomq_g@mail.gmail.com","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-06-15T07:10:40Z","receivedAt":"2014-06-15T07:10:40Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Fri, 2014-06-13 at 12:05 -0700, Ronnie Sahlberg wrote:\n> Thinking about it more.\n> \n> I think we want to wait until the ref transaction API work is\n> finished. The ref transactions API is in progress and it aims to add\n> transactions for ref updates as a first step but then it aims to\n> define a public API for all public ref functions. As part of that I\n> will also develop support for pluggable ref backends. The first\n> backend will be the current files based structure but I also will add\n> an optional backend using a TDB database.\n\nOK, in that case I'll shelve this patch for now (except that we'll\nprobably use it internally)  If for some reason pluggable ref backends\ndon't work out, we can always revive it.\n\nThanks to all for the reviews.  You can find the latest version at\nhttps://github.com/dturner-tw/git/tree/case-ref .\n"},{"id":"244510","messageId":"53A1791B.5010301@alum.mit.edu","threadId":"36895","inReplyTo":"xmqqfvj81oym.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-06-18T11:33:47Z","receivedAt":"2014-06-18T11:33:47Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 06/13/2014 11:25 PM, Junio C Hamano wrote:\n> Ronnie Sahlberg <sahlberg@google.com> writes:\n> \n>> It gets even more hairy :\n>> If the server has A/a and a/b and you clone it it becomes A/a and A/b\n>> locally. Then you push back to the server and you end up with three\n>> refs on the server:  A/a A/b and a/b.\n> \n> That is part of the transition in deployment.  David who wants to\n> forbid A/a and a/b mixed in his project will surely correct the\n> situation at the server end so \"somebody fetches A/a and a/b and\n> ends up with A/a and A/b\" will not happen.  They will fetch A/a and\n> A/b.\n> \n> If a user is with an older Git and he has his own a/c, fetching A/a\n> and A/b from a corrected central repository will still give the user\n> a/a and a/b, but then pushing it back will be forbidden.  The user's\n> repository needs to be fixed and installation of Git needs to be\n> updated to the version with an equivalent of David's \"optionally\n> deny\" feature implemented for the fetching side, so that the user\n> notices the local a/c is bad and not allowed within the context of\n> his project, deletes it and recreates it as A/c before he can fetch\n> A/a and A/b from the central repository.\n> \n> I agree that the transition may be painful, but as long as the\n> desired semantics is \"If you have A/a, you are not allowed to have\n> a/a or a/b\", it cannot be avoided---in that sense, I view it as a\n> lower priority issue.\n> \n> Having said that, it may indicate that the desired semantics above\n> may not be the optimal one.  Perhaps the flag might want to be \"on\n> this platform, we cannot do case sensitive refs, so pretend as if\n> all refs are lowercase\" instead.  I suspect that such a flag may\n> allow smoother transition than what has been proposed.\n> \n> Underlying refs \"A/a\" and \"a/b\" can stay as they are in David's\n> central repository, but ref enumeration with the flag enabled will\n> return a/a and a/b, and these are the names that will be fetched by\n> the users.  If the user had an existing A/c, then fetching these\n> will still create A/a and A/b locally, but pushing them back will,\n> under that flag enabled, be turned into updates to a/a, a/b, and a/c\n> on the central server side with updated Git.\n\nThe discussion here has made it pretty clear that, given our current\nloose reference and reflog storage schemes, it is not possible to\nimplement case-sensitive references or even case-insensitive but\ncase-preserving references correctly on a non-case-sensitive filesystem.\n We would always have spooky non-local conflicts like A/a vs. a/b.\n\nI think we *could* correctly implement\n\n* case-folded reference names (e.g., all lower-case; I wonder if\n  that would also apply to HEAD etc.?)\n\n* case-folded reference names except for the last component, which\n  could be case-insensitive but case-preserving:\n  refs/heads/MyCrazyBranch.  I suppose that many mixed-OS projects\n  effectively use this policy nowadays and that is why we don't hear\n  more complaints about this Git deficiency.\n\nIf we had an option to use only packed references in a repository, I\nthink we could also implement case-insensitive but case-preserving\nreferences on a non-case-preserving filesystem.  The packed-refs file\nwould be authoritative WRT case, and the case of the reflog directories\nand files would be ignored.  But I would be nervous about promoting this\nstyle, because it will likely cause performance problems for repos that\nhave a lot of refs.\n\nTo support arbitrary refname policies on arbitrary filesystems, we of\ncourse need a different way of storing references and reflogs.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"244515","messageId":"CAL=YDW=vfkxdv1viRdvUmWatMftN3yRq7OgyW-2cC9ftA0_G4g@mail.gmail.com","threadId":"36895","inReplyTo":"53A1791B.5010301@alum.mit.edu","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-06-18T15:03:36Z","receivedAt":"2014-06-18T15:03:36Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"On Wed, Jun 18, 2014 at 4:33 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> On 06/13/2014 11:25 PM, Junio C Hamano wrote:\n>> Ronnie Sahlberg <sahlberg@google.com> writes:\n>>\n>>> It gets even more hairy :\n>>> If the server has A/a and a/b and you clone it it becomes A/a and A/b\n>>> locally. Then you push back to the server and you end up with three\n>>> refs on the server:  A/a A/b and a/b.\n>>\n>> That is part of the transition in deployment.  David who wants to\n>> forbid A/a and a/b mixed in his project will surely correct the\n>> situation at the server end so \"somebody fetches A/a and a/b and\n>> ends up with A/a and A/b\" will not happen.  They will fetch A/a and\n>> A/b.\n>>\n>> If a user is with an older Git and he has his own a/c, fetching A/a\n>> and A/b from a corrected central repository will still give the user\n>> a/a and a/b, but then pushing it back will be forbidden.  The user's\n>> repository needs to be fixed and installation of Git needs to be\n>> updated to the version with an equivalent of David's \"optionally\n>> deny\" feature implemented for the fetching side, so that the user\n>> notices the local a/c is bad and not allowed within the context of\n>> his project, deletes it and recreates it as A/c before he can fetch\n>> A/a and A/b from the central repository.\n>>\n>> I agree that the transition may be painful, but as long as the\n>> desired semantics is \"If you have A/a, you are not allowed to have\n>> a/a or a/b\", it cannot be avoided---in that sense, I view it as a\n>> lower priority issue.\n>>\n>> Having said that, it may indicate that the desired semantics above\n>> may not be the optimal one.  Perhaps the flag might want to be \"on\n>> this platform, we cannot do case sensitive refs, so pretend as if\n>> all refs are lowercase\" instead.  I suspect that such a flag may\n>> allow smoother transition than what has been proposed.\n>>\n>> Underlying refs \"A/a\" and \"a/b\" can stay as they are in David's\n>> central repository, but ref enumeration with the flag enabled will\n>> return a/a and a/b, and these are the names that will be fetched by\n>> the users.  If the user had an existing A/c, then fetching these\n>> will still create A/a and A/b locally, but pushing them back will,\n>> under that flag enabled, be turned into updates to a/a, a/b, and a/c\n>> on the central server side with updated Git.\n>\n> The discussion here has made it pretty clear that, given our current\n> loose reference and reflog storage schemes, it is not possible to\n> implement case-sensitive references or even case-insensitive but\n> case-preserving references correctly on a non-case-sensitive filesystem.\n>  We would always have spooky non-local conflicts like A/a vs. a/b.\n>\n> I think we *could* correctly implement\n>\n> * case-folded reference names (e.g., all lower-case; I wonder if\n>   that would also apply to HEAD etc.?)\n>\n> * case-folded reference names except for the last component, which\n>   could be case-insensitive but case-preserving:\n>   refs/heads/MyCrazyBranch.  I suppose that many mixed-OS projects\n>   effectively use this policy nowadays and that is why we don't hear\n>   more complaints about this Git deficiency.\n>\n> If we had an option to use only packed references in a repository, I\n> think we could also implement case-insensitive but case-preserving\n> references on a non-case-preserving filesystem.  The packed-refs file\n> would be authoritative WRT case, and the case of the reflog directories\n> and files would be ignored.  But I would be nervous about promoting this\n> style, because it will likely cause performance problems for repos that\n> have a lot of refs.\n>\n> To support arbitrary refname policies on arbitrary filesystems, we of\n> course need a different way of storing references and reflogs.\n>\n\nAgree.\nI think the transaction work can help here for both the cases of refs\n(which might be solved by a pacxked-refs only setting) and reflogs.\n\nMy next two (small) series for transactions :\nhttps://github.com/rsahlberg/git/tree/ref-transactions-reflog\nhttps://github.com/rsahlberg/git/tree/ref-transactions-rename\n\nreworks the handling of reflogs and all reflog API callers.\nWith these two series we get to a stage where the reflog API and\nimplementation both shrinks and becomes more well defined.\nWe essentially end up with only these functions for touching reflogs:\n\ncheck if a reflog exists:\nextern int reflog_exists(const char *refname);\n\ncreate/initialize an empty reflog:\nextern int create_reflog(const char *refname);\n\ndelete a reflog:\nextern int delete_reflog(const char *refname);\n\nthese two functions iterate/read over all entries for a particular reflog:\nint for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn,\nvoid *cb_data);\nint for_each_reflog_ent_reverse(const char *refname,\neach_reflog_ent_fn fn, void *cb_data);\n\niterate over all reflogs:\nextern int for_each_reflog(each_ref_fn, void *);\n\nwrite/append an entry to a reflog:\nint transaction_update_reflog(struct ref_transaction *transaction,\n\n\nWhen we get to that point it will become much easier if we want to\nchange how reflogs are stored.\nIf for example we want to default to something else than \" a shadow\ndirectory structure under logs/\".\nSo I would like to see those two series as both refactoring the\nexisting code as well as defining the API for reflogs and in its\nextension make it easier to do changes.\n\nFor my plan with pluggable refs backends I plan to put the reflogs\ninside a TDB database, but if someone wants to, this would also\nprovide the framework for someone to add a\n\"core.store_reflog_in_xyz = True\" which do something to address the\nproblems with case insensitive filesystems.\n\n\n\nFollowing those series, I have another small series that has not been\nout for review yet:\nhttps://github.com/rsahlberg/git/tree/ref-transactions-req-packed-refs\n\nThis particular patch series starts using packed refs as part of a\nmulti ref transaction.\nThe main purpose of this is to make transaction_commit() become more\natomic, it particular for better rollbackability but also in order to\nmake the transaction commit become more atomic for an external\nobserver as well.\nIn doing so, for any transactions that affects 2 or more refs it will\nstart reading and writing to the packed refs file instead of loose\nrefs.\n\nFor example a transaction that will rename a ref, i.e. deleting the\nold ref and creating a new ref with the same sha1, it will do\n1, copy the old ref to the packed refs file and repack the packed refs.\n2, delete the old loose ref\n3, delete the old ref from the packed refs file and add the new ref\nand then commit this change to the packed refs file.\n\nOr if you are deleting multiple refs :\n1, copy all refs to the packed refs file and commit the packed refs\n2, delete the loose refs\n3, remove the refs from the packed refs file and commit it.\n\netc.\n\nThe patch series as it is today only do this for multi ref\ntransactions. That is for performance reasons since repacking a huge\npacked refs file would be heaps slower than just creating/updating a\nsingle loose ref for the common case of \"git commit, and update a\nsingle ref\".\nWe could see this series as also lay the groundwork for an option :\n\"core.packed_refs_only = True\"\nwhich would change transaction_commit() to only operate on packed refs\nalso for the single ref case, and then you would basically have a\n\"refs only exist as packed refs mode\"\nin which case you would have a solution to the case insensitive\nfilesystem as well.\n\nSo we could see this as, once the series is ready for review and/or\nuse, this would provide almost a solution for a \"packed refs only\"\nmode.\n\n\n\nWhat I am trying to say is that I think we will get multiple ways to\nsolve the case insensitive filesystem problem as a sideeffect of\nfinishing the ref transaction work.\nPerhaps not full solutions out of the box but at least we should have\nended up with a framework to make it a lot easier to solve these\nproblems than they are today.\n\n\n\n\nregards\nronnie sahlberg\n\n\n\n\n\n> Michael\n>\n> --\n> Michael Haggerty\n> mhagger@alum.mit.edu\n> http://softwareswirl.blogspot.com/\n"},{"id":"247671","messageId":"CAL=YDW=5ynUFFLVNAG=M9x-DRyEyM2q=GPVxzxHVPYDNGHYm=A@mail.gmail.com","threadId":"36895","inReplyTo":"1402525838-31975-1-git-send-email-dturner@twitter.com","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"Ronnie Sahlberg","fromEmail":"sahlberg@google.com","sentAt":"2014-08-13T16:20:41Z","receivedAt":"2014-08-13T16:20:41Z","isPatch":true,"sender":{"key":"sahlberg@google.com","avatar":"https://avatars.githubusercontent.com/u/7320636?v=4"},"body":"David,\n\nOne possible solution can be to use the external database daemon I am\nworking of for ref transactions.\nSince this makes all refs be stored in a dedicated database instead of\nthe filesystem you no longer are dependent on file system semantics.\n\nWhile not in the official git trees yet I would appreciate any testing\nand comments you may have it you want to test it.\nhttps://github.com/rsahlberg/git/tree/backend-struct-db-2\n\nNot for production use but seeing if it does build and works on your\nplatforms would be great.\n\n\n    refsd-tdb: example refs database daemon\n\n    refsd-tdb.c is a simple reference implementation of a refs daemon.\n    This will eventually be hosted in a separate project but can live in this\n    branch for now.\n\n    Compile with :\n    gcc refsd-tdb.c -o refsd-tdb -l tdb\n\n    Run with:\n    ./refsd-tdb /tmp/refsd.socket /tmp /tmp/refsd.log\n\n    Once the refs daemon is running you can start using it with newly\n    created git repositories by specifying the --db-socket and --db-repo-name\n    arguments to git clone/init-db :\n\n    git clone --db-repo-name=ROCKy --db-socket=/tmp/refsd.socket <some repo> .\n\n    This refs daemon is an example. It should be relatively straightforward\n    to modify it to attach to any other kind of data store.\n\nregards\nronnie sahlberg\n\n\nOn Wed, Jun 11, 2014 at 3:30 PM, David Turner <dturner@twopensource.com> wrote:\n> This issue bit us again recently.\n>\n> In talking with some colleagues, I realized that the previous version\n> of this patch, in addition to being potentially slow, was incomplete.\n> Specifically, it didn't handle the case of refs/heads/case/one vs\n> refs/heads/CASE/two; these are case clones even though they strcasecmp\n> different.\n>\n> This new version contains code to prevent this, as well as tests for\n> this case.\n>\n> Also it uses a hashmap to make lookups constant-time.\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"247681","messageId":"1407958108.9673.14.camel@leckie","threadId":"36895","inReplyTo":"CAL=YDW=5ynUFFLVNAG=M9x-DRyEyM2q=GPVxzxHVPYDNGHYm=A@mail.gmail.com","subject":"Re: [PATCH v4 0/1] receive-pack: optionally deny case clone refs","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2014-08-13T19:28:28Z","receivedAt":"2014-08-13T19:28:28Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Wed, 2014-08-13 at 09:20 -0700, Ronnie Sahlberg wrote:\n> David,\n> \n> One possible solution can be to use the external database daemon I am\n> working of for ref transactions.\n> Since this makes all refs be stored in a dedicated database instead of\n> the filesystem you no longer are dependent on file system semantics.\n> \n> While not in the official git trees yet I would appreciate any testing\n> and comments you may have it you want to test it.\n> https://github.com/rsahlberg/git/tree/backend-struct-db-2\n> \n> Not for production use but seeing if it does build and works on your\n> platforms would be great.\n\nThanks. We ultimately went with a custom hook (which also enforces other\nbusiness rules), but I'm very excited about the new backend and hope it\nwill improve performance generally.\n\nOne thing you could do to increase adoption would be to make a Homebrew\nformula for tdb. Homebrew is the defacto standard Mac packaging system.\nSadly, a large number of engineers use Macs, so this is a large barrier\n-- especially since the tdb build is somewhat baroque.\n\nIt would also be great if it didn't require a fresh clone; for a large\norganization with large repos, requiring every engineer to re-clone is\nsomewhat onerous.  I assume it's straightforward to do a conversion from\nan existing set of refs but that it just hasn't been a priority for you\nyet.\n\nThanks so much for working on this!\n"}]}