Skip to content

Commit

Permalink
Improve errors handling while downloading p2 artifacts from repository
Browse files Browse the repository at this point in the history
In case an artifact couldn't be downloaded due to e.g. a bad mirror,
p2 returns an RETRY error code. This code is currently ignored and
Tycho simply fails with an I/O exception.
Instead, Tycho attempts to download the artifact up to three times
(assuming that this error code was returned), before failing. This value
can be configured using the 'eclipse.p2.maxDownloadAttempts' system
property.

(cherry picked from commit 8462a37)
  • Loading branch information
ptziegler authored and eclipse-tycho-bot committed Sep 15, 2024
1 parent 900f0c6 commit f442563
Show file tree
Hide file tree
Showing 12 changed files with 339 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2022 Christoph Läubrich and others.
* Copyright (c) 2022, 2024 Christoph Läubrich 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 Down Expand Up @@ -37,6 +37,7 @@
import org.eclipse.equinox.p2.repository.metadata.IMetadataRepositoryManager;
import org.eclipse.tycho.IRepositoryIdManager;
import org.eclipse.tycho.MavenRepositoryLocation;
import org.eclipse.tycho.helper.MavenPropertyHelper;
import org.eclipse.tycho.p2maven.ListCompositeArtifactRepository;
import org.eclipse.tycho.p2maven.ListQueryable;
import org.eclipse.tycho.p2maven.LoggerProgressMonitor;
Expand All @@ -46,6 +47,10 @@
*/
@Component(role = P2RepositoryManager.class)
public class P2RepositoryManager {
private static final String PROPERTY_KEY = "eclipse.p2.maxDownloadAttempts";

@Requirement
MavenPropertyHelper propertyHelper;

@Requirement
IRepositoryIdManager repositoryIdManager;
Expand Down Expand Up @@ -151,18 +156,44 @@ private IMetadataRepository getMetadataRepositor(URI location, String id) throws
public void downloadArtifact(IInstallableUnit iu, IArtifactRepository artifactRepository,
OutputStream outputStream) throws IOException {
Collection<IArtifactKey> artifacts = iu.getArtifacts();
int maxDownloadAttempts = getMaxDownloadAttempts();

for (IArtifactKey key : artifacts) {
IArtifactDescriptor[] descriptors = artifactRepository.getArtifactDescriptors(key);
for (IArtifactDescriptor descriptor : descriptors) {
IStatus status = artifactRepository.getRawArtifact(descriptor, outputStream,
new LoggerProgressMonitor(logger));
if (status.isOK()) {
return;
for (int downloadAttempts = 0; downloadAttempts < maxDownloadAttempts; ++downloadAttempts) {
IStatus status = artifactRepository.getRawArtifact(descriptor, outputStream,
new LoggerProgressMonitor(logger));
if (status.isOK()) {
return;
}
// Might happen if e.g. a bad mirror was used
if (status.getCode() == IArtifactRepository.CODE_RETRY) {
logger.warn("Artifact repository requested retry (attempt [%d/%d]): '%s'"
.formatted(downloadAttempts + 1, maxDownloadAttempts, status));
continue;
}
throw new IOException("Download failed: " + status);
}
throw new IOException("Download failed: " + status);
}
}
throw new FileNotFoundException();
}

private int getMaxDownloadAttempts() {
String property = propertyHelper.getGlobalProperty(PROPERTY_KEY, "3");
try {
int maxDownloadAttempts = Integer.valueOf(property);
if (maxDownloadAttempts <= 0) {
logger.error("Value '%s' for property '%s', is not a positive number! Use 1 as default value."
.formatted(property, PROPERTY_KEY));
return 1;
}
return maxDownloadAttempts;
} catch (NumberFormatException e) {
logger.error("Value '%s' for property '%s', is not a number! Use 1 as default value.".formatted(property,
PROPERTY_KEY));
return 1;
}
}
}
5 changes: 3 additions & 2 deletions src/site/markdown/SystemProperties.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ tycho.debug.resolver | `true` or _artifactId_ | Enable debug output for the arti
### Baseline compare

Name | Value | Default | Documentation
--- | --- | ---
--- | --- | --- | ---
tycho.comparator.showDiff | true / false | false | If set to true if text-like files show a unified diff of possible differences in files
tycho.comparator.threshold | bytes | 5242880 (~5MB) | gives the number of bytes for content to be compared semantically, larger files will only be compared byte-by-byte

Expand All @@ -32,6 +32,7 @@ tycho.comparator.threshold | bytes | 5242880 (~5MB) | gives the number of bytes
These properties control the behaviour of P2 used by Tycho

Name | Value | Default | Documentation
--- | --- | ---
--- | --- | --- | ---
eclipse.p2.mirrors | true / false | true | Each p2 site can define a list of artifact repository mirrors, this controls if P2 mirrors should be used. This is independent from configuring mirrors in the maven configuration to be used by Tycho!
eclipse.p2.maxDownloadAttempts | _any positive integer_ | 3 | Describes how often Tycho attempts to re-download an artifact from a p2 repository in case e.g. a bad mirror was used. One can think of this value as the maximum number of mirrors Tycho/p2 will check.

20 changes: 20 additions & 0 deletions tycho-its/projects/p2Repository.mirror/baseline/artifacts.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?xml version='1.0' encoding='UTF-8'?>
<?artifactRepository version='1.1.0'?>
<repository name='baseline - artifacts' type='org.eclipse.equinox.p2.artifact.repository.simpleRepository' version='1'>
<properties size='2'>
<property name='p2.timestamp' value='1643973510842'/>
<property name='p2.compressed' value='false'/>
<property name ='p2.mirrorsURL' value='set by test' />
</properties>
<mappings size='1'>
<rule filter='(&amp; (classifier=osgi.bundle))' output='${repoUrl}/plugins/${id}_${version}.jar'/>
</mappings>
<artifacts size='1'>
<artifact classifier='osgi.bundle' id='bundle1' version='1.0.0'>
<properties size='2'>
<property name='artifact.size' value='417'/>
<property name='download.size' value='417'/>
</properties>
</artifact>
</artifacts>
</repository>
34 changes: 34 additions & 0 deletions tycho-its/projects/p2Repository.mirror/baseline/content.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?xml version='1.0' encoding='UTF-8'?>
<?metadataRepository version='1.2.0'?>
<repository name='baseline - metadata' type='org.eclipse.equinox.internal.p2.metadata.repository.LocalMetadataRepository' version='1'>
<properties size='2'>
<property name='p2.timestamp' value='1643973510843'/>
<property name='p2.compressed' value='false'/>
</properties>
<units size='1'>
<unit id='bundle1' version='1.0.0' singleton='false'>
<update id='bundle1' range='[0.0.0,1.0.0)' severity='0'/>
<provides size='4'>
<provided namespace='org.eclipse.equinox.p2.iu' name='bundle1' version='1.0.0'/>
<provided namespace='osgi.bundle' name='bundle1' version='1.0.0'/>
<provided namespace='osgi.identity' name='bundle1' version='1.0.0'>
<properties size='1'>
<property name='type' value='osgi.bundle'/>
</properties>
</provided>
<provided namespace='org.eclipse.equinox.p2.eclipse.type' name='bundle' version='1.0.0'/>
</provides>
<artifacts size='1'>
<artifact classifier='osgi.bundle' id='bundle1' version='1.0.0'/>
</artifacts>
<touchpoint id='org.eclipse.equinox.p2.osgi' version='1.0.0'/>
<touchpointData size='1'>
<instructions size='1'>
<instruction key='manifest'>
Bundle-SymbolicName: bundle1&#xA;Bundle-Version: 1.0.0&#xA;
</instruction>
</instructions>
</touchpointData>
</unit>
</units>
</repository>
4 changes: 4 additions & 0 deletions tycho-its/projects/p2Repository.mirror/baseline/mirrors.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<mirrors>
<mirror url ='set by test'/>
<mirror url ='set by test'/>
</mirrors>
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-SymbolicName: bundle1
Bundle-Version: 1.0.0
Automatic-Module-Name: bundle1

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
bin.includes = META-INF/
45 changes: 45 additions & 0 deletions tycho-its/projects/p2Repository.mirror/bundle/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd" xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
<modelVersion>4.0.0</modelVersion>

<parent>
<groupId>p2Repository.mirror</groupId>
<artifactId>parent</artifactId>
<version>0.0.1-SNAPSHOT</version>
</parent>

<packaging>eclipse-plugin</packaging>
<artifactId>bundle1</artifactId>
<version>1.0.0</version>

<build>
<plugins>
<plugin>
<groupId>org.eclipse.tycho</groupId>
<artifactId>tycho-baseline-plugin</artifactId>
<version>${tycho-version}</version>
<executions>
<execution>
<id>compare-version-with-baseline</id>
<phase>verify</phase>
<goals>
<goal>verify</goal>
</goals>
</execution>
</executions>
<configuration>
<ignores>
<ignore>META-INF/maven/**/*</ignore>
</ignores>
<baselines>
<repository>
<id>repo</id>
<url>${baseline}</url>
<layout>p2</layout>
</repository>
</baselines>
</configuration>
</plugin>
</plugins>
</build>
</project>
23 changes: 23 additions & 0 deletions tycho-its/projects/p2Repository.mirror/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>p2Repository.mirror</groupId>
<artifactId>parent</artifactId>
<version>0.0.1-SNAPSHOT</version>
<packaging>pom</packaging>

<modules>
<module>bundle</module>
</modules>

<build>
<plugins>
<plugin>
<groupId>org.eclipse.tycho</groupId>
<artifactId>tycho-maven-plugin</artifactId>
<version>${tycho-version}</version>
<extensions>true</extensions>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
/*******************************************************************************
* Copyright (c) 2024 Patrick Ziegler 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
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Patrick Ziegler - initial API and implementation
*******************************************************************************/
package org.eclipse.tycho.test.p2Repository;

import java.io.File;
import java.io.FileWriter;

import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.transform.Transformer;
import javax.xml.transform.TransformerFactory;
import javax.xml.transform.dom.DOMSource;
import javax.xml.transform.stream.StreamResult;
import javax.xml.xpath.XPath;
import javax.xml.xpath.XPathConstants;
import javax.xml.xpath.XPathFactory;

import org.apache.maven.it.Verifier;
import org.eclipse.jetty.servlet.DefaultServlet;
import org.eclipse.jetty.util.resource.EmptyResource;
import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.tycho.test.AbstractTychoIntegrationTest;
import org.eclipse.tycho.test.util.HttpServer;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.NodeList;

public class P2RepositoryMirrorTest extends AbstractTychoIntegrationTest {

private HttpServer server;

@Before
public void startServer() throws Exception {
server = HttpServer.startServer();
}

@After
public void stopServer() throws Exception {
server.stop();
}

/**
* Tests whether Tycho is able to recover from a bad mirror repository. If
* multiple mirrors are specified for a repository, Tycho might be able to
* continue by requesting an artifact from a different mirror, depending on the
* error code returned by Equinox.
*/
@Test
public void testMirrorWithRetry() throws Exception {
Verifier verifier = getVerifier("p2Repository.mirror", false);

String baseline = server.addServer("baseline", FirstBaselineRequestFailsServlet.class,
new File(verifier.getBasedir(), "baseline"));
// Two mirrors, to always have at least one that is still good
String mirror1 = server.addServer("mirror1", FirstBaselineRequestFailsServlet.class,
new File(verifier.getBasedir(), "baseline"));
String mirror2 = server.addServer("mirror2", FirstBaselineRequestFailsServlet.class,
new File(verifier.getBasedir(), "baseline"));
String mirrors = baseline + '/' + "mirrors.xml";

setMirrorsUrl(new File(verifier.getBasedir(), "baseline/artifacts.xml"), mirrors);
setMirrors(new File(verifier.getBasedir(), "baseline/mirrors.xml"), mirror1, mirror2);

// The verifier escapes the 'http://localhost' to 'http:/localhost'
verifier.addCliOption("-Dbaseline=" + baseline.replaceAll("//", "////"));
// Force an update of the HttpCache
verifier.addCliOption("-U");
verifier.executeGoal("verify");
verifier.verifyErrorFreeLog();
verifier.verifyTextInLog("Artifact repository requested retry (attempt [1/3]):");
}

@Override
protected boolean isDisableMirrors() {
return false;
}

/**
* Updates the "p2.mirrorsURL" property in the {@code artifacts.xml} file of the
* baseline repository to point to the {@code mirrors.xml} file.
*/
private static void setMirrorsUrl(File artifactsXml, String mirrorsUrl) throws Exception {
Document document = parseDocument(artifactsXml);

XPath path = XPathFactory.newInstance().newXPath();
String expression = "repository/properties/property[@name='p2.mirrorsURL']";
Element node = (Element) path.evaluate(expression, document, XPathConstants.NODE);

if (node != null) {
node.setAttribute("value", mirrorsUrl);
writeDocument(document, artifactsXml);
}
}

/**
* Updates the {@link mirrors.xml} file to contain the bad mirror.
*/
private static void setMirrors(File mirrorsXml, String mirror1, String mirror2) throws Exception {
Document document = parseDocument(mirrorsXml);

XPath path = XPathFactory.newInstance().newXPath();
String expression = "mirrors/mirror";
NodeList nodes = (NodeList) path.evaluate(expression, document, XPathConstants.NODESET);

if (nodes != null) {
((Element) nodes.item(0)).setAttribute("url", mirror1);
((Element) nodes.item(1)).setAttribute("url", mirror2);
writeDocument(document, mirrorsXml);
}
}

public static Document parseDocument(File file) throws Exception {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
DocumentBuilder builder = factory.newDocumentBuilder();
return builder.parse(file);
}

public static void writeDocument(Document document, File file) throws Exception {
TransformerFactory transformerFactory = TransformerFactory.newInstance();
Transformer transformer = transformerFactory.newTransformer();

try (FileWriter writer = new FileWriter(file)) {
DOMSource source = new DOMSource(document);
StreamResult result = new StreamResult(writer);
transformer.transform(source, result);
}
}

/**
* Helper servlet to simulate an illegal p2 repository. The first time a plugin
* is requested from the remote repository, a 404 is returned. Any further
* requests succeed, to test whether Tycho is able to recover from bad mirrors.
*/
public static class FirstBaselineRequestFailsServlet extends DefaultServlet {
// We don't know which mirror is selected, so anyone can fail
private static boolean firstRequest = true;

@Override
public Resource getResource(String pathInContext) {
if (firstRequest && pathInContext.startsWith("/plugins/")) {
firstRequest = false;
return EmptyResource.INSTANCE;
}
return super.getResource(pathInContext);
}
}
}
Loading

0 comments on commit f442563

Please sign in to comment.