Обсуждение: The dbase conrtib doesn't compile
In HEAD contrib/dbase: gcc -pipe -O2 -Wall -Wmissing-prototypes -Wmissing-declarations -I../../src/ interf aces/libpq -I. -I../../src/include -c -o dbf2pg.o dbf2pg.c dbf2pg.c:19: iconv.h: No such file or directory dbf2pg.c:38: syntax error before `iconv_d' dbf2pg.c:38: warning: type defaults to `int' in declaration of `iconv_d' dbf2pg.c:38: warning: data definition has no type or storage class dbf2pg.c: In function `convert_charset': dbf2pg.c:148: warning: implicit declaration of function `iconv' dbf2pg.c: In function `main': dbf2pg.c:789: warning: implicit declaration of function `iconv_open' dbf2pg.c:790: `iconv_t' undeclared (first use in this function) dbf2pg.c:790: (Each undeclared identifier is reported only once dbf2pg.c:790: for each function it appears in.) dbf2pg.c:810: warning: implicit declaration of function `iconv_close' gmake: *** [dbf2pg.o] Error 1 Chris
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> In HEAD contrib/dbase:
> gcc -pipe -O2 -Wall -Wmissing-prototypes -Wmissing-declarations -I../../src/
> interf
> aces/libpq -I. -I../../src/include -c -o dbf2pg.o dbf2pg.c
> dbf2pg.c:19: iconv.h: No such file or directory
Looks like someone took a shortcut in dealing with <iconv.h>.
What the heck is that, anyway, and do we need the ifdef'd code at all?
(FWIW, the code compiles fine if you do have <iconv.h>)
regards, tom lane
So what's the fix? I have iconv.h. This is my system: Intel: /usr/include/sys/iconv.h /usr/local/include/giconv.h /usr/local/include/iconv.h And the Alpha: /usr/include/sys/iconv.h Chris > -----Original Message----- > From: Tom Lane [mailto:tgl@sss.pgh.pa.us] > Sent: Friday, 21 December 2001 11:55 AM > To: Christopher Kings-Lynne > Cc: Hackers > Subject: Re: [HACKERS] The dbase conrtib doesn't compile > > > "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: > > In HEAD contrib/dbase: > > gcc -pipe -O2 -Wall -Wmissing-prototypes -Wmissing-declarations > -I../../src/ > > interf > > aces/libpq -I. -I../../src/include -c -o dbf2pg.o dbf2pg.c > > dbf2pg.c:19: iconv.h: No such file or directory > > Looks like someone took a shortcut in dealing with <iconv.h>. > What the heck is that, anyway, and do we need the ifdef'd code at all? > > (FWIW, the code compiles fine if you do have <iconv.h>) > > regards, tom lane >
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> So what's the fix? I have iconv.h. This is my system:
> Intel:
> /usr/include/sys/iconv.h
> /usr/local/include/giconv.h
> /usr/local/include/iconv.h
> And the Alpha:
> /usr/include/sys/iconv.h
Hmm, either it should be #include <sys/iconv.h> or you need to configure
--with-includes=/usr/local/include. Try it and see.
You have to remember that the contrib stuff is not ready for prime time;
if it were, it'd likely be in the mainframe. Portability problems are
par for the course. (The cast things you just found in pgcrypto look
like they might be real bugs, btw.)
regards, tom lane
> In HEAD contrib/dbase: > > gcc -pipe -O2 -Wall -Wmissing-prototypes -Wmissing-declarations -I../../src/ > interf > aces/libpq -I. -I../../src/include -c -o dbf2pg.o dbf2pg.c > dbf2pg.c:19: iconv.h: No such file or directory > dbf2pg.c:38: syntax error before `iconv_d' > dbf2pg.c:38: warning: type defaults to `int' in declaration of `iconv_d' > dbf2pg.c:38: warning: data definition has no type or storage class > dbf2pg.c: In function `convert_charset': > dbf2pg.c:148: warning: implicit declaration of function `iconv' > dbf2pg.c: In function `main': > dbf2pg.c:789: warning: implicit declaration of function `iconv_open' > dbf2pg.c:790: `iconv_t' undeclared (first use in this function) > dbf2pg.c:790: (Each undeclared identifier is reported only once > dbf2pg.c:790: for each function it appears in.) > dbf2pg.c:810: warning: implicit declaration of function `iconv_close' > gmake: *** [dbf2pg.o] Error 1 Yes, I am seeing the same failure here. I knew there was a /contrib module that needed iconv but I can't find it now. I suppose this was the one. I see the old Makefile used iconv: ! $(CC) $(CFLAGS) $(OBJS) $(libpq) $(LDFLAGS) $(LIBS) -liconv -o $@ but the overhaul of these Makefiles on 2001/09/06 removed it. I am applying the following patch to re-add it. You will need libiconv for it to link. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 Index: contrib/dbase/Makefile =================================================================== RCS file: /cvsroot/pgsql/contrib/dbase/Makefile,v retrieving revision 1.2 diff -c -r1.2 Makefile *** contrib/dbase/Makefile 2001/09/06 10:49:29 1.2 --- contrib/dbase/Makefile 2001/12/21 04:10:17 *************** *** 7,13 **** PROGRAM = dbf2pg OBJS = dbf.o dbf2pg.o endian.o PG_CPPFLAGS = -I$(libpq_srcdir) ! PG_LIBS = $(libpq) DOCS = README.dbf2pg MAN = dbf2pg.1 # XXX not implemented --- 7,13 ---- PROGRAM = dbf2pg OBJS = dbf.o dbf2pg.o endian.o PG_CPPFLAGS = -I$(libpq_srcdir) ! PG_LIBS = $(libpq) -liconv DOCS = README.dbf2pg MAN = dbf2pg.1 # XXX not implemented Index: contrib/dbase/README.dbf2pg =================================================================== RCS file: /cvsroot/pgsql/contrib/dbase/README.dbf2pg,v retrieving revision 1.1 diff -c -r1.1 README.dbf2pg *** contrib/dbase/README.dbf2pg 2001/05/10 14:41:23 1.1 --- contrib/dbase/README.dbf2pg 2001/12/21 04:10:17 *************** *** 107,113 **** ENVIRONMENT This program is affected by the environment-variables as used by "PostgresSQL." See the documentation of Post- ! gresSQL for more info. BUGS Fields larger than 8192 characters are not supported and --- 107,113 ---- ENVIRONMENT This program is affected by the environment-variables as used by "PostgresSQL." See the documentation of Post- ! gresSQL for more info. This program requires libiconv. BUGS Fields larger than 8192 characters are not supported and
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> I'm just checking them all cos I'm a bit of a FreeBSD & PostgreSQL fanatic
> and I want FBSD to be a good platform for pgsql. Maybe we could add to the
> regression test database fields for 'result of running "gmake all" in
> contrib/' ?
Okay, though you really should also try the regression tests for the
modules that have one.
> Unfortunately this process stops at the first error. Maybe it
> would be nice if it could continue on and give a report of the failures?
"gmake -k" is your friend.
regards, tom lane
> You have to remember that the contrib stuff is not ready for prime time; > if it were, it'd likely be in the mainframe. Portability problems are > par for the course. (The cast things you just found in pgcrypto look > like they might be real bugs, btw.) I'm just checking them all cos I'm a bit of a FreeBSD & PostgreSQL fanatic and I want FBSD to be a good platform for pgsql. Maybe we could add to the regression test database fields for 'result of running "gmake all" in contrib/' ? Unfortunately this process stops at the first error. Maybe it would be nice if it could continue on and give a report of the failures? Chris
> "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: > > So what's the fix? I have iconv.h. This is my system: > > Intel: > > > /usr/include/sys/iconv.h > > /usr/local/include/giconv.h > > /usr/local/include/iconv.h > > > And the Alpha: > > > /usr/include/sys/iconv.h > > Hmm, either it should be #include <sys/iconv.h> or you need to configure > --with-includes=/usr/local/include. Try it and see. > > You have to remember that the contrib stuff is not ready for prime time; > if it were, it'd likely be in the mainframe. Portability problems are > par for the course. (The cast things you just found in pgcrypto look > like they might be real bugs, btw.) Also, you need current CVS because I just added the -liconv in Makefile. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
> Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Also, you need current CVS because I just added the -liconv in Makefile. > > Well, it *used* to build under HPUX. And whatever the contributor's > system was. Your change has fixed one platform and broken two others. The -liconv used to be there before in 7.1 and earlier. It was only removed in September. Are you saying those system calls work for you, but you don't have a libiconv? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Also, you need current CVS because I just added the -liconv in Makefile.
Well, it *used* to build under HPUX. And whatever the contributor's
system was. Your change has fixed one platform and broken two others.
regards, tom lane
> "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: > > In HEAD contrib/dbase: > > gcc -pipe -O2 -Wall -Wmissing-prototypes -Wmissing-declarations -I../../src/ > > interf > > aces/libpq -I. -I../../src/include -c -o dbf2pg.o dbf2pg.c > > dbf2pg.c:19: iconv.h: No such file or directory > > Looks like someone took a shortcut in dealing with <iconv.h>. > What the heck is that, anyway, and do we need the ifdef'd code at all? > > (FWIW, the code compiles fine if you do have <iconv.h>) I see that now too. Seems we need to test for libiconv and set HAVE_ICONV_H accordingly, and the link line too. If I comment out the define, it does not compile so the conditionals in the code are not correct anyway. The following patch does allow it to compile with HAVE_ICONV_H not defined; clearly a step in the right direction. Of course, you will still need to remove the -liconv from the link line. I wonder if the best solution is to not assume libiconv exists. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026 Index: contrib/dbase/dbf2pg.c =================================================================== RCS file: /cvsroot/pgsql/contrib/dbase/dbf2pg.c,v retrieving revision 1.4 diff -c -r1.4 dbf2pg.c *** contrib/dbase/dbf2pg.c 2001/10/25 05:49:19 1.4 --- contrib/dbase/dbf2pg.c 2001/12/21 04:27:48 *************** *** 742,753 **** --- 742,755 ---- case 'U': username = (char *) strdup(optarg); break; + #ifdef HAVE_ICONV_H case 'F': charset_from = (char *) strdup(optarg); break; case 'T': charset_to = (char *) strdup(optarg); break; + #endif case ':': usage(); printf("missing argument!\n"); *************** *** 806,813 **** --- 808,817 ---- free(username); if (password) free(password); + #ifdef HAVE_ICONV_H if (charset_from) iconv_close(iconv_d); + #endif exit(1); } *************** *** 846,853 **** --- 850,859 ---- free(username); if (password) free(password); + #ifdef HAVE_ICONV_H if (charset_from) iconv_close(iconv_d); + #endif exit(1); } *************** *** 864,871 **** --- 870,879 ---- free(username); if (password) free(password); + #ifdef HAVE_ICONV_H if (charset_from) iconv_close(iconv_d); + #endif exit(1); } if (del) *************** *** 880,887 **** --- 888,897 ---- free(username); if (password) free(password); + #ifdef HAVE_ICONV_H if (charset_from) iconv_close(iconv_d); + #endif exit(1); } if (verbose > 1) *************** *** 903,910 **** --- 913,922 ---- free(username); if (password) free(password); + #ifdef HAVE_ICONV_H if (charset_from) iconv_close(iconv_d); + #endif exit(1); } if (verbose > 1) *************** *** 933,939 **** --- 945,953 ---- free(username); if (password) free(password); + #ifdef HAVE_ICONV_H if (charset_from) iconv_close(iconv_d); + #endif exit(0); }
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> The -liconv used to be there before in 7.1 and earlier. It was only
> removed in September. Are you saying those system calls work for you,
> but you don't have a libiconv?
The <iconv.h> routines live in libc on HPUX. And on Red Hat Linux
(I suppose also on other Linux flavors, but RHL 7.2 is the only one
I have handy to check). And presumably on whatever platform Peter uses,
else he'd not have removed the -liconv.
Christopher has not yet opined on where they are on his platform...
though since it's a BSD variant, it might be the same as yours.
To fix this correctly we'd need to add configure tests for <iconv.h>
and libiconv. I'm disinclined to do that, partly because it'd slow
down configure for everyone whether they intended to build contrib/dbase
or not, but mainly because in the present state of the build process
it'd cause libiconv (if present) to be linked to *every* executable
we build.
I wonder if it's practical for contrib modules to have their own
mini-configure checks above and beyond what the main configure script
does?
In the meantime, I don't really care that much whether dbase/Makefile
contains -liconv or not; clearly, that will help some platforms and
hurt others no matter which way we jump. I was just pointing out
that your makefile change is not a clear win.
regards, tom lane
> Bruce Momjian <pgman@candle.pha.pa.us> writes: > > The -liconv used to be there before in 7.1 and earlier. It was only > > removed in September. Are you saying those system calls work for you, > > but you don't have a libiconv? > > The <iconv.h> routines live in libc on HPUX. And on Red Hat Linux > (I suppose also on other Linux flavors, but RHL 7.2 is the only one > I have handy to check). And presumably on whatever platform Peter uses, > else he'd not have removed the -liconv. > > Christopher has not yet opined on where they are on his platform... > though since it's a BSD variant, it might be the same as yours. > > To fix this correctly we'd need to add configure tests for <iconv.h> > and libiconv. I'm disinclined to do that, partly because it'd slow > down configure for everyone whether they intended to build contrib/dbase > or not, but mainly because in the present state of the build process > it'd cause libiconv (if present) to be linked to *every* executable > we build. > > I wonder if it's practical for contrib modules to have their own > mini-configure checks above and beyond what the main configure script > does? > > In the meantime, I don't really care that much whether dbase/Makefile > contains -liconv or not; clearly, that will help some platforms and > hurt others no matter which way we jump. I was just pointing out > that your makefile change is not a clear win. Yes, glad you pointed it out. I think the best solution is to remove #define HAVE_ICONV_H and -liconv so it will work fine on all platforms. If someone wants the iconv conversions, they can add the needed #define and link library, OK? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Yes, glad you pointed it out. I think the best solution is to remove
> #define HAVE_ICONV_H and -liconv so it will work fine on all platforms.
> If someone wants the iconv conversions, they can add the needed #define
> and link library, OK?
That seems like a plan. Perhaps add some commented-out macro
definitions to the makefile to make it a simple addition. Something
like
# Uncomment this to provide charset translation
# CFLAGS += -DHAVE_ICONV_H
# You might need to uncomment this too, if libiconv is a separate
# library on your platform
# LIBS += -liconv
Untested but you get the idea ...
regards, tom lane
> Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Yes, glad you pointed it out. I think the best solution is to remove > > #define HAVE_ICONV_H and -liconv so it will work fine on all platforms. > > If someone wants the iconv conversions, they can add the needed #define > > and link library, OK? > > That seems like a plan. Perhaps add some commented-out macro > definitions to the makefile to make it a simple addition. Something > like > > # Uncomment this to provide charset translation > # CFLAGS += -DHAVE_ICONV_H > # You might need to uncomment this too, if libiconv is a separate > # library on your platform > # LIBS += -liconv > > Untested but you get the idea ... OK, patch attached and applied. I tested with and without the Makefile defines. iconv defaults to off, mentioned in README. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 Index: contrib/dbase/Makefile =================================================================== RCS file: /cvsroot/pgsql/contrib/dbase/Makefile,v retrieving revision 1.3 diff -c -r1.3 Makefile *** contrib/dbase/Makefile 2001/12/21 04:13:12 1.3 --- contrib/dbase/Makefile 2001/12/21 05:27:58 *************** *** 7,13 **** PROGRAM = dbf2pg OBJS = dbf.o dbf2pg.o endian.o PG_CPPFLAGS = -I$(libpq_srcdir) ! PG_LIBS = $(libpq) -liconv DOCS = README.dbf2pg MAN = dbf2pg.1 # XXX not implemented --- 7,19 ---- PROGRAM = dbf2pg OBJS = dbf.o dbf2pg.o endian.o PG_CPPFLAGS = -I$(libpq_srcdir) ! PG_LIBS = $(libpq) ! ! # Uncomment this to provide charset translation ! #PG_CPPFLAGS += -DHAVE_ICONV_H ! # You might need to uncomment this too, if libiconv is a separate ! # library on your platform ! #PG_LIBS += -liconv DOCS = README.dbf2pg MAN = dbf2pg.1 # XXX not implemented Index: contrib/dbase/README.dbf2pg =================================================================== RCS file: /cvsroot/pgsql/contrib/dbase/README.dbf2pg,v retrieving revision 1.2 diff -c -r1.2 README.dbf2pg *** contrib/dbase/README.dbf2pg 2001/12/21 04:13:12 1.2 --- contrib/dbase/README.dbf2pg 2001/12/21 05:27:58 *************** *** 97,113 **** fied charset. Example: -F IBM437 Consult your system documentation to see the con- ! vertions available. -T charset_to Together with -F charset_from , it converts the data to the specified charset. Default is ! "ISO-8859-1". ENVIRONMENT This program is affected by the environment-variables as used by "PostgresSQL." See the documentation of Post- ! gresSQL for more info. This program requires libiconv. BUGS Fields larger than 8192 characters are not supported and --- 97,116 ---- fied charset. Example: -F IBM437 Consult your system documentation to see the con- ! versions available. This requires iconv to be enabled ! in the compile. -T charset_to Together with -F charset_from , it converts the data to the specified charset. Default is ! "ISO-8859-1". This requires iconv to be enabled ! in the compile. ENVIRONMENT This program is affected by the environment-variables as used by "PostgresSQL." See the documentation of Post- ! gresSQL for more info. This program can optionally use iconv ! character set conversion routines. BUGS Fields larger than 8192 characters are not supported and Index: contrib/dbase/dbf2pg.c =================================================================== RCS file: /cvsroot/pgsql/contrib/dbase/dbf2pg.c,v retrieving revision 1.5 diff -c -r1.5 dbf2pg.c *** contrib/dbase/dbf2pg.c 2001/12/21 04:30:59 1.5 --- contrib/dbase/dbf2pg.c 2001/12/21 05:27:59 *************** *** 7,14 **** */ #include "postgres_fe.h" - #define HAVE_ICONV_H /* should be somewhere else */ - #include <fcntl.h> #include <unistd.h> #include <ctype.h> --- 7,12 ----
OK, that builds perfectly out of the box on Freebsd alpha and intel. Chris