ar linker: generate thin archives for uninstalled static libraries #9294
Conversation
| return self.std_args | ||
| def get_std_link_args(self, is_thin: bool) -> T.List[str]: | ||
| # not supported by armar | ||
| # FIXME: osx ld rejects this: "file built for unknown-unsupported file format" |
There was a problem hiding this comment.
I don't understand this OSX failure.
From https://github.com/eli-schwartz/meson/runs/3693121858
cc -o prog prog.p/prog.c.o -Wl,-dead_strip_dylibs -Wl,-headerpad_max_install_names -Wl,-undefined,error libhelper.a
ld: warning: ignoring file libhelper.a, building for macOS-x86_64 but attempting to link with file built for unknown-unsupported file format ( 0x21 0x3C 0x74 0x68 0x69 0x6E 0x3E 0x0A 0x2F 0x20 0x20 0x20 0x20 0x20 0x20 0x20 )
Undefined symbols for architecture x86_64:
"_func", referenced from:
_main in prog.c.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Codecov Report
@@ Coverage Diff @@
## master #9294 +/- ##
=======================================
Coverage 67.29% 67.29%
=======================================
Files 394 394
Lines 85326 85326
Branches 17455 17455
=======================================
Hits 57421 57421
Misses 23229 23229
Partials 4676 4676 Continue to review full report at Codecov.
|
eli-schwartz
force-pushed
the
ar-thin-archive
branch
from
e34b9f9
to
46463f4
Compare
|
Featurewise this is good, just have to ensure that it does not cause regressions for some use cases. (No, I don't know what those would be, just thinking out loud.) |
There was a problem hiding this comment.
We use a lot of convenience libraries for systemd, and this PR makes great improvements:
$ ls -l build{,-meson}/src/core/*.a
-rw-r--r-- 1 zbyszek zbyszek 10208766 Oct 1 14:47 build/src/core/libcore.a
-rw-r--r-- 1 zbyszek zbyszek 29218 Oct 3 14:13 build-meson/src/core/libcore.a
$ ls -l build{,-meson}/src/basic/libbasic.a
-rw-r--r-- 1 zbyszek zbyszek 3120020 Oct 1 14:46 build/src/basic/libbasic.a
-rw-r--r-- 1 zbyszek zbyszek 35110 Oct 3 14:12 build-meson/src/basic/libbasic.a
Me likes ;)
| self.std_args = ['csr'] | ||
| stdargs += 'D' | ||
| if '[T]' in stdo: | ||
| thinargs = 'T' |
There was a problem hiding this comment.
thinargs += 'T' (for consistency)
| self.std_args = [stdargs] | ||
| self.std_thin_args = [stdargs + thinargs] |
There was a problem hiding this comment.
Maybe write this as
self.std_args = ('csr' +
('D' if '[D]' in stdo else ''))
self.std_thin_args = (self.std_args +
('T' if '[T]' in stdo else ''))I think it's easier to read.
eli-schwartz
force-pushed
the
ar-thin-archive
branch
from
46463f4
to
fb290f7
Compare
|
Did some basic reorganization while I was at it, since I disliked having to use |
|
This pull request introduces 1 alert and fixes 2 when merging fb290f7 into ba432c8 - view on LGTM.com new alerts:
fixed alerts:
|
eli-schwartz
force-pushed
the
ar-thin-archive
branch
2 times, most recently
from
bd51e62
to
ad3b2e7
Compare
|
Not a new alert, lol, just a moved one. Still, why not fix that too... |
The `init__()` method basically existed solely to be overridden by every derivative class. Better to use it only in the class that needs it. This fixes several warnings, including missing calls to init because we skipped ArLinker due to not wanting it... also get rid of a pointless popen return code saved as pc, which we never checked.
eli-schwartz
force-pushed
the
ar-thin-archive
branch
from
ad3b2e7
to
795a4a4
Compare
|
This pull request fixes 2 alerts when merging 795a4a4 into ba432c8 - view on LGTM.com fixed alerts:
|
Since they will never be used outside of the build directory, they do not need to literally contain the .o files, and references will be sufficient. This covers a major use of object libraries, which is that the static library would potentially take up a lot of space by including another copy of every .o file. Fixes mesonbuild#9292 Fixes mesonbuild#8057 Fixes mesonbuild#2129
eli-schwartz
force-pushed
the
ar-thin-archive
branch
from
795a4a4
to
0a3a9fa
Compare
|
This passed CI before I added release notes. |
|
This pull request fixes 2 alerts when merging 0a3a9fa into ba432c8 - view on LGTM.com fixed alerts:
|
Since they will never be used outside of the build directory, they do not need to literally contain the .o files, and references will be sufficient.
This covers a major use of object libraries, which is that the static library would potentially take up a lot of space by including another copy of every .o file.
Fixes #9292
Fixes #8057
Fixes #2129