Re: pg_scanner - patch no.1

Hi,

On 11/9/12, Dave Page <dpage@pgadmin.org> wrote:

> I've spotted what I think are a few issues with the build system
> changes, which will contribute to this, and possibly other problems we
> may see in the future:
>
> - I believe the above issue is caused by not adding $OSX_ARCH to the
> LDADD variable for pg_scanners. See /acinclude.m4, which sets this
> sort of thing up and ensures we get the right flags passed to every
> individual build in the code.


Missing CFLAGS="$CFLAGS $OSX_ARCH"  added.


> - You shouldn't use ../xtra/pg_scanners in Makefiles, but
> $(top_srcdir)/xtra/pg_scanners. You can use the png2c as a roughly
> equivalent guide.


pgAdmin make system is VPATH enabled.
My build script 'build-debug.sh' works as expected:

#!/bin/sh
set -v
set -e

rm -rf debug
mkdir debug

cd pgadmin3
bash bootstrap

cd ./pgadmin
./ver_svn.sh
cd ../../debug

export CXXFLAGS="-gdwarf-2 -g3"
../pgadmin3/configure --prefix=/usr/local/pgadmin3-debug
--enable-debug --enable-databasedesigner --srcdir=../pgadmin3
--with-pgsql=/home/src/postgresql-devel/20120502 >
configure-out-debug.log 2>&1
make > make-out-debug.log 2>&1
make install > make-install-out-debug.log 2>&1

exit 0


> - Do you need to define the rules for each source file explicitly in
> the Makefile? At minimum this should be a generic rule automatically
> picking up all source files in the SOURCES list - ideally there should
> be no explicit rules there at all.


xtra/pg_scanners/Makefile.am is now simplified.


> Some other questions:
>
> - At some point we're going to need to go through all this on Windows
> as well :-/. Do you have a Windows system to work on that when we get
> to it?


I am linux only man !


> - Currently the code builds scanners with "92" in the name. Is this
> intended to allow us to have multiple versions of the same scanner in
> the future? If so, then it will also need to allow for Postgres Plus
> Advanced Server and Greenplum DB scanners, which may have the same
> versions but do support different syntax. If not, we should probably
> just remove the "92" and make it as generic as possible.


Name is optional, but folder name is xtra/pg_scanners/ which means the
place for more than one.
I like "92".


> - Can you please add a README file to xtra/pg_scanners that describes
> the various files in there, and explains what needs to be updated to
> move us to a new scanner from a different version of PostgreSQL?


xtra/pg_scanners/README added.
Please Dave,  take look for some English error or bad terms. Thanks.


Best regards
Vladimir Kokovic
Belgrade, Serbia, 10.November 2012