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

Port pgsodium to Windows #37

Closed
Godwottery opened this issue Nov 3, 2022 · 9 comments
Closed

Port pgsodium to Windows #37

Godwottery opened this issue Nov 3, 2022 · 9 comments

Comments

@Godwottery
Copy link

I have ported pgsodium to Windows. It need some minor changes in order to build. It needs plenty of clean-up in order to be in such a state that it can be upstreamed.

Do these changes have a chance of being accepted? Or would you prefer to keep Windows-support out?

@michelp
Copy link
Owner

michelp commented Nov 30, 2022

I don't mind windows support at all as long as I can run the tests with it using github actions. I'm fine with code cleanups in general, what kind of upstreaming were you thinking about? I've been considering using a git pre-commit hook to enforce the postgres code linter but haven't had the time to implement it yet. I'd prefer an automation based solution to a one-time manual clean up pass.

@Godwottery
Copy link
Author

I am going to double check, but the big change was to PGDLLEXPORT to all the external functions. From what I understand this should be compatible with the other OSes as well. This should pass any automatic linters as well. It is defined empty in src/include/c.h, with the Windows specific ones in src/include/port/win32.h.

I also made some other changes that I am not sure why they were necessary, so those I would verify first. The clean-up necessary would be only in my new changes. They should not have a problem passing the postgres linter. My changes will be as small as possible, only what is strictly required will be included.

Building it requires some work (see for example https://www.highgo.ca/2020/05/15/build-postgresql-and-extension-on-windows/), which I am unsure how to incorporate in a good way.

I will work on it some more and then prepare a pull request.

@andrewwasielewski
Copy link
Contributor

@Godwottery I am very interested in Windows support for pgsodium - would you be able to share your port?

@Godwottery
Copy link
Author

I have temporarily lost access to the machine where I had my port, so I can only give you a rough outline, but it should be enough to manage for yourself.

Check https://www.highgo.ca/2020/05/15/build-postgresql-and-extension-on-windows/ to find out how to build a PostgreSQL extension on Windows.

The change to the code that is necessary is "Add PGDLLEXPORT to all the exported functions."

The issue that I was looking into when I got stuck last time was the call to popen in pgsodium.c ( ... = popen (getkey_script, "r") ... ), I do not think that this should be needed since it is also included in the portability layer, but I made some changes that I do not understand in order to get it to work.

Give it a try with the PGDLLEXPORT additions, and I'll try to have a look and see if I can access a copy of my changes.

@andrewwasielewski
Copy link
Contributor

Thanks for your response - I found that blog post/example very helpful when attempting my windows port. It seems to closely mirror this older guide also: https://www.2ndquadrant.com/en/blog/compiling-postgresql-extensions-visual-studio-windows/

After adding PGDLLEXPORT to the source code (where applicable), I am able to build the pgsodium.dll with Visual Studio 2019, however, when trying to install the extension in my database, I get following error:

CREATE EXTENSION pgsodium;
ERROR:  security label provider "pgsodium" is not loaded

I haven't spent too much time trying to debug this yet - did you see something similar when working on your port? Thanks.

@Godwottery
Copy link
Author

Yes, I had similar problems.
There are a number of possible issues that I would look at first:

  • might be that it can not find libsodium.dll,
  • might be that you have built the wrong CPU architecture (32-bit vs 64-bit),
  • or there is something that doesn't work in the initialization portion.

@andrewwasielewski
Copy link
Contributor

Thanks. After throwing in some log messages, I realized I missed adding the PGDLLEXPORT to_PG_init(). Was able to fix up the shared_preload_library section as well and get the extension working.

@Godwottery
Copy link
Author

I got access to my changes. Apart from the PGDLLEXPORT I also changed popen(getkey_script, ... to _popen(getkey_script and a number of related changes. These ones I do not understand why I made, I think it might just be a missing import that didn't pull in the Windows compatability layer properly. Did you have to make any such changes?

@andrewwasielewski
Copy link
Contributor

andrewwasielewski commented Feb 3, 2023

I originally thought that would be required too, but actually _popen is not necessary since port.h defines #define popen(a,b) pgwin32_popen(a,b) which allows spaces in the path (adds enclosing quotations). I did need to rewrite the getline functionality

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

No branches or pull requests

3 participants