Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMykola Nikishov2018-03-05 03:32:30 +0000
committerMykola Nikishov2018-03-05 05:40:17 +0000
commit29b908a23c3f6db08e3ec3903489053bcf8b5160 (patch)
tree1aca269511a235bc0aa13355326809ef75abb288
parent610afbb97141f55c5743d5b14535afbc109bed99 (diff)
downloadrt.equinox.p2-29b908a23c3f6db08e3ec3903489053bcf8b5160.tar.gz
rt.equinox.p2-29b908a23c3f6db08e3ec3903489053bcf8b5160.tar.xz
rt.equinox.p2-29b908a23c3f6db08e3ec3903489053bcf8b5160.zip
Bug 423715 - Really preserve legacy MD5 propertiesI20180305-0800I20180305-0300
Workflow to calculate checksums and store them in artifact's properties assumes usage of two methods from org.eclipse.equinox.internal.p2.artifact.processors.checksum.ChecksumUtilities: - calculateChecksums(File, Map<String, String>, Collection<String>) - checksumsToProperties(String, Map<String, String>) Two users of these methods, PublisherHelper and RecreateRepositoryApplication handled legacy MD5 properties in an inconsistent way. Even more, RecreateRepositoryApplication effectively ignored legacy MD5 property when recreating artifact descriptor. Move logic to handle legacy MD5 properties from PublisherHelper and RecreateRepositoryApplication to ChecksumUtilities' checksumsToProperties(String, Map<String, String>). p2 codebase has no tests for RecreateRepositoryApplication but PDE Build does, specifically P2Tests' testBug265564. This time PDE Build detected such regression [1], but it would be better to have some test harness for RecreateRepositoryApplication in p2 itself and not rely on downstream projects. [1] Bug 531931 Change-Id: Ic651df754691c25ee824bb444eff251f64f0757a Signed-off-by: Mykola Nikishov <mn@mn.com.ua>
-rw-r--r--bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumUtilities.java42
-rw-r--r--bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java4
-rw-r--r--bundles/org.eclipse.equinox.p2.publisher/src/org/eclipse/equinox/spi/p2/publisher/PublisherHelper.java17
-rw-r--r--bundles/org.eclipse.equinox.p2.repository.tools/src/org/eclipse/equinox/p2/internal/repository/tools/RecreateRepositoryApplication.java12
4 files changed, 34 insertions, 41 deletions
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumUtilities.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumUtilities.java
index ec6b1cefc..5b0c52d5c 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumUtilities.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/processors/checksum/ChecksumUtilities.java
@@ -28,7 +28,8 @@ import org.eclipse.osgi.util.NLS;
public class ChecksumUtilities {
private static final String ARTIFACT_CHECKSUMS_POINT = "org.eclipse.equinox.p2.artifact.repository.artifactChecksums"; //$NON-NLS-1$
- public static final String MD5 = "md5"; //$NON-NLS-1$
+ public static final String MD5_ID = "md5"; //$NON-NLS-1$
+ private static final String MD5_MESSAGE_DIGEST = "MD5"; //$NON-NLS-1$
/**
* Instances of checksum verifiers applicable for the artifact descriptor
@@ -91,22 +92,35 @@ public class ChecksumUtilities {
// don't calculate checksum if algo is disabled
continue;
String algorithm = checksumVerifierConfiguration.getAttribute("algorithm"); //$NON-NLS-1$
- try {
- String checksum = ChecksumProducer.produce(pathOnDisk, algorithm);
- checksums.put(id, checksum);
- String message = NLS.bind(Messages.calculateChecksum_ok, new Object[] {id, algorithm, checksum});
- status.add(new Status(IStatus.OK, Activator.ID, message));
- } catch (NoSuchAlgorithmException e) {
- String message = NLS.bind(Messages.calculateChecksum_error, id, algorithm);
- status.add(new Status(IStatus.ERROR, Activator.ID, message, e));
- } catch (IOException e) {
- String message = NLS.bind(Messages.calculateChecksum_error, id, algorithm);
- status.add(new Status(IStatus.ERROR, Activator.ID, message, e));
- }
+ Optional<String> checksum = calculateChecksum(pathOnDisk, status, id, algorithm);
+ checksum.ifPresent(c -> checksums.put(id, c));
+ }
+
+ boolean doNotSkipMd5 = !checksumsToSkip.contains(MD5_ID);
+ if (doNotSkipMd5) {
+ Optional<String> md5 = calculateChecksum(pathOnDisk, status, MD5_ID, MD5_MESSAGE_DIGEST);
+ md5.ifPresent(c -> checksums.put(MD5_ID, c));
}
+
return status;
}
+ private static Optional<String> calculateChecksum(File pathOnDisk, MultiStatus status, String id, String algorithm) {
+ try {
+ String checksum = ChecksumProducer.produce(pathOnDisk, algorithm);
+ String message = NLS.bind(Messages.calculateChecksum_ok, new Object[] {id, algorithm, checksum});
+ status.add(new Status(IStatus.OK, Activator.ID, message));
+ return Optional.of(checksum);
+ } catch (NoSuchAlgorithmException e) {
+ String message = NLS.bind(Messages.calculateChecksum_error, id, algorithm);
+ status.add(new Status(IStatus.ERROR, Activator.ID, message, e));
+ } catch (IOException e) {
+ String message = NLS.bind(Messages.calculateChecksum_error, id, algorithm);
+ status.add(new Status(IStatus.ERROR, Activator.ID, message, e));
+ }
+ return Optional.empty();
+ }
+
/**
* @param property either {@link IArtifactDescriptor#ARTIFACT_CHECKSUM} or {@link IArtifactDescriptor#DOWNLOAD_CHECKSUM}
* @param checksums
@@ -146,7 +160,7 @@ public class ChecksumUtilities {
}
private static void putLegacyMd5Property(String propertyNamespace, Map<String, String> checksums, HashMap<String, String> result) {
- String md5 = checksums.get(ChecksumUtilities.MD5);
+ String md5 = checksums.get(ChecksumUtilities.MD5_ID);
if (md5 != null) {
if (IArtifactDescriptor.ARTIFACT_CHECKSUM.equals(propertyNamespace))
result.put(IArtifactDescriptor.ARTIFACT_MD5, md5);
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java
index efdfca82b..b268825c8 100644
--- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java
+++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/simple/SimpleArtifactRepository.java
@@ -472,7 +472,7 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
ArrayList<ProcessingStep> steps = new ArrayList<>();
steps.add(new SignatureVerifier());
- Set<String> skipChecksums = ARTIFACT_MD5_CHECKSUM_ENABLED ? Collections.emptySet() : Collections.singleton(ChecksumUtilities.MD5);
+ Set<String> skipChecksums = ARTIFACT_MD5_CHECKSUM_ENABLED ? Collections.emptySet() : Collections.singleton(ChecksumUtilities.MD5_ID);
addChecksumVerifiers(descriptor, steps, skipChecksums, IArtifactDescriptor.ARTIFACT_CHECKSUM);
if (steps.isEmpty())
@@ -487,7 +487,7 @@ public class SimpleArtifactRepository extends AbstractArtifactRepository impleme
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(ChecksumUtilities.MD5);
+ Set<String> skipChecksums = DOWNLOAD_MD5_CHECKSUM_ENABLED ? Collections.emptySet() : Collections.singleton(ChecksumUtilities.MD5_ID);
addChecksumVerifiers(descriptor, steps, skipChecksums, IArtifactDescriptor.DOWNLOAD_CHECKSUM);
// Add steps here if needed
diff --git a/bundles/org.eclipse.equinox.p2.publisher/src/org/eclipse/equinox/spi/p2/publisher/PublisherHelper.java b/bundles/org.eclipse.equinox.p2.publisher/src/org/eclipse/equinox/spi/p2/publisher/PublisherHelper.java
index 5fa4cadb7..9f8c66a98 100644
--- a/bundles/org.eclipse.equinox.p2.publisher/src/org/eclipse/equinox/spi/p2/publisher/PublisherHelper.java
+++ b/bundles/org.eclipse.equinox.p2.publisher/src/org/eclipse/equinox/spi/p2/publisher/PublisherHelper.java
@@ -15,16 +15,12 @@
package org.eclipse.equinox.spi.p2.publisher;
import java.io.File;
-import java.io.IOException;
import java.util.*;
import org.eclipse.core.runtime.IStatus;
-import org.eclipse.core.runtime.Status;
import org.eclipse.equinox.internal.p2.artifact.processors.checksum.ChecksumUtilities;
import org.eclipse.equinox.internal.p2.core.helpers.LogHelper;
import org.eclipse.equinox.internal.p2.metadata.ArtifactKey;
import org.eclipse.equinox.internal.p2.metadata.BasicVersion;
-import org.eclipse.equinox.internal.p2.publisher.Activator;
-import org.eclipse.equinox.internal.p2.repository.helpers.ChecksumProducer;
import org.eclipse.equinox.p2.metadata.*;
import org.eclipse.equinox.p2.metadata.MetadataFactory.InstallableUnitDescription;
import org.eclipse.equinox.p2.metadata.MetadataFactory.InstallableUnitFragmentDescription;
@@ -115,7 +111,6 @@ public class PublisherHelper {
boolean generateChecksums = info == null || (info.getArtifactOptions() & IPublisherInfo.A_NO_MD5) == 0;
if (generateChecksums) {
- calculateLegacyMd5(pathOnDisk, descriptor);
calculateChecksums(pathOnDisk, descriptor);
}
}
@@ -123,18 +118,6 @@ public class PublisherHelper {
return result;
}
- private static void calculateLegacyMd5(File pathOnDisk, ArtifactDescriptor descriptor) {
- try {
- String md5 = ChecksumProducer.computeMD5(pathOnDisk);
- if (md5 != null)
- descriptor.setProperty(IArtifactDescriptor.DOWNLOAD_MD5, md5);
- } catch (IOException e) {
- // don't care if failed to compute checksum
- // TODO provide message?
- LogHelper.log(new Status(IStatus.WARNING, Activator.ID, "", e)); //$NON-NLS-1$
- }
- }
-
private static void calculateChecksums(File pathOnDisk, ArtifactDescriptor descriptor) {
// TODO disable specific algorithms
List<String> checksumsToSkip = Collections.emptyList();
diff --git a/bundles/org.eclipse.equinox.p2.repository.tools/src/org/eclipse/equinox/p2/internal/repository/tools/RecreateRepositoryApplication.java b/bundles/org.eclipse.equinox.p2.repository.tools/src/org/eclipse/equinox/p2/internal/repository/tools/RecreateRepositoryApplication.java
index a56fdb73d..b67c9a3b6 100644
--- a/bundles/org.eclipse.equinox.p2.repository.tools/src/org/eclipse/equinox/p2/internal/repository/tools/RecreateRepositoryApplication.java
+++ b/bundles/org.eclipse.equinox.p2.repository.tools/src/org/eclipse/equinox/p2/internal/repository/tools/RecreateRepositoryApplication.java
@@ -31,7 +31,6 @@ import org.eclipse.equinox.p2.repository.artifact.spi.ProcessingStepDescriptor;
import org.eclipse.osgi.util.NLS;
public class RecreateRepositoryApplication extends AbstractApplication {
- private static final String MD5_CHECKSUM_ID = "md5"; //$NON-NLS-1$
static final private String PUBLISH_PACK_FILES_AS_SIBLINGS = "publishPackFilesAsSiblings"; //$NON-NLS-1$
private URI repoLocation;
private String repoName = null;
@@ -123,17 +122,14 @@ public class RecreateRepositoryApplication extends AbstractApplication {
newDescriptor.setProperty(IArtifactDescriptor.DOWNLOAD_SIZE, size);
Map<String, String> checksums = new HashMap<>();
- IStatus status = ChecksumUtilities.calculateChecksums(artifactFile, checksums, Collections.emptyList());
+ List<String> checksumsToSkip = Collections.emptyList();
+ IStatus status = ChecksumUtilities.calculateChecksums(artifactFile, checksums, checksumsToSkip);
if (!status.isOK())
// TODO handle errors in some way
LogHelper.log(status);
- String md5 = checksums.get(MD5_CHECKSUM_ID);
- if (md5 != null)
- // preserve legacy MD5 checksum location
- newDescriptor.setProperty(IArtifactDescriptor.DOWNLOAD_MD5, md5);
-
- newDescriptor.addProperties(ChecksumUtilities.checksumsToProperties(IArtifactDescriptor.DOWNLOAD_CHECKSUM, checksums));
+ Map<String, String> checksumsToProperties = ChecksumUtilities.checksumsToProperties(IArtifactDescriptor.DOWNLOAD_CHECKSUM, checksums);
+ newDescriptor.addProperties(checksumsToProperties);
File temp = new File(artifactFile.getParentFile(), artifactFile.getName() + ".pack.gz"); //$NON-NLS-1$
if (temp.exists()) {

Back to the top