Skip to content
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

Use objdump on Android to check static native libraries #746

Merged
merged 4 commits into from
Aug 21, 2020

Conversation

jperedadnr
Copy link
Contributor

Fixes #744, checking static native libraries.

Since we are testing with ProcessRunner, any invalid *.a file in the jar will log a severe message. To avoid this, I've added showSevere to ProcessRunner. In any case, the process result itself is logged to the usual log file, and if the process fails, (due to invalid format for instance), a file is logged and the message is visible

[INFO] We will now compile your code for aarch64-linux-android. This may take some time.
[INFO] Logging process [objdump] to file: HelloFX/target/client/log/process-objdump-1597658045777.log

@@ -64,37 +65,45 @@

public class AndroidTargetConfiguration extends PosixTargetConfiguration {

private static final String ANDROID_TRIPLET = "aarch64-linux-android";
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be in Constants.Profile (full name)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have Constants.PROFILE_ANDROID, but that is only for android. The target triplet is created with Triplet(Constants.Profiles.ANDROID), based on three constants, so probably we could have all possible triplets names as constants?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@johanvos Your suggestion of doing Constants.Profile (full name) brings back this PR: #229, where TripletProfile was created precisely.

I guess this is a big refactoring that should be tackled in a follow-up PR?

For now, what if we just do:

private static final String ANDROID_TRIPLET = new Triplet(Constants.Profile.ANDROID).toString();

super(paths,configuration);

this.sdk = fileDeps.getAndroidSDKPath().toString();
this.ndk = fileDeps.getAndroidNDKPath().toString();
this.hostPlatformFolder = configuration.getHostTriplet().getOs() + "-x86_64";
this.hostPlatformFolder = configuration.getHostTriplet().getOs() + "-" + Constants.ARCH_AMD64;
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this Triplet.getOsArch() ?

Copy link
Contributor Author

@jperedadnr jperedadnr Aug 19, 2020

Choose a reason for hiding this comment

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

using configuration.getHostTriplet() we get the Linux triplet, right, I'll change it

pr.showSevereMessage(false);
int op = pr.runProcess("objdump");
if (op == 0) {
return pr.getResponses().stream().anyMatch(line -> line.contains("architecture: " + Constants.ARCH_AARCH64));
Copy link
Contributor

Choose a reason for hiding this comment

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

better to replace with the getArch() on the targetTriplet (e.g. when we support x86-64 emulator/devices)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@eugener
Copy link
Contributor

eugener commented Aug 19, 2020

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- src/main/java/com/gluonhq/substrate/target/AndroidTargetConfiguration.java  1
         

See the complete overview on Codacy

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.

check native libraries architecture/format when extracting them
3 participants