From: Junio C Hamano <gitster@pobox.com>
To: "John Cai via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
Jonathan Tan <jonathantanmy@google.com>,
Eric Sunshine <sunshine@sunshineco.com>,
John Cai <johncai86@gmail.com>
Subject: Re: [PATCH v5 0/2] attr: add attr.tree config
Date: Fri, 13 Oct 2023 11:52:07 -0700 [thread overview]
Message-ID: <xmqqmswmz76w.fsf@gitster.g> (raw)
In-Reply-To: <pull.1577.v5.git.git.1697218770.gitgitgadget@gmail.com> (John Cai via GitGitGadget's message of "Fri, 13 Oct 2023 17:39:28 +0000")
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> 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?
next prev parent reply other threads:[~2023-10-13 18:52 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-20 14:00 [PATCH] attr: attr.allowInvalidSource config to allow invalid revision John Cai via GitGitGadget
2023-09-20 16:06 ` Junio C Hamano
2023-09-21 4:15 ` Jeff King
2023-09-21 8:52 ` Junio C Hamano
2023-09-21 21:40 ` Jeff King
2023-09-26 18:27 ` John Cai
2023-09-26 18:30 ` John Cai
2023-09-26 18:23 ` John Cai
2023-10-04 18:18 ` [PATCH v2 0/2] attr: add attr.tree and attr.allowInvalidSource configs John Cai via GitGitGadget
2023-10-04 18:18 ` [PATCH v2 1/2] attr: add attr.tree for setting the treeish to read attributes from John Cai via GitGitGadget
2023-10-04 19:58 ` Junio C Hamano
2023-10-05 17:07 ` Jeff King
2023-10-05 19:46 ` John Cai
2023-10-04 23:45 ` Junio C Hamano
2023-10-06 17:20 ` Jonathan Tan
2023-10-04 18:18 ` [PATCH v2 2/2] attr: add attr.allowInvalidSource config to allow invalid revision John Cai via GitGitGadget
2023-10-10 19:49 ` [PATCH v3 0/2] attr: add attr.tree config John Cai via GitGitGadget
2023-10-10 19:49 ` [PATCH v3 1/2] attr: read attributes from HEAD when bare repo John Cai via GitGitGadget
2023-10-10 19:58 ` Eric Sunshine
2023-10-10 19:49 ` [PATCH v3 2/2] attr: add attr.tree for setting the treeish to read attributes from John Cai via GitGitGadget
2023-10-10 22:14 ` Junio C Hamano
2023-10-11 2:19 ` John Cai
2023-10-11 17:13 ` [PATCH v4 0/2] attr: add attr.tree config John Cai via GitGitGadget
2023-10-11 17:13 ` [PATCH v4 1/2] attr: read attributes from HEAD when bare repo John Cai via GitGitGadget
2023-10-11 17:13 ` [PATCH v4 2/2] attr: add attr.tree for setting the treeish to read attributes from John Cai via GitGitGadget
2023-10-11 22:09 ` [PATCH v4 0/2] attr: add attr.tree config Junio C Hamano
2023-10-13 15:30 ` John Cai
2023-10-13 17:39 ` [PATCH v5 " John Cai via GitGitGadget
2023-10-13 17:39 ` [PATCH v5 1/2] attr: read attributes from HEAD when bare repo John Cai via GitGitGadget
2023-10-13 17:39 ` [PATCH v5 2/2] attr: add attr.tree for setting the treeish to read attributes from John Cai via GitGitGadget
2023-10-13 18:52 ` Junio C Hamano [this message]
2023-10-13 20:31 ` [PATCH v5 0/2] attr: add attr.tree config Junio C Hamano
2023-10-13 20:47 ` Junio C Hamano
2023-10-19 15:43 ` John Cai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqmswmz76w.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=johncai86@gmail.com \
--cc=jonathantanmy@google.com \
--cc=peff@peff.net \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).