Proguard contrib module #972

sbly · 2020-10-05T16:26:54Z

Create Proguard contrib module

This module will run proguard on the normal assembly output jar to shrink it. By default all steps - shrink, optimize, obfuscate, and preverify - are run though this can be configured. The default entrypoint is finalMainClass. Any additional options can be specified freely with additionalOptions (proguard has many).

lefou

Thank you for your contribution. The module looks ok to me. I added some comments I'd like you to address.

Also to accept this new contrib module, we need some documentation under docs/pages/9 - Contrib Modules.md (modules in alphabetical order). Having some Scaladoc for all public targets would greatly help later users of that module too.

build.sc

contrib/proguard/src/Proguard.scala

lefou

For consistency, if you use an library from an explicit Java version (via JAVA_HOME) you should also call the Java executable from this specific version.

build.sc

sbly · 2020-10-06T16:59:45Z

@lefou Thanks for the review! I've addressed most of your feedback. The one thing I'm a little confused about is this:

if you use an library from an explicit Java version (via JAVA_HOME) you should also call the Java executable from this specific version.

Does Mill use JAVA_HOME or does it get the standard library from elsewhere? I'm having trouble figuring out how the code is compiled in the first place

lefou · 2020-10-06T19:33:36Z

@lefou Thanks for the review! I've addressed most of your feedback. The one thing I'm a little confused about is this:

if you use an library from an explicit Java version (via JAVA_HOME) you should also call the Java executable from this specific version.

Does Mill use JAVA_HOME or does it get the standard library from elsewhere? I'm having trouble figuring out how the code is compiled in the first place

Mill uses JAVA_HOME, but I was referring to the call out to the java executable in the proguard target. It's not necessarily the same as $JAVA_HOME/bin/java and therefore problematic.

lefou · 2020-10-06T19:36:55Z

contrib/proguard/src/Proguard.scala

+    val outJar = PathRef(T.dest / "out.jar")
+
+    val cmd = os.proc(
+      "java",


Better use the java executable found under javaHome, to make it consistent with the later used classpath.

sbly · 2020-10-07T16:26:05Z

Thanks for the help. I switched to using the java binary under JAVA_HOME, and also added documentation.

lefou

Looks good to me. Do you plan to also add some ScalaDoc and/or tests anytime soon? If not I can merge now, but those additions would help greatly in comsumption and maintenance of this contrib module.

sbly · 2020-10-08T17:45:07Z

@lefou I can add some ScalaDoc. I was testing using just the scratch/folder. I wasn't sure how to write automated tests for this but I can look at some examples and try to do it. We can hold off on merging for now

lefou

Scaladoc looks good. I found two other subtle improvements that are specific to mills caching.

lefou · 2020-10-08T19:32:12Z

contrib/proguard/src/Proguard.scala

  def javaHome: T[PathRef] = T {
    PathRef(Path(System.getProperty("java.home")))
  }


We need to mark this target as input, otherwise mills caching would hide potential changes of the system property.

Suggested change

def javaHome: T[PathRef] = T {

PathRef(Path(System.getProperty("java.home")))

}

def javaHome: T[PathRef] = T.input {

PathRef(Path(System.getProperty("java.home")))

}

OK, I'll make the change. But I'm wondering now if this should actually be Sources? What's the difference?

Sources and also Source are specializations of Input. Also Source(s) support the watch mode (mill -w), whereas Input is not able to trigger a re-build on it's own.

contrib/proguard/src/Proguard.scala

sbly · 2020-11-02T15:03:30Z

@lefou Hi, I finally had some time to work on this again. I added some basic tests. I am ready to merge this now if that's OK with you

lefou · 2020-11-02T16:54:46Z

Thank you @sbly !

lefou · 2020-11-27T08:41:19Z

I just notices, that our test setup wasn't running the proguard tests. Running them locally gives me the following error:

[263/263] contrib.proguard.test.testCached 
-------------------------------- Running Tests --------------------------------
mill.contrib.proguard.ProguardTests: Using experimental parallel evaluator with 4 threads
mill.contrib.proguard.ProguardTests: [#1] [2/2] proguardClasspath 
+ mill.contrib.proguard.ProguardTests.proguard.should download proguard jars 1504ms  
mill.contrib.proguard.ProguardTests: Using experimental parallel evaluator with 4 threads
mill.contrib.proguard.ProguardTests: [#0] [40/52] libraryJars 
mill.contrib.proguard.ProguardTests: [#3] [9/52] mill.scalalib.ZincWorkerModule.classpath failed
mill.contrib.proguard.ProguardTests: [#2] [41/52] mill.scalalib.ZincWorkerModule.worker failed
mill.contrib.proguard.ProguardTests: [#2] [42/52] compile failed
mill.contrib.proguard.ProguardTests: [#2] [43/52] finalMainClassOpt failed
mill.contrib.proguard.ProguardTests: [#1] [44/52] localClasspath failed
mill.contrib.proguard.ProguardTests: [#3] [45/52] prependShellScript failed
mill.contrib.proguard.ProguardTests: [#0] [47/52] manifest failed
mill.contrib.proguard.ProguardTests: [#2] [46/52] finalMainClass failed
mill.contrib.proguard.ProguardTests: [#1] [48/52] upstreamAssembly failed
mill.contrib.proguard.ProguardTests: [#2] [49/52] entryPoint failed
mill.contrib.proguard.ProguardTests: [#1] [50/52] assembly failed
mill.contrib.proguard.ProguardTests: [#1] [51/52] inJar failed
mill.contrib.proguard.ProguardTests: [#1] [52/52] proguard failed
X mill.contrib.proguard.ProguardTests.proguard.create proguarded jar 1741ms 
  scala.MatchError: Left(Failure(
  Resolution failed for 1 modules:
  --------------------------------------------
    com.lihaoyi:mill-scalalib-worker_2.13:0.9.3-20-ea5b88 
        not found: /home/lefou/.ivy2/local/com.lihaoyi/mill-scalalib-worker_2.13/0.9.3-20-ea5b88/ivys/ivy.xml
        not found: https://repo1.maven.org/maven2/com/lihaoyi/mill-scalalib-worker_2.13/0.9.3-20-ea5b88/mill-scalalib-worker_2.13-0.9.3-20-ea5b88.pom
  ,None)) (of class scala.util.Left)
    mill.contrib.proguard.ProguardTests$.$anonfun$tests$7(ProguardTests.scala:44)
    mill.contrib.proguard.ProguardTests$.$anonfun$tests$7$adapted(ProguardTests.scala:43)
    mill.contrib.proguard.ProguardTests$.workspaceTest(ProguardTests.scala:33)
    mill.contrib.proguard.ProguardTests$.$anonfun$tests$6(ProguardTests.scala:36)
1 targets failed
contrib.proguard.test.testCached mill.contrib.proguard.ProguardTests mill.contrib.proguard.ProguardTests.proguard.create proguarded jar

sbly · 2020-12-17T05:02:16Z

Hi @lefou. Sorry about being super late getting to this, I don't regularly check Github when I'm not actively working on a PR.

So locally, I would always run mill -i all __.publishLocal before testing. If I didn't, I got the same error as you above. I thought this was expected and was just the way mill's tests were set up, so I didn't mention it. I just tried again and it still works.

If this is not the way tests are expected to work, I am happy to work out the error with you. I based my tests off of the existing tests, so there was some copy-pasting that may be the culprit.

See PR com-lihaoyi#972.

lefou · 2020-12-17T08:49:20Z

@sbly Thanks for making your assumptions clear. All mill tests should run as-is. I created a PR to fix the setup.

See PR com-lihaoyi#972.

Test was introduced in PR #972. * Added contrib.proguard test to test suite * Addressed compiler warnings * Accept specific test failures * Catch more exceptions/errors Pull request: #1080

Stephen Bly added 4 commits September 25, 2020 15:41

WIP: Initial proguard implementation

46971c0

More progress: get proguard actually running

7302ee5

Finish proguard plugin

81a8b16

Reset file

5bf236b

sbly changed the title ~~Proguard~~ Proguard contrib module Oct 5, 2020

lefou reviewed Oct 6, 2020

View reviewed changes

build.sc Outdated Show resolved Hide resolved

contrib/proguard/src/Proguard.scala Outdated Show resolved Hide resolved

contrib/proguard/src/Proguard.scala Outdated Show resolved Hide resolved

lefou requested changes Oct 6, 2020

View reviewed changes

build.sc Outdated Show resolved Hide resolved

build.sc Outdated Show resolved Hide resolved

PR feedback

562c3c4

lefou reviewed Oct 6, 2020

View reviewed changes

Add documentation; use Java home binary

6d0804a

lefou reviewed Oct 8, 2020

View reviewed changes

lefou approved these changes Oct 8, 2020

View reviewed changes

Add ScalaDoc

22acced

lefou reviewed Oct 8, 2020

View reviewed changes

Stephen Bly added 2 commits October 9, 2020 10:24

PR feedback

a901fbd

Add some basic tests

04ceab7

lefou merged commit f555644 into com-lihaoyi:master Nov 2, 2020

lefou added this to the after 0.8.0 milestone Nov 2, 2020

sbly deleted the proguard branch November 2, 2020 20:24

lefou added a commit to lefou/mill that referenced this pull request Dec 17, 2020

Fix porguard test runner which was never working as expected

ae37af2

See PR com-lihaoyi#972.

lefou added a commit to lefou/mill that referenced this pull request Dec 17, 2020

Fix porguard test runner which was never working as expected

f838a92

See PR com-lihaoyi#972.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proguard contrib module #972

Proguard contrib module #972

sbly commented Oct 5, 2020 •

edited

Loading

lefou left a comment

lefou left a comment •

edited

Loading

sbly commented Oct 6, 2020

lefou commented Oct 6, 2020 •

edited

Loading

lefou Oct 6, 2020

sbly commented Oct 7, 2020

lefou left a comment •

edited

Loading

sbly commented Oct 8, 2020

lefou left a comment

lefou Oct 8, 2020

sbly Oct 9, 2020

lefou Oct 9, 2020

sbly commented Nov 2, 2020

lefou commented Nov 2, 2020

lefou commented Nov 27, 2020

sbly commented Dec 17, 2020

lefou commented Dec 17, 2020

Proguard contrib module #972

Proguard contrib module #972

Conversation

sbly commented Oct 5, 2020 • edited Loading

lefou left a comment

Choose a reason for hiding this comment

lefou left a comment • edited Loading

Choose a reason for hiding this comment

sbly commented Oct 6, 2020

lefou commented Oct 6, 2020 • edited Loading

lefou Oct 6, 2020

Choose a reason for hiding this comment

sbly commented Oct 7, 2020

lefou left a comment • edited Loading

Choose a reason for hiding this comment

sbly commented Oct 8, 2020

lefou left a comment

Choose a reason for hiding this comment

lefou Oct 8, 2020

Choose a reason for hiding this comment

sbly Oct 9, 2020

Choose a reason for hiding this comment

lefou Oct 9, 2020

Choose a reason for hiding this comment

sbly commented Nov 2, 2020

lefou commented Nov 2, 2020

lefou commented Nov 27, 2020

sbly commented Dec 17, 2020

lefou commented Dec 17, 2020

sbly commented Oct 5, 2020 •

edited

Loading

lefou left a comment •

edited

Loading

lefou commented Oct 6, 2020 •

edited

Loading

lefou left a comment •

edited

Loading