{"thread":{"id":"58925","subject":"[PATCH v2 1/2] t0003: move setup for `--all` into new block","startedAt":"2022-12-09T21:03:32Z","lastAt":"2022-12-15T20:20:46Z","messageCount":8,"participants":["Karthik Nayak","Junio C Hamano","Phillip Wood"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"468843","messageId":"20221209210321.709156-2-karthik.188@gmail.com","threadId":"58925","inReplyTo":"20221209210321.709156-1-karthik.188@gmail.com","subject":"[PATCH v2 1/2] t0003: move setup for `--all` into new block","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2022-12-09T21:03:20Z","receivedAt":"2022-12-09T21:03:32Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"There is some setup code which is used by multiple tests being setup in\n`attribute test: --all option`. This means when we run \"sh\n./t0003-attributes.sh --run=setup,\" there is a chance of failing\nsince we missed this setup block.\n\nSo to ensure that setups are independent of test logic, move this to a\nnew setup block.\n\nSigned-off-by: Karthik Nayak \nCo-authored-by: toon@iotcl.com\n---\n t/t0003-attributes.sh | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh\nindex f7ee2f2ff0..b3aabb8aa3 100755\n--- a/t/t0003-attributes.sh\n+++ b/t/t0003-attributes.sh\n@@ -203,9 +203,12 @@ test_expect_success 'attribute test: read paths from stdin' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'attribute test: --all option' '\n+test_expect_success 'setup --all option' '\n \tgrep -v unspecified specified-all &&\n-\tsed -e \"s/:.*//\" stdin-all &&\n+\tsed -e \"s/:.*//\" stdin-all\n+'\n+\n+test_expect_success 'attribute test: --all option' '\n \tgit check-attr --stdin --all tmp &&\n \tsort tmp >actual &&\n \ttest_cmp specified-all actual\n-- \n2.38.1\n\n"},{"id":"468844","messageId":"20221209210321.709156-1-karthik.188@gmail.com","threadId":"58925","inReplyTo":null,"subject":"[PATCH v2 0/2] check-attr: add support to work with revisions","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2022-12-09T21:03:19Z","receivedAt":"2022-12-09T21:03:34Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"V1: https://lore.kernel.org/git/20221206103736.53909-1-karthik.188@gmail.com/\n\nGiven a pathname, git-check-attr(1) will list the attributes which apply to that\npathname by reading all relevant gitattributes files. Currently there is no way\nto specify a revision to read the gitattributes from.\n\nThis is specifically useful in bare repositories wherein the gitattributes are\nonly present in the git working tree but not available directly on the\nfilesystem.\n\nThis series aims to add a new flag `-r|--revisions` to git-check-attr(1) which\nallows us to read gitattributes from the specified revision.\n\nChanges since version 1:\n1. Now we parse the userinput revision at the start (builtin/attr.c) and only\npropagate the tree_oid object. This allows us to also exit early in case a wrong\nrevision is provided.\n2. Added missing documentation\n3. Changed the argument positioning\n4. Some code review fixes around the syntax\n\nrange-diff against version 1:\n\n1: 2e71cbbddd = 1: 2e71cbbddd Git 2.39-rc2\n2: 898041f243 = 2: 898041f243 t0003: move setup for `--all` into new block\n3: 36eec9ba1a ! 3: 12a72e09e0 attr: add flag `-r|--revisions` to work with revisions\n @@ Commit message\n Signed-off-by: Karthik Nayak \n Co-authored-by: toon@iotcl.com\n \n + ## Documentation/git-check-attr.txt ##\n +@@ Documentation/git-check-attr.txt: git-check-attr - Display gitattributes information\n + SYNOPSIS\n + --------\n + [verse]\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 + \n + DESCRIPTION\n + -----------\n +@@ Documentation/git-check-attr.txt: OPTIONS\n + \tIf `--stdin` is also given, input paths are separated\n + \twith a NUL character instead of a linefeed character.\n + \n ++--r ::\n ++--revision=::\n ++\tCheck attributes against the specified revision.\n ++\n + \\--::\n + \tInterpret all preceding arguments as attributes and all following\n + \targuments as path names.\n +\n ## archive.c ##\n @@ archive.c: static const struct attr_check *get_archive_attrs(struct index_state *istate,\n \tstatic struct attr_check *check;\n \tif (!check)\n \t\tcheck = attr_check_initl(\"export-ignore\", \"export-subst\", NULL);\n -\tgit_check_attr(istate, path, check);\n -+\tgit_check_attr(istate, path, check, NULL);\n ++\tgit_check_attr(istate, NULL, path, check);\n \treturn check;\n }\n \n @@ attr.c\n #include \"dir.h\"\n +#include \"git-compat-util.h\"\n +#include \"strbuf.h\"\n ++#include \"tree-walk.h\"\n #include \"utf8.h\"\n #include \"quote.h\"\n +#include \"revision.h\"\n @@ attr.c: static struct attr_stack *read_attr_from_file(const char *path, unsigned\n +\tchar *sp;\n \tint lineno = 0;\n \n -+\tif (buf == NULL)\n ++\tif (!buf)\n +\t\treturn NULL;\n +\n +\tCALLOC_ARRAY(res, 1);\n @@ attr.c: static struct attr_stack *read_attr_from_file(const char *path, unsigned\n +\treturn res;\n +}\n +\n -+static struct attr_stack *read_attr_from_blob(const char *path,\n -+\t\t\t\t\t const char *object_name,\n -+\t\t\t\t\t unsigned flags)\n ++static struct attr_stack *read_attr_from_blob(struct index_state *istate,\n ++\t\t\t\t\t const struct object_id *tree_oid,\n ++\t\t\t\t\t const char *path, unsigned flags)\n +{\n +\tstruct object_id oid;\n +\tunsigned long sz;\n +\tenum object_type type;\n +\tvoid *buf;\n -+\tstruct strbuf sb;\n ++\tunsigned short mode;\n +\n -+\tif (object_name == NULL)\n ++\tif (!tree_oid)\n +\t\treturn NULL;\n +\n -+\tstrbuf_init(&sb, strlen(path) + 1 + strlen(object_name));\n -+\tstrbuf_addstr(&sb, object_name);\n -+\tstrbuf_addstr(&sb, \":\");\n -+\tstrbuf_addstr(&sb, path);\n -+\n -+\tif (get_oid(sb.buf, &oid))\n ++\tif (get_tree_entry(istate->repo, tree_oid, path, &oid, &mode))\n +\t\treturn NULL;\n +\n +\tbuf = read_object_file(&oid, &type, &sz);\n @@ attr.c: static struct attr_stack *read_attr_from_index(struct index_state *istat\n }\n \n static struct attr_stack *read_attr(struct index_state *istate,\n --\t\t\t\t const char *path, unsigned flags)\n -+\t\t\t\t const char *path, const char *object_name,\n -+\t\t\t\t unsigned flags)\n ++\t\t\t\t const struct object_id *tree_oid,\n + \t\t\t\t const char *path, unsigned flags)\n {\n \tstruct attr_stack *res = NULL;\n \n \tif (direction == GIT_ATTR_INDEX) {\n \t\tres = read_attr_from_index(istate, path, flags);\n -+\t} else if (object_name != NULL) {\n -+\t\tres = read_attr_from_blob(path, object_name, flags);\n ++\t} else if (tree_oid) {\n ++\t\tres = read_attr_from_blob(istate, tree_oid, path, flags);\n \t} else if (!is_bare_repository()) {\n \t\tif (direction == GIT_ATTR_CHECKOUT) {\n \t\t\tres = read_attr_from_index(istate, path, flags);\n @@ attr.c: static void push_stack(struct attr_stack **attr_stack_p,\n }\n \n static void bootstrap_attr_stack(struct index_state *istate,\n --\t\t\t\t struct attr_stack **stack)\n -+\t\t\t\t struct attr_stack **stack,\n -+\t\t\t\t const char *object_name)\n ++\t\t\t\t const struct object_id *tree_oid,\n + \t\t\t\t struct attr_stack **stack)\n {\n \tstruct attr_stack *e;\n - \tunsigned flags = READ_ATTR_MACRO_OK;\n @@ attr.c: static void bootstrap_attr_stack(struct index_state *istate,\n \t}\n \n \t/* root directory */\n -\te = read_attr(istate, GITATTRIBUTES_FILE, flags | READ_ATTR_NOFOLLOW);\n -+\te = read_attr(istate, GITATTRIBUTES_FILE, object_name, flags | READ_ATTR_NOFOLLOW);\n ++\te = read_attr(istate, tree_oid, GITATTRIBUTES_FILE, flags | READ_ATTR_NOFOLLOW);\n \tpush_stack(stack, e, xstrdup(\"\"), 0);\n \n \t/* info frame */\n @@ attr.c: static void bootstrap_attr_stack(struct index_state *istate,\n - \tpush_stack(stack, e, NULL, 0);\n }\n \n --static void prepare_attr_stack(struct index_state *istate,\n --\t\t\t const char *path, int dirlen,\n --\t\t\t struct attr_stack **stack)\n -+static void prepare_attr_stack(struct index_state *istate, const char *path,\n -+\t\t\t int dirlen, struct attr_stack **stack,\n -+\t\t\t const char *object_name)\n + static void prepare_attr_stack(struct index_state *istate,\n ++\t\t\t const struct object_id *tree_oid,\n + \t\t\t const char *path, int dirlen,\n + \t\t\t struct attr_stack **stack)\n {\n - \tstruct attr_stack *info;\n - \tstruct strbuf pathbuf = STRBUF_INIT;\n @@ attr.c: static void prepare_attr_stack(struct index_state *istate,\n \t * .gitattributes in deeper directories to shallower ones,\n \t * and finally use the built-in set as the default.\n \t */\n -\tbootstrap_attr_stack(istate, stack);\n -+\tbootstrap_attr_stack(istate, stack, object_name);\n ++\tbootstrap_attr_stack(istate, tree_oid, stack);\n \n \t/*\n \t * Pop the \"info\" one that is always at the top of the stack.\n @@ attr.c: static void prepare_attr_stack(struct index_state *istate,\n \t\tstrbuf_addf(&pathbuf, \"/%s\", GITATTRIBUTES_FILE);\n \n -\t\tnext = read_attr(istate, pathbuf.buf, READ_ATTR_NOFOLLOW);\n -+\t\tnext = read_attr(istate, pathbuf.buf, object_name, READ_ATTR_NOFOLLOW);\n ++\t\tnext = read_attr(istate, tree_oid, pathbuf.buf, READ_ATTR_NOFOLLOW);\n \n \t\t/* reset the pathbuf to not include \"/.gitattributes\" */\n \t\tstrbuf_setlen(&pathbuf, len);\n @@ attr.c: static void determine_macros(struct all_attrs_item *all_attrs,\n - * If check->check_nr is non-zero, only attributes in check[] are collected.\n * Otherwise all attributes are collected.\n */\n --static void collect_some_attrs(struct index_state *istate,\n + static void collect_some_attrs(struct index_state *istate,\n -\t\t\t const char *path,\n -\t\t\t struct attr_check *check)\n -+static void collect_some_attrs(struct index_state *istate, const char *path,\n -+\t\t\t struct attr_check *check,\n -+\t\t\t const char *object_name)\n ++\t\t\t const struct object_id *tree_oid,\n ++\t\t\t const char *path, struct attr_check *check)\n {\n \tint pathlen, rem, dirlen;\n \tconst char *cp, *last_slash = NULL;\n @@ attr.c: static void collect_some_attrs(struct index_state *istate,\n \t}\n \n -\tprepare_attr_stack(istate, path, dirlen, &check->stack);\n -+\tprepare_attr_stack(istate, path, dirlen, &check->stack, object_name);\n ++\tprepare_attr_stack(istate, tree_oid, path, dirlen, &check->stack);\n \tall_attrs_init(&g_attr_hashmap, check);\n \tdetermine_macros(check->all_attrs, check->stack);\n \n @@ attr.c: static void collect_some_attrs(struct index_state *istate,\n - \tfill(path, pathlen, basename_offset, check->stack, check->all_attrs, rem);\n }\n \n --void git_check_attr(struct index_state *istate,\n + void git_check_attr(struct index_state *istate,\n -\t\t const char *path,\n --\t\t struct attr_check *check)\n -+void git_check_attr(struct index_state *istate, const char *path,\n -+\t\t struct attr_check *check, const char *object_name)\n ++\t\t const struct object_id *tree_oid, const char *path,\n + \t\t struct attr_check *check)\n {\n \tint i;\n \n -\tcollect_some_attrs(istate, path, check);\n -+\tcollect_some_attrs(istate, path, check, object_name);\n ++\tcollect_some_attrs(istate, tree_oid, path, check);\n \n \tfor (i = 0; i < check->nr; i++) {\n \t\tsize_t n = check->items[i].attr->attr_nr;\n @@ attr.c: void git_check_attr(struct index_state *istate,\n }\n \n -void git_all_attrs(struct index_state *istate,\n --\t\t const char *path, struct attr_check *check)\n -+void git_all_attrs(struct index_state *istate, const char *path,\n -+\t\t struct attr_check *check, const char *object_name)\n ++void git_all_attrs(struct index_state *istate, const struct object_id *tree_oid,\n + \t\t const char *path, struct attr_check *check)\n {\n \tint i;\n \n \tattr_check_reset(check);\n -\tcollect_some_attrs(istate, path, check);\n -+\tcollect_some_attrs(istate, path, check, object_name);\n ++\tcollect_some_attrs(istate, tree_oid, path, check);\n \n \tfor (i = 0; i < check->all_attrs_nr; i++) {\n \t\tconst char *name = check->all_attrs[i].attr->name;\n \n ## attr.h ##\n -@@ attr.h: struct attr_check {\n - \tint all_attrs_nr;\n - \tstruct all_attrs_item *all_attrs;\n - \tstruct attr_stack *stack;\n -+\tchar *object_id;\n - };\n +@@\n + #ifndef ATTR_H\n + #define ATTR_H\n \n - struct attr_check *attr_check_alloc(void);\n ++#include \"hash.h\"\n ++\n + /**\n + * gitattributes mechanism gives a uniform way to associate various attributes\n + * to set of paths.\n @@ attr.h: void attr_check_free(struct attr_check *check);\n const char *git_attr_name(const struct git_attr *);\n \n void git_check_attr(struct index_state *istate,\n -\t\t const char *path, struct attr_check *check);\n -+\t\t\t\t\tconst char *path, struct attr_check *check,\n -+\t\t\t\t\tconst char *object_name);\n ++\t\t const struct object_id *tree_oid, const char *path,\n ++\t\t struct attr_check *check);\n \n /*\n * Retrieve all attributes that apply to the specified path.\n * check holds the attributes and their values.\n */\n - void git_all_attrs(struct index_state *istate,\n --\t\t const char *path, struct attr_check *check);\n -+\t\t\t\t const char *path, struct attr_check *check,\n -+\t\t\t\t const char *object_name);\n +-void git_all_attrs(struct index_state *istate,\n ++void git_all_attrs(struct index_state *istate, const struct object_id *tree_oid,\n + \t\t const char *path, struct attr_check *check);\n \n enum git_attr_direction {\n - \tGIT_ATTR_CHECKIN,\n \n ## builtin/check-attr.c ##\n +@@\n ++#include \"repository.h\"\n + #define USE_THE_INDEX_VARIABLE\n + #include \"builtin.h\"\n + #include \"cache.h\"\n @@\n static int all_attrs;\n static int cached_attrs;\n static int stdin_paths;\n -+static char *object_name;\n ++static char *revision;\n static const char * const check_attr_usage[] = {\n - N_(\"git check-attr [-a | --all | ...] [--] ...\"),\n - N_(\"git check-attr --stdin [-z] [-a | --all | ...]\"),\n +-N_(\"git check-attr [-a | --all | ...] [--] ...\"),\n +-N_(\"git check-attr --stdin [-z] [-a | --all | ...]\"),\n ++N_(\"git check-attr [-a | --all | ...] [-r ] [--] ...\"),\n ++N_(\"git check-attr --stdin [-z] [-a | --all | ...] [-r ]\"),\n + NULL\n + };\n + \n @@ builtin/check-attr.c: static const struct option check_attr_options[] = {\n \tOPT_BOOL(0 , \"stdin\", &stdin_paths, N_(\"read file names from stdin\")),\n \tOPT_BOOL('z', NULL, &nul_term_line,\n \t\t N_(\"terminate input and output records by a NUL character\")),\n -+\tOPT_STRING('r', \"revision\", &object_name, N_(\"revision\"), N_(\"check attributes at this revision\")),\n ++\tOPT_STRING('r', \"revision\", &revision, N_(\"revision\"), N_(\"check attributes at this revision\")),\n \tOPT_END()\n };\n \n @@ builtin/check-attr.c: static void output_attr(struct attr_check *check, const ch\n -static void check_attr(const char *prefix,\n -\t\t struct attr_check *check,\n -\t\t int collect_all,\n --\t\t const char *file)\n +static void check_attr(const char *prefix, struct attr_check *check,\n -+\t\t int collect_all, const char *file,\n -+\t\t const char *object_name\n -+)\n ++\t\t const struct object_id *tree_oid, int collect_all,\n + \t\t const char *file)\n ++\n {\n \tchar *full_path =\n \t\tprefix_path(prefix, prefix ? strlen(prefix) : 0, file);\n \n \tif (collect_all) {\n -\t\tgit_all_attrs(&the_index, full_path, check);\n -+\t\tgit_all_attrs(&the_index, full_path, check, object_name);\n ++\t\tgit_all_attrs(&the_index, tree_oid, full_path, check);\n \t} else {\n -\t\tgit_check_attr(&the_index, full_path, check);\n -+\t\tgit_check_attr(&the_index, full_path, check, object_name);\n ++\t\tgit_check_attr(&the_index, tree_oid, full_path, check);\n \t}\n \toutput_attr(check, file);\n \n @@ builtin/check-attr.c: static void output_attr(struct attr_check *check, const ch\n -\t\t\t\t struct attr_check *check,\n -\t\t\t\t int collect_all)\n +static void check_attr_stdin_paths(const char *prefix, struct attr_check *check,\n -+\t\t\t\t int collect_all, const char *object_name)\n ++\t\t\t\t const struct object_id *tree_oid, int collect_all)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct strbuf unquoted = STRBUF_INIT;\n @@ builtin/check-attr.c: static void check_attr_stdin_paths(const char *prefix,\n \t\t\tstrbuf_swap(&buf, &unquoted);\n \t\t}\n -\t\tcheck_attr(prefix, check, collect_all, buf.buf);\n -+\t\tcheck_attr(prefix, check, collect_all, buf.buf, object_name);\n ++\t\tcheck_attr(prefix, check, tree_oid, collect_all, buf.buf);\n \t\tmaybe_flush_or_die(stdout, \"attribute to stdout\");\n \t}\n \tstrbuf_release(&buf);\n +@@ builtin/check-attr.c: static NORETURN void error_with_usage(const char *msg)\n + int cmd_check_attr(int argc, const char **argv, const char *prefix)\n + {\n + \tstruct attr_check *check;\n ++\tstruct object_id tree_oid;\n + \tint cnt, i, doubledash, filei;\n + \n + \tif (!is_bare_repository())\n @@ builtin/check-attr.c: int cmd_check_attr(int argc, const char **argv, const char *prefix)\n + \t\t}\n \t}\n \n ++\tif (revision)\n ++\t\tif (repo_get_oid_tree(the_repository, revision, &tree_oid))\n ++\t\t\terror(\"%s: not a valid revision\", revision);\n ++\n \tif (stdin_paths)\n -\t\tcheck_attr_stdin_paths(prefix, check, all_attrs);\n -+\t\tcheck_attr_stdin_paths(prefix, check, all_attrs, object_name);\n ++\t\tcheck_attr_stdin_paths(prefix, check, &tree_oid, all_attrs);\n \telse {\n \t\tfor (i = filei; i < argc; i++)\n -\t\t\tcheck_attr(prefix, check, all_attrs, argv[i]);\n -+\t\t\tcheck_attr(prefix, check, all_attrs, argv[i], object_name);\n ++\t\t\tcheck_attr(prefix, check, &tree_oid, all_attrs, argv[i]);\n \t\tmaybe_flush_or_die(stdout, \"attribute to stdout\");\n \t}\n \n @@ builtin/pack-objects.c: static int no_try_delta(const char *path)\n \tif (!check)\n \t\tcheck = attr_check_initl(\"delta\", NULL);\n -\tgit_check_attr(the_repository->index, path, check);\n -+\tgit_check_attr(the_repository->index, path, check, NULL);\n ++\tgit_check_attr(the_repository->index, NULL, path, check);\n \tif (ATTR_FALSE(check->items[0].value))\n \t\treturn 1;\n \treturn 0;\n @@ convert.c: void convert_attrs(struct index_state *istate,\n \t}\n \n -\tgit_check_attr(istate, path, check);\n -+\tgit_check_attr(istate, path, check, NULL);\n ++\tgit_check_attr(istate, NULL, path, check);\n \tccheck = check->items;\n \tca->crlf_action = git_path_check_crlf(ccheck + 4);\n \tif (ca->crlf_action == CRLF_UNDEFINED)\n @@ ll-merge.c: enum ll_merge_result ll_merge(mmbuffer_t *result_buf,\n \t}\n \n -\tgit_check_attr(istate, path, check);\n -+\tgit_check_attr(istate, path, check, NULL);\n ++\tgit_check_attr(istate, NULL, path, check);\n \tll_driver_name = check->items[0].value;\n \tif (check->items[1].value) {\n \t\tmarker_size = atoi(check->items[1].value);\n @@ ll-merge.c: int ll_merge_marker_size(struct index_state *istate, const char *pat\n \tif (!check)\n \t\tcheck = attr_check_initl(\"conflict-marker-size\", NULL);\n -\tgit_check_attr(istate, path, check);\n -+\tgit_check_attr(istate, path, check, NULL);\n ++\tgit_check_attr(istate, NULL, path, check);\n \tif (check->items[0].value) {\n \t\tmarker_size = atoi(check->items[0].value);\n \t\tif (marker_size <= 0)\n @@ pathspec.c: int match_pathspec_attrs(struct index_state *istate,\n \t\tname = to_free = xmemdupz(name, namelen);\n \n -\tgit_check_attr(istate, name, item->attr_check);\n -+\tgit_check_attr(istate, name, item->attr_check, NULL);\n ++\tgit_check_attr(istate, NULL, name, item->attr_check);\n \n \tfree(to_free);\n \n @@ t/t0003-attributes.sh: test_expect_success 'setup' '\n +\t(\n +\t\techo \"f\ttest=f\" &&\n +\t\techo \"a/i test=n\"\n -+\t) | git hash-object -w --stdin > id &&\n ++\t) | git hash-object -w --stdin >id &&\n +\tgit update-index --add --cacheinfo 100644,$(cat id),foo/bar/.gitattributes &&\n -+\tgit write-tree > id &&\n -+\tgit commit-tree $(cat id) -m \"random commit message\" > id &&\n -+\tgit update-ref refs/heads/branch1 $(cat id) &&\n ++\tgit write-tree >id &&\n ++\ttree_id=$(cat id) &&\n ++\tgit commit-tree $tree_id -m \"random commit message\" >id &&\n ++\tcommit_id=$(cat id) &&\n ++\tgit update-ref refs/heads/branch1 $commit_id &&\n +\n +\t(\n +\t\techo \"g test=g\" &&\n +\t\techo \"a/i test=m\"\n -+\t) | git hash-object -w --stdin > id &&\n ++\t) | git hash-object -w --stdin >id &&\n +\tgit update-index --add --cacheinfo 100644,$(cat id),foo/bar/.gitattributes &&\n -+\tgit write-tree > id &&\n -+\tgit commit-tree $(cat id) -m \"random commit message\" > id &&\n -+\tgit update-ref refs/heads/branch2 $(cat id)\n ++\tgit write-tree >id &&\n ++\ttree_id=$(cat id) &&\n ++\tgit commit-tree $tree_id -m \"random commit message\" >id &&\n ++\tcommit_id=$(cat id) &&\n ++\tgit update-ref refs/heads/branch2 $commit_id\n +'\n +\n test_expect_success 'command line checks' '\n \ttest_must_fail git check-attr &&\n \ttest_must_fail git check-attr -- &&\n + \ttest_must_fail git check-attr test &&\n + \ttest_must_fail git check-attr test -- &&\n + \ttest_must_fail git check-attr -- f &&\n ++\ttest_must_fail git check-attr -r &&\n ++\ttest_must_fail git check-attr -r not-a-valid-ref &&\n + \techo \"f\" | test_must_fail git check-attr --stdin &&\n + \techo \"f\" | test_must_fail git check-attr --stdin -- f &&\n + \techo \"f\" | test_must_fail git check-attr --stdin test -- f &&\n @@ t/t0003-attributes.sh: test_expect_success 'using --git-dir and --work-tree' '\n \t)\n '\n @@ t/t0003-attributes.sh: test_expect_success 'bare repository: check that .gitattr\n +\t\t(\n +\t\t\techo \"f\ttest=f\" &&\n +\t\t\techo \"a/i test=a/i\"\n -+\t\t) | git hash-object -w --stdin > id &&\n ++\t\t) | git hash-object -w --stdin >id &&\n +\t\tgit update-index --add --cacheinfo 100644 $(cat id) .gitattributes &&\n -+\t\tgit write-tree > id &&\n -+\t\tgit commit-tree $(cat id) -m \"random commit message\" > id &&\n -+\t\tgit update-ref refs/heads/master $(cat id) &&\n ++\t\tgit write-tree >id &&\n ++\t\ttree_id=$(cat id) &&\n ++\t\tgit commit-tree $tree_id -m \"random commit message\" >id &&\n ++\t\tcommit_id=$(cat id) &&\n ++\t\tgit update-ref refs/heads/master $commit_id &&\n +\t\tattr_check_revision f f HEAD &&\n +\t\tattr_check_revision a/f f HEAD &&\n +\t\tattr_check_revision a/c/f f HEAD &&\n @@ userdiff.c: struct userdiff_driver *userdiff_find_by_path(struct index_state *is\n \tif (!path)\n \t\treturn NULL;\n -\tgit_check_attr(istate, path, check);\n -+\tgit_check_attr(istate, path, check, NULL);\n ++\tgit_check_attr(istate, NULL, path, check);\n \n \tif (ATTR_TRUE(check->items[0].value))\n \t\treturn &driver_true;\n @@ ws.c: unsigned whitespace_rule(struct index_state *istate, const char *pathname)\n \t\tattr_whitespace_rule = attr_check_initl(\"whitespace\", NULL);\n \n -\tgit_check_attr(istate, pathname, attr_whitespace_rule);\n -+\tgit_check_attr(istate, pathname, attr_whitespace_rule, NULL);\n ++\tgit_check_attr(istate, NULL, pathname, attr_whitespace_rule);\n \tvalue = attr_whitespace_rule->items[0].value;\n \tif (ATTR_TRUE(value)) {\n \t\t/* true (whitespace) */\n\n\nKarthik Nayak (2):\n t0003: move setup for `--all` into new block\n attr: add flag `-r|--revisions` to work with revisions\n\n Documentation/git-check-attr.txt | 8 ++-\n archive.c | 2 +-\n attr.c | 100 ++++++++++++++++++++++---------\n attr.h | 7 ++-\n builtin/check-attr.c | 33 ++++++----\n builtin/pack-objects.c | 2 +-\n convert.c | 2 +-\n ll-merge.c | 4 +-\n pathspec.c | 2 +-\n t/t0003-attributes.sh | 71 +++++++++++++++++++++-\n userdiff.c | 2 +-\n ws.c | 2 +-\n 12 files changed, 180 insertions(+), 55 deletions(-)\n\n-- \n2.38.1\n\n"},{"id":"468845","messageId":"20221209210321.709156-3-karthik.188@gmail.com","threadId":"58925","inReplyTo":"20221209210321.709156-1-karthik.188@gmail.com","subject":"[PATCH v2 2/2] attr: add flag `-r|--revisions` to work with revisions","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2022-12-09T21:03:21Z","receivedAt":"2022-12-09T21:03:36Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Git check-attr currently doesn't check the git worktree, it either\nchecks the index or the files directly. This means we cannot check the\nattributes for a file against a certain revision.\n\nAdd a new flag `--revision`/`-r` which will allow it work with\nrevisions. This command will now, instead of checking the files/index,\ntry and receive the blob for the given attribute file against the\nprovided revision. The flag overrides checking against the index and\nfilesystem and also works with bare repositories.\n\nWe cannot use the `:` syntax like the one used in `git show`\nbecause any non-flag parameter before `--` is treated as an attribute\nand any parameter after `--` is treated as a pathname.\n\nThis involves creating a new function `read_attr_from_blob`, which given\nthe path reads the blob for the path against the provided revision and\nparses the attributes line by line. This function is plugged into\n`read_attr()` function wherein we go through the different attributes.\n\nSigned-off-by: Karthik Nayak \nCo-authored-by: toon@iotcl.com\n---\n Documentation/git-check-attr.txt | 8 ++-\n archive.c | 2 +-\n attr.c | 100 ++++++++++++++++++++++---------\n attr.h | 7 ++-\n builtin/check-attr.c | 33 ++++++----\n builtin/pack-objects.c | 2 +-\n convert.c | 2 +-\n ll-merge.c | 4 +-\n pathspec.c | 2 +-\n t/t0003-attributes.sh | 64 +++++++++++++++++++-\n userdiff.c | 2 +-\n ws.c | 2 +-\n 12 files changed, 175 insertions(+), 53 deletions(-)\n\ndiff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt\nindex 84f41a8e82..3dc61f0fdc 100644\n--- a/Documentation/git-check-attr.txt\n+++ b/Documentation/git-check-attr.txt\n@@ -9,8 +9,8 @@ git-check-attr - Display gitattributes information\n SYNOPSIS\n --------\n [verse]\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 \n DESCRIPTION\n -----------\n@@ -36,6 +36,10 @@ OPTIONS\n \tIf `--stdin` is also given, input paths are separated\n \twith a NUL character instead of a linefeed character.\n \n+--r ::\n+--revision=::\n+\tCheck attributes against the specified revision.\n+\n \\--::\n \tInterpret all preceding arguments as attributes and all following\n \targuments as path names.\ndiff --git a/archive.c b/archive.c\nindex 941495f5d7..81ff76fce9 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -120,7 +120,7 @@ static const struct attr_check *get_archive_attrs(struct index_state *istate,\n \tstatic struct attr_check *check;\n \tif (!check)\n \t\tcheck = attr_check_initl(\"export-ignore\", \"export-subst\", NULL);\n-\tgit_check_attr(istate, path, check);\n+\tgit_check_attr(istate, NULL, path, check);\n \treturn check;\n }\n \ndiff --git a/attr.c b/attr.c\nindex 42ad6de8c7..99883024ec 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -11,8 +11,13 @@\n #include \"exec-cmd.h\"\n #include \"attr.h\"\n #include \"dir.h\"\n+#include \"git-compat-util.h\"\n+#include \"strbuf.h\"\n+#include \"tree-walk.h\"\n #include \"utf8.h\"\n #include \"quote.h\"\n+#include \"revision.h\"\n+#include \"object-store.h\"\n #include \"thread-utils.h\"\n \n const char git_attr__true[] = \"(builtin)true\";\n@@ -729,14 +734,62 @@ static struct attr_stack *read_attr_from_file(const char *path, unsigned flags)\n \treturn res;\n }\n \n-static struct attr_stack *read_attr_from_index(struct index_state *istate,\n-\t\t\t\t\t const char *path,\n-\t\t\t\t\t unsigned flags)\n+static struct attr_stack *read_attr_from_buf(char *buf, const char *path,\n+\t\t\t\t\t unsigned flags)\n {\n \tstruct attr_stack *res;\n-\tchar *buf, *sp;\n+\tchar *sp;\n \tint lineno = 0;\n \n+\tif (!buf)\n+\t\treturn NULL;\n+\n+\tCALLOC_ARRAY(res, 1);\n+\tfor (sp = buf; *sp;) {\n+\t\tchar *ep;\n+\t\tint more;\n+\n+\t\tep = strchrnul(sp, '\\n');\n+\t\tmore = (*ep == '\\n');\n+\t\t*ep = '\\0';\n+\t\thandle_attr_line(res, sp, path, ++lineno, flags);\n+\t\tsp = ep + more;\n+\t}\n+\tfree(buf);\n+\n+\treturn res;\n+}\n+\n+static struct attr_stack *read_attr_from_blob(struct index_state *istate,\n+\t\t\t\t\t const struct object_id *tree_oid,\n+\t\t\t\t\t const char *path, unsigned flags)\n+{\n+\tstruct object_id oid;\n+\tunsigned long sz;\n+\tenum object_type type;\n+\tvoid *buf;\n+\tunsigned short mode;\n+\n+\tif (!tree_oid)\n+\t\treturn NULL;\n+\n+\tif (get_tree_entry(istate->repo, tree_oid, path, &oid, &mode))\n+\t\treturn NULL;\n+\n+\tbuf = read_object_file(&oid, &type, &sz);\n+\tif (!buf || type != OBJ_BLOB) {\n+\t\tfree(buf);\n+\t\treturn NULL;\n+\t}\n+\n+\treturn read_attr_from_buf(buf, path, flags);\n+}\n+\n+static struct attr_stack *read_attr_from_index(struct index_state *istate,\n+\t\t\t\t\t const char *path, unsigned flags)\n+{\n+\tchar *buf;\n+\n \tif (!istate)\n \t\treturn NULL;\n \n@@ -758,28 +811,19 @@ static struct attr_stack *read_attr_from_index(struct index_state *istate,\n \tif (!buf)\n \t\treturn NULL;\n \n-\tCALLOC_ARRAY(res, 1);\n-\tfor (sp = buf; *sp; ) {\n-\t\tchar *ep;\n-\t\tint more;\n-\n-\t\tep = strchrnul(sp, '\\n');\n-\t\tmore = (*ep == '\\n');\n-\t\t*ep = '\\0';\n-\t\thandle_attr_line(res, sp, path, ++lineno, flags);\n-\t\tsp = ep + more;\n-\t}\n-\tfree(buf);\n-\treturn res;\n+\treturn read_attr_from_buf(buf, path, flags);\n }\n \n static struct attr_stack *read_attr(struct index_state *istate,\n+\t\t\t\t const struct object_id *tree_oid,\n \t\t\t\t const char *path, unsigned flags)\n {\n \tstruct attr_stack *res = NULL;\n \n \tif (direction == GIT_ATTR_INDEX) {\n \t\tres = read_attr_from_index(istate, path, flags);\n+\t} else if (tree_oid) {\n+\t\tres = read_attr_from_blob(istate, tree_oid, path, flags);\n \t} else if (!is_bare_repository()) {\n \t\tif (direction == GIT_ATTR_CHECKOUT) {\n \t\t\tres = read_attr_from_index(istate, path, flags);\n@@ -839,6 +883,7 @@ static void push_stack(struct attr_stack **attr_stack_p,\n }\n \n static void bootstrap_attr_stack(struct index_state *istate,\n+\t\t\t\t const struct object_id *tree_oid,\n \t\t\t\t struct attr_stack **stack)\n {\n \tstruct attr_stack *e;\n@@ -864,7 +909,7 @@ static void bootstrap_attr_stack(struct index_state *istate,\n \t}\n \n \t/* root directory */\n-\te = read_attr(istate, GITATTRIBUTES_FILE, flags | READ_ATTR_NOFOLLOW);\n+\te = read_attr(istate, tree_oid, GITATTRIBUTES_FILE, flags | READ_ATTR_NOFOLLOW);\n \tpush_stack(stack, e, xstrdup(\"\"), 0);\n \n \t/* info frame */\n@@ -878,6 +923,7 @@ static void bootstrap_attr_stack(struct index_state *istate,\n }\n \n static void prepare_attr_stack(struct index_state *istate,\n+\t\t\t const struct object_id *tree_oid,\n \t\t\t const char *path, int dirlen,\n \t\t\t struct attr_stack **stack)\n {\n@@ -899,7 +945,7 @@ static void prepare_attr_stack(struct index_state *istate,\n \t * .gitattributes in deeper directories to shallower ones,\n \t * and finally use the built-in set as the default.\n \t */\n-\tbootstrap_attr_stack(istate, stack);\n+\tbootstrap_attr_stack(istate, tree_oid, stack);\n \n \t/*\n \t * Pop the \"info\" one that is always at the top of the stack.\n@@ -954,7 +1000,7 @@ static void prepare_attr_stack(struct index_state *istate,\n \t\tstrbuf_add(&pathbuf, path + pathbuf.len, (len - pathbuf.len));\n \t\tstrbuf_addf(&pathbuf, \"/%s\", GITATTRIBUTES_FILE);\n \n-\t\tnext = read_attr(istate, pathbuf.buf, READ_ATTR_NOFOLLOW);\n+\t\tnext = read_attr(istate, tree_oid, pathbuf.buf, READ_ATTR_NOFOLLOW);\n \n \t\t/* reset the pathbuf to not include \"/.gitattributes\" */\n \t\tstrbuf_setlen(&pathbuf, len);\n@@ -1074,8 +1120,8 @@ static void determine_macros(struct all_attrs_item *all_attrs,\n * Otherwise all attributes are collected.\n */\n static void collect_some_attrs(struct index_state *istate,\n-\t\t\t const char *path,\n-\t\t\t struct attr_check *check)\n+\t\t\t const struct object_id *tree_oid,\n+\t\t\t const char *path, struct attr_check *check)\n {\n \tint pathlen, rem, dirlen;\n \tconst char *cp, *last_slash = NULL;\n@@ -1094,7 +1140,7 @@ static void collect_some_attrs(struct index_state *istate,\n \t\tdirlen = 0;\n \t}\n \n-\tprepare_attr_stack(istate, path, dirlen, &check->stack);\n+\tprepare_attr_stack(istate, tree_oid, path, dirlen, &check->stack);\n \tall_attrs_init(&g_attr_hashmap, check);\n \tdetermine_macros(check->all_attrs, check->stack);\n \n@@ -1103,12 +1149,12 @@ static void collect_some_attrs(struct index_state *istate,\n }\n \n void git_check_attr(struct index_state *istate,\n-\t\t const char *path,\n+\t\t const struct object_id *tree_oid, const char *path,\n \t\t struct attr_check *check)\n {\n \tint i;\n \n-\tcollect_some_attrs(istate, path, check);\n+\tcollect_some_attrs(istate, tree_oid, path, check);\n \n \tfor (i = 0; i < check->nr; i++) {\n \t\tsize_t n = check->items[i].attr->attr_nr;\n@@ -1119,13 +1165,13 @@ void git_check_attr(struct index_state *istate,\n \t}\n }\n \n-void git_all_attrs(struct index_state *istate,\n+void git_all_attrs(struct index_state *istate, const struct object_id *tree_oid,\n \t\t const char *path, struct attr_check *check)\n {\n \tint i;\n \n \tattr_check_reset(check);\n-\tcollect_some_attrs(istate, path, check);\n+\tcollect_some_attrs(istate, tree_oid, path, check);\n \n \tfor (i = 0; i < check->all_attrs_nr; i++) {\n \t\tconst char *name = check->all_attrs[i].attr->name;\ndiff --git a/attr.h b/attr.h\nindex 3fb40cced0..f4a2bedd68 100644\n--- a/attr.h\n+++ b/attr.h\n@@ -1,6 +1,8 @@\n #ifndef ATTR_H\n #define ATTR_H\n \n+#include \"hash.h\"\n+\n /**\n * gitattributes mechanism gives a uniform way to associate various attributes\n * to set of paths.\n@@ -190,13 +192,14 @@ void attr_check_free(struct attr_check *check);\n const char *git_attr_name(const struct git_attr *);\n \n void git_check_attr(struct index_state *istate,\n-\t\t const char *path, struct attr_check *check);\n+\t\t const struct object_id *tree_oid, const char *path,\n+\t\t struct attr_check *check);\n \n /*\n * Retrieve all attributes that apply to the specified path.\n * check holds the attributes and their values.\n */\n-void git_all_attrs(struct index_state *istate,\n+void git_all_attrs(struct index_state *istate, const struct object_id *tree_oid,\n \t\t const char *path, struct attr_check *check);\n \n enum git_attr_direction {\ndiff --git a/builtin/check-attr.c b/builtin/check-attr.c\nindex 0fef10eb6b..28da31c2a5 100644\n--- a/builtin/check-attr.c\n+++ b/builtin/check-attr.c\n@@ -1,3 +1,4 @@\n+#include \"repository.h\"\n #define USE_THE_INDEX_VARIABLE\n #include \"builtin.h\"\n #include \"cache.h\"\n@@ -9,9 +10,10 @@\n static int all_attrs;\n static int cached_attrs;\n static int stdin_paths;\n+static char *revision;\n static const char * const check_attr_usage[] = {\n-N_(\"git check-attr [-a | --all | ...] [--] ...\"),\n-N_(\"git check-attr --stdin [-z] [-a | --all | ...]\"),\n+N_(\"git check-attr [-a | --all | ...] [-r ] [--] ...\"),\n+N_(\"git check-attr --stdin [-z] [-a | --all | ...] [-r ]\"),\n NULL\n };\n \n@@ -23,6 +25,7 @@ static const struct option check_attr_options[] = {\n \tOPT_BOOL(0 , \"stdin\", &stdin_paths, N_(\"read file names from stdin\")),\n \tOPT_BOOL('z', NULL, &nul_term_line,\n \t\t N_(\"terminate input and output records by a NUL character\")),\n+\tOPT_STRING('r', \"revision\", &revision, N_(\"revision\"), N_(\"check attributes at this revision\")),\n \tOPT_END()\n };\n \n@@ -55,27 +58,26 @@ static void output_attr(struct attr_check *check, const char *file)\n \t}\n }\n \n-static void check_attr(const char *prefix,\n-\t\t struct attr_check *check,\n-\t\t int collect_all,\n+static void check_attr(const char *prefix, struct attr_check *check,\n+\t\t const struct object_id *tree_oid, int collect_all,\n \t\t const char *file)\n+\n {\n \tchar *full_path =\n \t\tprefix_path(prefix, prefix ? strlen(prefix) : 0, file);\n \n \tif (collect_all) {\n-\t\tgit_all_attrs(&the_index, full_path, check);\n+\t\tgit_all_attrs(&the_index, tree_oid, full_path, check);\n \t} else {\n-\t\tgit_check_attr(&the_index, full_path, check);\n+\t\tgit_check_attr(&the_index, tree_oid, full_path, check);\n \t}\n \toutput_attr(check, file);\n \n \tfree(full_path);\n }\n \n-static void check_attr_stdin_paths(const char *prefix,\n-\t\t\t\t struct attr_check *check,\n-\t\t\t\t int collect_all)\n+static void check_attr_stdin_paths(const char *prefix, struct attr_check *check,\n+\t\t\t\t const struct object_id *tree_oid, int collect_all)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct strbuf unquoted = STRBUF_INIT;\n@@ -89,7 +91,7 @@ static void check_attr_stdin_paths(const char *prefix,\n \t\t\t\tdie(\"line is badly quoted\");\n \t\t\tstrbuf_swap(&buf, &unquoted);\n \t\t}\n-\t\tcheck_attr(prefix, check, collect_all, buf.buf);\n+\t\tcheck_attr(prefix, check, tree_oid, collect_all, buf.buf);\n \t\tmaybe_flush_or_die(stdout, \"attribute to stdout\");\n \t}\n \tstrbuf_release(&buf);\n@@ -105,6 +107,7 @@ static NORETURN void error_with_usage(const char *msg)\n int cmd_check_attr(int argc, const char **argv, const char *prefix)\n {\n \tstruct attr_check *check;\n+\tstruct object_id tree_oid;\n \tint cnt, i, doubledash, filei;\n \n \tif (!is_bare_repository())\n@@ -176,11 +179,15 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n+\tif (revision)\n+\t\tif (repo_get_oid_tree(the_repository, revision, &tree_oid))\n+\t\t\terror(\"%s: not a valid revision\", revision);\n+\n \tif (stdin_paths)\n-\t\tcheck_attr_stdin_paths(prefix, check, all_attrs);\n+\t\tcheck_attr_stdin_paths(prefix, check, &tree_oid, all_attrs);\n \telse {\n \t\tfor (i = filei; i < argc; i++)\n-\t\t\tcheck_attr(prefix, check, all_attrs, argv[i]);\n+\t\t\tcheck_attr(prefix, check, &tree_oid, all_attrs, argv[i]);\n \t\tmaybe_flush_or_die(stdout, \"attribute to stdout\");\n \t}\n \ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 573d0b20b7..89535cfa6a 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1318,7 +1318,7 @@ static int no_try_delta(const char *path)\n \n \tif (!check)\n \t\tcheck = attr_check_initl(\"delta\", NULL);\n-\tgit_check_attr(the_repository->index, path, check);\n+\tgit_check_attr(the_repository->index, NULL, path, check);\n \tif (ATTR_FALSE(check->items[0].value))\n \t\treturn 1;\n \treturn 0;\ndiff --git a/convert.c b/convert.c\nindex 9b67649032..a54d1690c0 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1308,7 +1308,7 @@ void convert_attrs(struct index_state *istate,\n \t\tgit_config(read_convert_config, NULL);\n \t}\n \n-\tgit_check_attr(istate, path, check);\n+\tgit_check_attr(istate, NULL, path, check);\n \tccheck = check->items;\n \tca->crlf_action = git_path_check_crlf(ccheck + 4);\n \tif (ca->crlf_action == CRLF_UNDEFINED)\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 22a603e8af..130d26501c 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -391,7 +391,7 @@ enum ll_merge_result ll_merge(mmbuffer_t *result_buf,\n \t\tnormalize_file(theirs, path, istate);\n \t}\n \n-\tgit_check_attr(istate, path, check);\n+\tgit_check_attr(istate, NULL, path, check);\n \tll_driver_name = check->items[0].value;\n \tif (check->items[1].value) {\n \t\tmarker_size = atoi(check->items[1].value);\n@@ -419,7 +419,7 @@ int ll_merge_marker_size(struct index_state *istate, const char *path)\n \n \tif (!check)\n \t\tcheck = attr_check_initl(\"conflict-marker-size\", NULL);\n-\tgit_check_attr(istate, path, check);\n+\tgit_check_attr(istate, NULL, path, check);\n \tif (check->items[0].value) {\n \t\tmarker_size = atoi(check->items[0].value);\n \t\tif (marker_size <= 0)\ndiff --git a/pathspec.c b/pathspec.c\nindex 46e77a85fe..48dec2c709 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -732,7 +732,7 @@ int match_pathspec_attrs(struct index_state *istate,\n \tif (name[namelen])\n \t\tname = to_free = xmemdupz(name, namelen);\n \n-\tgit_check_attr(istate, name, item->attr_check);\n+\tgit_check_attr(istate, NULL, name, item->attr_check);\n \n \tfree(to_free);\n \ndiff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh\nindex b3aabb8aa3..042af0239f 100755\n--- a/t/t0003-attributes.sh\n+++ b/t/t0003-attributes.sh\n@@ -25,7 +25,14 @@ attr_check_quote () {\n \tgit check-attr test -- \"$path\" >actual &&\n \techo \"\\\"$quoted_path\\\": test: $expect\" >expect &&\n \ttest_cmp expect actual\n+}\n+\n+attr_check_revision () {\n+\tpath=\"$1\" expect=\"$2\" revision=\"$3\" git_opts=\"$4\" &&\n \n+\tgit $git_opts check-attr -r $revision test -- \"$path\" >actual 2>err &&\n+\techo \"$path: test: $expect\" >expect &&\n+\ttest_cmp expect actual\n }\n \n test_expect_success 'open-quoted pathname' '\n@@ -33,7 +40,6 @@ test_expect_success 'open-quoted pathname' '\n \tattr_check a unspecified\n '\n \n-\n test_expect_success 'setup' '\n \tmkdir -p a/b/d a/c b &&\n \t(\n@@ -80,12 +86,38 @@ test_expect_success 'setup' '\n \tEOF\n '\n \n+test_expect_success 'setup branches' '\n+\t(\n+\t\techo \"f\ttest=f\" &&\n+\t\techo \"a/i test=n\"\n+\t) | git hash-object -w --stdin >id &&\n+\tgit update-index --add --cacheinfo 100644,$(cat id),foo/bar/.gitattributes &&\n+\tgit write-tree >id &&\n+\ttree_id=$(cat id) &&\n+\tgit commit-tree $tree_id -m \"random commit message\" >id &&\n+\tcommit_id=$(cat id) &&\n+\tgit update-ref refs/heads/branch1 $commit_id &&\n+\n+\t(\n+\t\techo \"g test=g\" &&\n+\t\techo \"a/i test=m\"\n+\t) | git hash-object -w --stdin >id &&\n+\tgit update-index --add --cacheinfo 100644,$(cat id),foo/bar/.gitattributes &&\n+\tgit write-tree >id &&\n+\ttree_id=$(cat id) &&\n+\tgit commit-tree $tree_id -m \"random commit message\" >id &&\n+\tcommit_id=$(cat id) &&\n+\tgit update-ref refs/heads/branch2 $commit_id\n+'\n+\n test_expect_success 'command line checks' '\n \ttest_must_fail git check-attr &&\n \ttest_must_fail git check-attr -- &&\n \ttest_must_fail git check-attr test &&\n \ttest_must_fail git check-attr test -- &&\n \ttest_must_fail git check-attr -- f &&\n+\ttest_must_fail git check-attr -r &&\n+\ttest_must_fail git check-attr -r not-a-valid-ref &&\n \techo \"f\" | test_must_fail git check-attr --stdin &&\n \techo \"f\" | test_must_fail git check-attr --stdin -- f &&\n \techo \"f\" | test_must_fail git check-attr --stdin test -- f &&\n@@ -287,6 +319,15 @@ test_expect_success 'using --git-dir and --work-tree' '\n \t)\n '\n \n+test_expect_success 'using --revision' '\n+\tattr_check_revision foo/bar/f f branch1 &&\n+\tattr_check_revision foo/bar/a/i n branch1 &&\n+\tattr_check_revision foo/bar/f unspecified branch2 &&\n+\tattr_check_revision foo/bar/a/i m branch2 &&\n+\tattr_check_revision foo/bar/g g branch2 &&\n+\tattr_check_revision foo/bar/g unspecified branch1\n+'\n+\n test_expect_success 'setup bare' '\n \tgit clone --template= --bare . bare.git\n '\n@@ -306,6 +347,27 @@ test_expect_success 'bare repository: check that .gitattribute is ignored' '\n \t)\n '\n \n+test_expect_success 'bare repository: with --revision' '\n+\t(\n+\t\tcd bare.git &&\n+\t\t(\n+\t\t\techo \"f\ttest=f\" &&\n+\t\t\techo \"a/i test=a/i\"\n+\t\t) | git hash-object -w --stdin >id &&\n+\t\tgit update-index --add --cacheinfo 100644 $(cat id) .gitattributes &&\n+\t\tgit write-tree >id &&\n+\t\ttree_id=$(cat id) &&\n+\t\tgit commit-tree $tree_id -m \"random commit message\" >id &&\n+\t\tcommit_id=$(cat id) &&\n+\t\tgit update-ref refs/heads/master $commit_id &&\n+\t\tattr_check_revision f f HEAD &&\n+\t\tattr_check_revision a/f f HEAD &&\n+\t\tattr_check_revision a/c/f f HEAD &&\n+\t\tattr_check_revision a/i a/i HEAD &&\n+\t\tattr_check_revision subdir/a/i unspecified HEAD\n+\t)\n+'\n+\n test_expect_success 'bare repository: check that --cached honors index' '\n \t(\n \t\tcd bare.git &&\ndiff --git a/userdiff.c b/userdiff.c\nindex 151d9a5278..b66f090a0b 100644\n--- a/userdiff.c\n+++ b/userdiff.c\n@@ -412,7 +412,7 @@ struct userdiff_driver *userdiff_find_by_path(struct index_state *istate,\n \t\tcheck = attr_check_initl(\"diff\", NULL);\n \tif (!path)\n \t\treturn NULL;\n-\tgit_check_attr(istate, path, check);\n+\tgit_check_attr(istate, NULL, path, check);\n \n \tif (ATTR_TRUE(check->items[0].value))\n \t\treturn &driver_true;\ndiff --git a/ws.c b/ws.c\nindex 6e69877f25..eadbbe5667 100644\n--- a/ws.c\n+++ b/ws.c\n@@ -78,7 +78,7 @@ unsigned whitespace_rule(struct index_state *istate, const char *pathname)\n \tif (!attr_whitespace_rule)\n \t\tattr_whitespace_rule = attr_check_initl(\"whitespace\", NULL);\n \n-\tgit_check_attr(istate, pathname, attr_whitespace_rule);\n+\tgit_check_attr(istate, NULL, pathname, attr_whitespace_rule);\n \tvalue = attr_whitespace_rule->items[0].value;\n \tif (ATTR_TRUE(value)) {\n \t\t/* true (whitespace) */\n-- \n2.38.1\n\n"},{"id":"468853","messageId":"xmqq1qp8yvjv.fsf@gitster.g","threadId":"58925","inReplyTo":"20221209210321.709156-1-karthik.188@gmail.com","subject":"Re: [PATCH v2 0/2] check-attr: add support to work with revisions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-10T00:03:16Z","receivedAt":"2022-12-10T00:03:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> ++'git check-attr' --stdin [-z] [-a | --all | ...] [-r ]\n\nPlease never design your CUI to take unbound number of non-option\narguments before dashed options.\n"},{"id":"468857","messageId":"CAOLa=ZSsFGBw3ta1jWN8cmUch2ca=zTEjp1xMA6Linafx9W53g@mail.gmail.com","threadId":"58925","inReplyTo":"xmqq1qp8yvjv.fsf@gitster.g","subject":"Re: [PATCH v2 0/2] check-attr: add support to work with revisions","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2022-12-10T14:37:48Z","receivedAt":"2022-12-10T14:46:11Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sat, Dec 10, 2022 at 1:03 AM Junio C Hamano wrote:\n>\n> Karthik Nayak writes:\n>\n> > ++'git check-attr' --stdin [-z] [-a | --all | ...] [-r ]\n>\n> Please never design your CUI to take unbound number of non-option\n> arguments before dashed options.\n\nI shall move it inward. Thanks!\n\n-- \n- Karthik\n"},{"id":"468992","messageId":"674caf56-940b-8130-4a5e-ea8dc4783e81@dunelm.org.uk","threadId":"58925","inReplyTo":"20221209210321.709156-3-karthik.188@gmail.com","subject":"Re: [PATCH v2 2/2] attr: add flag `-r|--revisions` to work with revisions","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-12-13T15:18:52Z","receivedAt":"2022-12-13T15:19:01Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Karthik\n\nI've got a couple of comments below about the details of the \nimplementation but the basic idea seems reasonable.\n\nOn 09/12/2022 21:03, Karthik Nayak wrote:\n> Git check-attr currently doesn't check the git worktree,\n\nNormally worktree refers to the directory on disk where the repository's \nworking copy is checked out. Here you seem to mean something else.\n\n> it either\n> checks the index or the files directly. This means we cannot check the\n> attributes for a file against a certain revision.\n> \n> Add a new flag `--revision`/`-r` which will allow it work with\n> revisions. This command will now, instead of checking the files/index,\n> try and receive the blob for the given attribute file against the\n> provided revision. The flag overrides checking against the index and\n> filesystem and also works with bare repositories.\n\nThe system, global and the attributes in .git/info/attributes from the \nfilesystem are still used. It would be useful to document that and \nexplain in the commit message why that is useful when using -r.\n\n-r is documented as accepting a revision but actually accepts any tree. \nThat means a user can pass \"-r HEAD:subdirectory\" and all the attributes \nwill be looked up as if subdirectory was the root directory of the \nrepository which might be confusing. It would be helpful to know if \npassing a tree rather than a revision is useful. If it isn't then you \ncould use lookup_commit_reference() to ensure the user passes a revision.\n\nBest Wishes\n\nPhillip\n\n> We cannot use the `:` syntax like the one used in `git show`\n> because any non-flag parameter before `--` is treated as an attribute\n> and any parameter after `--` is treated as a pathname.\n> \n> This involves creating a new function `read_attr_from_blob`, which given\n> the path reads the blob for the path against the provided revision and\n> parses the attributes line by line. This function is plugged into\n> `read_attr()` function wherein we go through the different attributes.\n> \n> Signed-off-by: Karthik Nayak \n> Co-authored-by: toon@iotcl.com\n> ---\n> Documentation/git-check-attr.txt | 8 ++-\n> archive.c | 2 +-\n> attr.c | 100 ++++++++++++++++++++++---------\n> attr.h | 7 ++-\n> builtin/check-attr.c | 33 ++++++----\n> builtin/pack-objects.c | 2 +-\n> convert.c | 2 +-\n> ll-merge.c | 4 +-\n> pathspec.c | 2 +-\n> t/t0003-attributes.sh | 64 +++++++++++++++++++-\n> userdiff.c | 2 +-\n> ws.c | 2 +-\n> 12 files changed, 175 insertions(+), 53 deletions(-)\n> \n> diff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt\n> index 84f41a8e82..3dc61f0fdc 100644\n> --- a/Documentation/git-check-attr.txt\n> +++ b/Documentation/git-check-attr.txt\n> @@ -9,8 +9,8 @@ git-check-attr - Display gitattributes information\n> SYNOPSIS\n> --------\n> [verse]\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> \n> DESCRIPTION\n> -----------\n> @@ -36,6 +36,10 @@ OPTIONS\n> \tIf `--stdin` is also given, input paths are separated\n> \twith a NUL character instead of a linefeed character.\n> \n> +--r ::\n> +--revision=::\n> +\tCheck attributes against the specified revision.\n> +\n> \\--::\n> \tInterpret all preceding arguments as attributes and all following\n> \targuments as path names.\n> diff --git a/archive.c b/archive.c\n> index 941495f5d7..81ff76fce9 100644\n> --- a/archive.c\n> +++ b/archive.c\n> @@ -120,7 +120,7 @@ static const struct attr_check *get_archive_attrs(struct index_state *istate,\n> \tstatic struct attr_check *check;\n> \tif (!check)\n> \t\tcheck = attr_check_initl(\"export-ignore\", \"export-subst\", NULL);\n> -\tgit_check_attr(istate, path, check);\n> +\tgit_check_attr(istate, NULL, path, check);\n> \treturn check;\n> }\n> \n> diff --git a/attr.c b/attr.c\n> index 42ad6de8c7..99883024ec 100644\n> --- a/attr.c\n> +++ b/attr.c\n> @@ -11,8 +11,13 @@\n> #include \"exec-cmd.h\"\n> #include \"attr.h\"\n> #include \"dir.h\"\n> +#include \"git-compat-util.h\"\n> +#include \"strbuf.h\"\n> +#include \"tree-walk.h\"\n> #include \"utf8.h\"\n> #include \"quote.h\"\n> +#include \"revision.h\"\n> +#include \"object-store.h\"\n> #include \"thread-utils.h\"\n> \n> const char git_attr__true[] = \"(builtin)true\";\n> @@ -729,14 +734,62 @@ static struct attr_stack *read_attr_from_file(const char *path, unsigned flags)\n> \treturn res;\n> }\n> \n> -static struct attr_stack *read_attr_from_index(struct index_state *istate,\n> -\t\t\t\t\t const char *path,\n> -\t\t\t\t\t unsigned flags)\n> +static struct attr_stack *read_attr_from_buf(char *buf, const char *path,\n> +\t\t\t\t\t unsigned flags)\n> {\n> \tstruct attr_stack *res;\n> -\tchar *buf, *sp;\n> +\tchar *sp;\n> \tint lineno = 0;\n> \n> +\tif (!buf)\n> +\t\treturn NULL;\n> +\n> +\tCALLOC_ARRAY(res, 1);\n> +\tfor (sp = buf; *sp;) {\n> +\t\tchar *ep;\n> +\t\tint more;\n> +\n> +\t\tep = strchrnul(sp, '\\n');\n> +\t\tmore = (*ep == '\\n');\n> +\t\t*ep = '\\0';\n> +\t\thandle_attr_line(res, sp, path, ++lineno, flags);\n> +\t\tsp = ep + more;\n> +\t}\n> +\tfree(buf);\n> +\n> +\treturn res;\n> +}\n> +\n> +static struct attr_stack *read_attr_from_blob(struct index_state *istate,\n> +\t\t\t\t\t const struct object_id *tree_oid,\n> +\t\t\t\t\t const char *path, unsigned flags)\n> +{\n> +\tstruct object_id oid;\n> +\tunsigned long sz;\n> +\tenum object_type type;\n> +\tvoid *buf;\n> +\tunsigned short mode;\n> +\n> +\tif (!tree_oid)\n> +\t\treturn NULL;\n> +\n> +\tif (get_tree_entry(istate->repo, tree_oid, path, &oid, &mode))\n> +\t\treturn NULL;\n> +\n> +\tbuf = read_object_file(&oid, &type, &sz);\n> +\tif (!buf || type != OBJ_BLOB) {\n> +\t\tfree(buf);\n> +\t\treturn NULL;\n> +\t}\n> +\n> +\treturn read_attr_from_buf(buf, path, flags);\n> +}\n> +\n> +static struct attr_stack *read_attr_from_index(struct index_state *istate,\n> +\t\t\t\t\t const char *path, unsigned flags)\n> +{\n> +\tchar *buf;\n> +\n> \tif (!istate)\n> \t\treturn NULL;\n> \n> @@ -758,28 +811,19 @@ static struct attr_stack *read_attr_from_index(struct index_state *istate,\n> \tif (!buf)\n> \t\treturn NULL;\n> \n> -\tCALLOC_ARRAY(res, 1);\n> -\tfor (sp = buf; *sp; ) {\n> -\t\tchar *ep;\n> -\t\tint more;\n> -\n> -\t\tep = strchrnul(sp, '\\n');\n> -\t\tmore = (*ep == '\\n');\n> -\t\t*ep = '\\0';\n> -\t\thandle_attr_line(res, sp, path, ++lineno, flags);\n> -\t\tsp = ep + more;\n> -\t}\n> -\tfree(buf);\n> -\treturn res;\n> +\treturn read_attr_from_buf(buf, path, flags);\n> }\n> \n> static struct attr_stack *read_attr(struct index_state *istate,\n> +\t\t\t\t const struct object_id *tree_oid,\n> \t\t\t\t const char *path, unsigned flags)\n> {\n> \tstruct attr_stack *res = NULL;\n> \n> \tif (direction == GIT_ATTR_INDEX) {\n> \t\tres = read_attr_from_index(istate, path, flags);\n> +\t} else if (tree_oid) {\n> +\t\tres = read_attr_from_blob(istate, tree_oid, path, flags);\n> \t} else if (!is_bare_repository()) {\n> \t\tif (direction == GIT_ATTR_CHECKOUT) {\n> \t\t\tres = read_attr_from_index(istate, path, flags);\n> @@ -839,6 +883,7 @@ static void push_stack(struct attr_stack **attr_stack_p,\n> }\n> \n> static void bootstrap_attr_stack(struct index_state *istate,\n> +\t\t\t\t const struct object_id *tree_oid,\n> \t\t\t\t struct attr_stack **stack)\n> {\n> \tstruct attr_stack *e;\n> @@ -864,7 +909,7 @@ static void bootstrap_attr_stack(struct index_state *istate,\n> \t}\n> \n> \t/* root directory */\n> -\te = read_attr(istate, GITATTRIBUTES_FILE, flags | READ_ATTR_NOFOLLOW);\n> +\te = read_attr(istate, tree_oid, GITATTRIBUTES_FILE, flags | READ_ATTR_NOFOLLOW);\n> \tpush_stack(stack, e, xstrdup(\"\"), 0);\n> \n> \t/* info frame */\n> @@ -878,6 +923,7 @@ static void bootstrap_attr_stack(struct index_state *istate,\n> }\n> \n> static void prepare_attr_stack(struct index_state *istate,\n> +\t\t\t const struct object_id *tree_oid,\n> \t\t\t const char *path, int dirlen,\n> \t\t\t struct attr_stack **stack)\n> {\n> @@ -899,7 +945,7 @@ static void prepare_attr_stack(struct index_state *istate,\n> \t * .gitattributes in deeper directories to shallower ones,\n> \t * and finally use the built-in set as the default.\n> \t */\n> -\tbootstrap_attr_stack(istate, stack);\n> +\tbootstrap_attr_stack(istate, tree_oid, stack);\n> \n> \t/*\n> \t * Pop the \"info\" one that is always at the top of the stack.\n> @@ -954,7 +1000,7 @@ static void prepare_attr_stack(struct index_state *istate,\n> \t\tstrbuf_add(&pathbuf, path + pathbuf.len, (len - pathbuf.len));\n> \t\tstrbuf_addf(&pathbuf, \"/%s\", GITATTRIBUTES_FILE);\n> \n> -\t\tnext = read_attr(istate, pathbuf.buf, READ_ATTR_NOFOLLOW);\n> +\t\tnext = read_attr(istate, tree_oid, pathbuf.buf, READ_ATTR_NOFOLLOW);\n> \n> \t\t/* reset the pathbuf to not include \"/.gitattributes\" */\n> \t\tstrbuf_setlen(&pathbuf, len);\n> @@ -1074,8 +1120,8 @@ static void determine_macros(struct all_attrs_item *all_attrs,\n> * Otherwise all attributes are collected.\n> */\n> static void collect_some_attrs(struct index_state *istate,\n> -\t\t\t const char *path,\n> -\t\t\t struct attr_check *check)\n> +\t\t\t const struct object_id *tree_oid,\n> +\t\t\t const char *path, struct attr_check *check)\n> {\n> \tint pathlen, rem, dirlen;\n> \tconst char *cp, *last_slash = NULL;\n> @@ -1094,7 +1140,7 @@ static void collect_some_attrs(struct index_state *istate,\n> \t\tdirlen = 0;\n> \t}\n> \n> -\tprepare_attr_stack(istate, path, dirlen, &check->stack);\n> +\tprepare_attr_stack(istate, tree_oid, path, dirlen, &check->stack);\n> \tall_attrs_init(&g_attr_hashmap, check);\n> \tdetermine_macros(check->all_attrs, check->stack);\n> \n> @@ -1103,12 +1149,12 @@ static void collect_some_attrs(struct index_state *istate,\n> }\n> \n> void git_check_attr(struct index_state *istate,\n> -\t\t const char *path,\n> +\t\t const struct object_id *tree_oid, const char *path,\n> \t\t struct attr_check *check)\n> {\n> \tint i;\n> \n> -\tcollect_some_attrs(istate, path, check);\n> +\tcollect_some_attrs(istate, tree_oid, path, check);\n> \n> \tfor (i = 0; i < check->nr; i++) {\n> \t\tsize_t n = check->items[i].attr->attr_nr;\n> @@ -1119,13 +1165,13 @@ void git_check_attr(struct index_state *istate,\n> \t}\n> }\n> \n> -void git_all_attrs(struct index_state *istate,\n> +void git_all_attrs(struct index_state *istate, const struct object_id *tree_oid,\n> \t\t const char *path, struct attr_check *check)\n> {\n> \tint i;\n> \n> \tattr_check_reset(check);\n> -\tcollect_some_attrs(istate, path, check);\n> +\tcollect_some_attrs(istate, tree_oid, path, check);\n> \n> \tfor (i = 0; i < check->all_attrs_nr; i++) {\n> \t\tconst char *name = check->all_attrs[i].attr->name;\n> diff --git a/attr.h b/attr.h\n> index 3fb40cced0..f4a2bedd68 100644\n> --- a/attr.h\n> +++ b/attr.h\n> @@ -1,6 +1,8 @@\n> #ifndef ATTR_H\n> #define ATTR_H\n> \n> +#include \"hash.h\"\n> +\n> /**\n> * gitattributes mechanism gives a uniform way to associate various attributes\n> * to set of paths.\n> @@ -190,13 +192,14 @@ void attr_check_free(struct attr_check *check);\n> const char *git_attr_name(const struct git_attr *);\n> \n> void git_check_attr(struct index_state *istate,\n> -\t\t const char *path, struct attr_check *check);\n> +\t\t const struct object_id *tree_oid, const char *path,\n> +\t\t struct attr_check *check);\n> \n> /*\n> * Retrieve all attributes that apply to the specified path.\n> * check holds the attributes and their values.\n> */\n> -void git_all_attrs(struct index_state *istate,\n> +void git_all_attrs(struct index_state *istate, const struct object_id *tree_oid,\n> \t\t const char *path, struct attr_check *check);\n> \n> enum git_attr_direction {\n> diff --git a/builtin/check-attr.c b/builtin/check-attr.c\n> index 0fef10eb6b..28da31c2a5 100644\n> --- a/builtin/check-attr.c\n> +++ b/builtin/check-attr.c\n> @@ -1,3 +1,4 @@\n> +#include \"repository.h\"\n> #define USE_THE_INDEX_VARIABLE\n> #include \"builtin.h\"\n> #include \"cache.h\"\n> @@ -9,9 +10,10 @@\n> static int all_attrs;\n> static int cached_attrs;\n> static int stdin_paths;\n> +static char *revision;\n> static const char * const check_attr_usage[] = {\n> -N_(\"git check-attr [-a | --all | ...] [--] ...\"),\n> -N_(\"git check-attr --stdin [-z] [-a | --all | ...]\"),\n> +N_(\"git check-attr [-a | --all | ...] [-r ] [--] ...\"),\n> +N_(\"git check-attr --stdin [-z] [-a | --all | ...] [-r ]\"),\n> NULL\n> };\n> \n> @@ -23,6 +25,7 @@ static const struct option check_attr_options[] = {\n> \tOPT_BOOL(0 , \"stdin\", &stdin_paths, N_(\"read file names from stdin\")),\n> \tOPT_BOOL('z', NULL, &nul_term_line,\n> \t\t N_(\"terminate input and output records by a NUL character\")),\n> +\tOPT_STRING('r', \"revision\", &revision, N_(\"revision\"), N_(\"check attributes at this revision\")),\n> \tOPT_END()\n> };\n> \n> @@ -55,27 +58,26 @@ static void output_attr(struct attr_check *check, const char *file)\n> \t}\n> }\n> \n> -static void check_attr(const char *prefix,\n> -\t\t struct attr_check *check,\n> -\t\t int collect_all,\n> +static void check_attr(const char *prefix, struct attr_check *check,\n> +\t\t const struct object_id *tree_oid, int collect_all,\n> \t\t const char *file)\n> +\n> {\n> \tchar *full_path =\n> \t\tprefix_path(prefix, prefix ? strlen(prefix) : 0, file);\n> \n> \tif (collect_all) {\n> -\t\tgit_all_attrs(&the_index, full_path, check);\n> +\t\tgit_all_attrs(&the_index, tree_oid, full_path, check);\n> \t} else {\n> -\t\tgit_check_attr(&the_index, full_path, check);\n> +\t\tgit_check_attr(&the_index, tree_oid, full_path, check);\n> \t}\n> \toutput_attr(check, file);\n> \n> \tfree(full_path);\n> }\n> \n> -static void check_attr_stdin_paths(const char *prefix,\n> -\t\t\t\t struct attr_check *check,\n> -\t\t\t\t int collect_all)\n> +static void check_attr_stdin_paths(const char *prefix, struct attr_check *check,\n> +\t\t\t\t const struct object_id *tree_oid, int collect_all)\n> {\n> \tstruct strbuf buf = STRBUF_INIT;\n> \tstruct strbuf unquoted = STRBUF_INIT;\n> @@ -89,7 +91,7 @@ static void check_attr_stdin_paths(const char *prefix,\n> \t\t\t\tdie(\"line is badly quoted\");\n> \t\t\tstrbuf_swap(&buf, &unquoted);\n> \t\t}\n> -\t\tcheck_attr(prefix, check, collect_all, buf.buf);\n> +\t\tcheck_attr(prefix, check, tree_oid, collect_all, buf.buf);\n> \t\tmaybe_flush_or_die(stdout, \"attribute to stdout\");\n> \t}\n> \tstrbuf_release(&buf);\n> @@ -105,6 +107,7 @@ static NORETURN void error_with_usage(const char *msg)\n> int cmd_check_attr(int argc, const char **argv, const char *prefix)\n> {\n> \tstruct attr_check *check;\n> +\tstruct object_id tree_oid;\n> \tint cnt, i, doubledash, filei;\n> \n> \tif (!is_bare_repository())\n> @@ -176,11 +179,15 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)\n> \t\t}\n> \t}\n> \n> +\tif (revision)\n> +\t\tif (repo_get_oid_tree(the_repository, revision, &tree_oid))\n> +\t\t\terror(\"%s: not a valid revision\", revision);\n> +\n> \tif (stdin_paths)\n> -\t\tcheck_attr_stdin_paths(prefix, check, all_attrs);\n> +\t\tcheck_attr_stdin_paths(prefix, check, &tree_oid, all_attrs);\n> \telse {\n> \t\tfor (i = filei; i < argc; i++)\n> -\t\t\tcheck_attr(prefix, check, all_attrs, argv[i]);\n> +\t\t\tcheck_attr(prefix, check, &tree_oid, all_attrs, argv[i]);\n> \t\tmaybe_flush_or_die(stdout, \"attribute to stdout\");\n> \t}\n> \n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index 573d0b20b7..89535cfa6a 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -1318,7 +1318,7 @@ static int no_try_delta(const char *path)\n> \n> \tif (!check)\n> \t\tcheck = attr_check_initl(\"delta\", NULL);\n> -\tgit_check_attr(the_repository->index, path, check);\n> +\tgit_check_attr(the_repository->index, NULL, path, check);\n> \tif (ATTR_FALSE(check->items[0].value))\n> \t\treturn 1;\n> \treturn 0;\n> diff --git a/convert.c b/convert.c\n> index 9b67649032..a54d1690c0 100644\n> --- a/convert.c\n> +++ b/convert.c\n> @@ -1308,7 +1308,7 @@ void convert_attrs(struct index_state *istate,\n> \t\tgit_config(read_convert_config, NULL);\n> \t}\n> \n> -\tgit_check_attr(istate, path, check);\n> +\tgit_check_attr(istate, NULL, path, check);\n> \tccheck = check->items;\n> \tca->crlf_action = git_path_check_crlf(ccheck + 4);\n> \tif (ca->crlf_action == CRLF_UNDEFINED)\n> diff --git a/ll-merge.c b/ll-merge.c\n> index 22a603e8af..130d26501c 100644\n> --- a/ll-merge.c\n> +++ b/ll-merge.c\n> @@ -391,7 +391,7 @@ enum ll_merge_result ll_merge(mmbuffer_t *result_buf,\n> \t\tnormalize_file(theirs, path, istate);\n> \t}\n> \n> -\tgit_check_attr(istate, path, check);\n> +\tgit_check_attr(istate, NULL, path, check);\n> \tll_driver_name = check->items[0].value;\n> \tif (check->items[1].value) {\n> \t\tmarker_size = atoi(check->items[1].value);\n> @@ -419,7 +419,7 @@ int ll_merge_marker_size(struct index_state *istate, const char *path)\n> \n> \tif (!check)\n> \t\tcheck = attr_check_initl(\"conflict-marker-size\", NULL);\n> -\tgit_check_attr(istate, path, check);\n> +\tgit_check_attr(istate, NULL, path, check);\n> \tif (check->items[0].value) {\n> \t\tmarker_size = atoi(check->items[0].value);\n> \t\tif (marker_size <= 0)\n> diff --git a/pathspec.c b/pathspec.c\n> index 46e77a85fe..48dec2c709 100644\n> --- a/pathspec.c\n> +++ b/pathspec.c\n> @@ -732,7 +732,7 @@ int match_pathspec_attrs(struct index_state *istate,\n> \tif (name[namelen])\n> \t\tname = to_free = xmemdupz(name, namelen);\n> \n> -\tgit_check_attr(istate, name, item->attr_check);\n> +\tgit_check_attr(istate, NULL, name, item->attr_check);\n> \n> \tfree(to_free);\n> \n> diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh\n> index b3aabb8aa3..042af0239f 100755\n> --- a/t/t0003-attributes.sh\n> +++ b/t/t0003-attributes.sh\n> @@ -25,7 +25,14 @@ attr_check_quote () {\n> \tgit check-attr test -- \"$path\" >actual &&\n> \techo \"\\\"$quoted_path\\\": test: $expect\" >expect &&\n> \ttest_cmp expect actual\n> +}\n> +\n> +attr_check_revision () {\n> +\tpath=\"$1\" expect=\"$2\" revision=\"$3\" git_opts=\"$4\" &&\n> \n> +\tgit $git_opts check-attr -r $revision test -- \"$path\" >actual 2>err &&\n> +\techo \"$path: test: $expect\" >expect &&\n> +\ttest_cmp expect actual\n> }\n> \n> test_expect_success 'open-quoted pathname' '\n> @@ -33,7 +40,6 @@ test_expect_success 'open-quoted pathname' '\n> \tattr_check a unspecified\n> '\n> \n> -\n> test_expect_success 'setup' '\n> \tmkdir -p a/b/d a/c b &&\n> \t(\n> @@ -80,12 +86,38 @@ test_expect_success 'setup' '\n> \tEOF\n> '\n> \n> +test_expect_success 'setup branches' '\n> +\t(\n> +\t\techo \"f\ttest=f\" &&\n> +\t\techo \"a/i test=n\"\n> +\t) | git hash-object -w --stdin >id &&\n> +\tgit update-index --add --cacheinfo 100644,$(cat id),foo/bar/.gitattributes &&\n> +\tgit write-tree >id &&\n> +\ttree_id=$(cat id) &&\n> +\tgit commit-tree $tree_id -m \"random commit message\" >id &&\n> +\tcommit_id=$(cat id) &&\n> +\tgit update-ref refs/heads/branch1 $commit_id &&\n> +\n> +\t(\n> +\t\techo \"g test=g\" &&\n> +\t\techo \"a/i test=m\"\n> +\t) | git hash-object -w --stdin >id &&\n> +\tgit update-index --add --cacheinfo 100644,$(cat id),foo/bar/.gitattributes &&\n> +\tgit write-tree >id &&\n> +\ttree_id=$(cat id) &&\n> +\tgit commit-tree $tree_id -m \"random commit message\" >id &&\n> +\tcommit_id=$(cat id) &&\n> +\tgit update-ref refs/heads/branch2 $commit_id\n> +'\n> +\n> test_expect_success 'command line checks' '\n> \ttest_must_fail git check-attr &&\n> \ttest_must_fail git check-attr -- &&\n> \ttest_must_fail git check-attr test &&\n> \ttest_must_fail git check-attr test -- &&\n> \ttest_must_fail git check-attr -- f &&\n> +\ttest_must_fail git check-attr -r &&\n> +\ttest_must_fail git check-attr -r not-a-valid-ref &&\n> \techo \"f\" | test_must_fail git check-attr --stdin &&\n> \techo \"f\" | test_must_fail git check-attr --stdin -- f &&\n> \techo \"f\" | test_must_fail git check-attr --stdin test -- f &&\n> @@ -287,6 +319,15 @@ test_expect_success 'using --git-dir and --work-tree' '\n> \t)\n> '\n> \n> +test_expect_success 'using --revision' '\n> +\tattr_check_revision foo/bar/f f branch1 &&\n> +\tattr_check_revision foo/bar/a/i n branch1 &&\n> +\tattr_check_revision foo/bar/f unspecified branch2 &&\n> +\tattr_check_revision foo/bar/a/i m branch2 &&\n> +\tattr_check_revision foo/bar/g g branch2 &&\n> +\tattr_check_revision foo/bar/g unspecified branch1\n> +'\n> +\n> test_expect_success 'setup bare' '\n> \tgit clone --template= --bare . bare.git\n> '\n> @@ -306,6 +347,27 @@ test_expect_success 'bare repository: check that .gitattribute is ignored' '\n> \t)\n> '\n> \n> +test_expect_success 'bare repository: with --revision' '\n> +\t(\n> +\t\tcd bare.git &&\n> +\t\t(\n> +\t\t\techo \"f\ttest=f\" &&\n> +\t\t\techo \"a/i test=a/i\"\n> +\t\t) | git hash-object -w --stdin >id &&\n> +\t\tgit update-index --add --cacheinfo 100644 $(cat id) .gitattributes &&\n> +\t\tgit write-tree >id &&\n> +\t\ttree_id=$(cat id) &&\n> +\t\tgit commit-tree $tree_id -m \"random commit message\" >id &&\n> +\t\tcommit_id=$(cat id) &&\n> +\t\tgit update-ref refs/heads/master $commit_id &&\n> +\t\tattr_check_revision f f HEAD &&\n> +\t\tattr_check_revision a/f f HEAD &&\n> +\t\tattr_check_revision a/c/f f HEAD &&\n> +\t\tattr_check_revision a/i a/i HEAD &&\n> +\t\tattr_check_revision subdir/a/i unspecified HEAD\n> +\t)\n> +'\n> +\n> test_expect_success 'bare repository: check that --cached honors index' '\n> \t(\n> \t\tcd bare.git &&\n> diff --git a/userdiff.c b/userdiff.c\n> index 151d9a5278..b66f090a0b 100644\n> --- a/userdiff.c\n> +++ b/userdiff.c\n> @@ -412,7 +412,7 @@ struct userdiff_driver *userdiff_find_by_path(struct index_state *istate,\n> \t\tcheck = attr_check_initl(\"diff\", NULL);\n> \tif (!path)\n> \t\treturn NULL;\n> -\tgit_check_attr(istate, path, check);\n> +\tgit_check_attr(istate, NULL, path, check);\n> \n> \tif (ATTR_TRUE(check->items[0].value))\n> \t\treturn &driver_true;\n> diff --git a/ws.c b/ws.c\n> index 6e69877f25..eadbbe5667 100644\n> --- a/ws.c\n> +++ b/ws.c\n> @@ -78,7 +78,7 @@ unsigned whitespace_rule(struct index_state *istate, const char *pathname)\n> \tif (!attr_whitespace_rule)\n> \t\tattr_whitespace_rule = attr_check_initl(\"whitespace\", NULL);\n> \n> -\tgit_check_attr(istate, pathname, attr_whitespace_rule);\n> +\tgit_check_attr(istate, NULL, pathname, attr_whitespace_rule);\n> \tvalue = attr_whitespace_rule->items[0].value;\n> \tif (ATTR_TRUE(value)) {\n> \t\t/* true (whitespace) */\n"},{"id":"468999","messageId":"xmqqzgbqydso.fsf@gitster.g","threadId":"58925","inReplyTo":"674caf56-940b-8130-4a5e-ea8dc4783e81@dunelm.org.uk","subject":"Re: [PATCH v2 2/2] attr: add flag `-r|--revisions` to work with revisions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-14T01:28:07Z","receivedAt":"2022-12-14T01:28:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> I've got a couple of comments below about the details of the\n> implementation but the basic idea seems reasonable.\n>\n> On 09/12/2022 21:03, Karthik Nayak wrote:\n>> Git check-attr currently doesn't check the git worktree,\n>\n> Normally worktree refers to the directory on disk where the\n> repository's working copy is checked out. Here you seem to mean\n> something else.\n\nStrictly speaking, what you just said is \"working tree\". The term\n\"worktree\" in Git's context yet means something slightly different.\nYou can arrange to have multiple working trees attached to a single\nrepository, and each of these is called a \"worktree\" attached to the\nrepository.\n\nIn any case, thanks for pointing out that the original's wording is\nwrong. It is natural to read it to claim that we do not check the\n.gitattributes files that are checked out in the working trees,\nwhich is utterly incorrect.\n\n>> it either\n>> checks the index or the files directly.\n\n> This means we cannot check the\n> attributes for a file against a certain revision.\n\nWhenever one is tempted to say \"This means\", one should realize that\none does not have absolute confidence in whatever written before it,\nin other words, without additional explanation, one suspects that\nwhat one wanted to say would not be understood.\n\nA good piece of advice for such a person is to try rewriting WITHOUT\nanything before (and including) \"This means\". And I think this is a\ngood example to which the advice applies well.\n\n There is no way with \"git check-attr\" to apply attributes from\n .gitattributes files recorded in the same treeish to paths in a\n treeish object.\n\nOur usual preference is to (1) start by describing the current state\nand (2) propose what can be done by deviating from it, in that\norder, so one might write it like so:\n\n The contents of the .gitattributes files may evolve over time,\n but \"git check-attr\" always checks attributes against them in\n the working tree and/or in the index. It may be beneficial to\n optionally allow the version of .gitattributes found in the same\n commit when checking the attributes for paths in an older commit.\n\nBy the way, applying the attributes from the working tree is by\ndesign and it should stay to be the default. People are almost\nalways working near the tip of the history, and working tree files\nare by definition ahead of any committed version---it is a feature\nthat users can correct attribute definitions in their working tree\nfiles and then apply them to paths in the committed version.\n\n>> Add a new flag `--revision`/`-r` which will allow it work with\n>> revisions. This command will now, instead of checking the files/index,\n>> try and receive the blob for the given attribute file against the\n>> provided revision. The flag overrides checking against the index and\n>> filesystem and also works with bare repositories.\n>\n> The system, global and the attributes in .git/info/attributes from the\n> filesystem are still used. It would be useful to document that and\n> explain in the commit message why that is useful when using -r.\n>\n> -r is documented as accepting a revision but actually accepts any\n> tree. That means a user can pass \"-r HEAD:subdirectory\" and all the\n> attributes will be looked up as if subdirectory was the root\n> directory of the repository which might be confusing. It would be\n> helpful to know if passing a tree rather than a revision is\n> useful. If it isn't then you could use lookup_commit_reference() to\n> ensure the user passes a revision.\n\nUnless you use ancestry relationships in any way [*], you do not\nwant to require commits when an operation only requires trees. In\nthis case, taking tree-ish and documenting it as such is the right\nthing to do.\n\n[Footnote]\n\n* A good example that makes sense to limit to commit-ishes is when\n merging two histories (without requiring the user to supply the\n merge-base). You'd need to compute the merge-bases, so you require\n two committishes and it is not enough to take two trees.\n"},{"id":"469139","messageId":"CAOLa=ZQ2CKcKNOq3AvZHWxKW_L=zLTJXGEGkUbCSYUmBRMDRZg@mail.gmail.com","threadId":"58925","inReplyTo":"xmqqzgbqydso.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] attr: add flag `-r|--revisions` to work with revisions","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2022-12-15T20:20:14Z","receivedAt":"2022-12-15T20:20:46Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Wed, Dec 14, 2022 at 2:28 AM Junio C Hamano wrote:\n>\n> Phillip Wood writes:\n>\n> > I've got a couple of comments below about the details of the\n> > implementation but the basic idea seems reasonable.\n> >\n> > On 09/12/2022 21:03, Karthik Nayak wrote:\n> >> Git check-attr currently doesn't check the git worktree,\n> >\n> > Normally worktree refers to the directory on disk where the\n> > repository's working copy is checked out. Here you seem to mean\n> > something else.\n>\n> Strictly speaking, what you just said is \"working tree\". The term\n> \"worktree\" in Git's context yet means something slightly different.\n> You can arrange to have multiple working trees attached to a single\n> repository, and each of these is called a \"worktree\" attached to the\n> repository.\n>\n> In any case, thanks for pointing out that the original's wording is\n> wrong. It is natural to read it to claim that we do not check the\n> .gitattributes files that are checked out in the working trees,\n> which is utterly incorrect.\n>\n> >> it either\n> >> checks the index or the files directly.\n>\n> > This means we cannot check the\n> > attributes for a file against a certain revision.\n>\n> Whenever one is tempted to say \"This means\", one should realize that\n> one does not have absolute confidence in whatever written before it,\n> in other words, without additional explanation, one suspects that\n> what one wanted to say would not be understood.\n>\n> A good piece of advice for such a person is to try rewriting WITHOUT\n> anything before (and including) \"This means\". And I think this is a\n> good example to which the advice applies well.\n>\n\nI agree with what you're saying here. I think that's excellent advice,\ntoo. Thanks!\n\n> There is no way with \"git check-attr\" to apply attributes from\n> .gitattributes files recorded in the same treeish to paths in a\n> treeish object.\n>\n> Our usual preference is to (1) start by describing the current state\n> and (2) propose what can be done by deviating from it, in that\n> order, so one might write it like so:\n>\n> The contents of the .gitattributes files may evolve over time,\n> but \"git check-attr\" always checks attributes against them in\n> the working tree and/or in the index. It may be beneficial to\n> optionally allow the version of .gitattributes found in the same\n> commit when checking the attributes for paths in an older commit.\n>\n\nFurthermore, I think you've put it nice here, I will copy this over and modify\nthe last statement to:\n\n It may be beneficial to optionally allow the users to check attributes\n against paths from older commits.\n\n> By the way, applying the attributes from the working tree is by\n> design and it should stay to be the default. People are almost\n> always working near the tip of the history, and working tree files\n> are by definition ahead of any committed version---it is a feature\n> that users can correct attribute definitions in their working tree\n> files and then apply them to paths in the committed version.\n>\n\nYeah, this was my understanding as well, I don't think I tried to change this or\nimplied the same anywhere.\n\n> >> Add a new flag `--revision`/`-r` which will allow it work with\n> >> revisions. This command will now, instead of checking the files/index,\n> >> try and receive the blob for the given attribute file against the\n> >> provided revision. The flag overrides checking against the index and\n> >> filesystem and also works with bare repositories.\n> >\n> > The system, global and the attributes in .git/info/attributes from the\n> > filesystem are still used. It would be useful to document that and\n> > explain in the commit message why that is useful when using -r.\n> >\n> > -r is documented as accepting a revision but actually accepts any\n> > tree. That means a user can pass \"-r HEAD:subdirectory\" and all the\n> > attributes will be looked up as if subdirectory was the root\n> > directory of the repository which might be confusing. It would be\n> > helpful to know if passing a tree rather than a revision is\n> > useful. If it isn't then you could use lookup_commit_reference() to\n> > ensure the user passes a revision.\n>\n> Unless you use ancestry relationships in any way [*], you do not\n> want to require commits when an operation only requires trees. In\n> this case, taking tree-ish and documenting it as such is the right\n> thing to do.\n>\n> [Footnote]\n>\n> * A good example that makes sense to limit to commit-ishes is when\n> merging two histories (without requiring the user to supply the\n> merge-base). You'd need to compute the merge-bases, so you require\n> two committishes and it is not enough to take two trees.\n\nWill leave it as it is then.\n\nThanks both for the review. I think I will push a version 3 now,\nmostly changes would be:\n1. Documentation\n2. Commit message\n\nAlso, it has been a while for me on this list, but I did notice this\ntopic missing from the\n`What's cooking in git.git` mail, do I need to do something further?\n\n--\n- Karthik\n"}]}