Support properties supplied by an agent

Currently P2 can be controlled by a set of properties that mostly
delegate to the Bundle Properties (what effectively is backed by system
properties). In certain situations it would be more suitable that the
IProvisioningAgent can control these (e.g. max threads, mirrors used or
not, checksums enabled, ...).

This adds new (default implemented) methods to IProvisioningAgent to
query it for a property where the default simply uses the Bundle
properties if available or system properties in all other cases.

bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/RawMirrorRequest.java

@@ -89,7 +89,7 @@ public IArtifactDescriptor getArtifactDescriptor() {
 	@Override
 	protected IStatus getArtifact(IArtifactDescriptor artifactDescriptor, OutputStream destination, IProgressMonitor monitor) {
 		SubMonitor subMon = SubMonitor.convert(monitor, 2);
-		if (SimpleArtifactRepository.CHECKSUMS_ENABLED) {
+		if (SimpleArtifactRepository.isChecksumsEnabled(target.getProvisioningAgent())) {
 			Collection<ChecksumVerifier> steps = ChecksumUtilities.getChecksumVerifiers(artifactDescriptor,
 					IArtifactDescriptor.DOWNLOAD_CHECKSUM, Collections.emptySet());
 			if (steps.isEmpty()) {

bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java

@@ -54,36 +54,21 @@
 import org.eclipse.equinox.p2.repository.artifact.spi.*;
 import org.eclipse.osgi.service.datalocation.Location;
 import org.eclipse.osgi.util.NLS;
+import org.osgi.framework.BundleContext;
 
 public class SimpleArtifactRepository extends AbstractArtifactRepository implements IFileArtifactRepository, IIndexProvider<IArtifactKey> {
-	/**
-	 * A boolean property controlling whether mirroring is enabled.
-	 */
-	public static final boolean MIRRORS_ENABLED = !"false".equals(Activator.getContext().getProperty("eclipse.p2.mirrors")); //$NON-NLS-1$//$NON-NLS-2$
 
-	/**
-	 * A boolean property controlling whether any checksums of the artifact should be checked.
-	 * @see IArtifactDescriptor#DOWNLOAD_MD5
-	 * @see IArtifactDescriptor#DOWNLOAD_CHECKSUM
-	 * @see IArtifactDescriptor#ARTIFACT_MD5
-	 * @see IArtifactDescriptor#ARTIFACT_CHECKSUM
-	 */
-	public static final boolean CHECKSUMS_ENABLED = !"true".equals(Activator.getContext().getProperty("eclipse.p2.checksums.disable")); //$NON-NLS-1$//$NON-NLS-2$
+	public static final String PROPERTY_ECLIPSE_P2_MD5_ARTIFACT_CHECK = "eclipse.p2.MD5ArtifactCheck"; //$NON-NLS-1$
 
-	/**
-	 * A boolean property controlling whether MD5 checksum of the artifact bytes that are transferred should be checked.
-	 * @see IArtifactDescriptor#DOWNLOAD_MD5
-	 * @see IArtifactDescriptor#DOWNLOAD_CHECKSUM
-	 */
-	public static final boolean DOWNLOAD_MD5_CHECKSUM_ENABLED = !"false".equals(Activator.getContext().getProperty("eclipse.p2.MD5Check")); //$NON-NLS-1$//$NON-NLS-2$
+	public static final String PROPERTY_ECLIPSE_P2_MD5_CHECK = "eclipse.p2.MD5Check"; //$NON-NLS-1$
 
-	/**
-	 * A boolean property controlling whether MD5 checksum of the artifact bytes in its native format (after processing steps have
-	 * been applied) should be checked.
-	 * @see IArtifactDescriptor#ARTIFACT_MD5
-	 * @see IArtifactDescriptor#ARTIFACT_CHECKSUM
-	 */
-	public static final boolean ARTIFACT_MD5_CHECKSUM_ENABLED = !"false".equals(Activator.getContext().getProperty("eclipse.p2.MD5ArtifactCheck")); //$NON-NLS-1$//$NON-NLS-2$
+	public static final String PROPERTY_ECLIPSE_P2_CHECKSUMS_DISABLE = "eclipse.p2.checksums.disable"; //$NON-NLS-1$
+
+	public static final String PROPERTY_ECLIPSE_P2_MIRRORS = "eclipse.p2.mirrors"; //$NON-NLS-1$
+
+	private static final String FALSE = "false"; //$NON-NLS-1$
+
+	private static final String TRUE = "true"; //$NON-NLS-1$
 
 	public static final String CONTENT_FILENAME = "artifacts"; //$NON-NLS-1$
 
@@ -470,8 +455,8 @@ private synchronized OutputStream addPostSteps(ProcessingStepHandler handler, IA
 		monitor = IProgressMonitor.nullSafe(monitor);
 		ArrayList<ProcessingStep> steps = new ArrayList<>();
 		steps.add(new SignatureVerifier());
-
-		Set<String> skipChecksums = ARTIFACT_MD5_CHECKSUM_ENABLED ? Collections.emptySet() : Collections.singleton(ChecksumHelper.MD5);
+		Set<String> skipChecksums = isArtifactMd5ChecksumEnabled(getProvisioningAgent()) ? Collections.emptySet()
+				: Collections.singleton(ChecksumHelper.MD5);
 		addChecksumVerifiers(descriptor, steps, skipChecksums, IArtifactDescriptor.ARTIFACT_CHECKSUM);
 
 		if (!isFolderBased(descriptor)) {
@@ -500,7 +485,8 @@ private OutputStream addPreSteps(ProcessingStepHandler handler, IArtifactDescrip
 		if (IArtifactDescriptor.TYPE_ZIP.equals(descriptor.getProperty(IArtifactDescriptor.DOWNLOAD_CONTENTTYPE)))
 			steps.add(new ZipVerifierStep());
 
-		Set<String> skipChecksums = DOWNLOAD_MD5_CHECKSUM_ENABLED ? Collections.emptySet() : Collections.singleton(ChecksumHelper.MD5);
+		Set<String> skipChecksums = isDownloadMd5ChecksumEnabled(getProvisioningAgent()) ? Collections.emptySet()
+				: Collections.singleton(ChecksumHelper.MD5);
 		ArrayList<ProcessingStep> downloadChecksumSteps = new ArrayList<>();
 		addChecksumVerifiers(descriptor, downloadChecksumSteps, skipChecksums, IArtifactDescriptor.DOWNLOAD_CHECKSUM);
 		if (downloadChecksumSteps.isEmpty() && !isLocal()) {
@@ -518,7 +504,7 @@ private OutputStream addPreSteps(ProcessingStepHandler handler, IArtifactDescrip
 	}
 
 	private void addChecksumVerifiers(IArtifactDescriptor descriptor, ArrayList<ProcessingStep> steps, Set<String> skipChecksums, String property) {
-		if (CHECKSUMS_ENABLED) {
+		if (isChecksumsEnabled(getProvisioningAgent())) {
 			Collection<ChecksumVerifier> checksumVerifiers = ChecksumUtilities.getChecksumVerifiers(descriptor,
 					property, skipChecksums);
 			steps.addAll(checksumVerifiers);
@@ -758,13 +744,75 @@ private IStatus downloadArtifact(IArtifactDescriptor descriptor, URI mirrorLocat
 	 */
 	private synchronized URI getMirror(URI baseLocation, IProgressMonitor monitor) {
 		monitor = IProgressMonitor.nullSafe(monitor);
-		if (!MIRRORS_ENABLED || (!isForceThreading() && isLocal()))
+		if (!isMirrorsEnabled(getProvisioningAgent()) || (!isForceThreading() && isLocal()))
 			return baseLocation;
 		if (mirrors == null)
 			mirrors = new MirrorSelector(this, getTransport());
 		return mirrors.getMirrorLocation(baseLocation, monitor);
 	}
 
+	/**
+	 * A boolean property controlling whether mirroring is enabled.
+	 *
+	 * @param agent the agent to use for determine properties, might be
+	 *              <code>null</code>
+	 */
+	public static boolean isMirrorsEnabled(IProvisioningAgent agent) {
+		return !FALSE.equals(getAgentPropertyWithFallback(agent, PROPERTY_ECLIPSE_P2_MIRRORS));
+	}
+
+	/**
+	 * A boolean property controlling whether any checksums of the artifact should
+	 * be checked.
+	 *
+	 * @param agent the agent to use for determine properties, might be
+	 *              <code>null</code>
+	 *
+	 * @see IArtifactDescriptor#DOWNLOAD_MD5
+	 * @see IArtifactDescriptor#DOWNLOAD_CHECKSUM
+	 * @see IArtifactDescriptor#ARTIFACT_MD5
+	 * @see IArtifactDescriptor#ARTIFACT_CHECKSUM
+	 */
+	public static boolean isChecksumsEnabled(IProvisioningAgent agent) {
+		return !FALSE.equals(getAgentPropertyWithFallback(agent, PROPERTY_ECLIPSE_P2_CHECKSUMS_DISABLE));
+	}
+
+	/**
+	 * A boolean property controlling whether MD5 checksum of the artifact bytes
+	 * that are transferred should be checked.
+	 *
+	 * @see IArtifactDescriptor#DOWNLOAD_MD5
+	 * @see IArtifactDescriptor#DOWNLOAD_CHECKSUM
+	 * @param agent the agent to use for determine properties, might be
+	 *              <code>null</code>
+	 */
+	public static boolean isDownloadMd5ChecksumEnabled(IProvisioningAgent agent) {
+		return !FALSE.equals(getAgentPropertyWithFallback(agent, PROPERTY_ECLIPSE_P2_MD5_CHECK));
+	}
+
+	/**
+	 * A boolean property controlling whether MD5 checksum of the artifact bytes in
+	 * its native format (after processing steps have been applied) should be
+	 * checked.
+	 *
+	 * @see IArtifactDescriptor#ARTIFACT_MD5
+	 * @see IArtifactDescriptor#ARTIFACT_CHECKSUM
+	 */
+	public static boolean isArtifactMd5ChecksumEnabled(IProvisioningAgent agent) {
+		return !FALSE.equals(getAgentPropertyWithFallback(agent, PROPERTY_ECLIPSE_P2_MD5_ARTIFACT_CHECK));
+	}
+
+	private static String getAgentPropertyWithFallback(IProvisioningAgent agent, String key) {
+		if (agent == null) {
+			BundleContext context = Activator.getContext();
+			if (context != null) {
+				return context.getProperty(key);
+			}
+			return System.getProperty(key);
+		}
+		return agent.getProperty(key);
+	}
+
 	@Override
 	public <T> T getAdapter(Class<T> adapter) {
 		// if we are adapting to file or writable repositories then make sure we have a file location
@@ -961,7 +1009,7 @@ public synchronized URI getLocation(IArtifactDescriptor descriptor) {
 	private int getMaximumThreads() {
 		int maxThreads = DEFAULT_MAX_THREADS;
 		try {
-			String maxThreadString = Activator.getContext().getProperty(PROP_MAX_THREADS);
+			String maxThreadString = getAgentPropertyWithFallback(getProvisioningAgent(), PROP_MAX_THREADS);
 			if (maxThreadString != null)
 				maxThreads = Math.max(1, Integer.parseInt(maxThreadString));
 		} catch (NumberFormatException nfe) {
@@ -1114,7 +1162,11 @@ private boolean isFolderBased(IArtifactDescriptor descriptor) {
 	}
 
 	private boolean isForceThreading() {
-		return "true".equals(getProperties().get(PROP_FORCE_THREADING)); //$NON-NLS-1$
+		String property = getAgentPropertyWithFallback(getProvisioningAgent(), PROP_FORCE_THREADING);
+		if (property != null) {
+			return TRUE.equals(property);
+		}
+		return TRUE.equals(getProperties().get(PROP_FORCE_THREADING));
 	}
 
 	private boolean isLocal() {
@@ -1298,7 +1350,7 @@ private IStatus reportStatus(IArtifactDescriptor descriptor, OutputStream destin
 	public void save() {
 		if (disableSave)
 			return;
-		boolean compress = "true".equalsIgnoreCase(getProperty(PROP_COMPRESSED)); //$NON-NLS-1$
+		boolean compress = TRUE.equalsIgnoreCase(getProperty(PROP_COMPRESSED));
 		save(compress);
 	}
 

bundles/org.eclipse.equinox.p2.core/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.equinox.p2.core;singleton:=true
-Bundle-Version: 2.10.300.qualifier
+Bundle-Version: 2.11.0.qualifier
 Bundle-Activator: org.eclipse.equinox.internal.p2.core.Activator
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin

bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/Activator.java

@@ -95,14 +95,22 @@ private static URI buildURL(String spec, boolean trailingSlash) {
 	}
 
 	private static String substituteVar(String source, String var, String prop) {
-		String value = Activator.context.getProperty(prop);
+		String value = getProperty(prop);
 		if (value == null)
 			value = ""; //$NON-NLS-1$
 		return value + source.substring(var.length());
 	}
 
+	public static String getProperty(String prop) {
+		BundleContext bundleContext = Activator.context;
+		if (bundleContext != null) {
+			return bundleContext.getProperty(prop);
+		}
+		return System.getProperty(prop);
+	}
+
 	private IAgentLocation buildLocation(String property, URI defaultLocation, boolean readOnlyDefault, boolean addTrailingSlash) {
-		String location = Activator.context.getProperty(property);
+		String location = getProperty(property);
 		// if the instance location is not set, predict where the workspace will be and
 		// put the instance area inside the workspace meta area.
 		if (location == null)

bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/p2/core/IProvisioningAgent.java

@@ -14,6 +14,7 @@
  *******************************************************************************/
 package org.eclipse.equinox.p2.core;
 
+import org.eclipse.equinox.internal.p2.core.Activator;
 import org.eclipse.equinox.p2.core.spi.IAgentServiceFactory;
 
 /**
@@ -38,22 +39,22 @@ public interface IProvisioningAgent {
 	 * obtain an agent for a different system the {@link IProvisioningAgentProvider}
 	 * service must be used.
 	 */
-	public static final String SERVICE_NAME = IProvisioningAgent.class.getName();
+	String SERVICE_NAME = IProvisioningAgent.class.getName();
 
-	public static final String INSTALLER_AGENT = "org.eclipse.equinox.p2.installer.agent"; //$NON-NLS-1$
-	public static final String INSTALLER_PROFILEID = "org.eclipse.equinox.p2.installer.profile.id"; //$NON-NLS-1$
+	String INSTALLER_AGENT = "org.eclipse.equinox.p2.installer.agent"; //$NON-NLS-1$
+	String INSTALLER_PROFILEID = "org.eclipse.equinox.p2.installer.profile.id"; //$NON-NLS-1$
 
 	/**
 	 * When running in "shared mode", this allows to retrieve from the IProvisioningAgent the agent representing what is in the shared location aka the base
 	 * @since 2.3
 	 */
-	public static final String SHARED_BASE_AGENT = "org.eclipse.equinox.shared.base.agent"; //$NON-NLS-1$
+	String SHARED_BASE_AGENT = "org.eclipse.equinox.shared.base.agent"; //$NON-NLS-1$
 
 	/**
 	 * When running in "shared mode", this allows to retrieve from the IProvisioningAgent identified by {@link #SHARED_BASE_AGENT} the current agent
 	 * @since 2.3
 	 */
-	public static final String SHARED_CURRENT_AGENT = "org.eclipse.equinox.shared.current.agent"; //$NON-NLS-1$
+	String SHARED_CURRENT_AGENT = "org.eclipse.equinox.shared.current.agent"; //$NON-NLS-1$
 	/**
 	 * Service property identifying whether an agent is the default agent.
 	 *
@@ -65,14 +66,14 @@ public interface IProvisioningAgent {
 	 * has any other value, then the service is not the agent for the currently running system.
 	 * </p>
 	 */
-	public static final String SERVICE_CURRENT = "agent.current"; //$NON-NLS-1$
+	String SERVICE_CURRENT = "agent.current"; //$NON-NLS-1$
 
 	/**
 	 * Returns the service with the given service name, or <code>null</code>
 	 * if no such service is available in this agent.
 	 * @exception IllegalStateException if this agent has been stopped
 	 */
-	public Object getService(String serviceName);
+	Object getService(String serviceName);
 
 	/**
 	 * Returns the service with the given service name, or <code>null</code>
@@ -82,7 +83,7 @@ public interface IProvisioningAgent {
 	 * @since 2.6
 	 */
 	@SuppressWarnings("unchecked")
-	public default <T> T getService(Class<T> key) {
+	default <T> T getService(Class<T> key) {
 		return (T) getService(key.getName());
 	}
 
@@ -93,7 +94,7 @@ public default <T> T getService(Class<T> key) {
 	 * @param service The service implementation
 	 * @exception IllegalStateException if this agent has been stopped
 	 */
-	public void registerService(String serviceName, Object service);
+	void registerService(String serviceName, Object service);
 
 	/**
 	 * Stops the provisioning agent. This causes services provided by this
@@ -105,7 +106,7 @@ public default <T> T getService(Class<T> key) {
 	 * by invoking {@link IProvisioningAgentProvider#createAgent(java.net.URI)}.
 	 * </p>
 	 */
-	public void stop();
+	void stop();
 
 	/**
 	 * Unregisters a service that has previously been registered with this
@@ -115,6 +116,30 @@ public default <T> T getService(Class<T> key) {
 	 * @param serviceName The name of the service to unregister
 	 * @param service The service implementation to unregister.
 	 */
-	public void unregisterService(String serviceName, Object service);
+	void unregisterService(String serviceName, Object service);
+
+	/**
+	 * Returns an agent bound property, the default implementation delegates to the
+	 * bundle properties if running and to the system properties otherwise.
+	 *
+	 * @since 2.11
+	 */
+	default String getProperty(String key) {
+		return Activator.getProperty(key);
+	}
+
+	/**
+	 * Returns an agent bound property, the default implementation delegates to the
+	 * bundle properties if running and to the system properties otherwise.
+	 *
+	 * @since 2.11
+	 */
+	default String getProperty(String key, String defaultValue) {
+		String property = getProperty(key);
+		if (property == null) {
+			return defaultValue;
+		}
+		return property;
+	}
 
 }