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.

Sign up for GitHub

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

Jump to bottom

Jack support #743

Merged
merged 6 commits into from
Oct 5, 2016
Merged

Jack support #743

merged 6 commits into from
Oct 5, 2016

Conversation

secondsun
Copy link
Contributor

@mosabua Initial / experimental support for jack here.

YouTube Demo : https://youtu.be/0b3wGdVulIs

@secondsun
Copy link
Contributor Author

This is the WIP for #536

@mosabua
Copy link
Member

mosabua commented Oct 4, 2016

Wdyt @secondsun .. merge to master or feature branch? I am kind of thinking master ... we wont cut a release for a while and then we have it right there.

@secondsun
Copy link
Contributor Author

secondsun commented Oct 4, 2016 •
edited
Loading

@mosabua I'm fine with master if you are

mosabua
mosabua reviewed Oct 5, 2016
View reviewed changes
pom.xml
@@ -886,6 +897,13 @@
<relaxed>false</relaxed>
</configuration>
</plugin>
<plugin>
Copy link
Member

Choose a reason for hiding this comment

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

This should be removed since a checkstyle enforcement is already part of the project.

mosabua
mosabua reviewed Oct 5, 2016
View reviewed changes
src/main/java/com/simpligility/maven/plugins/android/compiler/JackCompiler.java
public String[] createCommandLine( CompilerConfiguration cc ) throws CompilerException
{
String androidHome = System.getenv( "ANDROID_HOME" );
String jackJarPath = androidHome + "/build-tools/24.0.2/jack.jar";
Copy link
Member

Choose a reason for hiding this comment

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

Can we find a betterh way to get it on the classpath?

Copy link
Member

Choose a reason for hiding this comment

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

Just checked .. I think we should use this

http://search.maven.org/#search%7Cgav%7C1%7Cg%3A%22com.android.tools.jack%22%20AND%20a%3A%22jack-api%22

if possible

@mosabua mosabua merged commit 09d0e23 into simpligility:master Oct 5, 2016
mosabua
mosabua reviewed Oct 5, 2016
View reviewed changes
Copy link
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Merged to master now. Will still have to have a closer look and so on, but we can announce it tomorrow.

src/main/java/com/simpligility/maven/plugins/android/compiler/JackCompiler.java
@Override
public String[] createCommandLine( CompilerConfiguration cc ) throws CompilerException
{
String androidHome = System.getenv( "ANDROID_HOME" );
Copy link
Member

Choose a reason for hiding this comment

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

This should be available from AndroidSDK

src/main/java/com/simpligility/maven/plugins/android/compiler/JackCompiler.java
public String[] createCommandLine( CompilerConfiguration cc ) throws CompilerException
{
String androidHome = System.getenv( "ANDROID_HOME" );
String jackJarPath = androidHome + "/build-tools/24.0.2/jack.jar";
Copy link
Member

Choose a reason for hiding this comment

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

Just checked .. I think we should use this

http://search.maven.org/#search%7Cgav%7C1%7Cg%3A%22com.android.tools.jack%22%20AND%20a%3A%22jack-api%22

if possible

pom.xml
<type>jar</type>
</dependency>
<dependency>
<groupId>org.apache.maven.plugins</groupId>
Copy link
Member

Choose a reason for hiding this comment

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

Why is this dependency actually needed? Supposedly its not good practice to depend on another plugin.

@Shusshu
Copy link
Member

Shusshu commented Oct 5, 2016

Good job!

Should the proguardMojo also have the following code?

if ( getJack().isEnabled() ) 
{
    //proguard is handled by Jack
    return;
}

@mosabua
Copy link
Member

mosabua commented Oct 5, 2016

Probably @Shusshu .. this is in master now. You should commit the fix ;-)

@Shusshu
Copy link
Member

Shusshu commented Oct 5, 2016

#744 ;) via github

@mosabua
Copy link
Member

mosabua commented Oct 5, 2016

Merged already .. haha! Going to make a changelog entry next ..

@natami
Copy link

natami commented Jan 21, 2017

Any news on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mosabua mosabua mosabua left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@secondsun @mosabua @Shusshu @natami