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

[SHRINKRES-351] enh: using maven 4 project-local dependency resolution, if available #341

Merged
merged 2 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,8 @@ ShrinkWrap Resolvers allows you to override any programmatic configuration via S
- `maven.repo.local`: Path to local repository with cached artifacts. Overrides value defined in any of the settings.xml files.

- `maven.legacyLocalRepo`: Flag whether to ignore origin tracking for artifacts present in local repository.
- `org.jboss.shrinkwrap.resolver.maven.skipCompilation`: Flag to skip compilation of resolved artifacts (true/false) - default is false.
- `org.jboss.shrinkwrap.resolver.maven.disableProjectLocal`: Flag to disable Maven 4 project-local repository (true/false) - default is false.


## Embedded Maven
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,4 +135,17 @@ PomEquippedMavenImporter loadPomFromClassLoaderResource(String pathToPomResource
* @return Modified {@link PomlessMavenImporter} instance
*/
PomlessMavenImporter offline();

/**
* <i>Optional operation</i>. Skip dependency compilation during import Default is false
* @param skip boolean flag
* @return this configured {@link MavenImporter}
*/
ConfiguredMavenImporter skipCompilation(boolean skip);

/**
* <i>Optional operation</i>. Skip dependency compilation during import
* @return this configured {@link MavenImporter}
*/
ConfiguredMavenImporter skipCompilation();
}
Original file line number Diff line number Diff line change
Expand Up @@ -139,4 +139,14 @@ Collection<MavenResolvedArtifact> resolveDependencies(MavenResolutionStrategy st
* @throws IllegalArgumentException if argument is null
*/
void addRemoteRepo(MavenRemoteRepository repository);

/**
* @return true if dependency compilation is skipped
*/
boolean skipCompilation();

/**
* <i>Optional operation</i>. Skip dependency compilation during import
*/
void skipCompilation(boolean skip);
}
Original file line number Diff line number Diff line change
Expand Up @@ -183,4 +183,14 @@ public PomlessMavenImporter offline() {
return this.offline(true);
}

@Override
public ConfiguredMavenImporter skipCompilation(boolean skip) {
session.skipCompilation(skip);
return this;
}

@Override
public ConfiguredMavenImporter skipCompilation() {
return this.skipCompilation(true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,34 @@ public abstract class AbstractCompilingProcessor<ARCHIVETYPE extends Archive<ARC

// this pattern is used to check whether archive name was autogenerated
private static final Pattern UUID4_PATTERN = Pattern.compile("[a-f0-9]{8}-[a-f0-9]{4}-4[a-f0-9]{3}-[89aAbB][a-f0-9]{3}-[a-f0-9]{12}\\.[a-z]+");
private JavacCompiler compiler;
private boolean skipCompilation;

protected MavenWorkingSession session;

protected PackagingProcessor<ARCHIVETYPE> configure(MavenWorkingSession session) {
this.session = session;
this.skipCompilation = Boolean.getBoolean("org.jboss.shrinkwrap.resolver.maven.skipCompilation") || session.skipCompilation();
if (skipCompilation) {
log.fine("Compilation was skipped due to system property org.jboss.shrinkwrap.resolver.maven.importer.skipCompilation being set to true");
} else {
compiler = new JavacCompiler();
}
return this;
}

protected AbstractCompilingProcessor<ARCHIVETYPE> compile(File inputDirectory, File outputDirectory, ScopeType... scopes) {

Validate.notNullAndNoNullValues(scopes, "Cannot compile sources, there were no scopes defined");
Validate.notNull(inputDirectory, "Directory with sources to be compiled must not be null");
Validate.notNull(outputDirectory, "Target directory for compiled sources must not be null");

JavacCompiler compiler = new JavacCompiler();
if (compiler == null) {
if (!skipCompilation) {
log.warning("No compiler found, skipping compilation");
}
return this;
}

CompilerConfiguration configuration = getCompilerConfiguration();

if (log.isLoggable(Level.FINE)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,9 @@ private void extractFileInDestinationDir() {
} catch (IOException e) {
System.err.println("Failed to unzip file: " + e.getMessage());
}
markerFileHandler.deleteMarkerFile();
System.out.printf("Resolver: Successfully extracted maven binaries from %s%n", fileToExtract);
if (markerFileHandler.deleteMarkerFile()) {
Copy link
Member

Choose a reason for hiding this comment

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

Even though this is just a simple print, I wouldn't make printing the line dependent on deleting the file. Especially if the file is not related to the actual extraction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is because I was trying to fix flaky test that relies on this message.
Failing to delete marker file indicates a failure in the code, at least that's how I am reading this,
so actual functionality should be correct.

Copy link
Member

Choose a reason for hiding this comment

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

Got it, found the test. Must admit, not my favorite way of checking whether the file was extracted, but I can understand this change.

Can I ask you to put this in a separate commit? As this isn't related to the project-local dependencies I'd prefer git blame to show a related commit when it comes to checking history of this change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

System.out.printf("Resolver: Successfully extracted maven binaries from %s%n", fileToExtract);
}
}

private static InputStream getCompressorInputStream(String fileExtension, FileInputStream fileInputStream) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ void createMarkerFile() {
}
}

void deleteMarkerFile() {
boolean deleteMarkerFile() {
if (markerFile.exists() && !markerFile.delete()) {
log.warning("failed to delete marker file: " + markerFile);
return false;
}
return true;
}

boolean waitTillMarkerFileIsGone(long timeoutInMilliseconds, String processName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ void testDaemonShouldWaitForBuildSuccess() throws TimeoutException {
.withWaitUntilOutputLineMatches(".*BUILD SUCCESS.*")
.build();

Awaitility.await("Wait till thread is not be alive").atMost(20, TimeUnit.SECONDS)
Awaitility.await("Wait till thread is not be alive").atMost(45, TimeUnit.SECONDS)
Copy link
Member

Choose a reason for hiding this comment

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

Why increasing the timeout? Did the changes in the PR resulted in longer run?

Copy link
Contributor Author

@lprimak lprimak Sep 25, 2024

Choose a reason for hiding this comment

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

Same here, I noticed flaky test in CI. Has nothing to do with the changes in this PR, just trying to fix flakiness as I see it :)

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me. Can I ask for a separate commit here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I will do that. thanks

.until(() -> !daemonBuild.isAlive());

Assertions.assertNotNull(daemonBuild.getBuiltProject());
Expand All @@ -60,7 +60,7 @@ void testDaemonWithoutWaitShouldNotReachTheEndOfTheBuild() {
Assertions.assertTrue(daemonBuild.isAlive());
Assertions.assertNull(daemonBuild.getBuiltProject());

Awaitility.await("Wait till thread is not be alive").atMost(20, TimeUnit.SECONDS)
Awaitility.await("Wait till thread is not be alive").atMost(45, TimeUnit.SECONDS)
.until(() -> !daemonBuild.isAlive());

Assertions.assertFalse(daemonBuild.isAlive());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public abstract class ConfigurableMavenWorkingSessionImpl implements MavenWorkin
private boolean useLegacyLocalRepository = false;
private final MavenRepositorySystem system;
private boolean disableClassPathWorkspaceReader = false;
private boolean skipCompilation = false;

public ConfigurableMavenWorkingSessionImpl() {
this.system = new MavenRepositorySystem();
Expand Down Expand Up @@ -98,6 +99,16 @@ public void useLegacyLocalRepository(boolean useLegacyLocalRepository) {
this.useLegacyLocalRepository = useLegacyLocalRepository;
}

@Override
public boolean skipCompilation() {
return this.skipCompilation;
}

@Override
public void skipCompilation(boolean skip) {
this.skipCompilation = skip;
}

/**
* Returns an instance of the {@link DefaultRepositorySystemSession} that is generated if it hasn't been yet.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,21 @@
import eu.maveniverse.maven.mima.context.Runtime;
import eu.maveniverse.maven.mima.context.Runtimes;
import java.io.File;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Properties;
import java.util.Set;
import java.util.function.Predicate;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;

import org.apache.maven.model.Model;
import org.apache.maven.model.Profile;
Expand All @@ -42,12 +47,16 @@
import org.apache.maven.model.building.ModelBuildingResult;
import org.apache.maven.model.building.ModelProblem;
import org.eclipse.aether.artifact.Artifact;
import org.eclipse.aether.artifact.DefaultArtifact;
import org.eclipse.aether.collection.CollectRequest;
import org.eclipse.aether.collection.DependencyCollectionException;
import org.eclipse.aether.collection.DependencySelector;
import org.eclipse.aether.graph.DefaultDependencyNode;
import org.eclipse.aether.graph.Dependency;
import org.eclipse.aether.repository.RemoteRepository;
import org.eclipse.aether.repository.RemoteRepository.Builder;
import org.eclipse.aether.repository.RepositoryPolicy;
import org.eclipse.aether.resolution.ArtifactRequest;
import org.eclipse.aether.resolution.ArtifactResolutionException;
import org.eclipse.aether.resolution.ArtifactResult;
import org.eclipse.aether.resolution.DependencyResolutionException;
Expand Down Expand Up @@ -188,6 +197,92 @@ public MavenWorkingSession loadPomFromFile(File pomFile, Properties userProperti
return this;
}

private Collection<ArtifactResult> resolveProjectLocal(final List<MavenDependency> depsForResolution,
Set<MavenDependency> additionalDependencies) {
Collection<ArtifactResult> projectLocalDependencies = new ArrayList<>(depsForResolution.size());
for (MavenDependency dependency : depsForResolution) {
Path resolved = resolveProjectLocal(dependency.getGroupId(), dependency.getArtifactId(),
dependency.getVersion(), Optional.ofNullable(dependency.getClassifier()),
Optional.ofNullable(dependency.getPackaging().getExtension()), additionalDependencies);
if (resolved != null && resolved.toFile().exists()) {
Artifact artifact = new DefaultArtifact(dependency.getGroupId(), dependency.getArtifactId(),
dependency.getClassifier(), dependency.getPackaging().getExtension(), dependency.getVersion(),
null, resolved.toFile());
ArtifactResult result = new ArtifactResult(new ArtifactRequest()
.setDependencyNode(new DefaultDependencyNode(
new Dependency(artifact, dependency.getScope().name(), dependency.isOptional()))));
result.setArtifact(artifact);
projectLocalDependencies.add(result);
}
}
return projectLocalDependencies;
}

private Path resolveProjectLocal(String groupId, String artifactId, String version,
Optional<String> classifier, Optional<String> extension,
Set<MavenDependency> additionalDependencies) {
Path projectLocalRepository = findProjectLocalRepository();
if (projectLocalRepository == null) {
return null;
}

Predicate<String> isNotEmpty = s -> !s.isEmpty();
processAdditionalDependencies(projectLocalRepository, groupId, artifactId, version,
additionalDependencies);

return projectLocalRepository.resolve(groupId).resolve(artifactId).resolve(version)
.resolve(toVersionedArtifact(artifactId, version)
+ classifier.filter(isNotEmpty).map(c -> "-" + c).orElse("")
+ "." + extension.filter(isNotEmpty).orElse("jar"));
}

private static String toVersionedArtifact(String artifactId, String version) {
return artifactId + "-" + version;
}

private void processAdditionalDependencies(Path projectLocalRepository, String groupId,
String artifactId, String version,
Set<MavenDependency> additionalDependencies) {
Path directory = projectLocalRepository.resolve(groupId).resolve(artifactId).resolve(version);
File consumerPom = directory.resolve(toVersionedArtifact(artifactId, version) + "-consumer.pom").toFile();
if (consumerPom.exists()) {
Set<MavenDependency> transitiveDependencies = loadPomFromFile(consumerPom).getParsedPomFile().getDependencies();
transitiveDependencies.removeAll(additionalDependencies);
if (!transitiveDependencies.isEmpty()) {
additionalDependencies.addAll(transitiveDependencies);
transitiveDependencies.forEach(dependency -> processAdditionalDependencies(projectLocalRepository,
dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion(), additionalDependencies));
}
}
}

private List<MavenDependency> filterFromLocal(final List<MavenDependency> depsForResolution,
final Collection<ArtifactResult> projectLocalDependencies) {
return depsForResolution.stream()
.filter(dependency -> projectLocalDependencies.stream()
.noneMatch(result -> MavenConverter.asArtifact(dependency,
getSession().getArtifactTypeRegistry())
.setProperties(Collections.EMPTY_MAP)
.equals(result.getArtifact().setFile(null))))
.collect(Collectors.toList());
}

/**
* @return absolute path to the project-local repository or null if not found
*/
private Path findProjectLocalRepository() {
Path targetPath = Paths.get("target/project-local-repo");
Path currentPath = Paths.get("").toAbsolutePath();
while (currentPath != null) {
Path path = currentPath.resolve(targetPath);
if (path.toFile().exists()) {
return path;
}
currentPath = currentPath.getParent();
}
return null;
}

@Override
public Collection<MavenResolvedArtifact> resolveDependencies(final MavenResolutionStrategy strategy)
throws ResolutionException {
Expand All @@ -198,7 +293,18 @@ public Collection<MavenResolvedArtifact> resolveDependencies(final MavenResoluti

final List<RemoteRepository> repos = this.getRemoteRepositories();

final CollectRequest request = new CollectRequest(MavenConverter.asDependencies(depsForResolution,
List<MavenDependency> resolveFromRepository;
Collection<ArtifactResult> projectLocalDependencies = Collections.emptyList();
if (Boolean.getBoolean("org.jboss.shrinkwrap.resolver.maven.disableProjectLocal")) {
resolveFromRepository = depsForResolution;
} else {
Set<MavenDependency> allDependencies = new LinkedHashSet<>(depsForResolution);
projectLocalDependencies = resolveProjectLocal(depsForResolution, allDependencies);
resolveFromRepository = filterFromLocal(
allDependencies.stream().collect(Collectors.toList()), projectLocalDependencies);
}

final CollectRequest request = new CollectRequest(MavenConverter.asDependencies(resolveFromRepository,
getSession().getArtifactTypeRegistry()),
MavenConverter.asDependencies(depManagement, getSession().getArtifactTypeRegistry()), repos);

Expand Down Expand Up @@ -231,7 +337,11 @@ public Collection<MavenResolvedArtifact> resolveDependencies(final MavenResoluti
throw wrapException(e);
}

final Collection<MavenResolvedArtifact> resolvedArtifacts = new ArrayList<>(results.size());
final Collection<MavenResolvedArtifact> resolvedArtifacts = new ArrayList<>(results.size() + projectLocalDependencies.size());

for (final ArtifactResult result : projectLocalDependencies) {
resolvedArtifacts.add(MavenResolvedArtifactImpl.fromArtifactResult(result));
}

for (final ArtifactResult result : results) {
resolvedArtifacts.add(MavenResolvedArtifactImpl.fromArtifactResult(result));
Expand Down