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

Patch DSL: support patching multiple modules with one statement #127

Closed
jjohannes opened this issue Apr 24, 2024 · 5 comments
Closed

Patch DSL: support patching multiple modules with one statement #127

jjohannes opened this issue Apr 24, 2024 · 5 comments
Assignees
Labels
a:enhancement New feature or request in:patch Things related to 'patch { }' DSL

Comments

@jjohannes
Copy link
Member

Should be able to write:

jvmDependencyConflicts {
    patch {
        module("org.openjfx:javafx-base", "org.openjfx:javafx-graphics") {
            addTargetPlatformVariant("", "none", "none")
            addTargetPlatformVariant("linux", OperatingSystemFamily.LINUX, MachineArchitecture.X86_64)
            addTargetPlatformVariant("mac", OperatingSystemFamily.MACOS, MachineArchitecture.X86_64)
            addTargetPlatformVariant("mac-aarch64", OperatingSystemFamily.MACOS, MachineArchitecture.ARM64)
            addTargetPlatformVariant("win", OperatingSystemFamily.WINDOWS, MachineArchitecture.X86_64)
        }
    }
}
@jjohannes jjohannes added a:enhancement New feature or request in:patch Things related to 'patch { }' DSL labels Apr 24, 2024
@jjohannes jjohannes added this to the 2.1 milestone Apr 24, 2024
@jjohannes jjohannes self-assigned this Apr 24, 2024
@britter
Copy link
Member

britter commented Apr 24, 2024

If patching for OS specific artifacts is a common use case, maybe we can come up with an even more compat way of declaring just that in addition.

@jjohannes
Copy link
Member Author

addTargetPlatformVariant is already that: TargetPlatformVariant stands for OS+ARCH

Not sure what can be more specific, but happy to discuss. Although that would be a separate issue. This one is about the module(...) part which is for any kind of patching rules. The addTargetPlatformVariant is only used as example here.

@jjohannes
Copy link
Member Author

jjohannes commented May 24, 2024

Just realized that this signature is (of course) not possible in Java:

public void module(String... modules, Action<PatchModule> action)

Lets think about this a bit more.

You can already do something like this today with standard Kotlin/Groovy notations:

jvmDependencyConflicts {
    patch {
        listOf("org.openjfx:javafx-base", "org.openjfx:javafx-graphics").forEach {
            module(it) {
                addTargetPlatformVariant("", "none", "none")
                addTargetPlatformVariant("mac", "macos", "x86-64")
                addTargetPlatformVariant("mac-aarch64", "macos", "aarch64")
                addTargetPlatformVariant("win", "windows", "x86-64")
                addTargetPlatformVariant("linux-aarch64", "linux", "x86-64")
            }
        }
    }
}

@jjohannes jjohannes removed this from the 2.1 milestone May 24, 2024
@britter
Copy link
Member

britter commented May 24, 2024

If you want to have varargs the only way I see to get this working would be a method chain like the following:

jvmDependencyConflicts {
  patch {
    modules("org.openjfx:javafx-base", "org.openjfx:javafx-graphics").each {
       ...
    }
  }
}

The modules method would have the following signature:

public EachModuleConfigurer modules(String... modules);

interface EachModuleConfigurer { 
  public void each(Action<PatchModule> action)
}

But this would just wrap around a forEach call and would spare you from writing listOf. Not sure if this is really better. The problem with these kind of chaining APIs is always when you forget to make the terminal call nothing happens. And that is sometimes hard to find.

@jjohannes
Copy link
Member Author

Lets not do this now. I would prefer not to overload the DSL unless there is a really clean DSL concept (and not a technical API).

@jjohannes jjohannes closed this as not planned Won't fix, can't repro, duplicate, stale May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:enhancement New feature or request in:patch Things related to 'patch { }' DSL
Projects
None yet
Development

No branches or pull requests

2 participants