Re: What's cooking in git.git (Jun 2016, #05; Thu, 16)

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 28, 2016, 17:44 UTC
Message-ID: <xmqq1t3h9pjb.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <5772421F.3030100@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> (I suppose that `REFNAME_ALLOW_ONELEVEL` was
> meant for checking partial reference names, for example to vet "foo"
> that the caller wants to pass to `git branch`, which automatically turns
> it into `refs/heads/foo`.)

Correct.

> In summary, `check_refname_format()` is in some ways less strict than
> `refname_is_safe()`. For example, it allows reference names like
> `git-p4-tmp/6` or even `git-p4-tmp`.

Again, correct.

Show 25 quoted lines

> I don't know what I was thinking. Long ago, when I refactored and
> documented check_refname_format(), I realized that the checks are
> surprisingly lax and that the `REFNAME_ALLOW_ONELEVEL` option is
> misleading. But I was new to the project and wasn't brave or energetic
> enough to do something about it. Meanwhile I guess I forgot that it
> never got fixed. Commit
>
> d0f810f0 2014-09-03 refs.c: allow listing and deleting badly named refs
>
> , which introduced `refname_is_safe()`, perhaps muddled the situation by
> adding a "fallback" check that is not strictly laxer than the main check.
>
> Where does that leave us?
>
> * It is currently possible to create and delete references with
>   names that are neither within `refs/` nor ALL_CAPS (though not
>   remotely).
>
> * Such references do not participate in reachability checks, so
>   they have to be considered semi-broken.
>
> * Users could create chaos (though not remotely) by creating a
>   loose "reference" whose name coincides with that of another
>   file under `$GIT_DIR`.
>   (`git update-ref objects/info/alternates HEAD` anyone?)

All correct.

> * `git-p4` is apparently the only code within the Git project that
>   was making use of this questionable freedom.

True.

> * Presumably there is user code in the wild that creates and uses
>   such references.

I doubt it, if they value their data. .git/p4-tmp or .git/p4-tmp/6 are not just "partly broken"--they are just as transient as things like MERGE_HEAD in that the objects pointed by them are subject to be pruned.

Show 10 quoted lines

> I think we need to get this under control, but given that we've allowed
> such references (albeit partly broken) for a long time, we probably need
> to provide an escape hatch to aid the transition. I'm skeptical that the
> mh/split-under-lock patch series is the best place to start such a
> campaign. On the other hand, I don't want that patch series to open up
> any new avenues to creating references that escape `$GIT_DIR`.
>
> Let me think for a bit about the next step. Input is welcome.
>
> In any case, I think that Lars's patch to `git-p4` is a good thing.

Thanks.