-
Notifications
You must be signed in to change notification settings - Fork 35
Add 16KB page size support #251
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
Conversation
Update Android native libs for 16KB page size compatibility Include x86_64 .so builds
Update Crashpad Java handler Remove Crashpad executable handler path Validate native lib directory Fix attribute lookups for 'device.sdk' and 'device.abi' Improve env var setup and database path checks
Update Unity 6 workflow version Update Unity runners with correct OS per target Update Build package Update build.yml revert build.yml
…abs/backtrace-unity into feature/16kb_support
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.
lgtm
.github/workflows/build.yml
Outdated
targetPlatform: ${{ matrix.targetPlatform }} | ||
projectPath: ${{ matrix.projectPath }}/ | ||
allowDirtyBuild: true | ||
allowDirtyBuild: true |
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.
no end of file char
Exclude Editor: 1 | ||
Exclude Linux64: 1 | ||
Exclude OSXUniversal: 1 | ||
Exclude VisionOS: 1 |
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.
If i dont have visionos support, your sdk generates error or warning. Please bring the previous meta files
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 is was generated with the minimum Unity 2021/2022.3 versions.
.meta files here follow the new standard format.
- Exclude are standard now. for older versions they would be ignored.
- Other versions do not support simulator slices or 16kb either.
var apiLevelString = backtraceAttributes["device.sdk"]; | ||
int apiLevel; | ||
if (apiLevelString == null || !int.TryParse(apiLevelString, out apiLevel)) | ||
if (!backtraceAttributes.TryGetValue("device.sdk", out var apiLevelString) || !int.TryParse(apiLevelString, out var apiLevel)) |
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.
even in the newest version of unity you can still target older .net sdks - we should stick to the previous api
return; | ||
} | ||
|
||
if (!backtraceAttributes.TryGetValue("device.abi", out var deviceAbi)) |
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.
please look at the comment
return; | ||
} | ||
|
||
// Java Crash Handler |
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.
it has java in the method name. not sure if this comment is useful
{ | ||
{ | ||
// Guard malformed attributes | ||
if (attribute.Key == null || attribute.Value == null) continue; |
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.
AddAttribute already does it
var backtraceNativeLibraryPath = Path.Combine(nativeDirectory, _nativeLibraryName); | ||
if (!File.Exists(backtraceNativeLibraryPath)) { | ||
backtraceNativeLibraryPath = string.Format("{0}!/lib/{1}/{2}", Application.dataPath, deviceAbi, _nativeLibraryName); | ||
backtraceNativeLibraryPath = $"{Application.dataPath}!/lib/{deviceAbi}/{_nativeLibraryName}"; |
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.
please see my comment regardin new .net api
|
||
// prepare native crash handler environment variables | ||
var paths = new List<string>(); | ||
if (!string.IsNullOrEmpty(nativeDirectory)) paths.Add(nativeDirectory); |
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.
please use {}
after if statement
var paths = new List<string>(); | ||
if (!string.IsNullOrEmpty(nativeDirectory)) paths.Add(nativeDirectory); | ||
var parent = Directory.GetParent(nativeDirectory); | ||
if (parent != null && Directory.Exists(parent.FullName)) paths.Add(parent.FullName); |
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.
it would be better if you can compare if this is null or empty instead
AndroidJNIHelper.ConvertToJNIArray(new string[0]), | ||
AndroidJNIHelper.ConvertToJNIArray(new string[0]), | ||
AndroidJNIHelper.ConvertToJNIArray(attachments.ToArray()), | ||
AndroidJNIHelper.ConvertToJNIArray(Array.Empty<string>()), |
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 think this is also the newest .net syntax - can you bring back my old version?
Array.Empty is a sugar syntax on top of something we already had. it's just better to be compatible with more apis
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.
your meta files now don't have end of file line
userData: | ||
assetBundleName: | ||
assetBundleVariant: | ||
assetBundleVariant: |
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.
end of file
Adds 16KB page size support
Updates Android Native Client
ref: BT-5996