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

Add support for running on Windows #233

Merged
merged 18 commits into from
Jan 9, 2024
Merged

Add support for running on Windows #233

merged 18 commits into from
Jan 9, 2024

Conversation

lfittl
Copy link
Member

@lfittl lfittl commented Jan 1, 2024

Fixes #44

@@ -0,0 +1,108 @@
# Makefile for "nmake", part of Microsoft Visual Studio Compiler (MSVC) on Windows
Copy link
Member

@seanlinsley seanlinsley Jan 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should go ahead and switch to a platform-independent build script to avoid makefile drift. Since we already include Ruby, maybe it could be a set of rake tasks?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point re: Makefile drift. I wonder if there is a way to maintain the list of tests (and examples) another way, or just discover all of them in the test/examples directories, to avoid having to add new tests to both.

Generating Makefiles could work, but I worry that the indirection (i.e. having to go through a generation script first) isn't obvious - i.e. I'd prefer if we could find a way to avoid that extra step.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about something like CMake / Meson, which generates Makefiles on *nix platforms and NMake specific Makefiles on Windows?

Copy link
Member Author

@lfittl lfittl Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Postgres itself started using Meson for this purpose, but that of course then introduces a build-time dependency (which I'm 50/50 on whether its okay to add).

I think for now I'll not adjust this since this is already a massive PR, but I think we could come back to this at a future time.

One approach to solving the duplication for tests/examples specifically could be to have a Makefile in the tests and examples folders that can run on either platform. The main motivation for not doing that in the root folder Makefile is that there are a bunch of features not supported by nmake that make it hard to write a portable Makefile that can do the code generation/etc conditionally.

@lfittl lfittl requested a review from msepga January 8, 2024 21:54
@lfittl lfittl changed the base branch from 32-bit-support to 16-latest January 9, 2024 01:03
…units

If we place them together with regular source to be compiled, we always
need to maintain a special list of files not to compile directly. Instead,
treat them as include files in the src/postgres folder.
This avoids having any .c files in the top-level src/ folder that can't
be directly compiled, and thus lets us simplify the logic for defining
which source units are to be compiled.

In passing drop the src/postgres/guc-file.c stub which is no longer used.
Previously there used to be a recurring bug where the defines suddenly
became empty when regenerating sources, due to an ordering issue. Fix by
always using the versions defined on top of the Makefile as the source.

Additionally, rewrite PG_VERSION_STR to avoid the string containing
information from the environment the source was extracted in (which
caused unnecessary diffs between different machines).
There is no reason to keep using the old version, so we might as well
upgrade, since we're about to add another invocation of this.
This removes an unnecessary whitespace that was added between the
"__thread" keyword and the type, as well improves the regexp for cases
where variables are both extern and PGDLLIMPORT.
All systems we support have it (HAVE_DECL_STRNLEN is always 1),
and it might unnecessarily conflict with built-ins in some cases.
This adds guards for HAVE_PTHREAD around any logic that requires pthread
directly (used for automatically cleaning up memory on thread exit),
removes an unnecessary include for libgen.h, and ensures structs are
initialized with "{0}", not "{}", to support C89 compilers.
…ectly

This moves the decision which includes we copy, on top of those we
discover to be needed, from the Makefile to the extract_source script,
to make it easier to manage the list.

In passing this also broadens the patterns of which atomics includes
are kept (previously we only kept x86/ARM/PPC, but it doesn't hurt
to keep all of them).
In the pg_query context these do not need to do the work they usually do
(we always output to stderr, and we never output to the client), and
mocking them avoids pulling in postmaster.c amongst other benefits. It
also avoids problems with Windows-specific code in a follow-up commit.
This uses a few steps to workaround the problem that we're not actually
running on Windows during code extraction:

- Define EXEC_BACKEND for all code extraction, since this is safe to do
  always and mainly allows us to discover some code that can be removed
- Define WIN32 when processing elog.c and mbutils.c - this causes the
  extraction script to throw a bunch of errors, but we still get an
  analysis result that detects unused code guarded by a "#ifdef WIN32"
- Explicitly add a few methods to be resolve that otherwise wouldn't
  be needed (but are needed when compiling code guarded by WIN32 defines)
- Workaround our lack of extracting port/win32 by adding some mocks for
  port/win32/signal.c directly at the end of elog.c instead
This uses the defines gathered from running Postgres' own ./configure
script on Windows under either MSVC or mingw64.

Additionally it also defines "__thread" to be "__declspec( thread )"
when compiling with MSVC.
We previously had the hardcoded macOS ("darwin") contents for this file,
which is unnecessary and incorrect. Instead keep this file empty in all
cases except on Windows, where we include port/win32.h (just like Postgres
itself does), since it is actually necessary for our use case.

Different from Postgres itself we undef PGDLLIMPORT and PGDLLEXPORT, since
they are not needed for pg_query, and PGDLLIMPORT conflicts with the
thread local attribute.

Additionally, we workaround a Windows-specific problem with sigsetjmp
defines when using clang whilst __MINGW64__ is defined (which can happen
on MSYS2-based environments).
Git automatically converts simple LFs to CRLFs on Windows when checking
out code, which appears to break the deparser tests. For now simply turn
off this behaviour for any *.sql files in the repository.
Copy link
Contributor

@msepga msepga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -0,0 +1,108 @@
# Makefile for "nmake", part of Microsoft Visual Studio Compiler (MSVC) on Windows
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about something like CMake / Meson, which generates Makefiles on *nix platforms and NMake specific Makefiles on Windows?

@lfittl lfittl merged commit 8b58b8a into 16-latest Jan 9, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows Support
3 participants