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

kernel: must export symbols used by other crates #1054

Merged
merged 1 commit into from Jul 1, 2018

Conversation

Projects
None yet
3 participants
@ppannuto
Copy link
Member

ppannuto commented Jul 1, 2018

Pull Request Overview

#955 was a bit too overzealous in the pubs it removed.

These are global symbols referenced by the arch crate (and the chips crate), so they must
be exported. Building with LTO hid this error, but if you disable LTO
then the linker will (correctly) complain about missing symbols as
these symbols aren't visible to the arch crate.

@JayKickliter this fixes so I can build without LTO enabled.

Testing Strategy

Compile-tested only.

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make formatall.
@brghena

This comment has been minimized.

Copy link
Contributor

brghena commented Jul 1, 2018

Should we add a non-LTO build to our continuous integration builds?

@ppannuto ppannuto referenced this pull request Jul 1, 2018

Merged

Travis: Include one debug build #1055

2 of 2 tasks complete
kernel: must export symbols used by other crates
These are global symbols referenced by the arch and chip crates, so they
must be exported. Building with LTO hid this error, but if you disable
LTO then the linker will (correctly) complain about missing symbols as
these symbols aren't visible to the other crates.

@ppannuto ppannuto force-pushed the fix-symbol-visibility branch from 7c361b4 to 83bb246 Jul 1, 2018

@bradjc

This comment has been minimized.

Copy link
Contributor

bradjc commented Jul 1, 2018

Good fix, and future improvements will consolidate those variables to just the arch crate.

@brghena

brghena approved these changes Jul 1, 2018

@ppannuto ppannuto merged commit 10b0cd0 into master Jul 1, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@ppannuto ppannuto deleted the fix-symbol-visibility branch Jul 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment