jbangdev / jbang

Unleash the power of Java - JBang Lets Students, Educators and Professional Developers create, edit and run self-contained source-only Java programs with unprecedented ease.
https://jbang.dev
MIT License
1.43k stars 159 forks source link

Automatic source discovery fails if script contains //DEPS #1502

Open fbricon opened 2 years ago

fbricon commented 2 years ago

Describe the bug Given a Foo.java script and a companion Bar.java at the same level and no explicit //SOURCES Bar.java directive in Foo.java, running jbang Foo.java will yield different results depending on the presence of //DEPS directive or no.

To Reproduce Steps to reproduce the behavior:

  1. Have a Bar.java file containing
    public class Bar{}
  2. Have a Foo.java file in the same directory, containing
    ///usr/bin/env jbang "$0" "$@" ; exit $?
    public class Foo extends Bar {
    public static void main(String... args) {
        System.out.println("hello JBang");
    }
    }
  3. Running jbang Foo.java yields:

    [jbang] Building jar... hello JBang

  4. Now just add a //DEPS directive to Foo.java, like:
    ///usr/bin/env jbang "$0" "$@" ; exit $?
    //DEPS com.github.lalyos:jfiglet:0.0.9
    public class Foo extends Bar {
    public static void main(String... args) {
        System.out.println("hello JBang");
    }
    }
  5. Running jbang Foo.java now yields:

    [jbang] Building jar... /Users/fbricon/Dev/souk/jbangmain/Foo.java:4: error: cannot find symbol public class Foo extends Bar { ^ symbol: class Bar 1 error [jbang] [ERROR] Error during compile [jbang] Run with --verbose for more details

Expected behavior JBang should be able to compile the same file, regardless whether //DEPS directives are present or not.

JBang version

[jbang] jbang version 0.99.1 Cache: /Users/fbricon/.jbang/cache Config: /Users/fbricon/.jbang Repository:/Users/fbricon/.m2/repository 0.99.1

Additional context Adding //SOURCES Bar.java prevents compilation failures when //DEPS is present

quintesse commented 2 years ago

Wow, that's definitely a nasty regression. Thanks for reporting that.

quintesse commented 2 years ago

Hmm actually, this is not a regression, it seems it was always like this.

It's a behavior of the javac compiler I wasn't aware of: javac looks for sources in the classpath!

So when there's only a a Foo.java and a Bar.java the compiler is run like this: javac Foo.java and it will automatically pick up Bar.java.

But the moment you add a dependency the compiler invocation turns into this: javac --classpath /home/user/.m2/repository/some/artifact.jar Foo.java and suddenly the compilation fails because the compiler can't find Bar.java anymore.

If I manually add . to the classpath things start working like before again.

What's the behavior that we want here @maxandersen ? Should we allow auto-pickup of local sources or should we require explicit //SOURCES lines? (I'm assuming the former but I just want to confirm with you)

fbricon commented 2 years ago

Autopickup sources is the path to least surprises. Without it, IDEs would need to report errors as soon as //DEPS is added, which puts a lot of burden on the tooling

maxandersen commented 2 years ago

nice analysis!

Basically what you are saying is that it works like javac was designed ...that default classpath is current directory...

But adding . on every classpath feels wrong too.

maxandersen commented 2 years ago

jbang --cp . Foo.java works

What is probably more right is to explore using javac --source-path consistently.

i.e. add a --source-path for every identified "source root" path.

WDYT?

quintesse commented 2 years ago

Btw, this problem is actually not related to //DEPS at all. It's just that coincidentally if your code is in the default package the default class path of . just happens to be the same as the source folder. The moment the code uses a package it will always fail. This has actually never worked!

quintesse commented 1 year ago

So there's two additional issues here @maxandersen :

Seems to me then that we'd have to treat local source files differently than remote ones: we'd do the package detection and adding the source folders for local files but not for remote files. Is that how you see it as well?