Re: [PATCH v3 0/2] check-attr: add support to work with revisions

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Karthik

On 16/12/2022 09:35, Karthik Nayak wrote:
> v1: https://lore.kernel.org/git/20221206103736.53909-1-karthik.188@gmail.com/
> v2: https://lore.kernel.org/git/CAOLa=ZSsFGBw3ta1jWN8cmUch2ca=zTEjp1xMA6Linafx9W53g@mail.gmail.com/T/#t
> 
> Given a pathname, git-check-attr(1) will list the attributes which apply to that
> pathname by reading all relevant gitattributes files. Currently there is no way
> to specify a revision to read the gitattributes from.
> 
> This is specifically useful in bare repositories wherein the gitattributes are
> only present in the git working tree but not available directly on the
> filesystem.

I was thinking about this and wondering if the problem is really that 
bare repositories ignore attributes because they don't have a working 
copy. If that's the case then we should perhaps be looking to fix that 
so that all git commands such as diff as log benefit rather than just 
adding a flag to check-attr. A simple solution would be to read the 
attributes from HEAD in a bare repository in the same way that we 
fallback to the index if there are no attributes in the working copy for 
non-bare repositories.

Best Wishes

Phillip

> This series aims to add a new flag `-r|--revisions` to git-check-attr(1) which
> allows us to read gitattributes from the specified revision.
> 
> Changes since version 2:
> - Changes to the commit message [1/2] to use more specific terms and to
>    be more descriptive.
> - Moved the flag's position in the documentation to be before the unbound
>    list of non-options.
> 
> Range-diff against v2:
> 
> 1:  2e71cbbddd < -:  ---------- Git 2.39-rc2
> -:  ---------- > 1:  57e2c6ebbe Start the 2.40 cycle
> 2:  898041f243 = 2:  c386de2d42 t0003: move setup for `--all` into new block
> 3:  12a72e09e0 ! 3:  b93a68b0c9 attr: add flag `-r|--revisions` to work with revisions
>      @@ Metadata
>        ## Commit message ##
>           attr: add flag `-r|--revisions` to work with revisions
>       
>      -    Git check-attr currently doesn't check the git worktree, it either
>      -    checks the index or the files directly. This means we cannot check the
>      -    attributes for a file against a certain revision.
>      +    The contents of the .gitattributes files may evolve over time, but "git
>      +    check-attr" always checks attributes against them in the working tree
>      +    and/or in the index. It may be beneficial to optionally allow the users
>      +    to check attributes against paths from older commits.
>       
>      -    Add a new flag `--revision`/`-r` which will allow it work with
>      -    revisions. This command will now, instead of checking the files/index,
>      -    try and receive the blob for the given attribute file against the
>      -    provided revision. The flag overrides checking against the index and
>      -    filesystem and also works with bare repositories.
>      +    Add a new flag `--revision`/`-r` which will allow users to check the
>      +    attributes against a tree-ish revision. When the user uses this flag, we
>      +    go through the stack of .gitattributes files but instead of checking the
>      +    current working tree and/or in the index, we check the blobs from the
>      +    provided tree-ish object. This allows the command to also be used in
>      +    bare repositories.
>      +
>      +    Since we use a tree-ish object, the user can pass "-r HEAD:subdirectory"
>      +    and all the attributes will be looked up as if subdirectory was the root
>      +    directory of the repository.
>       
>           We cannot use the `<rev>:<path>` syntax like the one used in `git show`
>           because any non-flag parameter before `--` is treated as an attribute
>           and any parameter after `--` is treated as a pathname.
>       
>      -    This involves creating a new function `read_attr_from_blob`, which given
>      -    the path reads the blob for the path against the provided revision and
>      +    The change involves creating a new function `read_attr_from_blob`, which
>      +    given the path reads the blob for the path against the provided revision and
>           parses the attributes line by line. This function is plugged into
>      -    `read_attr()` function wherein we go through the different attributes.
>      +    `read_attr()` function wherein we go through the stack of attributes
>      +    files.
>       
>           Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>           Co-authored-by: toon@iotcl.com
>      @@ Documentation/git-check-attr.txt: git-check-attr - Display gitattributes informa
>        [verse]
>       -'git check-attr' [-a | --all | <attr>...] [--] <pathname>...
>       -'git check-attr' --stdin [-z] [-a | --all | <attr>...]
>      -+'git check-attr' [-a | --all | <attr>...] [-r <revision>] [--] <pathname>...
>      -+'git check-attr' --stdin [-z] [-a | --all | <attr>...] [-r <revision>]
>      ++'git check-attr' [-r <revision>] [-a | --all | <attr>...] [--] <pathname>...
>      ++'git check-attr' --stdin [-z] [-r <revision>] [-a | --all | <attr>...]
>        
>        DESCRIPTION
>        -----------
>      @@ Documentation/git-check-attr.txt: OPTIONS
>        
>       +--r <revision>::
>       +--revision=<revision>::
>      -+	Check attributes against the specified revision.
>      ++	Check attributes against the specified tree-ish revision. All the
>      ++	attributes will be checked against the provided revision. Paths provided
>      ++	as part of the revision will be treated as the root directory.
>       +
>        \--::
>        	Interpret all preceding arguments as attributes and all following
>      @@ builtin/check-attr.c
>        static const char * const check_attr_usage[] = {
>       -N_("git check-attr [-a | --all | <attr>...] [--] <pathname>..."),
>       -N_("git check-attr --stdin [-z] [-a | --all | <attr>...]"),
>      -+N_("git check-attr [-a | --all | <attr>...] [-r <revision>] [--] <pathname>..."),
>      -+N_("git check-attr --stdin [-z] [-a | --all | <attr>...] [-r <revision>]"),
>      ++N_("git check-attr [-r <revision>] [-a | --all | <attr>...] [--] <pathname>..."),
>      ++N_("git check-attr --stdin [-z] [-r <revision>] [-a | --all | <attr>...]"),
>        NULL
>        };
>        
> 
> 
> Karthik Nayak (2):
>    t0003: move setup for `--all` into new block
>    attr: add flag `-r|--revisions` to work with revisions
> 
>   Documentation/git-check-attr.txt |  10 +++-
>   archive.c                        |   2 +-
>   attr.c                           | 100 ++++++++++++++++++++++---------
>   attr.h                           |   7 ++-
>   builtin/check-attr.c             |  33 ++++++----
>   builtin/pack-objects.c           |   2 +-
>   convert.c                        |   2 +-
>   ll-merge.c                       |   4 +-
>   pathspec.c                       |   2 +-
>   t/t0003-attributes.sh            |  71 +++++++++++++++++++++-
>   userdiff.c                       |   2 +-
>   ws.c                             |   2 +-
>   12 files changed, 182 insertions(+), 55 deletions(-)
>