cstamas commented on code in PR #272: URL: https://github.com/apache/maven-resolver/pull/272#discussion_r1158052421
########## maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultArtifactResolver.java: ########## @@ -256,207 +257,226 @@ public List<ArtifactResult> resolveArtifacts( artifacts.add(request.getArtifact()); } - syncContext.acquire(artifacts, null); - - return resolve(session, requests); + return resolve(shared, exclusive, artifacts, session, requests); } } @SuppressWarnings("checkstyle:methodlength") private List<ArtifactResult> resolve( - RepositorySystemSession session, Collection<? extends ArtifactRequest> requests) + SyncContext shared, + SyncContext exclusive, + Collection<Artifact> subjects, + RepositorySystemSession session, + Collection<? extends ArtifactRequest> requests) throws ArtifactResolutionException { - List<ArtifactResult> results = new ArrayList<>(requests.size()); - boolean failures = false; - final boolean simpleLrmInterop = ConfigUtils.getBoolean(session, false, CONFIG_PROP_SIMPLE_LRM_INTEROP); + SyncContext current = shared; + try { + while (true) { + current.acquire(subjects, null); - LocalRepositoryManager lrm = session.getLocalRepositoryManager(); - WorkspaceReader workspace = session.getWorkspaceReader(); + boolean failures = false; + final List<ArtifactResult> results = new ArrayList<>(requests.size()); + final boolean simpleLrmInterop = ConfigUtils.getBoolean(session, false, CONFIG_PROP_SIMPLE_LRM_INTEROP); + final LocalRepositoryManager lrm = session.getLocalRepositoryManager(); + final WorkspaceReader workspace = session.getWorkspaceReader(); + final List<ResolutionGroup> groups = new ArrayList<>(); + // filter != null: means "filtering applied", if null no filtering applied (behave as before) + final RemoteRepositoryFilter filter = remoteRepositoryFilterManager.getRemoteRepositoryFilter(session); - List<ResolutionGroup> groups = new ArrayList<>(); - // filter != null: means "filtering applied", if null no filtering applied (behave as before) - RemoteRepositoryFilter filter = remoteRepositoryFilterManager.getRemoteRepositoryFilter(session); + for (ArtifactRequest request : requests) { + RequestTrace trace = RequestTrace.newChild(request.getTrace(), request); - for (ArtifactRequest request : requests) { - RequestTrace trace = RequestTrace.newChild(request.getTrace(), request); + ArtifactResult result = new ArtifactResult(request); + results.add(result); - ArtifactResult result = new ArtifactResult(request); - results.add(result); + Artifact artifact = request.getArtifact(); - Artifact artifact = request.getArtifact(); + if (current == shared) { + artifactResolving(session, trace, artifact); + } - artifactResolving(session, trace, artifact); + String localPath = artifact.getProperty(ArtifactProperties.LOCAL_PATH, null); + if (localPath != null) { + // unhosted artifact, just validate file + File file = new File(localPath); + if (!file.isFile()) { + failures = true; + result.addException(new ArtifactNotFoundException(artifact, null)); + } else { + artifact = artifact.setFile(file); + result.setArtifact(artifact); + artifactResolved(session, trace, artifact, null, result.getExceptions()); + } + continue; + } - String localPath = artifact.getProperty(ArtifactProperties.LOCAL_PATH, null); - if (localPath != null) { - // unhosted artifact, just validate file - File file = new File(localPath); - if (!file.isFile()) { - failures = true; - result.addException(new ArtifactNotFoundException(artifact, null)); - } else { - artifact = artifact.setFile(file); - result.setArtifact(artifact); - artifactResolved(session, trace, artifact, null, result.getExceptions()); - } - continue; - } + List<RemoteRepository> remoteRepositories = request.getRepositories(); + List<RemoteRepository> filteredRemoteRepositories = new ArrayList<>(remoteRepositories); + if (filter != null) { + for (RemoteRepository repository : remoteRepositories) { + RemoteRepositoryFilter.Result filterResult = filter.acceptArtifact(repository, artifact); + if (!filterResult.isAccepted()) { + result.addException(new ArtifactFilteredOutException( + artifact, repository, filterResult.reasoning())); + filteredRemoteRepositories.remove(repository); + } + } + } - List<RemoteRepository> remoteRepositories = request.getRepositories(); - List<RemoteRepository> filteredRemoteRepositories = new ArrayList<>(remoteRepositories); - if (filter != null) { - for (RemoteRepository repository : remoteRepositories) { - RemoteRepositoryFilter.Result filterResult = filter.acceptArtifact(repository, artifact); - if (!filterResult.isAccepted()) { - result.addException( - new ArtifactFilteredOutException(artifact, repository, filterResult.reasoning())); - filteredRemoteRepositories.remove(repository); + VersionResult versionResult; + try { + VersionRequest versionRequest = + new VersionRequest(artifact, filteredRemoteRepositories, request.getRequestContext()); + versionRequest.setTrace(trace); + versionResult = versionResolver.resolveVersion(session, versionRequest); + } catch (VersionResolutionException e) { + result.addException(e); + continue; } - } - } - VersionResult versionResult; - try { - VersionRequest versionRequest = - new VersionRequest(artifact, filteredRemoteRepositories, request.getRequestContext()); - versionRequest.setTrace(trace); - versionResult = versionResolver.resolveVersion(session, versionRequest); - } catch (VersionResolutionException e) { - result.addException(e); - continue; - } + artifact = artifact.setVersion(versionResult.getVersion()); - artifact = artifact.setVersion(versionResult.getVersion()); + if (versionResult.getRepository() != null) { + if (versionResult.getRepository() instanceof RemoteRepository) { + filteredRemoteRepositories = + Collections.singletonList((RemoteRepository) versionResult.getRepository()); + } else { + filteredRemoteRepositories = Collections.emptyList(); + } + } - if (versionResult.getRepository() != null) { - if (versionResult.getRepository() instanceof RemoteRepository) { - filteredRemoteRepositories = - Collections.singletonList((RemoteRepository) versionResult.getRepository()); - } else { - filteredRemoteRepositories = Collections.emptyList(); - } - } + if (workspace != null) { + File file = workspace.findArtifact(artifact); + if (file != null) { + artifact = artifact.setFile(file); + result.setArtifact(artifact); + result.setRepository(workspace.getRepository()); + artifactResolved(session, trace, artifact, result.getRepository(), null); + continue; + } + } - if (workspace != null) { - File file = workspace.findArtifact(artifact); - if (file != null) { - artifact = artifact.setFile(file); - result.setArtifact(artifact); - result.setRepository(workspace.getRepository()); - artifactResolved(session, trace, artifact, result.getRepository(), null); - continue; - } - } + LocalArtifactResult local = lrm.find( + session, + new LocalArtifactRequest( + artifact, filteredRemoteRepositories, request.getRequestContext())); + result.setLocalArtifactResult(local); + boolean found = (filter != null && local.isAvailable()) || isLocallyInstalled(local, versionResult); + // with filtering it is availability that drives logic + // without filtering it is simply presence of file that drives the logic + // "interop" logic with simple LRM leads to RRF breakage: hence is ignored when filtering in effect + if (found) { + if (local.getRepository() != null) { + result.setRepository(local.getRepository()); + } else { + result.setRepository(lrm.getRepository()); + } + + try { + artifact = artifact.setFile(getFile(session, artifact, local.getFile())); + result.setArtifact(artifact); + artifactResolved(session, trace, artifact, result.getRepository(), null); + } catch (ArtifactTransferException e) { + result.addException(e); + } + if (filter == null && simpleLrmInterop && !local.isAvailable()) { + /* + * NOTE: Interop with simple local repository: An artifact installed by a simple local repo + * manager will not show up in the repository tracking file of the enhanced local repository. + * If however the maven-metadata-local.xml tells us the artifact was installed locally, we + * sync the repository tracking file. + */ + lrm.add(session, new LocalArtifactRegistration(artifact)); + } + + continue; + } - LocalArtifactResult local = lrm.find( - session, - new LocalArtifactRequest(artifact, filteredRemoteRepositories, request.getRequestContext())); - result.setLocalArtifactResult(local); - boolean found = (filter != null && local.isAvailable()) || isLocallyInstalled(local, versionResult); - // with filtering it is availability that drives logic - // without filtering it is simply presence of file that drives the logic - // "interop" logic with simple LRM leads to RRF breakage: hence is ignored when filtering in effect - if (found) { - if (local.getRepository() != null) { - result.setRepository(local.getRepository()); - } else { - result.setRepository(lrm.getRepository()); - } + if (local.getFile() != null) { + LOGGER.info( + "Artifact {} is present in the local repository, but cached from a remote repository ID that is unavailable in current build context, verifying that is downloadable from {}", + artifact, + remoteRepositories); + } - try { - artifact = artifact.setFile(getFile(session, artifact, local.getFile())); - result.setArtifact(artifact); - artifactResolved(session, trace, artifact, result.getRepository(), null); - } catch (ArtifactTransferException e) { - result.addException(e); - } - if (filter == null && simpleLrmInterop && !local.isAvailable()) { - /* - * NOTE: Interop with simple local repository: An artifact installed by a simple local repo - * manager will not show up in the repository tracking file of the enhanced local repository. - * If however the maven-metadata-local.xml tells us the artifact was installed locally, we - * sync the repository tracking file. - */ - lrm.add(session, new LocalArtifactRegistration(artifact)); + LOGGER.debug("Resolving artifact {} from {}", artifact, remoteRepositories); + AtomicBoolean resolved = new AtomicBoolean(false); + Iterator<ResolutionGroup> groupIt = groups.iterator(); + for (RemoteRepository repo : filteredRemoteRepositories) { + if (!repo.getPolicy(artifact.isSnapshot()).isEnabled()) { + continue; + } + + try { + Utils.checkOffline(session, offlineController, repo); + } catch (RepositoryOfflineException e) { + Exception exception = new ArtifactNotFoundException( + artifact, + repo, + "Cannot access " + repo.getId() + " (" + + repo.getUrl() + ") in offline mode and the artifact " + artifact + + " has not been downloaded from it before.", + e); + result.addException(exception); + continue; + } + + ResolutionGroup group = null; + while (groupIt.hasNext()) { + ResolutionGroup t = groupIt.next(); + if (t.matches(repo)) { + group = t; + break; + } + } + if (group == null) { + group = new ResolutionGroup(repo); + groups.add(group); + groupIt = Collections.emptyIterator(); + } + group.items.add(new ResolutionItem(trace, artifact, resolved, result, local, repo)); + } } - continue; - } - - if (local.getFile() != null) { - LOGGER.info( - "Artifact {} is present in the local repository, but cached from a remote repository ID that is unavailable in current build context, verifying that is downloadable from {}", - artifact, - remoteRepositories); - } - - LOGGER.debug("Resolving artifact {} from {}", artifact, remoteRepositories); - AtomicBoolean resolved = new AtomicBoolean(false); - Iterator<ResolutionGroup> groupIt = groups.iterator(); - for (RemoteRepository repo : filteredRemoteRepositories) { - if (!repo.getPolicy(artifact.isSnapshot()).isEnabled()) { + if (!groups.isEmpty() && current == shared) { Review Comment: Yes, groups are "prepared download batches" (per remote repo, 1 or more artifact). By group being not empty, means that resolver could not fulfill resolution from local repo, hence is about to download. Here, the "downside" is that when this happens, and we switch from shared to exclusive lock, groups are rebuilt from scratch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org