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

Windows and Darwin support #268

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aikawayataro
Copy link

The current makefile is not portable. These changes will allow to compile the library under Windows (cygwin/MinGW/MSYS) and Darwin.
Tested with cygwin, MinGW, MSYS2, macOS Sonoma, Debian

@aikawayataro
Copy link
Author

By the way, any thought about to add optional CMake support? I'd be happy to help with that.

@LoupVaillant
Copy link
Owner

Thanks for the effort.

I believe the additions to the .gitignore are good. The modifications for the makefiles however, feel like overkill veering on scope creep. CMake is likewise out of scope, though if you fork Monocypher and add a CMake on top, I add a link to your work from the download page under the "third party distributions" section (it will stay there as long as you maintain your fork).

Here’s my philosophy: the main way to put Monocypher into your project is to copy & paste the files from src/ (that’s 2 or 4 files), and put them directly in your own project. If you’re using C or C++ your build system should handle that out of the box. If you’re using another language you’ll need to write or use bindings, at which point I consider you competent enough to compile 1 or 2 C files on your platform. Ideally you should compile and run the test suite as well, but if you start from the tarball it should be fairly easy. (The tarball version has the test vectors already generated, which eliminate all external dependencies besides libc.)

Thus, the makefile is a convenience that mostly helps me develop Monocypher, though I did go out of my way to make it as portable as possible to facilitate third party distributions.


I believe your patch comes from a different vision of portability than mine, one that may be inevitable when you start supporting outliers like Windows: I try my hardest to be platform agnostic, while your patch is platform aware. Personally that’s where I draw the line: I can stop using GNU make extensions to make BSD users happy, but writing dedicated code for a specific OS feels like going too far.

The same reasoning prevents me from providing an RNG, managing nonces myself, or locking memory. Platform specific stuff is out of scope.


That being said, we still have options for Windows and Darwin. Make allows us to override all its variables, and we could use that to our advantage. Reading your patch, Windows support requires the following:

  • A static library file that ends in .lib instead of .a (wait, you used .dll.a…).
  • A shared library file that ends in .dll istead of .so.
  • Different directories for the static and shared libraries (seriously??).
  • Test executables that end in .exe instead of being arbitrary.

Darwin seems to need similar customisation and on top of that a different way to generate the shared library (which you encoded in SOFLAGS).

I believe we can avoid the OS specific paths, and instead let users override the relevant Makefile variables. We do however need to provide enough variables to override in the first place:

  • LIBNAME: libmonocypher.a by default. Assigned with =.
  • SONAME: libmonocypher.so.4 by default. Assigned with =.
  • SOLATEST: libmonocypher.so by default. Assigned with =.
  • EXESUFFIX: .out by default (.exe on Windows). Assigned with =.
  • LIBDIR: where the static library goes. Assigned with ?=.
  • SOLIBDIR: where the shared library goes. Assigned with ?=.
  • SOFLAGS: -Wl,-soname,$(SONAME) by default. Assigned with =.

(The choice of = and ?= is kind of arbitrary, but I’m trying to keep the ?= to a standard minimum. The others can always be explicitly overridden. Also note the absence of SOVERSION, which I think is not really needed if we modify the soname directly.)

Then we modify the makefile to support all these variables. Once we do, Windows and Darwin users should be able to call make with the proper options and everything should work. What options exactly should probably be documented in the README (I can accept platform specific instructions there).

What do you think of this approach?

@aikawayataro
Copy link
Author

aikawayataro commented Nov 13, 2023

I understood your philosophy, thanks for clarifying.

while your patch is platform aware

Yeah, but for Windows there's no simple solution :)

Windows support requires the following:

A static library file that ends in .lib instead of .a (wait, you used .dll.a…).

Not true for gcc on Windows. And I believe you are not familiar with Windows development here. You got the suffixes a bit wrong. The .dll.a suffix is an implib for dynamic library. In Windows world we have the implibs - the static library for linking .dll library to executable, and for static libraries (for gcc) the static library has just .a. Not simple like in the UNIX world, where imports generated on-fly from .so or .dylib. So for Windows you need two static libraries - one library is real static library (libmonocypher.a), and the second just an implib (libmonocypher.dll.a) for dynamic library (DLL loader + symbol resolver).

Then we modify the makefile to support all these variables. Once we do, Windows and Darwin users should be able to call make with the proper options and everything should work. What options exactly should probably be documented in the README (I can accept platform specific instructions there).

The thing that's there's no way to handle it via variables at least for Windows. Because we have the implib and the DLL should be put in bin not the lib (Easy way to cooperate with stupid Windows' dynamic resolving). Darwin use its own prefixes and soname convention too...

I fully agreed with you in scope of your project and philosophy, so at this moment my changes are invalid.
What do you think about separate makefiles for each target? Like makefile.mingw or makefile.darwin, still platform-aware but... maybe it's more OK for you.

@aikawayataro
Copy link
Author

I just want to have a nice way to compile and package your library. Anyways if it's all bad I can just create a repo with makefiles for Windows/Darwin strugglers like I am :).

@LoupVaillant
Copy link
Owner

I believe you are not familiar with Windows development here

That’s an understatement. 🤣 I know nothing of Windows development, and the only times I actually developed on Windows the build magic was already done for me, and I never touched the Windows API.

Now I confess I have a hard time deciphering your patch, and right now I’m too lazy to apply it on a local copy on a Windows machine. So, could you share the output of a make install from scratch? I want to know the exact commands that need to be called for a Windows build to happen, and how you make it so. And unless it’s clear from the commands, I also need to know which exact files are generated and installed.

My hope is that despite the differences, we can perhaps extract a common structure between Linux, Windows, and Darwin, so users can make it work if they override enough variables.

@aikawayataro
Copy link
Author

That’s an understatement. 🤣 I know nothing of Windows development, and the only times I actually developed on Windows the build magic was already done for me, and I never touched the Windows API.

Yeah MS has great buildsystem but it's limited and not libre. I prefer to use gcc on Windows, unix-like environment do the best thing :) but there's still many things to keep in mind as described before...
There's also Microsoft Make implementation called "NMake" but it can't do half of what GNUMake does...

So, could you share the output of a make install from scratch?

No mandoc for Windows... What about make install-lib?

$ make install-lib PREFIX=./testprefix
c99 -O1 -I src -I src/optional -fPIC -c -o lib/monocypher.o src/monocypher.c
c99 -O1 -I src -I src/optional -fPIC -c -o lib/monocypher-ed25519.o src/optional/monocypher-ed25519.c
ar cr lib/libmonocypher.a lib/monocypher.o lib/monocypher-ed25519.o
c99 -O1  -shared -Wl,-soname,libmonocypher-4.dll,--out-implib,lib/libmonocypher.dll.a -o lib/libmonocypher-4.dll lib/monocypher.o lib/monocypher-ed25519.o
mkdir -p ./testprefix/include
mkdir -p ./testprefix/lib
mkdir -p ./testprefix/bin
cp -P lib/libmonocypher.a lib/libmonocypher.dll.a   ./testprefix/lib
cp -P lib/libmonocypher*.dll                        ./testprefix/bin
cp -P src/monocypher.h                              ./testprefix/include
cp -P src/optional/monocypher-ed25519.h             ./testprefix/include

As you can see, the key steps steps are:

  1. Pack the static (.a) library.
  2. Compile the dynamic (.dll) library and export implib (.dll.a) for future linking.
  3. Skip the symlink step (NTFS "symlinks" are not good and the implib will load correct dynamic library itself).
  4. Copy .dll into bin instead (The way to handle Windows' dynamic linking without bug-prone PATH appending).

@aikawayataro
Copy link
Author

aikawayataro commented Nov 13, 2023

Ahead of the question about Darwin here is the same for it:

% make install-lib PREFIX=./testprefix
cc -pedantic -Wall -Wextra -O3 -march=native -I src -I src/optional -fPIC -c -o lib/monocypher.o src/monocypher.c
cc -pedantic -Wall -Wextra -O3 -march=native -I src -I src/optional -fPIC -c -o lib/monocypher-ed25519.o src/optional/monocypher-ed25519.c
ar cr lib/libmonocypher.a lib/monocypher.o lib/monocypher-ed25519.o
cc -pedantic -Wall -Wextra -O3 -march=native  -shared -install_name libmonocypher.4.dylib -current_version 4 -compatibility_version 4 -o lib/libmonocypher.4.dylib lib/monocypher.o lib/monocypher-ed25519.o
ln -sf `basename lib/libmonocypher.4.dylib` lib/libmonocypher.dylib
mkdir -p ./testprefix/include
mkdir -p ./testprefix/lib
cp -P lib/libmonocypher.a lib/libmonocypher*.dylib  ./testprefix/lib
install_name_tool -change libmonocypher.4.dylib ./testprefix/lib/libmonocypher.4.dylib        \
        ./testprefix/lib/libmonocypher.4.dylib
cp -P src/monocypher.h                              ./testprefix/include
cp -P src/optional/monocypher-ed25519.h             ./testprefix/include

That's why we need SOVERSION - for Darwin's -current_version and -compatibility_version. Different suffix, no soname but install name and extra step to adjust it after install.
Anyways soname is optional but if we do the right thing for Linux, why not to do the right thing for Windows/Darwin :)?

@aikawayataro
Copy link
Author

I think the only good solution is separate makefiles then.
Let me know if you have something in mind.

@LoupVaillant
Copy link
Owner

Wait a minute, I’m not entirely done with the idea. This PR is likely to linger for a while, but the initial intent is good even if I’m not sure about the execution. If that’s okay with you I’m not closing it until I reach a definite decision.

Plus, your patch has valuable information in it that I didn’t have before, so whatever I decide, if I do anything to support Windows and MacOS you’ll have a commit under your name.

I’ll try to play around and let you know when I have a stronger opinion.

@LoupVaillant LoupVaillant reopened this Nov 18, 2023
@aikawayataro
Copy link
Author

If that’s okay with you I’m not closing it until I reach a definite decision.

No problem, I just wanted to mark my changes as not acceptable at this moment.

Plus, your patch has valuable information in it that I didn’t have before, so whatever I decide, if I do anything to support Windows and MacOS you’ll have a commit under your name.

Sure I'd be happy to help. MSVC support (if planned) definitely requires a separate makefile.

For now, I'm stick with my patch to install Monocypher on my systems, so I'm good :)

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.

2 participants