{"thread":{"id":"55828","subject":"Request feature: –no-submodule","startedAt":"2021-06-02T10:31:29Z","lastAt":"2021-06-05T05:45:48Z","messageCount":12,"participants":["Ilias Apostolou","Taylor Blau","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"426214","messageId":"c5709df6-c357-56b4-f0e0-673bc8b4e668@gmail.com","threadId":"55828","inReplyTo":null,"subject":"Request feature: –no-submodule","fromName":"Ilias Apostolou","fromEmail":"ilias.apostolou.zero@gmail.com","sentAt":"2021-06-02T10:31:11Z","receivedAt":"2021-06-02T10:31:29Z","isPatch":false,"sender":{"key":"ilias.apostolou.zero@gmail.com","avatar":"https://gravatar.com/avatar/0aad193a02ca9571a7d74037dca67419a90cd041716272d7ffcf0571ddf8ddc5?d=mp&s=160"},"body":"Hello Git community.\n\nAs you already know, git ls-files command lists all of the tracked \nfiles, but submodule names are included.\n\nMy team would like a –no-submodule switch to exclude those.\n\nBest rewards,\nIlias Apostolou\n\nGithub: https://github.com/LiakosC\n"},{"id":"426262","messageId":"YLfqiLbpPXWXHlBi@nand.local","threadId":"55828","inReplyTo":"c5709df6-c357-56b4-f0e0-673bc8b4e668@gmail.com","subject":"Re: Request feature: –no-submodule","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-06-02T20:31:04Z","receivedAt":"2021-06-02T20:31:12Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Jun 02, 2021 at 01:31:11PM +0300, Ilias Apostolou wrote:\n> Hello Git community.\n>\n> As you already know, git ls-files command lists all of the tracked files,\n> but submodule names are included.\n>\n> My team would like a –no-submodule switch to exclude those.\n\nIn all honesty, though this seems like a niche request for ls-files to\nfulfill, ls-files already has quite the collection of options, so I\nwouldn't be sad to see it learn how to do this, too.\n\nThe change boils down to having builtin/ls-files.c:show_ce() avoid\nprinting out submoudles in when '--exclude-submoudles' is given).\nSomething like this:\n\n--- >8 ---\n\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 45cc3b23dd..09d76fd068 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -35,6 +35,7 @@ static int show_fsmonitor_bit;\n static int line_terminator = '\\n';\n static int debug_mode;\n static int show_eol;\n+static int exclude_submodules;\n static int recurse_submodules;\n static int skipping_duplicates;\n\n@@ -230,6 +231,9 @@ static void show_ce(struct repository *repo, struct dir_struct *dir,\n \tif (max_prefix_len > strlen(fullname))\n \t\tdie(\"git ls-files: internal error - cache entry not superset of prefix\");\n\n+\tif (exclude_submodules && S_ISGITLINK(ce->ce_mode))\n+\t\treturn;\n+\n \tif (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&\n \t is_submodule_active(repo, ce->name)) {\n \t\tshow_submodule(repo, dir, ce->name);\n@@ -662,6 +666,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\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+\t\tOPT_BOOL(0, \"exclude-submodules\", &exclude_submodules,\n+\t\t\tN_(\"avoid showing submodules altogether\")),\n \t\tOPT_BOOL(0, \"recurse-submodules\", &recurse_submodules,\n \t\t\tN_(\"recurse through submodules\")),\n \t\tOPT_BOOL(0, \"error-unmatch\", &error_unmatch,\n\n--- 8< ---\n\nI'll leave it at that to decide if others think this is a good idea or\nnot.\n\nThanks,\nTaylor\n"},{"id":"426278","messageId":"xmqq8s3r7oma.fsf@gitster.g","threadId":"55828","inReplyTo":"YLfqiLbpPXWXHlBi@nand.local","subject":"Re: Request feature: –no-submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-03T00:55:57Z","receivedAt":"2021-06-03T00:56:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> On Wed, Jun 02, 2021 at 01:31:11PM +0300, Ilias Apostolou wrote:\n>> Hello Git community.\n>>\n>> As you already know, git ls-files command lists all of the tracked files,\n>> but submodule names are included.\n>>\n>> My team would like a –no-submodule switch to exclude those.\n>\n> In all honesty, though this seems like a niche request for ls-files to\n> fulfill, ls-files already has quite the collection of options, so I\n> wouldn't be sad to see it learn how to do this, too.\n\nI would be somewhat sad for two reasons.\n\n - If \"I am not interested in any submodule\" in a project with\n submodules is a common thing people would want, teaching a trick\n only to \"ls-files\" is an expensive and ineffective approach, and\n adding the option to everything would just be ugly. \"git diff\n --no-submodule\"? \"git add --no-submodule .\"?\n\n - Is \"not interested in any submodule\" so special and fundamental,\n or is it merely because the project the original requestor is\n looking at happens to have an optional submodule? If the project\n had that optional part as a subdirectory instead, would the\n request have been not --no-submodule but something else? What\n happens when the project that led to the original request\n acquires another submodule that is more interesting, or what if\n the requestor's interest shifts and makes some submodules\n interesting but others not? Would the --no-submodule option\n become totally useless in such a case?\n\nI wonder if the \"attr\" magic of the pathspec, that allows you to\nchoose paths based on the attributes you set on them, is what the\noriginal requestor missed.\n\n"},{"id":"426283","messageId":"YLg/dLqYFEqHZJUn@nand.local","threadId":"55828","inReplyTo":"xmqq8s3r7oma.fsf@gitster.g","subject":"Re: Request feature: –no-submodule","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-06-03T02:33:24Z","receivedAt":"2021-06-03T02:33:43Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Jun 03, 2021 at 09:55:57AM +0900, Junio C Hamano wrote:\n> Taylor Blau writes:\n>\n> > In all honesty, though this seems like a niche request for ls-files to\n> > fulfill, ls-files already has quite the collection of options, so I\n> > wouldn't be sad to see it learn how to do this, too.\n>\n> I would be somewhat sad for two reasons.\n\nMmm, that's a convincing set of reasons to think that this is a bad\nidea. (And I was sort of on the fence about it anyway by posting the\nwhole thing as a short diff instead of a polished patch).\n\n> I wonder if the \"attr\" magic of the pathspec, that allows you to\n> choose paths based on the attributes you set on them, is what the\n> original requestor missed.\n\nMaybe... but relying on the attr magic for this particular case would\nforce the requester to set that attribute on all submodules in their\nproject, and constantly keep that in-sync with their .gitmodules. So, it\ncertainly make it easier to handle a request like \"I don't care about\nls-files telling me about any submodule(s) except these ones\", but\nperhaps at the expense of some extra bookkeeping.\n\nI might be missing something, though.\n\nThanks,\nTaylor\n"},{"id":"426318","messageId":"9cc98ca3-bdc5-61bf-450a-99bb47673d6c@gmail.com","threadId":"55828","inReplyTo":"YLg/dLqYFEqHZJUn@nand.local","subject":"Re: Request feature: –no-submodule","fromName":"Ilias Apostolou","fromEmail":"ilias.apostolou.zero@gmail.com","sentAt":"2021-06-03T10:48:39Z","receivedAt":"2021-06-03T10:48:46Z","isPatch":false,"sender":{"key":"ilias.apostolou.zero@gmail.com","avatar":"https://gravatar.com/avatar/0aad193a02ca9571a7d74037dca67419a90cd041716272d7ffcf0571ddf8ddc5?d=mp&s=160"},"body":"The reason we need to list all of the true files (except submodules) is \nfor code refactoring using the sed unility, for example:\n\ngit ls-files | grep -Ev '(png$|ico$)' | xargs sed -i 's/\\r\\n/\\n/'\n\nAll of the other alternatives we could think of are very ugly.\n\nOn 3/6/2021 5:33 π.μ., Taylor Blau wrote:\n> On Thu, Jun 03, 2021 at 09:55:57AM +0900, Junio C Hamano wrote:\n>> Taylor Blau writes:\n>>\n>>> In all honesty, though this seems like a niche request for ls-files to\n>>> fulfill, ls-files already has quite the collection of options, so I\n>>> wouldn't be sad to see it learn how to do this, too.\n>> I would be somewhat sad for two reasons.\n\nIn my opinion, this should be a feature for \"ls-files\" only, since it \nwould be nice to have a clean stream of true files.\n\nThank you for your replies,\nIlias\n\n"},{"id":"426335","messageId":"xmqq7dja6e46.fsf@gitster.g","threadId":"55828","inReplyTo":"YLg/dLqYFEqHZJUn@nand.local","subject":"Re: Request feature: –no-submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-03T17:40:25Z","receivedAt":"2021-06-03T17:40:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> Mmm, that's a convincing set of reasons to think that this is a bad\n> idea. (And I was sort of on the fence about it anyway by posting the\n> whole thing as a short diff instead of a polished patch).\n>\n>> I wonder if the \"attr\" magic of the pathspec, that allows you to\n>> choose paths based on the attributes you set on them, is what the\n>> original requestor missed.\n>\n> Maybe... but relying on the attr magic for this particular case would\n> force the requester to set that attribute on all submodules in their\n> project, and constantly keep that in-sync with their .gitmodules.\n\nWell, it contradicts with the above \"convincing\" adjective, and\nshows that you are not convinced that \"submodule\"-ness is not all\nthat essential and it merely is an artifact that the paths that the\noriginal requester happens to be uninterested in are all submodules.\n\nBut if we agree that focusing too narrowly on \"submodule\"-ness is a\nbad idea and open our mind to elsewhere, we'd realize that once we\nlearn that we can \"mark\" any path with attributes and use that in\nmagic pathspec, we can mark not just submodules but a subdirectory\nas \"uninteresting\", which will not become useless even when it turns\nout that \"submodule\"-ness wasn't really what the request was about.\n\nBesides, you can iterate over the available submodules with \"git\nsubmodule foreach\" fairly mechanically, and maintaining the\nattribute per path shouldn't be all that hard, I would imagine.\n"},{"id":"426346","messageId":"YLkr+kLdjmHyPjgl@coredump.intra.peff.net","threadId":"55828","inReplyTo":"xmqq7dja6e46.fsf@gitster.g","subject":"Re: Request feature: –no-submodule","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-03T19:22:34Z","receivedAt":"2021-06-03T19:22:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 04, 2021 at 02:40:25AM +0900, Junio C Hamano wrote:\n\n> Besides, you can iterate over the available submodules with \"git\n> submodule foreach\" fairly mechanically, and maintaining the\n> attribute per path shouldn't be all that hard, I would imagine.\n\nIt doesn't seem outrageous to me for Git to automatically populate\n\"pseudo-attributes\" that connect properties of paths to the attribute\nsystem. I.e., could we just act as if every path that is a gitlink has\nthe \"gitlink\" attribute set to true, and let people do:\n\n git ls-files ':(attr:-gitlink)'\n\nThat uses the existing generic mechanism, so it supports complex\nsituations, but it also makes the \"easy\" case of \"just ignore\nsubmodules\" easy, with no attribute maintenance.\n\nI didn't look at the code, though, so I'm not sure how awkward it would\nbe to implement (usually we decide on attributes only from looking at\nthe attribute files, not the trees/index themselves, but I think most\ncode asking about a path would be iterating a list of files in the first\nplace, and could feed the mode information).\n\nAn alternative view is allowing a pathspec that asks about the mode:\n\n git ls-files ':(mode=160000)'\n\nThat also lets you ask about other things, like:\n\n git ls-files ':(mode=100755)'\n\nbut it is probably unnecessarily arcane (even I had to look up the\ncorrect mode for a gitlink just now :) ).\n\n-Peff\n"},{"id":"426364","messageId":"xmqqo8cm4nst.fsf@gitster.g","threadId":"55828","inReplyTo":"YLkr+kLdjmHyPjgl@coredump.intra.peff.net","subject":"Re: Request feature: –no-submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-03T21:54:10Z","receivedAt":"2021-06-03T21:54:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> It doesn't seem outrageous to me for Git to automatically populate\n> \"pseudo-attributes\" that connect properties of paths to the attribute\n> system.\n\nSounds sensible. The attribute assignment was designed to purely\ndepend on paths and not contents, so it might be a bit awkwarad to\nimplement, but it should be doable.\n\n> An alternative view is allowing a pathspec that asks about the mode:\n>\n> git ls-files ':(mode=160000)'\n>\n> That also lets you ask about other things, like:\n>\n> git ls-files ':(mode=100755)'\n>\n> but it is probably unnecessarily arcane (even I had to look up the\n> correct mode for a gitlink just now :) ).\n\nThe original request, as I understand the clarification posted\nupthread, is not \"submodules are uninteresting\", but is \"we want\nregular files\" (and we postprocess further the output), so such a\n\"mode\" (pseudo-)attribute that is automatically populated would be a\nbetter fit anyway. With the current system, they can already do:\n\n git ls-files -s ':(exclude)*.png' ':(exclude)*.ico)' |\n sed -n -e 's/^100[76][54][54] [0-9a-f]* 0 //p' |\n xargs dos2unix\n\n(cf. <9cc98ca3-bdc5-61bf-450a-99bb47673d6c@gmail.com>)\n\nand with such an auto-pseudo-attribute, presumably something along\nthis line would work, removing the need for the intermediate filter:\n\n git ls-files \\\n\t':(attr:mode=100755)' ':(attr:mode=100644)' \\\n\t':(exclude)*.png' ':(exclude)*.ico' |\n xargs dos2unix\n\n"},{"id":"426365","messageId":"xmqqim2u4n7n.fsf@gitster.g","threadId":"55828","inReplyTo":"9cc98ca3-bdc5-61bf-450a-99bb47673d6c@gmail.com","subject":"Re: Request feature: –no-submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-03T22:06:52Z","receivedAt":"2021-06-03T22:06:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ilias Apostolou writes:\n\n> The reason we need to list all of the true files (except submodules)\n> is for code refactoring using the sed unility, for example:\n>\n> git ls-files | grep -Ev '(png$|ico$)' | xargs sed -i 's/\\r\\n/\\n/'\n> ...\n> In my opinion, this should be a feature for \"ls-files\" only, since it\n> would be nice to have a clean stream of true files.\n\nAh, then pathspec is still the right tool to use, I would think.\n\nFor example,\n\n git ls-files -s ':(exclude)*.png' ':(exclude)*.ico' |\n sed -n -e 's/^100[76][54][54] [0-9a-f]* 0 //p' |\n xargs sed -i 's/\\r\\n/\\n/'\n\nthat is,\n\n (1) ls-files -s can be used to learn what kind of path it is.\n Regular files are either 100644 or 100755. That way, you can\n also exclude symbolic links, which your example use case would\n probably not want to touch. And you can filter the output to\n have 'a clean stream of true files' fairly easily Depending on\n the details of your needs, it can be tweaked into 'a clean\n stream of executable files' etc., too.\n\n (2) pathspec magic like ':(exclude)' can be used to lose your \"we\n know png and ico are not text files\".\n\nHope this helps.\n"},{"id":"426387","messageId":"YLmmIXy/SITKh/Yw@coredump.intra.peff.net","threadId":"55828","inReplyTo":"xmqqo8cm4nst.fsf@gitster.g","subject":"Re: Request feature: –no-submodule","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-04T04:03:45Z","receivedAt":"2021-06-04T04:03:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 04, 2021 at 06:54:10AM +0900, Junio C Hamano wrote:\n\n> > An alternative view is allowing a pathspec that asks about the mode:\n> >\n> > git ls-files ':(mode=160000)'\n> >\n> > That also lets you ask about other things, like:\n> >\n> > git ls-files ':(mode=100755)'\n> >\n> > but it is probably unnecessarily arcane (even I had to look up the\n> > correct mode for a gitlink just now :) ).\n> \n> The original request, as I understand the clarification posted\n> upthread, is not \"submodules are uninteresting\", but is \"we want\n> regular files\" (and we postprocess further the output), so such a\n> \"mode\" (pseudo-)attribute that is automatically populated would be a\n> better fit anyway. With the current system, they can already do:\n> \n> git ls-files -s ':(exclude)*.png' ':(exclude)*.ico)' |\n> sed -n -e 's/^100[76][54][54] [0-9a-f]* 0 //p' |\n> xargs dos2unix\n> \n> (cf. <9cc98ca3-bdc5-61bf-450a-99bb47673d6c@gmail.com>)\n> \n> and with such an auto-pseudo-attribute, presumably something along\n> this line would work, removing the need for the intermediate filter:\n> \n> git ls-files \\\n> \t':(attr:mode=100755)' ':(attr:mode=100644)' \\\n> \t':(exclude)*.png' ':(exclude)*.ico' |\n> xargs dos2unix\n> \n\nYeah, that makes sense.\n\nBy the way, another reason (beyond a simpler pipeline) that the \"magic\npathspec that understands modes\" is nicer is that it can be applied to a\nmore dynamic set of paths.\n\nFor instance, you could use a pipeline like the one you showed above to\nlimit the ls-files output, and then you could feed that set of literal\npaths to a command like \"git add\" (perhaps with --literal-pathspecs).\nBut you would not want to feed it to git-log like:\n\n git --literal-pathspecs log $(git ls-files -s | sed ...)\n\nbecause you'd really want to expand the set of paths at each commit, not\nonce based on the current index (i.e., it would fail to match paths that\nwere removed or changed modes).\n\nThis is kind of a subtle and esoteric point, but it makes me more\nconvinced that having powerful pathspec selectors is potentially useful.\nOr at least solving a problem that's hard to otherwise solve correctly. :)\n\n-Peff\n"},{"id":"426394","messageId":"xmqqbl8m2p71.fsf@gitster.g","threadId":"55828","inReplyTo":"YLmmIXy/SITKh/Yw@coredump.intra.peff.net","subject":"Re: Request feature: –no-submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-04T05:06:58Z","receivedAt":"2021-06-04T05:07:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">> The original request, as I understand the clarification posted\n>> ...\n>> (cf. <9cc98ca3-bdc5-61bf-450a-99bb47673d6c@gmail.com>)\n>> \n>> and with such an auto-pseudo-attribute, presumably something along\n>> this line would work, removing the need for the intermediate filter:\n>> \n>> git ls-files \\\n>> \t':(attr:mode=100755)' ':(attr:mode=100644)' \\\n>> \t':(exclude)*.png' ':(exclude)*.ico' |\n>> xargs dos2unix\n>> \n>\n> Yeah, that makes sense.\n>\n> By the way, another reason (beyond a simpler pipeline) that the \"magic\n> pathspec that understands modes\" is nicer is that it can be applied to a\n> more dynamic set of paths.\n\nIn the longer term, the project the original request wanted to\ninvent the \"--no-submodule\" option for may want to lose the specific\n\"we know that the only paths we do not want to run dos2unix happen\nto be png and ico files in the current codebase\" from the above\nsample command line, and replace it with something like ':(attr:text)'\n\nAlas, the \"text\" attribute does *not* work that way, though ;-)\nJust like any other attributes, what you assigned yourself counts,\nand our \"is this file a text?\" auto-detection logic only kicks in\nwhen there is no attribute that tells if a path is text or not.\n\nIt would be expensive at runtime; even if we were to introduce a\n\"dynamic\" pseudo attribute to tell text files and others apart, we\nprobably shouldn't use the same \"attr:*\" magic but use something\ndistinct (e.g. \"dynamic-attr:*\") in order to make sure that the\nusers understand the performance implications. I think the above\n\"mode=100755\" matic (or anything that requires more than the basic\n\"which entries in the .gitattributes files does this pathname\nmatch?\") would fall into the same category.\n\n"},{"id":"426470","messageId":"d75f856e-f219-3d15-e94b-837fb2355df9@gmail.com","threadId":"55828","inReplyTo":"xmqqbl8m2p71.fsf@gitster.g","subject":"Re: Request feature: –no-submodule","fromName":"Ilias Apostolou","fromEmail":"ilias.apostolou.zero@gmail.com","sentAt":"2021-06-05T05:45:18Z","receivedAt":"2021-06-05T05:45:48Z","isPatch":false,"sender":{"key":"ilias.apostolou.zero@gmail.com","avatar":"https://gravatar.com/avatar/0aad193a02ca9571a7d74037dca67419a90cd041716272d7ffcf0571ddf8ddc5?d=mp&s=160"},"body":"If I understand correctly, we should learn to use git ls-files special \nparameters like 'attr' and 'exclude'. I'm impressed by the fact that \nthese are not wide-spread known. We should learn to use these advanced \nfeatures instead of extra piping. Your emails are precious to my team \nand will help us advance.\n\nPlease consider this request closed.\nIlias\n\n"}]}