On Thu, Feb 21, 2013 at 06:24:03PM +0400, Sergey Segeev wrote:
> Some time we need to get valid commit without a ref but with proper
> tree-ish, now we can't do that.
>
> This patch allow upload-archive's to use reachability checking
> rather than checking that is a ref. This means a remote client can
> fetch a tip of any valid sha1 or tree-ish.
That sounds like a good goal, but...
> @@ -252,22 +260,16 @@ static void parse_treeish_arg(const char **argv,
> const struct commit *commit;
> unsigned char sha1[20];
>
> - /* Remotes are only allowed to fetch actual refs */
> - if (remote) {
> - char *ref = NULL;
> - const char *colon = strchr(name, ':');
> - int refnamelen = colon ? colon - name : strlen(name);
> -
> - if (!dwim_ref(name, refnamelen, sha1, &ref))
> - die("no such ref: %.*s", refnamelen, name);
> - free(ref);
> - }
The point of this was to allow "commit:path" syntax, and check that
commit pointed to a ref. The natural extension would be to also check
that the commit part is reachable.
I think it is also not sufficient to just check whether the left-hand
side of the colon is a reachable commit. You would also want to handle
non-commits which are directly pointed-to by a ref or its tag (e.g.,
think of a tag pointing directly to a tree, like the v2.6.11 tag in the
linux repo).
Your check...
> commit = lookup_commit_reference_gently(sha1, 1);
> if (commit) {
> +
> + /* Remotes are only allowed to fetch actual objects */
> + if (remote && !for_each_ref(check_reachable, (void *)commit))
> + die("Not a valid object name");
> +
> commit_sha1 = commit->object.sha1;
> archive_time = commit->date;
> } else {
...will do nothing if we do not have a commit reference (e.g., an arbitrary
sha1, or commit:path syntax). We follow the "else" of this branch, and
allow arbitrary sha1's to be fetched (like "unreachable_sha1:subdir").
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html