From f0056f6419deb6b4e226976c571f6429875519f1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Oct 2016 02:16:41 -0400 Subject: read info/{attributes,exclude} only when in repository The low-level attribute and gitignore code will try to look in $GIT_DIR/info for any repo-level configuration files, even if we have not actually determined that we are in a repository (e.g., running "git grep --no-index"). In such a case they end up looking for ".git/info/attributes", etc. This is generally harmless, as such a file is unlikely to exist outside of a repository, but it's still conceptually the wrong thing to do. Let's detect this situation explicitly and skip reading the file (i.e., the same behavior we'd get if we were in a repository and the file did not exist). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- attr.c | 6 +++++- dir.c | 12 ++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/attr.c b/attr.c index eec5d7d15a..1fcf042b87 100644 --- a/attr.c +++ b/attr.c @@ -531,7 +531,11 @@ static void bootstrap_attr_stack(void) debug_push(elem); } - elem = read_attr_from_file(git_path_info_attributes(), 1); + if (startup_info->have_repository) + elem = read_attr_from_file(git_path_info_attributes(), 1); + else + elem = NULL; + if (!elem) elem = xcalloc(1, sizeof(*elem)); elem->origin = NULL; diff --git a/dir.c b/dir.c index f9412e0213..bfa8c8a9a5 100644 --- a/dir.c +++ b/dir.c @@ -2237,8 +2237,6 @@ static GIT_PATH_FUNC(git_path_info_exclude, "info/exclude") void setup_standard_excludes(struct dir_struct *dir) { - const char *path; - dir->exclude_per_dir = ".gitignore"; /* core.excludefile defaulting to $XDG_HOME/git/ignore */ @@ -2249,10 +2247,12 @@ void setup_standard_excludes(struct dir_struct *dir) dir->untracked ? &dir->ss_excludes_file : NULL); /* per repository user preference */ - path = git_path_info_exclude(); - if (!access_or_warn(path, R_OK, 0)) - add_excludes_from_file_1(dir, path, - dir->untracked ? &dir->ss_info_exclude : NULL); + if (startup_info->have_repository) { + const char *path = git_path_info_exclude(); + if (!access_or_warn(path, R_OK, 0)) + add_excludes_from_file_1(dir, path, + dir->untracked ? &dir->ss_info_exclude : NULL); + } } int remove_path(const char *name) -- cgit v1.2.1 From 4ce742fc9c09cd3b95be309bc093f8fa54c15f96 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Oct 2016 02:16:59 -0400 Subject: test-*-cache-tree: setup git dir These test helper programs access the index, but do not ever setup_git_directory(), meaning we just blindly looked in ".git/index". This happened to work for the purposes of our tests (which do not run from subdirectories, nor in non-repos), but it's a bad habit. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/helper/test-dump-cache-tree.c | 1 + t/helper/test-scrap-cache-tree.c | 1 + 2 files changed, 2 insertions(+) diff --git a/t/helper/test-dump-cache-tree.c b/t/helper/test-dump-cache-tree.c index 44f3290258..7af116d49e 100644 --- a/t/helper/test-dump-cache-tree.c +++ b/t/helper/test-dump-cache-tree.c @@ -58,6 +58,7 @@ int cmd_main(int ac, const char **av) { struct index_state istate; struct cache_tree *another = cache_tree(); + setup_git_directory(); if (read_cache() < 0) die("unable to read index file"); istate = the_index; diff --git a/t/helper/test-scrap-cache-tree.c b/t/helper/test-scrap-cache-tree.c index 5b2fd09908..27fe0405b8 100644 --- a/t/helper/test-scrap-cache-tree.c +++ b/t/helper/test-scrap-cache-tree.c @@ -7,6 +7,7 @@ static struct lock_file index_lock; int cmd_main(int ac, const char **av) { + setup_git_directory(); hold_locked_index(&index_lock, 1); if (read_cache() < 0) die("unable to read index file"); -- cgit v1.2.1 From ef2ed5013c4160284d9de18903bd4f7d0542d810 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Oct 2016 02:19:19 -0400 Subject: find_unique_abbrev: use 4-buffer ring Some code paths want to format multiple abbreviated sha1s in the same output line. Because we use a single static buffer for our return value, they have to either break their output into several calls or allocate their own arrays and use find_unique_abbrev_r(). Intead, let's mimic sha1_to_hex() and use a ring of several buffers, so that the return value stays valid through multiple calls. This shortens some of the callers, and makes it harder to for them to make a silly mistake. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/merge.c | 11 +++++------ builtin/receive-pack.c | 16 ++++++---------- cache.h | 4 ++-- sha1_name.c | 4 +++- 4 files changed, 16 insertions(+), 19 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index a8b57c7d98..b65eeaa87d 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1374,12 +1374,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix) struct commit *commit; if (verbosity >= 0) { - char from[GIT_SHA1_HEXSZ + 1], to[GIT_SHA1_HEXSZ + 1]; - find_unique_abbrev_r(from, head_commit->object.oid.hash, - DEFAULT_ABBREV); - find_unique_abbrev_r(to, remoteheads->item->object.oid.hash, - DEFAULT_ABBREV); - printf(_("Updating %s..%s\n"), from, to); + printf(_("Updating %s..%s\n"), + find_unique_abbrev(head_commit->object.oid.hash, + DEFAULT_ABBREV), + find_unique_abbrev(remoteheads->item->object.oid.hash, + DEFAULT_ABBREV)); } strbuf_addstr(&msg, "Fast-forward"); if (have_message) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 04ed38e17d..680759d256 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1163,10 +1163,6 @@ static void check_aliased_update(struct command *cmd, struct string_list *list) struct string_list_item *item; struct command *dst_cmd; unsigned char sha1[GIT_SHA1_RAWSZ]; - char cmd_oldh[GIT_SHA1_HEXSZ + 1], - cmd_newh[GIT_SHA1_HEXSZ + 1], - dst_oldh[GIT_SHA1_HEXSZ + 1], - dst_newh[GIT_SHA1_HEXSZ + 1]; int flag; strbuf_addf(&buf, "%s%s", get_git_namespace(), cmd->ref_name); @@ -1197,14 +1193,14 @@ static void check_aliased_update(struct command *cmd, struct string_list *list) dst_cmd->skip_update = 1; - find_unique_abbrev_r(cmd_oldh, cmd->old_sha1, DEFAULT_ABBREV); - find_unique_abbrev_r(cmd_newh, cmd->new_sha1, DEFAULT_ABBREV); - find_unique_abbrev_r(dst_oldh, dst_cmd->old_sha1, DEFAULT_ABBREV); - find_unique_abbrev_r(dst_newh, dst_cmd->new_sha1, DEFAULT_ABBREV); rp_error("refusing inconsistent update between symref '%s' (%s..%s) and" " its target '%s' (%s..%s)", - cmd->ref_name, cmd_oldh, cmd_newh, - dst_cmd->ref_name, dst_oldh, dst_newh); + cmd->ref_name, + find_unique_abbrev(cmd->old_sha1, DEFAULT_ABBREV), + find_unique_abbrev(cmd->new_sha1, DEFAULT_ABBREV), + dst_cmd->ref_name, + find_unique_abbrev(dst_cmd->old_sha1, DEFAULT_ABBREV), + find_unique_abbrev(dst_cmd->new_sha1, DEFAULT_ABBREV)); cmd->error_string = dst_cmd->error_string = "inconsistent aliased update"; diff --git a/cache.h b/cache.h index f7ee414563..607fe6ed30 100644 --- a/cache.h +++ b/cache.h @@ -903,8 +903,8 @@ extern char *sha1_pack_index_name(const unsigned char *sha1); * The result will be at least `len` characters long, and will be NUL * terminated. * - * The non-`_r` version returns a static buffer which will be overwritten by - * subsequent calls. + * The non-`_r` version returns a static buffer which remains valid until 4 + * more calls to find_unique_abbrev are made. * * The `_r` variant writes to a buffer supplied by the caller, which must be at * least `GIT_SHA1_HEXSZ + 1` bytes. The return value is the number of bytes diff --git a/sha1_name.c b/sha1_name.c index 4092836146..36ce9b9f45 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -472,7 +472,9 @@ int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len) const char *find_unique_abbrev(const unsigned char *sha1, int len) { - static char hex[GIT_SHA1_HEXSZ + 1]; + static int bufno; + static char hexbuffer[4][GIT_SHA1_HEXSZ + 1]; + char *hex = hexbuffer[3 & ++bufno]; find_unique_abbrev_r(hex, sha1, len); return hex; } -- cgit v1.2.1 From d5e3b01e5bd6b06c06dbd5d1e2257d57e6b1deb7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Oct 2016 02:19:43 -0400 Subject: diff_unique_abbrev: rename to diff_aligned_abbrev The word "align" describes how the function actually differs from find_unique_abbrev, and will make it less confusing when we add more diff-specific abbrevation functions that do not do this alignment. Since this is a globally available function, let's also move its descriptive comment to the header file, where we typically document function interfaces. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- combine-diff.c | 6 +++--- diff.c | 10 +++------- diff.h | 6 +++++- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/combine-diff.c b/combine-diff.c index 8e2a577bdb..b36c2d16bd 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -1203,9 +1203,9 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re /* Show sha1's */ for (i = 0; i < num_parent; i++) - printf(" %s", diff_unique_abbrev(p->parent[i].oid.hash, - opt->abbrev)); - printf(" %s ", diff_unique_abbrev(p->oid.hash, opt->abbrev)); + printf(" %s", diff_aligned_abbrev(p->parent[i].oid.hash, + opt->abbrev)); + printf(" %s ", diff_aligned_abbrev(p->oid.hash, opt->abbrev)); } if (opt->output_format & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS)) { diff --git a/diff.c b/diff.c index ae87888d1f..3cdf920672 100644 --- a/diff.c +++ b/diff.c @@ -4157,11 +4157,7 @@ void diff_free_filepair(struct diff_filepair *p) free(p); } -/* - * This is different from find_unique_abbrev() in that - * it stuffs the result with dots for alignment. - */ -const char *diff_unique_abbrev(const unsigned char *sha1, int len) +const char *diff_aligned_abbrev(const unsigned char *sha1, int len) { int abblen; const char *abbrev; @@ -4209,9 +4205,9 @@ static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt) fprintf(opt->file, "%s", diff_line_prefix(opt)); if (!(opt->output_format & DIFF_FORMAT_NAME_STATUS)) { fprintf(opt->file, ":%06o %06o %s ", p->one->mode, p->two->mode, - diff_unique_abbrev(p->one->oid.hash, opt->abbrev)); + diff_aligned_abbrev(p->one->oid.hash, opt->abbrev)); fprintf(opt->file, "%s ", - diff_unique_abbrev(p->two->oid.hash, opt->abbrev)); + diff_aligned_abbrev(p->two->oid.hash, opt->abbrev)); } if (p->score) { fprintf(opt->file, "%c%03d%c", p->status, similarity_index(p), diff --git a/diff.h b/diff.h index 25ae60d5ff..f2b04b6d81 100644 --- a/diff.h +++ b/diff.h @@ -340,7 +340,11 @@ extern void diff_warn_rename_limit(const char *varname, int needed, int degraded #define DIFF_STATUS_FILTER_AON '*' #define DIFF_STATUS_FILTER_BROKEN 'B' -extern const char *diff_unique_abbrev(const unsigned char *, int); +/* + * This is different from find_unique_abbrev() in that + * it stuffs the result with dots for alignment. + */ +extern const char *diff_aligned_abbrev(const unsigned char *sha1, int); /* do not report anything on removed paths */ #define DIFF_SILENT_ON_REMOVED 01 -- cgit v1.2.1 From d6cece51b83db5d8a523c4ba857013c4242e310e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Oct 2016 02:20:07 -0400 Subject: diff_aligned_abbrev: use "struct oid" Since we're modifying this function anyway, it's a good time to update it to the more modern "struct oid". We can also drop some of the magic numbers in favor of GIT_SHA1_HEXSZ, along with some descriptive comments. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- combine-diff.c | 4 ++-- diff.c | 20 +++++++++++--------- diff.h | 2 +- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/combine-diff.c b/combine-diff.c index b36c2d16bd..59501db99a 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -1203,9 +1203,9 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re /* Show sha1's */ for (i = 0; i < num_parent; i++) - printf(" %s", diff_aligned_abbrev(p->parent[i].oid.hash, + printf(" %s", diff_aligned_abbrev(&p->parent[i].oid, opt->abbrev)); - printf(" %s ", diff_aligned_abbrev(p->oid.hash, opt->abbrev)); + printf(" %s ", diff_aligned_abbrev(&p->oid, opt->abbrev)); } if (opt->output_format & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS)) { diff --git a/diff.c b/diff.c index 3cdf920672..b94e0e0711 100644 --- a/diff.c +++ b/diff.c @@ -4157,14 +4157,15 @@ void diff_free_filepair(struct diff_filepair *p) free(p); } -const char *diff_aligned_abbrev(const unsigned char *sha1, int len) +const char *diff_aligned_abbrev(const struct object_id *oid, int len) { int abblen; const char *abbrev; - if (len == 40) - return sha1_to_hex(sha1); - abbrev = find_unique_abbrev(sha1, len); + if (len == GIT_SHA1_HEXSZ) + return oid_to_hex(oid); + + abbrev = find_unique_abbrev(oid->hash, len); abblen = strlen(abbrev); /* @@ -4186,15 +4187,16 @@ const char *diff_aligned_abbrev(const unsigned char *sha1, int len) * the automatic sizing is supposed to give abblen that ensures * uniqueness across all objects (statistically speaking). */ - if (abblen < 37) { - static char hex[41]; + if (abblen < GIT_SHA1_HEXSZ - 3) { + static char hex[GIT_SHA1_HEXSZ + 1]; if (len < abblen && abblen <= len + 2) xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, ".."); else xsnprintf(hex, sizeof(hex), "%s...", abbrev); return hex; } - return sha1_to_hex(sha1); + + return oid_to_hex(oid); } static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt) @@ -4205,9 +4207,9 @@ static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt) fprintf(opt->file, "%s", diff_line_prefix(opt)); if (!(opt->output_format & DIFF_FORMAT_NAME_STATUS)) { fprintf(opt->file, ":%06o %06o %s ", p->one->mode, p->two->mode, - diff_aligned_abbrev(p->one->oid.hash, opt->abbrev)); + diff_aligned_abbrev(&p->one->oid, opt->abbrev)); fprintf(opt->file, "%s ", - diff_aligned_abbrev(p->two->oid.hash, opt->abbrev)); + diff_aligned_abbrev(&p->two->oid, opt->abbrev)); } if (p->score) { fprintf(opt->file, "%c%03d%c", p->status, similarity_index(p), diff --git a/diff.h b/diff.h index f2b04b6d81..01afc70bd5 100644 --- a/diff.h +++ b/diff.h @@ -344,7 +344,7 @@ extern void diff_warn_rename_limit(const char *varname, int needed, int degraded * This is different from find_unique_abbrev() in that * it stuffs the result with dots for alignment. */ -extern const char *diff_aligned_abbrev(const unsigned char *sha1, int); +extern const char *diff_aligned_abbrev(const struct object_id *sha1, int); /* do not report anything on removed paths */ #define DIFF_SILENT_ON_REMOVED 01 -- cgit v1.2.1 From 4f03666ac69ec4799998f010d04916c12e38edf8 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 20 Oct 2016 02:21:25 -0400 Subject: diff: handle sha1 abbreviations outside of repository When generating diffs outside a repository (e.g., with "diff --no-index"), we may write abbreviated sha1s as part of "--raw" output or the "index" lines of "--patch" output. Since we have no object database, we never find any collisions, and these sha1s get whatever static abbreviation length is configured (typically 7). However, we do blindly look in ".git/objects" to see if any objects exist, even though we know we are not in a repository. This is usually harmless because such a directory is unlikely to exist, but could be wrong in rare circumstances. Let's instead notice when we are not in a repository and behave as if the object database is empty (i.e., just use the default abbrev length). It would perhaps make sense to be conservative and show full sha1s in that case, but showing the default abbreviation is what we've always done (and is certainly less ugly). Note that this does mean that: cd /not/a/repo GIT_OBJECT_DIRECTORY=/some/real/objdir git diff --no-index ... used to look for collisions in /some/real/objdir but now does not. This could be considered either a bugfix (we do not look at objects if we have no repository) or a regression, but it seems unlikely that anybody would care much either way. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diff.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/diff.c b/diff.c index b94e0e0711..4c47bb3a2d 100644 --- a/diff.c +++ b/diff.c @@ -3096,6 +3096,19 @@ static int similarity_index(struct diff_filepair *p) return p->score * 100 / MAX_SCORE; } +static const char *diff_abbrev_oid(const struct object_id *oid, int abbrev) +{ + if (startup_info->have_repository) + return find_unique_abbrev(oid->hash, abbrev); + else { + char *hex = oid_to_hex(oid); + if (abbrev < 0 || abbrev > GIT_SHA1_HEXSZ) + die("BUG: oid abbreviation out of range: %d", abbrev); + hex[abbrev] = '\0'; + return hex; + } +} + static void fill_metainfo(struct strbuf *msg, const char *name, const char *other, @@ -3154,9 +3167,9 @@ static void fill_metainfo(struct strbuf *msg, (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two))) abbrev = 40; } - strbuf_addf(msg, "%s%sindex %s..", line_prefix, set, - find_unique_abbrev(one->oid.hash, abbrev)); - strbuf_add_unique_abbrev(msg, two->oid.hash, abbrev); + strbuf_addf(msg, "%s%sindex %s..%s", line_prefix, set, + diff_abbrev_oid(&one->oid, abbrev), + diff_abbrev_oid(&two->oid, abbrev)); if (one->mode == two->mode) strbuf_addf(msg, " %06o", one->mode); strbuf_addf(msg, "%s\n", reset); @@ -4165,7 +4178,7 @@ const char *diff_aligned_abbrev(const struct object_id *oid, int len) if (len == GIT_SHA1_HEXSZ) return oid_to_hex(oid); - abbrev = find_unique_abbrev(oid->hash, len); + abbrev = diff_abbrev_oid(oid, len); abblen = strlen(abbrev); /* -- cgit v1.2.1