{"thread":{"id":"51998","subject":"git cat-file --batch surprising carriage return behavior","startedAt":"2019-10-08T19:30:16Z","lastAt":"2019-10-11T06:21:39Z","messageCount":4,"participants":["Joey Hess","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"383702","messageId":"20191008192257.GA16870@kitenet.net","threadId":"51998","inReplyTo":null,"subject":"git cat-file --batch surprising carriage return behavior","fromName":"Joey Hess","fromEmail":"id@joeyh.name","sentAt":"2019-10-08T19:22:57Z","receivedAt":"2019-10-08T19:30:16Z","isPatch":false,"sender":{"key":"id@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"I'm surprised to find that git cat-file --batch, on a Linux system,\nstrips the \\r from an input like \"HEAD:foo\\r\\n\"\n\nIt's obvious, of course, that it will remove the newline, and so this\ninterface cannot be used to query about a filename that, for some\nhorrible reason[1], contains a newline. But very surprising that it\ncannot be used for filename that contains a carriage return, at least\non a non-Windows system.\n\nThe docs for cat-file --batch say the list of objects is separated by\nlinefeeds. I don't know if updating the docs is the best fix.\n(I'd be happy to use a -z if it had one.)\n\n-- \nsee shy jo\n\n[1] aka \"a large enough number of monkeys\"\n"},{"id":"383709","messageId":"20191008200050.GA26453@sigill.intra.peff.net","threadId":"51998","inReplyTo":"20191008192257.GA16870@kitenet.net","subject":"Re: git cat-file --batch surprising carriage return behavior","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-10-08T20:00:50Z","receivedAt":"2019-10-08T20:00:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 08, 2019 at 03:22:57PM -0400, Joey Hess wrote:\n\n> I'm surprised to find that git cat-file --batch, on a Linux system,\n> strips the \\r from an input like \"HEAD:foo\\r\\n\"\n> \n> It's obvious, of course, that it will remove the newline, and so this\n> interface cannot be used to query about a filename that, for some\n> horrible reason[1], contains a newline. But very surprising that it\n> cannot be used for filename that contains a carriage return, at least\n> on a non-Windows system.\n\nThis is likely due to b42ca3dd0f (cat-file: read batch stream with\nstrbuf_getline(), 2015-10-28), and the matching c8aa9fdf5d (strbuf: make\nstrbuf_getline_crlf() global, 2015-10-28).\n\nI agree it's a bit surprising (though OTOH, I imagine the old behavior\nsurprised some people in the opposite direction).\n\n> The docs for cat-file --batch say the list of objects is separated by\n> linefeeds. I don't know if updating the docs is the best fix.\n> (I'd be happy to use a -z if it had one.)\n\nYeah, I agree that a -z option is the best path forward. For non-z\ninput, I'm tempted to say we could unquote entries that start with a\ndouble-quote (the match to how we handle filenames in non-z diff\noutput). That would mean breaking compatibility for refnames that start\nwith a quote, though. If we just add a new \"-z\", that's less disruptive\n_and_ easier to use.\n\nI suspect it's not entirely sufficient for clean input, though. You're\nnot feeding filenames but rather full \"object names\". I wouldn't be\nsurprised if we mis-parse \"$rev:$path\" when $path has \"@{}\" or similar\nin it.\n\nSo what you may actually want is some more robust input format that lets\nyou specify the filename as an independent NUL-terminated entity.\n\n-Peff\n"},{"id":"383755","messageId":"20191009152851.GC19679@kitenet.net","threadId":"51998","inReplyTo":"20191008200050.GA26453@sigill.intra.peff.net","subject":"Re: git cat-file --batch surprising carriage return behavior","fromName":"Joey Hess","fromEmail":"id@joeyh.name","sentAt":"2019-10-09T15:28:51Z","receivedAt":"2019-10-09T15:29:04Z","isPatch":false,"sender":{"key":"id@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Jeff King wrote:\n> If we just add a new \"-z\", that's less disruptive_and_ easier to use.\n\nAgreed. \n\n> I suspect it's not entirely sufficient for clean input, though. You're\n> not feeding filenames but rather full \"object names\". I wouldn't be\n> surprised if we mis-parse \"$rev:$path\" when $path has \"@{}\" or similar\n> in it.\n\nNothing I've tried along the lines of \"HEAD:{yesterday}\" has misparsed\nthe part after the colon as anything but a filename.\n\nThe one I can think of where there's a parse ambiguity is that while\n:foo gets file foo, :1:foo does not get file \"1:foo\". Instead it's\ntreated as a stage number. Using either HEAD:1:foo or :./1:foo\nwill avoid that ambiguity.\n\n-- \nsee shy jo\n"},{"id":"383878","messageId":"20191011062136.GA25741@sigill.intra.peff.net","threadId":"51998","inReplyTo":"20191009152851.GC19679@kitenet.net","subject":"Re: git cat-file --batch surprising carriage return behavior","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-10-11T06:21:37Z","receivedAt":"2019-10-11T06:21:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 09, 2019 at 11:28:51AM -0400, Joey Hess wrote:\n\n> > I suspect it's not entirely sufficient for clean input, though. You're\n> > not feeding filenames but rather full \"object names\". I wouldn't be\n> > surprised if we mis-parse \"$rev:$path\" when $path has \"@{}\" or similar\n> > in it.\n> \n> Nothing I've tried along the lines of \"HEAD:{yesterday}\" has misparsed\n> the part after the colon as anything but a filename.\n\nIt's possible we've fixed them all. We definitely don't parse strictly\nleft-to-right. The first thing we try to do is strip bits like ^{commit}\noff the end, before we even find the colon. But after doing so, we\nshould generally be left with a resolvable name, and I think that uses\nthe \"basic\" parser which will not allow colons. I.e., this:\n\n  mkdir subdir\n  echo whatever >subdir/file\n  git add subdir\n  git commit -m 'add directory'\n  git show HEAD:subdir^{tree}\n\nreally does look for the file \"subdir^{tree}\" in HEAD, and not\n\"HEAD:subdir\" as a tree.\n\nWe have had bugs in the past; I'm thinking specifically of 8cd4249c4c\n(interpret_branch_name: always respect \"namelen\" parameter, 2014-01-15).\nBut I couldn't find any problematic inputs after poking around for a few\nminutes.\n\n> The one I can think of where there's a parse ambiguity is that while\n> :foo gets file foo, :1:foo does not get file \"1:foo\". Instead it's\n> treated as a stage number. Using either HEAD:1:foo or :./1:foo\n> will avoid that ambiguity.\n\nYeah, that makes sense.\n\n-Peff\n"}]}