mctools / mcpl

Monte Carlo Particle Lists
https://mctools.github.io/mcpl/
Other
28 stars 13 forks source link

Embedded zlib compression looks like it is not fully portable #59

Open willend opened 2 years ago

willend commented 2 years ago

Hi there,

This is intended mostly for documentation purposes...

When running MCPL on windows, e.g. using the McStas-provided Test_MCPL_output.instr, compression using zlib/gzip consistently fails with this warning message:

MCPL: Attempting to compress file voutput.mcpl with zlib MCPL ERROR: Problems encountered while compressing file voutput.mcpl.

Source mcpl file is left as is and a 0-length gzip file is written.

I consider this issue non-important since

tkittel commented 2 years ago

This might work as designed, not really sure I remember what the design was here :-)

Does the system you run on have a gzip command available?

willend commented 2 years ago

I initially had a system w/o gzip, then installed it. No difference so far.

willend commented 2 years ago

Wasn't it (at least originally) a "portable", i.e. stand-alone code-snippet that was included in the "fat" mcpl code, i.e. without dependence on external libs or executables?

tkittel commented 2 years ago

Well, I don't know about "portable" as in "works on all platforms", but it is included in the fat mcpl code. However, I am not sure if that ever worked on windows? Because I explicitly remember that I (for the sake of McStas on Windows) added the capability for the code to fall-back to using an external gzip command. It is long ago however... perhaps the usage of the fat mcpl.cc rather than the normal mcpl.cc will mean that it will never actually go and look for the external gzip command.

willend commented 2 years ago

This could very well be.

Let's see if we can figure this out together when we meet at DMSC in the near future?

tkittel commented 2 years ago

Looking at the code now, I can't find any reference to an invocation of an external gzip command, so maybe this is something that was removed again. From the error message, it seems that this internal function returns a failure:

int _mcpl_custom_gzip(const char filename, const char mode)

Is it easy for you to add some printouts or otherwise debug where this fails? Are you 100% sure it is not something as silly as a wrong filename or the dreaded spaces in paths or something?

tkittel commented 2 years ago

Posted at the same time, but yes, let us look at it together :-)

tkittel commented 2 years ago

Might be related to #40 and #57

willend commented 1 year ago

I just stumbled upon https://stackoverflow.com/questions/21322707/zlib-header-not-found-when-cross-compiling-with-mingw, which has an answer that seems to be of help:

On my cross-compilation box, I have now 1) cloned https://github.com/madler/zlib 2) Edited win32/Makefile.gcc, setting PREFIX = x86_64-w64-mingw32- 3) Run BINARY_PATH=/usr/x86_64-w64-mingw32/bin INCLUDE_PATH=/usr/x86_64-w64-mingw32/include LIBRARY_PATH=/usr/x86_64-w64-mingw32/lib make -f win32/Makefile.gcc followed by 4) sudo BINARY_PATH=/usr/x86_64-w64-mingw32/bin INCLUDE_PATH=/usr/x86_64-w64-mingw32/include LIBRARY_PATH=/usr/x86_64-w64-mingw32/lib make -f win32/Makefile.gcc install

tkittel commented 1 year ago

@willend, try the new -DMCPL_ZLIB=FETCH option for MCPL, which I created as a followup to the McStas hackaton. It might just do everything for you already (or perhaps we need a few tweaks). Assuming your cmake toolchain already sets all of the mingw stuff of course.

What MCPL_ZLIB=FETCH does is to (as you likely guessed) use FetchContent to retrive madler's source. However, it doesn't actually add any installation targets - it merely tacks all the zlib source files into the MCPL binaries. In a way this is a more proper way to do a "fat" MCPL :-)

willend commented 1 year ago

This worked! (And will be available with McStas 3.2 on Windows during December.) Screenshot 2022-11-27 at 15 10 46

tkittel commented 1 year ago

Hurray! Did you use the MCPL_ZLIB=FETCH option, or your first approach?

willend commented 1 year ago

I used MCPL_ZLIB=FETCH (but admittedly on a build machine where I had already cross-compiled the lib - so perhaps that local ZLIB was in fact used? - Will try later with a clean Linux box.)

tkittel commented 1 year ago

It should not have been, using the FETCH option should always fetch and use the srcs directly - although it never hurts to check of course.

The only potential downside I can see of the FETCH option for this use-case is if another McStas components links in another (incompatible) zlib from somewhere on the system, with resulting symbol clashes.

willend commented 1 year ago

As is, I believe no other McStas comps use zlib.

tkittel commented 1 year ago

And actually, IIRC zlib has some sort of option to prefix all symbols with a custom name. I could at some point look into using that when doing the zlib-fetch version.