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

Fixes to allow for MINGW32 compilation #193

Merged
merged 1 commit into from Jan 15, 2016
Merged

Conversation

jp-bennett
Copy link
Collaborator

@mrash @damienstuart This does seem to fix the issues I was having. I thought I had this all fixed without some of these changes, but with further work, this does all seem to be needed.

mrash added a commit that referenced this pull request Jan 15, 2016
@mrash mrash merged commit dbd5ae7 into mrash:master Jan 15, 2016
@damienstuart
Copy link
Collaborator

On my mac and Linux-based systems, It appears the the configure script errors with:
configure: error: conditional "MINGW" was never defined.
Usually this means the macro was only invoked conditionally.

@jp-bennett
Copy link
Collaborator Author

@damienstuart configure.ac probably should set MINGW to false before that case statement. Not sure how I missed that, will finish it up once I get home.

@damienstuart
Copy link
Collaborator

Yes. Just committed that change,

@jp-bennett
Copy link
Collaborator Author

Great. Sorry to make you clean up after me on this one.

@damienstuart
Copy link
Collaborator

No sweat.

On another note, I was testing autoreconf -i, configure, make, make clean, and make distclean, and make distcheck. I just found that make distclean and make distcheck are also broken. It appears to be a related to the change in how the common files are referenced/accessed.

@mrash
Copy link
Owner

mrash commented Jan 16, 2016

I should have helped on the testing side - I just eyeballed the change and merged from my phone (the "codehub" app on the iPhone is really cool btw). No worries.

@damienstuart
Copy link
Collaborator

What's happening is the inclusion of the ../common/xxx files from the lib/Makefile.am makes autoconf treat them as files to be tracked and cleaned up. If the MINGW conditional (which I am renaming to USE_MINGW) evaluates to false, automake will not include them for compilation, but still includes the files that would have been generated by them for cleanup.

The problem is, those files are already tracked and cleaned via their inclusion in the common/Makefile.am file. So during the clean phase, 2 different Makefiles have references to those files. So, the server/Makefile will clean them because it is called before the common/Makefile. When the common/Makefile hits its distclean target, those files are not there.

Jonathan, is the reason the references to those files are in lib/Makefile.am for MINGW builds because you need to include files from the ../win32 directory? If so, I think we need to try moving that build logic to the common/Makefile.am for libfko_util.a and see if we can get that to work.

I will try arranging it that way, and if I have time, I will try getting a MINGW setup installed on my Windows 7 VM to test it - and/or check it in and have you try it in your environment.

What do you think?

@jp-bennett
Copy link
Collaborator Author

Lots going on here. I think I have a better understanding of it all now.

The first issue is that the getlogin.* files in win32 are not in any of our Makefiles. I can copy them into the source root by hand, and they are picked up that way. I was hoping to avoid this step by conditionally adding them to the list of source files in lib/Makefile.am. This may still be a viable idea, not sure. This will probably be the easiest fix.

The second issue is that when building libfko in mingw, and then trying to build fwknop-gui the same way, the functions from common/* are showing up as undefined references during linking. I thought I had this sorted earlier, but apparently files were left over from an old compile and giving false positives.

I believe I now understand this second issue. libfko_util.a is not linked into the libfko static library, libfko.a. It is supposed to be linked into the dll, but a separate issue discussed below breaks the dll build. I am not currently using the dll, but the static libs. So the common/* functions really aren't in libfko.a, because they aren't being linked in until the dll is created. I think my patch above was simply adding the missing functions to libfko.a, and still ignoring libfko_util.a.

Also, libfko_util.a is not being installed to my /usr/i686-w64-mingw32/sys-root/mingw/lib/ folder, so even if I were looking for it while building fwknop-gui, it wouldn't get picked up.

A third issue is that to build a dll, the LDFLAG -no-undefined has to be added. Best reference here: http://article.gmane.org/gmane.comp.gnu.mingw.user/18727 I'm currently getting a warning about undefined functions in dlls, and no dll built. This should probably be dependent on win32 and not added to all platforms.

Once the dll is being built correctly, I can ship it with fwknop-gui instead of compiling libfko in statically. This would likely fix most of the problems I've had. The fact that libfko.a is incomplete might be an issue to fix, though. It seems like a better solution than the whole archive import should exist.

I'm also fighting yet another problem with mingw in trunk. With the dns functions being added to common/fko_util.c, we suddenly need to link against some windows libs for windows compilation. The functions used require -lwsock32 -lws2_32 as LDFLAGS. The problem is that these flags need to go after the the archive import of libfko_util.a in the link command. Even though I add them to the proper place in lib/Makefile.am, the link command line is re-arranged before it is executed, putting these flags too early in the command to properly. I have yet to find a solution, other than either running the final link command by hand, or removing the archive import LDFLAG and specifying all the common/* files in libfko_source_files.

Conclusion, I think my change, the conditional inclusion of the common/* files, should be removed. Perhaps the win32/* files should stay, conditional on MINGW. -no-undefined should probably be added to LDFLAGS, conditional on win32.

@damienstuart
Copy link
Collaborator

I think we can add the MINGW conditionals to build libfko as a static library (with libfko_util linked in as well). It is just a matter of working out the flags for compilation and linking for that mode. My thinking is the ../win32/getlogin.* files and '-lwsock32 -lws2_32' linker options should be in common/Makefile.am. Also the getlogin.* files would be defined in Makefile.am as "EXTRA_libfko_util_a_SOURCES".

Since we are reworking this - and the current AM_CONDITIONAL for MINGW in configure.ac is wrong (my bad in how I added setting it to false actually evaluates to true), I will be checking in an update to master with changes as described above.

I have been able to clean up enough on my windows VM to install MinGW - If you can send me any info on how yours is setup and the process you are using for building libfko, it will save me some time in getting setup to help on this.

@jp-bennett
Copy link
Collaborator Author

Putting those options and flags in common/Make file.am would work, I think.

I have been using mingw in Fedora. There are packages there to install the entire toolchain quite easily. I spun up a windows 7 VM last night, will see about building libfko and the GUI client there this afternoon. I'll keep notes and share once I make it work.

@damienstuart
Copy link
Collaborator

I have checked those changes into Master. I have tested the configure, build, clean, and distclean functions work on the Centos 6, Mac, and Windows builds.

@jp-bennett
Copy link
Collaborator Author

@damienstuart After way too much trial and error, I have a working mingw32 toolchain on a Windows7 VM. The best solution right now is the msys2 project. I'm using the 32 bit version for maximum compatibility. Grab it here: http://msys2.github.io/

Once installed, don't launch the msys2 shell. Instead, use the "MinGW-w64 Win32 Shell" Start by running "update-core", installing those updates, and then closing that shell and reopening it.

From there, run
pacman -S mingw-w64-i686-toolchain
pacman -S git make mingw-w64-i686-make mingw-w64-i686-libtool texinfo autoconf automake
git clone https://github.com/mrash/fwknop.git

From here, we could go ahead and compile, but neither the dll nor the static dll are built properly. The flag "-no-undefined" must be added to the lib's LDFLAG string in order to enable dll building. This should eventually be added as either a mingw or win32 conditional, but for now I'm just tacking it onto lib/Makefile.am as such:
libfko_la_LDFLAGS = -version-info 2:4:0 $(GPGME_LIBS)
-export-symbols-regex '^fko_' -no-undefined
-Wl,--whole-archive,$(top_builddir)/common/libfko_util.a,--no-whole-archive

From there, to build use:
sh autogen.sh
./configure --disable-server --disable-client --disable-stack-protector
mingw32-make

On this system, I'm still getting the undefined references to the winsock stuff. I was able to copy the final link command and add -lwsock32 -lws2_32 after the archive import, giving me a valid dll.a and dll. Installing these by hand did allow me to successfully build and run fwknop-gui. It would be super useful if we could get that dll to build correctly without having to do it by hand.

@jp-bennett
Copy link
Collaborator Author

@damienstuart I believe I've found a suitable fix. I'll finish testing and open a pull request asap.

@jp-bennett jp-bennett deleted the mingw32-fixes branch January 19, 2016 02:56
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.

None yet

3 participants