Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to specify multiple server includes towards mingw64/msys2 windows build #1129

Open
robe2 opened this issue May 1, 2023 · 4 comments
Open

Comments

@robe2
Copy link

robe2 commented May 1, 2023

I've been working on trying to get pgrx to work under windows. More specifically msys2/mingw64 which I think is an easier task to do than MSVC builds since msys2 uses unix tooling for building.

I must admit I'm a newbie in Rust, so some of my questions might be obvious. But I have a fair amount of experience with windows building extensions for PostgreSQL and am the packager of the PostGIS windows bundle builds which includes postgis, pgrouting, ogr_fdw.

My work in progress is here:

https://github.com/robe2/pgrx/tree/support-windows-via-msys2

Right now I've gotten to the point where I can build cargo-pgrx, and initialize the database with it.
I'm now stuck on the tests

https://github.com/robe2/pgrx/actions/runs/4846299083/jobs/8635750605

The part here clang diagnosed error: D:/a/pgrx/pgrx/pgsql/include/server/port.h:422:10: fatal error: 'netinet/in.h' file not found

I've had this same issue with other PostgreSQL Extension projects, and the way we fixed it is that for

mingw64 -- adding the additional includedir_server (or CPP FLAGS) needs to include the include/server/win32 folder
For MSVC it needs to include: include/server/win32_msvc

I tried setting variable

export PGRX_INCLUDEDIR_SERVER="/pgsql/include/server -I/pgsql/include/server/win32"

That clearly did something as it broke things more.

also tried

export PGRX_INCLUDEDIR_SERVER="-I/pgsql/include/server -I/pgsql/include/server/win32"

For example here is is the PostGIS snippet that handles this in the configure.ac

  dnl Extract the include flags for the backend (libpgcommon)
  PGSRV_INC=`"$PG_CONFIG" --includedir-server`
  PGSQL_BE_CPPFLAGS="-I${PGSRV_INC}"
  dnl Add $PGSRV_INC/port/win32 to MinGW build to pick up netdb.h
  case $host in
    *mingw32*)
      PGSQL_BE_CPPFLAGS="${PGSQL_BE_CPPFLAGS} -I${PGSRV_INC}/port/win32"
      ;;
  esac

Here is the equivalent in pgRouting CMakeLists

include_directories(SYSTEM ${POSTGRESQL_INCLUDE_DIR})
if(WIN32)
    include_directories(SYSTEM ${POSTGRESQL_INCLUDE_DIR}/port/win32)
    if(MSVC)
        include_directories(SYSTEM ${POSTGRESQL_INCLUDE_DIR}/port/win32_msvc/)
    endif()
endif()

P.S. I don't expect getting this to work to be a quick process, but am hopeful I can get it working in a couple of months. I think it would be well worth it since this seems to be a great project.

@eeeebbbbrrrr
Copy link
Contributor

Quickly looking at pgrx-pg-sys/build.rs, I'd kinda expect "export PGRX_INCLUDEDIR_SERVER="-I/pgsql/include/server -I/pgsql/include/server/win32"" to be the correct spelling, however!...

We then pass all of that to bindgen/clang (around line 707) as 1 argument, and I bet it needs to be split into one argument per -I.

So you might can change pg_target_include_flags to return a Result<Vec<String>> instead (around line 794) by having it split the include dir flags on whitespace and collect into a Vec. Then you'd add each entry as a clang_arg back around line 707.

That's probably where I'd go next if I were doing this. That may solve this quest, leading to another...

@robe2
Copy link
Author

robe2 commented May 2, 2023

@eeeebbbbrrrr Thanks I'll give that a try and report back once I get the right incantation of rust. I understand the general idea, just trying to grok the whole Some, Ok, overriden stuff.

robe2 added a commit to robe2/pgrx that referenced this issue May 2, 2023
and add additional include path needed for mingw64
Closes pgcentralfoundation#1129
@robe2
Copy link
Author

robe2 commented May 2, 2023

Okay I think that worked, though some of stuff I need to revisit. clang_args is supposed to be able to take a vector set so in theory I shouldn't have to have changed that but I was getting syntax errors.

My changes are here to support multiple includes

robe2@00d8384

and here to explicitly handle windows:

robe2@68b39b1

Now I have a pgxs issue, but I think that's cause I was trying to use VC build and the VC build doesn't ship with pgxs.mk files so I'll revise to just build a mingw64 pg from scratch.

BTW PostgreSQL is phasing out pgxs once they fully switch their autoconf setup to Meson building. But we can deal with that some other time as it's probably 2 years down the pipe.

@eeeebbbbrrrr
Copy link
Contributor

Now I have a pgxs issue, but I think that's cause I was trying to use VC build and the VC build doesn't ship with pgxs.mk files so I'll revise to just build a mingw64 pg from scratch.

cargo-pgrx manages custom-built Postgres installations for you and this is where all the various Makefile stuff should be coming from. On linux, this is the ~/.pgrx/ directory, in which there's a subdirectory for each Postgres version 11 through 15. IOW, you shouldn't need to "just build a mingw64 pg from scratch". pgrx needs to be finding the headers and such from what it built during cargo pgrx init.

pgrx-pg-sys has a "cshim" that is itself a little Postgres extension, and it's compiled as part of the "build.rs" process in that crate.

robe2 added a commit to robe2/pgrx that referenced this issue May 9, 2023
and add additional include path needed for mingw64
Closes pgcentralfoundation#1129
passcod pushed a commit to passcod/pgrx that referenced this issue Oct 13, 2023
and add additional include path needed for mingw64
Closes pgcentralfoundation#1129
passcod pushed a commit to passcod/pgrx that referenced this issue Oct 14, 2023
and add additional include path needed for mingw64
Closes pgcentralfoundation#1129
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants