Re: Reduce the number of special cases to build contrib modules on windows
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Andres Freund <andres@anarazel.de>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2020-12-23T05:46:19Z
Lists: pgsql-hackers
On Tue, Dec 22, 2020 at 11:24:40PM +1300, David Rowley wrote: > On Wed, 11 Nov 2020 at 13:44, Michael Paquier <michael@paquier.xyz> wrote: >> It seems to me that your patch is doing the right thing for adminpack >> and that its Makefile has no need to include a reference to libpq >> source path, no? > > Yeah. Likely a separate commit should remove the -I$(libpq_srcdir) > from adminpack and old_snapshot I have begun a new thread about this point as that's a separate topic. I did not see other places in need of a similar cleanup: https://www.postgresql.org/message-id/X+LQpfLyk7jgzUki@paquier.xyz > I didn't look in detail, but it looks like if we define LOWER_NODE on > Windows that it might break pg_upgrade. I guess you could say it's > partially broken now as the behaviour there will depend on if you > build using Visual Studio or cygwin. We'd define LOWER_NODE on cygwin > but not on VS. Looks like a pg_upgrade might be problematic there > today. > > It feels a bit annoying to add some special case to the script to > maintain the status quo there. An alternative to that would be to > modify the .c code at #ifdef LOWER_NODE to also check we're not > building on VS. Neither option seems nice. Hmm. It seems that you are right here. This influences lquery parsing so it may be nasty and this exists since ltree is present in the tree (2002). I think that I would choose the update in the C code and remove LOWER_NODE while keeping the scripts clean, and documenting directly in the code why this compatibility issue exists. REFINT_VERBOSE is no problem, fortunately. > I've attached the updated patch and also a diff showing the changes in > the *.vcxproj files. Thanks! > There are quite a few places where the hash table code for includes > and references gets rid of duplicates that already exist today. For > example pgbench.vcxproj references libpgport.vcxproj and > libpgcommon.vcxproj twice. The diffs look clean. dblink has lost src/backend/, there are the additions of REFINT_VERBOSE and LOWER_NODE but the bulk of the diffs comes from a change in the order of items listed, while removing duplicates. I have tested your patch, and this is causing compilation failures for hstore_plpython, jsonb_plpython and ltree_plpython. So AddTransformModule is missing something here when compiling with Python. -- Michael
Commits
-
Remove some special cases from MSVC build scripts
- 76ad24400d73 15.0 landed
-
Adjust MSVC build scripts to parse Makefiles for defines
- 245de48455da 15.0 landed
-
Don't duplicate references and libraries in MSVC scripts
- 15f16ec6511c 15.0 landed
-
Make the includes field an array in MSVC build scripts
- 33d74c5d00e4 15.0 landed
-
Use the AddFile function consistently in MSVC build scripts
- ed1884a2fede 15.0 landed
-
Remove seemingly unneeded include directory in MSVC scripts
- 4b763ff642e1 15.0 landed