Skip to content

Commit f4f1cfb

Browse files
committed
Use dependency substitution rules instead of constraints
Dependency substitutions are only considered and applied for incoming configurations. Consumable configurations are not affected and thus the attributes used to enforce AccessTransformers will not pollute the Gradle Module Metadata. This is important because it is theoretically possible for stray AccessTransformer attributes to either: 1. Cause a dependency lock-up for dependencies that have weird variants or complex resolutions. 2. Cause an incoming dependency with those attributes to have unwanted AccessTransformers applied to them.
1 parent 2d4b62f commit f4f1cfb

File tree

8 files changed

+113
-98
lines changed

8 files changed

+113
-98
lines changed

at-gradle-demo/build.gradle

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
plugins {
22
id 'java'
3+
id 'maven-publish'
34
id 'net.minecraftforge.accesstransformers'
45
}
56

7+
group = 'net.minecraftforge'
8+
69
java.toolchain.languageVersion = JavaLanguageVersion.of(17)
710

811
dependencies {
@@ -11,5 +14,12 @@ dependencies {
1114
config = project.file('accesstransformer.cfg')
1215
}
1316
}
17+
1418
compileOnly libs.log4j.api
1519
}
20+
21+
publishing {
22+
publications.register('mavenJava', MavenPublication) {
23+
from components.java
24+
}
25+
}

at-gradle-demo/settings.gradle

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ pluginManagement {
55
mavenCentral()
66
gradlePluginPortal()
77
maven { url = 'https://maven.minecraftforge.net' }
8-
//mavenLocal()
8+
mavenLocal()
99
}
1010
}
1111

1212
plugins {
1313
id 'org.gradle.toolchains.foojay-resolver-convention' version '1.0.0'
14-
id 'net.minecraftforge.gradleutils' version '3.3.12'
14+
id 'net.minecraftforge.gradleutils' version '3.3.13'
1515
}
1616

1717
rootProject.name = 'at-gradle-demo'

at-gradle/settings.gradle

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ dependencyResolutionManagement {
1313
repositories {
1414
mavenCentral()
1515
maven gradleutils.forgeMaven
16+
mavenLocal()
1617
maven { url = 'https://maven.moddinglegacy.com/maven' } // Gradle API
17-
//mavenLocal()
1818
}
1919

2020
//@formatter:off
@@ -35,7 +35,7 @@ dependencyResolutionManagement {
3535
library 'gradle', 'name.remal.gradle-api', 'gradle-api' versionRef 'gradle'
3636

3737
// GradleUtils Shared Base
38-
library 'gradleutils-shared', 'net.minecraftforge', 'gradleutils-shared' version '3.3.12'
38+
library 'gradleutils-shared', 'net.minecraftforge', 'gradleutils-shared' version '3.3.13'
3939

4040
// Utils
4141
library 'utils-hash', 'net.minecraftforge', 'hash-utils' version '0.1.9'

at-gradle/src/main/java/net/minecraftforge/accesstransformers/gradle/AccessTransformersContainer.java

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,6 @@
55
package net.minecraftforge.accesstransformers.gradle;
66

77
import org.gradle.api.Action;
8-
import org.gradle.api.Project;
9-
import org.gradle.api.artifacts.Dependency;
10-
import org.gradle.api.attributes.HasConfigurableAttributes;
118
import org.gradle.api.file.ConfigurableFileCollection;
129
import org.gradle.api.file.FileCollection;
1310
import org.gradle.api.logging.LogLevel;
@@ -19,21 +16,11 @@
1916
/// Represents a container of dependencies that will be access transformed.
2017
///
2118
/// Containers are created with [AccessTransformersExtension#register(Action)]. Dependencies can be registered to it
22-
/// inside their configuring closures using [#configure(Dependency)].
19+
/// inside their configuring closures using [#configure].
2320
///
2421
/// @apiNote This interface is effectively sealed and [must not be extended][ApiStatus.NonExtendable].
2522
/// @see AccessTransformersExtension
2623
public sealed interface AccessTransformersContainer permits AccessTransformersContainerInternal, AccessTransformersExtension {
27-
/// Registers a new container using the given attribute and options.
28-
///
29-
/// @param project The project to make the container for]
30-
/// @param options The options to apply
31-
/// @return The registered container
32-
/// @see AccessTransformersContainer.Options
33-
static AccessTransformersContainer register(Project project, Action<? super AccessTransformersContainer.Options> options) {
34-
return AccessTransformersContainerInternal.register(project, options);
35-
}
36-
3724
/// Gets the access transformer options.
3825
///
3926
/// @return The options
@@ -51,15 +38,15 @@ default void options(Action<? super AccessTransformersContainer.Options> action)
5138
/// Configures the given dependency to use this AccessTransformers container.
5239
///
5340
/// @param dependency The dependency to configure AccessTransformers for
54-
default void configure(Dependency dependency) {
41+
default void configure(Object dependency) {
5542
this.configure(dependency, it -> { });
5643
}
5744

5845
/// Configures the given dependency to use this AccessTransformers container.
5946
///
6047
/// @param dependency The dependency to configure AccessTransformers for
6148
/// @param action A configuring action to modify dependency-level AccessTransformer options
62-
void configure(Dependency dependency, Action<? super AccessTransformersConfiguration> action);
49+
void configure(Object dependency, Action<? super AccessTransformersConfiguration> action);
6350

6451
/// When initially registering an AccessTransformers container, the consumer should define key information regarding
6552
/// how AccessTransformers will be used. This interface is used to define that information.

at-gradle/src/main/java/net/minecraftforge/accesstransformers/gradle/AccessTransformersContainerImpl.java

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66

77
import org.gradle.api.Action;
88
import org.gradle.api.Project;
9-
import org.gradle.api.artifacts.Dependency;
10-
import org.gradle.api.attributes.HasConfigurableAttributes;
119
import org.gradle.api.file.ConfigurableFileCollection;
1210
import org.gradle.api.file.RegularFileProperty;
1311
import org.gradle.api.logging.LogLevel;
@@ -22,8 +20,6 @@
2220
import java.util.List;
2321

2422
abstract class AccessTransformersContainerImpl implements AccessTransformersContainerInternal {
25-
private final AccessTransformersProblems problems;
26-
2723
private final OptionsImpl options;
2824

2925
protected abstract @Inject ObjectFactory getObjects();
@@ -33,8 +29,6 @@ public AccessTransformersContainerImpl(
3329
Project project,
3430
Action<? super AccessTransformersContainer.Options> options
3531
) {
36-
this.problems = this.getObjects().newInstance(AccessTransformersProblems.class);
37-
3832
options.execute(this.options = this.getObjects().newInstance(OptionsImpl.class, project));
3933
}
4034

@@ -44,11 +38,7 @@ public AccessTransformersContainer.Options getOptions() {
4438
}
4539

4640
@Override
47-
@SuppressWarnings("unchecked")
48-
public void configure(Dependency dependency, Action<? super AccessTransformersConfiguration> action) {
49-
if (!(dependency instanceof HasConfigurableAttributes))
50-
this.problems.reportIllegalTargetDependency(dependency);
51-
41+
public void configure(Object dependency, Action<? super AccessTransformersConfiguration> action) {
5242
var ext = ((ExtensionAware) dependency).getExtensions().getExtraProperties();
5343
var attributes = ext.has("__accessTransformers_configs")
5444
? (List<AccessTransformersConfigurationImpl>) ext.get("__accessTransformers_configs")

at-gradle/src/main/java/net/minecraftforge/accesstransformers/gradle/AccessTransformersContainerInternal.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,16 @@
55
package net.minecraftforge.accesstransformers.gradle;
66

77
import org.gradle.api.Action;
8+
import org.gradle.api.DomainObjectSet;
9+
import org.gradle.api.NamedDomainObjectProvider;
810
import org.gradle.api.Project;
11+
import org.gradle.api.artifacts.DependencyScopeConfiguration;
912
import org.gradle.api.reflect.HasPublicType;
1013
import org.gradle.api.reflect.TypeOf;
1114

1215
non-sealed interface AccessTransformersContainerInternal extends AccessTransformersContainer, HasPublicType {
1316
static AccessTransformersContainer register(Project project, Action<? super Options> options) {
14-
return project.getObjects().newInstance(AccessTransformersContainerImpl.class, options);
17+
return project.getObjects().newInstance(AccessTransformersContainerImpl.class, project, options);
1518
}
1619

1720
@Override

at-gradle/src/main/java/net/minecraftforge/accesstransformers/gradle/AccessTransformersExtensionImpl.java

Lines changed: 83 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,22 @@
66

77
import net.minecraftforge.gradleutils.shared.Closures;
88
import org.gradle.api.Action;
9-
import org.gradle.api.InvalidUserDataException;
10-
import org.gradle.api.NamedDomainObjectProvider;
119
import org.gradle.api.Project;
10+
import org.gradle.api.artifacts.Configuration;
1211
import org.gradle.api.artifacts.Dependency;
13-
import org.gradle.api.artifacts.DependencyScopeConfiguration;
14-
import org.gradle.api.artifacts.MinimalExternalModuleDependency;
12+
import org.gradle.api.artifacts.DependencySubstitutions;
1513
import org.gradle.api.artifacts.ModuleVersionSelector;
1614
import org.gradle.api.artifacts.ProjectDependency;
15+
import org.gradle.api.artifacts.component.ComponentSelector;
1716
import org.gradle.api.artifacts.dsl.DependencyHandler;
1817
import org.gradle.api.artifacts.type.ArtifactTypeDefinition;
1918
import org.gradle.api.attributes.Attribute;
2019
import org.gradle.api.attributes.AttributeContainer;
21-
import org.gradle.api.attributes.HasConfigurableAttributes;
2220
import org.gradle.api.file.ProjectLayout;
2321
import org.gradle.api.file.RegularFile;
2422
import org.gradle.api.file.RegularFileProperty;
2523
import org.gradle.api.model.ObjectFactory;
2624
import org.gradle.api.plugins.ExtensionAware;
27-
import org.gradle.api.plugins.JavaPluginExtension;
2825
import org.gradle.api.provider.ProviderFactory;
2926
import org.jetbrains.annotations.Nullable;
3027

@@ -33,15 +30,15 @@
3330
import java.io.IOException;
3431
import java.util.ArrayList;
3532
import java.util.List;
33+
import java.util.function.Function;
34+
import java.util.stream.Collectors;
3635

3736
abstract class AccessTransformersExtensionImpl implements AccessTransformersExtensionInternal {
3837
private final Project project;
3938

4039
private final AccessTransformersPlugin plugin;
4140
private final AccessTransformersProblems problems = this.getObjects().newInstance(AccessTransformersProblems.class);
4241

43-
private final NamedDomainObjectProvider<DependencyScopeConfiguration> constraintsConfiguration;
44-
4542
private @Nullable AccessTransformersContainer container;
4643

4744
protected abstract @Inject ObjectFactory getObjects();
@@ -56,56 +53,83 @@ public AccessTransformersExtensionImpl(AccessTransformersPlugin plugin, Project
5653
this.plugin = plugin;
5754

5855
var configurations = project.getConfigurations();
59-
{
60-
NamedDomainObjectProvider<DependencyScopeConfiguration> c;
61-
try {
62-
c = configurations.dependencyScope("accessTransformersDependencyConstraints", configuration -> {
63-
configuration.setDescription("Dependency constraints to enforce access transformer usage.");
64-
});
65-
} catch (InvalidUserDataException e) {
66-
c = configurations.named("accessTransformerDependencyConstraints", DependencyScopeConfiguration.class);
56+
configurations.configureEach(configuration -> configuration.withDependencies(dependencies -> {
57+
var constraints = configurations
58+
.stream()
59+
.flatMap(c -> c.getDependencyConstraints().matching(it ->
60+
((ExtensionAware) it).getExtensions().getExtraProperties().has("__accessTransformers_configs")
61+
).stream())
62+
.collect(Collectors.toMap(ModuleVersionSelector::getModule, Function.identity()));
63+
64+
for (var c : configurations) {
65+
for (var dependency : c.getDependencies().matching(it ->
66+
((ExtensionAware) it).getExtensions().getExtraProperties().has("__accessTransformers_configs")
67+
)) {
68+
var configs = (List<AccessTransformersConfigurationImpl>) ((ExtensionAware) dependency).getExtensions().getExtraProperties().get("__accessTransformers_configs");
69+
var attributes = new ArrayList<Attribute<Boolean>>(configs.size());
70+
for (var config : configs) {
71+
attributes.add(this.register(dependency, config));
72+
}
73+
74+
if (dependency instanceof ModuleVersionSelector m) {
75+
var constraint = constraints.remove(m.getModule());
76+
if (constraint != null) {
77+
var constraintConfigs = (List<AccessTransformersConfigurationImpl>) ((ExtensionAware) constraint).getExtensions().getExtraProperties().get("__accessTransformers_configs");
78+
for (var config : constraintConfigs) {
79+
attributes.add(this.register(constraint, config));
80+
}
81+
}
82+
}
83+
84+
Function<DependencySubstitutions, ComponentSelector> componentSelector;
85+
if (dependency instanceof ProjectDependency p) {
86+
var path = p.getPath();
87+
componentSelector = s -> s.project(path);
88+
} else if (dependency instanceof ModuleVersionSelector m) {
89+
var module = "%s%s".formatted(m.getModule().toString(), m.getVersion() != null ? ":" + m.getVersion() : "");
90+
componentSelector = s -> s.module(module);
91+
} else {
92+
this.problems.reportIllegalTargetDependency(dependency);
93+
return;
94+
}
95+
96+
this.apply(configuration, componentSelector, attributes);
97+
}
6798
}
68-
this.constraintsConfiguration = c;
69-
}
7099

71-
// FUCK
72-
project.getGradle().projectsEvaluated(gradle -> this.finish(project));
73-
}
74-
75-
@SuppressWarnings({"unchecked", "DataFlowIssue"})
76-
private void finish(Project project) {
77-
project.getExtensions().getByType(JavaPluginExtension.class).getSourceSets().forEach(sourceSet ->
78-
Util.forEachClasspath(project.getConfigurations(), sourceSet, c -> c.extendsFrom(this.constraintsConfiguration.get()))
79-
);
80-
81-
project.getConfigurations().forEach(c -> c.getDependencies().matching(it ->
82-
((ExtensionAware) it).getExtensions().getExtraProperties().has("__accessTransformers_configs")
83-
).forEach(dependency -> {
84-
Object dependencyNotation = dependency;
85-
if (!(dependencyNotation instanceof MinimalExternalModuleDependency)
86-
&& !(dependencyNotation instanceof ProjectDependency)
87-
&& dependencyNotation instanceof ModuleVersionSelector module) {
88-
dependencyNotation = module.getModule().toString();
89-
}
100+
for (var constraint : constraints.values()) {
101+
var configs = (List<AccessTransformersConfigurationImpl>) ((ExtensionAware) constraint).getExtensions().getExtraProperties().get("__accessTransformers_configs");
102+
var attributes = new ArrayList<Attribute<Boolean>>(configs.size());
103+
for (var config : configs) {
104+
attributes.add(this.register(constraint, config));
105+
}
90106

91-
var configs = (List<AccessTransformersConfigurationImpl>) ((ExtensionAware) dependency).getExtensions().getExtraProperties().get("__accessTransformers_configs");
92-
var attributes = new ArrayList<Attribute<Boolean>>(configs.size());
93-
for (var i = 0; i < configs.size(); i++) {
94-
var config = configs.get(i);
95-
attributes.add(this.register(i, dependency, config));
107+
var module = "%s%s".formatted(constraint.getModule().toString(), constraint.getVersion() != null ? ":" + constraint.getVersion() : "");
108+
this.apply(configuration, s -> s.module(module), attributes);
96109
}
97-
98-
this.constraintsConfiguration.get().getDependencyConstraints().add(this.project.getDependencies().getConstraints().create(dependencyNotation, constraint ->
99-
constraint.attributes(a -> {
100-
for (var attribute : attributes) a.attribute(attribute, true);
101-
})
102-
));
103110
}));
104111
}
105112

106-
private Attribute<Boolean> register(int index, Dependency dependency, AccessTransformersConfigurationImpl config) {
113+
private void apply(Configuration configuration, Function<DependencySubstitutions, ComponentSelector> componentSelector, Iterable<Attribute<Boolean>> attributes) {
114+
configuration.getResolutionStrategy().dependencySubstitution(s -> {
115+
var component = componentSelector.apply(s);
116+
s.substitute(component).using(s.variant(component, variant -> {
117+
variant.attributes(a -> {
118+
for (var attribute : attributes) {
119+
a.attribute(attribute, true);
120+
}
121+
});
122+
}));
123+
});
124+
}
125+
126+
private Attribute<Boolean> register(Object dependency, AccessTransformersConfigurationImpl config) {
107127
this.validateATFile(dependency, config.getConfig());
108128

129+
var ext = this.project.getGradle().getExtensions().getExtraProperties();
130+
int index = ext.has("__accessTransformers_automatic_index") ? (int) ext.get("__accessTransformers_automatic_index") + 1 : 0;
131+
this.project.getGradle().getExtensions().getExtraProperties().set("__accessTransformers_automatic_index", index);
132+
109133
var attribute = Attribute.of("net.minecraftforge.accesstransformers.automatic." + index, Boolean.class);
110134
this.project.dependencies(Closures.<DependencyHandler>consumer(this, dependencies -> {
111135
dependencies.attributesSchema(attributesSchema -> {
@@ -145,39 +169,41 @@ private static void setAttributes(AttributeContainer attributes, Attribute<Boole
145169
.attribute(attribute, value);
146170
}
147171

148-
private void validateATFile(Dependency dependency, RegularFileProperty atFileProperty) {
172+
private void validateATFile(Object dependency, RegularFileProperty atFileProperty) {
173+
var dependencyToString = dependency instanceof Dependency d ? Util.toString(d) : dependency.toString();
174+
149175
// check that consumer has defined the config
150176
RegularFile atFileSource;
151177
try {
152178
atFileSource = atFileProperty.get();
153179
} catch (IllegalStateException e) {
154-
throw this.problems.accessTransformerConfigNotDefined(new RuntimeException("Failed to resolve config file property", e), dependency);
180+
throw this.problems.accessTransformerConfigNotDefined(new RuntimeException("Failed to resolve config file property", e), dependencyToString);
155181
}
156182

157183
// check that the file exists
158184
var atFile = atFileSource.getAsFile();
159185
var atFilePath = this.getLayout().getProjectDirectory().getAsFile().toPath().relativize(atFile.toPath()).toString();
160186
if (!atFile.exists())
161-
throw this.problems.accessTransformerConfigMissing(new RuntimeException(new FileNotFoundException("Config file does not exist at " + atFilePath)), dependency, atFilePath);
187+
throw this.problems.accessTransformerConfigMissing(new RuntimeException(new FileNotFoundException("Config file does not exist at " + atFilePath)), dependencyToString, atFilePath);
162188

163189
// check that the file can be read and isn't empty
164190
String atFileContents;
165191
try {
166192
atFileContents = this.getProviders().fileContents(atFileSource).getAsText().get();
167193
} catch (Throwable e) {
168-
throw this.problems.accessTransformerConfigUnreadable(new RuntimeException(new IOException("Failed to read config file at " + atFilePath, e)), dependency, atFilePath);
194+
throw this.problems.accessTransformerConfigUnreadable(new RuntimeException(new IOException("Failed to read config file at " + atFilePath, e)), dependencyToString, atFilePath);
169195
}
170196
if (atFileContents.isBlank())
171-
throw this.problems.accessTransformerConfigEmpty(new IllegalStateException("Config file must not be blank at " + atFilePath), dependency, atFilePath);
197+
throw this.problems.accessTransformerConfigEmpty(new IllegalStateException("Config file must not be blank at " + atFilePath), dependencyToString, atFilePath);
172198
}
173199

174200
@Override
175201
public AccessTransformersContainer register(Action<? super AccessTransformersContainer.Options> options) {
176-
return this.container = AccessTransformersContainer.register(this.project, options);
202+
return this.container = AccessTransformersContainerInternal.register(this.project, options);
177203
}
178204

179205
private AccessTransformersContainer getContainer() {
180-
return this.container == null ? this.container = AccessTransformersContainer.register(this.project, it -> { }) : this.container;
206+
return this.container == null ? this.container = AccessTransformersContainerInternal.register(this.project, it -> { }) : this.container;
181207
}
182208

183209
@Override
@@ -191,7 +217,7 @@ public void options(Action<? super Options> action) {
191217
}
192218

193219
@Override
194-
public void configure(Dependency dependency, Action<? super AccessTransformersConfiguration> action) {
220+
public void configure(Object dependency, Action<? super AccessTransformersConfiguration> action) {
195221
this.getContainer().configure(dependency, action);
196222
}
197223
}

0 commit comments

Comments
 (0)