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

Every second build fails due to compiler not finding things changed with DexReplace #28

Open
Lanchon opened this issue Dec 3, 2019 · 24 comments

Comments

@Lanchon
Copy link
Member

Lanchon commented Dec 3, 2019

originally filed by: @andrewleech
moved to new issue by Lanchon


often every second build fails due to the compiler not finding a bunch of things I've changed with DexReplace. I hit build again and it always works though. Have you seen this? Thinking about it now, it might be from cases


the build issue I get, it's reliable for me with this project, this commit: https://gitlab.com/alelec/fossil_smartwatches_alelec_android/tree/9c8fecfea815dbf33c68e13b5c78498bce3773b8
It should just checkout and build, I'm currently on Android Studio 3.5.1 (though I've had this issue for a long time now)
Basically build, it works. build again, it's still fine (it doesn't re-compile anything). If I make one change (even just change formatting on a file, no functional change) the build fails, with the first failure being

> Task :compileDebugJavaWithJavac
C:\Users\corona\src\fossil_app\src\main\java\com\portfolio\platform\MigrationManager.java:761: error: constructor Device in class Device cannot be applied to given types;
                Device device = new Device(
                                ^
  required: String,String,String,String,int,Integer,boolean
  found: String,String,String,String,int,int,boolean,int,<null>

In this case, jadx decompiles a number of functions like this with a couple more args than the builtin decompilers comes up with. So I've got the jadx copy sitting there completely DexIgnored and that satisfies the compiler (every second time) and function calls I make to the long version seem to work fine in practice. But every other time, the compiler only sees the jar version and complains the signature's wrong. Not the end of the world, I just compile again, but a bit odd nonetheless.

@andrewleech
Copy link

Cheers, yeah I probably should have opened a new issue from the start!

@Lanchon
Copy link
Member Author

Lanchon commented Dec 3, 2019

one workaround that might help you for now is using a 'clean assembleDebug' or similar as a build target, as shown here:

image

@andrewleech
Copy link

Ah yeah, that should work reliably - though for a larger app it's probably also slower than hitting compile twice!

@Lanchon
Copy link
Member Author

Lanchon commented Dec 3, 2019

clean building is pretty fast if you enable gradle caching. MAKE SURE YOU DO!!! i don't think that's in the docs... it should! the v2 gradle plugins fully take advantage of caching.

@Lanchon
Copy link
Member Author

Lanchon commented Dec 3, 2019

hey! i'm missing the orig/*apk. lanchon at google's email.

@Lanchon
Copy link
Member Author

Lanchon commented Dec 3, 2019

and an unrelated comment.

you are using:

  • Android Gradle plugin 3.4.2... OK
  • build tools 28... OK
  • but platform 29... hmmm

platform 29 might require newer versions of these tools. consider downgrading to 28 if your app permits.

@andrewleech
Copy link

The apk should be there, but it is checked in with LFS https://gitlab.com/alelec/fossil_smartwatches_alelec_android/blob/9c8fecfea815dbf33c68e13b5c78498bce3773b8/orig/Fossil_Smartwatches_v4.1.3.apk

If you install git lfs it should work, or you could download it directly from that link.

The original app appears to be compiled with build tools 29 so I assumed I needed to add well, haven't tried going back to 28.

@Lanchon
Copy link
Member Author

Lanchon commented Dec 3, 2019

The apk should be there, but it is checked in with LFS

but i need the apk you feed dxp-gradle. is this the file that you already put though apktool and your class renaming script?

@andrewleech
Copy link

Ah, yes it is the repacked/renamed one, this is what I feed into dxp-gradle.
I preprocess it (with apktool and smali_patch.py script) then point gradle to it, I never needs to know about the renaming. The apk is still valid and runs fine after such renaming.

When I try your new auto-renaming I'll go back to the original unmodified apk, I can update this ticket when I have it up of you'd prefer.

@Lanchon
Copy link
Member Author

Lanchon commented Dec 3, 2019

no, it's ok for investigating this issue.

meanwhile, your use of rootProject.files('orig/dex-tools-2.1-SNAPSHOT') instead of a zip file generates this error for me:

> Task :dedexAppClasses FAILED
Error: Could not find or load main class com.googlecode.dex2jar.tools.Dex2jarCmd

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':dedexAppClasses'.
> Process 'command '/home/rod/android/android-studio/android-studio/jre/bin/java'' finished with non-zero exit value 1

which is not surprising, as this dependency is processed by a zipTree(...) which shouldn't work on a straight directory. (or maybe i coded it to autodetect?)

im suprised this works for you. what platform are you working on?

@Lanchon
Copy link
Member Author

Lanchon commented Dec 3, 2019

FYI, im building using 'dexpatcher-repo.dexpatcher.dex2jar:dex-tools:2.1-20171001-lanchon@zip'. any issues with that?

i still can't build. now i'm getting lots of these:

> Task :compileDebugJavaWithJavac FAILED
error: package com.misfit.frameworks... does not exist
error: package com.portfolio... does not exist
error: cannot find symbol

what's wrong?

@andrewleech
Copy link

Yep that error is from the older dex-tools, I just switched back to your copy and sync'd it, the first error is

> Task :dedexAppClasses
dex2jar C:\Users\corona\src\fossil_app\orig\Fossil_Smartwatches_v4.1.3.apk -> C:\Users\corona\src\fossil_app\build\intermediates\dexpatcher\dedexed-classes\app-classes.jar
com.googlecode.d2j.DexException: fail convert code for Lcom/google/common/reflect/TypeToken$f;.a(Ljava/lang/Object;Ljava/util/Map;)I
	at com.googlecode.d2j.dex.ExDex2Asm.convertCode(ExDex2Asm.java:44)
	at com.googlecode.d2j.dex.Dex2jar$2.convertCode(Dex2jar.java:132)
	at com.googlecode.d2j.dex.Dex2Asm.convertMethod(Dex2Asm.java:582)
	at com.googlecode.d2j.dex.Dex2Asm.convertClass(Dex2Asm.java:441)
	at com.googlecode.d2j.dex.Dex2Asm.convertDex(Dex2Asm.java:457)
	at com.googlecode.d2j.dex.Dex2jar.doTranslate(Dex2jar.java:126)
	at com.googlecode.d2j.dex.Dex2jar.to(Dex2jar.java:275)
	at com.googlecode.dex2jar.tools.Dex2jarCmd.doCommandLine(Dex2jarCmd.java:107)
	at com.googlecode.dex2jar.tools.BaseCmd.doMain(BaseCmd.java:290)
	at com.googlecode.dex2jar.tools.Dex2jarCmd.main(Dex2jarCmd.java:33)
Caused by: java.lang.RuntimeException
	at com.googlecode.dex2jar.ir.ts.TypeTransformer$TypeAnalyze.mergeProviderType(TypeTransformer.java:613)
	at com.googlecode.dex2jar.ir.ts.TypeTransformer$TypeAnalyze.mergeTypeToSubRef(TypeTransformer.java:457)
	at com.googlecode.dex2jar.ir.ts.TypeTransformer$TypeAnalyze.copyTypes(TypeTransformer.java:560)
	at com.googlecode.dex2jar.ir.ts.TypeTransformer$TypeAnalyze.fixTypes(TypeTransformer.java:392)
	at com.googlecode.dex2jar.ir.ts.TypeTransformer$TypeAnalyze.analyze(TypeTransformer.java:369)
	at com.googlecode.dex2jar.ir.ts.TypeTransformer.transform(TypeTransformer.java:45)
	at com.googlecode.d2j.dex.Dex2jar$2.optimize(Dex2jar.java:165)
	at com.googlecode.d2j.dex.Dex2Asm.convertCode(Dex2Asm.java:449)
	at com.googlecode.d2j.dex.ExDex2Asm.convertCode(ExDex2Asm.java:41)
	... 9 more

IIRC there's a newer android feature that trips it up, but works with the newer snapshot version included in the repo.

I remember it taking a bit of trial and error (late at night) to get the gradle reference to the newer dex-tools working, but what's there builds fine in a clean checkout in CI so seems to work fine for me. Again, all the dex-tools files are checked in with LFS so may not have cloned correctly?
https://gitlab.com/alelec/fossil_smartwatches_alelec_android/-/jobs/367504097

@Lanchon
Copy link
Member Author

Lanchon commented Dec 3, 2019

Again, all the dex-tools files are checked in with LFS so may not have cloned correctly?

ok so that's the issue.

@Lanchon
Copy link
Member Author

Lanchon commented Dec 3, 2019

i was able to repro.

i have a strong suspicion that this is caused by gradle's java compile avoidance:

  • importSymbols=true causes all types such as AppType to be added to javac classpath.
  • AppType is also defined in your java sources.
  • when javac is invoked with both a java source and a classpath of the same class, javac ignores the binary and compiles the source, and uses the source to compile all other sources at the same time.
  • when you change AppType, somehow gradle is not feeding your source to the compilation of other client classes of AppType, possibly because it finds the binary via importSymbols.
  • the client classes can't compile because they depend on the AppType changes.
  • but AppType itself compiled, so its compiled class file is used for the next compilation. it masks the binary in importSymbols, and now the client classes compile.

i might be able to build a mockup of this without the dxp plugin and verify. and report it to gradle.

@Lanchon
Copy link
Member Author

Lanchon commented Dec 3, 2019

observation: you should avoid as much as possible copying code from the source app.

  1. because it is copyrighted
  2. because it can change under your feet, requiring you to update the patch.

for instance, look at AppType:

@DexReplace
public class AppType extends DynaEnum {
    public static AppType BADOO = new AppType(-1, "com.badoo.mobile");
    public static AppType CLOCK = new AppType(R.string.app_clock, "");
    public static AppType ESPN = new AppType(-1, "com.espn.score_center");
    public static AppType FACEBOOK = new AppType(-1, "com.facebook.katana");
    public static AppType FACEBOOK_MESSENGER = new AppType(-1, "com.facebook.orca");

why not this instead?

@DexEdit
public class AppType extends DynaEnum {
    @DexIgnore public static AppType USED_VALUE_1;  // only bring here the ones YOUR code depends on
    @DexIgnore public static AppType USED_VALUE_2;
    @DexAdd public static AppType MY_NEW_VALUE = new AppType(...);

    @DexAdd
    public static AppType get(String appName) {
        return AppType.get(AppType.class, appName);
    }

XXX.class can always give you trouble (compiler synthesizes fields), so you can do something like this for that:

@DexEdit
public class AppType extends DynaEnum {
    static class CLASS {
        static final Class AppType = AppType.class;
    }
 
    @DexAdd
    public static AppType get(String appName) {
        return AppType.get(CLASS.AppType, appName);
    }

@andrewleech
Copy link

Interesting... and thanks for the pointers.

In this case, with AppType, the original in the jar was a regular java enum. I brought in DynaEnum from a blog online and converted AppType to entend from it instead of enum as I wanted to make it runtime editable. I didn't think such a conversion would be possible as just a DexEdit, or more importantly the existing fields were enum entries, not static AppType fields.

But yeah I do tend to bring in more and more, even when I don't continue to use it (usually after a bit of debug tracing). Habits from my navdy project where I know the source apk's are never going to change again since the company closed.

@Lanchon
Copy link
Member Author

Lanchon commented Dec 4, 2019

the original in the jar was a regular java enum

lol of course i didn't notice this :)) sorry

I didn't think such a conversion would be possible as just a DexEdit, or more importantly the existing fields were enum entries, not static AppType fields.

interesting... it probably is possible. an enum is just syntactic sugar. the values indeed are static final XXX = constructor(of, args);. as long as you dexedit to a regular class and provided the needed constructor, the original <clinit> should initialize the original values (static fields). you just need to dexreplace and dexremove the Enum methods you don't want. just extend DynaEnum and dexremove the methods defined in the original enum and DynaEnum. dexremove will remove them from the class, so the class will inherit them from DynaEnum.

@andrewleech
Copy link

one workaround that might help you for now is using a 'clean assembleDebug' or similar as a build target, as shown here:

image

Adding a gradle [clean assembleDebug] target is definitely a step in the right direction. Do you know whether there's a way to just clean the compiler artifacts, leaving the decodeApk and dedex outputs there?
Runing [clean assembleDebug] does resolve the issue, but needing re-run the dxp apk extraction each time take build time from a few seconds up to a minute and a half.

update: scratch that, I found that by deleting build/intermediates/javac I get the same result, ie no more build failures. It also only adds a few seconds to the build time.
So in build.gradle adding

tasks.withType(JavaCompile) {
    options.compilerArgs << '-proc:none'
    project.delete(files("${buildDir}/intermediates/javac"))
}

Permanenetly fixes my issue! (I already had this task with the compilerArgs from a previous issue)

@Lanchon
Copy link
Member Author

Lanchon commented Dec 5, 2019

lol, i told you! enable the gradle build cache. clean builds will be almost instantaneous

@andrewleech
Copy link

Oh I did look into that, everything I could find said the gradle cache is automatically enabled by recent versions of Android studio. Is there something different you do?

@Lanchon
Copy link
Member Author

Lanchon commented Dec 5, 2019

i dont remember, let me take a look. what i can tell you is, the cache works for stuff like apktool decode and dex2jar. if clean build causes those tasks to run, then it is not enabled in your setup.

@Lanchon
Copy link
Member Author

Lanchon commented Dec 5, 2019

  • Run with --build-cache on the command-line
    • in your case: ./gradlew clean assembleDebug --build-cache
    • Gradle will use the build cache for this build only.
  • Put org.gradle.caching=true in your gradle.properties
    • Gradle will try to reuse outputs from previous builds for all builds, unless explicitly disabled with --no-build-cache.

@Lanchon
Copy link
Member Author

Lanchon commented Dec 5, 2019

everything I could find said the gradle cache is automatically enabled by recent versions of Android studio

i think that was the Android Gradle plugin build cache. this was a "proprietary" solution for android. later, Gradle develop their own and better Gradle-wide cache, and then Google removed the ad-hoc old build cache from the Android plugin.

@andrewleech
Copy link

andrewleech commented Dec 5, 2019

Ah, I see. I guess this is what you mean, this makes more sense: https://docs.gradle.org/current/userguide/build_cache.html

yep, [clean debug]:

BUILD SUCCESSFUL in 5s

thank you, thank you very much

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

No branches or pull requests

2 participants