FabricMC / fabric-loom

Gradle build system plugin used to automate the setup of a minecraft mod development environment.
MIT License
241 stars 211 forks source link

Pass File-based runDir in run configuration fails on Windows #575

Open Adirelle opened 2 years ago

Adirelle commented 2 years ago

The following piece of configuration causes an error on windows:

loom {
        create("myRun") {
            server()
            runDir("${project.buildDir}/datagen")
        }
    }
}

Error message:

Cannot run program "C:\Program Files\Eclipse Adoptium\jdk-17.0.1.12-hotspot\bin\java.exe" (in directory "C:\Users\user\Projets\minecraft\mymod\C:\Users\user\Projets\minecraft\mymod\build\datagen")

As you can see, the project directory is included twice in the run path.

As an alternative, I have tested runDir(project.buildDir.resolve("datagen").toString()) to no avail.

I think the runDir directive should accept Files in addition to Strings.

For reference : https://github.com/badasintended/slotlink/pull/125#discussion_r786229731

Juuxel commented 2 years ago

I think you can work around this for now with just setting it to "build/datagen" instead of using the file.

AbstractRunTask does this: https://github.com/FabricMC/fabric-loom/blob/e9d1f005d927ad23bb1acbe8b78903d4a41ff1cc/src/main/java/net/fabricmc/loom/task/AbstractRunTask.java#L53-L58

(maybe getProject().file would be better here in any case?)

Adirelle commented 2 years ago

Yes, this is the workaround I use for the time being.

However, maybe the line in AbstractRunTask should be moved in the runSettings so one can pass a String that would be resolved in place into a File using the project root directory. And a runDir overload could accept a File that would be left untouched. This would be backwards compatible while allowing "advanced" configuration.

IMO this is a case of Primitive Obsession anti-pattern.

PS: I would submit a PR myself but I have issues compiling loom-fabric. Also, I am available on the discord server if you want to chat about it.

Juuxel commented 2 years ago

I think this can be done in a backwards compatible way pretty easily: an Object run directory instead of a String and then use rootProject.file(path) instead of new File(rootDir, path) (Project.getRootDir() is the project dir of the root project, I'm not sure why the run directory is evaluated against that)