{"thread":{"id":"17160","subject":"[RESEND PATCH] fast-import: Cleanup mode setting.","startedAt":"2009-01-14T01:37:07Z","lastAt":"2009-01-14T02:51:11Z","messageCount":8,"participants":["Felipe Contreras","Johannes Schindelin","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"100350","messageId":"1231897027-16197-1-git-send-email-felipe.contreras@gmail.com","threadId":"17160","inReplyTo":null,"subject":"[RESEND PATCH] fast-import: Cleanup mode setting.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-01-14T01:37:07Z","receivedAt":"2009-01-14T01:37:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"\"S_IFREG | mode\" probably is only required for 0644 and 0755.\n\nSigned-off-by: Felipe Contreras \n---\n fast-import.c | 7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex a6bce66..f0e08ac 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1872,12 +1872,13 @@ static void file_change_m(struct branch *b)\n \tif (!p)\n \t\tdie(\"Corrupt mode: %s\", command_buf.buf);\n \tswitch (mode) {\n+\tcase 0644:\n+\tcase 0755:\n+\t\tmode |= S_IFREG;\n \tcase S_IFREG | 0644:\n \tcase S_IFREG | 0755:\n \tcase S_IFLNK:\n \tcase S_IFGITLINK:\n-\tcase 0644:\n-\tcase 0755:\n \t\t/* ok */\n \t\tbreak;\n \tdefault:\n@@ -1944,7 +1945,7 @@ static void file_change_m(struct branch *b)\n \t\t\t typename(type), command_buf.buf);\n \t}\n \n-\ttree_content_set(&b->branch_tree, p, sha1, S_IFREG | mode, NULL);\n+\ttree_content_set(&b->branch_tree, p, sha1, mode, NULL);\n }\n \n static void file_change_d(struct branch *b)\n-- \n1.6.0.6.5.ga66c\n"},{"id":"100351","messageId":"alpine.DEB.1.00.0901140308200.3586@pacific.mpi-cbg.de","threadId":"17160","inReplyTo":"1231897027-16197-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [RESEND PATCH] fast-import: Cleanup mode setting.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-14T02:08:50Z","receivedAt":"2009-01-14T02:08:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 14 Jan 2009, Felipe Contreras wrote:\n\n> \"S_IFREG | mode\" probably is only required for 0644 and 0755.\n\nWhy should we want to have that patch? IOW what does it fix, and what \nmight it break?\n\nCiao,\nDscho\n"},{"id":"100352","messageId":"20090114021648.GV10179@spearce.org","threadId":"17160","inReplyTo":"1231897027-16197-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [RESEND PATCH] fast-import: Cleanup mode setting.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-01-14T02:16:48Z","receivedAt":"2009-01-14T02:16:48Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Felipe Contreras wrote:\n> \"S_IFREG | mode\" probably is only required for 0644 and 0755.\n> \n> Signed-off-by: Felipe Contreras \n\nAcked-by: Shawn O. Pearce \n\n> ---\n> fast-import.c | 7 ++++---\n> 1 files changed, 4 insertions(+), 3 deletions(-)\n> \n> diff --git a/fast-import.c b/fast-import.c\n> index a6bce66..f0e08ac 100644\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -1872,12 +1872,13 @@ static void file_change_m(struct branch *b)\n> \tif (!p)\n> \t\tdie(\"Corrupt mode: %s\", command_buf.buf);\n> \tswitch (mode) {\n> +\tcase 0644:\n> +\tcase 0755:\n> +\t\tmode |= S_IFREG;\n> \tcase S_IFREG | 0644:\n> \tcase S_IFREG | 0755:\n> \tcase S_IFLNK:\n> \tcase S_IFGITLINK:\n> -\tcase 0644:\n> -\tcase 0755:\n> \t\t/* ok */\n> \t\tbreak;\n> \tdefault:\n> @@ -1944,7 +1945,7 @@ static void file_change_m(struct branch *b)\n> \t\t\t typename(type), command_buf.buf);\n> \t}\n> \n> -\ttree_content_set(&b->branch_tree, p, sha1, S_IFREG | mode, NULL);\n> +\ttree_content_set(&b->branch_tree, p, sha1, mode, NULL);\n> }\n> \n> static void file_change_d(struct branch *b)\n> -- \n> 1.6.0.6.5.ga66c\n> \n\n-- \nShawn.\n"},{"id":"100353","messageId":"20090114021751.GW10179@spearce.org","threadId":"17160","inReplyTo":"alpine.DEB.1.00.0901140308200.3586@pacific.mpi-cbg.de","subject":"Re: [RESEND PATCH] fast-import: Cleanup mode setting.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-01-14T02:17:51Z","receivedAt":"2009-01-14T02:17:51Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin wrote:\n> On Wed, 14 Jan 2009, Felipe Contreras wrote:\n> \n> > \"S_IFREG | mode\" probably is only required for 0644 and 0755.\n> \n> Why should we want to have that patch? IOW what does it fix, and what \n> might it break?\n\nIt cleans up the code to make it more readable.\n\nIt makes no sense to be doing S_IFREG | S_IFLINK, which happens when\nthe input is for a symlink. It doesn't break anything to do that |\noperation, but it also looks damn odd when reading the function.\n\n-- \nShawn.\n"},{"id":"100354","messageId":"alpine.DEB.1.00.0901140326530.3586@pacific.mpi-cbg.de","threadId":"17160","inReplyTo":"20090114021751.GW10179@spearce.org","subject":"Re: [RESEND PATCH] fast-import: Cleanup mode setting.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-14T02:28:31Z","receivedAt":"2009-01-14T02:28:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 13 Jan 2009, Shawn O. Pearce wrote:\n\n> Johannes Schindelin wrote:\n> > On Wed, 14 Jan 2009, Felipe Contreras wrote:\n> > \n> > > \"S_IFREG | mode\" probably is only required for 0644 and 0755.\n> > \n> > Why should we want to have that patch? IOW what does it fix, and what \n> > might it break?\n> \n> It cleans up the code to make it more readable.\n> \n> It makes no sense to be doing S_IFREG | S_IFLINK, which happens when\n> the input is for a symlink. It doesn't break anything to do that |\n> operation, but it also looks damn odd when reading the function.\n\nImagining myself reading the commit message 6 months from now, in all \nlikeliness I will have wished that those two paragraphs were in there. \nVerbatim.\n\nCiao,\nDscho\n"},{"id":"100355","messageId":"20090114022933.GX10179@spearce.org","threadId":"17160","inReplyTo":"alpine.DEB.1.00.0901140326530.3586@pacific.mpi-cbg.de","subject":"Re: [RESEND PATCH] fast-import: Cleanup mode setting.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-01-14T02:29:33Z","receivedAt":"2009-01-14T02:29:33Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Schindelin wrote:\n> On Tue, 13 Jan 2009, Shawn O. Pearce wrote:\n> > \n> > It cleans up the code to make it more readable.\n> > \n> > It makes no sense to be doing S_IFREG | S_IFLINK, which happens when\n> > the input is for a symlink. It doesn't break anything to do that |\n> > operation, but it also looks damn odd when reading the function.\n> \n> Imagining myself reading the commit message 6 months from now, in all \n> likeliness I will have wished that those two paragraphs were in there. \n> Verbatim.\n\nMaybe Junio or Felipe can copy it into the message.\n\nOr you can use a git note now to attach it to the commit Junio\nhasn't yet created, so you can look it up in the future. :-)\n\n-- \nShawn.\n"},{"id":"100357","messageId":"94a0d4530901131846u2121e433ka8620fbb37c8470b@mail.gmail.com","threadId":"17160","inReplyTo":"20090114022933.GX10179@spearce.org","subject":"Re: [RESEND PATCH] fast-import: Cleanup mode setting.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2009-01-14T02:46:35Z","receivedAt":"2009-01-14T02:46:35Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, Jan 14, 2009 at 4:29 AM, Shawn O. Pearce wrote:\n> Johannes Schindelin wrote:\n>> On Tue, 13 Jan 2009, Shawn O. Pearce wrote:\n>> >\n>> > It cleans up the code to make it more readable.\n>> >\n>> > It makes no sense to be doing S_IFREG | S_IFLINK, which happens when\n>> > the input is for a symlink. It doesn't break anything to do that |\n>> > operation, but it also looks damn odd when reading the function.\n>>\n>> Imagining myself reading the commit message 6 months from now, in all\n>> likeliness I will have wished that those two paragraphs were in there.\n>> Verbatim.\n>\n> Maybe Junio or Felipe can copy it into the message.\n>\n> Or you can use a git note now to attach it to the commit Junio\n> hasn't yet created, so you can look it up in the future. :-)\n\nHow about the attached patch?\n\n\"S_IFREG | mode\" probably is only required for 0644 and 0755.\n\nIt doesn't make sense to do S_IFREG | S_IFLINK (0100000 | 0120000),\nsince no bits are changed.\n\n-- \nFelipe Contreras\n\n\nFrom 2eee342bb2613a3378ea05d22ecdcc31e58f2e22 Mon Sep 17 00:00:00 2001\nFrom: Felipe Contreras \nDate: Mon, 22 Dec 2008 04:13:59 +0200\nSubject: [PATCH] fast-import: Cleanup mode setting.\n\n\"S_IFREG | mode\" probably is only required for 0644 and 0755.\n\nIt doesn't make sense to do S_IFREG | S_IFLINK (0100000 | 0120000),\nsince no bits are changed.\n\nSigned-off-by: Felipe Contreras \nSigned-off-by: Shawn O. Pearce \n---\n fast-import.c | 7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex a6bce66..f0e08ac 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1872,12 +1872,13 @@ static void file_change_m(struct branch *b)\n \tif (!p)\n \t\tdie(\"Corrupt mode: %s\", command_buf.buf);\n \tswitch (mode) {\n+\tcase 0644:\n+\tcase 0755:\n+\t\tmode |= S_IFREG;\n \tcase S_IFREG | 0644:\n \tcase S_IFREG | 0755:\n \tcase S_IFLNK:\n \tcase S_IFGITLINK:\n-\tcase 0644:\n-\tcase 0755:\n \t\t/* ok */\n \t\tbreak;\n \tdefault:\n@@ -1944,7 +1945,7 @@ static void file_change_m(struct branch *b)\n \t\t\t typename(type), command_buf.buf);\n \t}\n \n-\ttree_content_set(&b->branch_tree, p, sha1, S_IFREG | mode, NULL);\n+\ttree_content_set(&b->branch_tree, p, sha1, mode, NULL);\n }\n \n static void file_change_d(struct branch *b)\n-- \n1.6.0.6.5.ga66c\n\n"},{"id":"100358","messageId":"20090114025111.GY10179@spearce.org","threadId":"17160","inReplyTo":"94a0d4530901131846u2121e433ka8620fbb37c8470b@mail.gmail.com","subject":"Re: [RESEND PATCH] fast-import: Cleanup mode setting.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-01-14T02:51:11Z","receivedAt":"2009-01-14T02:51:11Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Felipe Contreras wrote:\n> \n> How about the attached patch?\n> \n> \"S_IFREG | mode\" probably is only required for 0644 and 0755.\n> \n> It doesn't make sense to do S_IFREG | S_IFLINK (0100000 | 0120000),\n> since no bits are changed.\n\nLooks fine to me.\n\n-- \nShawn.\n"}]}