Module support for -javaagent arguments to dex command #5645
Conversation
|
Oops, this is the whole line that should be added to their timodule.xml if they want -javaagent: |
|
I've created https://jira.appcelerator.org/browse/TIMOB-18114 to correspond to this PR. @kennr Have you signed the CLA? Would you also update this PR so that it's in sync with the current master? |
| @@ -3806,7 +3813,11 @@ AndroidBuilder.prototype.runDexer = function runDexer(next) { | |||
| '--output=' + this.buildBinClassesDex, | |||
| this.buildBinClassesDir, | |||
| path.join(this.platformPath, 'lib', 'titanium-verify.jar') | |||
| ].concat(Object.keys(this.moduleJars)).concat(Object.keys(this.jarLibraries)); | |||
There was a problem hiding this comment.
Why did this line need to be changed? It appears to be changed for no reason. I prefer it be restored.
|
@skypanther Thank you! I'm waiting for the OK from our end before signing the CLA, but in the meantime, I've updated the PR so it's in sync with the current master. @cb1kenobi I've since added comments in the PR explaining a little why this changed. The -javaagent arg needs to come before the rest of the dexArgs, otherwise it won't work for our dex process. As you'll see at the bottom of our change, the line is restored, just with the dexArgs updated. Here is our latest module that you can test the dexAgent option with. You'll notice the android module's timodule.xml file now has the dexAgent property in it: Thanks! |
|
@kennr |
Cleaned this up by putting the concat() call back.
|
Attention: We do not appear to have a record of @kennr signing the contributors license agreement (CLA). This is a required step for all open-source contributions. If you are the contributor, please visit Appcelerator CLA for more information. If you believe you signed already and you have received this message in error, please email cla@appcelerator.com and we will address it promptly. |
|
@ingo @cb1kenobi The CLA has been signed by the necessary parties at New Relic today, December 18th. Please let me know if there's anything else that needs to be done for this to move forward. Thanks! |
|
CR looks good. Needs a FR. @kennr how do we test this? |
|
@cb1kenobi Here is our latest module that you can test the dexAgent option with. You'll notice the android module's timodule.xml file now has the dexAgent property in it: You can also verify this PR doesn't break by building and running the Kitchen Sink app or any other app with a module that doesn't have the dexAgent property. If you'd like to verify the data you're sending to New Relic, I can set you up with a trial account (that's not public). Let me know if you need anything. Thanks! |
|
@cb1kenobi Just checking in on the status of this. Do you need anything else from us on this PR or are we good to go? Thanks! |
|
@kennr We will be testing this next sprint. Thanks! |
|
Closing this PR in favor of the above linked one. Thanks! |
Added option for module developers to specify a property name of "dexAgent" with a jar file name in their timodule.xml. This allows for prepending the dexArgs in the runDexer function with "-javaagent:", which is necessary for some modules.
@cb1kenobi @ingo
We tried using the build hook system, but it does not allow for the correct placement of "-javaagent" in the dexArgs. This is a generic solution for any module developers who need to add -javaagent. They put the .jar file in their modules directory add this line to their timodule.xml:
class.foo.jar