{"thread":{"id":"13795","subject":"[WIP/RFC PATCH 1/2] Introduce GIT_INDEX_PREFIX","startedAt":"2008-06-04T16:28:25Z","lastAt":"2008-06-05T12:03:35Z","messageCount":5,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Nguyen Thai Ngoc Duy","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"78635","messageId":"20080604162825.GB23975@laptop","threadId":"13795","inReplyTo":null,"subject":"[WIP/RFC PATCH 1/2] Introduce GIT_INDEX_PREFIX","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-06-04T16:28:25Z","receivedAt":"2008-06-04T16:28:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"GIT_INDEX_PREFIX is used to limit write access to a specific directory.\nOnly \"important\" information is protected by index prefix (those will\nbe used to create tree objects)\n\nWhen GIT_INDEX_PREFIX is set, any attempt to modify the index (refresh\nit is okay though) will bail out. read-tree and merge, however, can\nwrite to full index. For merge, no conflict is allowed outside index\nprefix.\n---\n builtin-merge-recursive.c |    4 ++-\n builtin-read-tree.c       |    5 +++\n builtin-rev-parse.c       |    5 +++\n builtin-update-index.c    |    3 ++\n cache.h                   |    2 +\n environment.c             |   12 ++++++\n read-cache.c              |   84 +++++++++++++++++++++++++++++++++++++++++++-\n unpack-trees.c            |    3 ++\n unpack-trees.h            |    3 +-\n 9 files changed, 117 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 362c290..94ca808 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -209,8 +209,10 @@ static int git_merge_trees(int index_only,\n \tmemset(&opts, 0, sizeof(opts));\n \tif (index_only)\n \t\topts.index_only = 1;\n-\telse\n+\telse {\n \t\topts.update = 1;\n+\t\topts.check_index_prefix = 1;\n+\t}\n \topts.merge = 1;\n \topts.head_idx = 2;\n \topts.fn = threeway_merge;\ndiff --git a/builtin-read-tree.c b/builtin-read-tree.c\nindex 5a09e17..ea0a929 100644\n--- a/builtin-read-tree.c\n+++ b/builtin-read-tree.c\n@@ -242,6 +242,11 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)\n \t\t\topts.head_idx = 1;\n \t}\n \n+\tif (get_index_prefix()) {\n+\t\tread_cache();\n+\t\topts.check_index_prefix = 1;\n+\t}\n+\n \tfor (i = 0; i < nr_trees; i++) {\n \t\tstruct tree *tree = trees[i];\n \t\tparse_tree(tree);\ndiff --git a/builtin-rev-parse.c b/builtin-rev-parse.c\nindex a7860ed..1888208 100644\n--- a/builtin-rev-parse.c\n+++ b/builtin-rev-parse.c\n@@ -499,6 +499,11 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tputs(prefix);\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--show-index-prefix\")) {\n+\t\t\t\tif (get_index_prefix())\n+\t\t\t\t\tputs(get_index_prefix());\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--show-cdup\")) {\n \t\t\t\tconst char *pfx = prefix;\n \t\t\t\tif (!is_inside_work_tree()) {\ndiff --git a/builtin-update-index.c b/builtin-update-index.c\nindex 9e0d7ab..71c83ec 100644\n--- a/builtin-update-index.c\n+++ b/builtin-update-index.c\n@@ -242,6 +242,9 @@ static void chmod_path(int flip, const char *path)\n \tstruct cache_entry *ce;\n \tunsigned int mode;\n \n+\tif (get_index_prefix() && prefixcmp(path, get_index_prefix()))\n+\t\tdie(\"%s: cannot update index outside %s\", path, get_index_prefix());\n+\n \tpos = cache_name_pos(path, strlen(path));\n \tif (pos < 0)\n \t\tgoto fail;\ndiff --git a/cache.h b/cache.h\nindex eab1a17..9f5a613 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -300,6 +300,7 @@ static inline enum object_type object_type(unsigned int mode)\n #define CONFIG_ENVIRONMENT \"GIT_CONFIG\"\n #define CONFIG_LOCAL_ENVIRONMENT \"GIT_CONFIG_LOCAL\"\n #define EXEC_PATH_ENVIRONMENT \"GIT_EXEC_PATH\"\n+#define INDEX_PREFIX_ENVIRONMENT \"GIT_INDEX_PREFIX\"\n #define GITATTRIBUTES_FILE \".gitattributes\"\n #define INFOATTRIBUTES_FILE \"info/attributes\"\n #define ATTRIBUTE_MACRO_PREFIX \"[attr]\"\n@@ -318,6 +319,7 @@ extern int set_git_dir(const char *path);\n extern const char *get_git_work_tree(void);\n extern const char *read_gitfile_gently(const char *path);\n extern void set_git_work_tree(const char *tree);\n+extern const char *get_index_prefix(void);\n \n #define ALTERNATE_DB_ENVIRONMENT \"GIT_ALTERNATE_OBJECT_DIRECTORIES\"\n \ndiff --git a/environment.c b/environment.c\nindex 73feb2d..a8edaf5 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -46,6 +46,8 @@ enum rebase_setup_type autorebase = AUTOREBASE_NEVER;\n char *git_work_tree_cfg;\n static char *work_tree;\n \n+static const char *index_prefix;\n+\n static const char *git_dir;\n static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;\n \n@@ -71,6 +73,9 @@ static void setup_git_env(void)\n \tgit_graft_file = getenv(GRAFT_ENVIRONMENT);\n \tif (!git_graft_file)\n \t\tgit_graft_file = xstrdup(git_path(\"info/grafts\"));\n+\tindex_prefix = getenv(INDEX_PREFIX_ENVIRONMENT);\n+\tif (index_prefix && (!*index_prefix || index_prefix[strlen(index_prefix)-1] != '/'))\n+\t\tdie(\"GIT_INDEX_PREFIX must end with a slash\");\n }\n \n int is_bare_repository(void)\n@@ -122,6 +127,13 @@ const char *get_git_work_tree(void)\n \treturn work_tree;\n }\n \n+const char *get_index_prefix()\n+{\n+\tif (!git_dir)\n+\t\tsetup_git_env();\n+\treturn index_prefix;\n+}\n+\n char *get_object_directory(void)\n {\n \tif (!git_object_dir)\ndiff --git a/read-cache.c b/read-cache.c\nindex ac9a8e7..4f8d44b 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -23,6 +23,11 @@\n \n struct index_state the_index;\n \n+static int outside_index_prefix(const struct index_state *istate, const char *ce_name)\n+{\n+\treturn istate == &the_index && get_index_prefix() && prefixcmp(ce_name, get_index_prefix());\n+}\n+\n static void set_index_entry(struct index_state *istate, int nr, struct cache_entry *ce)\n {\n \tistate->cache[nr] = ce;\n@@ -380,6 +385,8 @@ int remove_index_entry_at(struct index_state *istate, int pos)\n {\n \tstruct cache_entry *ce = istate->cache[pos];\n \n+\tif (outside_index_prefix(istate, ce->name))\n+\t\tdie(\"%s: cannot remove from index outside %s\", ce->name, get_index_prefix());\n \tremove_name_hash(ce);\n \tistate->cache_changed = 1;\n \tistate->cache_nr--;\n@@ -394,6 +401,10 @@ int remove_index_entry_at(struct index_state *istate, int pos)\n int remove_file_from_index(struct index_state *istate, const char *path)\n {\n \tint pos = index_name_pos(istate, path, strlen(path));\n+\n+\tif (outside_index_prefix(istate, path))\n+\t\tdie(\"%s: cannot remove from index from outside %s\", path, get_index_prefix());\n+\n \tif (pos < 0)\n \t\tpos = -pos-1;\n \tcache_tree_invalidate_path(istate->cache_tree, path);\n@@ -793,21 +804,35 @@ static int check_file_directory_conflict(struct index_state *istate,\n \treturn retval + has_dir_name(istate, ce, pos, ok_to_replace);\n }\n \n+static int ce_compare(const struct cache_entry *ce1, const struct cache_entry *ce2)\n+{\n+\treturn ce1->ce_mode == ce2->ce_mode &&\n+\t\t((ce1->ce_flags ^ ce2->ce_flags) & ~(CE_HASHED | CE_UPDATE)) == 0 &&\n+\t\t!memcmp(ce1->sha1, ce2->sha1, 20) &&\n+\t\t!strcmp(ce1->name, ce2->name);\n+}\n+\n static int add_index_entry_with_check(struct index_state *istate, struct cache_entry *ce, int option)\n {\n \tint pos;\n \tint ok_to_add = option & ADD_CACHE_OK_TO_ADD;\n \tint ok_to_replace = option & ADD_CACHE_OK_TO_REPLACE;\n \tint skip_df_check = option & ADD_CACHE_SKIP_DFCHECK;\n+\tint is_outside_index_prefix = outside_index_prefix(istate, ce->name);\n \n \tcache_tree_invalidate_path(istate->cache_tree, ce->name);\n \tpos = index_name_pos(istate, ce->name, ce->ce_flags);\n \n \t/* existing match? Just replace it. */\n \tif (pos >= 0) {\n+\t\tif (is_outside_index_prefix && !ce_compare(istate->cache[pos], ce))\n+\t\t\tdie(\"%s: cannot add to index outside %s\", ce->name, get_index_prefix());\n \t\treplace_index_entry(istate, pos, ce);\n \t\treturn 0;\n \t}\n+\n+\tif (is_outside_index_prefix)\n+\t\tdie(\"%s: cannot add to index outside %s\", ce->name, get_index_prefix());\n \tpos = -pos-1;\n \n \t/*\n@@ -842,9 +867,11 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti\n {\n \tint pos;\n \n-\tif (option & ADD_CACHE_JUST_APPEND)\n+\tif (option & ADD_CACHE_JUST_APPEND) {\n+\t\tif (outside_index_prefix(istate, ce->name))\n+\t\t\tdie(\"%s: cannot add to index outside %s\", ce->name, get_index_prefix());\n \t\tpos = istate->cache_nr;\n-\telse {\n+\t} else {\n \t\tint ret;\n \t\tret = add_index_entry_with_check(istate, ce, option);\n \t\tif (ret <= 0)\n@@ -1385,3 +1412,56 @@ int write_index(const struct index_state *istate, int newfd)\n \t}\n \treturn ce_flush(&c, newfd);\n }\n+\n+int check_index_prefix(const struct index_state *index, int is_merge)\n+{\n+\tunsigned start,end1,end2,i;\n+\tstruct cache_entry **cache1, **cache2;\n+\tconst char *index_prefix = get_index_prefix();\n+\n+\tif (!index_prefix)\n+\t\treturn 0;\n+\n+\t/* Check for unmerged entries first */\n+\tfor (i = 0; i < index->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = index->cache[i];\n+\t\tif (ce_stage(ce) && prefixcmp(ce->name, index_prefix))\n+\t\t\treturn 1;\n+\t}\n+\n+\tif (is_merge)\n+\t\treturn 0;\n+\n+\t/* not a merge, no change is allowed outside index prefix */\n+\n+\tcache1 = the_index.cache;\n+\tcache2 = index->cache;\n+\tstart = 0;\n+\tend1 = the_index.cache_nr ? the_index.cache_nr - 1 : 0;\n+\tend2 = index->cache_nr ? index->cache_nr - 1 : 0;\n+\twhile (start < end1 && start < end2 &&\n+\t\tce_compare(cache1[start], cache2[start]))\n+\t\tstart ++;\n+\n+\twhile (end1 > start && end2 > start &&\n+\t\tce_compare(cache1[end1], cache2[end2])) {\n+\t\tend1 --;\n+\t\tend2 --;\n+\t}\n+\n+\t/*\n+\t * everything in start..end1 and start..end2 must\n+\t * be prefixed by get_index_prefix()\n+\t */\n+\tif (start < end1 &&\n+\t\t(prefixcmp(cache1[start]->name, index_prefix) ||\n+\t\tprefixcmp(cache1[end1]->name, index_prefix)))\n+\t\treturn 1;\n+\n+\tif (start < end2 &&\n+\t\t(prefixcmp(cache2[start]->name, index_prefix) ||\n+\t\tprefixcmp(cache2[end2]->name, index_prefix)))\n+\t\treturn 1;\n+\n+\treturn 0;\n+}\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 0de5a31..2ef4827 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -403,6 +403,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \tif (o->trivial_merges_only && o->nontrivial_merge)\n \t\treturn unpack_failed(o, \"Merge requires file-level merging\");\n \n+\tif (o->check_index_prefix && check_index_prefix(&o->result, o->merge && !o->prefix))\n+\t\treturn unpack_failed(o, \"Merge outside index prefix\");\n+\n \to->src_index = NULL;\n \tif (check_updates(o))\n \t\treturn -1;\ndiff --git a/unpack-trees.h b/unpack-trees.h\nindex 94e5672..a1b46f9 100644\n--- a/unpack-trees.h\n+++ b/unpack-trees.h\n@@ -26,7 +26,8 @@ struct unpack_trees_options {\n \t\t     verbose_update:1,\n \t\t     aggressive:1,\n \t\t     skip_unmerged:1,\n-\t\t     gently:1;\n+\t\t     gently:1,\n+\t\t     check_index_prefix:1;\n \tconst char *prefix;\n \tint pos;\n \tstruct dir_struct *dir;\n-- \n1.5.5.GIT\n"},{"id":"78642","messageId":"7vabi1xepi.fsf@gitster.siamese.dyndns.org","threadId":"13795","inReplyTo":"20080604162825.GB23975@laptop","subject":"Re: [WIP/RFC PATCH 1/2] Introduce GIT_INDEX_PREFIX","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-04T18:00:41Z","receivedAt":"2008-06-04T18:00:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:\n\n> GIT_INDEX_PREFIX is used to limit write access to a specific directory.\n> Only \"important\" information is protected by index prefix (those will\n> be used to create tree objects)\n>\n> When GIT_INDEX_PREFIX is set, any attempt to modify the index (refresh\n> it is okay though) will bail out. read-tree and merge, however, can\n> write to full index. For merge, no conflict is allowed outside index\n> prefix.\n\nThis is kind of hard to judge as part of \"narrow checkout\" series, because\nit is not clear how this will actually _help_ narrow checkout.\n\nIn other words, as a standalone \"protect parts outside a single\nsubdirectory\" it can be reviewed and judged, but it is unclear how it\nwould help narrow checkout if you excempted only a _single_ subdirectory.\nE.g. you might want to limit yourself to arch/x86 _and_ include/asm-x86.\n\nI sympathize that this series will be hard to get right as the places _you\nidentify_ in the code as \"touching index entries\" are the only places that\nyou _could_ write test scripts for --- if you missed one codepath it would\nbe easy for you to forget writing the test for that codepath.\n\nHaving said that...\n\n> @@ -71,6 +73,9 @@ static void setup_git_env(void)\n>  \tgit_graft_file = getenv(GRAFT_ENVIRONMENT);\n>  \tif (!git_graft_file)\n>  \t\tgit_graft_file = xstrdup(git_path(\"info/grafts\"));\n> +\tindex_prefix = getenv(INDEX_PREFIX_ENVIRONMENT);\n> +\tif (index_prefix && (!*index_prefix || index_prefix[strlen(index_prefix)-1] != '/'))\n> +\t\tdie(\"GIT_INDEX_PREFIX must end with a slash\");\n\nNot nice (aka \"why 'must'?\").\n\n> diff --git a/read-cache.c b/read-cache.c\n> index ac9a8e7..4f8d44b 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -23,6 +23,11 @@\n>  \n>  struct index_state the_index;\n>  \n> +static int outside_index_prefix(const struct index_state *istate, const char *ce_name)\n> +{\n> +\treturn istate == &the_index && get_index_prefix() && prefixcmp(ce_name, get_index_prefix());\n> +}\n\nThe first check above needs to be justified.\n\nIf you say \"outside of this path are off-limits\", why do you allow a\ntemporary index that is used during a partial commit and other\nindex_states excempt from that rule?\n\n> @@ -793,21 +804,35 @@ static int check_file_directory_conflict(struct index_state *istate,\n>  \treturn retval + has_dir_name(istate, ce, pos, ok_to_replace);\n>  }\n>  \n> +static int ce_compare(const struct cache_entry *ce1, const struct cache_entry *ce2)\n> +{\n> +\treturn ce1->ce_mode == ce2->ce_mode &&\n> +\t\t((ce1->ce_flags ^ ce2->ce_flags) & ~(CE_HASHED | CE_UPDATE)) == 0 &&\n> +\t\t!memcmp(ce1->sha1, ce2->sha1, 20) &&\n> +\t\t!strcmp(ce1->name, ce2->name);\n> +}\n\nThis needs a bit of commenting to explain why HASHED and UPDATE are\nspecifically ignored, so that people who later add their own bit to the\nflag can easily tell if their new bit should also be ignored.\n\n> +\tcache1 = the_index.cache;\n> +\tcache2 = index->cache;\n> +\tstart = 0;\n> +\tend1 = the_index.cache_nr ? the_index.cache_nr - 1 : 0;\n> +\tend2 = index->cache_nr ? index->cache_nr - 1 : 0;\n> +\twhile (start < end1 && start < end2 &&\n> +\t\tce_compare(cache1[start], cache2[start]))\n> +\t\tstart ++;\n> +\n> +\twhile (end1 > start && end2 > start &&\n> +\t\tce_compare(cache1[end1], cache2[end2])) {\n> +\t\tend1 --;\n> +\t\tend2 --;\n> +\t}\n\nStyle.  Drop excess SP before post-increment and -decrement.\n\n> +\t/*\n> +\t * everything in start..end1 and start..end2 must\n> +\t * be prefixed by get_index_prefix()\n> +\t */\n> +\tif (start < end1 &&\n> +\t\t(prefixcmp(cache1[start]->name, index_prefix) ||\n> +\t\tprefixcmp(cache1[end1]->name, index_prefix)))\n> +\t\treturn 1;\n\nIt is conventional to return negative or 0 for a function that checks \"is\nthis kosher\".  Returning 0/1 risks getting misunderstood that this returns\nok for 1 and bad for 0.\n"},{"id":"78688","messageId":"fcaeb9bf0806042015w72295f07k25ad709f5d976b20@mail.gmail.com","threadId":"13795","inReplyTo":"7vabi1xepi.fsf@gitster.siamese.dyndns.org","subject":"Re: [WIP/RFC PATCH 1/2] Introduce GIT_INDEX_PREFIX","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-06-05T03:15:26Z","receivedAt":"2008-06-05T03:15:26Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jun 5, 2008 at 1:00 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n>\n>> GIT_INDEX_PREFIX is used to limit write access to a specific directory.\n>> Only \"important\" information is protected by index prefix (those will\n>> be used to create tree objects)\n>>\n>> When GIT_INDEX_PREFIX is set, any attempt to modify the index (refresh\n>> it is okay though) will bail out. read-tree and merge, however, can\n>> write to full index. For merge, no conflict is allowed outside index\n>> prefix.\n>\n> This is kind of hard to judge as part of \"narrow checkout\" series, because\n> it is not clear how this will actually _help_ narrow checkout.\n\nWhen you do narrow checkout. You only have a subdirectory, so you\nwould not want some commands to accidentally change things outside\nthat subdirectory in index.\n\n> In other words, as a standalone \"protect parts outside a single\n> subdirectory\" it can be reviewed and judged, but it is unclear how it\n> would help narrow checkout if you excempted only a _single_ subdirectory.\n> E.g. you might want to limit yourself to arch/x86 _and_ include/asm-x86.\n\nWell it could be extended to support multiple path separated by colon\nlater if someone needs it :)\n\n> I sympathize that this series will be hard to get right as the places _you\n> identify_ in the code as \"touching index entries\" are the only places that\n> you _could_ write test scripts for --- if you missed one codepath it would\n> be easy for you to forget writing the test for that codepath.\n>\n> Having said that...\n>\n>> @@ -71,6 +73,9 @@ static void setup_git_env(void)\n>>       git_graft_file = getenv(GRAFT_ENVIRONMENT);\n>>       if (!git_graft_file)\n>>               git_graft_file = xstrdup(git_path(\"info/grafts\"));\n>> +     index_prefix = getenv(INDEX_PREFIX_ENVIRONMENT);\n>> +     if (index_prefix && (!*index_prefix || index_prefix[strlen(index_prefix)-1] != '/'))\n>> +             die(\"GIT_INDEX_PREFIX must end with a slash\");\n>\n> Not nice (aka \"why 'must'?\").\n\nSimpler handling. Maybe it should append slash by itself if missing.\n\n>> diff --git a/read-cache.c b/read-cache.c\n>> index ac9a8e7..4f8d44b 100644\n>> --- a/read-cache.c\n>> +++ b/read-cache.c\n>> @@ -23,6 +23,11 @@\n>>\n>>  struct index_state the_index;\n>>\n>> +static int outside_index_prefix(const struct index_state *istate, const char *ce_name)\n>> +{\n>> +     return istate == &the_index && get_index_prefix() && prefixcmp(ce_name, get_index_prefix());\n>> +}\n>\n> The first check above needs to be justified.\n>\n> If you say \"outside of this path are off-limits\", why do you allow a\n> temporary index that is used during a partial commit and other\n> index_states excempt from that rule?\n\nBecause unpack_trees writes new index from scratch so it will always\nviolate that. That check is IMO enough for simple index manipulation\nlike add/remove an entry. For unpack_trees, I have\ncheck_index_prefix() to match a temporary index with  current index\nbefore it gets written to disk.\n\nYes I did miss partial commit. Did not know it used temporary index too.\n\n> [the rest]\n\nOK will do.\n\n-- \nDuy\n"},{"id":"78697","messageId":"alpine.DEB.1.00.0806050532240.21190@racer","threadId":"13795","inReplyTo":"fcaeb9bf0806042015w72295f07k25ad709f5d976b20@mail.gmail.com","subject":"Re: [WIP/RFC PATCH 1/2] Introduce GIT_INDEX_PREFIX","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-06-05T04:35:53Z","receivedAt":"2008-06-05T04:35:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 5 Jun 2008, Nguyen Thai Ngoc Duy wrote:\n\n> On Thu, Jun 5, 2008 at 1:00 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> > Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n> >\n> >> GIT_INDEX_PREFIX is used to limit write access to a specific directory.\n> >> Only \"important\" information is protected by index prefix (those will\n> >> be used to create tree objects)\n> >>\n> >> When GIT_INDEX_PREFIX is set, any attempt to modify the index (refresh\n> >> it is okay though) will bail out. read-tree and merge, however, can\n> >> write to full index. For merge, no conflict is allowed outside index\n> >> prefix.\n> >\n> > This is kind of hard to judge as part of \"narrow checkout\" series, \n> > because it is not clear how this will actually _help_ narrow checkout.\n> \n> When you do narrow checkout. You only have a subdirectory, so you would \n> not want some commands to accidentally change things outside that \n> subdirectory in index.\n\nWhen they operate purely on the index?  Why not?\n\nI guess that Junio's point was that the semantics are not at all clear.\n\n> > In other words, as a standalone \"protect parts outside a single \n> > subdirectory\" it can be reviewed and judged, but it is unclear how it \n> > would help narrow checkout if you excempted only a _single_ \n> > subdirectory. E.g. you might want to limit yourself to arch/x86 _and_ \n> > include/asm-x86.\n> \n> Well it could be extended to support multiple path separated by colon\n> later if someone needs it :)\n\nThe thing is: if the concept is sound, chances are that it is _easy_ to \nsupport that, should someone need it.\n\n> >> @@ -71,6 +73,9 @@ static void setup_git_env(void)\n> >>       git_graft_file = getenv(GRAFT_ENVIRONMENT);\n> >>       if (!git_graft_file)\n> >>               git_graft_file = xstrdup(git_path(\"info/grafts\"));\n> >> +     index_prefix = getenv(INDEX_PREFIX_ENVIRONMENT);\n> >> +     if (index_prefix && (!*index_prefix || index_prefix[strlen(index_prefix)-1] != '/'))\n> >> +             die(\"GIT_INDEX_PREFIX must end with a slash\");\n> >\n> > Not nice (aka \"why 'must'?\").\n> \n> Simpler handling. Maybe it should append slash by itself if missing.\n\nNot maybe.  Definitely.  That was what the comment was about.\n\n> >> diff --git a/read-cache.c b/read-cache.c\n> >> index ac9a8e7..4f8d44b 100644\n> >> --- a/read-cache.c\n> >> +++ b/read-cache.c\n> >> @@ -23,6 +23,11 @@\n> >>\n> >>  struct index_state the_index;\n> >>\n> >> +static int outside_index_prefix(const struct index_state *istate, const char *ce_name)\n> >> +{\n> >> +     return istate == &the_index && get_index_prefix() && prefixcmp(ce_name, get_index_prefix());\n> >> +}\n> >\n> > The first check above needs to be justified.\n> >\n> > If you say \"outside of this path are off-limits\", why do you allow a\n> > temporary index that is used during a partial commit and other\n> > index_states excempt from that rule?\n> \n> Because unpack_trees writes new index from scratch so it will always \n> violate that. That check is IMO enough for simple index manipulation \n> like add/remove an entry. For unpack_trees, I have check_index_prefix() \n> to match a temporary index with current index before it gets written to \n> disk.\n\nunpack_trees() is what drives merges.  Does that mean that the only \nopportunity to end up with conflicts is _not_ prevented?\n\nCiao,\nDscho\n"},{"id":"78751","messageId":"fcaeb9bf0806050503y321b6730l7269443ac84ae8a6@mail.gmail.com","threadId":"13795","inReplyTo":"alpine.DEB.1.00.0806050532240.21190@racer","subject":"Re: [WIP/RFC PATCH 1/2] Introduce GIT_INDEX_PREFIX","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-06-05T12:03:35Z","receivedAt":"2008-06-05T12:03:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jun 5, 2008 at 11:35 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Thu, 5 Jun 2008, Nguyen Thai Ngoc Duy wrote:\n>\n>> On Thu, Jun 5, 2008 at 1:00 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> > Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n>> >\n>> >> GIT_INDEX_PREFIX is used to limit write access to a specific directory.\n>> >> Only \"important\" information is protected by index prefix (those will\n>> >> be used to create tree objects)\n>> >>\n>> >> When GIT_INDEX_PREFIX is set, any attempt to modify the index (refresh\n>> >> it is okay though) will bail out. read-tree and merge, however, can\n>> >> write to full index. For merge, no conflict is allowed outside index\n>> >> prefix.\n>> >\n>> > This is kind of hard to judge as part of \"narrow checkout\" series,\n>> > because it is not clear how this will actually _help_ narrow checkout.\n>>\n>> When you do narrow checkout. You only have a subdirectory, so you would\n>> not want some commands to accidentally change things outside that\n>> subdirectory in index.\n>\n> When they operate purely on the index?  Why not?\n\nThey don't. But if they only operate on a subdirectory only, they must\noperate only on that subdirectory in index (exceptions: git-merge and\ngit-read-tree)\n\n>\n> I guess that Junio's point was that the semantics are not at all clear.\n\nOn the second thought, I think this series is not a requirement for\nnarrow checkout. But it does help to\n -  detect any index violation (from unsupported commands)\n - draw an inititial path to index manipulation, needed for narrow\ncheckout (cache_refresh_ent() will be enhanced not to stat() outside\nsubtree though)\n\n>\n>> > In other words, as a standalone \"protect parts outside a single\n>> > subdirectory\" it can be reviewed and judged, but it is unclear how it\n>> > would help narrow checkout if you excempted only a _single_\n>> > subdirectory. E.g. you might want to limit yourself to arch/x86 _and_\n>> > include/asm-x86.\n>>\n>> Well it could be extended to support multiple path separated by colon\n>> later if someone needs it :)\n>\n> The thing is: if the concept is sound, chances are that it is _easy_ to\n> support that, should someone need it.\n\nIt should be easy except check_index_prefix() for multiple index prefixes.\n\n>\n>> >> @@ -71,6 +73,9 @@ static void setup_git_env(void)\n>> >>       git_graft_file = getenv(GRAFT_ENVIRONMENT);\n>> >>       if (!git_graft_file)\n>> >>               git_graft_file = xstrdup(git_path(\"info/grafts\"));\n>> >> +     index_prefix = getenv(INDEX_PREFIX_ENVIRONMENT);\n>> >> +     if (index_prefix && (!*index_prefix || index_prefix[strlen(index_prefix)-1] != '/'))\n>> >> +             die(\"GIT_INDEX_PREFIX must end with a slash\");\n>> >\n>> > Not nice (aka \"why 'must'?\").\n>>\n>> Simpler handling. Maybe it should append slash by itself if missing.\n>\n> Not maybe.  Definitely.  That was what the comment was about.\n\nOK. Definitetly :)\n\n>\n>> >> diff --git a/read-cache.c b/read-cache.c\n>> >> index ac9a8e7..4f8d44b 100644\n>> >> --- a/read-cache.c\n>> >> +++ b/read-cache.c\n>> >> @@ -23,6 +23,11 @@\n>> >>\n>> >>  struct index_state the_index;\n>> >>\n>> >> +static int outside_index_prefix(const struct index_state *istate, const char *ce_name)\n>> >> +{\n>> >> +     return istate == &the_index && get_index_prefix() && prefixcmp(ce_name, get_index_prefix());\n>> >> +}\n>> >\n>> > The first check above needs to be justified.\n>> >\n>> > If you say \"outside of this path are off-limits\", why do you allow a\n>> > temporary index that is used during a partial commit and other\n>> > index_states excempt from that rule?\n>>\n>> Because unpack_trees writes new index from scratch so it will always\n>> violate that. That check is IMO enough for simple index manipulation\n>> like add/remove an entry. For unpack_trees, I have check_index_prefix()\n>> to match a temporary index with current index before it gets written to\n>> disk.\n>\n> unpack_trees() is what drives merges.  Does that mean that the only\n> opportunity to end up with conflicts is _not_ prevented?\n\nI am not sure I understand this. In my opinion, due to unpack_trees()\nbehavior, it is free to write to temporary indexes. I don't know if\nany channce conflicts will be generated without unpack_trees(). But\nthen if an index is to be written to .git/index, it must be checked\nagainst .git/index plus index prefix. Any opportunity that ends up\nwith conflicts does not matter, as long as it will not be written to\n.git/index or conflicts are visible in narrow checkout. Otherwise it\nmust be checked. What I care is that .git/index is controlled, so that\npeople won't be able to change anything outside index prefix by\naccident.\n\n> Ciao,\n> Dscho\n>\n>\n\n\n\n-- \nDuy\n"}]}