FabricMC / fabric-language-kotlin

Fabric language module for Kotlin.
Apache License 2.0
293 stars 33 forks source link

Plans for Kotlin 1.4 (or earlier?) #14

Closed natanfudge closed 3 years ago

natanfudge commented 4 years ago
ChloeDawn commented 4 years ago

I do not agree that coroutines should be dropped from the group. I do think your 3rd point should be put into effect, however, with each sublibrary of Kotlin broken up into modules and the option to use a jar that includes all; much like Fabric API's design.

NikkyAI commented 4 years ago

there is nothing preventing modauthors from using JiJ for the bits that they are missing, there is no good reason to repackage a single library into a different namespace we would most likely not add any functionality to any of the libraries anyways

natanfudge commented 4 years ago

There is a reason, when you don't want to JiJ Kotlin. For example if you make a library with a Kotlin API, you want to give whoever consumes the library the choice to JIJ the library or to depend on it and instruct to add it separately.

natanfudge commented 4 years ago

And yes, you can depend on the entirety of Fabric-Language-Kotlin, but that is super overkill for just providing a Kotlin API,

NikkyAI commented 4 years ago

but most kotlin libs apart from the stdlib and the embedded-compiler are not that big, JiJ is not as much of a issue

just don't JiJ the stdlib in mods please

natanfudge commented 4 years ago

Kotlin-reflect is twice as big as stdlib

Fuyukai commented 4 years ago

I would simply just include everything and not care about the size because removing stuff will just lead to people JiJing it anyway, meaning duplicating it over several mods

natanfudge commented 4 years ago

We're not removing anything, we're providing the option to depend on less.

natanfudge commented 4 years ago

We let mod authors say, "you know what, my mod will run even if you don't include this"

Fuyukai commented 4 years ago

Remove coroutines

natanfudge commented 4 years ago

Oh. That's a different case. We have agreed that coroutines tend to be so unuseful in Minecraft that there's no reason to keep it in. Although I can see keeping it if you find it useful. But not without the option of not depending on it.

Fuyukai commented 4 years ago

I don't understand the merits of trying to reduce file size in an ecosystem where the widespread solution to dependencies is copying them into every mod that needs them

natanfudge commented 4 years ago

You don't understand the perfectly reasonable sentence of "trying to reduce file size in an ecosystem where the widespread solution to dependencies is copying them into every mod that needs them"?

If every mod copied library A that weighs 10mb, and there are 100 mods, that would be 1gb. If the library size is reduced to 2mb, that would be 0.2gb. So we go from 1gb to 0.2gb. An 800mb difference.

By comparison, if the widespread solution was to make the users include the mods they need once, then if library A weighs 10mb, and there are 100 mods, that would be 10mb. If the library size is reduced to 2mb, that would be 2mb. So we go from 10mb to 2mb. An 8m difference.

I think It's pretty self-evident that if JIJ was widespread then reducing library size would be a lot more important.

Fuyukai commented 4 years ago

No, I don't understand why you're proposing splitting this into two so that you have two copies of kotlin-stdlib running around and then one with -reflect added on instead of having a proper solution so that libraries don't need to be copied into every jar.

natanfudge commented 4 years ago

instead of having a proper solution so that libraries don't need to be copied into every jar.

Yes that would be great. Until then, we will do out best to minimize damages.

natanfudge commented 4 years ago

You're saying "instead of having a proper solution" as if I'm hiding one in my back pocket.

Proximyst commented 4 years ago

@natanfudge Would just like to add that you're fully able to do:

compile("thing:here") {
    exclude(group = "kotlin")
}

to exclude kotlin from shading.

natanfudge commented 4 years ago

@Proximyst Who does this? A mod that uses kotlin? A mod that depends on a mod that uses Kotlin?

Proximyst commented 4 years ago

@natanfudge Anyone unhappy with how things are packaged (read: you).

natanfudge commented 4 years ago

While it is very nice to drop "knowledge bombs" and disappear for 3 days, try to follow through what happens. I exclude Kotlin, now Kotlin is no longer pulled. My mod no longer works because it needs Kotlin. What next?

Proximyst commented 4 years ago

While it is nice to drop "knowledge bombs" and not check a Git website I merely push to for 3 and 11 days, I'll try following through from now :)

Then you pull in your own Kotlin version.

modmuss50 commented 4 years ago

Backwards compatibility must be kept imo, so removing modules isn’t an option.

natanfudge commented 4 years ago

@Proximyst then, since Kotlin is not a mod, you would have 10 different kotlin versions on the classpath which would most definitely not work.

Proximyst commented 4 years ago

@natanfudge I'm sure you'd be able to relocate the Kotlin packages. Only one I'd be hesitant to say you'd be able to relocate is reflect.

natanfudge commented 4 years ago

“Relocate”? Rehost?

Proximyst commented 4 years ago

@natanfudge Quote from shadow's page about relocation:

scanning a project's classes and relocating specific dependencies to a new location. This is often required when one of the dependencies is susceptible to breaking changes in versions or to classpath pollution in a downstream project.

natanfudge commented 4 years ago

This doesn’t take into account binaries that don’t exist in the development environment...

Proximyst commented 4 years ago

@natanfudge Sorry, I'm not aware of that means; mind elaborating?

natanfudge commented 4 years ago

I make mod A bundling Kotlin stdlib using relocation scanning or whatever and publish it. Someone else makes mod B bundling Fabric Language Kotlin and publishes it. None of us has any problems while developing. Then, both of our mods are put into the mod folder and loaded together. Both kotlin versions are loaded at the same time, and problems ensue.

Proximyst commented 4 years ago

@natanfudge that's what relocating fixes. If you want kotlin.* to be moved to com.github.natanfudge.${project.name}.dependencies.kotlin.*, it does that; the classpath differences are just fine.

NikkyAI commented 4 years ago
  1. we can remove coroutines now for all i care, might break mods that use it through, but i guess the warning has been given for long enough @modmuss50 i guess having a lite module without the deps that are not useful would be the right way.. but see 3.

~~2. adding serialization-runtime is possible in fabric-kotlin however we cannot provide serializaers for minecraft classes, do we want to do that? maybe this should be added to api~~ https://github.com/natanfudge/Fabric-Drawer already handles this

  1. not sure how practical 2 flavors of kotlin-fabric is.. even just from buildscript/dev point of view and also just to have no reflect? i am not sure whatever gains is worth the effort.. also do we then have 2 curseforge projects or both artifacts in the same ? either way seems like a pain for at least someone
natanfudge commented 4 years ago

in fabric-kotlin however we cannot provide serializaers for minecraft classes, do we want to do that?

This is implemented in drawer https://github.com/natanfudge/Fabric-Drawer/blob/1.15/src/main/kotlin/drawer/Serializers.kt

NikkyAI commented 4 years ago

then drawer also bundles serialization already.. do we need to bundle it in here then?

natanfudge commented 4 years ago

No, not really. ( You mean serialization not coroutines)

natanfudge commented 4 years ago

I don't think this issue makes that much sense anymore.
There's basically two situations, you bundle FLK or don't bundle FLK. In the former, you probably shouldn't, but if you really wanted to you can do the shadow thing that @Proximyst showed. In the latter, the size doesn't matter.

modmuss50 commented 3 years ago

I believe most of this has been resloved. please open a new issue if I missed something.