Re: [PATCH] get_sha1: support relative path ":path" syntax

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 15, 2010, 17:29 UTC
Message-ID: <7v4obimr5l.fsf@alter.siamese.dyndns.org>
In-Reply-To: <AANLkTi=aVyOiRkE-+y8DDwcBQK36bVFrAX5PFw3TjCey@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

Show 8 quoted lines

> 2010/11/15 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
>> +               die("Relative path syntax is not supported in this command.\n"
>> +                   "Please report to git@vger.kernel.org.");
>
> Might it save us a lot of debugging hassle later if we report what
> "this command" is? E.g., if the user is using some internal tool that
> happens to dispatch to a command that doesn't support this, it could
> help us to know what command is being used?

In the absence of programming errors, should all git command support <tree>:./<path> syntax to name an object, or are there cases where the relative does not make any sense?

What I am trying to get at is that if this is diagnosing a user error, or if this is showing that the mechanism to implement the relative path is unnecessarily hard for the programmers to misuse.

For example, if "git show HEAD:./Makefile" in a bare repository is a user error, it is not "not supported in this command" but "not supported in this situation (more specifically, in a bare repository)", so the first line is wrong, and more importantly, if that is a user error, there is nothing to report to the list.

If on the other hand, a command that ought to allow use of relative path didn't set up necessary startup_info, this is diagnosing a programming error. But if that is the case, shouldn't we be able to do much better to avoid such mistakes in the first place than triggering a BUG() here when the user happens to try using this particular feature? In other words, even when this "feature" isn't used in a particular invocation of a command, a command that is capable of supporting the feature should always be setting up startup_info, perhaps by the time its cmd_foo() is called, no? Shouldn't we be putting a BUG() somewhere higher and more visible in the callchain to help catching such bugs?