{"thread":{"id":"59889","subject":"[PATCH] ls-files: add an --exclude-links option","startedAt":"2023-06-21T06:08:13Z","lastAt":"2023-06-21T20:02:39Z","messageCount":7,"participants":["Guido Martínez via GitGitGadget","Jeff King","Guido Martínez","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"478609","messageId":"pull.1549.git.1687327684909.gitgitgadget@gmail.com","threadId":"59889","inReplyTo":null,"subject":"[PATCH] ls-files: add an --exclude-links option","fromName":"Guido Martínez via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-06-21T06:08:04Z","receivedAt":"2023-06-21T06:08:13Z","isPatch":true,"sender":{"key":"mtzguido@gmail.com","avatar":null},"body":"From: =?UTF-8?q?Guido=20Mart=C3=ADnez?= \n\nAdd an option to exclude symlinks from the listed files. This is useful\nin case we are listing the files in order to process the contents,\nfor instance to do some text replacement with `sed -i`. In that case,\nthere is no point in processing the links, and it could even be\ncounterproductive as some tools (like sed) will replace the link with a\nfresh regular file.\n\nThis option enables a straightforward implementation of a `git sed`:\n\n #!/bin/bash\n git ls-files --exclude-links -z | xargs -0 -P $(nproc) -- sed -i -e \"$@\"\n\nSigned-off-by: Guido Martínez \n---\n ls-files: add an --exclude-links option\n \n Hi, not sure if this is desirable, but I found it very useful to\n implement the git sed shown in the commit message to do some\n replacements in a big repo. I placed the option right after\n --exclude-standard and (I think) updated the relevant docs. The flag\n applies to all modes: -c, -d, -m and -o, though I think it's mostly\n useful on the default -c.\n \n Also is there interest in a standard git sed?\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1549%2Fmtzguido%2Fls_files_exclude_links-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1549/mtzguido/ls_files_exclude_links-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1549\n\n Documentation/git-ls-files.txt | 5 ++++-\n builtin/ls-files.c | 10 ++++++++++\n 2 files changed, 14 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt\nindex 1bc0328bb78..138ba49bd8c 100644\n--- a/Documentation/git-ls-files.txt\n+++ b/Documentation/git-ls-files.txt\n@@ -18,7 +18,7 @@ SYNOPSIS\n \t\t[-x |--exclude=]\n \t\t[-X |--exclude-from=]\n \t\t[--exclude-per-directory=]\n-\t\t[--exclude-standard]\n+\t\t[--exclude-standard] [--exclude-links]\n \t\t[--error-unmatch] [--with-tree=]\n \t\t[--full-name] [--recurse-submodules]\n \t\t[--abbrev[=]] [--format=] [--] [...]\n@@ -126,6 +126,9 @@ OPTIONS\n \tAdd the standard Git exclusions: .git/info/exclude, .gitignore\n \tin each directory, and the user's global exclusion file.\n \n+--exclude-links::\n+\tDo not list symbolic links.\n+\n --error-unmatch::\n \tIf any does not appear in the index, treat this as an\n \terror (return 1).\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 72012c0f0f7..0826e89a496 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -46,6 +46,7 @@ static int show_eol;\n static int recurse_submodules;\n static int skipping_duplicates;\n static int show_sparse_dirs;\n+static int exclude_links;\n \n static const char *prefix;\n static int max_prefix_len;\n@@ -171,6 +172,11 @@ static void show_other_files(struct index_state *istate,\n \t\tstruct dir_entry *ent = dir->entries[i];\n \t\tif (!index_name_is_other(istate, ent->name, ent->len))\n \t\t\tcontinue;\n+\t\tif (exclude_links) {\n+\t\t\tstruct stat st;\n+\t\t\tif (!lstat(ent->name, &st) && S_ISLNK(st.st_mode))\n+\t\t\t\tcontinue;\n+\t\t}\n \t\tshow_dir_entry(istate, tag_other, ent);\n \t}\n }\n@@ -451,6 +457,8 @@ static void show_files(struct repository *repo, struct dir_struct *dir)\n \t\t\tcontinue;\n \t\tif (ce->ce_flags & CE_UPDATE)\n \t\t\tcontinue;\n+\t\tif (exclude_links && S_ISLNK(ce->ce_mode))\n+\t\t\tcontinue;\n \t\tif ((show_cached || show_stage) &&\n \t\t (!show_unmerged || ce_stage(ce))) {\n \t\t\tshow_ce(repo, dir, ce, fullname.buf,\n@@ -780,6 +788,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \t\t\tN_(\"add the standard git exclusions\"),\n \t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG,\n \t\t\toption_parse_exclude_standard),\n+\t\tOPT_BOOL(0, \"exclude-links\", &exclude_links,\n+\t\t\t N_(\"do not print symbolic links\")),\n \t\tOPT_SET_INT_F(0, \"full-name\", &prefix_len,\n \t\t\t N_(\"make the output relative to the project top directory\"),\n \t\t\t 0, PARSE_OPT_NONEG),\n\nbase-commit: 6640c2d06d112675426cf436f0594f0e8c614848\n-- \ngitgitgadget\n"},{"id":"478615","messageId":"20230621081754.GA803861@coredump.intra.peff.net","threadId":"59889","inReplyTo":"pull.1549.git.1687327684909.gitgitgadget@gmail.com","subject":"Re: [PATCH] ls-files: add an --exclude-links option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-06-21T08:17:54Z","receivedAt":"2023-06-21T08:18:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 21, 2023 at 06:08:04AM +0000, Guido Martínez via GitGitGadget wrote:\n\n> From: =?UTF-8?q?Guido=20Mart=C3=ADnez?= \n> \n> Add an option to exclude symlinks from the listed files. This is useful\n> in case we are listing the files in order to process the contents,\n> for instance to do some text replacement with `sed -i`. In that case,\n> there is no point in processing the links, and it could even be\n> counterproductive as some tools (like sed) will replace the link with a\n> fresh regular file.\n> \n> This option enables a straightforward implementation of a `git sed`:\n> \n> #!/bin/bash\n> git ls-files --exclude-links -z | xargs -0 -P $(nproc) -- sed -i -e \"$@\"\n\nThis invocation would likewise have a problem with gitlink entries (for\nsubmodules). I think what you really want is not \"exclude symlinks\" but\n\"show only regular files\". There is no option for that, but you can do\nit by grepping modes from \"-s\", like:\n\n git ls-files -s | grep '^100[67]' | ...\n\nYou do unfortunately have to then pull the filename out of the rest of\nthe line, and since we didn't use \"-z\", it will be quoted (and using\n\"-z\" makes it hard to use tools like grep and sed). A mild application\nof perl works, though:\n\n git ls-files -s |\n perl -0ne 'print if s/^100(644|755).*?\\t//' |\n xargs -0 ...\n\nSo I dunno. That is not exactly pretty, but if you were hiding it in a\n\"git sed\" alias or script, it's not so bad.\n\nIf we were to add an option to ls-files, it would make more sense to me\nto allow the user to include/exclude by mode or possibly naming the type\n(e.g., \"f\" for regular files, \"l\" for symbolic links, etc, which matches\n\"find\"). And then allow inclusion/exclusion similar to the way that\ngit-diff's --diff-filter option works.\n\n-Peff\n"},{"id":"478644","messageId":"CA++DQUkM-o4_zR=oK=kF=wE5youvQ4iQrw2KyTTGhNbcUViTdg@mail.gmail.com","threadId":"59889","inReplyTo":"20230621081754.GA803861@coredump.intra.peff.net","subject":"Re: [PATCH] ls-files: add an --exclude-links option","fromName":"Guido Martínez","fromEmail":"mtzguido@gmail.com","sentAt":"2023-06-21T15:19:24Z","receivedAt":"2023-06-21T15:21:55Z","isPatch":true,"sender":{"key":"mtzguido@gmail.com","avatar":null},"body":"Hi Jeff, thanks for reviewing!\n\nOn Wed, Jun 21, 2023 at 1:17 AM Jeff King wrote:\n>\n> On Wed, Jun 21, 2023 at 06:08:04AM +0000, Guido Martínez via GitGitGadget wrote:\n>\n> > From: =?UTF-8?q?Guido=20Mart=C3=ADnez?= \n> >\n> > Add an option to exclude symlinks from the listed files. This is useful\n> > in case we are listing the files in order to process the contents,\n> > for instance to do some text replacement with `sed -i`. In that case,\n> > there is no point in processing the links, and it could even be\n> > counterproductive as some tools (like sed) will replace the link with a\n> > fresh regular file.\n> >\n> > This option enables a straightforward implementation of a `git sed`:\n> >\n> > #!/bin/bash\n> > git ls-files --exclude-links -z | xargs -0 -P $(nproc) -- sed -i -e \"$@\"\n>\n> This invocation would likewise have a problem with gitlink entries (for\n> submodules). I think what you really want is not \"exclude symlinks\" but\n> \"show only regular files\".\nAh, thanks! I hadn't realized that. I suppose my patch should also\ncome with a few tests, including one with submodules.\n\n> There is no option for that, but you can do\n> it by grepping modes from \"-s\", like:\n>\n> git ls-files -s | grep '^100[67]' | ...\n>\n> You do unfortunately have to then pull the filename out of the rest of\n> the line, and since we didn't use \"-z\", it will be quoted (and using\n> \"-z\" makes it hard to use tools like grep and sed). A mild application\n> of perl works, though:\n>\n> git ls-files -s |\n> perl -0ne 'print if s/^100(644|755).*?\\t//' |\n> xargs -0 ...\nMy perl-foo is not as strong, so I didn't realize perl could do this\nkind of thing :) Still it may be desirable for ls-files to do the\nfiltering itself?\n\n> So I dunno. That is not exactly pretty, but if you were hiding it in a\n> \"git sed\" alias or script, it's not so bad.\n>\n> If we were to add an option to ls-files, it would make more sense to me\n> to allow the user to include/exclude by mode or possibly naming the type\n> (e.g., \"f\" for regular files, \"l\" for symbolic links, etc, which matches\n> \"find\"). And then allow inclusion/exclusion similar to the way that\n> git-diff's --diff-filter option works.\nYes, this is totally reasonable. I'll try to get that working here and resubmit.\n\nThanks,\nGuido\n\n>\n> -Peff\n"},{"id":"478646","messageId":"xmqqzg4siyru.fsf@gitster.g","threadId":"59889","inReplyTo":"20230621081754.GA803861@coredump.intra.peff.net","subject":"Re: [PATCH] ls-files: add an --exclude-links option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-21T16:08:21Z","receivedAt":"2023-06-21T16:08:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> This option enables a straightforward implementation of a `git sed`:\n>> \n>> #!/bin/bash\n>> git ls-files --exclude-links -z | xargs -0 -P $(nproc) -- sed -i -e \"$@\"\n\nUnrelated nit. \n\nI think -i -e above is iffy, as it does not distribute -e across\n\"$@\" and your users may not always want to edit the files. It is\nbetter to leave them to the callers.\n\n\"sed\" is also something the caller can easily pass from their\ncommand line, for that matter ;-). Passing the entire command\npart run under xargs from the command line of the wrapper,\n\n $ git for-all-paths grep -e pattern\n\nwould also work just fine, for example.\n\n> ... A mild application of perl works, though:\n>\n> git ls-files -s -z |\n> perl -0ne 'print if s/^100(644|755).*?\\t//' |\n> xargs -0 ...\n>\n> So I dunno. That is not exactly pretty, but if you were hiding it in a\n> \"git sed\" alias or script, it's not so bad.\n\nYes, the above would be a perfectly reasonable implementation of\n\"git for-all-paths\", especially if you do not hardcode anything in\nthe ... part and instead use something like xargs -0 \"$@\" there.\n\nWhat is somewhat unsatisfactory is that we cannot pass pathspec to\nthe \"ls-files\" so that the command does not have to be for-all-paths\nbut can be usable as \"git do-for-paths -c '' \".\n"},{"id":"478651","messageId":"CA++DQUkVzz8Zn0x6BR+sAEo_LXFR67Z+JOgMhcY2JpS5UGi4rw@mail.gmail.com","threadId":"59889","inReplyTo":"xmqqzg4siyru.fsf@gitster.g","subject":"Re: [PATCH] ls-files: add an --exclude-links option","fromName":"Guido Martínez","fromEmail":"mtzguido@gmail.com","sentAt":"2023-06-21T17:54:27Z","receivedAt":"2023-06-21T17:56:12Z","isPatch":true,"sender":{"key":"mtzguido@gmail.com","avatar":null},"body":"Hi Junio, thanks for taking a look!\n\nOn Wed, Jun 21, 2023 at 9:08 AM Junio C Hamano wrote:\n>\n> Jeff King writes:\n>\n> >> This option enables a straightforward implementation of a `git sed`:\n> >>\n> >> #!/bin/bash\n> >> git ls-files --exclude-links -z | xargs -0 -P $(nproc) -- sed -i -e \"$@\"\n>\n> Unrelated nit.\n>\n> I think -i -e above is iffy, as it does not distribute -e across\n> \"$@\" and your users may not always want to edit the files. It is\n> better to leave them to the callers.\n>\n> \"sed\" is also something the caller can easily pass from their\n> command line, for that matter ;-).\n\nI should have known my one-liner would be torn to shreds :-). But yes, agreed!\n\n> Passing the entire command\n> part run under xargs from the command line of the wrapper,\n>\n> $ git for-all-paths grep -e pattern\n>\n> would also work just fine, for example.\n>\n> > ... A mild application of perl works, though:\n> >\n> > git ls-files -s -z |\n> > perl -0ne 'print if s/^100(644|755).*?\\t//' |\n> > xargs -0 ...\n> >\n> > So I dunno. That is not exactly pretty, but if you were hiding it in a\n> > \"git sed\" alias or script, it's not so bad.\n>\n> Yes, the above would be a perfectly reasonable implementation of\n> \"git for-all-paths\", especially if you do not hardcode anything in\n> the ... part and instead use something like xargs -0 \"$@\" there.\n>\n> What is somewhat unsatisfactory is that we cannot pass pathspec to\n> the \"ls-files\" so that the command does not have to be for-all-paths\n> but can be usable as \"git do-for-paths -c '' \".\n\nIndeed, a command like that would be great, and doing `git\ndo-for-paths -c \"sed -i 's/old/new/g'\"` is still extremely convenient.\nI suppose this command should take options -n and -P to pass to xargs.\nIn the case of sed, running in batches is _much_ faster than 1-by-1,\nand it trivially parallelizes.\n\nSo would it make sense to\n1- Add the file type filter to ls-files\n2- Use that to implement a proper git-do-for-paths script/binary,\nwhich can take pathspecs, filetype filters, -n, -P, and maybe more\n?\n\nThanks again!\nGuido\n"},{"id":"478658","messageId":"xmqq352kha1l.fsf@gitster.g","threadId":"59889","inReplyTo":"CA++DQUkVzz8Zn0x6BR+sAEo_LXFR67Z+JOgMhcY2JpS5UGi4rw@mail.gmail.com","subject":"Re: [PATCH] ls-files: add an --exclude-links option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-21T19:47:50Z","receivedAt":"2023-06-21T19:47:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Guido Martínez writes:\n\n> So would it make sense to\n> 1- Add the file type filter to ls-files\n\nI think it definitely makes sense to extend the filtering criteria\nsupported by ls-files beyond what we currently support (i.e.\npathspec).\n\nI also wonder if \"file type filter\" could just be implemented as the\nmagic pathspec. For example, we can already use the magic pathspec\n'attr' (read on \"pathspec\" in \"git help glossary\") this way:\n\n $ git ls-files \":(attr:eol=crlf)\"\n\nto list only those files for which the 'eol' attribute is set to\n'crlf' (i.e. they must be checked out for DOS no matter what your\nplatform actually is). That is even more flexible than the\nhardcoded \"is it a regular file? is it a symlink? is it a\nsubmodule\" file types. And the magic pathspec is understood not\neverywhere but by git subcommands other than \"ls-files\".\n\nWe can either invent a new pathspec magic \"filetype\" and express\nthem this way,\n\n $ git ls-files \":(filetype:regular)\" # 100644 and 100755\n $ git ls-files \":(filetype:symbolic-link)\" # 120000\n $ git ls-files \":(filetype:submodule)\" # 160000\n\nor we invent a magic attribute \"filetype\" that is automatically\ngiven to every path, and express the above more like so:\n\n $ git ls-files \":(attr:filetype=regular)\" # 100644 and 100755\n $ git ls-files \":(attr:filetype=symbolic-link)\" # 120000\n $ git ls-files \":(attr:filetype=submodule)\" # 160000\n\nmay be even better, as there are git subcommands other than ls-files\nthat supports magic pathspec. For example, it might be even useful\nto do something like\n\n $ git diff v1.0 v2.0 -- \":(attr:filetype=executable)\"\n\ninstead of saying\n\n $ git diff v1.0 v2.0 -- \\*.sh \\*.perl \\*.py \\*.bat\n\nSo, yeah, whether it is done via the magic pathspec or \"ls-files\"\nspecific option, teaching \"ls-files\" to support more filtering\ncriteria would make sense.\n\n> 2- Use that to implement a proper git-do-for-paths script/binary,\n> which can take pathspecs, filetype filters, -n, -P, and maybe more\n> ?\n\nThe primary obstacle was you'd need a custom perl script to filter\n\"ls-files -z\" output, but once that need is gone, I actually do not\nthink it buys us a lot to have such a wrapper. Treat the improved\nls-files as what it is, i.e. a plumbing command that can be used as\na building block of your workflow, and piping its output to xargs\nwould just be fine.\n\nThanks.\n"},{"id":"478659","messageId":"CA++DQU=5fSHcOssVD-qCiX_kpRh-Pa3TeSRMwRggLDWTK0j_XQ@mail.gmail.com","threadId":"59889","inReplyTo":"xmqq352kha1l.fsf@gitster.g","subject":"Re: [PATCH] ls-files: add an --exclude-links option","fromName":"Guido Martínez","fromEmail":"mtzguido@gmail.com","sentAt":"2023-06-21T20:02:21Z","receivedAt":"2023-06-21T20:02:39Z","isPatch":true,"sender":{"key":"mtzguido@gmail.com","avatar":null},"body":"On Wed, Jun 21, 2023 at 12:47 PM Junio C Hamano wrote:\n>\n> Guido Martínez writes:\n>\n> > So would it make sense to\n> > 1- Add the file type filter to ls-files\n>\n> I think it definitely makes sense to extend the filtering criteria\n> supported by ls-files beyond what we currently support (i.e.\n> pathspec).\n>\n> I also wonder if \"file type filter\" could just be implemented as the\n> magic pathspec. For example, we can already use the magic pathspec\n> 'attr' (read on \"pathspec\" in \"git help glossary\") this way:\n>\n> $ git ls-files \":(attr:eol=crlf)\"\n>\n> to list only those files for which the 'eol' attribute is set to\n> 'crlf' (i.e. they must be checked out for DOS no matter what your\n> platform actually is). That is even more flexible than the\n> hardcoded \"is it a regular file? is it a symlink? is it a\n> submodule\" file types. And the magic pathspec is understood not\n> everywhere but by git subcommands other than \"ls-files\".\n>\n> We can either invent a new pathspec magic \"filetype\" and express\n> them this way,\n>\n> $ git ls-files \":(filetype:regular)\" # 100644 and 100755\n> $ git ls-files \":(filetype:symbolic-link)\" # 120000\n> $ git ls-files \":(filetype:submodule)\" # 160000\n>\n> or we invent a magic attribute \"filetype\" that is automatically\n> given to every path, and express the above more like so:\n>\n> $ git ls-files \":(attr:filetype=regular)\" # 100644 and 100755\n> $ git ls-files \":(attr:filetype=symbolic-link)\" # 120000\n> $ git ls-files \":(attr:filetype=submodule)\" # 160000\n>\n> may be even better, as there are git subcommands other than ls-files\n> that supports magic pathspec. For example, it might be even useful\n> to do something like\n>\n> $ git diff v1.0 v2.0 -- \":(attr:filetype=executable)\"\n>\n> instead of saying\n>\n> $ git diff v1.0 v2.0 -- \\*.sh \\*.perl \\*.py \\*.bat\n>\n> So, yeah, whether it is done via the magic pathspec or \"ls-files\"\n> specific option, teaching \"ls-files\" to support more filtering\n> criteria would make sense.\nThanks, I was totally unaware of this magic pathspec, I'll see what it\ntakes to add attr:filetype.\n\n> > 2- Use that to implement a proper git-do-for-paths script/binary,\n> > which can take pathspecs, filetype filters, -n, -P, and maybe more\n> > ?\n>\n> The primary obstacle was you'd need a custom perl script to filter\n> \"ls-files -z\" output, but once that need is gone, I actually do not\n> think it buys us a lot to have such a wrapper. Treat the improved\n> ls-files as what it is, i.e. a plumbing command that can be used as\n> a building block of your workflow, and piping its output to xargs\n> would just be fine.\nYup, totally fine by me. I'll just keep that script locally then.\n\nThanks again,\nGuido\n\n>\n> Thanks.\n"}]}