{"thread":{"id":"18196","subject":"[PATCH] MinGW: fix diff --no-index /dev/null ...","startedAt":"2009-03-07T15:51:33Z","lastAt":"2009-03-07T21:40:44Z","messageCount":6,"participants":["Johannes Schindelin","Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"107304","messageId":"dba002b9e521c639847650fbaeb8b87b66b9562e.1236441065u.git.johannes.schindelin@gmx.de","threadId":"18196","inReplyTo":"cover.1236441065u.git.johannes.schindelin@gmx.de","subject":"[PATCH] MinGW: fix diff --no-index /dev/null ...","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-07T15:51:33Z","receivedAt":"2009-03-07T15:51:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When launching \"diff --no-index\" with a parameter \"/dev/null\", the MSys\nbash converts the \"/dev/null\" to a \"nul\", which usually makes sense. But\ndiff --no-index got confused and tried to access a _file_ called \"nul\".\n\nWhile at it, the comment in t4012, expressed as \":# \" was turned\ninto \": \" so that MSys' path name mangling does not kick in.\n\nWith this patch, t4012 passes in msysGit.\n\nSigned-off-by: Johannes Schindelin \n---\n diff-no-index.c | 4 ++++\n t/t4012-diff-binary.sh | 2 +-\n 2 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 0a14268..598687b 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -38,6 +38,10 @@ static int get_mode(const char *path, int *mode)\n \n \tif (!path || !strcmp(path, \"/dev/null\"))\n \t\t*mode = 0;\n+#ifdef _WIN32\n+\telse if (!strcasecmp(path, \"nul\"))\n+\t\t*mode = 0;\n+#endif\n \telse if (!strcmp(path, \"-\"))\n \t\t*mode = create_ce_mode(0666);\n \telse if (lstat(path, &st))\ndiff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh\nindex 3cf5b5c..f64aa48 100755\n--- a/t/t4012-diff-binary.sh\n+++ b/t/t4012-diff-binary.sh\n@@ -87,7 +87,7 @@ nul_to_q() {\n \n test_expect_success 'diff --no-index with binary creation' '\n \techo Q | q_to_nul >binary &&\n-\t(:# hide error code from diff, which just indicates differences\n+\t(: hide error code from diff, which just indicates differences\n \t git diff --binary --no-index /dev/null binary >current ||\n \t true\n \t) &&\n-- \n1.6.2.327.g0fa6c\n"},{"id":"107330","messageId":"200903072056.58465.j6t@kdbg.org","threadId":"18196","inReplyTo":"dba002b9e521c639847650fbaeb8b87b66b9562e.1236441065u.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] MinGW: fix diff --no-index /dev/null ...","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-03-07T19:56:58Z","receivedAt":"2009-03-07T19:56:58Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 7. März 2009, Johannes Schindelin wrote:\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> @@ -38,6 +38,10 @@ static int get_mode(const char *path, int *mode)\n>\n> \tif (!path || !strcmp(path, \"/dev/null\"))\n> \t\t*mode = 0;\n> +#ifdef _WIN32\n> +\telse if (!strcasecmp(path, \"nul\"))\n> +\t\t*mode = 0;\n> +#endif\n> \telse if (!strcmp(path, \"-\"))\n> \t\t*mode = create_ce_mode(0666);\n> \telse if (lstat(path, &st))\n\nIMO this #ifdef is a reasonable compromise. Trying to move this into \ngit-compat-util.h or mingw.h somehow would only obfuscate the code.\n\nTested-by: Johannes Sixt \n\n-- Hannes\n"},{"id":"107335","messageId":"7v8wnhnl6t.fsf@gitster.siamese.dyndns.org","threadId":"18196","inReplyTo":"dba002b9e521c639847650fbaeb8b87b66b9562e.1236441065u.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] MinGW: fix diff --no-index /dev/null ...","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-07T20:15:54Z","receivedAt":"2009-03-07T20:15:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index 0a14268..598687b 100644\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> @@ -38,6 +38,10 @@ static int get_mode(const char *path, int *mode)\n> \n> \tif (!path || !strcmp(path, \"/dev/null\"))\n> \t\t*mode = 0;\n> +#ifdef _WIN32\n> +\telse if (!strcasecmp(path, \"nul\"))\n> +\t\t*mode = 0;\n> +#endif\n> \telse if (!strcmp(path, \"-\"))\n> \t\t*mode = create_ce_mode(0666);\n> \telse if (lstat(path, &st))\n\nI had a brief \"Huh? -- doesn't this call for an is_dev_null() helper\"\nmoment, but I think you are right that diff-no-index.c is the right place\nto special case it.\n\nShould this go to 'maint'?\n\n> diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh\n> index 3cf5b5c..f64aa48 100755\n> --- a/t/t4012-diff-binary.sh\n> +++ b/t/t4012-diff-binary.sh\n> @@ -87,7 +87,7 @@ nul_to_q() {\n> \n> test_expect_success 'diff --no-index with binary creation' '\n> \techo Q | q_to_nul >binary &&\n> -\t(:# hide error code from diff, which just indicates differences\n> +\t(: hide error code from diff, which just indicates differences\n> \t git diff --binary --no-index /dev/null binary >current ||\n> \t true\n> \t) &&\n> -- \n> 1.6.2.327.g0fa6c\n"},{"id":"107340","messageId":"alpine.DEB.1.00.0903072133270.10279@pacific.mpi-cbg.de","threadId":"18196","inReplyTo":"7v8wnhnl6t.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] MinGW: fix diff --no-index /dev/null ...","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-07T20:47:18Z","receivedAt":"2009-03-07T20:47:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 7 Mar 2009, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > diff --git a/diff-no-index.c b/diff-no-index.c\n> > index 0a14268..598687b 100644\n> > --- a/diff-no-index.c\n> > +++ b/diff-no-index.c\n> > @@ -38,6 +38,10 @@ static int get_mode(const char *path, int *mode)\n> > \n> > \tif (!path || !strcmp(path, \"/dev/null\"))\n> > \t\t*mode = 0;\n> > +#ifdef _WIN32\n> > +\telse if (!strcasecmp(path, \"nul\"))\n> > +\t\t*mode = 0;\n> > +#endif\n> > \telse if (!strcmp(path, \"-\"))\n> > \t\t*mode = create_ce_mode(0666);\n> > \telse if (lstat(path, &st))\n> \n> I had a brief \"Huh? -- doesn't this call for an is_dev_null() helper\"\n> moment, but I think you are right that diff-no-index.c is the right place\n> to special case it.\n\nYou're right, it needs to be explained in the commit message. So how \nabout this:\n\n-- snip --\nUnlike other places where a string is compared to /dev/null, in this case \ndo not parse a diff, but rather command line parameters that have \npossibly been transformed from /dev/null to nul, so that the file can be \nopened.\n-- snap --\n\nIf you like it, may I ask you to amend the message?\n\n> Should this go to 'maint'?\n\nTechnically, yes, as it is a fix.\n\nHowever, it might not be necessary, as literally all Windows work on Git \nhappens in git/mingw.git, git/mingw/j6t.git and git/mingw/4msysgit.git.\n\nYour call.\n\nThanks,\nDscho\n"},{"id":"107348","messageId":"200903072218.39087.j6t@kdbg.org","threadId":"18196","inReplyTo":"7v8wnhnl6t.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] MinGW: fix diff --no-index /dev/null ...","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-03-07T21:18:39Z","receivedAt":"2009-03-07T21:18:39Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 7. März 2009, Junio C Hamano wrote:\n> Should this go to 'maint'?\n\nI don't think that's necessary. It fixes only that someone literally writes on \nthe Windows command line\n\n git diff --no-index nul foo.c\n\nwhen on Unix one would have written\n\n git diff --no-index /dev/null foo.c\n\nIn practice, only the test suite does this ;)\n\n-- Hannes\n"},{"id":"107349","messageId":"7vhc25m2oz.fsf@gitster.siamese.dyndns.org","threadId":"18196","inReplyTo":"alpine.DEB.1.00.0903072133270.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH] MinGW: fix diff --no-index /dev/null ...","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-07T21:40:44Z","receivedAt":"2009-03-07T21:40:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Unlike other places where a string is compared to /dev/null, in this case \n> do not parse a diff, but rather command line parameters that have \n> possibly been transformed from /dev/null to nul, so that the file can be \n> opened.\n> -- snap --\n>\n> If you like it, may I ask you to amend the message?\n>\n>> Should this go to 'maint'?\n>\n> Technically, yes, as it is a fix.\n>\n> However, it might not be necessary, as literally all Windows work on Git \n> happens in git/mingw.git, git/mingw/j6t.git and git/mingw/4msysgit.git.\n\nYeah, I should have been clearer. Unless msysgit will have a maintenance\ntrack for 1.6.1.X, there is no point.\n\nThanks for reading my mind and a clarified message I can use for amending.\n"}]}