Skip to content

fix libdispatch.a to include Swift overlay symbols #209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 20, 2017

Conversation

weissi
Copy link
Contributor

@weissi weissi commented Feb 6, 2017

The static version (libdispatch.a) that can be compiled using
./configure --enable static=yes was missing all the Swift overlay symbols
(everything in swift_overlay.o). The reason for that is that the linker is
invoked through libtool which wants .lo files but the Swift overlay got passed
as a .o file. This first of all leads to this warning:

*** Warning: Linking the shared library libdispatch.la against the non-libtool
*** objects [...]/swift_overlay.o is not portable!

And the result is that libtool doesn't include swift_overlay.o when putting
together the static library libdispatch.a.

This patch fixes this problem is a pretty crude way. The real problem is that
libtool doesn't understand Swift. So it can't be used when compiling
swift_overlay.o. In order to still get a .lo file this patch tricks libtool
into generating one from the swift_overlay.o generated by swiftc. It's very
hacky but I'm not sure what else to do.

@weissi weissi changed the title fix libdispatch.a fix libdispatch.a to include Swift overlay symbols Feb 6, 2017
src/Makefile.am Outdated
# the PIC and non-PIC case.
$(abs_builddir)/swift/swift_overlay.lo: $(abs_builddir)/swift/swift_overlay.o
mv $(abs_builddir)/swift/swift_overlay.o $(abs_builddir)/swift/.libs/swift_overlay.o.save
$(LIBTOOL) --mode=compile --tag=CC true -o $< -c foo.c
Copy link
Contributor

@dgrove-oss dgrove-oss Feb 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest using /dev/null instead of foo.c to make it clearer that your compilation command ('true') doesn't actually care about the input file you are giving it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 thanks, will fix right now

The static version (`libdispatch.a`) that can be compiled using
`./configure --enable static=yes` was missing all the Swift overlay symbols
(everything in `swift_overlay.o`). The reason for that is that the linker is
invoked through libtool which wants `.lo` files but the Swift overlay got passed
as a `.o` file. This first of all leads to this warning:

    *** Warning: Linking the shared library libdispatch.la against the non-libtool
    *** objects [...]/swift_overlay.o is not portable!

And the result is that libtool doesn't include `swift_overlay.o` when putting
together the static library `libdispatch.a`.

This patch fixes this problem is a pretty crude way. The real problem is that
libtool doesn't understand Swift. So it can't be used when compiling
`swift_overlay.o`. In order to still get a `.lo` file this patch tricks libtool
into generating one from the `swift_overlay.o` generated by `swiftc`. It's very
hacky but I'm not sure what else to do.
@weissi weissi force-pushed the jw-fix-libdispatch.a branch from 5b46774 to eaa77e9 Compare February 6, 2017 18:10
@dgrove-oss
Copy link
Contributor

looks plausible to me; it's a hack, but it is better than what we were doing. @MadCoder any thoughts?

@alblue
Copy link

alblue commented Feb 8, 2017

@swift-ci please test

@MadCoder
Copy link
Contributor

this is so gross. but I guess we need to be unblocked until we get a better build system

@MadCoder MadCoder merged commit cd12dcb into swiftlang:master Feb 20, 2017
das pushed a commit that referenced this pull request May 25, 2017
fix libdispatch.a to include Swift overlay symbols

Signed-off-by: Daniel A. Steffen <dsteffen@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants