git submodules for third party libraries

359 views
Skip to first unread message

Maarten Bent

unread,
Sep 27, 2017, 1:45:39 PM9/27/17
to wx-...@googlegroups.com
Hi all,

Comments on a recent commit reminded me of my tests using git submodules for third party libraries.
Git submodules make it easier to keep these libraries up-to-date.

I added submodules for expat, jpeg, png, tiff and zlib in this branch.
I located (official and unofficial) git repositories for these libraries, forked them, added a wxwidgets branch,
and committed the changes that are currently used by wxWidgets (see tiff, jpeg, png, expat and zlib).
There are some jpeg files added and removed, and expat moved to a subdirectory, so I also had an attempt at rebaking the project files.
Eventually, everything builds successfully on my system (Windows 10, msvc and mingw).

To merge it into the main wxWidgets branch, I guess the third party libraries should be forked under the wxWidgets organization,
and a wxWidgets branch has to be created from the last official release of the library.

Regards,
Maarten

Bryan Petty

unread,
Sep 28, 2017, 9:41:38 AM9/28/17
to wxWidgets Development
On Wed, Sep 27, 2017 at 11:45 AM, Maarten Bent <maart...@gmail.com> wrote:
> Comments on a recent commit reminded me of my tests using git submodules for
> third party libraries.
> Git submodules make it easier to keep these libraries up-to-date.

Did you experiment with using git subtree to maintain them? I proposed
that here:
http://trac.wxwidgets.org/ticket/15588#comment:21

--
Regards,
Bryan Petty

Igor Korot

unread,
Sep 28, 2017, 9:45:10 AM9/28/17
to wx-dev
Bryan,
Should this ticket still be open?
What's left there?

Thank you.

>
> --
> Regards,
> Bryan Petty
>
> --
> To unsubscribe, send email to wx-dev+un...@googlegroups.com
> or visit http://groups.google.com/group/wx-dev
>

Vadim Zeitlin

unread,
Sep 28, 2017, 7:21:06 PM9/28/17
to wx-...@googlegroups.com
On Wed, 27 Sep 2017 19:45:34 +0200 Maarten Bent wrote:

MB> Comments on a recent commit reminded me of my tests using git submodules
MB> for third party libraries.
MB> Git submodules make it easier to keep these libraries up-to-date.
MB>
MB> I added submodules for expat, jpeg, png, tiff and zlib in this
MB> <https://github.com/MaartenBent/wxWidgets/tree/submodules> branch.

Wow, great, thanks a lot (and sorry for still not having had time to look
at your PR 527 and 528 -- but, on the bright side, perhaps they're not
needed any more?)!

MB> I located (official and unofficial) git repositories for these
MB> libraries, forked them, added a wxwidgets branch,
MB> and committed the changes that are currently used by wxWidgets (see tiff
MB> <https://github.com/MaartenBent/libtiff/tree/wxwidgets>, jpeg
MB> <https://github.com/MaartenBent/libjpeg/tree/wxwidgets>, png
MB> <https://github.com/MaartenBent/libpng/tree/wxwidgets>, expat
MB> <https://github.com/MaartenBent/libexpat/tree/wxwidgets> and zlib
MB> <https://github.com/MaartenBent/zlib/tree/wxwidgets>).
MB> There are some jpeg files added and removed, and expat moved to a
MB> subdirectory, so I also had an attempt at rebaking the project files.
MB> Eventually, everything builds successfully on my system (Windows 10,
MB> msvc and mingw).

Have you by chance tried building under Unix (with --disable-sys-libs to
ensure that the bundled libraries are picked up)?

MB> To merge it into the main wxWidgets branch, I guess the third party
MB> libraries should be forked under the wxWidgets organization,
MB> and a wxWidgets branch has to be created from the last official release
MB> of the library.

Yes, I'd be more ready to do this. I guess I should just fork your
repositories under wxWidgets org, right?


On Thu, 28 Sep 2017 07:41:35 -0600 Bryan Petty wrote:

BP> On Wed, Sep 27, 2017 at 11:45 AM, Maarten Bent <maart...@gmail.com> wrote:
BP> > Comments on a recent commit reminded me of my tests using git submodules for
BP> > third party libraries.
BP> > Git submodules make it easier to keep these libraries up-to-date.
BP>
BP> Did you experiment with using git subtree to maintain them? I proposed
BP> that here:
BP> http://trac.wxwidgets.org/ticket/15588#comment:21

I know that many people strongly dislike submodules and I'm not blind to
their shortcomings myself (it's hard not to be, after having used them for
a while), but they still remain the official Git way of solving this
problem and I think it would be wrong to use anything else just from the
point of view of familiarity and support. Besides, the problems with
submodules don't matter that much to us and support for them in Git has
already become much better and will undoubtedly improve even more in the
future.

Regards,
VZ

Maarten Bent

unread,
Sep 29, 2017, 4:55:17 AM9/29/17
to wx-...@googlegroups.com
I tried using git subtree for the third party libraries in this branch. It seems to work fine as well.
I guess the choice between submodules and subtrees comes down to preference.
Personally, I prefer submodules. It's less cluttered, and it's easier to see at which version the module is (it still has the tags).

I will test on Unix with --disable-sys-libs and see if it has issues.
While trying subtrees, I already noticed I made some errors with the png config files.

You shouldn't fork my repositories, I don't want to be in the middle. Just fork the official ones:
zlib: https://github.com/madler/zlib
expat: https://github.com/libexpat/libexpat
png: https://github.com/glennrp/libpng
I couldn't find official git repos for tiff and jpeg, but used the following:
tiff: https://github.com/vadz/libtiff
jpeg: https://github.com/winlibs/libjpeg

Then create a branch for the required wxWidgets changes. I guess at the latest release tag. It is probably useful to set this as the 'default' branch on github.
After testing, the required code changes and configuration files can be added using PRs, or you can commit them directly.

Regards,
Maarten

Maarten Bent

unread,
Sep 29, 2017, 1:58:25 PM9/29/17
to wx-...@googlegroups.com
Tested submodule branch on Ubuntu 16.04 with
../configure --enable-cxx11 --with-opengl --with-gtk=2 --disable-sys-libs
And it works. Ran the image sample and it still shows all the horses.

Didn't work at first. Apparently the recent update to expat 2.2.2
introduced a new HAVE_GETRANDOM check:
../src/expat/expat/lib/xmlparse.c:63:3: error: #error You do not have
support for any sources of high quality entropy enabled.
Fixed with export CPPFLAGS="-DXML_POOR_ENTROPY" before configuring.
But I guess this should be solved within wxWidgets. But it is not
relevant to this discussion about submodules.

Regards,
Maarten

Vadim Zeitlin

unread,
Nov 12, 2017, 12:01:23 PM11/12/17
to wx-...@googlegroups.com
On Fri, 29 Sep 2017 10:55:12 +0200 Maarten Bent wrote:

MB> On Wed, 27 Sep 2017 19:45:34 +0200 Maarten Bent wrote:
MB> MB> Comments on a recent commit reminded me of my tests using git submodules
MB> MB> for third party libraries.
MB> MB> Git submodules make it easier to keep these libraries up-to-date.
MB> MB>
MB> MB> I added submodules for expat, jpeg, png, tiff and zlib in this
MB> MB> <https://github.com/MaartenBent/wxWidgets/tree/submodules> branch.

Just to close this thread: I've finally integrated Maarten's changes and
all 3rd party libraries are in submodules now. Thanks again!

MB> You shouldn't fork my repositories, I don't want to be in the middle.
MB> Just fork the official ones:
MB> zlib: https://github.com/madler/zlib
MB> expat: https://github.com/libexpat/libexpat
MB> png: https://github.com/glennrp/libpng
MB> I couldn't find official git repos for tiff and jpeg, but used the following:
MB> tiff: https://github.com/vadz/libtiff
MB> jpeg: https://github.com/winlibs/libjpeg

I've used libjpeg-turbo repository instead of this one, as it's much more
active and had all libjpeg releases too. And maybe it could be worth
actually switching to libjpeg-turbo one of these days, I remember it was
already proposed quite some time ago because it's supposed to be much
faster.

As always, please let me know if you run into any problems with the
submodules or the new library versions that we now use. The good news is
that it will be much simpler to revert to the previously used version or
upgrade to the latest version with fixes from upstream if any such problems
are indeed found.

Thanks in advance!
VZ

Maarten Bent

unread,
Nov 12, 2017, 12:16:15 PM11/12/17
to wx-...@googlegroups.com
Great work.

Two comments for now:
1) stc also has the 3rd party scintilla component which could be a
submodule.
2) you removed wxjpeg_boolean from wxWidgets and jpeg sources, but it is
still in the tiff sources.

Maarten

digifuzzy

unread,
Nov 12, 2017, 12:18:42 PM11/12/17
to wx-dev
On Sunday, 12 November 2017 10:01:23 UTC-7, VZ wrote:
On Fri, 29 Sep 2017 10:55:12 +0200 Maarten Bent wrote:

MB> You shouldn't fork my repositories, I don't want to be in the middle.
MB> Just fork the official ones:
MB> zlib: https://github.com/madler/zlib
MB> expat: https://github.com/libexpat/libexpat
MB> png: https://github.com/glennrp/libpng
MB> I couldn't find official git repos for tiff and jpeg, but used the following:
MB> tiff: https://github.com/vadz/libtiff
MB> jpeg: https://github.com/winlibs/libjpeg

Is there an external repository for regex? That's another of the "baked in" dependencies.

Igor Korot

unread,
Nov 12, 2017, 12:23:27 PM11/12/17
to wx-dev
Hi,
I guess somewhere here: http://www.regexlib.com/?AspxAutoDetectCookieSupport=1.
But with introduction to C++11 I think it will soon be obsolete?

Thank you.

digifuzzy

unread,
Nov 12, 2017, 12:38:58 PM11/12/17
to wx-dev
On Sunday, 12 November 2017 10:23:27 UTC-7, Igor Korot wrote:
On Sun, Nov 12, 2017 at 11:18 AM, digifuzzy <scott.w...@gmail.com> wrote:
> Is there an external repository for regex? That's another of the "baked in"
> dependencies.

I guess somewhere here: http://www.regexlib.com/?AspxAutoDetectCookieSupport=1.
But with introduction to C++11 I think it will soon be obsolete?


I completely forgot about this:
Regex has been incorporated into the C++ standard library post-C++11.
http://www.cplusplus.com/reference/regex/

How this compares to the the regex used internally to wx I have no clue.
Throwing it out there...

Vadim Zeitlin

unread,
Nov 12, 2017, 5:19:43 PM11/12/17
to wx-...@googlegroups.com
On Sun, 12 Nov 2017 09:18:42 -0800 (PST) digifuzzy wrote:

d> On Sunday, 12 November 2017 10:01:23 UTC-7, VZ wrote:
d> >
d> > On Fri, 29 Sep 2017 10:55:12 +0200 Maarten Bent wrote:
d> >
d> > MB> You shouldn't fork my repositories, I don't want to be in the middle.
d> > MB> Just fork the official ones:
d> > MB> zlib: https://github.com/madler/zlib
d> > MB> expat: https://github.com/libexpat/libexpat
d> > MB> png: https://github.com/glennrp/libpng
d> > MB> I couldn't find official git repos for tiff and jpeg, but used the
d> > following:
d> > MB> tiff: https://github.com/vadz/libtiff
d> > MB> jpeg: https://github.com/winlibs/libjpeg
d>
d> Is there an external repository for regex?

I'm not sure, it's some really old code and I think we should replace it
with something else in the future. Using C++11 std::regex would be a
natural possibility, but I had some pretty poor experience with it when
trying to migrate an existing codebase from boost::regex to it: it was
mostly (although, annoyingly, not quite) compatible, but performance
dropped by something like 800%, which was really unacceptable (runtime went
for a representative case went from a few seconds to a minute). I'm quite
wary of the quality of std::regex implementations since then...

Regards,
VZ

Vadim Zeitlin

unread,
Nov 12, 2017, 5:26:03 PM11/12/17
to wx-...@googlegroups.com
On Sun, 12 Nov 2017 18:16:11 +0100 Maarten Bent wrote:

MB> Two comments for now:
MB> 1) stc also has the 3rd party scintilla component which could be a
MB> submodule.

Yes, indeed, and it's updated relatively often, so it would be nice to
change this. I really don't think I can afford to work on this if I want to
have any chance of making 3.1.1 before the end of the year though...

MB> 2) you removed wxjpeg_boolean from wxWidgets and jpeg sources, but it is
MB> still in the tiff sources.

Thanks for noticing, should be fixed now,
VZ

newp...@gmail.com

unread,
Nov 12, 2017, 6:55:47 PM11/12/17
to wx-dev
I think the command for updating the modules in the blog post might be wrong.  I tried `git submodule update` after the recent changes to the tiff module and it didn't seem to do anything.  After googling, I found this page and tried the command `git submodule update --recursive --remote` and it updated the tiff module for me.

I'm not super knowledgeable about git, so I could be wrong, but that's what worked for me.

digifuzzy

unread,
Nov 12, 2017, 9:28:55 PM11/12/17
to wx-dev
On Sunday, 12 November 2017 16:55:47 UTC-7, newp...@gmail.com wrote:
I think the command for updating the modules in the blog post might be wrong.  I tried `git submodule update` after the recent changes to the tiff module and it didn't seem to do anything.  After googling, I found this page and tried the command `git submodule update --recursive --remote` and it updated the tiff module for me.

I'm not super knowledgeable about git, so I could be wrong, but that's what worked for me.

What I found to work...

cd [path to wx source]

if first time:
git submodule init
will initialize all submodules in the [wx source] folder

then execute:
git submodule update
will clone or update cloned submodules in the [wx source folder]

For more details, see here for theory and here for the command.


 

Vadim Zeitlin

unread,
Nov 13, 2017, 9:20:17 AM11/13/17
to wx-...@googlegroups.com
On Sun, 12 Nov 2017 15:52:02 -0800 (PST) wrote:

> I think the command for updating the modules in the blog post might be
> wrong. I tried `git submodule update` after the recent changes to the tiff
> module and it didn't seem to do anything.

Thanks for reporting this, apparently the behaviour depends on Git
version because looking at 2.8 man page, I see, in "git submodule update"
description:

When no option is given and submodule.<name>.update is set to none,
the submodule is not updated.

which would explain what you saw.

We probably should explicitly set submodule.xxx.update to "rebase" to
make this work automatically. Or you could add "--rebase" option to the
"git submodule update" command line explicitly.

> and tried the command `git submodule update --recursive --remote` and it
> updated the tiff module for me.

Sorry, I have to reply to this to make sure anybody reading this thread in
the archives knows that the above is *not* the correct command to use. The
"--recursive" option is harmless, but useless, as we don't have any nested
submodules, but the "--remote" option is really wrong as it fetches the
latest submodule commit and not the commit actually used by the main wx
repository. Currently they are the same, but this won't always be the case
and you really want to use the submodule version corresponding to the one
recorded in the wx superproject unless you know very well what you're
doing.

Regards,
VZ

newp...@gmail.com

unread,
Nov 13, 2017, 10:32:29 AM11/13/17
to wx-dev
Thanks for clarifying that.

Maarten Bent

unread,
Nov 21, 2017, 3:16:00 PM11/21/17
to wx-...@googlegroups.com
I tested updating scintilla to a submodule in the following branch:
https://github.com/MaartenBent/wxWidgets/tree/submodule-scintilla
Using the following submodule:
https://github.com/MaartenBent/scintilla/tree/wxWidgets
Which was forked from (scintilla uses mercurial):
https://github.com/mirror/scintilla

I kept it at version 3.7.2 to keep it as simple as possible.
There was a README.TXT in the scintilla dir with upgrade instructions, I
moved it a directory up and renamed it.

Maarten

Eric Jensen

unread,
Dec 17, 2017, 5:09:21 AM12/17/17
to Vadim Zeitlin
Hello Vadim,

Sunday, November 12, 2017, 6:01:20 PM, you wrote:

VZ> On Fri, 29 Sep 2017 10:55:12 +0200 Maarten Bent wrote:

MB>> On Wed, 27 Sep 2017 19:45:34 +0200 Maarten Bent wrote:
MB>> MB> Comments on a recent commit reminded me of my tests using git submodules
MB>> MB> for third party libraries.
MB>> MB> Git submodules make it easier to keep these libraries up-to-date.
MB>> MB>
MB>> MB> I added submodules for expat, jpeg, png, tiff and zlib in this
MB>> MB> <https://github.com/MaartenBent/wxWidgets/tree/submodules> branch.

VZ> Just to close this thread: I've finally integrated Maarten's changes and
VZ> all 3rd party libraries are in submodules now. Thanks again!

Could you please add some documentation about this to the readme file
on Github?

About the fact that submodules exist, that you have to init them
and how to do it?

It took me unnecessarily long to find that information.

Thanks.
Eric



Vadim Zeitlin

unread,
Dec 17, 2017, 10:03:51 AM12/17/17
to wx-...@googlegroups.com
On Sun, 17 Dec 2017 11:09:18 +0100 Eric Jensen wrote:

EJ> Could you please add some documentation about this to the readme file
EJ> on Github?

We have this information (briefly) in BuildGit.txt and I've also updated
http://wxwidgets.org/develop/code-repository/, so I'm not sure if it's
worth mentioning it here too. But maybe we should rename BuildGit.txt to
.md and link it from here?

Regards,
VZ

Eric Jensen

unread,
Dec 17, 2017, 12:27:09 PM12/17/17
to Vadim Zeitlin
Hello Vadim,

Sunday, December 17, 2017, 4:03:48 PM, you wrote:

VZ> On Sun, 17 Dec 2017 11:09:18 +0100 Eric Jensen wrote:

EJ>> Could you please add some documentation about this to the readme file
EJ>> on Github?

VZ> We have this information (briefly) in BuildGit.txt and I've also updated
VZ> http://wxwidgets.org/develop/code-repository/, so I'm not sure if it's
VZ> worth mentioning it here too. But maybe we should rename BuildGit.txt to
VZ> .md and link it from here?

That might be enough. In any case i think it should be in a prominent
location where it's easy to find if someone suddenly encounters build
errors and has no idea what's the problem.

Eric

Vadim Zeitlin

unread,
Dec 17, 2017, 1:32:05 PM12/17/17
to wx-...@googlegroups.com
On Sun, 17 Dec 2017 18:27:08 +0100 Eric Jensen wrote:

EJ> Sunday, December 17, 2017, 4:03:48 PM, you wrote:
EJ>
EJ> VZ> On Sun, 17 Dec 2017 11:09:18 +0100 Eric Jensen wrote:
EJ>
EJ> EJ>> Could you please add some documentation about this to the readme file
EJ> EJ>> on Github?
EJ>
EJ> VZ> We have this information (briefly) in BuildGit.txt and I've also updated
EJ> VZ> http://wxwidgets.org/develop/code-repository/, so I'm not sure if it's
EJ> VZ> worth mentioning it here too. But maybe we should rename BuildGit.txt to
EJ> VZ> .md and link it from here?
EJ>
EJ> That might be enough.

Done now, please don't hesitate to open PRs if you see any obvious
improvements.

TIA,
VZ

John Roberts

unread,
Dec 18, 2017, 6:36:09 AM12/18/17
to wx-...@googlegroups.com
On 18/12/2017 1:27 AM, Eric Jensen wrote:
> Hello Vadim,
>
> Sunday, December 17, 2017, 4:03:48 PM, you wrote:
>
> VZ> On Sun, 17 Dec 2017 11:09:18 +0100 Eric Jensen wrote:
>
> EJ>> Could you please add some documentation about this to the readme file
> EJ>> on Github?
>
I had fun updating after the change to submodules but have it working
now with one exception so far.

Using the xrcdemo I get 15 unresolved externals beginning with
XML_ParserCreate from the expat lib:

wxbase31ud_xml.lib(xmllib_xml.obj) : error LNK2019: unresolved external
symbol __imp_XML_ParserCreate referenced in function "public: virtual
bool __cdecl wxXmlDocument::Load(class wxInputStream &,class wxString
const &,int)"
(?Load@wxXmlDocument@@UEAA_NAEAVwxInputStream@@AEBVwxString@@H@Z)

win10 64bit using MVS community 2015.

I am not sure whether it is my installation or otherwise. Any quick
pointers?

John

Vadim Zeitlin

unread,
Dec 18, 2017, 7:24:04 AM12/18/17
to wx-...@googlegroups.com
On Mon, 18 Dec 2017 19:36:02 +0800 John Roberts wrote:

JR> Using the xrcdemo I get 15 unresolved externals beginning with
JR> XML_ParserCreate from the expat lib:
JR>
JR> wxbase31ud_xml.lib(xmllib_xml.obj) : error LNK2019: unresolved external
JR> symbol __imp_XML_ParserCreate referenced in function "public: virtual
JR> bool __cdecl wxXmlDocument::Load(class wxInputStream &,class wxString
JR> const &,int)"
JR> (?Load@wxXmlDocument@@UEAA_NAEAVwxInputStream@@AEBVwxString@@H@Z)
JR>
JR> win10 64bit using MVS community 2015.

The "__imp" part is wrong as it means that the code expects Expat to have
been built as a DLL while we always build it as static library, but I have
no idea how is it possible because we unconditionally define XML_STATIC in
our Expat version. Please check that you don't have some other expat.h
elsewhere which gets included instead of the one under src/expat/expat/lib.

Regards,
VZ

John Roberts

unread,
Dec 19, 2017, 5:19:44 AM12/19/17
to wx-...@googlegroups.com
Thanks. There is only one expat.h and it is in src/expat/expat/lib. I
did not find where we define XML_STATIC. Within wxWidgets the only
references to XML_STATIC I find are in src/expat/expat/...

lib/expat_external.h, CMakeLists.txt, win32/Readme.txt and
doc/reference.html.

I found that a diff between my own repository and wx master showed code
from an old setup.h had been merged with include/msvc/wx/setup.h. A
checkout of this file has fixed things.

Regards, John

Reply all
Reply to author
Forward
0 new messages