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

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

Open
pieper opened this issue Oct 28, 2018 · 29 comments · May be fixed by #77
Open

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

pieper opened this issue Oct 28, 2018 · 29 comments · May be fixed by #77

Comments

@pieper
Copy link

pieper commented Oct 28, 2018

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?

@neurolabusc
Copy link

neurolabusc commented Dec 18, 2019

@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
Copy link

@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.

@pieper
Copy link
Author

pieper commented Dec 23, 2019

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

@dzenanz
Copy link

dzenanz commented Dec 23, 2019

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

@neurolabusc
Copy link

neurolabusc commented Dec 23, 2019

@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.

@dzenanz
Copy link

dzenanz commented Dec 23, 2019

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.

@neurolabusc
Copy link

@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.

@ningfei
Copy link

ningfei commented Jan 2, 2020

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.

@neurolabusc
Copy link

@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.

@madler
Copy link
Owner

madler commented Jan 2, 2020

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.

@neurolabusc
Copy link

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
Copy link

@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.

@kjk
Copy link

kjk commented Feb 4, 2020

@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.

@pieper pieper mentioned this issue Feb 4, 2020
@neurolabusc
Copy link

@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
Copy link

@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
Copy link

@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 added a commit to neurolabusc/pigz that referenced this issue Feb 22, 2020
@neurolabusc neurolabusc linked a pull request Feb 27, 2020 that will close this issue
@pieper
Copy link
Author

pieper commented Mar 10, 2020

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.

@neurolabusc
Copy link

@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.

@sergeevabc
Copy link

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

@rafadess
Copy link

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.

@madler
Copy link
Owner

madler commented Feb 23, 2021

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

@neurolabusc
Copy link

neurolabusc commented Feb 23, 2021

@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.

@ningfei
Copy link

ningfei commented Feb 23, 2021

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

@dieperdev
Copy link

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

@neurolabusc
Copy link

@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.

@sergeevabc
Copy link

sergeevabc commented Apr 16, 2024

@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.

@DanielRuf
Copy link

@neurolabusc maybe the repo needs an update, since pigz 2.8 is available now.

Also I think it would be helpful to have a GitHub Actions setup that compiles the binary (for tagged commits). With this we don't need a full local development environment just for compiling the new binaries.

@DanielRuf
Copy link

Personally I would like to see a setup like GNU uses which is quite simple (using bootstrap, config and make).

https://github.com/DanielRuf/gzip-windows-build/blob/main/.github/workflows/main.yml

@neurolabusc
Copy link

@DanielRuf it has been 53 months since I submitted PR#77. I am reluctant to invest time if there is no chance of my efforts being merged - I did target the current pigz when I made my PR. I am happy to put effort into this if there is a chance that the subsequent PR has a chance of being part of the main project. @madler can you provide any guidance on how to proceed. I think many would benefit if pigz could support non-Latin characters for the Windows operating system.

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 a pull request may close this issue.

10 participants