Skip to content

Commit

Permalink
Swap back to detached configurations for internal configurations.
Browse files Browse the repository at this point in the history
This prevents a CME caused by the resolution of the dependent projects configurations triggering the creation of new configuration in its configuration container.
Creating detached configurations, and using them either as tool resolvers, or as carriers for dependency objects which are then attached to an outer configuration should prevent this.
Added tests that cover the new corver cases, and validate publishing logic
  • Loading branch information
marchermans committed Oct 31, 2024
1 parent 24c0adc commit e6f6b78
Show file tree
Hide file tree
Showing 27 changed files with 446 additions and 131 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,10 @@ public void apply(Project project) {
//Set up reporting tasks
project.getTasks().register("runs", RunsReport.class);

//Configure sdk configurations
final SourceSetContainer sourceSets = project.getExtensions().getByType(SourceSetContainer.class);
sourceSets.all(ConfigurationUtils::getSdkConfiguration);

//Needs to be before after evaluate
ConventionConfigurator.configureConventions(project);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import net.neoforged.gradle.dsl.common.extensions.subsystems.conventions.Runs;
import net.neoforged.gradle.dsl.common.extensions.subsystems.conventions.SourceSets;
import net.neoforged.gradle.dsl.common.extensions.subsystems.conventions.ide.IDEA;
import net.neoforged.gradle.dsl.common.runs.run.Run;
import net.neoforged.gradle.dsl.common.runs.run.RunManager;
import org.gradle.StartParameter;
import org.gradle.TaskExecutionRequest;
Expand Down Expand Up @@ -62,11 +63,23 @@ private static void configureRunConventions(Project project, Conventions convent
final Configuration runRuntimeConfiguration = project.getConfigurations().maybeCreate(configurations.getRunRuntimeConfigurationName().get());

project.getExtensions().configure(RunManager.class, runContainer -> runContainer.configureAll(run -> {
run.getDependencies().getRuntime().add(runRuntimeConfiguration);
}));

final RunManager runManager = project.getExtensions().getByType(RunManager.class);
project.getConfigurations().addRule("Create run specific runtime configuration", configurationName -> {
if (!configurationName.endsWith(configurations.getPerRunRuntimeConfigurationPostFix().get()))
return;

final String runName = configurationName.substring(0, configurationName.length() - configurations.getPerRunRuntimeConfigurationPostFix().get().length());
if (runManager.findByName(runName) == null)
return;

final Run run = runManager.getByName(runName);
final Configuration runSpecificRuntimeConfiguration = project.getConfigurations().maybeCreate(ConfigurationUtils.getRunName(run, configurations.getPerRunRuntimeConfigurationPostFix().get()));

run.getDependencies().getRuntime().add(runRuntimeConfiguration);
run.getDependencies().getRuntime().add(runSpecificRuntimeConfiguration);
}));
});
}

private static void configureIDEConventions(Project project, Conventions conventions) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import net.minecraftforge.gdi.ConfigurableDSLElement;
import net.neoforged.gradle.common.extensions.IdeManagementExtension;
import net.neoforged.gradle.common.tasks.ArtifactFromOutput;
import net.neoforged.gradle.common.util.ConfigurationUtils;
import net.neoforged.gradle.dsl.common.extensions.dependency.replacement.DependencyReplacement;
import net.neoforged.gradle.dsl.common.extensions.dependency.replacement.DependencyReplacementHandler;
import net.neoforged.gradle.dsl.common.extensions.dependency.replacement.ReplacementAware;
Expand All @@ -13,7 +14,6 @@
import net.neoforged.gradle.dsl.common.extensions.repository.Repository;
import net.neoforged.gradle.dsl.common.tasks.WithOutput;
import net.neoforged.gradle.dsl.common.util.CommonRuntimeUtils;
import net.neoforged.gradle.common.util.ConfigurationUtils;
import org.gradle.api.GradleException;
import org.gradle.api.NamedDomainObjectContainer;
import org.gradle.api.Project;
Expand All @@ -27,6 +27,7 @@

import javax.inject.Inject;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;

/**
* Defines the implementation of the @{link DependencyReplacement} extension.
Expand All @@ -39,7 +40,7 @@ public abstract class ReplacementLogic implements ConfigurableDSLElement<Depende

private final Table<Dependency, Configuration, Optional<ReplacementResult>> dependencyReplacementInformation = HashBasedTable.create();
private final Table<Dependency, ReplacementResult, Entry> repositoryEntries = HashBasedTable.create();
private final Table<Dependency, Configuration, Dependency> originalDependencyLookup = HashBasedTable.create();
private final Map<Dependency, Dependency> originalDependencyLookup = new ConcurrentHashMap<>();
private final NamedDomainObjectContainer<DependencyReplacementHandler> dependencyReplacementHandlers;

private final List<DependencyReplacedCallback> whenDependencyReplaced = new ArrayList<>();
Expand All @@ -59,9 +60,7 @@ public ReplacementLogic(Project project) {
public void whenDependencyReplaced(DependencyReplacedCallback dependencyAction) {
this.whenDependencyReplaced.add(dependencyAction);

for (Table.Cell<Dependency, Configuration, Dependency> dependencyConfigurationDependencyCell : this.originalDependencyLookup.cellSet()) {
dependencyAction.apply(dependencyConfigurationDependencyCell.getRowKey(), dependencyConfigurationDependencyCell.getColumnKey(), dependencyConfigurationDependencyCell.getValue());
}
this.originalDependencyLookup.forEach(dependencyAction::apply);
}

@Override
Expand Down Expand Up @@ -113,18 +112,9 @@ public NamedDomainObjectContainer<DependencyReplacementHandler> getReplacementHa

@NotNull
@Override
public Dependency optionallyConvertBackToOriginal(@NotNull final Dependency dependency, Configuration configuration) {
final Dependency originalDependency = originalDependencyLookup.get(dependency, configuration);
if (originalDependency == null && !configuration.getExtendsFrom().isEmpty()) {
//Check if we have a parent configuration that might have the original dependency.
for (Configuration parentConfiguration : configuration.getExtendsFrom()) {
return optionallyConvertBackToOriginal(dependency, parentConfiguration);
}
} else if (originalDependency != null) {
return originalDependency;
}

return dependency;
public Dependency optionallyConvertBackToOriginal(@NotNull final Dependency dependency) {
final Dependency originalDependency = originalDependencyLookup.get(dependency);
return Objects.requireNonNullElse(originalDependency, dependency);
}

/**
Expand Down Expand Up @@ -273,36 +263,34 @@ private void handleDependencyReplacement(Configuration configuration, Dependency
//Find the configurations that the dependency should be replaced in.
final List<Configuration> targetConfigurations = ConfigurationUtils.findReplacementConfigurations(project, configuration);

//Create a dependency from the tasks that copies the raw jar to the repository.
//The sources jar is not needed here.
final ConfigurableFileCollection replacedFiles = createDependencyFromTask(rawTask);
final Dependency replacedDependency = project.getDependencies().create(replacedFiles);
final Dependency localRepoDependency = newRepoEntry.getDependency();

//For each configuration that we target we now need to add the new dependencies to.
for (Configuration targetConfiguration : targetConfigurations) {
try {
//Create a dependency from the tasks that copies the raw jar to the repository.
//The sources jar is not needed here.
final Provider<ConfigurableFileCollection> replacedFiles = createDependencyFromTask(rawTask);

//Add the new dependency to the target configuration.
final DependencySet targetDependencies = targetConfiguration == configuration ?
configuredSet :
targetConfiguration.getDependencies();

final Provider<Dependency> replacedDependencies = replacedFiles
.map(files -> project.getDependencies().create(files));
final Provider<Dependency> newRepoDependency = project.provider(newRepoEntry::getDependency);

//Add the new dependency to the target configuration.
targetDependencies.addLater(replacedDependencies);
targetDependencies.addLater(newRepoDependency);

//Keep track of the original dependency, so we can convert back if needed.
originalDependencyLookup.put(newRepoEntry.getDependency(), targetConfiguration, dependency);

for (DependencyReplacedCallback dependencyReplacedCallback : this.whenDependencyReplaced) {
dependencyReplacedCallback.apply(newRepoEntry.getDependency(), targetConfiguration, dependency);
}
targetDependencies.add(replacedDependency);
targetDependencies.add(localRepoDependency);
} catch (Exception exception) {
throw new GradleException("Failed to add the replaced dependency to the configuration " + targetConfiguration.getName() + ": " + exception.getMessage(), exception);
}
}

//Keep track of the original dependency, so we can convert back if needed.
originalDependencyLookup.put(localRepoDependency, dependency);

for (DependencyReplacedCallback dependencyReplacedCallback : this.whenDependencyReplaced) {
dependencyReplacedCallback.apply(localRepoDependency, dependency);
}
}

/**
Expand Down Expand Up @@ -475,7 +463,7 @@ Entry createDummyDependency(final Dependency dependency, final ReplacementResult
return entry;
}

public Provider<ConfigurableFileCollection> createDependencyFromTask(TaskProvider<? extends WithOutput> task) {
return task.map(taskWithOutput -> project.files(taskWithOutput.getOutput()));
public ConfigurableFileCollection createDependencyFromTask(TaskProvider<? extends WithOutput> task) {
return project.files(task.flatMap(WithOutput::getOutput));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ private void configureFromSDKs() {
});

final DependencyReplacement replacementLogic = project.getExtensions().getByType(DependencyReplacement.class);
replacementLogic.whenDependencyReplaced((virtualDependency, targetConfiguration, originalDependency) -> {
replacementLogic.whenDependencyReplaced((virtualDependency, originalDependency) -> {
if (unconfiguredSourceSets.isEmpty()) {
return;
}
Expand Down
Loading

0 comments on commit e6f6b78

Please sign in to comment.