{"thread":{"id":"42293","subject":"proposal for extending smudge/clean filters with raw file access","startedAt":"2016-05-12T18:24:32Z","lastAt":"2016-05-12T21:17:52Z","messageCount":7,"participants":["Joey Hess","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"286347","messageId":"20160512182432.GA27427@kitenet.net","threadId":"42293","inReplyTo":null,"subject":"proposal for extending smudge/clean filters with raw file access","fromName":"Joey Hess","fromEmail":"id@joeyh.name","sentAt":"2016-05-12T18:24:32Z","receivedAt":"2016-05-12T18:24:32Z","isPatch":false,"sender":{"key":"id@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"I'm using smudge/clean filters in git-annex now, and it's not been an\nentirely smooth fit between the interface and what git-annex wants\nto do.\n\nThe clean filter has to consume the whole file content on stdin;\nnot reading it all will make git think the clean filter failed.\nBut, git-annex often doesn't need to read the whole content of a\nwork-tree file in order to clean it.\n\nThe smudge filter has to output the whole file content to stdout. But\ngit-annex often has the file's content on disk already, and could just\nmove it into place in the working tree. This would save CPU and IO and\noften disk space too. But the smudge interface doesn't let git-annex use\nthe efficient approach.\n\nSo I propose extending the filter driver with two more optional\ncommands. Call them raw-clean and raw-smudge for now.\n\nraw-clean would be like clean, but rather than being fed the whole\ncontent of a large file on stdin, it would be passed the filename, and\ncan access the file itself. Like the clean filter, it outputs the\ncleaned version on stdout.\n\nraw-smudge would be like smudge, but rather than needing to output the\nwhole content of a large file on stdout, it would be passed a filename,\nand can create that file itself.\n\nTo keep this backwards compatible, and to handle the cases where the\nobject being filtered is not a file on disk, the smudge and clean\nfilters would be required to be configured too, in order for raw-clean\nand raw-smudge to be used.\n\nIt seems fairly easy to implement raw-clean. In sha1_file.c, index_path\nwould use raw-clean when available, while index_fd etc keep on using\nthe clean filter. I have not investigated what would be needed to implement\nraw-smudge yet.\n\n-- \nsee shy jo\n"},{"id":"286348","messageId":"xmqqvb2jf6dg.fsf@gitster.mtv.corp.google.com","threadId":"42293","inReplyTo":"20160512182432.GA27427@kitenet.net","subject":"Re: proposal for extending smudge/clean filters with raw file access","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-12T19:01:15Z","receivedAt":"2016-05-12T19:01:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joey Hess <id@joeyh.name> writes:\n\n> I'm using smudge/clean filters in git-annex now, and it's not been an\n> entirely smooth fit between the interface and what git-annex wants\n> to do.\n>\n> The clean filter has to consume the whole file content on stdin;\n> not reading it all will make git think the clean filter failed.\n> But, git-annex often doesn't need to read the whole content of a\n> work-tree file in order to clean it.\n>\n> The smudge filter has to output the whole file content to stdout. But\n> git-annex often has the file's content on disk already, and could just\n> move it into place in the working tree. This would save CPU and IO and\n> often disk space too. But the smudge interface doesn't let git-annex use\n> the efficient approach.\n\nThe smudge happens to be the last to run, so it is quite true that\nit can say \"Hey Git, I've written it out already\".\n\nI didn't check all codepaths to ensure that we won't need the\nsmudged result in core at all, but I am guessing you did so before\nmaking this proposal, so in that case I would say this sounds fine.\n"},{"id":"286371","messageId":"xmqq7fezdoe7.fsf@gitster.mtv.corp.google.com","threadId":"42293","inReplyTo":"20160512182432.GA27427@kitenet.net","subject":"Re: proposal for extending smudge/clean filters with raw file access","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-12T20:14:56Z","receivedAt":"2016-05-12T20:14:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joey Hess <id@joeyh.name> writes:\n\n> The clean filter has to consume the whole file content on stdin;\n> not reading it all will make git think the clean filter failed.\n> But, git-annex often doesn't need to read the whole content of a\n> work-tree file in order to clean it.\n\nThis side, I do not think we even need a new variant.  We can just\nupdate the code to interact with \"clean\" so that it the writer to\nthe pipe ignores SIGPIPE, detects EPIPE on write(2), says \"ah, the\nother end does not need the full input to produce its output\".  The\nreader from the pipe consumes its output without failing AS LONG AS\nthe \"clean\" filter exits with zero (we do check its exit status,\nright?)\n\nAnd I think we should do the same for any codepath that spawns\ncustom script and feeds it via a pipe from us (I am talking about\nhooks here).  \n\nWhat may require a new variant is when your clean filter may not\neven need earlier contents of the file, in which case we are better\noff not opening the file ourselves and slurping it into core, only\nto pipe it and earlier part discarded by the filter.  \"clean-from-fs\"\nfilter that gets a path on the working tree and feeds us via pipe\nwould be appropriate to deal with such a requirement.\n\n> The smudge filter has to output the whole file content to stdout.\n\nWe cannot do a similar \"we can just unconditionally update\" like I\nsaid on the \"clean\" side to \"smudge\", so it would need a new\nvariant.  I do not want to call it anything \"raw\", as there is\nnothing \"raw\" about it.  \"smudge-to-fs\" would be a better name.\n"},{"id":"286396","messageId":"20160512204634.GA9292@kitenet.net","threadId":"42293","inReplyTo":"xmqq7fezdoe7.fsf@gitster.mtv.corp.google.com","subject":"Re: proposal for extending smudge/clean filters with raw file access","fromName":"Joey Hess","fromEmail":"id@joeyh.name","sentAt":"2016-05-12T20:46:34Z","receivedAt":"2016-05-12T20:46:34Z","isPatch":false,"sender":{"key":"id@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Junio C Hamano wrote:\n> This side, I do not think we even need a new variant.  We can just\n> update the code to interact with \"clean\" so that it the writer to\n> the pipe ignores SIGPIPE, detects EPIPE on write(2), says \"ah, the\n> other end does not need the full input to produce its output\".  The\n> reader from the pipe consumes its output without failing AS LONG AS\n> the \"clean\" filter exits with zero (we do check its exit status,\n> right?)\n\nThere are two problems with doing that. First, any clean filter that\nrelied on that would not work with older versions of git.\n\nSecondly, and harder to get around, the filename passed to the clean\nfilter is not necessarily a path to the actual existing file that is\nbeing cleaned. For example, git hash-object --stdin --path=whatever.\nSo the current clean filter can't really safely rely on accessing the\nfile to short-circuit its cleaning. (Although it seems to mostly work..\ncurrently..)\n\n> We cannot do a similar \"we can just unconditionally update\" like I\n> said on the \"clean\" side to \"smudge\", so it would need a new\n> variant.  I do not want to call it anything \"raw\", as there is\n> nothing \"raw\" about it.  \"smudge-to-fs\" would be a better name.\n\n\"raw\" was just a placeholder. \"clean-from-fs\" and \"smudge-to-fs\" are\npretty descriptive.\n\n-- \nsee shy jo\n"},{"id":"286397","messageId":"xmqqy47fc7y7.fsf@gitster.mtv.corp.google.com","threadId":"42293","inReplyTo":"20160512204634.GA9292@kitenet.net","subject":"Re: proposal for extending smudge/clean filters with raw file access","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-12T20:55:28Z","receivedAt":"2016-05-12T20:55:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joey Hess <id@joeyh.name> writes:\n\n> Junio C Hamano wrote:\n>> This side, I do not think we even need a new variant.  We can just\n>> update the code to interact with \"clean\" so that it the writer to\n>> the pipe ignores SIGPIPE, detects EPIPE on write(2), says \"ah, the\n>> other end does not need the full input to produce its output\".  The\n>> reader from the pipe consumes its output without failing AS LONG AS\n>> the \"clean\" filter exits with zero (we do check its exit status,\n>> right?)\n>\n> There are two problems with doing that. First, any clean filter that\n> relied on that would not work with older versions of git.\n\nThat's a fair point.\n\n> Secondly, and harder to get around, the filename passed to the clean\n> filter is not necessarily a path to the actual existing file that is\n> being cleaned.\n\nEither one of us is confused.  I was talking about updating the\ncurrent \"clean\" implementation without changing its interface,\ni.e. gets fed via its standard input, expected to respond to its\nstandard output.  There is no filename involved.\n\nAnd yes, \"clean-from-fs\" that is spawned by us with the name of the\nfile in the filesystem that has the contents as its argument, must\nbe prepared to see a filename that does not have any relation to the\nfilename in the working tree.\n"},{"id":"286398","messageId":"20160512210227.GA10786@kitenet.net","threadId":"42293","inReplyTo":"xmqqvb2jf6dg.fsf@gitster.mtv.corp.google.com","subject":"Re: proposal for extending smudge/clean filters with raw file access","fromName":"Joey Hess","fromEmail":"id@joeyh.name","sentAt":"2016-05-12T21:02:27Z","receivedAt":"2016-05-12T21:02:27Z","isPatch":false,"sender":{"key":"id@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Junio C Hamano wrote:\n> The smudge happens to be the last to run, so it is quite true that\n> it can say \"Hey Git, I've written it out already\".\n> \n> I didn't check all codepaths to ensure that we won't need the\n> smudged result in core at all, but I am guessing you did so before\n> making this proposal, so in that case I would say this sounds fine.\n\nWell, the idea is to only use smudge-to-file when the smudged content is\ngoing to be written out to a file. Any other code paths that need to\nsmudge some content would use the smudge filter.\n\nSo, try_create_file would use it. Maybe some other places I have not\nfound yet could as well.\n\n-- \nsee shy jo\n"},{"id":"286402","messageId":"20160512211752.GB10786@kitenet.net","threadId":"42293","inReplyTo":"xmqqy47fc7y7.fsf@gitster.mtv.corp.google.com","subject":"Re: proposal for extending smudge/clean filters with raw file access","fromName":"Joey Hess","fromEmail":"id@joeyh.name","sentAt":"2016-05-12T21:17:52Z","receivedAt":"2016-05-12T21:17:52Z","isPatch":false,"sender":{"key":"id@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Junio C Hamano wrote:\n> > Secondly, and harder to get around, the filename passed to the clean\n> > filter is not necessarily a path to the actual existing file that is\n> > being cleaned.\n> \n> Either one of us is confused.  I was talking about updating the\n> current \"clean\" implementation without changing its interface,\n> i.e. gets fed via its standard input, expected to respond to its\n> standard output.  There is no filename involved.\n\nI'm talking about the %f that can be passed to the clean filter.\nThe context that I left out is that my clean filter could avoid reading\nall of stdin, and quickly produce the cleaned result, but only if it\ncan examine the file that's being cleaned. Which is not currently\nentirely safe to use the %f for.\n\nThere may be a way to make a clean filter that can do something useful\nwithout reading all of stdin, and without examining the file that's\nbeing cleaned. Maybe. Hard to see how. I don't feel such a hypothetical\nclean filter is worth changing the current EPIPE behavior to support.\n\nSo I think it's better to add a separate clean-from-fs and keep the\ncurrent clean filter interface as it stands.\n\n-- \nsee shy jo\n"}]}