{"thread":{"id":"22810","subject":"[PATCHv5 0/5] repo-local env vars handling","startedAt":"2010-02-24T23:34:13Z","lastAt":"2010-02-27T21:07:04Z","messageCount":7,"participants":["Giuseppe Bilotta","Heiko Voigt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"135629","messageId":"1267054458-11877-1-git-send-email-giuseppe.bilotta@gmail.com","threadId":"22810","inReplyTo":null,"subject":"[PATCHv5 0/5] repo-local env vars handling","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-24T23:34:13Z","receivedAt":"2010-02-24T23:34:13Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Changes from the previous iteration:\n\n* the first two patches were squashed together\n* local_repo_env_size is now a #define to the number of non-NULL entries in \n  the list of repo-local env vars. The last patch is consequently adjusted.\n\nGiuseppe Bilotta (5):\n  Refactor list of of repo-local env vars\n  rev-parse: --local-env-vars option\n  shell setup: clear_local_git_env() function\n  submodules: ensure clean environment when operating in a submodule\n  is_submodule_modified(): clear environment properly\n\n Documentation/git-rev-parse.txt |    6 ++++++\n builtin-rev-parse.c             |    8 ++++++++\n cache.h                         |    8 ++++++++\n connect.c                       |   14 ++------------\n environment.c                   |   15 +++++++++++++++\n git-sh-setup.sh                 |    7 +++++++\n git-submodule.sh                |   20 ++++++++++----------\n submodule.c                     |   21 +++++++++++----------\n 8 files changed, 67 insertions(+), 32 deletions(-)\n"},{"id":"135632","messageId":"1267054458-11877-2-git-send-email-giuseppe.bilotta@gmail.com","threadId":"22810","inReplyTo":"1267054458-11877-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv5 1/5] Refactor list of of repo-local env vars","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-24T23:34:14Z","receivedAt":"2010-02-24T23:34:14Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Move the list of GIT_* environment variables that are local to a\nrepository into a static list in environment.c, as it is also\nuseful elsewhere. Also add the missing GIT_CONFIG variable to the\nlist.\n\nMake it easy to use the list both by NULL-termination and by size;\nthe latter (excluding the terminating NULL) is stored in the\nlocal_repo_env_size define.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n cache.h       |    8 ++++++++\n connect.c     |   14 ++------------\n environment.c |   15 +++++++++++++++\n 3 files changed, 25 insertions(+), 12 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex d454b7e..0832ee9 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -388,6 +388,14 @@ static inline enum object_type object_type(unsigned int mode)\n #define GIT_NOTES_REF_ENVIRONMENT \"GIT_NOTES_REF\"\n #define GIT_NOTES_DEFAULT_REF \"refs/notes/commits\"\n \n+/* Repository-local GIT_* environment variables */\n+/* The array is NULL-terminated to simplify its usage in contexts such\n+   environment creation or simple walk of the list.\n+   The number of non-NULL entries is stored in the following define\n+   to simplify preallocation of arrays that need to extend it */\n+#define local_repo_env_size 8\n+extern const char *const local_repo_env[local_repo_env_size+1];\n+\n extern int is_bare_repository_cfg;\n extern int is_bare_repository(void);\n extern int is_inside_git_dir(void);\ndiff --git a/connect.c b/connect.c\nindex 9f39038..323a771 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -582,18 +582,8 @@ struct child_process *git_connect(int fd[2], const char *url_orig,\n \t\t*arg++ = host;\n \t}\n \telse {\n-\t\t/* remove these from the environment */\n-\t\tconst char *env[] = {\n-\t\t\tALTERNATE_DB_ENVIRONMENT,\n-\t\t\tDB_ENVIRONMENT,\n-\t\t\tGIT_DIR_ENVIRONMENT,\n-\t\t\tGIT_WORK_TREE_ENVIRONMENT,\n-\t\t\tGRAFT_ENVIRONMENT,\n-\t\t\tINDEX_ENVIRONMENT,\n-\t\t\tNO_REPLACE_OBJECTS_ENVIRONMENT,\n-\t\t\tNULL\n-\t\t};\n-\t\tconn->env = env;\n+\t\t/* remove repo-local variables from the environment */\n+\t\tconn->env = local_repo_env;\n \t\tconn->use_shell = 1;\n \t}\n \t*arg++ = cmd.buf;\ndiff --git a/environment.c b/environment.c\nindex 739ec27..b3e9e8c 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -63,6 +63,21 @@ static char *work_tree;\n static const char *git_dir;\n static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;\n \n+/* Repository-local GIT_* environment variables */\n+/* Remember to update local_repo_env_size in cache.h when the size\n+   of the list changes */\n+const char *const local_repo_env[local_repo_env_size+1] = {\n+\tALTERNATE_DB_ENVIRONMENT,\n+\tCONFIG_ENVIRONMENT,\n+\tDB_ENVIRONMENT,\n+\tGIT_DIR_ENVIRONMENT,\n+\tGIT_WORK_TREE_ENVIRONMENT,\n+\tGRAFT_ENVIRONMENT,\n+\tINDEX_ENVIRONMENT,\n+\tNO_REPLACE_OBJECTS_ENVIRONMENT,\n+\tNULL\n+};\n+\n static void setup_git_env(void)\n {\n \tgit_dir = getenv(GIT_DIR_ENVIRONMENT);\n-- \n1.7.0.212.g5b851b.dirty\n"},{"id":"135633","messageId":"1267054458-11877-3-git-send-email-giuseppe.bilotta@gmail.com","threadId":"22810","inReplyTo":"1267054458-11877-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv5 2/5] rev-parse: --local-env-vars option","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-24T23:34:15Z","receivedAt":"2010-02-24T23:34:15Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"This prints the list of repo-local environment variables.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n Documentation/git-rev-parse.txt |    6 ++++++\n builtin-rev-parse.c             |    8 ++++++++\n 2 files changed, 14 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt\nindex 1a613aa..8db600f 100644\n--- a/Documentation/git-rev-parse.txt\n+++ b/Documentation/git-rev-parse.txt\n@@ -148,6 +148,12 @@ shown.  If the pattern does not contain a globbing character (`?`,\n --is-bare-repository::\n \tWhen the repository is bare print \"true\", otherwise \"false\".\n \n+--local-env-vars::\n+\tList the GIT_* environment variables that are local to the\n+\trepository (e.g. GIT_DIR or GIT_WORK_TREE, but not GIT_EDITOR).\n+\tOnly the names of the variables are listed, not their value,\n+\teven if they are set.\n+\n --short::\n --short=number::\n \tInstead of outputting the full SHA1 values of object names try to\ndiff --git a/builtin-rev-parse.c b/builtin-rev-parse.c\nindex a8c5043..ab7b520 100644\n--- a/builtin-rev-parse.c\n+++ b/builtin-rev-parse.c\n@@ -455,7 +455,15 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \tif (argc > 1 && !strcmp(\"--sq-quote\", argv[1]))\n \t\treturn cmd_sq_quote(argc - 2, argv + 2);\n \n+\tif (argc > 0 && !strcmp(\"--local-env-vars\", argv[1])) {\n+\t\tunsigned int i = 0;\n+\t\tconst char *var;\n+\t\twhile (var = local_repo_env[i++])\n+\t\t\tprintf(\"%s\\n\", var);\n+\t\treturn 0;\n+\t}\n \tif (argc > 1 && !strcmp(\"-h\", argv[1]))\n+\n \t\tusage(builtin_rev_parse_usage);\n \n \tprefix = setup_git_directory();\n-- \n1.7.0.212.g5b851b.dirty\n"},{"id":"135631","messageId":"1267054458-11877-4-git-send-email-giuseppe.bilotta@gmail.com","threadId":"22810","inReplyTo":"1267054458-11877-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv5 3/5] shell setup: clear_local_git_env() function","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-24T23:34:16Z","receivedAt":"2010-02-24T23:34:16Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Introduce an auxiliary function to clear all repo-local environment\nvariables. This should be invoked by any shell script that switches\nrepository during execution, to ensure that the environment is clean\nand that things such as the git dir and worktree are set up correctly.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n git-sh-setup.sh |    7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex 7a09566..6131670 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -172,6 +172,13 @@ get_author_ident_from_commit () {\n \tLANG=C LC_ALL=C sed -ne \"$pick_author_script\"\n }\n \n+# Clear repo-local GIT_* environment variables. Useful when switching to\n+# another repository (e.g. when entering a submodule). See also the env\n+# list in git_connect()\n+clear_local_git_env() {\n+\tunset $(git rev-parse --local-env-vars)\n+}\n+\n # Make sure we are in a valid repository of a vintage we understand,\n # if we require to be in a git repository.\n if test -z \"$NONGIT_OK\"\n-- \n1.7.0.212.g5b851b.dirty\n"},{"id":"135630","messageId":"1267054458-11877-5-git-send-email-giuseppe.bilotta@gmail.com","threadId":"22810","inReplyTo":"1267054458-11877-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv5 4/5] submodules: ensure clean environment when operating in a submodule","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-24T23:34:17Z","receivedAt":"2010-02-24T23:34:17Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"git-submodule used to take care of clearing GIT_DIR whenever it operated\non a submodule index or configuration, but forgot to unset GIT_WORK_TREE\nor other repo-local variables. This would lead to failures e.g. when\nGIT_WORK_TREE was set.\n\nThis only happened in very unusual contexts such as operating on the\nmain worktree from outside of it, but since \"git-gui: set GIT_DIR and\nGIT_WORK_TREE after setup\" (a9fa11fe5bd5978bb) such failures could also\nbe provoked by invoking an external tool such as \"git submodule update\"\nfrom the Git Gui in a standard setup.\n\nSolve by using the newly introduced clear_local_git_env() shell function\nto ensure that all repo-local environment variables are unset.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n git-submodule.sh |   20 ++++++++++----------\n 1 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 5869c00..1ea4143 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -222,7 +222,7 @@ cmd_add()\n \n \t\tmodule_clone \"$path\" \"$realrepo\" \"$reference\" || exit\n \t\t(\n-\t\t\tunset GIT_DIR\n+\t\t\tclear_local_git_env\n \t\t\tcd \"$path\" &&\n \t\t\t# ash fails to wordsplit ${branch:+-b \"$branch\"...}\n \t\t\tcase \"$branch\" in\n@@ -278,7 +278,7 @@ cmd_foreach()\n \t\t\tname=$(module_name \"$path\")\n \t\t\t(\n \t\t\t\tprefix=\"$prefix$path/\"\n-\t\t\t\tunset GIT_DIR\n+\t\t\t\tclear_local_git_env\n \t\t\t\tcd \"$path\" &&\n \t\t\t\teval \"$@\" &&\n \t\t\t\tif test -n \"$recursive\"\n@@ -434,7 +434,7 @@ cmd_update()\n \t\t\tmodule_clone \"$path\" \"$url\" \"$reference\"|| exit\n \t\t\tsubsha1=\n \t\telse\n-\t\t\tsubsha1=$(unset GIT_DIR; cd \"$path\" &&\n+\t\t\tsubsha1=$(clear_local_git_env; cd \"$path\" &&\n \t\t\t\tgit rev-parse --verify HEAD) ||\n \t\t\tdie \"Unable to find current revision in submodule path '$path'\"\n \t\tfi\n@@ -454,7 +454,7 @@ cmd_update()\n \n \t\t\tif test -z \"$nofetch\"\n \t\t\tthen\n-\t\t\t\t(unset GIT_DIR; cd \"$path\" &&\n+\t\t\t\t(clear_local_git_env; cd \"$path\" &&\n \t\t\t\t\tgit-fetch) ||\n \t\t\t\tdie \"Unable to fetch in submodule path '$path'\"\n \t\t\tfi\n@@ -477,14 +477,14 @@ cmd_update()\n \t\t\t\t;;\n \t\t\tesac\n \n-\t\t\t(unset GIT_DIR; cd \"$path\" && $command \"$sha1\") ||\n+\t\t\t(clear_local_git_env; cd \"$path\" && $command \"$sha1\") ||\n \t\t\tdie \"Unable to $action '$sha1' in submodule path '$path'\"\n \t\t\tsay \"Submodule path '$path': $msg '$sha1'\"\n \t\tfi\n \n \t\tif test -n \"$recursive\"\n \t\tthen\n-\t\t\t(unset GIT_DIR; cd \"$path\" && cmd_update $orig_args) ||\n+\t\t\t(clear_local_git_env; cd \"$path\" && cmd_update $orig_args) ||\n \t\t\tdie \"Failed to recurse into submodule path '$path'\"\n \t\tfi\n \tdone\n@@ -492,7 +492,7 @@ cmd_update()\n \n set_name_rev () {\n \trevname=$( (\n-\t\tunset GIT_DIR\n+\t\tclear_local_git_env\n \t\tcd \"$1\" && {\n \t\t\tgit describe \"$2\" 2>/dev/null ||\n \t\t\tgit describe --tags \"$2\" 2>/dev/null ||\n@@ -760,7 +760,7 @@ cmd_status()\n \t\telse\n \t\t\tif test -z \"$cached\"\n \t\t\tthen\n-\t\t\t\tsha1=$(unset GIT_DIR; cd \"$path\" && git rev-parse --verify HEAD)\n+\t\t\t\tsha1=$(clear_local_git_env; cd \"$path\" && git rev-parse --verify HEAD)\n \t\t\t\tset_name_rev \"$path\" \"$sha1\"\n \t\t\tfi\n \t\t\tsay \"+$sha1 $displaypath$revname\"\n@@ -770,7 +770,7 @@ cmd_status()\n \t\tthen\n \t\t\t(\n \t\t\t\tprefix=\"$displaypath/\"\n-\t\t\t\tunset GIT_DIR\n+\t\t\t\tclear_local_git_env\n \t\t\t\tcd \"$path\" &&\n \t\t\t\tcmd_status $orig_args\n \t\t\t) ||\n@@ -821,7 +821,7 @@ cmd_sync()\n \t\tif test -e \"$path\"/.git\n \t\tthen\n \t\t(\n-\t\t\tunset GIT_DIR\n+\t\t\tclear_local_git_env\n \t\t\tcd \"$path\"\n \t\t\tremote=$(get_default_remote)\n \t\t\tsay \"Synchronizing submodule url for '$name'\"\n-- \n1.7.0.212.g5b851b.dirty\n"},{"id":"135634","messageId":"1267054458-11877-6-git-send-email-giuseppe.bilotta@gmail.com","threadId":"22810","inReplyTo":"1267054458-11877-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCHv5 5/5] is_submodule_modified(): clear environment properly","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-24T23:34:18Z","receivedAt":"2010-02-24T23:34:18Z","isPatch":false,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Rather than only clearing GIT_INDEX_FILE, take the list of environment\nvariables to clear from local_repo_env, appending the settings for\nGIT_DIR and GIT_WORK_TREE.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n submodule.c |   21 +++++++++++----------\n 1 files changed, 11 insertions(+), 10 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 7d70c4f..3c34d39 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -124,15 +124,19 @@ void show_submodule_summary(FILE *f, const char *path,\n int is_submodule_modified(const char *path)\n {\n \tint len;\n+\tunsigned int i = 0;\n \tstruct child_process cp;\n \tconst char *argv[] = {\n \t\t\"status\",\n \t\t\"--porcelain\",\n \t\tNULL,\n \t};\n-\tchar *env[4];\n+\tconst char *env[local_repo_env_size+3];\n \tstruct strbuf buf = STRBUF_INIT;\n \n+\tfor ( ; i < local_repo_env_size; ++i)\n+\t\tenv[i] = local_repo_env[i];\n+\n \tstrbuf_addf(&buf, \"%s/.git/\", path);\n \tif (!is_directory(buf.buf)) {\n \t\tstrbuf_release(&buf);\n@@ -143,16 +147,14 @@ int is_submodule_modified(const char *path)\n \tstrbuf_reset(&buf);\n \n \tstrbuf_addf(&buf, \"GIT_WORK_TREE=%s\", path);\n-\tenv[0] = strbuf_detach(&buf, NULL);\n+\tenv[i++] = strbuf_detach(&buf, NULL);\n \tstrbuf_addf(&buf, \"GIT_DIR=%s/.git\", path);\n-\tenv[1] = strbuf_detach(&buf, NULL);\n-\tstrbuf_addf(&buf, \"GIT_INDEX_FILE\");\n-\tenv[2] = strbuf_detach(&buf, NULL);\n-\tenv[3] = NULL;\n+\tenv[i++] = strbuf_detach(&buf, NULL);\n+\tenv[i] = NULL;\n \n \tmemset(&cp, 0, sizeof(cp));\n \tcp.argv = argv;\n-\tcp.env = (const char *const *)env;\n+\tcp.env = env;\n \tcp.git_cmd = 1;\n \tcp.no_stdin = 1;\n \tcp.out = -1;\n@@ -165,9 +167,8 @@ int is_submodule_modified(const char *path)\n \tif (finish_command(&cp))\n \t\tdie(\"git status --porcelain failed\");\n \n-\tfree(env[0]);\n-\tfree(env[1]);\n-\tfree(env[2]);\n+\tfree((char *)env[--i]);\n+\tfree((char *)env[--i]);\n \tstrbuf_release(&buf);\n \treturn len != 0;\n }\n-- \n1.7.0.212.g5b851b.dirty\n"},{"id":"135850","messageId":"20100227210701.GE33000@book.hvoigt.net","threadId":"22810","inReplyTo":"1267054458-11877-3-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCHv5 2/5] rev-parse: --local-env-vars option","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2010-02-27T21:07:04Z","receivedAt":"2010-02-27T21:07:04Z","isPatch":false,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Thu, Feb 25, 2010 at 12:34:15AM +0100, Giuseppe Bilotta wrote:\n> This prints the list of repo-local environment variables.\n> \n> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n> ---\n[...]\n> diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c\n> index a8c5043..ab7b520 100644\n> --- a/builtin-rev-parse.c\n> +++ b/builtin-rev-parse.c\n[...]\n>  \tif (argc > 1 && !strcmp(\"-h\", argv[1]))\n> +\n>  \t\tusage(builtin_rev_parse_usage);\n\nYou probably forgot to remove that empty line there.\n\ncheers Heiko\n"}]}