"John Cai via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> Changes since v4:
>
> * removed superfluous test
An alternative would have been to point with the ref some non-tree
object like a blob, but as the outcome should be the same as missing
case (from the code --- which is not exactly kosher), it should be
OK.
if (repo_get_oid_treeish(the_repository,
default_attr_source_tree_object_name,
attr_source) && !ignore_bad_attr_tree)
die(_("bad --attr-source or GIT_ATTR_SOURCE"));
OOPS! Sorry for not noticing earlier, but repo_get_oid_treeish()
does *NOT* error out when the discovered object is not a treeish, as
the suggested object type is merely supplied for disambiguation
purposes (e.g., with objects 012345 that is a tree and 012346 that
is a blob, you can still ask for treeish "01234" but if you ask for
an object "01234" it will fail).
So, the alternative test would have caught this bug, no? Instead of
silently treating the non-treeish as an empty tree, we would have
died much later when the object supposedly a tree-ish turns out to
be a blob, or something?
