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

Detection of main method behaves differently for mill #56

Closed
ckipp01 opened this issue Jul 26, 2020 · 8 comments
Closed

Detection of main method behaves differently for mill #56

ckipp01 opened this issue Jul 26, 2020 · 8 comments

Comments

@ckipp01
Copy link
Collaborator

ckipp01 commented Jul 26, 2020

I noticed that when using mill installed via cs install mill behaves a bit differently than when using the mill wrapper that is normally in mill projects. For example:

  1. Download the example project for cask
  2. Using the mill wrapper to run ./mill app.run seems to work as expected.

Now instead of using the wrapper if you try to use mill installed via cs you'll see an error.

  1. Instead of using the wrapper script, use mill installed via cs.
~/Downloads/minimalApplication-0.7.3
❯ mill app.run
  1. You should see the following:
~/Downloads/minimalApplication-0.7.3
❯ mill app.run
Compiling /Users/ckipp/Downloads/minimalApplication-0.7.3/build.sc
[26/36] app.compile
[info] Compiling 1 Scala source to /Users/ckipp/Downloads/minimalApplication-0.7.3/out/app/compile/dest/classes ...
[info] Done compiling.
[28/36] app.finalMainClass
1 targets failed
app.finalMainClass No main class specified or found

Should there be a difference in behavior for finding the main class for the two of these?

@alexarchambault
Copy link
Member

@ckipp01 Could it be a mill version mismatch? I'm getting the following:

$ cs launch mill:0.7.3 -- app.run
Compiling /Users/alexandre/projects/test/minimalApplication-0.7.3/build.sc
[26/36] app.compile
Compiling compiler interface...
9 warnings
[info] Compiling 1 Scala source to /Users/alexandre/projects/test/minimalApplication-0.7.3/out/app/compile/dest/classes ...
[info] Done compiling.
[36/36] app.run
juil. 27, 2020 3:52:07 PM org.xnio.Xnio <clinit>
INFO: XNIO version 3.3.8.Final
juil. 27, 2020 3:52:07 PM org.xnio.nio.NioXnio <clinit>
INFO: XNIO NIO Implementation Version 3.3.8.Final
^C

$ cs launch mill:0.8.0 -- app.run
[26/36] app.compile
[info] Compiling 1 Scala source to /Users/alexandre/projects/test/minimalApplication-0.7.3/out/app/compile/dest/classes ...
[info] Done compiling.
[28/36] app.finalMainClass
1 targets failed
app.finalMainClass No main class specified or found

Right now, the mill binary installed by cs isn't really a universal mill launcher… It only launches its own mill version, which may not be the one of the project at hand.

@ckipp01
Copy link
Collaborator Author

ckipp01 commented Jul 27, 2020

Yes so that's exactly what I see as well @alexarchambault. I guess the thing that confuses me is that in the minimalApplication-0.7.3 the DEFAULT_MILL_VERSION is set to 0.7.3 and again, this works. If I manually change that value for it to default to 0.8.0 and then run ./mill app.run it still works. But then when again running from mill app.run it doesn't find the main class.

Just to be super clear:

  1. Freshly unzip the project and run with coursier installed mill
❯ cs launch mill:0.8.0 -- app.run
Compiling /Users/ckipp/Downloads/minimalApplication-0.7.3/build.sc
[26/36] app.compile
[info] Compiling 1 Scala source to /Users/ckipp/Downloads/minimalApplication-0.7.3/out/app/compile/dest/classes ...
[info] Done compiling.
[28/36] app.finalMainClass
1 targets failed
app.finalMainClass No main class specified or found
  1. Try with mill wrapper, see version change and it works
❯ ./mill app.run
Mill version changed (0.8.0 -> 0.7.3), re-starting server
[36/36] app.run
Jul 27, 2020 4:05:44 PM org.xnio.Xnio <clinit>
INFO: XNIO version 3.3.8.Final
Jul 27, 2020 4:05:44 PM org.xnio.nio.NioXnio <clinit>
INFO: XNIO NIO Implementation Version 3.3.8.Final
  1. So then manually change DEFAULT_MILL_VERSION to 0.8.0 in wrapper and try again
❯ ./mill --version
Mill Build Tool version 0.8.0
Java version: 1.8.0_242, vendor: BellSoft, runtime: /Users/ckipp/.sdkman/candidates/java/8.0.242-librca/jre
Default locale: en_NL, platform encoding: UTF-8
OS name: "Mac OS X", version: 10.15.5, arch: x86_64
...
❯ ./mill app.run
Compiling /Users/ckipp/Downloads/minimalApplication-0.7.3/build.sc
[36/36] app.run
Jul 27, 2020 4:08:37 PM org.xnio.Xnio <clinit>
INFO: XNIO version 3.3.8.Final
Jul 27, 2020 4:08:37 PM org.xnio.nio.NioXnio <clinit>
INFO: XNIO NIO Implementation Version 3.3.8.Final
  1. Then try again with mill installed via coursier
❯ cs launch mill:0.8.0 -- app.run
Compiling /Users/ckipp/Downloads/minimalApplication-0.7.3/build.sc
[26/36] app.compile
[info] Compiling 1 Scala source to /Users/ckipp/Downloads/minimalApplication-0.7.3/out/app/compile/dest/classes ...
[info] Done compiling.
[28/36] app.finalMainClass
1 targets failed
app.finalMainClass No main class specified or found

So I just don't get why using ./mill app.run here and cs launch mill:0.8.0 -- app.run don't act the same when they are using 0.8.0. If you don't think this has anything to do with Coursier, by all means close it, but when I pointed this out to Li Haoyi he pointed me here.

@alexarchambault
Copy link
Member

Mmmhh… Something may have changed in the way mill starts, or maybe we don't fetch the exact same class path as the one it generates its assembly with…

Bisected that to those commits:

$ cs launch mill:0.7.4-5-438ebc -- app.run
[26/36] app.compile
Compiling compiler interface...
9 warnings
[info] Compiling 1 Scala source to /Users/alexandre/projects/test/minimalApplication-0.7.3/out/app/compile/dest/classes ...
[info] Done compiling.
[36/36] app.run
juil. 27, 2020 5:18:06 PM org.xnio.Xnio <clinit>
INFO: XNIO version 3.3.8.Final
juil. 27, 2020 5:18:06 PM org.xnio.nio.NioXnio <clinit>
INFO: XNIO NIO Implementation Version 3.3.8.Final
^C

$ cs launch mill:0.7.4-7-6b492a -- app.run
[26/36] app.compile
[info] Compiling 1 Scala source to /Users/alexandre/projects/test/minimalApplication-0.7.3/out/app/compile/dest/classes ...
[info] Done compiling.
[28/36] app.finalMainClass
1 targets failed
app.finalMainClass No main class specified or found

@alexarchambault
Copy link
Member

alexarchambault commented Jul 27, 2020

Possibly a class path issue then… Those forced versions do not end up in the POM, so we don't respect them when fetching mill from coursier.

@alexarchambault
Copy link
Member

Yeah, confirmed:

$ cs launch mill:0.7.4-7-6b492a -V org.scalameta:trees_2.13:4.3.7 -- app.run
Compiling /Users/alexandre/projects/test/minimalApplication-0.7.3/build.sc
[26/36] app.compile
Compiling compiler interface...
9 warnings
[info] Compiling 1 Scala source to /Users/alexandre/projects/test/minimalApplication-0.7.3/out/app/compile/dest/classes ...
[info] Done compiling.
[36/36] app.run
juil. 27, 2020 5:23:20 PM org.xnio.Xnio <clinit>
INFO: XNIO version 3.3.8.Final
juil. 27, 2020 5:23:20 PM org.xnio.nio.NioXnio <clinit>
INFO: XNIO NIO Implementation Version 3.3.8.Final
^C

@alexarchambault
Copy link
Member

alexarchambault commented Jul 27, 2020

Basically, to fix that, mill itself should exclude org.scalameta::trees when it depends on com.lihaoyi:::ammonite-interp:

$ cs resolve mill:0.7.4-7-6b492a --what-depends-on org.scalameta:trees_2.13
  Result:
└─ org.scalameta:trees_2.13:4.3.20
   ├─ com.lihaoyi:ammonite-interp_2.13.2:2.2.0
   │  ├─ com.lihaoyi:ammonite-repl_2.13.2:2.2.0
   │  │  └─ com.lihaoyi:ammonite_2.13.2:2.2.0
   │  │     └─ com.lihaoyi:mill-main-core_2.13:0.7.4-7-6b492a
   │  │        └─ com.lihaoyi:mill-main_2.13:0.7.4-7-6b492a
   │  │           └─ com.lihaoyi:mill-scalalib_2.13:0.7.4-7-6b492a
   │  │              ├─ com.lihaoyi:mill-dev_2.13:0.7.4-7-6b492a
   │  │              ├─ com.lihaoyi:mill-scalajslib_2.13:0.7.4-7-6b492a
   │  │              │  └─ com.lihaoyi:mill-dev_2.13:0.7.4-7-6b492a
   │  │              └─ com.lihaoyi:mill-scalanativelib_2.13:0.7.4-7-6b492a
   │  │                 └─ com.lihaoyi:mill-dev_2.13:0.7.4-7-6b492a
   │  └─ com.lihaoyi:ammonite_2.13.2:2.2.0
   │     └─ com.lihaoyi:mill-main-core_2.13:0.7.4-7-6b492a
   │        └─ com.lihaoyi:mill-main_2.13:0.7.4-7-6b492a
   │           └─ com.lihaoyi:mill-scalalib_2.13:0.7.4-7-6b492a
   │              ├─ com.lihaoyi:mill-dev_2.13:0.7.4-7-6b492a
   │              ├─ com.lihaoyi:mill-scalajslib_2.13:0.7.4-7-6b492a
   │              │  └─ com.lihaoyi:mill-dev_2.13:0.7.4-7-6b492a
   │              └─ com.lihaoyi:mill-scalanativelib_2.13:0.7.4-7-6b492a
   │                 └─ com.lihaoyi:mill-dev_2.13:0.7.4-7-6b492a
   └─ com.lihaoyi:mill-main-core_2.13:0.7.4-7-6b492a org.scalameta:trees_2.13:4.3.7 -> 4.3.20
      └─ com.lihaoyi:mill-main_2.13:0.7.4-7-6b492a
         └─ com.lihaoyi:mill-scalalib_2.13:0.7.4-7-6b492a
            ├─ com.lihaoyi:mill-dev_2.13:0.7.4-7-6b492a
            ├─ com.lihaoyi:mill-scalajslib_2.13:0.7.4-7-6b492a
            │  └─ com.lihaoyi:mill-dev_2.13:0.7.4-7-6b492a
            └─ com.lihaoyi:mill-scalanativelib_2.13:0.7.4-7-6b492a
               └─ com.lihaoyi:mill-dev_2.13:0.7.4-7-6b492a

That way, org.scalameta::trees would stay on 4.3.7 and wouldn't be bumped to 4.3.20, and the forceVersion() wouldn't be necessary.

@ckipp01
Copy link
Collaborator Author

ckipp01 commented Jul 27, 2020

Thanks a ton for you work on tracking this down and for explaining it. This makes sense and seems doable. I've just been playing around a bit with Mill so this will be good for me to try and figure out and fix. If it's alright I'll leave this open a bit just to reference until I get it fixed. Thanks again @alexarchambault 🙏 .

ckipp01 added a commit to ckipp01/mill that referenced this issue Jul 28, 2020
The reason for doing this is to ensure that the version of
trees that mill wants is indeeed used instead of the newer
version that is being pulled in by ammonite-interp. Before
this was using `forceVersion()`, but by doing this the forced
version doesn't end up in the POM meaning that when someone
bootstraps/uses mill via coursier, they are getting the newer
trees, which is causing issues with how the main class is detected.

You can see more details on this coursier/apps#56
and a huge pros to @alexarchambault for finding this.
ckipp01 added a commit to ckipp01/mill that referenced this issue Jul 28, 2020
The reason for doing this is to ensure that the version of
trees that mill wants is indeed used instead of the newer
version that is being pulled in by ammonite-interp. Before
this was using `forceVersion()`, but by doing this the forced
version doesn't end up in the POM meaning that when someone
bootstraps/uses mill via coursier, they are getting the newer
trees, which is causing issues with how the main class is detected.

You can see more details on this coursier/apps#56
and a huge props to @alexarchambault for finding this.
ckipp01 added a commit to ckipp01/mill that referenced this issue Nov 3, 2020
The reason for doing this is to ensure that the version of
trees that mill wants is indeed used instead of the newer
version that is being pulled in by ammonite-interp. Before
this was using `forceVersion()`, but by doing this the forced
version doesn't end up in the POM meaning that when someone
bootstraps/uses mill via coursier, they are getting the newer
trees, which is causing issues with how the main class is detected.

You can see more details on this coursier/apps#56
and a huge props to @alexarchambault for finding this.
lefou pushed a commit to com-lihaoyi/mill that referenced this issue Nov 3, 2020
The reason for doing this is to ensure that the version of
trees that mill wants is indeed used instead of the newer
version that is being pulled in by ammonite-interp. Before
this was using `forceVersion()`, but by doing this the forced
version doesn't end up in the POM meaning that when someone
bootstraps/uses mill via coursier, they are getting the newer
trees, which is causing issues with how the main class is detected.

You can see more details on this coursier/apps#56
and a huge props to @alexarchambault for finding this.

Pull request: #941
@ckipp01
Copy link
Collaborator Author

ckipp01 commented Nov 3, 2020

This is taken care of now in com-lihaoyi/mill#941

@ckipp01 ckipp01 closed this as completed Nov 3, 2020
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