Re: [PATCH v5 0/2] attr: add attr.tree config

"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:

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?