forax / pro

A Java build tool that works seamlessly with modules
GNU General Public License v3.0
103 stars 15 forks source link

Add static helper class `Tools` to module `com.github.forax.pro` #44

Closed sormuras closed 6 years ago

sormuras commented 7 years ago

~Locally with jdk-9-b169, I only get one failure:~

  JUnit Jupiter:ToolsTests:copyAndDeleteTree()
    MethodSource [className = 'integration.pro.ToolsTests', methodName = 'copyAndDeleteTree', methodParameterTypes = '']
    => java.lang.IllegalAccessError: class integration.pro.ToolsTests (in module integration.pro) cannot access class com.github.forax.pro.helper.FileHelper$IOBiConsumer (in module com.github.forax.pro.helper) because module integration.pro does not read module
 com.github.forax.pro.helper

Strict module boundaries are strict...

sormuras commented 7 years ago

But why can't the build process / Bootstrap(?) find the Tools class at Travis. Locally build. and pro build test.pro works.

forax commented 7 years ago

It seems that the package com.github.forax.pro taken by the tester is the one of the bootstrap pro and not the one of the pro you are testing. And because Tools does not exist in bootstrap pro, it fails.

Locally, you are using the pro you are testing because you boostrap with the jdk and not pro itself

sormuras commented 7 years ago

"tester"?

It's already the compiler that doesn't find the Tools class. Although it finds the Pro one. Strange.

sormuras commented 7 years ago

Tools.java is compiled by Bootstrap. See right above line 168 https://travis-ci.org/forax/pro/builds/232088674#L168

sormuras commented 7 years ago

Ah! The "installed" pro version ( installed from https://github.com/forax/pro/releases ) does not contain Tools.java. But that smells like an error (or cycle) in the general build/bootstrap setup. How do I solve it?

sormuras commented 7 years ago

Tools.class is next to Pro.class: https://travis-ci.org/sormuras/pro/builds/232105230#L248 And the test compiler paths look good:

[compiler] javac --release 9
      -Xlint:all,-varargs,-overloads
      -d ./target/test/exploded
      --module-source-path ./target/test/merged
      --module-path ./deps:./target/main/exploded

I'll try to move the ToolTests.java away from "integration.pro" and into "com.github.forax.pro"

sormuras commented 7 years ago

Now it compiles and tests -- but needs a review regarding the module paths and dependencies.

Especially the "StableList list(...)" methods in Pro "leak" an internal type, if I read the warnings correctly: https://travis-ci.org/sormuras/pro/builds/232114170#L170 and following

sormuras commented 7 years ago

Addressed all comments. Back to green, again. Except for the "[export]" warning regarding StableList in Pro...

sormuras commented 7 years ago

Small refactoring in progress... removing the "if on windows" checks in the download test.

sormuras commented 7 years ago

Except for the "[export]" warning regarding StableList in Pro...

With https://github.com/forax/pro/pull/44/commits/6b442239fd86386d07d58a5310877b27227f579f the warnings are history.

sormuras commented 6 years ago

No need for the proposed tool for over a year. Thus closing this PR.