-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Fixed iOS build after oibit #19729
Fixed iOS build after oibit #19729
Conversation
Fixed iOS build after oibit Reviewed-by: alexcrichton
Fixed iOS build after oibit Reviewed-by: alexcrichton
Using a type alias for iterator implementations is fragile since this exposes the implementation to users of the iterator, and any changes could break existing code. This commit changes the iterators of `VecMap` to use proper new types, rather than type aliases. However, since it is fair-game to treat a type-alias as the aliased type, this is a: [breaking-change].
Fixed iOS build after oibit Reviewed-by: alexcrichton
@@ -21,6 +21,7 @@ pub use self::_Unwind_Action::*; | |||
pub use self::_Unwind_State::*; | |||
pub use self::_Unwind_Reason_Code::*; | |||
|
|||
use core::kinds::Copy; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line caused the build to fail (unused import), so you may remove the line and use an absolute path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's weird as I had no problems building it (OS X and iOS)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, but maybe the warning level is different? The buildbot considers any warnings to be critical.
You can see the detailed error in here: http://buildbot.rust-lang.org/builders/auto-linux-64-x-android-t/builds/2691/steps/compile/logs/stdio
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, now I see. The build fails only on Android. Your #[cfg]
states "Not ARM or iOS", so it builds correctly on non-ARM targets. However it fails on ARM-based non-iOS (say, Android) targets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. The fun thing is that I can't use absolute path - in order to do that I have to use core
which will fail with the same reason. Got a pretty weird workaround with a module, will check how it builds now.
@@ -33,6 +33,13 @@ pub enum _Unwind_Action { | |||
_UA_END_OF_STACK = 16, | |||
} | |||
|
|||
#[cfg(any(not(target_arch = "arm"), target_os = "ios"))] | |||
mod copy_hack { | |||
use core::kinds::Copy; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😄 This looks a bit funny. Is it because we are currently forcing use
directives at the top of a block? I remember having seen a suggestion to relieve this restriction, but that's another story...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've finally come with another way to express it, although to be honest, I can't say I like any of available options too much.
I think this may be able to avoid the weird inner module with |
@alexcrichton |
You'll just need to add |
Hmm, I've never thought about such module hacks. |
Does Android build run tests on device or it just builds like iOS? |
It runs tests in the standard emulator (does both) |
Fixed iOS build after oibit Reviewed-by: alexcrichton
No description provided.