summaryrefslogtreecommitdiff
path: root/builtin/describe.c
AgeCommit message (Collapse)AuthorFilesLines
2020-03-26Merge branch 'jc/describe-misnamed-annotated-tag'Libravatar Junio C Hamano1-6/+9
When "git describe C" finds an annotated tag with tagname A to be the best name to explain commit C, and the tag is stored in a "wrong" place in the refs/tags hierarchy, e.g. refs/tags/B, the command gave a warning message but used A (not B) to describe C. If C is exactly at the tag, the describe output would be "A", but "git rev-parse A^0" would not be equal as "git rev-parse C^0". The behavior of the command has been changed to use the "long" form i.e. A-0-gOBJECTNAME, which is correctly interpreted by rev-parse. * jc/describe-misnamed-annotated-tag: describe: force long format for a name based on a mislocated tag
2020-02-26describe: don't abort too early when searching tagsLibravatar Benno Evers1-4/+18
When searching the commit graph for tag candidates, `git-describe` will stop as soon as there is only one active branch left and it already found an annotated tag as a candidate. This works well as long as all branches eventually connect back to a common root, but if the tags are found across branches with no common ancestor B o----. \ o-----o---o----x A it can happen that the search on one branch terminates prematurely because a tag was found on another, independent branch. This scenario isn't quite as obscure as it sounds, since cloning with a limited depth often introduces many independent "dead ends" into the commit graph. The help text of `git-describe` states pretty clearly that when describing a commit D, the number appended to the emitted tag X should correspond to the number of commits found by `git log X..D`. Thus, this commit modifies the stopping condition to only abort the search when only one branch is left to search *and* all current best candidates are descendants from that branch. For repositories with a single root, this condition is always true: When the search is reduced to a single active branch, the current commit must be an ancestor of *all* tag candidates. This means that in the common case, this change will have no negative performance impact since the same number of commits as before will be traversed. Signed-off-by: Benno Evers <benno@bmevers.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-20describe: force long format for a name based on a mislocated tagLibravatar Junio C Hamano1-6/+9
An annotated tag has two names: where it sits in the refs/tags hierarchy and the tagname recorded in the "tag" field in the object itself. They usually should match. Since 212945d4 ("Teach git-describe to verify annotated tag names before output", 2008-02-28), a commit described using an annotated tag bases its name on the tagname from the object. While this was a deliberate design decision to make it easier to converse about tags with others, even if the tags happen to be fetched to a different name than it was given by its creator, it had one downside. The output from "git describe", at least in the modern Git, should be usable as an object name to name the exact commit given to the "git describe" command. Using the tagname, when two names differ, breaks this property, when describing a commit that is directly pointed at by such a tag. An annotated tag Bob made as "v1.0" may sit at "refs/tags/v1.0-bob" in the ref hierarchy, and output from "git describe v1.0-bob^0" would say "v1.0", but there may not be any tag at "refs/tags/v1.0" locally or there may be another tag that points at a different object. Note that this won't be a problem if a commit being described is not directly pointed at by such a mislocated tag. In the example in the previous paragraph, describing a commit whose parent is v1.0-bob would result in "v1.0" (i.e. the tagname taken from the tag object) followed by "-1-gXXXXX" where XXXXX is the abbreviated object name, and a string that ends with "-g" followed by a hexadecimal string is an object name for the object whose name begins with hexadecimal string (as long as it is unique), so it does not matter if the leading part is "v1.0" or "v1.0-bob". Show the name in the long format, i.e. with "-0-gXXXXX" suffix, when the name we give is based on a mislocated annotated tag to ensure that the output can be used as the object name for the object originally given to the command to fix the issue. While at it, remove an overly cautious dead code to protect against an annotated tag object without the tagname. Such a tag is filtered out much earlier in the codeflow, and will not reach this part of the code. Helped-by: Matheus Tavares <matheus.bernardino@usp.br> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-15Merge branch 'ew/hashmap'Libravatar Junio C Hamano1-9/+12
Code clean-up of the hashmap API, both users and implementation. * ew/hashmap: hashmap_entry: remove first member requirement from docs hashmap: remove type arg from hashmap_{get,put,remove}_entry OFFSETOF_VAR macro to simplify hashmap iterators hashmap: introduce hashmap_free_entries hashmap: hashmap_{put,remove} return hashmap_entry * hashmap: use *_entry APIs for iteration hashmap_cmp_fn takes hashmap_entry params hashmap_get{,_from_hash} return "struct hashmap_entry *" hashmap: use *_entry APIs to wrap container_of hashmap_get_next returns "struct hashmap_entry *" introduce container_of macro hashmap_put takes "struct hashmap_entry *" hashmap_remove takes "const struct hashmap_entry *" hashmap_get takes "const struct hashmap_entry *" hashmap_add takes "struct hashmap_entry *" hashmap_get_next takes "const struct hashmap_entry *" hashmap_entry_init takes "struct hashmap_entry *" packfile: use hashmap_entry in delta_base_cache_entry coccicheck: detect hashmap_entry.hash assignment diff: use hashmap_entry_init on moved_entry.ent
2019-10-07OFFSETOF_VAR macro to simplify hashmap iteratorsLibravatar Eric Wong1-1/+1
While we cannot rely on a `__typeof__' operator being portable to use with `offsetof'; we can calculate the pointer offset using an existing pointer and the address of a member using pointer arithmetic for compilers without `__typeof__'. This allows us to simplify usage of hashmap iterator macros by not having to specify a type when a pointer of that type is already given. In the future, list iterator macros (e.g. list_for_each_entry) may also be implemented using OFFSETOF_VAR to save hackers the trouble of using container_of/list_entry macros and without relying on non-portable `__typeof__'. v3: use `__typeof__' to avoid clang warnings Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap: use *_entry APIs for iterationLibravatar Eric Wong1-2/+2
Inspired by list_for_each_entry in the Linux kernel. Once again, these are somewhat compromised usability-wise by compilers lacking __typeof__ support. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_cmp_fn takes hashmap_entry paramsLibravatar Eric Wong1-4/+6
Another step in eliminating the requirement of hashmap_entry being the first member of a struct. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_get{,_from_hash} return "struct hashmap_entry *"Libravatar Eric Wong1-1/+2
Update callers to use hashmap_get_entry, hashmap_get_entry_from_hash or container_of as appropriate. This is another step towards eliminating the requirement of hashmap_entry being the first field in a struct. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_add takes "struct hashmap_entry *"Libravatar Eric Wong1-1/+1
This is less error-prone than "void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_entry_init takes "struct hashmap_entry *"Libravatar Eric Wong1-1/+1
C compilers do type checking to make life easier for us. So rely on that and update all hashmap_entry_init callers to take "struct hashmap_entry *" to avoid future bugs while improving safety and readability. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-04treewide: remove duplicate #include directivesLibravatar René Scharfe1-1/+0
Found with "git grep '^#include ' '*.c' | sort | uniq -d". Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-05use get_tagged_oid()Libravatar René Scharfe1-1/+1
Avoid derefencing ->tagged without checking for NULL by using the convenience wrapper for getting the ID of the tagged object. It die()s when encountering a broken tag instead of segfaulting. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-20hashmap: convert sha1hash() to oidhash()Libravatar Jeff King1-2/+2
There are no callers left of sha1hash() that do not simply pass the "hash" member of a "struct object_id". Let's get rid of the outdated sha1-specific function and provide one that operates on the whole struct (even though the technique, taking the first few bytes of the hash, will remain the same). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-20describe: fix accidental oid/hash type-punningLibravatar Jeff King1-1/+1
The find_commit_name() function passes an object_id.hash as the key of a hashmap. That ends up in commit_name_neq(), which then feeds it to oideq(). Which means we should actually be the whole "struct object_id". It works anyway because pointers to the two are interchangeable. And because we're going through a layer of void pointers, the compiler doesn't notice the type mismatch. But it's worth cleaning up (especially since once we switch away from sha1hash() on the same line, accessing the hash member will look doubly out of place). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-06Merge branch 'ss/describe-dirty-in-the-right-directory'Libravatar Junio C Hamano1-0/+1
"git --work-tree=$there --git-dir=$here describe --dirty" did not work correctly as it did not pay attention to the location of the worktree specified by the user by mistake, which has been corrected. * ss/describe-dirty-in-the-right-directory: t6120: test for describe with a bare repository describe: setup working tree for --dirty
2019-02-04describe: setup working tree for --dirtyLibravatar Sebastian Staudt1-0/+1
We don't use NEED_WORK_TREE when running the git-describe builtin, since you should be able to describe a commit even in a bare repository. However, the --dirty flag does need a working tree. Since we don't call setup_work_tree(), it uses whatever directory we happen to be in. That's unlikely to match our index, meaning we'd say "dirty" even when the real working tree is clean. We can fix that by calling setup_work_tree() once we know that the user has asked for --dirty. The --broken option also needs a working tree. But because its implementation calls git-diff-index we don‘t have to setup the working tree in the git-describe process. Signed-off-by: Sebastian Staudt <koraktor@gmail.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-24cache.h: flip NO_THE_INDEX_COMPATIBILITY_MACROS switchLibravatar Nguyễn Thái Ngọc Duy1-0/+1
By default, index compat macros are off from now on, because they could hide the_index dependency. Only those in builtin can use it. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-14read-cache.c: replace update_index_if_able with repo_&Libravatar Nguyễn Thái Ngọc Duy1-1/+1
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-06refresh_index: remove unnecessary calls to preload_index()Libravatar Ben Peart1-1/+1
With refresh_index() learning to utilize preload_index() to speed up its operation there is no longer any benefit to having the caller preload the index first. Remove those unneeded calls by calling read_index() instead of the preload variant. There is no measurable performance impact of this patch - the 2nd call to preload_index() bails out quickly but there is no reason to call it twice. Signed-off-by: Ben Peart <benpeart@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-10-19Merge branch 'nd/the-index'Libravatar Junio C Hamano1-2/+2
Various codepaths in the core-ish part learn to work on an arbitrary in-core index structure, not necessarily the default instance "the_index". * nd/the-index: (23 commits) revision.c: reduce implicit dependency the_repository revision.c: remove implicit dependency on the_index ws.c: remove implicit dependency on the_index tree-diff.c: remove implicit dependency on the_index submodule.c: remove implicit dependency on the_index line-range.c: remove implicit dependency on the_index userdiff.c: remove implicit dependency on the_index rerere.c: remove implicit dependency on the_index sha1-file.c: remove implicit dependency on the_index patch-ids.c: remove implicit dependency on the_index merge.c: remove implicit dependency on the_index merge-blobs.c: remove implicit dependency on the_index ll-merge.c: remove implicit dependency on the_index diff-lib.c: remove implicit dependency on the_index read-cache.c: remove implicit dependency on the_index diff.c: remove implicit dependency on the_index grep.c: remove implicit dependency on the_index diff.c: remove the_index dependency in textconv() functions blame.c: rename "repo" argument to "r" combine-diff.c: remove implicit dependency on the_index ...
2018-09-21revision.c: remove implicit dependency on the_indexLibravatar Nguyễn Thái Ngọc Duy1-2/+2
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-29convert hashmap comparison functions to oideq()Libravatar Jeff King1-3/+3
The comparison functions used for hashmaps don't care about strict ordering; they only want to compare entries for equality. Let's use the oideq() function instead, which can potentially be better optimized. Note that unlike the previous patches mass-converting calls like "!oidcmp()", this patch could actually provide an improvement even with the current implementation. Those comparison functions are passed around as function pointers, so at compile-time the compiler cannot realize that the caller (which is in another file completely) will treat the return value as a boolean. Note that this does change the return values in quite a subtle way (it's still an int, but now the sign bit is irrelevant for ordering). Because of their funny hashmap-specific signature, it's unlikely that any of these static functions would be reused for more generic ordering. But to be double-sure, let's stop using "cmp" in their names. Calling them "eq" doesn't quite work either, because the hashmap convention is actually _inverted_. "0" means "same", and non-zero means "different". So I've called them "neq" by convention here. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-29convert "oidcmp() == 0" to oideq()Libravatar Jeff King1-2/+2
Using the more restrictive oideq() should, in the long run, give the compiler more opportunities to optimize these callsites. For now, this conversion should be a complete noop with respect to the generated code. The result is also perhaps a little more readable, as it avoids the "zero is equal" idiom. Since it's so prevalent in C, I think seasoned programmers tend not to even notice it anymore, but it can sometimes make for awkward double negations (e.g., we can drop a few !!oidcmp() instances here). This patch was generated almost entirely by the included coccinelle patch. This mechanical conversion should be completely safe, because we check explicitly for cases where oidcmp() is compared to 0, which is what oideq() is doing under the hood. Note that we don't have to catch "!oidcmp()" separately; coccinelle's standard isomorphisms make sure the two are treated equivalently. I say "almost" because I did hand-edit the coccinelle output to fix up a few style violations (it mostly keeps the original formatting, but sometimes unwraps long lines). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-29tag: add repository argument to lookup_tagLibravatar Stefan Beller1-3/+3
Add a repository argument to allow the callers of lookup_tag to be more specific about which repository to act on. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-29commit: add repository argument to lookup_commit_referenceLibravatar Stefan Beller1-1/+1
Add a repository argument to allow callers of lookup_commit_reference to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-29commit: add repository argument to lookup_commit_reference_gentlyLibravatar Stefan Beller1-2/+3
Add a repository argument to allow callers of lookup_commit_reference_gently to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-29Merge branch 'sb/object-store-grafts' into sb/object-store-lookupLibravatar Junio C Hamano1-0/+1
* sb/object-store-grafts: commit: allow lookup_commit_graft to handle arbitrary repositories commit: allow prepare_commit_graft to handle arbitrary repositories shallow: migrate shallow information into the object parser path.c: migrate global git_path_* to take a repository argument cache: convert get_graft_file to handle arbitrary repositories commit: convert read_graft_file to handle arbitrary repositories commit: convert register_commit_graft to handle arbitrary repositories commit: convert commit_graft_pos() to handle arbitrary repositories shallow: add repository argument to is_repository_shallow shallow: add repository argument to check_shallow_file_for_update shallow: add repository argument to register_shallow shallow: add repository argument to set_alternate_shallow_file commit: add repository argument to lookup_commit_graft commit: add repository argument to prepare_commit_graft commit: add repository argument to read_graft_file commit: add repository argument to register_commit_graft commit: add repository argument to commit_graft_pos object: move grafts to object parser object-store: move object access functions to object-store.h
2018-06-25Merge branch 'nd/commit-util-to-slab'Libravatar Junio C Hamano1-3/+13
The in-core "commit" object had an all-purpose "void *util" field, which was tricky to use especially in library-ish part of the code. All of the existing uses of the field has been migrated to a more dedicated "commit-slab" mechanism and the field is eliminated. * nd/commit-util-to-slab: commit.h: delete 'util' field in struct commit merge: use commit-slab in merge remote desc instead of commit->util log: use commit-slab in prepare_bases() instead of commit->util show-branch: note about its object flags usage show-branch: use commit-slab for commit-name instead of commit->util name-rev: use commit-slab for rev-name instead of commit->util bisect.c: use commit-slab for commit weight instead of commit->util revision.c: use commit-slab for show_source sequencer.c: use commit-slab to associate todo items to commits sequencer.c: use commit-slab to mark seen commits shallow.c: use commit-slab for commit depth instead of commit->util describe: use commit-slab for commit names instead of commit->util blame: use commit-slab for blame suspects instead of commit->util commit-slab: support shared commit-slab commit-slab.h: code split
2018-05-30Merge branch 'ma/lockfile-cleanup'Libravatar Junio C Hamano1-1/+1
Code clean-up to adjust to a more recent lockfile API convention that allows lockfile instances kept on the stack. * ma/lockfile-cleanup: lock_file: move static locks into functions lock_file: make function-local locks non-static refs.c: do not die if locking fails in `delete_pseudoref()` refs.c: do not die if locking fails in `write_pseudoref()` t/helper/test-write-cache: clean up lock-handling
2018-05-23Merge branch 'sb/oid-object-info'Libravatar Junio C Hamano1-1/+1
The codepath around object-info API has been taught to take the repository object (which in turn tells the API which object store the objects are to be located). * sb/oid-object-info: cache.h: allow oid_object_info to handle arbitrary repositories packfile: add repository argument to cache_or_unpack_entry packfile: add repository argument to unpack_entry packfile: add repository argument to read_object packfile: add repository argument to packed_object_info packfile: add repository argument to packed_to_object_type packfile: add repository argument to retry_bad_packed_offset cache.h: add repository argument to oid_object_info cache.h: add repository argument to oid_object_info_extended
2018-05-21describe: use commit-slab for commit names instead of commit->utilLibravatar Nguyễn Thái Ngọc Duy1-3/+13
It's done so that commit->util can be removed. See more explanation in the commit that removes commit->util. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-16object-store: move object access functions to object-store.hLibravatar Stefan Beller1-0/+1
This should make these functions easier to find and cache.h less overwhelming to read. In particular, this moves: - read_object_file - oid_object_info - write_object_file As a result, most of the codebase needs to #include object-store.h. In this patch the #include is only added to files that would fail to compile otherwise. It would be better to #include wherever identifiers from the header are used. That can happen later when we have better tooling for it. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-10lock_file: make function-local locks non-staticLibravatar Martin Ågren1-1/+1
Placing `struct lock_file`s on the stack used to be a bad idea, because the temp- and lockfile-machinery would keep a pointer into the struct. But after 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can safely have lockfiles on the stack. (This applies even if a user returns early, leaving a locked lock behind.) These `struct lock_file`s are local to their respective functions and we can drop their staticness. For good measure, I have inspected these sites and come to believe that they always release the lock, with the possible exception of bailing out using `die()` or `exit()` or by returning from a `cmd_foo()`. As pointed out by Jeff King, it would be bad if someone held on to a `struct lock_file *` for some reason. After some grepping, I agree with his findings: no-one appears to be doing that. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-26cache.h: add repository argument to oid_object_infoLibravatar Stefan Beller1-1/+1
Add a repository argument to allow the callers of oid_object_info to be more specific about which repository to handle. This is a small mechanical change; it doesn't change the implementation to handle repositories other than the_repository yet. As with the previous commits, use a macro to catch callers passing a repository other than the_repository at compile time. Signed-off-by: Stefan Beller <sbeller@google.com> Reviewed-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-11exec_cmd: rename to use dash in file nameLibravatar Stefan Beller1-1/+1
This is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names. Signed-off-by: Stefan Beller <sbeller@google.com>
2018-03-14sha1_file: convert sha1_object_info* to object_idLibravatar brian m. carlson1-1/+1
Convert sha1_object_info and sha1_object_info_extended to take pointers to struct object_id and rename them to use "oid" instead of "sha1" in their names. Update the declaration and definition and apply the following semantic patch, plus the standard object_id transforms: @@ expression E1, E2; @@ - sha1_object_info(E1.hash, E2) + oid_object_info(&E1, E2) @@ expression E1, E2; @@ - sha1_object_info(E1->hash, E2) + oid_object_info(E1, E2) @@ expression E1, E2, E3; @@ - sha1_object_info_extended(E1.hash, E2, E3) + oid_object_info_extended(&E1, E2, E3) @@ expression E1, E2, E3; @@ - sha1_object_info_extended(E1->hash, E2, E3) + oid_object_info_extended(E1, E2, E3) Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-14Convert find_unique_abbrev* to struct object_idLibravatar brian m. carlson1-2/+2
Convert find_unique_abbrev and find_unique_abbrev_r to each take a pointer to struct object_id. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-27Merge branch 'sb/describe-blob'Libravatar Junio C Hamano1-1/+1
"git describe $garbage" stopped giving any errors when the garbage happens to be a string with 40 hexadecimal letters. * sb/describe-blob: describe: confirm that blobs actually exist
2018-02-13Merge branch 'rs/describe-unique-abbrev'Libravatar Junio C Hamano1-1/+1
Code clean-up. * rs/describe-unique-abbrev: describe: use strbuf_add_unique_abbrev() for adding short hashes
2018-02-12describe: confirm that blobs actually existLibravatar Jeff King1-1/+1
Prior to 644eb60bd0 (builtin/describe.c: describe a blob, 2017-11-15), we noticed and complained about missing objects, since they were not valid commits: $ git describe 0000000000000000000000000000000000000000 fatal: 0000000000000000000000000000000000000000 is not a valid 'commit' object After that commit, we feed any non-commit to lookup_blob(), and complain only if it returns NULL. But the lookup_* functions do not actually look at the on-disk object database at all. They return an entry from the in-memory object hash if present (and if it matches the requested type), and otherwise auto-create a "struct object" of the requested type. A missing object would hit that latter case: we create a bogus blob struct, walk all of history looking for it, and then exit successfully having produced no output. One reason nobody may have noticed this is that some related cases do still work OK: 1. If we ask for a tree by sha1, then the call to lookup_commit_referecne_gently() would have parsed it, and we would have its true type in the in-memory object hash. 2. If we ask for a name that doesn't exist but isn't a 40-hex sha1, then get_oid() would complain before we even look at the objects at all. We can fix this by replacing the lookup_blob() call with a check of the true type via sha1_object_info(). This is not quite as efficient as we could possibly make this check. We know in most cases that the object was already parsed in the earlier commit lookup, so we could call lookup_object(), which does auto-create, and check the resulting struct's type (or NULL). However it's not worth the fragility nor code complexity to save a single object lookup. The new tests cover this case, as well as that of a tree-by-sha1 (which does work as described above, but was not explicitly tested). Noticed-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Jeff King <peff@peff.net> Acked-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-23Merge branch 'dk/describe-all-output-fix'Libravatar Junio C Hamano1-2/+5
An old regression in "git describe --all $annotated_tag^0" has been fixed. * dk/describe-all-output-fix: describe: prepend "tags/" when describing tags with embedded name
2018-01-16describe: use strbuf_add_unique_abbrev() for adding short hashesLibravatar René Scharfe1-1/+1
Call strbuf_add_unique_abbrev() to add an abbreviated hash to a strbuf instead of taking a detour through find_unique_abbrev() and its static buffer. This is shorter and a bit more efficient. Patch generated by Coccinelle (and contrib/coccinelle/strbuf.cocci). Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-12-28Merge branch 'sb/describe-blob'Libravatar Junio C Hamano1-27/+94
"git describe" was taught to dig trees deeper to find a <commit-ish>:<path> that refers to a given blob object. * sb/describe-blob: builtin/describe.c: describe a blob builtin/describe.c: factor out describe_commit builtin/describe.c: print debug statements earlier builtin/describe.c: rename `oid` to avoid variable shadowing revision.h: introduce blob/tree walking in order of the commits list-objects.c: factor out traverse_trees_and_blobs t6120: fix typo in test name
2017-12-27describe: prepend "tags/" when describing tags with embedded nameLibravatar Daniel Knittl-Frank1-2/+5
The man page of the "git describe" command explains the expected output when using the --all option, i.e. the full reference path is shown, including heads/ or tags/ prefix. When 212945d4a85dfa172ea55ec73b1d830ef2d8582f ("Teach git-describe to verify annotated tag names before output") made Git favor the embedded name of annotated tags, it accidentally changed the output format when the --all flag is given, only printing the tag's name without the prefix. Check if --all was specified and re-add the "tags/" prefix for this special case to fix the regresssion. Signed-off-by: Daniel Knittl-Frank <knittl89+git@googlemail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-12-19builtin/describe.c: describe a blobLibravatar Stefan Beller1-5/+57
Sometimes users are given a hash of an object and they want to identify it further (ex.: Use verify-pack to find the largest blobs, but what are these? or [1]) When describing commits, we try to anchor them to tags or refs, as these are conceptually on a higher level than the commit. And if there is no ref or tag that matches exactly, we're out of luck. So we employ a heuristic to make up a name for the commit. These names are ambiguous, there might be different tags or refs to anchor to, and there might be different path in the DAG to travel to arrive at the commit precisely. When describing a blob, we want to describe the blob from a higher layer as well, which is a tuple of (commit, deep/path) as the tree objects involved are rather uninteresting. The same blob can be referenced by multiple commits, so how we decide which commit to use? This patch implements a rather naive approach on this: As there are no back pointers from blobs to commits in which the blob occurs, we'll start walking from any tips available, listing the blobs in-order of the commit and once we found the blob, we'll take the first commit that listed the blob. For example git describe --tags v0.99:Makefile conversion-901-g7672db20c2:Makefile tells us the Makefile as it was in v0.99 was introduced in commit 7672db20. The walking is performed in reverse order to show the introduction of a blob rather than its last occurrence. [1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-16builtin/describe.c: factor out describe_commitLibravatar Stefan Beller1-26/+37
Factor out describing commits into its own function `describe_commit`, which will put any output to stdout into a strbuf, to be printed afterwards. As the next patch will teach Git to describe blobs using a commit and path, this refactor will make it easy to reuse the code describing commits. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-16builtin/describe.c: print debug statements earlierLibravatar Stefan Beller1-1/+4
When debugging, print the received argument at the start of the function instead of in the middle. This ensures that the received argument is printed in all code paths, and also allows a subsequent refactoring to not need to move the "arg" parameter. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-16builtin/describe.c: rename `oid` to avoid variable shadowingLibravatar Stefan Beller1-4/+4
The function `describe` has already a variable named `oid` declared at the beginning of the function for an object id. Do not shadow that variable with a pointer to an object id. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-06Merge branch 'bc/object-id'Libravatar Junio C Hamano1-1/+1
Conversion from uchar[20] to struct object_id continues. * bc/object-id: (25 commits) refs/files-backend: convert static functions to object_id refs: convert read_raw_ref backends to struct object_id refs: convert peel_object to struct object_id refs: convert resolve_ref_unsafe to struct object_id worktree: convert struct worktree to object_id refs: convert resolve_gitlink_ref to struct object_id Convert remaining callers of resolve_gitlink_ref to object_id sha1_file: convert index_path and index_fd to struct object_id refs: convert reflog_expire parameter to struct object_id refs: convert read_ref_at to struct object_id refs: convert peel_ref to struct object_id builtin/pack-objects: convert to struct object_id pack-bitmap: convert traverse_bitmap_commit_list to object_id refs: convert dwim_log to struct object_id builtin/reflog: convert remaining unsigned char uses to object_id refs: convert dwim_ref and expand_ref to struct object_id refs: convert read_ref and read_ref_full to object_id refs: convert resolve_refdup and refs_resolve_refdup to struct object_id Convert check_connected to use struct object_id refs: update ref transactions to use struct object_id ...
2017-11-06Merge branch 'jc/no-cmd-as-subroutine'Libravatar Junio C Hamano1-4/+11
Calling cmd_foo() as if it is a general purpose helper function is a no-no. Correct two instances of such to set an example. * jc/no-cmd-as-subroutine: merge-ours: do not use cmd_*() as a subroutine describe: do not use cmd_*() as a subroutine