pigz icon indicating copy to clipboard operation
pigz copied to clipboard

cross platform support (e.g. with CMake)?

Open pieper opened this issue 7 years ago • 29 comments

We'd be interested in using pigz as a library in our application and would want to support Windows, Linux, and Mac with a uniform build system that supports native compilers (e.g. Visual Studio, not mingw). CMake has been our go-to solution for that so it would be great if pigz had CMake support.

Has anyone looked into CMake for pigz before?

pieper avatar Oct 28 '18 23:10 pieper

@pieper can you try the branch below which adapts @ningfei's Cmake super build scripts. These scripts have been used for years with dcm2niix. One clever feature is you can specify the ZLIB_IMPLEMENTATION to be System or Cloudflare (the default). The Cloudflare zlib accelerates pigz 40%, though it requires an x86-64 CPU built since 2009.

You can try this out with the following command:

git clone https://github.com/neurolabusc/pigz.git
cd pigz
mkdir build && cd build

I would be happy if @madler would consider including this Cmake script in the main pigz repository (though he might choose whether the System zlib should be the default).

Since the Cmake uses the glibc of your system, it would be great to test if these address issue 68. None of my computers have ever exhibited that issue, it would be great to know that it has been addressed.

neurolabusc avatar Dec 18 '19 13:12 neurolabusc

@pieper the current CMake scripts support Unix (MacOS and Linux), but not Windows. There is a fork of pigz that supports Windows, albeit v2.3.1 versus v2.4. @ningfei's dcm2niix scripts do support Windows, so maybe he can can extend this to pigz. It would be nice to have Windows support be part of the main trunk and even better if users can leverage the Cloudflare zlib.

neurolabusc avatar Dec 18 '19 13:12 neurolabusc

See also the discussion here: https://discourse.vtk.org/t/cloudflare-zlib/2330/7

pieper avatar Dec 23 '19 20:12 pieper

pigz is a program and not a library, therefore it cannot really replace zlib.

dzenanz avatar Dec 23 '19 21:12 dzenanz

@dzenanz, you are correct that pigz is a program and not a library. However, on Unix you can use it piped mode. After calling popen you can write to pigz using the same routines that you would to write to an uncompressed file, closing it with pclose. Please take a look at my modification of znzlib where a few lines of code allows one to select pigz output using the same functions designed to write uncompressed data. That project demonstrates how a copy of pigz compiled with CloudFlare zlib provides optimized parallel compression with very little changes to the code and no changes to the make script. In my example, I simply detect an environment variable to see if the user wants to write the data to pigz.

neurolabusc avatar Dec 23 '19 21:12 neurolabusc

About 50% of programmers use Windows, and that percentage is much greater for ordinary users. If you don't have to support Windows (for whatever reason), PIGZ is fine. If you do (as all multi-platform libraries aim to do), PIGZ is out of the question.

dzenanz avatar Dec 23 '19 21:12 dzenanz

@dzenanz point taken. I evaluated znzlib since it is used by several of the most popular tools in my field (AFNI, FSL, FreeSurfer) and those tools only target Unix (though many Windows users run them via Windows Subsystem for Linux). I do not people have ported pigz to Windows, though I do not know if they have added modern CMake support or if pipes work with those. Using a pipe does have performance benefits. @ningfei is a Cmake guru who developed the dcm2niix scripts to work on Windows, Linux and MacOS as well as the Unix pigz CMake from my repository. Your comments might give him the incentive to look at making a CMake script for pigz that includes all the operating systems.

neurolabusc avatar Dec 23 '19 21:12 neurolabusc

Not really a CMake guru :D As I searched, currently there are only binaries available for the latest pigz for Windows. The open source port might be not up-to-date. Will have a look when I have time.

ningfei avatar Jan 02 '20 08:01 ningfei

@ningfei thanks for looking into this. As you note, forks of pigz that support the Microsoft compiler are based on pigz 2.3.1, which predates piped storage and the notes go up to Visual Studio 2012 (and remake instead of CMake). There are Windows compatible executables for pigz 2.4 built using gcc on MSYS2. I know the recent versions of Microsoft compilers have improved their compatibility with GCC conventions. Popular tools in my field (FSL, AFNI) only support Unix (though they run fine on Windows Subsystem for Linux), and the vtk thread linked above suggests the developers of that tool prefer libraries over stand alone applications.

@madler can you tell us how to proceed. To summarize @pieper asked for an enhancement for pigz to support CMake for deploying to MacOS, Linux and Windows. @ningfei and I provide a CMake script that supports MacOS and Linux (and optionally optimized versions of zlib). However, this script does not support Windows. The options are:

  1. If you request, I can generate a pull request to add the CMake script to your repository.
  2. You could relabel this issue as an enhancement and keep it open until some developer steps up to extend the CMake to support Windows.
  3. You could close the issue, as it is an enhancement and not a bug, and outside of the remit of your project.

neurolabusc avatar Jan 02 '20 12:01 neurolabusc

Sure, you can generate a pull request for a CMake script, and I'll include it. However it sounds like your script does not resolve the original request for Windows support.

madler avatar Jan 02 '20 12:01 madler

You are correct, the CMake script currently only supports MacOS and Linux. It has the general benefits of CMake (detecting capabilities, installing missing components) and liabilities (requiring CMake), plus the added benefit of simplifying building with optimized zlib. So it would be a simple alternative to the current make file.

Before I make a pull request, I make a call out to Windows developers to see if anyone wants to extend the CMake script to Windows. @kjk do you have any interest in this?

neurolabusc avatar Jan 02 '20 13:01 neurolabusc

@madler I need some guidance on how to proceed with the pull request. There are three options:

  1. The existing CMake makes minimal changes to pigz, targets Unix and MacOS. Allows the user to select CloudFlare-zlib, System or zlib-ng. Since the core code is virtually untouched, this has low risk of unintended consequences.

  2. The windows branch includes the minimal changes to compile to Windows (while retaining compatibility with MacOS and Linux). At the moment, this can be built from the command line, but I think @ningfei could get a CMake working with Windows pretty easily. Since the core code is virtually untouched, this also has a low risk of unintended consequences. However, it does have a clear known limitation: it requires that filenames use Latin characters, and will fail with non-Latin values, e.g. pigz Описание.pdf returns pigz.exe: skipping: ????????.pdf does not exist.

  3. I am willing to create a more thorough modification that will handle Windows files. But this course is only worth pursuing if you are willing to thoroughly vet the changes and would consider incorporating the changes in the main release. Given the large Windows community, I do think bugs could be quickly identified, but the code will have a lot of ifdefs that will make it harder to maintain. Specifically, the char * type will need to be wchar_t * . I think this is the simplest robust solution, but it will create widespread changes throughout the code. Below is a minimal example to demonstrate the changes.

// arg.c
// compile with: 
// cl /EHsc /TP arg.c
// arg  Описание.pdf

#include <io.h> // _setmode()
#include <fcntl.h> //_O_U16TEXT
#include <stdio.h>
                        
// For local functions and globals.
#define local static

#if defined(_MSC_VER)
local void process(wchar_t* arg) {
 wprintf(L"processing '%wS'\n", arg);
#else
local void process(char *path) {
 printf("processing '%s'\n", arg);
#endif
}

#if defined(_MSC_VER)
int wmain(int argc, wchar_t* argv[]) {
#else
int main(int argc, char **argv) {
#endif
    _setmode(_fileno(stdout), _O_U16TEXT);
    for (int i = 1; i < argc; ++i) {
    	process(argv[i]);
    }
 }

I am happy to follow any path you recommend, would just like to know your preference. I appreciate all your work to the community, and respect that ultimately you must maintain this code.

neurolabusc avatar Feb 04 '20 20:02 neurolabusc

@neurolabusc to minimize use of #ifdef and amount of changes I would recommend using utf8 on windows

Do WCHAR -> utf8 at the beginning of wmain.

Do utf8 => WCHAR conversions in leaf functions like "file open". That way most of the code is unchanged.

kjk avatar Feb 04 '20 21:02 kjk

@kjk would you be interested in trying to implement this? I was worried there would be a lot of changes, including the --name storage and in try.c. Maybe I just have the wrong approach. You could make a fork of my branch, will has the minimal Windows support without handling non-Latin characters. Here are the commands to download my fork and compile on Windows (assuming you are running the "x64 Native Tools Command Prompt":

 git clone --branch windows https://github.com/neurolabusc/pigz
 cd pigz\src\win
 msvc.bat

I have very little experience with MSVC, so having someone with more experience would be greatly appreciated.

neurolabusc avatar Feb 05 '20 21:02 neurolabusc

@kjk, @dzenanz and @pieper I would be grateful if you could try out my fork's latest release. I have implemented @kjk's suggestion of converting WCHAR -> utf8 at the beginning of wmain. This results with minimal changes and in my testing handles both Latin and non-Latin filenames. Since utf8 is used internally, I believe the --name works as intended. I am not an experienced Windows programmer, and would appreciate any suggestions as well as testing.

Assuming everyone is happy with the Windows compilation, the final step is to implement CMake for Windows. I have updated @ningfei's CMake script to compile with System, Cloudflare, Intel or ng variants of zlib. I have also created a simple script to compile, test and benchmark these variants. The file msvc.bat shows how simple it is to compile pigz on Windows, but my experience with CMake is very limited. I realize you are busy, but if @ningfei or @pieper can help update CMake to handle Windows, I think this will result in a compelling pull request.

neurolabusc avatar Feb 15 '20 16:02 neurolabusc

@jcfr would you be interested in helping @pieper out on this. I notice you do a lot of the CMake commits for your joint tools. The new pigz CMake scripts work on Linux and MacOS, and a tiny command line script compiles this for Windows. However, I am very unfamiliar with CMake, and it is unclear to me how to link the pthread library for Windows. Failing that, can you suggest a Windows CMake guru who can help out. I notice that other tools like Blender use CMake to link the Pthread library on Windows.

neurolabusc avatar Feb 20 '20 16:02 neurolabusc

Hi @neurolabusc , sorry to leave you hanging on this but it's not really in my critical path and I know how much time it can take to iron out all the details when you mix windows, cmake, and legacy code. It's not that it cannot be done, thing just tend to pop up over time so long term maintenance may always be an issue (for example we keep a fork of libarchive in Slicer that ends up needing debugging from time to time on various platforms.

pieper avatar Mar 10 '20 19:03 pieper

@pieper it should be fully functional now. I no longer need any help. I have submitted a Pull Request so just waiting for @madler to review.

neurolabusc avatar Mar 10 '20 19:03 neurolabusc

Could you be so kind to generate the latest .exe for the rest of us who are mere Windows users w/o compiler?

sergeevabc avatar Feb 20 '21 23:02 sergeevabc

I'm really looking forward to see the PR implemented to the source code. We use that windows implementation of pigz but it is kind of outdated. So would be awesome if we could build for windows from the original repo. I'm subscribing this topic and the PR to receive any news. Thank you for this implementation.

rafadess avatar Feb 23 '21 19:02 rafadess

If someone would like to make an .exe, I can include it in the distribution. I don't have a Windows machine.

madler avatar Feb 23 '21 20:02 madler

@madler you can get a Windows executable from here: https://github.com/neurolabusc/pigz/releases This was built using pull request 77. This pull request has been waiting one year for review. I realize free projects rely on people's free time, but please tell me if there is any way I can expedite the review of that PR. It provides a CMake system, provides Windows threading, and supports Windows UTF16 filenames. It does not change any of the internal logic, so I think the chance of unintended consequences is low.

The executable is v2.4, which was current when the PR was submitted. I am happy to recompile to v2.6 once the PR is merged.

neurolabusc avatar Feb 23 '21 21:02 neurolabusc

Not fully tested but two versions here: one built using MingW GCC 10.2, one built using Visual Studio 2019 Version 16.8. Both based on the latest code of pigz 2.6 and PR #77 (I merged them locally), using the CloudFlare zlib with intrinsics enabled. I've actually been trying to setup a CI pipeline so we can easily get the executables, but don't have enough time to finish it... pigz.zip

ningfei avatar Feb 23 '21 22:02 ningfei

@neurolabusc Do you have an executable with large file support? I'm getting this error when I try to compress a 40gb file: image

dieperdev avatar Apr 15 '24 21:04 dieperdev

@dieperdev I submitted my pull request to support Windows appropriately four years ago, and it has yet to be reviewed. My sense is that supporting this platform is not a priority for the lead developer. I really only want to support this if it can be made part of the core code base to support all users and be maintained. Therefore, I am focusing my development efforts elsewhere. Happy to renew my work if @madler or another maintainer wants to merge my contributions or suggest how my work could be improved to allow a merge.

neurolabusc avatar Apr 16 '24 10:04 neurolabusc

@ningfei Not fully tested but two versions here: pigz.zip

Both of them crash on my end (Core2Duo with SSE3). Not sure, but maybe due to the need for AVX instructions.

@neurolabusc you can get a Windows executable from here: https://github.com/neurolabusc/pigz/releases

pigz_Windows.zip works, pigz_Windows_CloudFlareZlib.zip crashes.

sergeevabc avatar Apr 16 '24 12:04 sergeevabc