Skip to content

Commit

Permalink
tycho-p2-director:director: Fix handling of destination on macOS
Browse files Browse the repository at this point in the history
Partially revert changes to destination handling
from 3f41779
and dc99997

The helper methods in DirectorRuntime are removed again and the special
handling in put back into AbstractEclipseTestMojo like before
3f41779.

No pre-processing of the destination is done in DirectorMojo anymore:

1) Deriving the pre-processing path from the running environment was
   incorrect (it would have to consider 'p2os').
2) We actually do not want any pre-procesing at all.
   This restores API compatibility of Tycho's
       tycho-p2-director:director
   with a legacy
       eclipse -application org.eclipse.equinox.p2.director
           -destination <destination> ...
   stand-alone invocation.

This also adds a consistency check that p2os/p2ws/p2arch must all be
specified if any of them is.

Integration tests for the stand-alone director are added.

Fixes #3548.
  • Loading branch information
sratz committed Mar 7, 2024
1 parent d6261cc commit 40dd605
Show file tree
Hide file tree
Showing 6 changed files with 325 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import org.eclipse.equinox.p2.engine.IPhaseSet;
import org.eclipse.tycho.DependencySeed;
import org.eclipse.tycho.PlatformPropertiesUtils;
import org.eclipse.tycho.TargetEnvironment;

/**
Expand Down Expand Up @@ -71,34 +70,4 @@ public interface Command {
*/
public Command newInstallCommand(String name);

/**
* Computes the destination of a director install based on a target environment
*
* @param baseLocation
* @param env
* @return
*/
public static File getDestination(File baseLocation, TargetEnvironment env) {
if (PlatformPropertiesUtils.OS_MACOSX.equals(env.getOs()) && !hasRequiredMacLayout(baseLocation)) {
return new File(baseLocation, "Eclipse.app/Contents/Eclipse/");
}
return baseLocation;
}

private static boolean hasRequiredMacLayout(File folder) {
//TODO if we do not have this exact layout then director fails with:
//The framework persistent data location (/work/MacOS/configuration) is not the same as the framework configuration location /work/Contents/Eclipse/configuration)
//maybe we can simply configure the "persistent data location" to point to the expected one?
//or the "configuration location" must be configured and look like /work/Contents/<work>/configuration ?
//the actual values seem even depend on if this is an empty folder where one installs or an existing one
//e.g. if one installs multiple env Equinox finds the launcher and then set the location different...
if ("Eclipse".equals(folder.getName())) {
File folder2 = folder.getParentFile();
if (folder2 != null && "Contents".equals(folder2.getName())) {
File parent = folder2.getParentFile();
return parent != null && parent.getName().endsWith(".app");
}
}
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,206 @@
<version>0.1.0-SNAPSHOT</version>
<packaging>pom</packaging>

<build>
<plugins>
<plugin>
<groupId>org.eclipse.tycho</groupId>
<artifactId>tycho-p2-director-plugin</artifactId>
<version>${tycho-version}</version>
<executions>
<execution>
<id>run-director</id>
<goals>
<goal>director</goal>
</goals>
<phase>package</phase>
<configuration>
<repositories>${target-platform}</repositories>
<installIUs>org.eclipse.osgi</installIUs>
<destination>target/dummy</destination>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
<profiles>
<profile>
<id>director-windows</id>
<build>
<plugins>
<plugin>
<groupId>org.eclipse.tycho</groupId>
<artifactId>tycho-p2-director-plugin</artifactId>
<version>${tycho-version}</version>
<executions>
<execution>
<id>run-director-windows</id>
<goals>
<goal>director</goal>
</goals>
<phase>package</phase>
<configuration>
<repositories>${target-platform}</repositories>
<installIUs>org.eclipse.osgi</installIUs>
<destination>target/productwindows</destination>
<p2os>win32</p2os>
<p2ws>win32</p2ws>
<p2arch>x86_64</p2arch>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
<profile>
<id>director-linux</id>
<build>
<plugins>
<plugin>
<groupId>org.eclipse.tycho</groupId>
<artifactId>tycho-p2-director-plugin</artifactId>
<version>${tycho-version}</version>
<executions>
<execution>
<id>run-director-linux</id>
<goals>
<goal>director</goal>
</goals>
<phase>package</phase>
<configuration>
<repositories>${target-platform}</repositories>
<installIUs>org.eclipse.osgi</installIUs>
<destination>target/productlinux</destination>
<p2os>linux</p2os>
<p2ws>gtk</p2ws>
<p2arch>x86_64</p2arch>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
<profile>
<id>director-macos-destination-with-app-suffix</id>
<build>
<plugins>
<plugin>
<groupId>org.eclipse.tycho</groupId>
<artifactId>tycho-p2-director-plugin</artifactId>
<version>${tycho-version}</version>
<executions>
<execution>
<id>run-director-macos-destination-with-app-suffix</id>
<goals>
<goal>director</goal>
</goals>
<phase>package</phase>
<configuration>
<repositories>${target-platform}</repositories>
<installIUs>org.eclipse.osgi</installIUs>
<destination>target/productmacos.app</destination>
<p2os>macosx</p2os>
<p2ws>cocoa</p2ws>
<p2arch>x86_64</p2arch>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
<profile>
<id>director-macos-destination-without-app-suffix</id>
<build>
<plugins>
<plugin>
<groupId>org.eclipse.tycho</groupId>
<artifactId>tycho-p2-director-plugin</artifactId>
<version>${tycho-version}</version>
<executions>
<execution>
<id>run-director-macos-destinationout-with-app-suffix</id>
<goals>
<goal>director</goal>
</goals>
<phase>package</phase>
<configuration>
<repositories>${target-platform}</repositories>
<installIUs>org.eclipse.osgi</installIUs>
<destination>target/productmacos</destination>
<p2os>macosx</p2os>
<p2ws>cocoa</p2ws>
<p2arch>x86_64</p2arch>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
<profile>
<id>director-macos-destination-with-full-bundle-path</id>
<build>
<plugins>
<plugin>
<groupId>org.eclipse.tycho</groupId>
<artifactId>tycho-p2-director-plugin</artifactId>
<version>${tycho-version}</version>
<executions>
<execution>
<id>run-director-macos-destination-with-full-bundle-path</id>
<goals>
<goal>director</goal>
</goals>
<phase>package</phase>
<configuration>
<repositories>${target-platform}</repositories>
<installIUs>org.eclipse.osgi</installIUs>
<destination>target/productmacos.app/Contents/Eclipse</destination>
<p2os>macosx</p2os>
<p2ws>cocoa</p2ws>
<p2arch>x86_64</p2arch>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
<profile>
<id>director-running-environment</id>
<build>
<plugins>
<plugin>
<groupId>org.eclipse.tycho</groupId>
<artifactId>tycho-p2-director-plugin</artifactId>
<version>${tycho-version}</version>
<executions>
<execution>
<id>run-director-running-environment</id>
<goals>
<goal>director</goal>
</goals>
<phase>package</phase>
<configuration>
<repositories>${target-platform}</repositories>
<installIUs>org.eclipse.osgi</installIUs>
<destination>target/product</destination>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
<profile>
<id>director-iconsistent-p2-arguments</id>
<build>
<plugins>
<plugin>
<groupId>org.eclipse.tycho</groupId>
<artifactId>tycho-p2-director-plugin</artifactId>
<version>${tycho-version}</version>
<executions>
<execution>
<id>run-director-iconsistent-p2-arguments</id>
<goals>
<goal>director</goal>
</goals>
<phase>package</phase>
<configuration>
<repositories>${target-platform}</repositories>
<installIUs>org.eclipse.osgi</installIUs>
<destination>target/product</destination>
<p2os>win32</p2os>
<!-- other ones missing -->
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
</profiles>

</project>
Original file line number Diff line number Diff line change
@@ -1,15 +1,104 @@
package org.eclipse.tycho.test;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.nio.file.Files;
import java.nio.file.Path;

import org.apache.maven.it.VerificationException;
import org.apache.maven.it.Verifier;
import org.junit.Test;

public class P2DirectorPluginTest extends AbstractTychoIntegrationTest {

@Test
public void testDirectorStandalone() throws Exception {
public void testDirectorStandaloneWindows() throws Exception {
Verifier verifier = getVerifier("tycho-p2-director-plugin/director-goal-standalone", true, true);
verifier.addCliOption("-Pdirector-windows");
verifier.executeGoal("package");
verifier.verifyErrorFreeLog();
assertTrue(Files.isDirectory(Path.of(verifier.getBasedir(), "target", "productwindows", "plugins")));
}

@Test
public void testDirectorStandaloneLinux() throws Exception {
Verifier verifier = getVerifier("tycho-p2-director-plugin/director-goal-standalone", true, true);
verifier.addCliOption("-Pdirector-linux");
verifier.executeGoal("package");
verifier.verifyErrorFreeLog();
assertTrue(Files.isDirectory(Path.of(verifier.getBasedir(), "target", "productlinux", "plugins")));
}

@Test
public void testDirectorStandaloneMacOsDestinationWithAppSuffix() throws Exception {
Verifier verifier = getVerifier("tycho-p2-director-plugin/director-goal-standalone", true, true);
verifier.addCliOption("-Pdirector-macos-destination-with-app-suffix");
verifier.executeGoal("package");
verifier.verifyErrorFreeLog();
// the DirectorApplication will detect the .app suffix and add Contents/Eclipse
assertTrue(Files.isDirectory(
Path.of(verifier.getBasedir(), "target", "productmacos.app", "Contents", "Eclipse", "plugins")));
}

@Test
public void testDirectorStandaloneMacOsDestinationWithoutAppSuffix() throws Exception {
Verifier verifier = getVerifier("tycho-p2-director-plugin/director-goal-standalone", true, true);
verifier.addCliOption("-Pdirector-macos-destination-without-app-suffix");
verifier.executeGoal("package");
verifier.verifyErrorFreeLog();
// no pre-processing must be done by DirectorMojo to the destination to be
// compatible with how a
// eclipse -application org.eclipse.equinox.p2.director -destination
// <destination> ...
// invocation works.
assertTrue(Files.isDirectory(Path.of(verifier.getBasedir(), "target", "productmacos", "plugins")));
}

@Test
public void testDirectorStandaloneMacOsDestinationWithFullBundlePath() throws Exception {
Verifier verifier = getVerifier("tycho-p2-director-plugin/director-goal-standalone", true, true);
verifier.addCliOption("-Pdirector-macos-destination-with-full-bundle-path");
verifier.executeGoal("package");
verifier.verifyErrorFreeLog();
// no pre-processing must be done by DirectorMojo to the destination to be
// compatible with how a
// eclipse -application org.eclipse.equinox.p2.director -destination
// <destination> ...
// invocation works.
assertTrue(Files.isDirectory(
Path.of(verifier.getBasedir(), "target", "productmacos.app", "Contents", "Eclipse", "plugins")));
}

@Test
public void testDirectorStandaloneUsingRunningEnvironment() throws Exception {
Verifier verifier = getVerifier("tycho-p2-director-plugin/director-goal-standalone", true, true);
verifier.addCliOption("-Pdirector-running-environemnt");
verifier.executeGoal("package");
verifier.verifyErrorFreeLog();

// to be consistent with the classic
// eclipse -application org.eclipse.equinox.p2.director -destination
// <destination> ...
// API, we also do not want to prep-process <destination> in DirectorMojo,
// see https://github.com/eclipse-tycho/tycho/issues/3548
// Therefore, the resulting path must be identical, no matter on which platform
// it is executed.
assertTrue(Files.isDirectory(Path.of(verifier.getBasedir(), "target", "product", "plugins")));
}

@Test
public void testDirectorStandaloneInconsistentP2Options() throws Exception {
Verifier verifier = getVerifier("tycho-p2-director-plugin/director-goal-standalone", true, true);
verifier.addCliOption("-Pdirector-iconsistent-p2-arguments");
try {
verifier.executeGoal("package");
fail(VerificationException.class.getName() + " expected");
} catch (VerificationException e) {
}
verifier.verifyTextInLog("If any of p2os / p2ws / p2arch is specified, all of them must be specified.");
assertFalse(Files.isDirectory(Path.of(verifier.getBasedir(), "target", "product")));
}

}
Loading

0 comments on commit 40dd605

Please sign in to comment.