Skip to content

stage2/ml9: Fix merging of provided services into KFF #31

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.

Sign up for GitHub

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

Merged
Johni0702 merged 1 commit into from
Jan 31, 2026
Merged

stage2/ml9: Fix merging of provided services into KFF #31

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion ...modlauncher9/src/main/java/gg/essential/loader/stage2/DescriptorRewritingJarMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,17 @@
import gg.essential.loader.stage2.util.Lazy;

import java.lang.module.ModuleDescriptor;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

/**
* Re-creates the {@link #descriptor()} with an updated {@link SecureJar#getPackages() package list}.
* This is needed when one adds classes (even internal ones) to a {@link SecureJar} in previously non-existent packages
* because the module system needs to know about all packages because ModLauncher will use that package list to build
* a lookup table.
* Also updates the provided services with the ones from the new meta.
*/
public class DescriptorRewritingJarMetadata implements JarMetadata {
private final JarMetadata delegate;
Expand Down Expand Up @@ -47,7 +52,13 @@ public ModuleDescriptor descriptor() {
}
org.uses().forEach(builder::uses);
}
org.provides().forEach(builder::provides);
Map<String, List<String>> orgProvides = org.provides()
.stream().collect(Collectors.toMap(ModuleDescriptor.Provides::service, ModuleDescriptor.Provides::providers));
Map<String, List<String>> newProvides = newPkgsMeta.descriptor().provides()
.stream().collect(Collectors.toMap(ModuleDescriptor.Provides::service, ModuleDescriptor.Provides::providers));
Map<String, List<String>> mergedProvides = new HashMap<>(orgProvides);
mergedProvides.putAll(newProvides);
Copy link

@Traben-0 Traben-0 Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: I assume we merge to clear duplicate keys, is there any scenario in which we would want to keep the entries in the "value" (List) of the replaced entry?
i.e. would putAll ever need to merge the preexisting list with the new putted list, for duplicate keys

Copy link
Contributor Author

@Johni0702 Johni0702 Jan 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the opposite is true actually, it shouldn't do that because if e.g. the Kotlin which shipped with KFF had a service A with provider A1, which in a later Kotlin version was replaced with provider A2, then it'd be wrong for our list to contain A1 and A2 since A1 won't work any more.

(One might think that there might be a case where KFF itself provides an impl for a Kotlin service, then if Kotlin itself also provides an impl, we'd be hiding KFF's impl. But that's not the case, and except for really old KFF versions, which don't seem to receive any updates any more, KFF's own code is jar-in-jar'd, so would be in a separate module anyway).

mergedProvides.forEach(builder::provides);
org.mainClass().ifPresent(builder::mainClass);
this.descriptor = builder.build();
}
Expand Down