@matheusbsbs / matheus-bagliano / commits / 07098b81a4

Simplify handling of setup_git_directory_gently() failure cases.

setup_git_directory_gently() expects two types of failures to discover a git directory (e.g. .git/): - GIT_DIR_HIT_CEILING: could not find a git directory in any parent directories of the cwd. - GIT_DIR_HIT_MOUNT_POINT: could not find a git directory in any parent directories up to the mount point of the cwd. Both cases are handled in a similar way, but there are misleading and unimportant differences. In both cases, setup_git_directory_gently() should: - Die if we are not in a git repository. Otherwise: - Set nongit_ok = 1, indicating that we are not in a git repository but this is ok. - Call strbuf_release() on any non-static struct strbufs that we allocated. Before this change are two misleading additional behaviors: - GIT_DIR_HIT_CEILING: setup_nongit() changes to the cwd for no apparent reason. We never had the chance to change directories up to this point so chdir(current cwd) is pointless. - GIT_DIR_HIT_MOUNT_POINT: strbuf_release() frees the buffer of a static struct strbuf (cwd). This is unnecessary because the struct is static so its buffer is always reachable. This is also misleading because nowhere else in the function is this buffer released. This change eliminates these two misleading additional behaviors and deletes setup_nogit() because the code is clearer without it. The result is that we can see clearly that GIT_DIR_HIT_CEILING and GIT_DIR_HIT_MOUNT_POINT lead to the same behavior (ignoring the different help messages). During review, this change was amended to additionally include: - Neither GIT_DIR_HIT_CEILING nor GIT_DIR_HIT_MOUNT_POINT may return early from setup_git_directory_gently() before the GIT_PREFIX environment variable is reset. Change both cases to break instead of return. See GIT_PREFIX below for more details. - GIT_DIR_NONE: setup_git_directory_gently_1() never returns this value, but if it ever did, setup_git_directory_gently() would incorrectly record that it had found a repository. Explicitly BUG on this case because it is underspecified. - GIT_PREFIX: this environment variable must always match the value of startup_info->prefix and the prefix returned from setup_git_directory_gently(). Make how we handle this slightly more repetitive but also more clear. - setup_git_env() and repo_set_hash_algo(): Add comments showing that only GIT_DIR_EXPLICIT, GIT_DIR_DISCOVERED, and GIT_DIR_BARE will cause setup_git_directory_gently() to call these setup functions. This was obvious (but partly incorrect) before this change when GIT_DIR_HIT_MOUNT_POINT returned early from setup_git_directory_gently(). Signed-off-by: Junio C Hamano <gitster@pobox.com>

Erin Dahlgren committed Dec 27, 2018 at 15:36 UTC 07098b81a432cf19e3bbe3fe949e77a64aa970c6

1 file changed +43 -31

setup.c

+43 -31

@@ -831,16 +831,6 @@ static const char *setup_bare_git_dir(struct strbuf *cwd, int offset,

831 return NULL;

832 }

833

834 -static const char *setup_nongit(const char *cwd, int *nongit_ok)

835 -{

836 - if (!nongit_ok)

837 - die(_("not a git repository (or any of the parent directories): %s"), DEFAULT_GIT_DIR_ENVIRONMENT);

838 - if (chdir(cwd))

839 - die_errno(_("cannot come back to cwd"));

840 - *nongit_ok = 1;

841 - return NULL;

842 -}

843 -

834 static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_len)

835 {

836 struct stat buf;

@@ -1054,7 +1044,7 @@ const char *setup_git_directory_gently(int *nongit_ok)

1044 {

1045 static struct strbuf cwd = STRBUF_INIT;

1046 struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT;

1057 - const char *prefix;

1047 + const char *prefix = NULL;

1048 struct repository_format repo_fmt;

1049

1050 /*

@@ -1079,9 +1069,6 @@ const char *setup_git_directory_gently(int *nongit_ok)

1069 strbuf_addbuf(&dir, &cwd);

1070

1071 switch (setup_git_directory_gently_1(&dir, &gitdir, 1)) {

1082 - case GIT_DIR_NONE:

1083 - prefix = NULL;

1084 - break;

1072 case GIT_DIR_EXPLICIT:

1073 prefix = setup_explicit_git_dir(gitdir.buf, &cwd, &repo_fmt, nongit_ok);

1074 break;

@@ -1097,29 +1084,51 @@ const char *setup_git_directory_gently(int *nongit_ok)

1084 prefix = setup_bare_git_dir(&cwd, dir.len, &repo_fmt, nongit_ok);

1085 break;

1086 case GIT_DIR_HIT_CEILING:

1100 - prefix = setup_nongit(cwd.buf, nongit_ok);

1087 + if (!nongit_ok)

1088 + die(_("not a git repository (or any of the parent directories): %s"),

1089 + DEFAULT_GIT_DIR_ENVIRONMENT);

1090 + *nongit_ok = 1;

1091 break;

1092 case GIT_DIR_HIT_MOUNT_POINT:

1103 - if (nongit_ok) {

1104 - *nongit_ok = 1;

1105 - strbuf_release(&cwd);

1106 - strbuf_release(&dir);

1107 - return NULL;

1108 - }

1109 - die(_("not a git repository (or any parent up to mount point %s)\n"

1110 - "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),

1111 - dir.buf);

1093 + if (!nongit_ok)

1094 + die(_("not a git repository (or any parent up to mount point %s)\n"

1095 + "Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)."),

1096 + dir.buf);

1097 + *nongit_ok = 1;

1098 + break;

1099 + case GIT_DIR_NONE:

1100 + /*

1101 + * As a safeguard against setup_git_directory_gently_1 returning

1102 + * this value, fallthrough to BUG. Otherwise it is possible to

1103 + * set startup_info->have_repository to 1 when we did nothing to

1104 + * find a repository.

1105 + */

1106 default:

1107 BUG("unhandled setup_git_directory_1() result");

1108 }

1109

1116 - if (prefix)

1117 - setenv(GIT_PREFIX_ENVIRONMENT, prefix, 1);

1118 - else

1110 + /*

1111 + * At this point, nongit_ok is stable. If it is non-NULL and points

1112 + * to a non-zero value, then this means that we haven't found a

1113 + * repository and that the caller expects startup_info to reflect

1114 + * this.

1115 + *

1116 + * Regardless of the state of nongit_ok, startup_info->prefix and

1117 + * the GIT_PREFIX environment variable must always match. For details

1118 + * see Documentation/config/alias.txt.

1119 + */

1120 + if (nongit_ok && *nongit_ok) {

1121 + startup_info->have_repository = 0;

1122 + startup_info->prefix = NULL;

1123 setenv(GIT_PREFIX_ENVIRONMENT, "", 1);

1120 -

1121 - startup_info->have_repository = !nongit_ok || !*nongit_ok;

1122 - startup_info->prefix = prefix;

1124 + } else {

1125 + startup_info->have_repository = 1;

1126 + startup_info->prefix = prefix;

1127 + if (prefix)

1128 + setenv(GIT_PREFIX_ENVIRONMENT, prefix, 1);

1129 + else

1130 + setenv(GIT_PREFIX_ENVIRONMENT, "", 1);

1131 + }

1132

1133 /*

1134 * Not all paths through the setup code will call 'set_git_dir()' (which

@@ -1132,7 +1141,10 @@ const char *setup_git_directory_gently(int *nongit_ok)

1141 * the user has set GIT_DIR. It may be beneficial to disallow bogus

1142 * GIT_DIR values at some point in the future.

1143 */

1135 - if (startup_info->have_repository || getenv(GIT_DIR_ENVIRONMENT)) {

1144 + if (/* GIT_DIR_EXPLICIT, GIT_DIR_DISCOVERED, GIT_DIR_BARE */

1145 + startup_info->have_repository ||

1146 + /* GIT_DIR_EXPLICIT */

1147 + getenv(GIT_DIR_ENVIRONMENT)) {

1148 if (!the_repository->gitdir) {

1149 const char *gitdir = getenv(GIT_DIR_ENVIRONMENT);

1150 if (!gitdir)