{"thread":{"id":"40643","subject":"[PATCH/RFC] receive-pack: allow for hiding refs outside the namespace","startedAt":"2015-10-26T08:09:59Z","lastAt":"2015-11-01T18:18:37Z","messageCount":17,"participants":["Lukas Fleischer","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"272230","messageId":"1445846999-8627-1-git-send-email-lfleischer@lfos.de","threadId":"40643","inReplyTo":null,"subject":"[PATCH/RFC] receive-pack: allow for hiding refs outside the namespace","fromName":"Lukas Fleischer","fromEmail":"lfleischer@lfos.de","sentAt":"2015-10-26T08:09:59Z","receivedAt":"2015-10-26T08:09:59Z","isPatch":true,"sender":{"key":"lfleischer@lfos.de","avatar":"https://avatars.githubusercontent.com/u/5530842?v=4"},"body":"Right now, we always advertise all refs as \".have\", even those outside\nthe current namespace. This leads to problems when trying to push to a\nrepository with a huge number of namespaces from a slow connection.\n\nAdd a configuration option receive.advertiseAllRefs that can be used to\ndetermine whether refs outside the current namespace should be\nadvertised or not.\n\nSigned-off-by: Lukas Fleischer <lfleischer@lfos.de>\n---\nWe are using Git namespaces to store a huge number of (virtual)\nrepositories inside a shared repository. While the blobs in the virtual\nrepositories are fairly similar, they do not share any refs, so\nadvertising any refs outside the current namespace is undesirable. See\nthe discussion on [1] for details.\n\nNote that this patch is just a draft: I didn't do any testing, apart\nfrom checking that it compiles. I would like to hear some opinions\nbefore sending a polished version.\n\nIs our use case considered common enough to justify the inclusion of\nsuch a configuration option in mainline?\n\nAre there suggestions for a better name for the option? Ideally, it\nshould contain the word \"namespace\" but I could not come up with\nsomething sensible that is short enough.\n\n[1] https://lists.archlinux.org/pipermail/aur-general/2015-October/031596.html\n\n Documentation/config.txt |  6 ++++++\n builtin/receive-pack.c   | 31 +++++++++++++++++++++----------\n 2 files changed, 27 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 315f271..aa101a7 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2201,6 +2201,12 @@ receive.advertiseAtomic::\n \tcapability to its clients. If you don't want to this capability\n \tto be advertised, set this variable to false.\n \n+receive.advertiseAllRefs::\n+\tBy default, git-receive-pack will advertise all refs, even those\n+\toutside the current namespace, so that the client can use them to\n+\tminimize data transfer. If you only want to advertise refs from the\n+\tactive namespace to be advertised, set this variable to false.\n+\n receive.autogc::\n \tBy default, git-receive-pack will run \"git-gc --auto\" after\n \treceiving data from git-push and updating refs.  You can stop\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e6b93d0..ea9a820 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -41,6 +41,7 @@ static struct strbuf fsck_msg_types = STRBUF_INIT;\n static int receive_unpack_limit = -1;\n static int transfer_unpack_limit = -1;\n static int advertise_atomic_push = 1;\n+static int advertise_all_refs = 1;\n static int unpack_limit = 100;\n static int report_status;\n static int use_sideband;\n@@ -190,6 +191,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (strcmp(var, \"receive.advertiseallrefs\") == 0) {\n+\t\tadvertise_all_refs = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \treturn git_default_config(var, value, cb);\n }\n \n@@ -222,16 +228,21 @@ static void show_ref(const char *path, const unsigned char *sha1)\n static int show_ref_cb(const char *path, const struct object_id *oid, int flag, void *unused)\n {\n \tpath = strip_namespace(path);\n-\t/*\n-\t * Advertise refs outside our current namespace as \".have\"\n-\t * refs, so that the client can use them to minimize data\n-\t * transfer but will otherwise ignore them. This happens to\n-\t * cover \".have\" that are thrown in by add_one_alternate_ref()\n-\t * to mark histories that are complete in our alternates as\n-\t * well.\n-\t */\n-\tif (!path)\n-\t\tpath = \".have\";\n+\tif (!path) {\n+\t\tif (advertise_all_refs) {\n+\t\t\t/*\n+\t\t\t * Advertise refs outside our current namespace as\n+\t\t\t * \".have\" refs, so that the client can use them to\n+\t\t\t * minimize data transfer but will otherwise ignore\n+\t\t\t * them. This happens to cover \".have\" that are thrown\n+\t\t\t * in by add_one_alternate_ref() to mark histories that\n+\t\t\t * are complete in our alternates as well.\n+\t\t\t */\n+\t\t\tpath = \".have\";\n+\t\t} else {\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n \tshow_ref(path, oid->hash);\n \treturn 0;\n }\n-- \n2.6.2\n"},{"id":"272259","messageId":"xmqqk2q9h05h.fsf@gitster.mtv.corp.google.com","threadId":"40643","inReplyTo":"1445846999-8627-1-git-send-email-lfleischer@lfos.de","subject":"Re: [PATCH/RFC] receive-pack: allow for hiding refs outside the namespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-26T19:58:50Z","receivedAt":"2015-10-26T19:58:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Is there a reason why transfer.hiderefs is not sufficient?\n"},{"id":"272310","messageId":"20151027143207.18755.82151@s-8d3a2f8b.on.site.uni-stuttgart.de","threadId":"40643","inReplyTo":"20151027055911.4877.94179@typhoon.lan","subject":"Re: [PATCH/RFC] receive-pack: allow for hiding refs outside the namespace","fromName":"Lukas Fleischer","fromEmail":"lfleischer@lfos.de","sentAt":"2015-10-27T14:32:07Z","receivedAt":"2015-10-27T14:32:07Z","isPatch":true,"sender":{"key":"lfleischer@lfos.de","avatar":"https://avatars.githubusercontent.com/u/5530842?v=4"},"body":"On Tue, 27 Oct 2015 at 06:59:11, Lukas Fleischer wrote:\n> [...]\n> On second thought, it might be possible to overwrite the value of\n> transfer.hiderefs using the -c command line option. If we combine that\n> with the negative patterns supported by hiderefs, we might get a\n> solution that is clean and that avoids race conditions. I will check\n> whether that works with git-http-backend as well and will report back.\n> \n> Thanks for the pointer!\n\nUsing receive.hideRefs seems to work but there are two minor issues:\n\n1. There does not seem to be a way to pass configuration parameters to\n   git-shell commands. Right now, the only way to work around this seems\n   to write a wrapper script around git-shell that catches\n   git-receive-pack commands and executes something like\n   \n       git -c receive.hideRefs=[...] receive-pack [...]\n   \n   instead of forwarding those commands to git-shell. How about allowing\n   to overwrite configuration parameters via an environment variable?\n   Has that been discussed before?\n\n2. transfer.hideRefs and receive.hideRefs do not seem to work with Git\n   namespaces in general. show_ref_cb() replaces each ref outside the\n   current namespace with \".have\" before passing it to show_ref() which\n   in turn performs the ref_is_hidden() check. This has the nice side\n   effect that receive.hideRefs=.have does exactly what I want, however\n   it also means that hideRefs feature does not allow for excluding only\n   specific tags outside the current namespace. Is that intended? Can we\n   rely on Git always looking for \".have\" in the hideRefs list in this\n   case? Should the documentation be updated?\n\nRegards,\nLukas\n"},{"id":"272330","messageId":"xmqqfv0wcgzx.fsf@gitster.mtv.corp.google.com","threadId":"40643","inReplyTo":"20151027143207.18755.82151@s-8d3a2f8b.on.site.uni-stuttgart.de","subject":"Re: [PATCH/RFC] receive-pack: allow for hiding refs outside the namespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-27T18:18:26Z","receivedAt":"2015-10-27T18:18:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lukas Fleischer <lfleischer@lfos.de> writes:\n\n> 2. transfer.hideRefs and receive.hideRefs do not seem to work with Git\n>    namespaces in general. show_ref_cb() replaces each ref outside the\n>    current namespace with \".have\" before passing it to show_ref() which\n>    in turn performs the ref_is_hidden() check. This has the nice side\n>    effect that receive.hideRefs=.have does exactly what I want, however\n>    it also means that hideRefs feature does not allow for excluding only\n>    specific tags outside the current namespace. Is that intended? Can we\n>    rely on Git always looking for \".have\" in the hideRefs list in this\n>    case?\n\nWhen I asked 'Is transfer.hiderefs insufficient?', I wasn't\nexpecting it to be usable out of box.  It was a suggestion to build\non top of it, instead of adding a parallel support for something\nspecific to namespaces.\n\nFor example, if the problem is that you cannot tell ref_is_hidden()\nwhat namespace the ref is from because it is called after running\nstrip_namespace(), perhaps you can find a way to have the original\n\"namespaced ref\" specified on transfer.hiderefs and match them?\nThen in repository for project A, namespaced refs for project B can\nbe excluded by specifying refs/namespaces/B/* on transfer.hiderefs.\n\nPerhaps along the lines of this?\n\n builtin/receive-pack.c | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex bcb624b..db0a99d 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -221,6 +221,15 @@ static void show_ref(const char *path, const unsigned char *sha1)\n \n static int show_ref_cb(const char *path, const struct object_id *oid, int flag, void *unused)\n {\n+\tconst char *ns = get_git_namespace();\n+\n+\t/*\n+\t * Give the \"hiderefs\" mechanism a chance to inspect and\n+\t * reject the namespaced ref itself.\n+\t */\n+\tif (ns[0] && ref_is_hidden(path))\n+\t\treturn 0;\n+\n \tpath = strip_namespace(path);\n \t/*\n \t * Advertise refs outside our current namespace as \".have\"\n"},{"id":"272397","messageId":"20151028070045.5031.43810@s-8d3a2f8b.on.site.uni-stuttgart.de","threadId":"40643","inReplyTo":"xmqqfv0wcgzx.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/RFC] receive-pack: allow for hiding refs outside the namespace","fromName":"Lukas Fleischer","fromEmail":"lfleischer@lfos.de","sentAt":"2015-10-28T07:00:45Z","receivedAt":"2015-10-28T07:00:45Z","isPatch":true,"sender":{"key":"lfleischer@lfos.de","avatar":"https://avatars.githubusercontent.com/u/5530842?v=4"},"body":"On Tue, 27 Oct 2015 at 19:18:26, Junio C Hamano wrote:\n> [...]\n> When I asked 'Is transfer.hiderefs insufficient?', I wasn't\n> expecting it to be usable out of box.  It was a suggestion to build\n> on top of it, instead of adding a parallel support for something\n> specific to namespaces.\n> \n\nAgreed, and I do have a couple of patches to improve hideRefs. I still\nhave some questions before submitting them, though. See below.\n\n> For example, if the problem is that you cannot tell ref_is_hidden()\n> what namespace the ref is from because it is called after running\n> strip_namespace(), perhaps you can find a way to have the original\n> \"namespaced ref\" specified on transfer.hiderefs and match them?\n> Then in repository for project A, namespaced refs for project B can\n> be excluded by specifying refs/namespaces/B/* on transfer.hiderefs.\n> \n> Perhaps along the lines of this?\n> [...]\n\nMy original question remains: Do we want to continue supporting things\nlike transfer.hideRefs=.have (which currently magically hides all refs\noutside the current namespace)? For 100% backwards compatibility, we\nwould have to. On the other hand, one could consider the current\nbehavior a bug and one could argue that it is weird enough that probably\nnobody (apart from me) relies on it right now. If we decide to keep it\nanyway, I think it should be documented.\n\nAnother patch I have in my patch queue adds support for a whitelist mode\nto hideRefs. There are several ways to implement that:\n\n1. Make transfer.hideRefs='' hide all refs (it currently does not). The\n   user can then whitelist refs explicitly using negative patterns\n   below that rule. This is how my current implementation works. Using\n   the empty string seemed most natural since hideRefs matches prefixes\n   and every string has the empty string as a prefix. If that seems too\n   weird, we could probably special case something like\n   transfer.hideRefs='*' instead.\n\n2. Detect whether hideRefs only contains negative patterns. Switch to\n   whitelist mode (\"hide by default\") in that case.\n\n3. Add another option to switch between \"hide by default\" and \"show by\n   default\".\n\nI personally prefer the first option. Any other opinions?\n"},{"id":"272406","messageId":"20151028134212.GA9657@sigill.intra.peff.net","threadId":"40643","inReplyTo":"20151028070045.5031.43810@s-8d3a2f8b.on.site.uni-stuttgart.de","subject":"Re: [PATCH/RFC] receive-pack: allow for hiding refs outside the namespace","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-10-28T13:42:13Z","receivedAt":"2015-10-28T13:42:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 28, 2015 at 08:00:45AM +0100, Lukas Fleischer wrote:\n\n> My original question remains: Do we want to continue supporting things\n> like transfer.hideRefs=.have (which currently magically hides all refs\n> outside the current namespace)? For 100% backwards compatibility, we\n> would have to. On the other hand, one could consider the current\n> behavior a bug and one could argue that it is weird enough that probably\n> nobody (apart from me) relies on it right now. If we decide to keep it\n> anyway, I think it should be documented.\n\nI don't think that hiding \".have\" refs at that level is especially\nuseful. I do not use namespaces, but I do use alternates extensively,\nand that is the original source of these \".have\" refs. But filtering\nthem at the advertisement layer is very inefficient, as it is expensive\nto get the list in the first place (we spawn ls-remote, which spawns\nupload-pack in the alternate!). So we'd want to prevent that process\nmuch earlier.\n\nI have an unpublished patch to specially disable alternates\nadvertisement entirely (i.e., adding a new boolean config,\nreceive.advertiseAlternates). In my case, it is because the alternates\nrepositories have huge numbers of refs (sometimes ranging into the\ngigabytes) and the performance hit on even loading that packed-refs file\nis too large.\n\nI suppose that behavior _could_ be triggered by \".have\" appearing in the\nhiderefs config, though (i.e., before accessing the alternate, check\nref_is_hidden(\".have\")). That seems a bit too subtle to me, though.\n\n> Another patch I have in my patch queue adds support for a whitelist mode\n> to hideRefs. There are several ways to implement that:\n> \n> 1. Make transfer.hideRefs='' hide all refs (it currently does not). The\n>    user can then whitelist refs explicitly using negative patterns\n>    below that rule. This is how my current implementation works. Using\n>    the empty string seemed most natural since hideRefs matches prefixes\n>    and every string has the empty string as a prefix. If that seems too\n>    weird, we could probably special case something like\n>    transfer.hideRefs='*' instead.\n> \n> 2. Detect whether hideRefs only contains negative patterns. Switch to\n>    whitelist mode (\"hide by default\") in that case.\n> \n> 3. Add another option to switch between \"hide by default\" and \"show by\n>    default\".\n> \n> I personally prefer the first option. Any other opinions?\n\nI am just a bystander and would not use this myself, but I think the 1st\nis the least ugly. I am not sure why ignoring \"refs/\" does not work,\nthough (it does not catch \".have\", of course, but I think that is a\nfeature; there are a finite set of pseudo-refs, so you can ignore those,\ntoo, if you want).\n\n-Peff\n"},{"id":"272414","messageId":"1446046920-15646-1-git-send-email-lfleischer@lfos.de","threadId":"40643","inReplyTo":"1445846999-8627-1-git-send-email-lfleischer@lfos.de","subject":"[PATCH] Allow hideRefs to match refs outside the namespace","fromName":"Lukas Fleischer","fromEmail":"lfleischer@lfos.de","sentAt":"2015-10-28T15:42:00Z","receivedAt":"2015-10-28T15:42:00Z","isPatch":true,"sender":{"key":"lfleischer@lfos.de","avatar":"https://avatars.githubusercontent.com/u/5530842?v=4"},"body":"Right now, refs with a path outside the current namespace are replaced\nby \".have\" before passing them to show_ref() which in turn checks\nwhether the ref matches the hideRefs pattern. Move the check before the\npath substitution in show_ref_cb() such that the hideRefs feature can be\nused to hide specific refs outside the current namespace.\n\nSigned-off-by: Lukas Fleischer <lfleischer@lfos.de>\n---\nThe other show_ref() call sites are in show_one_alternate_sha1() and in\nwrite_head_info(). The call site in show_one_alternate_sha1() is for\nalternates and passes \".have\". The other one is\n\n    show_ref(\"capabilities^{}\", null_sha1);\n\nand is not relevant to the hideRefs feature. Note that this kind of\nbreaks backwards compatibility since the \"magic\" hideRefs patterns\n\".have\" and \"capabilities^{}\" no longer work, as explained in the\ndiscussion.\n\n builtin/receive-pack.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex bcb624b..4a5d0ae 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -195,9 +195,6 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n \n static void show_ref(const char *path, const unsigned char *sha1)\n {\n-\tif (ref_is_hidden(path))\n-\t\treturn;\n-\n \tif (sent_capabilities) {\n \t\tpacket_write(1, \"%s %s\\n\", sha1_to_hex(sha1), path);\n \t} else {\n@@ -221,6 +218,9 @@ static void show_ref(const char *path, const unsigned char *sha1)\n \n static int show_ref_cb(const char *path, const struct object_id *oid, int flag, void *unused)\n {\n+\tif (ref_is_hidden(path))\n+\t\treturn 0;\n+\n \tpath = strip_namespace(path);\n \t/*\n \t * Advertise refs outside our current namespace as \".have\"\n-- \n2.6.2\n"},{"id":"272415","messageId":"xmqq611rm1u0.fsf@gitster.mtv.corp.google.com","threadId":"40643","inReplyTo":"20151028070045.5031.43810@s-8d3a2f8b.on.site.uni-stuttgart.de","subject":"Re: [PATCH/RFC] receive-pack: allow for hiding refs outside the namespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-28T15:48:07Z","receivedAt":"2015-10-28T15:48:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lukas Fleischer <lfleischer@lfos.de> writes:\n\n> Another patch I have in my patch queue adds support for a whitelist mode\n> to hideRefs. There are several ways to implement that:\n>\n> 1. Make transfer.hideRefs='' hide all refs (it currently does not). The\n\nHmph, that even sounds like a bug.  parse_hide_refs_config() does\nnot seem to reject ref[] whose length is zero, and ref_is_hidden()\nwould just check \"starts_with(refname, match)\" with an empty string\nas \"match\", so I would naively have expected that to work already.\n\nAhh, there is \"if refname[len] is at the end or slash boundary\"\ncheck after that.  You're right--you'd need to tweak that one for it\nto work.\n\n>    user can then whitelist refs explicitly using negative patterns\n>    below that rule. This is how my current implementation works.\n\nThat sounds like a good way to go.\n\nThanks.\n"},{"id":"272416","messageId":"xmqq1tcfm09k.fsf@gitster.mtv.corp.google.com","threadId":"40643","inReplyTo":"1446046920-15646-1-git-send-email-lfleischer@lfos.de","subject":"Re: [PATCH] Allow hideRefs to match refs outside the namespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-28T16:21:59Z","receivedAt":"2015-10-28T16:21:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lukas Fleischer <lfleischer@lfos.de> writes:\n\n> Right now, refs with a path outside the current namespace are replaced\n> by \".have\" before passing them to show_ref() which in turn checks\n> whether the ref matches the hideRefs pattern. Move the check before the\n> path substitution in show_ref_cb() such that the hideRefs feature can be\n> used to hide specific refs outside the current namespace.\n>\n> Signed-off-by: Lukas Fleischer <lfleischer@lfos.de>\n> ---\n> The other show_ref() call sites are in show_one_alternate_sha1() and in\n> write_head_info(). The call site in show_one_alternate_sha1() is for\n> alternates and passes \".have\". The other one is\n>\n>     show_ref(\"capabilities^{}\", null_sha1);\n>\n> and is not relevant to the hideRefs feature. Note that this kind of\n> breaks backwards compatibility since the \"magic\" hideRefs patterns\n> \".have\" and \"capabilities^{}\" no longer work, as explained in the\n> discussion.\n\nIf somebody is using namespaces and has \"refs/frotz/\" in the\nhiderefs configuration, we hide refs/frotz/ no matter which\nnamespace is being accessed.  With this change, with the removal the\ncheck from show_ref(), wouldn't such a repository suddenly see a\nbehaviour change?\n\n>  builtin/receive-pack.c | 6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\n> index bcb624b..4a5d0ae 100644\n> --- a/builtin/receive-pack.c\n> +++ b/builtin/receive-pack.c\n> @@ -195,9 +195,6 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n>  \n>  static void show_ref(const char *path, const unsigned char *sha1)\n>  {\n> -\tif (ref_is_hidden(path))\n> -\t\treturn;\n> -\n>  \tif (sent_capabilities) {\n>  \t\tpacket_write(1, \"%s %s\\n\", sha1_to_hex(sha1), path);\n>  \t} else {\n> @@ -221,6 +218,9 @@ static void show_ref(const char *path, const unsigned char *sha1)\n>  \n>  static int show_ref_cb(const char *path, const struct object_id *oid, int flag, void *unused)\n>  {\n> +\tif (ref_is_hidden(path))\n> +\t\treturn 0;\n> +\n>  \tpath = strip_namespace(path);\n>  \t/*\n>  \t * Advertise refs outside our current namespace as \".have\"\n"},{"id":"272583","messageId":"xmqqmvv0jb67.fsf@gitster.mtv.corp.google.com","threadId":"40643","inReplyTo":"20151027143207.18755.82151@s-8d3a2f8b.on.site.uni-stuttgart.de","subject":"Re: [PATCH/RFC] receive-pack: allow for hiding refs outside the namespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-30T21:31:28Z","receivedAt":"2015-10-30T21:31:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lukas Fleischer <lfleischer@lfos.de> writes:\n\n> 1. There does not seem to be a way to pass configuration parameters to\n>    git-shell commands. Right now, the only way to work around this seems\n>    to write a wrapper script around git-shell that catches\n>    git-receive-pack commands and executes something like\n>    \n>        git -c receive.hideRefs=[...] receive-pack [...]\n>    \n>    instead of forwarding those commands to git-shell.\n\nThis part we have never discussed in the thread, I think.  Why do\nyou need to override, instead of having these in the repository's\nconfig files?\n\nIs it because a repository may host multiple pseudo repositories in\nthe form of \"namespaces\" but they must share the same config file,\nand you would want to customize per \"namespace\"?\n\nFor that we may want to enhance the [include] mechanism.  Something\nlike\n\n\t[include \"namespace=foo\"]\n        \tpath = /path/to/foo/specific/config.txt\n\n\t[include \"namespace=bar\"]\n        \tpath = /path/to/bar/specific/config.txt\n\nCc'ing Peff as we have discussed this kind of conditional inclusion\nin the past...\n"},{"id":"272585","messageId":"20151030214618.GA11426@sigill.intra.peff.net","threadId":"40643","inReplyTo":"xmqqmvv0jb67.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/RFC] receive-pack: allow for hiding refs outside the namespace","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-10-30T21:46:19Z","receivedAt":"2015-10-30T21:46:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 30, 2015 at 02:31:28PM -0700, Junio C Hamano wrote:\n\n> Lukas Fleischer <lfleischer@lfos.de> writes:\n> \n> > 1. There does not seem to be a way to pass configuration parameters to\n> >    git-shell commands. Right now, the only way to work around this seems\n> >    to write a wrapper script around git-shell that catches\n> >    git-receive-pack commands and executes something like\n> >    \n> >        git -c receive.hideRefs=[...] receive-pack [...]\n> >    \n> >    instead of forwarding those commands to git-shell.\n> \n> This part we have never discussed in the thread, I think.  Why do\n> you need to override, instead of having these in the repository's\n> config files?\n> \n> Is it because a repository may host multiple pseudo repositories in\n> the form of \"namespaces\" but they must share the same config file,\n> and you would want to customize per \"namespace\"?\n> \n> For that we may want to enhance the [include] mechanism.  Something\n> like\n> \n> \t[include \"namespace=foo\"]\n>         \tpath = /path/to/foo/specific/config.txt\n> \n> \t[include \"namespace=bar\"]\n>         \tpath = /path/to/bar/specific/config.txt\n> \n> Cc'ing Peff as we have discussed this kind of conditional inclusion\n> in the past...\n\nYeah, that sort of conditional matching is exactly what I had intended\nfor the \"subsection\" of include to be. We just haven't come up with a\ngood condition to act as our first use case. :)\n\nI am happy with any syntax that does not paint us into a corner (and\nyour example above looks fine, assuming we could later add other keys on\nthe left-hand of the \"=\").\n\nI am slightly confused, though, where the namespace is set in such a\ngit-shell example. I have no really used ref namespaces myself, but my\nunderstanding is that they have to come from the environment. You can\nsimilarly set config through the environment. I don't think we've ever\npublicized that, but it is how \"git -c\" works. E.g.:\n\n  $ git -c alias.foo='!env' -c another.option=true foo | grep GIT_\n  GIT_CONFIG_PARAMETERS='alias.foo='\\!'env' 'another.option=true'\n\nI think it is very particular that you single-quote each item, though:\n\n  $ GIT_CONFIG_PARAMETERS=foo.bar=true git config foo.bar\n  error: bogus format in GIT_CONFIG_PARAMETERS\n  fatal: unable to parse command-line config\n\n  $ GIT_CONFIG_PARAMETERS=\"'foo.bar=true'\" git config foo.bar\n  true\n\nSo we may want to make it a little more friendly before truly\nrecommending it as an interface, but I don't think there is any\nconceptual problem with doing so.\n\n-Peff\n"},{"id":"272607","messageId":"20151031084917.26006.98611@typhoon.lan","threadId":"40643","inReplyTo":"xmqq1tcfm09k.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Allow hideRefs to match refs outside the namespace","fromName":"Lukas Fleischer","fromEmail":"lfleischer@lfos.de","sentAt":"2015-10-31T08:49:17Z","receivedAt":"2015-10-31T08:49:17Z","isPatch":true,"sender":{"key":"lfleischer@lfos.de","avatar":"https://avatars.githubusercontent.com/u/5530842?v=4"},"body":"I wrote this email on Thursday but it seems like it did not make it\nthrough the mailing list. Resubmitting...\n\nOn Wed, 28 Oct 2015 at 17:21:59, Junio C Hamano wrote:\n> Lukas Fleischer <lfleischer@lfos.de> writes:\n> \n> > Right now, refs with a path outside the current namespace are replaced\n> > by \".have\" before passing them to show_ref() which in turn checks\n> > whether the ref matches the hideRefs pattern. Move the check before the\n> > path substitution in show_ref_cb() such that the hideRefs feature can be\n> > used to hide specific refs outside the current namespace.\n> >\n> > Signed-off-by: Lukas Fleischer <lfleischer@lfos.de>\n> > ---\n> > The other show_ref() call sites are in show_one_alternate_sha1() and in\n> > write_head_info(). The call site in show_one_alternate_sha1() is for\n> > alternates and passes \".have\". The other one is\n> >\n> >     show_ref(\"capabilities^{}\", null_sha1);\n> >\n> > and is not relevant to the hideRefs feature. Note that this kind of\n> > breaks backwards compatibility since the \"magic\" hideRefs patterns\n> > \".have\" and \"capabilities^{}\" no longer work, as explained in the\n> > discussion.\n> \n> If somebody is using namespaces and has \"refs/frotz/\" in the\n> hiderefs configuration, we hide refs/frotz/ no matter which\n> namespace is being accessed.  With this change, with the removal the\n> check from show_ref(), wouldn't such a repository suddenly see a\n> behaviour change?\n> [...]\n\nIt would indeed. However, we cannot stay 100% backwards compatible when\nadding support for matching refs outside the current namespace without\nintroducing new syntax. For example, if Git namespaces are in use (i.e.\nGIT_NAMESPACE is set), \"refs/namespaces/foo/refs/bar\" in hideRefs would\nnot have hidden refs/namespaces/foo/refs/bar before the change but it\ndoes afterwards. You might argue that nobody would have added\n\"refs/namespaces/foo/refs/bar\" to hideRefs in the first place but\nnamespaces can be nested and it might be that the user meant to hide\nrefs/namespaces/bar/refs/namespaces/foo/refs/bar instead. Yes, those are\nweird corner cases. But then again, I think that using hideRefs with\nnamespaces already is a corner case as well. I also think that using the\nsame syntax to match both original and stripped refs is bad design. It\nmakes things complicated and the resulting feature doesn't have the full\nexpressive power of the simpler version only matching original refs.\n\nSo, we can either intentionally break backwards compatibility for some\nrare corner cases, or keep the current behavior and introduce some new\nsyntax for matching the original (unstripped) refs. For the latter, we\ncould either introduce a new option (\"hideUnstrippedRefs\"?) or special\nsyntax inside hideRefs (\"/refs/foo\" instead of \"refs/foo\" for matching\nthe unstripped version only?) What do you think?\n"},{"id":"272608","messageId":"20151031090311.26712.64475@typhoon.lan","threadId":"40643","inReplyTo":"20151030214618.GA11426@sigill.intra.peff.net","subject":"Re: [PATCH/RFC] receive-pack: allow for hiding refs outside the namespace","fromName":"Lukas Fleischer","fromEmail":"lfleischer@lfos.de","sentAt":"2015-10-31T09:03:11Z","receivedAt":"2015-10-31T09:03:11Z","isPatch":true,"sender":{"key":"lfleischer@lfos.de","avatar":"https://avatars.githubusercontent.com/u/5530842?v=4"},"body":"On Fri, 30 Oct 2015 at 22:46:19, Jeff King wrote:\n> On Fri, Oct 30, 2015 at 02:31:28PM -0700, Junio C Hamano wrote:\n> \n> > Lukas Fleischer <lfleischer@lfos.de> writes:\n> > \n> > > 1. There does not seem to be a way to pass configuration parameters to\n> > >    git-shell commands. Right now, the only way to work around this seems\n> > >    to write a wrapper script around git-shell that catches\n> > >    git-receive-pack commands and executes something like\n> > >    \n> > >        git -c receive.hideRefs=[...] receive-pack [...]\n> > >    \n> > >    instead of forwarding those commands to git-shell.\n> > \n> > This part we have never discussed in the thread, I think.  Why do\n> > you need to override, instead of having these in the repository's\n> > config files?\n> > \n> > Is it because a repository may host multiple pseudo repositories in\n> > the form of \"namespaces\" but they must share the same config file,\n> > and you would want to customize per \"namespace\"?\n> > \n\nYes. As I said in the original thread, I want to set receive.hideRefs to\nhide everything outside the current namespace, i.e. something equivalent\nto\n\n    git -c receive.hideRefs='refs/' -c receive.hideRefs=\"!refs/namespaces/$foo\" receive-pack /some/path\n\nif receive.hideRefs would work with absolute (unstripped) namespaces.\n\n> > For that we may want to enhance the [include] mechanism.  Something\n> > like\n> > \n> >       [include \"namespace=foo\"]\n> >               path = /path/to/foo/specific/config.txt\n> > \n> >       [include \"namespace=bar\"]\n> >               path = /path/to/bar/specific/config.txt\n> > \n> > Cc'ing Peff as we have discussed this kind of conditional inclusion\n> > in the past...\n> \n\nThat would work but it would still be very cumbersome. Imagine that\nthere is a single repository with 100000 pseudo repositories inside. You\nreally don't want to create a config file and a indirection in the main\nconfiguration for each of these pseudo repositories, just to build a\nconfiguration equivalent to the single line I described above.\n\n> [...]\n> I am slightly confused, though, where the namespace is set in such a\n> git-shell example. I have no really used ref namespaces myself, but my\n> understanding is that they have to come from the environment. You can\n> similarly set config through the environment. I don't think we've ever\n> publicized that, but it is how \"git -c\" works. E.g.:\n> \n>   $ git -c alias.foo='!env' -c another.option=true foo | grep GIT_\n>   GIT_CONFIG_PARAMETERS='alias.foo='\\!'env' 'another.option=true'\n> [...]\n\nYes, the Git namespace is passed through the environment by setting\nGIT_NAMESPACE and GIT_CONFIG_PARAMETERS is exactly what I was looking\nfor! Thanks!\n"},{"id":"272622","messageId":"xmqqsi4rhrmc.fsf@gitster.mtv.corp.google.com","threadId":"40643","inReplyTo":"20151031084917.26006.98611@typhoon.lan","subject":"Re: [PATCH] Allow hideRefs to match refs outside the namespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-31T17:31:23Z","receivedAt":"2015-10-31T17:31:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lukas Fleischer <lfleischer@lfos.de> writes:\n\n>> If somebody is using namespaces and has \"refs/frotz/\" in the\n>> hiderefs configuration, we hide refs/frotz/ no matter which\n>> namespace is being accessed.  With this change, with the removal the\n>> check from show_ref(), wouldn't such a repository suddenly see a\n>> behaviour change?\n>> [...]\n>\n> It would indeed. However, we cannot stay 100% backwards compatible when\n> adding support for matching refs outside the current namespace without\n> introducing new syntax. For example, if Git namespaces are in use (i.e.\n> GIT_NAMESPACE is set), \"refs/namespaces/foo/refs/bar\" in hideRefs would\n> not have hidden refs/namespaces/foo/refs/bar before the change but it\n> does afterwards. You might argue that nobody would have added\n> \"refs/namespaces/foo/refs/bar\" to hideRefs in the first place...\n\nI won't.  To the current users, when they say they want to exclude\n\"refs/foo\", they mean they do not want to advertise the fact that a\nref \"refs/foo/*\" exists in their repository (either the whole thing\nif that is how it is accessed, or in the namespace being\naccessed). and you can replace \"foo\" with any string, including the\nones that contain \"/namespaces/\", i.e. the user wanted to exclude\nrefs from nested ones.\n\nI suspect what you wrote in the above is being a bit too defeatist,\nthough.  We only need to prevent regressions to user with existing\nand valid configurations.\n\nYou earlier (re)discovered a good approach to introduce a new\nfeature without breaking settings of existing users when we\ndiscussed a \"whitelist\".  Since setting the configuration to an\nempty string did not do anything in the old code, an empty string\nwas an invalid and non-working setting.  By taking advantage of that\nfact, you safely can say \"if you start with an empty that would\nmatch everything, we'll treat all the others differently from the\nway we did before\" if you wanted to.  I think you can follow the\nsame principle here.  For example, I can imagine that the rule for\nthe \"ref-is-hidden\" can be updated to:\n\n * it now takes refname and also the fullname before stripping the\n   namespace;\n\n * hide patterns that is prefixed with '!' means negative, just as\n   before;\n\n * (after possibly '!' is stripped), hide patterns that is prefixed\n   with '^', which was invalid before, means check the fullname with\n   namespace prefix, which is a new rule;\n\n * otherwise, check the refname after stripping the namespace.\n\nSuch an update would allow a new feature \"we now allow you to write\na pattern that determines the match before stripping the namespace\nprefix\" without breaking the existing repositories, no?\n\nAfter looking at the current code, I have to say that the way\nref-is-hidden and show_ref_cb() interact with each other is not very\nwell designed when namespaces are in use.  I suspect that this is\nbecause the \"namespace\" stuff was bolted on to the system without\nthinking things through.  For example, people may want to hide\nrefs/changes/* and with the current code, refs/changes/* from your\nown namespace will be filtered out, but the corresponding hierarchy\nfrom other namespaces will be included after getting turned into\n\".have\".  And that cannot be a useful behaviour.  Tips of\nrefs/changes/* would be closely related to the corresponding\nbranches, which means that it would help reducing the object\ntransfer if they are included, and the fact that the user hides them\nis that the user values it more to reduce the size of the initial\nref advertisement more than the potential reduction of the object\ntransfer cost.  If other pseudo repositories (aka namespaces) are\nprojects totally unrelated to yours, inluding their refs/changes/*\n(or any of their refs, for that matter) would not help the later\nobject transfer cost, and including them in the initial ref\nadvertisement would not achieve anything.  Even if other namespaces\nare projects that are closely related to yours, if you are excluding\nrefs/changes/* from your own, that is a strong sign that you do not\nwant their refs/changes/*, either.\n\nAssuming other namespaces are forks of the same project as yours\n(and otherwise the repository management strategy needs to be\nrethought--using namespace for them is not gaining anything other\nthan making your repack more costly), it is likely that all of them\nshare a lot of refs that point at the same object (think \"tags\").\nDo we end up sending a lot of \".have\" for exactly the same object\nnumber of times?  Even though we cannot dedup show_ref() lines that\ntalk about concrete refs (because they talk about what refs exist at\nwhich value, and the sending side would use them to locally reject\nnon-ff pushes, for example), \".have\" lines that talk about the same\nobject can be safely deduped.  This is not directly related to your\ntopic of \"what should be included in the advertisement\", but a\npotentially good thing to fix, if it indeed turns out that we are\nsending a lot of duplicate \".have\"s.  A fix in that would make\nthings better for everybody (not just namespace users, but those who\nshow the \".have\" lines from the refs in the repository we borrow\nobjects from).\n"},{"id":"272640","messageId":"20151031234039.3799.78352@typhoon.lan","threadId":"40643","inReplyTo":"xmqqsi4rhrmc.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Allow hideRefs to match refs outside the namespace","fromName":"Lukas Fleischer","fromEmail":"lfleischer@lfos.de","sentAt":"2015-10-31T23:40:39Z","receivedAt":"2015-10-31T23:40:39Z","isPatch":true,"sender":{"key":"lfleischer@lfos.de","avatar":"https://avatars.githubusercontent.com/u/5530842?v=4"},"body":"On Sat, 31 Oct 2015 at 18:31:23, Junio C Hamano wrote:\n> [...]\n> You earlier (re)discovered a good approach to introduce a new\n> feature without breaking settings of existing users when we\n> discussed a \"whitelist\".  Since setting the configuration to an\n> empty string did not do anything in the old code, an empty string\n> was an invalid and non-working setting.  By taking advantage of that\n> fact, you safely can say \"if you start with an empty that would\n> match everything, we'll treat all the others differently from the\n> way we did before\" if you wanted to.  I think you can follow the\n> same principle here.  For example, I can imagine that the rule for\n> the \"ref-is-hidden\" can be updated to:\n> \n>  * it now takes refname and also the fullname before stripping the\n>    namespace;\n> \n>  * hide patterns that is prefixed with '!' means negative, just as\n>    before;\n> \n>  * (after possibly '!' is stripped), hide patterns that is prefixed\n>    with '^', which was invalid before, means check the fullname with\n>    namespace prefix, which is a new rule;\n> \n>  * otherwise, check the refname after stripping the namespace.\n> \n> Such an update would allow a new feature \"we now allow you to write\n> a pattern that determines the match before stripping the namespace\n> prefix\" without breaking the existing repositories, no?\n> \n\nYes. If I understood you correctly, this is exactly what I suggested in\nthe last paragraph of my previous email (the only difference being that\nI suggested to use \"/\" as full name indicator instead of \"^\" but that is\njust an implementation detail). I will look into implementing this if\nthat is the way we want to go.\n\n> [...]\n> Assuming other namespaces are forks of the same project as yours\n> (and otherwise the repository management strategy needs to be\n> rethought--using namespace for them is not gaining anything other\n> than making your repack more costly), it is likely that all of them\n> share a lot of refs that point at the same object (think \"tags\").\n> Do we end up sending a lot of \".have\" for exactly the same object\n> number of times?  Even though we cannot dedup show_ref() lines that\n> talk about concrete refs (because they talk about what refs exist at\n> which value, and the sending side would use them to locally reject\n> non-ff pushes, for example), \".have\" lines that talk about the same\n> object can be safely deduped.  This is not directly related to your\n> topic of \"what should be included in the advertisement\", but a\n> potentially good thing to fix, if it indeed turns out that we are\n> sending a lot of duplicate \".have\"s.  A fix in that would make\n> things better for everybody (not just namespace users, but those who\n> show the \".have\" lines from the refs in the repository we borrow\n> objects from).\n\nYes, I think we currently send a lot of duplicate lines. Would be nice\nto have that fixed as well.\n\nNote that we do use Git namespaces to store a lot of different but\nsimilar pseudo repositories (i.e. they do not share any history but the\nobjects have huge similarities). Even though the pseudo repositories\nitself are tiny, having the objects in a shared object storage reduces\nthe size significantly. Other people probably use separate repositories,\ncombined with something like GIT_OBJECT_DIRECTORY and preciousObjects\nfor that. Using Git namespaces, however, allows to run `git gc`/`git\nrepack` without needing to take care of maintaining back references to\nthe pseudo repositories and, more importantly, allows for storing all\nthe refs in a single \"packed-refs\" file which did reduce the size the\nsize by another factor of 10 in our tests. That massive difference in\nsize is probably mostly due to the fact that the actual content of each\nrepository is just some 100 bytes. Not sure if saving that much space\ncan currently be achieved with any other approach.\n"},{"id":"272654","messageId":"20151101112716.3758.7843@typhoon.lan","threadId":"40643","inReplyTo":"20151031234039.3799.78352@typhoon.lan","subject":"Re: [PATCH] Allow hideRefs to match refs outside the namespace","fromName":"Lukas Fleischer","fromEmail":"lfleischer@lfos.de","sentAt":"2015-11-01T11:27:16Z","receivedAt":"2015-11-01T11:27:16Z","isPatch":true,"sender":{"key":"lfleischer@lfos.de","avatar":"https://avatars.githubusercontent.com/u/5530842?v=4"},"body":"On Sun, 01 Nov 2015 at 00:40:39, Lukas Fleischer wrote:\n> On Sat, 31 Oct 2015 at 18:31:23, Junio C Hamano wrote:\n> > [...]\n> > You earlier (re)discovered a good approach to introduce a new\n> > feature without breaking settings of existing users when we\n> > discussed a \"whitelist\".  Since setting the configuration to an\n> > empty string did not do anything in the old code, an empty string\n> > was an invalid and non-working setting.  By taking advantage of that\n> > fact, you safely can say \"if you start with an empty that would\n> > match everything, we'll treat all the others differently from the\n> > way we did before\" if you wanted to.  I think you can follow the\n> > same principle here.  For example, I can imagine that the rule for\n> > the \"ref-is-hidden\" can be updated to:\n> > \n> >  * it now takes refname and also the fullname before stripping the\n> >    namespace;\n> > \n> >  * hide patterns that is prefixed with '!' means negative, just as\n> >    before;\n> > \n> >  * (after possibly '!' is stripped), hide patterns that is prefixed\n> >    with '^', which was invalid before, means check the fullname with\n> >    namespace prefix, which is a new rule;\n> > \n> >  * otherwise, check the refname after stripping the namespace.\n> > \n> > Such an update would allow a new feature \"we now allow you to write\n> > a pattern that determines the match before stripping the namespace\n> > prefix\" without breaking the existing repositories, no?\n> > \n> \n> Yes. If I understood you correctly, this is exactly what I suggested in\n> the last paragraph of my previous email (the only difference being that\n> I suggested to use \"/\" as full name indicator instead of \"^\" but that is\n> just an implementation detail). I will look into implementing this if\n> that is the way we want to go.\n> [...]\n\nThere are two more things I noticed.\n\nFirstly, while looking for other callers of ref_is_hidden(), I realized\nthat send_ref() in upload-pack.c contains these lines of code:\n\n    const char *refname_nons = strip_namespace(refname);                    \n    struct object_id peeled;                                                \n                                                                            \n    if (mark_our_ref(refname, oid))                                         \n            return 0;                                                       \n\nwhere mark_our_ref() performs the ref_is_hidden() check on its first\nparameter. So, in contrast to receive-pack, we already match the\noriginal full reference (and not the stripped one) against the hideRefs\npattern there. In particular, when using transfer.hideRefs, the same\npattern does different things when receiving and uploading.\n\nNow, this cannot be intended behavior and I do not think this is\nsomething we want to retain when improving that feature. My suggestion\nis:\n\n1. Define the (current) semantics of hideRefs pattern. It either needs\n   to be defined to match full references or stripped references. Both\n   definitions are equivalent when Git namespaces are not used.\n   \n   It probably makes sense to define hideRefs patterns to match stripped\n   references. If anybody relied on the upload-pack behavior of patterns\n   matching full references, it may happen that more refs are hidden\n   when that behavior is adjusted to match the new hideRefs semantics.\n   The administrator would become aware of that change soon if it\n   affects anything (i.e. hides things that should not be hidden). But I\n   am pretty sure that this behavior isn't currently being relied on\n   either way. Both Git namespaces and hideRefs aren't very popular\n   features and anybody using that combination would probably have\n   noticed the inconsistency and reported a bug earlier.\n\n2. Improve the documentation and describe the hideRefs semantics better.\n   Include details on the choice we made in (1).\n\n3. Fix the send_ref() code in either receive-pack or upload-pack,\n   depending on which is buggy according to our new definition.\n\n4. Improve hideRefs patterns and allow to match both full references and\n   stripped references by using a special indicator as suggested\n   earlier.\n\n5. Add a note on the change in behavior to the release notes of the\n   release that \"breaks backwards compatibility\". Putting it in quotes\n   because I actually think that we are fixing a bug rather than\n   breaking compatibility. But since there was no documentation on the\n   correct behavior, the former implementation was, technically, the\n   only specification of \"correct\" behavior that existed at that\n   point...\n\nThe second thing I noticed is that having syntax for allowing matches\nagainst both full references and stripped references is extremely handy\nand desirable, even if we would not have to introduce it for backwards\ncompatibility. For example, using the syntax Junio described earlier, my\ninitial use case could be solved by\n\n    receive.hideRefs=^refs/\n    receive.hideRefs=!refs/\n\nwhich means \"Hide all references but do not hide references from the\ncurrent namespace.\" Here, I am assuming that patterns for stripped refs\nnever match anything outside the current namespace because those\npatterns become NULL after stripping. This kind of supersedes the other\ndiscussion on setting configuration via environment as well (at least in\nthis context).\n\nRegards,\nLukas\n"},{"id":"272684","messageId":"xmqqy4ehh9c2.fsf@gitster.mtv.corp.google.com","threadId":"40643","inReplyTo":"20151101112716.3758.7843@typhoon.lan","subject":"Re: [PATCH] Allow hideRefs to match refs outside the namespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-11-01T18:18:37Z","receivedAt":"2015-11-01T18:18:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lukas Fleischer <lfleischer@lfos.de> writes:\n\n> Now, this cannot be intended behavior and I do not think this is\n> something we want to retain when improving that feature.\n\nYup, that makes me suspect that namespace support with hiderefs was\ndone without giving much thought even stronger than before, and the\nfact that nobody has brought it up so far suggests it would be much\nsmaller deal than usual if a fix brings in incompatibilities to\nthose who use namespaces.\n\n> 1. Define the (current) semantics of hideRefs pattern. It either needs\n>    to be defined to match full references or stripped references. Both\n>    definitions are equivalent when Git namespaces are not used.\n>    \n>    It probably makes sense to define hideRefs patterns to match stripped\n>    references.\n\nOK.\n\n> 2. Improve the documentation and describe the hideRefs semantics better.\n> 3. Fix the send_ref() code in either receive-pack or upload-pack,\n> 4. Improve hideRefs patterns and allow to match both full references and\n> 5. Add a note on the change in behavior to the release notes of the\n\nAll OK.\n\n> The second thing I noticed is that having syntax for allowing matches\n> against both full references and stripped references is extremely handy\n> and desirable, even if we would not have to introduce it for backwards\n> compatibility. For example, using the syntax Junio described earlier, my\n> initial use case could be solved by\n>\n>     receive.hideRefs=^refs/\n>     receive.hideRefs=!refs/\n>\n> which means \"Hide all references but do not hide references from the\n> current namespace.\" Here, I am assuming that patterns for stripped refs\n> never match anything outside the current namespace because those\n> patterns become NULL after stripping.\n\nI would instead assume that the presence of ^ (or !^) in front would\nsignal \"do not strip before checking\".  !refs/ would mean \"after\nstripping, does it begin with refs/?  If so then do not hide it\".\n\nBut that does not change the conclusion.  With ^refs/ that says\n\"hide everything that matches refs/ before stripping\" (i.e. do not\ninclude anything from anywhere), that is overriden by !refs/ that\nsays \"but do not hide anything that matches refs/ after stripping\"\n(do include everything from my namespace), I'd think that you'd get\nyour desired behaviour.\n\nThanks.\n"}]}