Bug 109052 – [Qt] Use GNU ar's thin archive format for intermediate static libs

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Andras Becsi]

[Qt] Use GNU ar's thin archive format for intermediate static libs

[Andras Becsi]

Created attachment 186851 [details]
Patch

[Jocelyn Turcotte]

Comment on attachment 186851 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=186851&action=review

> Tools/ChangeLog:16
> +        Currently qmake does not support GNU ar's thin archive format so for

What kind of support would we need from qmake?

> Tools/qmake/mkspecs/features/default_post.prf:275
> +    gnu_thin_archives {

Why do we need the extra flag, can't we just scope it to *-g++* directly here and use it even in release?
I'm wondering if it's possible to try supporting less different configurations in the build system.

> Tools/qmake/mkspecs/features/functions.prf:28
>  defineReplace(activeBuildConfig) {
> -    CONFIG(debug, debug|release): return(debug)
> +    # The paths to the elements of the thin archive are stored

It feels like this doesn't fit with the name of the function, could you do it where activeBuildConfig is called?

[Csaba Osztrogonác]

Great patch, many thanks for it! ;) This patch decreased the size
of the WebKitBuild directory from 15Gb to 9.3G. Not mentioning
sparing IO operations after touching a simple file. (read objets
write the archive, read the archive for linking the shared object)

HDDs of the debug buildbots would be very happy. :)

[Andras Becsi]

(In reply to comment #2)
> (From update of attachment 186851 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186851&action=review
> 
> > Tools/ChangeLog:16
> > +        Currently qmake does not support GNU ar's thin archive format so for
> 
> What kind of support would we need from qmake?
An extra configuration option (see: https://bugreports.qt-project.org/browse/QTBUG-27218).
Especially because of the issues related to DESTDIR, where the library is created and then moved, which break the build with thin archives.


> 
> > Tools/qmake/mkspecs/features/default_post.prf:275
> > +    gnu_thin_archives {
> 
> Why do we need the extra flag, can't we just scope it to *-g++* directly here and use it even in release?
> I'm wondering if it's possible to try supporting less different configurations in the build system.
That would be a possibility, but because of the relative paths to the objects in the archive this is just a "glorified workaround".
I'm also hesitating to make it default for debug builds.

> 
> > Tools/qmake/mkspecs/features/functions.prf:28
> >  defineReplace(activeBuildConfig) {
> > -    CONFIG(debug, debug|release): return(debug)
> > +    # The paths to the elements of the thin archive are stored
> 
> It feels like this doesn't fit with the name of the function, could you do it where activeBuildConfig is called?

The problem is that activeBuildDir is called in a lot of places, and this would make the patch much bigger than needed, so I would rather leave this hack in there.

[Andras Becsi]

Created attachment 186870 [details]
Patch

[Andras Becsi]

(In reply to comment #5)
> Created an attachment (id=186870) [details]
> Patch

As discussed on IRC, this patch creates thin archives by default unless it is a debug_and_release build.

[Jocelyn Turcotte]

Comment on attachment 186870 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=186870&action=review

> Tools/qmake/mkspecs/features/default_post.prf:280
> +    debug_and_release: DESTDIR = $$targetSubDir())

The old isEmpty line would still work here since you added some logic inside targetSubDir that would return an empty string.

Looks good to me otherwise, r=me if you revert that line (and if what I'm saying makes any sense).

[Andras Becsi]

Committed r142088: <http://trac.webkit.org/changeset/142088>

[Andras Becsi]

Comment on attachment 186870 [details]
Patch

Clearing flags from attachment.