-
-
Notifications
You must be signed in to change notification settings - Fork 621
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
Android module changes, stage 1 [defunct] #1263
Conversation
Looks like some of the tests are failing, is the order of these really important? stack traceback:
...make-core/modules/android/tests/test_android_project.lua:35: in function 'testFunction'
[ FAILED ] test_android_project.rttiOn
...make-core/modules/android/tests/test_android_project.lua:46: (4) expected:
<ExceptionHandling>Enabled</ExceptionHandling>
...but was:
<RuntimeTypeInfo>true</RuntimeTypeInfo>
fulltext:
<ClCompile>
<PrecompiledHeader>NotUsing</PrecompiledHeader>
<Optimization>Disabled</Optimization>
<RuntimeTypeInfo>true</RuntimeTypeInfo>
</ClCompile> |
the other wrong test aren't because of order but because of default values in both if the new behaviour is better you need to edit the tests. |
premake.override(vc2010, "runtimeTypeInfo", function(oldfn, cfg, condition) | ||
if cfg.system == premake.ANDROID then | ||
-- Note: Android defaults to 'off' | ||
if cfg.rtti then | ||
if cfg.rtti and cfg.rtti ~= premake.OFF then |
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.
rtti
, if not set, defaults to "Default" which will be a true
here, while the comment says the default is off
, so false
.
i don't know which one is correct but either the comment is wrong or you need to add a check for "Default".
also you'll need to change almost every test in the test_android_project
suite to look for the correct behaviour for both rtti
and exceptionhandling
.
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 believe this bit is from @TurkeyMan's PR.
the rest looks clean but since i know what sam will say i can say it now before he reads it, so you're already done by then: |
Thanks for the quick reviews, will do once I figure out the rest of the unit test issues |
@WorldofBay All of the tests pass now. |
I messed up my local checkout beyond repair after squashing with premake-core's changes from master, I don't like having a lot of merge commits so here's another PR: #1264 |
Android changes, stage 1 (#1263)
As discussed in #1251, this is a PR that contains merely fixes and minor changes, I'll do the second PR with the new functions and features once this gets merged.
Here's a quick brief of what I did:
stl
API, usingstaticruntime
to decide on shared/static.MultiProcessorCompilation
flag.androidproj
toPackaging
.NEON
support forvectorextensions
.UnwindTables
support forexceptionhandling
.targetName
not working in Packaging projects.disablewarnings
,exceptionhandling
andrtti
.cppdialect
andcdialect
, added support forC++latest
which maps to C++17 at the moment.pic
.thumbmode
.