Fix #658 - preserve p2 artifact properties (eg PGP, maven info...)

Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>

tycho-bundles/org.eclipse.tycho.embedder.shared/src/main/java/org/eclipse/tycho/TychoConstants.java

@@ -1,12 +1,13 @@
 /*******************************************************************************
- * Copyright (c) 2008, 2021 Sonatype Inc. and others.
+ * Copyright (c) 2008, 2022 Sonatype Inc. 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:
  *    Sonatype Inc. - initial API and implementation
+ *    Christoph Läubrich - Issue #658 - Tycho strips p2 artifact properties (eg PGP, maven info...)
  *******************************************************************************/
 package org.eclipse.tycho;
 
@@ -49,6 +50,10 @@ public interface TychoConstants {
 
     String PROP_REPOSITORY = "maven-repository";
 
+    String PROP_PGP_KEYS = "pgp.publicKeys";
+
+    String PROP_PGP_SIGNATURES = "pgp.signatures";
+
     /**
      * @deprecated this is deprecated but can't be removed as we otherwise loose compatibility for
      *             older repository format, this should never be used in new code and usage should

tycho-bundles/org.eclipse.tycho.p2.maven.repository.tests/src/test/java/org/eclipse/tycho/repository/local/LocalArtifactRepositoryP2APITest.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2013 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
@@ -9,6 +9,7 @@
  *
  * Contributors:
  *    Tobias Oberlies (SAP SE) - initial API and implementation
+ *    Christoph Läubrich - API adjust
  *******************************************************************************/
 package org.eclipse.tycho.repository.local;
 
@@ -508,7 +509,7 @@ public void testGetRawArtifactOfNonContainedFormatToStream() {
 
     @Test
     public void testWriteArtifact() throws Exception {
-        IArtifactSink addSink = subject.newAddingArtifactSink(NEW_KEY);
+        IArtifactSink addSink = subject.newAddingArtifactSink(new ArtifactDescriptor(NEW_KEY));
         addSink.beginWrite().write(new byte[33]);
         addSink.commitWrite();
 
@@ -522,7 +523,7 @@ public void testReWriteArtifactFails() throws Exception {
         // LocalArtifactRepository doesn't allow overwrites -> this may be different in other IArtifactRepository implementations
         ProvisionException expectedException = null;
         try {
-            IArtifactSink addSink = subject.newAddingArtifactSink(ARTIFACT_A_KEY);
+            IArtifactSink addSink = subject.newAddingArtifactSink(new ArtifactDescriptor(ARTIFACT_A_KEY));
             addSink.beginWrite();
             addSink.commitWrite();
         } catch (ProvisionException e) {
@@ -535,7 +536,7 @@ public void testReWriteArtifactFails() throws Exception {
 
     @Test
     public void testWriteArtifactAndCancel() throws Exception {
-        IArtifactSink addSink = subject.newAddingArtifactSink(NEW_KEY);
+        IArtifactSink addSink = subject.newAddingArtifactSink(new ArtifactDescriptor(NEW_KEY));
         addSink.beginWrite().write(new byte[33]);
         addSink.abortWrite();
 
@@ -545,7 +546,7 @@ public void testWriteArtifactAndCancel() throws Exception {
 
     @Test
     public void testWriteArtifactOnSecondAttempt() throws Exception {
-        IArtifactSink addSink = subject.newAddingArtifactSink(NEW_KEY);
+        IArtifactSink addSink = subject.newAddingArtifactSink(new ArtifactDescriptor(NEW_KEY));
         addSink.beginWrite().write(new byte[11]);
         addSink.beginWrite().write(new byte[22]);
         addSink.commitWrite();

tycho-bundles/org.eclipse.tycho.p2.maven.repository.tests/src/test/java/org/eclipse/tycho/repository/local/MirroringArtifactProviderErrorTest.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2013 SAP SE and others.
+ * Copyright (c) 2013, 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
@@ -20,6 +20,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 
 import org.eclipse.equinox.p2.metadata.IArtifactKey;
+import org.eclipse.tycho.core.shared.MockMavenContext;
 import org.eclipse.tycho.p2.maven.repository.tests.TestRepositoryContent;
 import org.eclipse.tycho.repository.local.MirroringArtifactProvider.MirroringFailedException;
 import org.eclipse.tycho.repository.local.testutil.TemporaryLocalMavenRepository;
@@ -57,7 +58,7 @@ public void before() throws Exception {
         subject = MirroringArtifactProvider.createInstance(localRepository,
                 new RepositoryArtifactProvider(singletonList(TestRepositoryContent.REPO_BUNLDE_AB_PACK_CORRUPT),
                         ArtifactTransferPolicies.forRemoteArtifacts(), p2Context.getAgent()),
-                false, logVerifier.getLogger());
+                false, new MockMavenContext(null, logVerifier.getLogger()));
     }
 
     @Test(expected = MirroringFailedException.class)

tycho-bundles/org.eclipse.tycho.p2.maven.repository.tests/src/test/java/org/eclipse/tycho/repository/local/MirroringArtifactProviderPack200CornerCasesTest.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2013 SAP SE and others.
+ * Copyright (c) 2013, 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
@@ -9,6 +9,7 @@
  *
  * Contributors:
  *    SAP SE - initial API and implementation
+ *    Christoph Läubrich - API adjust
  *******************************************************************************/
 package org.eclipse.tycho.repository.local;
 
@@ -27,6 +28,8 @@
 import org.eclipse.equinox.p2.core.ProvisionException;
 import org.eclipse.equinox.p2.metadata.IArtifactKey;
 import org.eclipse.equinox.p2.repository.artifact.IArtifactDescriptor;
+import org.eclipse.equinox.p2.repository.artifact.spi.ArtifactDescriptor;
+import org.eclipse.tycho.core.shared.MockMavenContext;
 import org.eclipse.tycho.p2.maven.repository.tests.TestRepositoryContent;
 import org.eclipse.tycho.repository.local.MirroringArtifactProvider.MirroringFailedException;
 import org.eclipse.tycho.repository.local.testutil.TemporaryLocalMavenRepository;
@@ -73,7 +76,8 @@ public void expectNoWarningsInLog() throws Exception {
     @Test
     public void testIgnoreIfPackedArtifactNotAvailableRemotely() throws Exception {
         subject = MirroringArtifactProvider.createInstance(localRepository,
-                providerFor(TestRepositoryContent.REPO_BUNDLE_A), true, logVerifier.getLogger());
+                providerFor(TestRepositoryContent.REPO_BUNDLE_A), true,
+                new MockMavenContext(null, logVerifier.getLogger()));
 
         IArtifactDescriptor[] mirroredDescriptors = subject.getArtifactDescriptors(BUNDLE_A_KEY);
 
@@ -84,7 +88,8 @@ public void testIgnoreIfPackedArtifactNotAvailableRemotely() throws Exception {
     @Test
     public void testErrorIfPackedArtifactIsAvailableButCorrupt() throws Exception {
         subject = MirroringArtifactProvider.createInstance(localRepository,
-                providerFor(TestRepositoryContent.REPO_BUNLDE_AB_PACK_CORRUPT), true, logVerifier.getLogger());
+                providerFor(TestRepositoryContent.REPO_BUNLDE_AB_PACK_CORRUPT), true,
+                new MockMavenContext(null, logVerifier.getLogger()));
 
         logVerifier.expectError(containsString(BUNDLE_A_KEY.toString()));
 
@@ -102,7 +107,7 @@ public void testPackedArtifactMirroredEvenIfCanonicalArtifactPresent() throws Ex
         assertThat(isCanonicalFormat(localRepository.getArtifactDescriptors(BUNDLE_A_KEY)[0]), is(true)); // self-test
 
         subject = MirroringArtifactProvider.createInstance(localRepository, remoteProvider, true,
-                logVerifier.getLogger());
+                new MockMavenContext(null, logVerifier.getLogger()));
 
         assertThat(subject.getArtifactDescriptors(BUNDLE_A_KEY).length, is(2));
         assertThat(localRepository.getArtifactDescriptors(BUNDLE_A_KEY).length, is(2));
@@ -128,7 +133,7 @@ public void testCanonicalArtifactCreatedIfPackedArtifactAlreadyMirrored(Boolean
 
         // also expect this for the non-pack200 mirroring implementation (so that it doesn't fail if a different build left the local Maven repository in this state)
         subject = MirroringArtifactProvider.createInstance(localRepository, emptyProvider, mirrorPacked,
-                logVerifier.getLogger());
+                new MockMavenContext(null, logVerifier.getLogger()));
 
         assertThat(subject.getArtifactDescriptors(BUNDLE_A_KEY).length, is(2));
         assertThat(localRepository.getArtifactDescriptors(BUNDLE_A_KEY).length, is(2));
@@ -145,7 +150,7 @@ private static void prefillLocalRepositoryWithPackedArtifact(LocalArtifactReposi
 
     private static void prefillLocalRepositoryWithCanonicalArtifact(LocalArtifactRepository localRepository,
             IRawArtifactProvider provider, IArtifactKey artifactKey) throws Exception {
-        provider.getArtifact(localRepository.newAddingArtifactSink(artifactKey), null);
+        provider.getArtifact(localRepository.newAddingArtifactSink(new ArtifactDescriptor(artifactKey)), null);
     }
 
 }

tycho-bundles/org.eclipse.tycho.p2.maven.repository.tests/src/test/java/org/eclipse/tycho/repository/local/MirroringArtifactProviderTest.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2013 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
@@ -42,6 +42,7 @@
 import org.eclipse.equinox.p2.metadata.Version;
 import org.eclipse.equinox.p2.query.IQueryResult;
 import org.eclipse.equinox.p2.repository.artifact.IArtifactDescriptor;
+import org.eclipse.tycho.core.shared.MockMavenContext;
 import org.eclipse.tycho.p2.maven.repository.tests.ResourceUtil;
 import org.eclipse.tycho.p2.maven.repository.tests.TestRepositoryContent;
 import org.eclipse.tycho.repository.local.testutil.TemporaryLocalMavenRepository;
@@ -77,7 +78,9 @@ public class MirroringArtifactProviderTest {
     private static final IArtifactKey BUNDLE_L_KEY = new ArtifactKey("osgi.bundle", "org.eclipse.core.jobs",
             Version.parseVersion("3.4.1.R34x_v20081128"));
     private static final Set<String> BUNDLE_L_CONTENT_FILES = new HashSet<>(
-            Arrays.asList("META-INF/", "META-INF/MANIFEST.MF", "org/", "org/eclipse/", "org/eclipse/core/", "org/eclipse/core/internal/", "org/eclipse/core/internal/jobs/", "org/eclipse/core/runtime/", "org/eclipse/core/runtime/jobs/", "plugin.properties"));
+            Arrays.asList("META-INF/", "META-INF/MANIFEST.MF", "org/", "org/eclipse/", "org/eclipse/core/",
+                    "org/eclipse/core/internal/", "org/eclipse/core/internal/jobs/", "org/eclipse/core/runtime/",
+                    "org/eclipse/core/runtime/jobs/", "plugin.properties"));
 
     // not available bundle
     private static final IArtifactKey OTHER_KEY = TestRepositoryContent.NOT_CONTAINED_ARTIFACT_KEY;
@@ -122,7 +125,7 @@ public void initSubject() throws Exception {
         localRepository = localRepositoryManager.getLocalArtifactRepository();
 
         subject = MirroringArtifactProvider.createInstance(localRepository, remoteProvider, mirrorPacked,
-                logVerifier.getLogger());
+                new MockMavenContext(null, logVerifier.getLogger()));
     }
 
     @Before

tycho-bundles/org.eclipse.tycho.p2.maven.repository/src/main/java/org/eclipse/tycho/repository/local/MirroringArtifactProvider.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2021 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
@@ -9,12 +9,15 @@
  *
  * Contributors:
  *    Tobias Oberlies (SAP SE) - initial API and implementation
+ *    Christoph Läubrich - Issue #658 - Tycho strips p2 artifact properties (eg PGP, maven info...)
  *******************************************************************************/
 package org.eclipse.tycho.repository.local;
 
 import static org.eclipse.tycho.repository.util.internal.BundleConstants.BUNDLE_ID;
 
 import java.io.File;
+import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.locks.Lock;
 
 import org.eclipse.core.runtime.IProgressMonitor;
@@ -28,6 +31,10 @@
 import org.eclipse.equinox.p2.query.IQueryResult;
 import org.eclipse.equinox.p2.query.IQueryable;
 import org.eclipse.equinox.p2.repository.artifact.IArtifactDescriptor;
+import org.eclipse.equinox.p2.repository.artifact.spi.ArtifactDescriptor;
+import org.eclipse.tycho.PackagingType;
+import org.eclipse.tycho.TychoConstants;
+import org.eclipse.tycho.core.shared.MavenContext;
 import org.eclipse.tycho.core.shared.MavenLogger;
 import org.eclipse.tycho.core.shared.MultiLineLogger;
 import org.eclipse.tycho.repository.p2base.artifact.provider.IRawArtifactFileProvider;
@@ -66,6 +73,7 @@ public class MirroringArtifactProvider implements IRawArtifactFileProvider {
     protected final LocalArtifactRepository localArtifactRepository;
 
     protected final IProgressMonitor monitor;
+    private MavenContext mavenContext;
 
     /**
      * Creates a new {@link MirroringArtifactProvider} instance.
@@ -83,21 +91,22 @@ public class MirroringArtifactProvider implements IRawArtifactFileProvider {
      *            a logger for progress output
      */
     public static MirroringArtifactProvider createInstance(LocalArtifactRepository localArtifactRepository,
-            IRawArtifactProvider remoteProviders, boolean mirrorPacked, MavenLogger logger) {
+            IRawArtifactProvider remoteProviders, boolean mirrorPacked, MavenContext mavenContext) {
         if (!mirrorPacked) {
-            return new MirroringArtifactProvider(localArtifactRepository, remoteProviders, logger);
+            return new MirroringArtifactProvider(localArtifactRepository, remoteProviders, mavenContext);
         } else {
-            return new PackedFormatMirroringArtifactProvider(localArtifactRepository, remoteProviders, logger);
+            return new PackedFormatMirroringArtifactProvider(localArtifactRepository, remoteProviders, mavenContext);
         }
     }
 
     MirroringArtifactProvider(LocalArtifactRepository localArtifactRepository, IRawArtifactProvider remoteProviders,
-            MavenLogger logger) {
+            MavenContext mavenContext) {
         this.remoteProviders = remoteProviders;
         this.localArtifactRepository = localArtifactRepository;
-        this.logger = logger;
-        this.splittingLogger = new MultiLineLogger(logger);
-        this.monitor = new LoggingProgressMonitor(logger);
+        this.mavenContext = mavenContext;
+        this.logger = mavenContext.getLogger();
+        this.splittingLogger = new MultiLineLogger(this.logger);
+        this.monitor = new LoggingProgressMonitor(this.logger);
     }
 
     // pass through methods
@@ -208,13 +217,16 @@ private boolean makeLocallyAvailable(IArtifactKey key) throws MirroringFailedExc
     protected boolean makeOneFormatLocallyAvailable(IArtifactKey key)
             throws MirroringFailedException, ProvisionException, ArtifactSinkException {
 
-        if (localArtifactRepository.contains(key)) {
+        if (isFileAlreadyAvailable(key)) {
             return true;
         } else if (remoteProviders.contains(key)) {
             Lock downloadLock = localArtifactRepository.getLockForDownload(key);
             downloadLock.lock();
             try {
-                if (!localArtifactRepository.contains(key)) { // check again within lock
+                if (!isFileAlreadyAvailable(key)) { // check again within lock
+                    if (localArtifactRepository.contains(key)) {
+                        localArtifactRepository.removeDescriptor(key);
+                    }
                     downloadArtifact(key);
                 }
             } finally {
@@ -251,7 +263,22 @@ protected IStatus downloadMostSpecificNeededFormatOfArtifact(IArtifactKey key)
     protected final IStatus downloadCanonicalArtifact(IArtifactKey key)
             throws ProvisionException, ArtifactSinkException {
         // TODO 397355 ignore ProvisionException.ARTIFACT_EXISTS - artifact may have been added by other thread in the meantime
-        IArtifactSink localSink = localArtifactRepository.newAddingArtifactSink(key);
+        IArtifactDescriptor descriptor = localArtifactRepository.createArtifactDescriptor(key);
+        if (descriptor instanceof ArtifactDescriptor) {
+            ArtifactDescriptor localDescriptor = (ArtifactDescriptor) descriptor;
+            IArtifactDescriptor remoteDescriptor = findCanonicalDescriptor(remoteProviders.getArtifactDescriptors(key));
+            if (remoteDescriptor != null) {
+                remoteDescriptor.getProperties().forEach(localDescriptor::setProperty);
+                //fix bad metadata in p2...
+                if (TychoConstants.PACK200_CLASSIFIER
+                        .equals(localDescriptor.getProperty(TychoConstants.PROP_CLASSIFIER))) {
+                    localDescriptor.setProperty(TychoConstants.PROP_CLASSIFIER, "");
+                    localDescriptor.setProperty(TychoConstants.PROP_EXTENSION, "jar");
+                    localDescriptor.setProperty(TychoConstants.PROP_TYPE, PackagingType.TYPE_ECLIPSE_PLUGIN);
+                }
+            }
+        }
+        IArtifactSink localSink = localArtifactRepository.newAddingArtifactSink(descriptor);
         return remoteProviders.getArtifact(localSink, monitorForDownload());
     }
 
@@ -273,7 +300,9 @@ private void createCanonicalArtifactFromLocalPackedArtifact(IArtifactKey key)
         logger.info("Unpacking " + key.getId() + "_" + key.getVersion() + "...");
 
         // TODO 397355 ignore ProvisionException.ARTIFACT_EXISTS
-        IArtifactSink sink = localArtifactRepository.newAddingArtifactSink(key);
+
+        IArtifactSink sink = localArtifactRepository
+                .newAddingArtifactSink(localArtifactRepository.createArtifactDescriptor(key));
         localArtifactRepository.getArtifact(sink, monitor);
     }
 
@@ -329,6 +358,24 @@ public static class MirroringFailedException extends RuntimeException {
 
     @Override
     public boolean isFileAlreadyAvailable(IArtifactKey artifactKey) {
-        return localArtifactRepository.contains(artifactKey);
+        if (localArtifactRepository.contains(artifactKey)) {
+            if (mavenContext.isOffline()) {
+                return true;
+            }
+            if (remoteProviders.contains(artifactKey)) {
+                //we must compare remote versus local!
+                IArtifactDescriptor remoteDescriptor = findCanonicalDescriptor(
+                        remoteProviders.getArtifactDescriptors(artifactKey));
+                IArtifactDescriptor localDescriptor = findCanonicalDescriptor(
+                        localArtifactRepository.getArtifactDescriptors(artifactKey));
+                if (remoteDescriptor != null && localDescriptor != null) {
+                    Map<String, String> remoteProperties = remoteDescriptor.getProperties();
+                    Map<String, String> localProperties = localDescriptor.getProperties();
+                    return Objects.equals(remoteProperties, localProperties);
+                }
+            }
+            return true;
+        }
+        return false;
     }
 }