Re: [PATCH 1/5] refs: Introduce pseudoref and per-worktree ref concepts

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 28, 2015, 17:13 UTC
Message-ID: <CAPig+cRgp1F3S9DXWGABmrSDZM1As-R6FB7oBQbkoDLy-v6_PA@mail.gmail.com>
In-Reply-To: <1438027720-23074-2-git-send-email-dturner@twopensource.com>

On Mon, Jul 27, 2015 at 4:08 PM, David Turner <dturner@twopensource.com> wrote:

> refs: Introduce pseudoref and per-worktree ref concepts
>
> Add glossary entries for both concepts.

Based upon the above, I thought this was going to be a documentation-only patch and was mildly surprised to find that it also changed code. Perhaps:

    Describe these concepts in the glossary and introduce
    is_per_worktree_ref() to distinguish such files.

or something. Of course the "and" in there suggests that this might be better off split into two patches...

More below.

Show 24 quoted lines

> Pseudorefs and per-worktree refs do not yet have special handling,
> because the files refs backend already handles them correctly.  Later,
> we will make the LMDB backend call out to the files backend to handle
> per-worktree refs.
>
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
> diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
> index ab18f4b..67952f3 100644
> --- a/Documentation/glossary-content.txt
> +++ b/Documentation/glossary-content.txt
> @@ -411,6 +411,27 @@ exclude;;
>         core Git. Porcelains expose more of a <<def_SCM,SCM>>
>         interface than the <<def_plumbing,plumbing>>.
>
> +[[def_per_worktree_ref]]per-worktree ref::
> +       Refs that are per-<<def_worktree,worktree>>, rather than
> +       global.  This is presently only <<def_HEAD,HEAD>>, but might
> +       later include other unusual refs.
> +
> +[[def_pseudoref]]pseudoref::
> +       Pseudorefs are a class of files under `$GIT_DIR` which behave
> +       like refs for the purposes of rev-parse, but which are treated
> +       specially by git.  Psuedorefs both have names are all-caps,

s/names/& that/

> +       and always start with a line consisting of a
> +       <<def_sha1,SHA-1>> followed by whitespace.  So, HEAD is not a
> +       pseudoref, because it is sometimes a symbolic ref.  They might
> +       optionally some additional data.  `MERGE_HEAD` and

s/optionally/& contain/

Show 41 quoted lines

> +       `CHERRY_PICK_HEAD` are examples.  Unlike
> +       <<def_per_worktree_ref,per-worktree refs>>, these files cannot
> +       be symbolic refs, and never have reflogs.  They also cannot be
> +       updated through the normal ref update machinery.  Instead,
> +       they are updated by directly writing to the files.  However,
> +       they can be read as if they were refs, so `git rev-parse
> +       MERGE_HEAD` will work.
> +
>  [[def_pull]]pull::
>         Pulling a <<def_branch,branch>> means to <<def_fetch,fetch>> it and
>         <<def_merge,merge>> it.  See also linkgit:git-pull[1].
> diff --git a/refs.c b/refs.c
> index 0b96ece..0d10b7b 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2848,6 +2848,29 @@ static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf *err)
>         return 0;
>  }
>
> +int is_per_worktree_ref(const char *refname)
> +{
> +       return !strcmp(refname, "HEAD");
> +}
> +
> +static int is_pseudoref(const char *refname)
> +{
> +       const char *c;
> +
> +       if (strchr(refname, '/'))
> +               return 0;
> +
> +       if (is_per_worktree_ref(refname))
> +               return 0;
> +
> +       for (c = refname; *c; ++c) {
> +               if (!isupper(*c) && *c != '-' && *c != '_')
> +                       return 0;
> +       }
> +
> +       return 1;
> +}

This static function doesn't seem to have any callers, thus seems out of place in this patch.

> +
>  int delete_ref(const char *refname, const unsigned char *old_sha1,
>                unsigned int flags)
>  {