Closed dannyedel closed 8 years ago
I chose the approach to have a push branch that contains every change that is supposed to be in the main project as well. My master branch contains all adjustments that were necessary on my system in order to compile with all dependencies. First of all, neither is C++ my usual programming language nor do I have experience with cmake. So possibly there are better ways to do than what I did. That said, I don't know any good package management system for Windows. There are some collections which include Windows ports of lots of packages, but they are mostly terribly outdated. So I did download every dependency by hand, made adjustments necessary for a build on Windows MSVC (which, thanks to cmake are not much. Execpt for cairo, which only works with automake; but finally, I found a cmake port). Then, I built every dependency for poppler, the library itself, boost and Qt. Due to my lack in knowledge of cmake, I went the easiest way and simply forced cmake to use these libraries using absolute paths. This should of course not be merged in the official branch, but I decided to put it on GitHub to give some idea of what's necessary to build poppler on Windows to anyone who wants to do so. Before the Qt5 port, I made use of the outdated pre-builds. The poppler-OpenJPEG/libjpeg integration is somewhat mysterious, I think resolving the JPEG rendering issue will take some time. The other difference in my master branch is the Windows help. I build it manually from the manpage file, as the man -> HTML converters either didn't work for me or their output works not very well for WinHelp. The drawback is of course that I have to keep track of changes manually, but this should not be a too big problem. There's only one "major" one-line change: When building the libaries, I used the static versions which remove any DLL dependencies. So in testhelpers.hh, I needed to comment out the dynamic-linking directive of boost. But once again, this is specific to my choice of using the static versions, so it should also be not included in the main repository. So in my next commit, I will include the Windows help files in the push branch. Do you also want to have the compiled .chm with it?
I chose the approach to have a push branch
Ah okay, I see : ) If possible, please start actual pull-requests for features that are meant to go upstream, so I see when you've got a feature or bugfix ready. (Bonus points for semantic pull-request-branch-names, like windows-patch-for-cmake
or similar. You can always have multiple topic-branches open at the same time, and sometimes it's easier to merge things individually, rather than merge one big thing. "Patch cmake" vs. "include docs" comes to mind.)
Once it's marked as a pull-request on github, I can send comments in-line and if you rebase/re-push the pullrequest, they will be marked as "commented on an older version of the pullrequest", which I find quite useful for incremental improvements. Also, pull-requests get built on travis, checking whether they break anything on the linux or mac machines.
So possibly there are better ways to do than what I did.
While that may be true, what you did works. So it's definitely a great starting point to take upstream, and start improving from there.
So for the first upstream inclusion, simply leave the hardcoded build paths in, and we'll work from there. (just box them into something like if (WIN32)/else
. You can also use if(MSVC)
for MSVC-specific compiler arguments, says the cmake wiki)
The other difference in my master branch is the Windows help. I build it manually from the manpage file
If it's written manually, I will include it, and we'll keep it up-to-date using pullrequests.
There's only one "major" one-line change: When building the libaries, I used the static versions which remove any DLL dependencies. So in testhelpers.hh, I needed to comment out the dynamic-linking directive of boost. But once again, this is specific to my choice of using the static versions, so it should also be not included in the main repository.
It's a choice, and you have proven it makes sense to link statically on windows (especially if there's no package manager that makes sure dynamically linked stuff is there on runtime, as there normally is on Linux distributions)
We can adapt the build system to leave this choice to the user: If we use a cmake option for it, for example BoostTestDynLink (default ON)
, and then, in external-libraries.cmake
where it scans for boost, make something along the lines of if(BoostTestDynLink) add_definitions(-DBOOST_TEST_DYN_LINK)
. We can then remove it from the source file completely, and the user can choose it via cmake -DBoostTestDynLink=OFF /path/to/source
step.)
Do you also want to have the compiled .chm with it?
No compilation results please, but a small script that calls the build program with the correct arguments would be nice. Then we can later integrate the call into cmake for more automation.
If its possible, please try to separate this into two pullrequests, because we might need a few iterations on one, and it's easier if each pullrequest/discussion chain is specific to one feature.
cmake
to windows (very small diff, but might need a few iterations)The main reason being that the windows help diff is very large, so it becomes easy to miss a few things while scrolling through it.
I have added a switch for static/dynamic linking of boost in #105, so that's one out of the way.
Turned out, cmake provided out-of-the-box support for "link boost statically", so I used that in the hope that it will work better across all platforms.
I don't know how to check osx, since they dont have ldd
command that linux uses to inspect a binary for dynamic links, but I hope it makes it a bit easier on windows aswell if I use a cmake builtin.
On Sat, Nov 21, 2015 at 02:11:30AM -0800, Benjamin Desef wrote: [...]
That said, I don't know any good package management system for Windows.
Did you know of ZeroInstall?
http://0install.net/
The software package is described with an xml file (download url, dependencies, checksum, ...) and is downloaded with the 0install client into a cache directory. I like this solution because everything is done locally without root/admin right.
The software package is described with an xml file (download url, dependencies, checksum, ...) and is downloaded with the 0install client
@fhgd: I did read through the website, and I didn't find any search engine for packages. Do you know if there are already packages for the build-dependencies of dspdfviewer (boost
, Qt
and the poppler
library with Qt bindings)?
On Mon, Nov 23, 2015 at 07:03:14AM -0800, Danny Edel wrote:
The software package is described with an xml file (download url, dependencies, checksum, ...) and is downloaded with the 0install client
@fhgd: I did read through the website, and I didn't find any search engine for packages. Do you know if there are already packages for the build-dependencies of dspdfviewer (boost, Qt and the poppler library with Qt bindings)?
The old and short package list is
http://0install.net/injector-feeds.html
THe newer one is
http://roscidus.com/0mirror/
http://roscidus.com/0mirror/feed-list
BTW: Since this is a decentralized package system there is no central place where all feed are listed. For me its a disadvantage...
@fhgd regarding the packages: I've gone through it, and I can't find poppler or qt4/qt5. I did find a qt3 package, but I think I must have operated it wrong (I doesn't seem right qt4 or qt5 is not packaged at all, its used by a lot of cross-platform apps these days)
@projekter The windows help is included now, all thats missing is the CMake changes.
Please box them into if (WIN32)
blocks for windows-specific changes, and if (MSVC)
blocks for msvc-specific things such as compile-flags.
You can enter newlines almost everywhere in cmake (including inside a pair of brackets), please use that to keep the (currently very long line) qt/poppler library list within a reasonable line length. (Benchmark: github's diff view should not spawn a horizontal scroll bar. One line per "C:\path\to\some.lib" optimized
is probably best.)
I was waiting with the CMake changes as "something" has to be wrong there (JPEG rendering issue). I just wanted to find out which library causes the problems (OpenJPEG or turbojpeg). In all builds, I have used OpenJPEG 2.1, but it is reported here that the 2 version might cause some issues. Yesterday, I checked building with OpenJPEG 1.5.2, which did not improve anything. I will give it another try using the conventional libjpeg instead of turbojpeg and also not using any of the JPEG libraries (which is strongly discouraged by poppler, but they provide a non-maintained fallback); this should be done until the weekend. If nothing of this is successful, debugging will need some more time. As it is quite busy at the moment, I don't think I will manage to (start to) fix it before Christmas, but of course I will provide the CMake files in the meantime.
I think it's perfectly fine to start the pullrequest with the JPEG bug still in it; Use a [WIP]
prefix (work-in-progress) then I know I'm not supposed to merge yet. (You can always edit the title later)
If I have some spare time, I might try reproducing this in a VM, maybe I can help track down the bug.
@projekter: In my attempts to reproduce building dspdfviewer
on Windows, my already big respect for your achievement has only grown. If I wouldn't know that it is possible (because you did it) I would have given up a while ago.
Long post ahead.
So far, I have not been able to compile dspdfviewer
, but I have learned a few things about Windows : )
The DLL Hell is real!
pacman
program, and their repository of open-source things seems to be huge, however coming from the Linux department their libraries are compiled with MINGW and are most likely not usable by MSVC.cmake
has excellent support for MSVC.
cmake //path/to/source
followed by cmake --build .
writes a MSVC project solution file and calls the MSVC compiler toolchain.dspdfviewer
s dependencies:
All in all, it looks like one actually has to compile everything from source, without any automation support (in the sense of packagemanager, install libfoo-dev and all of its dependencies
) when a program is not packaged for a specific MSVC version.
The main issue seems to be poppler, but I doubt I can replace this dependency on windows (As far as I remember, they don't include a PDF renderer, so there will be no "default pdf lib")
Out of curiosity, I will give the MSYS/pacman system a try, to check whether their versions of boost, qt and poppler play nicely with each other (and with jpeg files), and to see whether they can be used to compile a binary that does not require MSYS installed to run.
For MSVC, it seems I'll have to walk through the dependency chain projekter has written down and download, extract, compile, install everything by hand, to reproduce the problem.
Update: I did get it to compile quite easily using the MSYS/pacman system. But -- as expected -- the resulting binary is dynamically linked against the files in the MSYS environment. Back to working through the dependency hell, i guess.
@projekter: I give up trying to reproduce. I simply don't have enough knowledge of the windows/msvc system to get all those dependencies to work.
I did add a few things that I discovered using the MSYS toolchain, and I've gotten through to cmake
with a MSVC toolchain, but I can't manage to get the dependencies to compile, so obviously dspdfviewer fails at configure time.
Once again, massive respect for getting that to work.
I hope the MSYS related changes make things for you a bit easier too:
if (WINDOWS)
for windows-specific things in the cmake fragmentselseif(MSVC)
section in the cmake/compilers.cmake
fragment (MSYS uses the GNU GCC fragment)-fPIC
(GCC+Clang) removed from compiler-independent external_libraries.cmake
fragmentenv
on windows-DBoostStaticLink
that gets passed through to find_package(Boost)
*.lib
files (at least for now)If you can, please rebase your changes against current master (that should remove the windows help diff, plus it would include the jpeg unittest I wrote). If the test works correctly on windows, it should become easier to debug the library issue.
While it would be nice to have something before christmas, this is a spare-time project for me too (I just have a bit at the moment), so I understand if this takes a bit. Please don't feel hurried.
I think it would be a good idea to create a Poppler on Windows repository, containing all dependencies. But this is definitely a project for the next holidays. There is only one thing which I did not include in this pull request. In my former master branch, I added a string replacement of /MD into /MT for every C_FLAGS variable. I now did the replacement manually in the CMakeCache file. This is a necessary step by choice: I compiled every library in the /MT mode, so now using /MD would not work. /MD basically means that two DLLs of Microsoft have to be supplied with the application, /MT means they need not. /MD is also targeted to applications which use DLLs while /MT is rather for static linking. I decided that it is of no use if I compile every third-party DLL in static mode but then I still need to supply those runtime DLLs.
There is only one thing which I did not include in this pull request. In my former master branch, I added a string replacement of /MD into /MT for every C_FLAGS variable. I now did the replacement manually in the CMakeCache file.
This sounds like something you might want to include, but hidden behind a switch.
suggestion:
if(MSVC)
option(CompileMTMode "Build in MT mode" ON)
if(CompileMTMode)
#string replace magic goes here
endif()
endif()
Since these are compiler specific, they should go into a cmake/compiler_msvc.cmake
fragment. But you should include it, after all the whole idea of this is to add the build-on-windows things into upstream.
I think I will drop my master branch and use the push branch as new master, as rebasing lead to horrible artifacts in lots of files.
This is one of the reasons I wanted to your changes upstream. Your branches should only differ in additions (new features) that are ideally independent of each other (this makes the whole pull-request workflow a lot more fluid) and you should always be able to fast-forward your master to mine
The following basic workflow has proven successful to me:
git clone
your fork to local machinegit checkout -b some-new-feature master
some-new-feature
branch
git rebase upstream/master
(on the feature branch)upstream/master
now)git checkout master ; git merge --ff-only upstream/master
(fast-forward master)This workflow also allows you to work on two independent feature branches "at the same time", obviously that requires the build system to be completely upstreamed so you can build my master without needing any patches.
Since all pull-requests are merged, and the windows build system changes are included in master
, I'm closing this.
@projekter, you have done an amazing job porting this to windows. :thumbsup:
Looking at the diff from master to your windows port, it seems there's still a few changes necessary, and I'd like to include them in upstream, blocked into
#if defined ( _WIN32 )
andif ( WIN32 )
for sourcecode and cmake, respectively.If it's not too much to ask, It would be nice if you could make a pullrequest onto
master
of everything I can include upstream, to make both our lives easier.Also, it seems parts of your windows documentation are not auto-generated from the manpage, but manually written; please include them too.
Btw: Do you use any kind of automation or package management to get the dependencies, or did you work out the dependency list by hand? (If so, is there anything I can do to help automate things?)