Bumping for reviews, before I re-roll with the fix for `make hdr-check` - Karthik On Mon, Jan 2, 2023 at 12:04 PM Karthik Nayak <karthik.188@xxxxxxxxx> wrote: > > v1: https://lore.kernel.org/git/20221206103736.53909-1-karthik.188@xxxxxxxxx/ > v2: https://lore.kernel.org/git/CAOLa=ZSsFGBw3ta1jWN8cmUch2ca=zTEjp1xMA6Linafx9W53g@xxxxxxxxxxxxxx/T/#t > v3: https://lore.kernel.org/git/20221216093552.3171319-1-karthik.188@xxxxxxxxx/ > v4: https://lore.kernel.org/git/cover.1671630304.git.karthik.188@xxxxxxxxx > > 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 tree-ish 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. > > This series aims to add a new flag `--source` to git-check-attr(1) which > allows us to read gitattributes from the specified tree-ish. > > Changes since version 4: > - Changed the flag from `--revision` to `--source` > - Removed uneeded header imports > - Using a pre-initialized variable instead of malloc for the tree_oid > - Using `die()` instead of `error()` for bad tree-ish provided > > Range-diff against v4: > > 1: 6224754179 = 1: 6224754179 t0003: move setup for `--all` into new block > 2: a161dbdf8b ! 2: d835d989ad attr: add flag `--revision` to work with revisions > @@ Metadata > Author: Karthik Nayak <karthik.188@xxxxxxxxx> > > ## Commit message ## > - attr: add flag `--revision` to work with revisions > + attr: add flag `--source` to work with tree-ish > > 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 taken from a commit other than HEAD against paths. > > - Add a new flag `--revision` which will allow users to check the > + Add a new flag `--source` which will allow users to check the > attributes against a commit (actually any tree-ish would do). 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 "--revision > + Since we use a tree-ish object, the user can pass "--source > HEAD:subdirectory" and all the attributes will be looked up as if > subdirectory was the root directory of the repository. > > - We cannot simply use the `<rev>:<path>` syntax without the `--revision` > + We cannot simply use the `<rev>:<path>` syntax without the `--source` > flag, similar to how it is used in `git show` because any non-flag > parameter before `--` is treated as an attribute and any parameter after > `--` is treated as a pathname. > > 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 > + given the path reads the blob for the path against the provided source and > parses the attributes line by line. This function is plugged into > `read_attr()` function wherein we go through the stack of attributes > files. > @@ 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' [--revision <revision>] [-a | --all | <attr>...] [--] <pathname>... > -+'git check-attr' --stdin [-z] [--revision <revision>] [-a | --all | <attr>...] > ++'git check-attr' [--source <tree>] [-a | --all | <attr>...] [--] <pathname>... > ++'git check-attr' --stdin [-z] [--source <tree>] [-a | --all | <attr>...] > > DESCRIPTION > ----------- > @@ Documentation/git-check-attr.txt: OPTIONS > If `--stdin` is also given, input paths are separated > with a NUL character instead of a linefeed character. > > -+--revision=<revision>:: > -+ Check attributes against the specified commit. All the attributes will > -+ be checked against the provided revision. Paths provided as part of the > -+ revision will be treated as the root directory. > ++--source=<tree>:: > ++ Check attributes against the specified tree-ish. Paths provided as part > ++ of the revision will be treated as the root directory. It is common to > ++ specify the source tree by naming a commit, branch or tag associated > ++ with it. > + > \--:: > Interpret all preceding arguments as attributes and all following > @@ archive.c: static const struct attr_check *get_archive_attrs(struct index_state > > ## attr.c ## > @@ > - #include "exec-cmd.h" > - #include "attr.h" > #include "dir.h" > -+#include "strbuf.h" > -+#include "tree-walk.h" > #include "utf8.h" > #include "quote.h" > +#include "revision.h" > @@ attr.c: void git_check_attr(struct index_state *istate, > const char *name = check->all_attrs[i].attr->name; > > ## attr.h ## > -@@ > - #ifndef ATTR_H > - #define ATTR_H > - > -+#include "hash.h" > -+ > - /** > - * gitattributes mechanism gives a uniform way to associate various attributes > - * to set of paths. > @@ attr.h: void attr_check_free(struct attr_check *check); > const char *git_attr_name(const struct git_attr *); > > @@ attr.h: void attr_check_free(struct attr_check *check); > enum git_attr_direction { > > ## builtin/check-attr.c ## > -@@ > -+#include "repository.h" > - #define USE_THE_INDEX_VARIABLE > - #include "builtin.h" > - #include "cache.h" > @@ > static int all_attrs; > static int cached_attrs; > static int stdin_paths; > -+static char *revision; > ++static char *source; > 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 [--revision <revision>] [-a | --all | <attr>...] [--] <pathname>..."), > -+N_("git check-attr --stdin [-z] [--revision <revision>] [-a | --all | <attr>...]"), > ++N_("git check-attr [--source <tree>] [-a | --all | <attr>...] [--] <pathname>..."), > ++N_("git check-attr --stdin [-z] [--source <tree>] [-a | --all | <attr>...]"), > NULL > }; > > @@ builtin/check-attr.c: static const struct option check_attr_options[] = { > OPT_BOOL(0 , "stdin", &stdin_paths, N_("read file names from stdin")), > OPT_BOOL('z', NULL, &nul_term_line, > N_("terminate input and output records by a NUL character")), > -+ OPT_STRING(0, "revision", &revision, N_("revision"), N_("check attributes at this revision")), > ++ OPT_STRING(0, "source", &source, N_("<tree-ish>"), N_("which tree-ish to check attributes at")), > OPT_END() > }; > > @@ builtin/check-attr.c: static NORETURN void error_with_usage(const char *msg) > { > struct attr_check *check; > + struct object_id *tree_oid = NULL; > ++ struct object_id initialized_oid; > int cnt, i, doubledash, filei; > > if (!is_bare_repository()) > @@ builtin/check-attr.c: int cmd_check_attr(int argc, const char **argv, const char > } > } > > -+ if (revision) { > -+ tree_oid = xmalloc(sizeof(struct object_id)); > -+ > -+ if (repo_get_oid_tree(the_repository, revision, tree_oid)) > -+ error("%s: not a valid revision", revision); > ++ if (source) { > ++ if (repo_get_oid_tree(the_repository, source, &initialized_oid)) > ++ die("%s: not a valid tree-ish source", source); > ++ tree_oid = &initialized_oid; > + } > + > if (stdin_paths) > @@ t/t0003-attributes.sh: attr_check_quote () { > test_cmp expect actual > +} > + > -+attr_check_revision () { > -+ path="$1" expect="$2" revision="$3" git_opts="$4" && > ++attr_check_source () { > ++ path="$1" expect="$2" source="$3" git_opts="$4" && > > -+ git $git_opts check-attr --revision $revision test -- "$path" >actual 2>err && > ++ git $git_opts check-attr --source $source test -- "$path" >actual 2>err && > + echo "$path: test: $expect" >expect && > + test_cmp expect actual > ++ test_must_be_empty err > } > > test_expect_success 'open-quoted pathname' ' > @@ t/t0003-attributes.sh: test_expect_success 'setup' ' > ' > > +test_expect_success 'setup branches' ' > -+ ( > -+ echo "f test=f" && > -+ echo "a/i test=n" > -+ ) | git hash-object -w --stdin >id && > -+ git update-index --add --cacheinfo 100644,$(cat id),foo/bar/.gitattributes && > -+ git write-tree >id && > -+ tree_id=$(cat id) && > -+ git commit-tree $tree_id -m "random commit message" >id && > -+ commit_id=$(cat id) && > -+ git update-ref refs/heads/branch1 $commit_id && > ++ mkdir -p foo/bar && > ++ test_commit --printf "add .gitattributes" foo/bar/.gitattribute \ > ++ "f test=f\na/i test=n\n" tag-1 && > + > -+ ( > -+ echo "g test=g" && > -+ echo "a/i test=m" > -+ ) | git hash-object -w --stdin >id && > -+ git update-index --add --cacheinfo 100644,$(cat id),foo/bar/.gitattributes && > -+ git write-tree >id && > -+ tree_id=$(cat id) && > -+ git commit-tree $tree_id -m "random commit message" >id && > -+ commit_id=$(cat id) && > -+ git update-ref refs/heads/branch2 $commit_id > ++ mkdir -p foo/bar && > ++ test_commit --printf "add .gitattributes" foo/bar/.gitattribute \ > ++ "g test=g\na/i test=m\n" tag-2 > +' > + > test_expect_success 'command line checks' ' > @@ t/t0003-attributes.sh: test_expect_success 'setup' ' > test_must_fail git check-attr test && > test_must_fail git check-attr test -- && > test_must_fail git check-attr -- f && > -+ test_must_fail git check-attr --revision && > -+ test_must_fail git check-attr --revision not-a-valid-ref && > ++ test_must_fail git check-attr --source && > ++ test_must_fail git check-attr --source not-a-valid-ref && > echo "f" | test_must_fail git check-attr --stdin && > echo "f" | test_must_fail git check-attr --stdin -- f && > echo "f" | test_must_fail git check-attr --stdin test -- f && > @@ t/t0003-attributes.sh: test_expect_success 'using --git-dir and --work-tree' ' > ) > ' > > -+test_expect_success 'using --revision' ' > -+ attr_check_revision foo/bar/f f branch1 && > -+ attr_check_revision foo/bar/a/i n branch1 && > -+ attr_check_revision foo/bar/f unspecified branch2 && > -+ attr_check_revision foo/bar/a/i m branch2 && > -+ attr_check_revision foo/bar/g g branch2 && > -+ attr_check_revision foo/bar/g unspecified branch1 > ++test_expect_success 'using --source' ' > ++ attr_check_source foo/bar/f f tag-1 && > ++ attr_check_source foo/bar/a/i n tag-1 && > ++ attr_check_source foo/bar/f unspecified tag-2 && > ++ attr_check_source foo/bar/a/i m tag-2 && > ++ attr_check_source foo/bar/g g tag-2 && > ++ attr_check_source foo/bar/g unspecified tag-1 > +' > + > test_expect_success 'setup bare' ' > @@ t/t0003-attributes.sh: test_expect_success 'bare repository: check that .gitattr > ) > ' > > -+test_expect_success 'bare repository: with --revision' ' > ++test_expect_success 'bare repository: with --source' ' > + ( > + cd bare.git && > -+ ( > -+ echo "f test=f" && > -+ echo "a/i test=a/i" > -+ ) | git hash-object -w --stdin >id && > -+ git update-index --add --cacheinfo 100644 $(cat id) .gitattributes && > -+ git write-tree >id && > -+ tree_id=$(cat id) && > -+ git commit-tree $tree_id -m "random commit message" >id && > -+ commit_id=$(cat id) && > -+ git update-ref refs/heads/master $commit_id && > -+ attr_check_revision f f HEAD && > -+ attr_check_revision a/f f HEAD && > -+ attr_check_revision a/c/f f HEAD && > -+ attr_check_revision a/i a/i HEAD && > -+ attr_check_revision subdir/a/i unspecified HEAD > ++ attr_check_source foo/bar/f f tag-1 && > ++ attr_check_source foo/bar/a/i n tag-1 && > ++ attr_check_source foo/bar/f unspecified tag-2 && > ++ attr_check_source foo/bar/a/i m tag-2 && > ++ attr_check_source foo/bar/g g tag-2 && > ++ attr_check_source foo/bar/g unspecified tag-1 > + ) > +' > + > > > Karthik Nayak (2): > t0003: move setup for `--all` into new block > attr: add flag `--source` to work with tree-ish > > Documentation/git-check-attr.txt | 10 +++- > archive.c | 2 +- > attr.c | 97 +++++++++++++++++++++++--------- > attr.h | 5 +- > builtin/check-attr.c | 35 +++++++----- > builtin/pack-objects.c | 2 +- > convert.c | 2 +- > ll-merge.c | 4 +- > pathspec.c | 2 +- > t/t0003-attributes.sh | 49 +++++++++++++++- > userdiff.c | 2 +- > ws.c | 2 +- > 12 files changed, 157 insertions(+), 55 deletions(-) > > -- > 2.39.0 >
