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

Building with CMAKE on Windows #543

Closed
maxhora opened this issue Apr 28, 2017 · 15 comments
Closed

Building with CMAKE on Windows #543

maxhora opened this issue Apr 28, 2017 · 15 comments

Comments

@maxhora
Copy link

maxhora commented Apr 28, 2017

Could you please add instructions to run build with cmake on Windows.
Also it seems it's not obvious how to build shared and static versions to have both of them in one INSTALL_PREFIX directory.
Another issue, install places *.lib files into "bin" folder, but "lib" is expected on Windows.

@eustas
Copy link
Collaborator

eustas commented Apr 28, 2017

Thanks for the report.

Is there a way to have both static and shared libraries on windows. IIUC, the problem is that on Windows the result of static and shared build are both library.lib, so they can not coexist in one place?

Going to try to fix the problem ASAP. Would be very grateful, if you point to the CMake project that have those problems solved, to we could grab the best practices from there.

@maxhora
Copy link
Author

maxhora commented Apr 28, 2017

@eustas , it looks like that usual solution is to name static lib differently on Windows, so both .lib files (for shared and static builds) can reside in the same directory.
But it seems that standard naming convention for this is not available. For example, conda-forge's Windows builds add "_static" suffix. Boost adds "lib" prefix for static libs (but it might be because boost's suffixes already depends on build configuration).
Current ugly workaround to build static and shared libs with manual moving of built files can be found here https://github.com/conda-forge/brotli-feedstock/blob/master/recipe/bld.bat#L25
Some sample of how "_static" suffix can be added https://github.com/apache/arrow/blob/master/cpp/cmake_modules/BuildUtils.cmake#L151
Thank you

@eustas
Copy link
Collaborator

eustas commented Apr 28, 2017

Thanks. Going to try adding suffix soon.
Meanwhile I've split auto-cmake and cmake build manual. I hope plain CMake scenario is possible on Windows?

Don't know why we put all artifacts to bin on Windows; most likely it for historical reasons =) going to update libraries installation path very soon.

@eustas
Copy link
Collaborator

eustas commented Apr 28, 2017

I forsee one drawback with building shared+static. Unfortenately, currently we have to stick to CMake 2.8.6. This means that we can not use OBJECT libraries implemented in 2.8.8. As a consequence, sources will be compiled twice...

@maxhora
Copy link
Author

maxhora commented Apr 28, 2017

Yes, just plain cmake it's supposed way. Two runs is not an issue, the issues seems are no obvious support of static build, wrong installation paths and lack of documentation.

@eustas
Copy link
Collaborator

eustas commented Apr 28, 2017

Max, could you take a look at #544.
I've unified libraries installation across platforms. But there is an open question, what to do with includes...

@maxhora
Copy link
Author

maxhora commented Apr 30, 2017

On Windows includes should be processed similar. I guess, you can just remove if(NOT WIN32)
And ${CMAKE_INSTALL_INCLUDEDIR} should be equal to include I suppose.
So, finally, whole include directory on install will reside as ${INSTALL_PREFIX}/include/brotli/*.h

@eustas
Copy link
Collaborator

eustas commented Aug 4, 2017

Hello, Max.
Can't remember for sure, if all the problems have been resolved... Please, write back if there is something still to be done.
Now we are preparing to release v1.0.0 (finally)... so it is time to put build scripts to the perfect state =)

@maxhora
Copy link
Author

maxhora commented Aug 4, 2017

Great news @eustas ! Going to check as much as I can...

@maxhora
Copy link
Author

maxhora commented Aug 4, 2017

It seems, the libs are in expected paths on Windows now, thanks!

  1. I've noticed that it seems there is still no possibility to build static and dynamic libs in one run.

  2. Also, as you mentioned before, static and dynamic import lib files default names the same and conflicts, so can't be installed to the same directory without renaming.
    Above things seems are not critical, but in theory should a little bit simplify build process of brotli for brotli-feedstock.

  3. On Windows cmake provides "NMake Makefiles" and "Visual Studio xxx" generators. Current version of Readme.md describes the build process with gcc make tool, but it's not available with above generator on Windows. Universal cmake command line for all platforms might be as following:

mkdir build && cd build
cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=./installed  ..
# cmake command to build and install, should be universal for all platforms hopefully
cmake --build . --config Release --target install  

@maxhora
Copy link
Author

maxhora commented Aug 4, 2017

Also it's might make sense to mention in Readme.md the possibility to rename output libs for static build, if it's required. For example, in the way like it's done here.
Hope that above helps, thanks!

@gregbown
Copy link

gregbown commented Sep 15, 2017

Unsure if this is the appropriate thread for CMAKE build issues on windows, so please excuse me, I do not intend to hijack the thread.

I have attempted all the build options and they all arrive at a less than ideal conclusion. I will describe each below. Through trial and error I was able to finally build a stand alone Brotli executable...see end of post.

Autotools-style CMake The basic commands to build
mkdir out && cd out
../configure-cmake <- fails as indicated below

`C:\Users\gbown\Documents\google-brotli\brotli>mkdir out && cd out

C:\Users\gbown\Documents\google-brotli\brotli\out>../configure-cmake
'..' is not recognized as an internal or external command,
operable program or batch file.
C:\Users\gbown\Documents\google-brotli\brotli\out>`

Unable to proceed further, obviously this would apply to static build with autotools as well.
make
make test
make install

CMake The basic commands to build and install
mkdir out && cd out
cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=./installed ..
cmake --build . --config Release --target install

Build succeeded 0 Errors
No clear direction on installing or use. Dropping dll files in Windows system folder for dlls and setting path to brotli.exe appears to make it work however none of this is documented

CMake to build static libraries
mkdir out-static && cd out-static
cmake .. -DBUILD_SHARED_LIBS=OFF
make

Result

No targets specified and no makefile found:

`C:\Users\gbown\Documents\google-brotli\brotli\out-static>cmake .. -DBUILD_SHARED_LIBS=OFF
-- Building for: Visual Studio 15 2017
-- The C compiler identification is MSVC 19.11.25508.2
-- The CXX compiler identification is MSVC 19.11.25508.2
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/2017/BuildTools/VC/Tools/MSVC/14.11.25503/bin/HostX86/x86/cl.exe
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/2017/BuildTools/VC/Tools/MSVC/14.11.25503/bin/HostX86/x86/cl.exe -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/2017/BuildTools/VC/Tools/MSVC/14.11.25503/bin/HostX86/x86/cl.exe
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/2017/BuildTools/VC/Tools/MSVC/14.11.25503/bin/HostX86/x86/cl.exe -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Looking for log2
-- Looking for log2 - found
-- Configuring done
-- Generating done
-- Build files have been written to: C:/Users/gbown/Documents/google-brotli/brotli/out-static

C:\Users\gbown\Documents\google-brotli\brotli\out-static>make
make: *** No targets specified and no makefile found. Stop.
`

If I add the flag -DBUILD_SHARED_LIBS=OFF to the first set of CMake instructions, it does build a stand alone brotli.exe without errors which is usable, however; I had to sort of guess that this was how to do it, point being not all of us are full on c++ developers.

Running brotli -9 -f -o my-file.brotli -- my-file works great however there is no clear way to run tests on the newly created executable.

The -q and --quality AND -# seem to be two different ways of setting the same thing however setting -q 11 produces a smaller file than -9 and the two commands cannot be used at the same time; is there documentation on this anywhere?
-# compression level (0-9)
-q NUM, --quality=NUM compression level (0-11)

So to sum things up, to build a stand alone executable on Windows with CMake, the following works.

mkdir out && cd out

cmake -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF -DCMAKE_INSTALL_PREFIX=./installed ..

cmake --build . --config Release --target install

After this you should have a brotli.exe file that you can then set an environment path to allowing you to use brotli compression in your Windows build scripts anywhere.

@eustas
Copy link
Collaborator

eustas commented Sep 19, 2017

Thanks for the reporting.
Updated readme and cmake configuration. Please respond if it works better now. Thanks.

@gregbown
Copy link

gregbown commented Sep 27, 2017

Just to clarify, the current README documented CMake commands will create a brotli.exe and a dozen other files. The brotli.exe that is generated will function as a standalone executable and there is no need to install the .dll files in your system

image

@eustas
Copy link
Collaborator

eustas commented Sep 28, 2017

BUILD_SHARED_LIBS=OFF is no-op now. Removing it from options renders your command sequence to be exactly the same as one given in readme.

@eustas eustas closed this as completed Sep 28, 2017
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

No branches or pull requests

3 participants