# git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD 17 messages from 2005-10-26 to 2005-10-26. Participants: Randal L. Schwartz, Johannes Schindelin, Jan Harkes, Linus Torvalds, Horst von Brand, Junio C Hamano. Thread: https://gitlist.dev/t/2235 ## Randal L. Schwartz, 2005-10-26 14:11 Subject: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: <86y84gfjv4.fsf@blue.stonehenge.com> URL: https://gitlist.dev/e/86y84gfjv4.fsf%40blue.stonehenge.com ``` gcc -o sha1_file.o -c -g -O2 -Wall -I/usr/local/include -L/usr/local/lib -Dstrcasestr=gitstrcasestr -DNO_STRCASESTR=1 -DSHA1_HEADER='' sha1_file.c sha1_file.c: In function `move_temp_to_file': sha1_file.c:1247: error: `ENOTSUP' undeclared (first use in this function) sha1_file.c:1247: error: (Each undeclared identifier is reported only once sha1_file.c:1247: error: for each function it appears in.) gmake: *** [sha1_file.o] Error 1 -- Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095 Perl/Unix/security consulting, Technical writing, Comedy, etc. etc. See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training! ``` ## Randal L. Schwartz, 2005-10-26 14:24 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: <86u0f4fjah.fsf@blue.stonehenge.com> URL: https://gitlist.dev/e/86u0f4fjah.fsf%40blue.stonehenge.com In-Reply-To: <86y84gfjv4.fsf@blue.stonehenge.com> ``` >>>>> "Randal" == Randal L Schwartz writes: Randal> gcc -o sha1_file.o -c -g -O2 -Wall -I/usr/local/include -L/usr/local/lib -Dstrcasestr=gitstrcasestr -DNO_STRCASESTR=1 -DSHA1_HEADER='' sha1_file.c Randal> sha1_file.c: In function `move_temp_to_file': Randal> sha1_file.c:1247: error: `ENOTSUP' undeclared (first use in this function) Randal> sha1_file.c:1247: error: (Each undeclared identifier is reported only once Randal> sha1_file.c:1247: error: for each function it appears in.) Randal> gmake: *** [sha1_file.o] Error 1 got it... a bit messy, but here it is: Subject: [PATCH] fix for openbsd --- sha1_file.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) applies-to: b16bae041dfd1c1081873f2e88a5f82858fb2051 28dcc3eaf13856585a81a24f1b4393032f825053 diff --git a/sha1_file.c b/sha1_file.c index 7fdc469..1079ca5 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1244,7 +1244,11 @@ int move_temp_to_file(const char *tmpfil * When this succeeds, we just return 0. We have nothing * left to unlink. */ - if ((ret == EXDEV || ret == ENOTSUP) && !rename(tmpfile, filename)) + if ((ret == EXDEV +#ifdef ENOTSUP + || ret == ENOTSUP +#endif + ) && !rename(tmpfile, filename)) return 0; ret = errno; } --- 0.99.8.GIT -- Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095 Perl/Unix/security consulting, Technical writing, Comedy, etc. etc. See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training! ``` ## Johannes Schindelin, 2005-10-26 15:09 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0510261708280.2803%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <86u0f4fjah.fsf@blue.stonehenge.com> ``` Hi, On Wed, 26 Oct 2005, Randal L. Schwartz wrote: > Randal> sha1_file.c:1247: error: `ENOTSUP' undeclared (first use in this > function) I thought ENOTSUP is POSIX standard? What does your man page for link() say? If it is not supported, what value will be in errno? Ciao, Dscho ``` ## Randal L. Schwartz, 2005-10-26 15:10 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: <86mzkwfh54.fsf@blue.stonehenge.com> URL: https://gitlist.dev/e/86mzkwfh54.fsf%40blue.stonehenge.com In-Reply-To: ``` >>>>> "Johannes" == Johannes Schindelin writes: Johannes> Hi, Johannes> On Wed, 26 Oct 2005, Randal L. Schwartz wrote: Randal> sha1_file.c:1247: error: `ENOTSUP' undeclared (first use in this >> function) Johannes> I thought ENOTSUP is POSIX standard? Johannes> What does your man page for link() say? If it is not supported, what value Johannes> will be in errno? EOPNOTSUPP Probably a BSD thing. -- Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095 Perl/Unix/security consulting, Technical writing, Comedy, etc. etc. See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training! ``` ## Johannes Schindelin, 2005-10-26 15:26 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0510261725150.8507%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <86mzkwfh54.fsf@blue.stonehenge.com> ``` Hi, On Wed, 26 Oct 2005, Randal L. Schwartz wrote: > >>>>> "Johannes" == Johannes Schindelin writes: > > Johannes> What does your man page for link() say? If it is not > Johannes> supported, what value will be in errno? > > EOPNOTSUPP How about this, then: --- Subject: [PATCH] Support EOPNOTSUPP of OpenBSD Signed-off-by: Johannes Schindelin --- Makefile | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) applies-to: 01ba9ea9d9dfba5ae94c32691ac8d9fdd859fccf eaebda7362215aa0c7349e9971848c0535782a40 diff --git a/Makefile b/Makefile index 4877abb..5cb7194 100644 --- a/Makefile +++ b/Makefile @@ -211,6 +211,7 @@ ifeq ($(uname_S),OpenBSD) NO_STRCASESTR = YesPlease NEEDS_LIBICONV = YesPlease PLATFORM_DEFINES += -I/usr/local/include -L/usr/local/lib + PLATFORM_DEFINES += -DENOTSUP=EOPNOTSUPP endif ifneq (,$(findstring arm,$(uname_M))) ARM_SHA1 = YesPlease --- 0.99.8.GIT ``` ## Randal L. Schwartz, 2005-10-26 15:30 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: <86irvkfg80.fsf@blue.stonehenge.com> URL: https://gitlist.dev/e/86irvkfg80.fsf%40blue.stonehenge.com In-Reply-To: ``` >>>>> "Johannes" == Johannes Schindelin writes: Johannes> Hi, Johannes> On Wed, 26 Oct 2005, Randal L. Schwartz wrote: >> >>>>> "Johannes" == Johannes Schindelin writes: >> Johannes> What does your man page for link() say? If it is not Johannes> supported, what value will be in errno? >> >> EOPNOTSUPP Interesting... google for "ENOTSUPP EOPNOTSUPP". Apparently hits like this: shows that on some platforms, they may have both. This is not as easy as it looks. Maybe define an extra || condition to handle either or both? -- Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095 Perl/Unix/security consulting, Technical writing, Comedy, etc. etc. See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training! ``` ## Linus Torvalds, 2005-10-26 16:11 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0510260910290.10477%40g5.osdl.org In-Reply-To: <86u0f4fjah.fsf@blue.stonehenge.com> ``` On Wed, 26 Oct 2005, Randal L. Schwartz wrote: > > got it... a bit messy, but here it is: > > Subject: [PATCH] fix for openbsd I think we might as well do it for any error, and skip the test of the actual errno entirely. Linus ``` ## Jan Harkes, 2005-10-26 16:15 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: <20051026161552.GA11483@delft.aura.cs.cmu.edu> URL: https://gitlist.dev/e/20051026161552.GA11483%40delft.aura.cs.cmu.edu In-Reply-To: <86irvkfg80.fsf@blue.stonehenge.com> ``` On Wed, Oct 26, 2005 at 08:30:39AM -0700, Randal L. Schwartz wrote: > >> >>>>> "Johannes" == Johannes Schindelin writes: > Johannes> What does your man page for link() say? If it is not > Johannes> supported, what value will be in errno? > >> > >> EOPNOTSUPP > > Interesting... google for "ENOTSUPP EOPNOTSUPP". Apparently > hits like this: > > > > shows that on some platforms, they may have both. This is not as > easy as it looks. Maybe define an extra || condition to handle either > or both? It looks like glibc assumes that the kernel doesn't have ENOTSUP and defines it as EOPNOTSUPP in . So even if the kernel does return different values, any glibc linked application probably wouldn't be able to test for the ENOTSUP error. Checking the SuSv2 manpage for link(2), it doesn't mention either of these error codes. But does mention EPERM as a valid error when the filesystem doesn't support hardlinks. http://www.opengroup.org/onlinepubs/007908799/xsh/link.html My local manual page for link(2) agrees with that definition, EPERM oldpath is a directory. EPERM The filesystem containing oldpath and newpath does not support the creation of hard links. I also took a look at the Linux 2.4/2.6 kernel sources, and msdos, fat and vfat all do not set a function for 'dir->i_op->link'. In that case, vfs_link() will return EPERM. So I don't know where ENOTSUP/EOPNOTSUPP would be coming from. From the few places I looked, I don't think the original patch can be right. Not sure yet if I want to change Coda's return value to EPERM as well, the thing is that we do support hardlinks, just not between different directories, but I guess EXDEV isn't really accurate either. Jan ``` ## Johannes Schindelin, 2005-10-26 16:43 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0510261842580.29044%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: ``` Hi, On Wed, 26 Oct 2005, Linus Torvalds wrote: > On Wed, 26 Oct 2005, Randal L. Schwartz wrote: > > > > got it... a bit messy, but here it is: > > > > Subject: [PATCH] fix for openbsd > > I think we might as well do it for any error, and skip the test of the > actual errno entirely. Hah! I beat you at your own game! (I already sent out a totally untested patch ;-) Ciao, Dscho ``` ## Johannes Schindelin, 2005-10-26 16:46 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0510261844020.29044%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <20051026161552.GA11483@delft.aura.cs.cmu.edu> ``` Hi, On Wed, 26 Oct 2005, Jan Harkes wrote: > Checking the SuSv2 manpage for link(2), it doesn't mention either of > these error codes. But does mention EPERM as a valid error when the > filesystem doesn't support hardlinks. > http://www.opengroup.org/onlinepubs/007908799/xsh/link.html > > My local manual page for link(2) agrees with that definition, > > EPERM oldpath is a directory. > > EPERM The filesystem containing oldpath and newpath does not support > the creation of hard links. On my iBook, connecting a USB stick (FAT formatted), link() sets errno=ENOTSUP. I checked the man page, and erroneously thought that they got it right. *turns his eyes helplessly to the sky* Ciao, Dscho ``` ## Jan Harkes, 2005-10-26 16:55 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: <20051026165515.GA16616@delft.aura.cs.cmu.edu> URL: https://gitlist.dev/e/20051026165515.GA16616%40delft.aura.cs.cmu.edu In-Reply-To: <20051026161552.GA11483@delft.aura.cs.cmu.edu> ``` On Wed, Oct 26, 2005 at 12:15:52PM -0400, Jan Harkes wrote: > Checking the SuSv2 manpage for link(2), it doesn't mention either of > these error codes. But does mention EPERM as a valid error when the > filesystem doesn't support hardlinks. > http://www.opengroup.org/onlinepubs/007908799/xsh/link.html Re-reading this, I seem to have misinterpreted the text. EPERM is returned when the filesystem doesn't allow hard linking a directory. I guess the closest error would actually be EMLINK, assuming that a filesystem that doesn't support hardlinks would have LINK_MAX set to 1. Linux returns EPERM Coda returns EXDEV BSDs seem to return EOPNOTSUPP. MacOS X/Darwin doesn't mention it in their manpages. Simply falling back on rename when there is any error, as Linus suggested, seems like a pretty good solution right now. Rename should give the same set of errors except when the link target already existed. Jan diff --git a/sha1_file.c b/sha1_file.c index e456799..0eba58c 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1234,15 +1234,14 @@ int move_temp_to_file(const char *tmpfil int ret = link_temp_to_file(tmpfile, filename); if (ret) { /* - * Coda hack - coda doesn't like cross-directory links, - * so we fall back to a rename, which will mean that it - * won't be able to check collisions, but that's not a - * big deal. + * The file system may not support hardlinks, so we fall back + * to a rename. We won't be able to check for collisions, but + * that's not a big deal. * * When this succeeds, we just return 0. We have nothing * left to unlink. */ - if (ret == EXDEV && !rename(tmpfile, filename)) + if (ret && ret != EEXIST && !rename(tmpfile, filename)) return 0; } unlink(tmpfile); ``` ## Johannes Schindelin, 2005-10-26 17:03 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0510261901140.9686%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <20051026165515.GA16616@delft.aura.cs.cmu.edu> ``` Hi, On Wed, 26 Oct 2005, Jan Harkes wrote: > - if (ret == EXDEV && !rename(tmpfile, filename)) > + if (ret && ret != EEXIST && !rename(tmpfile, filename)) We don't need to test for ret, as we're already in a "if (ret)" block. What would happen if ret==EEXIST, and we try to rename() nevertheless? It should fail, too, right? Ciao, Dscho ``` ## Horst von Brand, 2005-10-26 17:22 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: <200510261722.j9QHMLGY006576@laptop11.inf.utfsm.cl> URL: https://gitlist.dev/e/200510261722.j9QHMLGY006576%40laptop11.inf.utfsm.cl In-Reply-To: <86u0f4fjah.fsf@blue.stonehenge.com> ``` Randal L. Schwartz wrote: > >>>>> "Randal" == Randal L Schwartz writes: > > Randal> gcc -o sha1_file.o -c -g -O2 -Wall -I/usr/local/include -L/usr/local/lib -Dstrcasestr=gitstrcasestr -DNO_STRCASESTR=1 -DSHA1_HEADER='' sha1_file.c > Randal> sha1_file.c: In function `move_temp_to_file': > Randal> sha1_file.c:1247: error: `ENOTSUP' undeclared (first use in this function) > Randal> sha1_file.c:1247: error: (Each undeclared identifier is reported only once > Randal> sha1_file.c:1247: error: for each function it appears in.) > Randal> gmake: *** [sha1_file.o] Error 1 > > got it... a bit messy, but here it is: > > Subject: [PATCH] fix for openbsd > > --- > > sha1_file.c | 6 +++++- > 1 files changed, 5 insertions(+), 1 deletions(-) > > applies-to: b16bae041dfd1c1081873f2e88a5f82858fb2051 > 28dcc3eaf13856585a81a24f1b4393032f825053 > diff --git a/sha1_file.c b/sha1_file.c > index 7fdc469..1079ca5 100644 > --- a/sha1_file.c > +++ b/sha1_file.c > @@ -1244,7 +1244,11 @@ int move_temp_to_file(const char *tmpfil > * When this succeeds, we just return 0. We have nothing > * left to unlink. > */ > - if ((ret == EXDEV || ret == ENOTSUP) && !rename(tmpfile, filename)) > + if ((ret == EXDEV > +#ifdef ENOTSUP > + || ret == ENOTSUP > +#endif > + ) && !rename(tmpfile, filename)) This is EVIL.... why not just: #ifndef ENOTSUP #define ENOTSUP EXDEV #endif someplace where it doesn't hurt the eyes? (The EXDEV makes the ENOTSUPP fold away, I presume; not that it should really matter). -- Dr. Horst H. von Brand User #22616 counter.li.org Departamento de Informatica Fono: +56 32 654431 Universidad Tecnica Federico Santa Maria +56 32 654239 Casilla 110-V, Valparaiso, Chile Fax: +56 32 797513 ``` ## Linus Torvalds, 2005-10-26 17:27 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.64.0510261025440.10477%40g5.osdl.org In-Reply-To: ``` On Wed, 26 Oct 2005, Johannes Schindelin wrote: > > On Wed, 26 Oct 2005, Jan Harkes wrote: > > > - if (ret == EXDEV && !rename(tmpfile, filename)) > > + if (ret && ret != EEXIST && !rename(tmpfile, filename)) > > We don't need to test for ret, as we're already in a "if (ret)" block. > What would happen if ret==EEXIST, and we try to rename() nevertheless? It > should fail, too, right? No, the rename will succeed and overwrite the existing file. Which is strictly correct, but it's the wrong thing if we ever want to do collission detection. So I'd actually prefer something like this patch instead. Linus --- diff --git a/sha1_file.c b/sha1_file.c index 7fdc469..642f00d 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1232,19 +1232,20 @@ static int link_temp_to_file(const char int move_temp_to_file(const char *tmpfile, char *filename) { int ret = link_temp_to_file(tmpfile, filename); - if (ret) { - /* - * Coda hack - coda doesn't like cross-directory links, - * so we fall back to a rename, which will mean that it - * won't be able to check collisions, but that's not a - * big deal. - * - * The same holds for FAT formatted media. - * - * When this succeeds, we just return 0. We have nothing - * left to unlink. - */ - if ((ret == EXDEV || ret == ENOTSUP) && !rename(tmpfile, filename)) + + /* + * Coda hack - coda doesn't like cross-directory links, + * so we fall back to a rename, which will mean that it + * won't be able to check collisions, but that's not a + * big deal. + * + * The same holds for FAT formatted media. + * + * When this succeeds, we just return 0. We have nothing + * left to unlink. + */ + if (ret && ret != EEXIST) { + if (!rename(tmpfile, filename)) return 0; ret = errno; } ``` ## Randal L. Schwartz, 2005-10-26 18:50 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: <86mzkwdsef.fsf@blue.stonehenge.com> URL: https://gitlist.dev/e/86mzkwdsef.fsf%40blue.stonehenge.com In-Reply-To: <200510261722.j9QHMLGY006576@laptop11.inf.utfsm.cl> ``` >>>>> "Horst" == Horst von Brand writes: Horst> This is EVIL.... why not just: Horst> #ifndef ENOTSUP Horst> #define ENOTSUP EXDEV Horst> #endif I'd consider that a worse hack. It affects any place where EXDEV is not really equivalent to ENOTSUP. It's cleaner to test for the ones that work, building up an OR'ed expression. -- Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095 Perl/Unix/security consulting, Technical writing, Comedy, etc. etc. See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training! ``` ## Junio C Hamano, 2005-10-26 20:08 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: <7vr7a8ca8j.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vr7a8ca8j.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <86mzkwdsef.fsf@blue.stonehenge.com> ``` merlyn@stonehenge.com (Randal L. Schwartz) writes: > It's cleaner to test for the ones that work, building up an OR'ed expression. You are right. In this case the only thing we care about is if it failed because of EEXIST, so checking that and trying to fall back on rename() otherwise seems cleanest, as Linus and others suggests. I swallowed Linus version. Thanks for reporting (and also ctype fix the other day). ``` ## Randal L. Schwartz, 2005-10-26 20:12 Subject: Re: git 565ebbf79f61873042c22a7126d002c104e056f4 broken on OpenBSD Message-ID: <86ll0gavgn.fsf@blue.stonehenge.com> URL: https://gitlist.dev/e/86ll0gavgn.fsf%40blue.stonehenge.com In-Reply-To: <7vr7a8ca8j.fsf@assigned-by-dhcp.cox.net> ``` >>>>> "Junio" == Junio C Hamano writes: Junio> I swallowed Linus version. Thanks for reporting (and also ctype Junio> fix the other day). I apologize that I don't immediately post the patch, but generally I figure that maybe Smarter People Than Me might recognize the issue before I can get around to figuring out what broke. After all, my C is rusty. My Perl is much better. :) Also, I'm learning how git works, to generate diffs and patches, etc. -- Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095 Perl/Unix/security consulting, Technical writing, Comedy, etc. etc. See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training! ```