Skip to content

Commit

Permalink
Fix #496 ResolutionArguments#hashcode is not stable
Browse files Browse the repository at this point in the history
Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
  • Loading branch information
laeubi committed Jan 4, 2022
1 parent 5ca5155 commit cf98ee4
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 16 deletions.
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2012, 2020 SAP SE and others.
* Copyright (c) 2012, 2022 SAP SE and others.
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
Expand All @@ -12,14 +12,15 @@
* Christoph Läubrich - [Bug 538144] - Support other target locations (Directory, Features, Installations)
* - [Bug 533747] - Target file is read and parsed over and over again
* - [Bug 568729] - Support new "Maven" Target location
* - [Issue #496] - ResolutionArguments#hashcode is not stable
*******************************************************************************/
package org.eclipse.tycho.p2.target;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicReference;

Expand All @@ -40,12 +41,10 @@ public class TargetDefinitionResolverService {

private static final String CACHE_MISS_MESSAGE = "Target definition content cache miss: ";

private Map<ResolutionArguments, CompletableFuture<TargetDefinitionContent>> resolutionCache = new ConcurrentHashMap<>();
private ConcurrentMap<ResolutionArguments, CompletableFuture<TargetDefinitionContent>> resolutionCache = new ConcurrentHashMap<>();

private MavenContext mavenContext;

private IProvisioningAgent provisioningAgent;

private final AtomicReference<MavenDependenciesResolver> dependenciesResolver = new AtomicReference<>();

// constructor for DS
Expand All @@ -55,10 +54,8 @@ public TargetDefinitionResolverService() {
public TargetDefinitionContent getTargetDefinitionContent(TargetDefinition definition,
List<TargetEnvironment> environments, ExecutionEnvironmentResolutionHints jreIUs,
IncludeSourceMode includeSourceMode, IProvisioningAgent agent) {
this.provisioningAgent = agent;
ResolutionArguments arguments = new ResolutionArguments(definition, environments, jreIUs, includeSourceMode,
agent);

CompletableFuture<TargetDefinitionContent> future = resolutionCache.computeIfAbsent(arguments,
this::resolveFromArguments);

Expand All @@ -78,15 +75,15 @@ public TargetDefinitionContent getTargetDefinitionContent(TargetDefinition defin

// this method must only have the cache key as parameter (to make sure that the key is complete)
private CompletableFuture<TargetDefinitionContent> resolveFromArguments(ResolutionArguments arguments) {
mavenContext.getLogger().info("Resolving " + arguments + "...");
if (mavenContext.getLogger().isDebugEnabled()) {
debugCacheMiss(arguments);
mavenContext.getLogger().debug("Resolving target definition content...");
}

TargetDefinitionResolver resolver = new TargetDefinitionResolver(arguments.environments, arguments.jreIUs,
arguments.includeSourceMode, mavenContext, dependenciesResolver.get());
try {
return CompletableFuture.completedFuture(resolver.resolveContent(arguments.definition, provisioningAgent));
return CompletableFuture.completedFuture(resolver.resolveContent(arguments.definition, arguments.agent));
} catch (Exception e) {
return CompletableFuture.failedFuture(e);
}
Expand Down Expand Up @@ -185,6 +182,13 @@ public List<String> getNonEqualFields(ResolutionArguments other) {
return result;
}

@Override
public String toString() {
return "target definition " + definition.getOrigin() + " for environments=" + environments
+ ", include source mode=" + includeSourceMode + ", execution environment=" + jreIUs
+ ", remote p2 repository options=" + agent;
}

}

static <T> boolean eq(T left, T right) {
Expand Down
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2008, 2018 Sonatype Inc. and others.
* Copyright (c) 2008, 2022 Sonatype Inc. and others.
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
Expand All @@ -9,6 +9,7 @@
*
* Contributors:
* Sonatype Inc. - initial API and implementation
* Christoph Läubrich - add to string for easier debugging
*******************************************************************************/
package org.eclipse.tycho.p2.target.ee;

Expand Down Expand Up @@ -180,4 +181,13 @@ private static Stream<Version> parseEECapabilityVersion(ManifestElement eeCapabi
return Stream.empty();
}

@Override
public String toString() {
StringBuilder builder = new StringBuilder();
builder.append("StandardEEResolutionHints [executionEnvironment=");
builder.append(executionEnvironment);
builder.append("]");
return builder.toString();
}

}
@@ -1,12 +1,13 @@
/*******************************************************************************
* Copyright (c) 2010, 2012 SAP AG and others.
* Copyright (c) 2010, 2022 SAP AG and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*
* Contributors:
* SAP AG - initial API and implementation
* Christoph Läubrich - #496 - ResolutionArguments#hashcode is not stable
*******************************************************************************/
package org.eclipse.tycho.core.ee;

Expand Down Expand Up @@ -199,7 +200,7 @@ public String toString() {
}

@Override
public Collection<SystemPackageEntry> getSystemPackages() {
public synchronized Collection<SystemPackageEntry> getSystemPackages() {
if (systemPackages == null) {
// EE definitions in Tycho for JVMs 11+ will no longer contain system packages as with modular JVMs it's not sure
// all packages will be available at runtime
Expand Down Expand Up @@ -252,8 +253,7 @@ public int compareTo(StandardExecutionEnvironment otherEnv) {

@Override
public int hashCode() {
return Objects.hash(compilerSourceLevel, compilerTargetLevel, eeVersion, profileName, profileProperties,
systemPackages);
return Objects.hash(compilerSourceLevel, compilerTargetLevel, eeVersion, profileName, profileProperties);
}

@Override
Expand All @@ -272,8 +272,7 @@ public boolean equals(Object obj) {
&& Objects.equals(this.compilerTargetLevel, other.compilerTargetLevel)
&& Objects.equals(this.eeVersion, other.eeVersion)
&& Objects.equals(this.profileName, other.profileName)
&& Objects.equals(this.profileProperties, other.profileProperties)
&& Objects.equals(this.systemPackages, other.systemPackages);
&& Objects.equals(this.profileProperties, other.profileProperties);
}

@Override
Expand Down

0 comments on commit cf98ee4

Please sign in to comment.