From: Phillip Wood Date: Sat, 17 Dec 2022 10:53:43 GMT Subject: Re: [PATCH v3 0/2] check-attr: add support to work with revisions Message-ID: <071e44b5-607e-0db5-af90-85daae81f611@dunelm.org.uk> In-Reply-To: <20221216093552.3171319-1-karthik.188@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 `:` 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 > Co-authored-by: toon@iotcl.com > @@ Documentation/git-check-attr.txt: git-check-attr - Display gitattributes informa > [verse] > -'git check-attr' [-a | --all | ...] [--] ... > -'git check-attr' --stdin [-z] [-a | --all | ...] > -+'git check-attr' [-a | --all | ...] [-r ] [--] ... > -+'git check-attr' --stdin [-z] [-a | --all | ...] [-r ] > ++'git check-attr' [-r ] [-a | --all | ...] [--] ... > ++'git check-attr' --stdin [-z] [-r ] [-a | --all | ...] > > DESCRIPTION > ----------- > @@ Documentation/git-check-attr.txt: OPTIONS > > +--r :: > +--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 | ...] [--] ..."), > -N_("git check-attr --stdin [-z] [-a | --all | ...]"), > -+N_("git check-attr [-a | --all | ...] [-r ] [--] ..."), > -+N_("git check-attr --stdin [-z] [-a | --all | ...] [-r ]"), > ++N_("git check-attr [-r ] [-a | --all | ...] [--] ..."), > ++N_("git check-attr --stdin [-z] [-r ] [-a | --all | ...]"), > 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(-) >