{"thread":{"id":"7660","subject":"Expose subprojects as special files to \"git diff\" machinery","startedAt":"2007-04-15T18:14:28Z","lastAt":"2007-04-19T08:28:30Z","messageCount":12,"participants":["Linus Torvalds","Sam Ravnborg","Andy Parkins","Sam Vilain","Alex Riesen","Johannes Schindelin","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"39377","messageId":"Pine.LNX.4.64.0704151100550.5473@woody.linux-foundation.org","threadId":"7660","inReplyTo":null,"subject":"Expose subprojects as special files to \"git diff\" machinery","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-15T18:14:28Z","receivedAt":"2007-04-15T18:14:28Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nThe same way we generate diffs on symlinks as the the diff of text of the \nsymlink, we can generate subproject diffs (when not recursing into them!) \nas the diff of the text that describes the subproject.\n\nOf course, since what descibes a subproject is just the SHA1, that's what \nwe'll use. Add some pretty-printing to make it a bit more obvious what is \ngoing on, and we're done.\n\nSo with this, we can get both raw diffs and \"textual\" diffs of subproject \nchanges:\n\n - git diff --raw:\n\n\t:160000 160000 2de597b5ad348b7db04bd10cdd38cd81cbc93ab5 0000000... M sub-A\n\n\n - git diff:\n\n\tdiff --git a/sub-A b/sub-A\n\tindex 2de597b..e8f11a4 160000\n\t--- a/sub-A\n\t+++ b/sub-A\n\t@@ -1 +1 @@\n\t-Subproject commit 2de597b5ad348b7db04bd10cdd38cd81cbc93ab5\n\t+Subproject commit e8f11a45c5c6b9e2fec6d136d3fb5aff75393d42\n\nNOTE! We'll also want to have the ability to recurse into the subproject \nand actually diff it recursively, but that will involve a new command line \noption (I'd suggest \"--subproject\" and \"-S\", but the latter is in use by \npickaxe), and some very different code.\n\nBut regardless of ay future recursive behaviour, we need the non-recursive \nversion too (and it should be the default, at least in the absense of \nconfig options, so that large superprojects don't default to something \nextremely expensive).\n\nSigned-off-by: Linus Torvalds \n---\n diff.c | 19 +++++++++++++++++++\n 1 files changed, 19 insertions(+), 0 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex e4efb65..137f5b4 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1419,6 +1419,21 @@ static int populate_from_stdin(struct diff_filespec *s)\n \treturn 0;\n }\n \n+static int diff_populate_gitlink(struct diff_filespec *s, int size_only)\n+{\n+\tint len;\n+\tchar *data = xmalloc(100);\n+\tlen = snprintf(data, 100,\n+\t\t\"Subproject commit %s\\n\", sha1_to_hex(s->sha1));\n+\ts->data = data;\n+\ts->size = len;\n+\tif (size_only) {\n+\t\ts->data = NULL;\n+\t\tfree(data);\n+\t}\n+\treturn 0;\n+}\n+\n /*\n * While doing rename detection and pickaxe operation, we may need to\n * grab the data for the blob (or file) for our own in-core comparison.\n@@ -1437,6 +1452,10 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)\n \n \tif (s->data)\n \t\treturn err;\n+\n+\tif (S_ISDIRLNK(s->mode))\n+\t\treturn diff_populate_gitlink(s, size_only);\n+\n \tif (!s->sha1_valid ||\n \t reuse_worktree_file(s->path, s->sha1, 0)) {\n \t\tstruct stat st;\n"},{"id":"39381","messageId":"20070415190119.GA21786@uranus.ravnborg.org","threadId":"7660","inReplyTo":"Pine.LNX.4.64.0704151100550.5473@woody.linux-foundation.org","subject":"Re: Expose subprojects as special files to \"git diff\" machinery","fromName":"Sam Ravnborg","fromEmail":"sam@ravnborg.org","sentAt":"2007-04-15T19:01:19Z","receivedAt":"2007-04-15T19:01:19Z","isPatch":false,"sender":{"key":"sam@ravnborg.org","avatar":"https://gravatar.com/avatar/168a912606ed0742d840bb365e3cc21db390c36531a58341dc7a069cc1f15f62?d=mp&s=160"},"body":"> \n> +static int diff_populate_gitlink(struct diff_filespec *s, int size_only)\n> +{\n> +\tint len;\n> +\tchar *data = xmalloc(100);\n> +\tlen = snprintf(data, 100,\n> +\t\t\"Subproject commit %s\\n\", sha1_to_hex(s->sha1));\n\nIn userland I would use a local variable for an array of the size of 100.\nI would normally only allocate when we are say 5x bigger.\n\nI wonder if there is a specific reason why you decided upon xmalloc here?\nI see no problem in using xmalloc but wonder if there is something I\nshould start to do differently??\n\n\tSam\n"},{"id":"39382","messageId":"Pine.LNX.4.64.0704151204320.5473@woody.linux-foundation.org","threadId":"7660","inReplyTo":"20070415190119.GA21786@uranus.ravnborg.org","subject":"Re: Expose subprojects as special files to \"git diff\" machinery","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-15T19:05:44Z","receivedAt":"2007-04-15T19:05:44Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 15 Apr 2007, Sam Ravnborg wrote:\n\n> > \n> > +static int diff_populate_gitlink(struct diff_filespec *s, int size_only)\n> > +{\n> > +\tint len;\n> > +\tchar *data = xmalloc(100);\n> > +\tlen = snprintf(data, 100,\n> > +\t\t\"Subproject commit %s\\n\", sha1_to_hex(s->sha1));\n> \n> In userland I would use a local variable for an array of the size of 100.\n> I would normally only allocate when we are say 5x bigger.\n\nWe're _returning_ the pointer to the caller, so no, we cannot use an \nautomatic array.\n\nBut I do think I had a bug - I think I should have set\n\n\ts->should_free = 1;\n\nto let the caller know it should be free'd with free(), and not leak the \nthing.\n\n\t\tLinus\n"},{"id":"39384","messageId":"20070415192035.GA21934@uranus.ravnborg.org","threadId":"7660","inReplyTo":"Pine.LNX.4.64.0704151204320.5473@woody.linux-foundation.org","subject":"Re: Expose subprojects as special files to \"git diff\" machinery","fromName":"Sam Ravnborg","fromEmail":"sam@ravnborg.org","sentAt":"2007-04-15T19:20:35Z","receivedAt":"2007-04-15T19:20:35Z","isPatch":false,"sender":{"key":"sam@ravnborg.org","avatar":"https://gravatar.com/avatar/168a912606ed0742d840bb365e3cc21db390c36531a58341dc7a069cc1f15f62?d=mp&s=160"},"body":"On Sun, Apr 15, 2007 at 12:05:44PM -0700, Linus Torvalds wrote:\n> \n> \n> On Sun, 15 Apr 2007, Sam Ravnborg wrote:\n> \n> > > \n> > > +static int diff_populate_gitlink(struct diff_filespec *s, int size_only)\n> > > +{\n> > > +\tint len;\n> > > +\tchar *data = xmalloc(100);\n> > > +\tlen = snprintf(data, 100,\n> > > +\t\t\"Subproject commit %s\\n\", sha1_to_hex(s->sha1));\n> > \n> > In userland I would use a local variable for an array of the size of 100.\n> > I would normally only allocate when we are say 5x bigger.\n> \n> We're _returning_ the pointer to the caller, so no, we cannot use an \n> automatic array.\nDo not know how I missed that...\nThanks,\n\tSam\n"},{"id":"39392","messageId":"200704152116.26773.andyparkins@gmail.com","threadId":"7660","inReplyTo":"Pine.LNX.4.64.0704151100550.5473@woody.linux-foundation.org","subject":"Re: Expose subprojects as special files to \"git diff\" machinery","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-04-15T20:16:23Z","receivedAt":"2007-04-15T20:16:23Z","isPatch":false,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Sunday 2007, April 15, Linus Torvalds wrote:\n\n> \tdiff --git a/sub-A b/sub-A\n> \tindex 2de597b..e8f11a4 160000\n> \t--- a/sub-A\n> \t+++ b/sub-A\n> \t@@ -1 +1 @@\n> \t-Subproject commit 2de597b5ad348b7db04bd10cdd38cd81cbc93ab5\n> \t+Subproject commit e8f11a45c5c6b9e2fec6d136d3fb5aff75393d42\n\nIsn't this dangerous because it looks just like a normal diff with a \nfile being rewritten, when in truth it is a tree entry record being \nrewritten. Possibly this is in the same category as the enhancements \nneeded for rename support\n\ncf.\n\ndiff --git a/file1 b/file2\nsimilarity index 100%\nrename from file1\nrename to file2\n\nThere's no need to have a line marker, as there is only one \"line\" in \nthe subproject \"file\", and no need for the index line because that \ninformation is contained in the change itself. So something like:\n\ndiff --git a/sub-A b/sub-B\nsubproject from commit 2de597b5ad348b7db04bd10cdd38cd81cbc93ab5\nsubproject to commit e8f11a45c5c6b9e2fec6d136d3fb5aff75393d42\n\nRegardless of the format, I'm sure this should be treated as part of \ngit's extended diff syntax, so that if a diff like this ever got into \nthe wild it wouldn't be a disaster if someone tried to apply it.\n\n\n\nAndy\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"39405","messageId":"Pine.LNX.4.64.0704151402130.5473@woody.linux-foundation.org","threadId":"7660","inReplyTo":"200704152116.26773.andyparkins@gmail.com","subject":"Re: Expose subprojects as special files to \"git diff\" machinery","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-15T21:03:26Z","receivedAt":"2007-04-15T21:03:26Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 15 Apr 2007, Andy Parkins wrote:\n> > \t+++ b/sub-A\n> > \t@@ -1 +1 @@\n> > \t-Subproject commit 2de597b5ad348b7db04bd10cdd38cd81cbc93ab5\n> > \t+Subproject commit e8f11a45c5c6b9e2fec6d136d3fb5aff75393d42\n> \n> Isn't this dangerous because it looks just like a normal diff with a \n> file being rewritten, when in truth it is a tree entry record being \n> rewritten.\n\nWell, that's exactly what symlinks do too.\n\nYou have to look at the mode to know what the rewriting *means*.\n\nBut yeah, I wouldn't object at all to making it an \"extended git header\" \ninstead (possibly just for subprojects, possibly for symlinks too)\n\n\t\tLinus\n"},{"id":"39750","messageId":"4625DAEE.3090702@vilain.net","threadId":"7660","inReplyTo":"Pine.LNX.4.64.0704151402130.5473@woody.linux-foundation.org","subject":"git-format-patch for binary files / merges [Re: Expose subprojects as special files to \"git diff\" machinery]","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2007-04-18T08:46:38Z","receivedAt":"2007-04-18T08:46:38Z","isPatch":false,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"Linus Torvalds wrote:\n>> Isn't this dangerous because it looks just like a normal diff with a \n>> file being rewritten, when in truth it is a tree entry record being \n>> rewritten.\n>> \n> Well, that's exactly what symlinks do too.\n>\n> You have to look at the mode to know what the rewriting *means*.\n>\n> But yeah, I wouldn't object at all to making it an \"extended git header\" \n> instead (possibly just for subprojects, possibly for symlinks too)\n> \n\nSpeaking of 'custom' patch file formats, anyone put any thought to a\nformat for the commits which can't be represented with patch, like\nbinary files and merges?\n\nSam.\n"},{"id":"39768","messageId":"81b0412b0704180449k34ac2606w82b8557d7d05945f@mail.gmail.com","threadId":"7660","inReplyTo":"4625DAEE.3090702@vilain.net","subject":"Re: git-format-patch for binary files / merges [Re: Expose subprojects as special files to \"git diff\" machinery]","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-04-18T11:49:46Z","receivedAt":"2007-04-18T11:49:46Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On 4/18/07, Sam Vilain wrote:\n> Linus Torvalds wrote:\n> >> Isn't this dangerous because it looks just like a normal diff with a\n> >> file being rewritten, when in truth it is a tree entry record being\n> >> rewritten.\n> >>\n> > Well, that's exactly what symlinks do too.\n> >\n> > You have to look at the mode to know what the rewriting *means*.\n> >\n> > But yeah, I wouldn't object at all to making it an \"extended git header\"\n> > instead (possibly just for subprojects, possibly for symlinks too)\n> >\n>\n> Speaking of 'custom' patch file formats, anyone put any thought to a\n> format for the commits which can't be represented with patch, like\n> binary files and merges?\n>\n\nThere is binary diff (look for --binary in Documentation/).\nAnd regarding merges: have you looked at git-bundle yet?\n"},{"id":"39769","messageId":"Pine.LNX.4.64.0704181348410.12094@racer.site","threadId":"7660","inReplyTo":"4625DAEE.3090702@vilain.net","subject":"Re: git-format-patch for binary files / merges [Re: Expose subprojects as special files to \"git diff\" machinery]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-18T11:52:20Z","receivedAt":"2007-04-18T11:52:20Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Apr 2007, Sam Vilain wrote:\n\n> Speaking of 'custom' patch file formats, anyone put any thought to a \n> format for the commits which can't be represented with patch, like \n> binary files and merges?\n\nGit has support for binary patches. They are base85 encoded, but you have \nto enable them explicitely (like renames) with --binary.\n\nAs for merges, there is the combined diff format. It has more than one \ncell at the beginning to indicate more than one \"new\" file (we do not use \nthis format for merges, but rather for conflicts, but you could easily \ninvert that meaning).\n\nCiao,\nDscho\n"},{"id":"39779","messageId":"alpine.LFD.0.98.0704180831330.2828@woody.linux-foundation.org","threadId":"7660","inReplyTo":"4625DAEE.3090702@vilain.net","subject":"Re: git-format-patch for binary files / merges [Re: Expose subprojects as special files to \"git diff\" machinery]","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-18T15:47:39Z","receivedAt":"2007-04-18T15:47:39Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 18 Apr 2007, Sam Vilain wrote:\n> \n> Speaking of 'custom' patch file formats, anyone put any thought to a\n> format for the commits which can't be represented with patch, like\n> binary files and merges?\n\nWe do already support binary patches, so those can be represented well in \na patch (but you need \"git-apply\" to apply them, so they are disabled by \ndefault.. And while we have tests for them, I suspect not a lot of people \nreally use them widely, so who knows how complete the coverage is. For \nexample, will \"git rebase\" really work? Probably. Do I know for sure? No).\n\nAs to merges, they are certainly something that *can* be represented as a \npatch (just tell what all the parents were, and desribe the end result as \na patch against _one_ of them: that already really encodes all the \ninformation). So we *could* support it with some trivial extension (and I \ndon't think the extension is necessarily even an expansion of the diff \nformat itself - I think the parents information is a \"higher-level\" \ninformation, and independent of the actual patch).\n\nHowever, representing merges as a patch doesn't really make much sense. \nYou really have two separate cases, and neither of them really makes sense \nto have a patch with:\n\n - you want to \"rebase\" the history, including merges. Quite frankly, I \n don't think a patch makes sense: you'd really need to re-do the merge \n with the old parents \"remapped\" to the new rebased parents, and what \n you might want to have is the \"rerere\" information from the previous \n merge. Maybe that's a patch, maybe it's not, but regardless, I don't\n think it really makes sense to describe it as a \"patch\" even if the \n implementation would do that: you don't do \"remote rebases\", so \n whatever it is would really be totally an internal thing to git-rebase.\n\n We currently simply don't support rebasing history with merges. Rebase \n is a linear history operation. If somebody wants to try to rebase \n complex history, I'd applaud the effort, but I think the problems are \n elsewhere than the actual \"patch\" part. \n\n - you want to send raw git commits to somebody else. Use the native git \n protocol. We already support that over email, it's called \"bundles\".\n\n I used \"bundles\" once or twice under BK (it was called \"bk send\" and \n \"bk receive + bk resolve\" or something like that), and I hated them. It \n wasn't BK's fault: I just found the workflow annoying. So I haven't \n even tested the git bundles, but if what you were looking for was to do \n a \"git push/pull\" by email, they are what you'd be using.\n\n(The reason I don't like bundles is that if I use email, I want to see the \nindividual patches _in_ the email. And once I see them, I think it's \nbetter to take advantage of the flexibility that individual patches gives \nme, ie I can re-order them, decide to skip one, edit the messages or \nwhatever before applying them. If I were to use bundles, I might as well \njust not blow up my mailbox, and just ask people to tell me where to \npull from).\n\n\t\t\tLinus\n"},{"id":"39784","messageId":"7vlkgpis2m.fsf@assigned-by-dhcp.cox.net","threadId":"7660","inReplyTo":"alpine.LFD.0.98.0704180831330.2828@woody.linux-foundation.org","subject":"Re: git-format-patch for binary files / merges [Re: Expose subprojects as special files to \"git diff\" machinery]","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T16:08:01Z","receivedAt":"2007-04-18T16:08:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> We do already support binary patches, so those can be represented well in \n> a patch (but you need \"git-apply\" to apply them, so they are disabled by \n> default.. And while we have tests for them, I suspect not a lot of people \n> really use them widely, so who knows how complete the coverage is. For \n> example, will \"git rebase\" really work? Probably. Do I know for sure? No).\n\nI've seen it in action for at least a few times in my day-job,\nso it appears to work ;-)\n\n> I used \"bundles\" once or twice under BK (it was called \"bk send\" and \n> \"bk receive + bk resolve\" or something like that), and I hated them. It \n> wasn't BK's fault: I just found the workflow annoying. So I haven't \n> even tested the git bundles, but if what you were looking for was to do \n> a \"git push/pull\" by email, they are what you'd be using.\n\nI've send a bundle over e-mail exactly once; it appears to work,\ntoo.\n"},{"id":"39870","messageId":"Pine.LNX.4.64.0704191026500.8822@racer.site","threadId":"7660","inReplyTo":"alpine.LFD.0.98.0704180831330.2828@woody.linux-foundation.org","subject":"Re: git-format-patch for binary files / merges [Re: Expose subprojects as special files to \"git diff\" machinery]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-04-19T08:28:30Z","receivedAt":"2007-04-19T08:28:30Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Apr 2007, Linus Torvalds wrote:\n\n> As to merges, they are certainly something that *can* be represented as a \n> patch [...]\n\n> However, representing merges as a patch doesn't really make much sense.\n\nNot if you want to apply it. But if you want to review it. That's why we \nhave the combined diff format, which can be generated by diff, but not \napplied.\n\nCiao,\nDscho\n"}]}