WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
109052
[Qt] Use GNU ar's thin archive format for intermediate static libs
https://bugs.webkit.org/show_bug.cgi?id=109052
Summary
[Qt] Use GNU ar's thin archive format for intermediate static libs
Andras Becsi
Reported
2013-02-06 07:29:00 PST
[Qt] Use GNU ar's thin archive format for intermediate static libs
Attachments
Patch
(4.44 KB, patch)
2013-02-06 07:30 PST
,
Andras Becsi
no flags
Details
Formatted Diff
Diff
Patch
(7.10 KB, patch)
2013-02-06 09:23 PST
,
Andras Becsi
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Andras Becsi
Comment 1
2013-02-06 07:30:11 PST
Created
attachment 186851
[details]
Patch
Jocelyn Turcotte
Comment 2
2013-02-06 07:42:16 PST
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
Comment 3
2013-02-06 07:45:32 PST
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
Comment 4
2013-02-06 07:53:23 PST
(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
Comment 5
2013-02-06 09:23:23 PST
Created
attachment 186870
[details]
Patch
Andras Becsi
Comment 6
2013-02-06 09:26:18 PST
(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 7
2013-02-06 09:43:45 PST
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
Comment 8
2013-02-07 03:03:50 PST
Committed
r142088
: <
http://trac.webkit.org/changeset/142088
>
Andras Becsi
Comment 9
2013-02-07 03:04:40 PST
Comment on
attachment 186870
[details]
Patch Clearing flags from attachment.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug