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
Show quoted text
-----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 directoryLooks 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 19026Attachments:
/bjm/difftext/plainDownload
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
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:
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
"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 directoryLooks 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, Pennsylvania 19026Attachments:
/bjm/difftext/plainDownload
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);
}
"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.hAnd 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, Pennsylvania 19026Bruce 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
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, Pennsylvania 19026Bruce 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, Pennsylvania 19026Bruce 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 += -liconvUntested 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 19026Attachments:
/bjm/difftext/plainDownload
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 ----