Skip to content

Commit 96143c8

Browse files
sratzeclipse-tycho-bot
sratz
authored and
eclipse-tycho-bot
committed
tycho-p2-director:director: Fix handling of destination on macOS
* In DirectorMojo, the adjustment of 'destination' must consider the actual target environment (p2os/p2ws/p2arch parameters) that is to be installed and only fall back to the running environment if no explicit target environment is given. * Document in the tycho-p2-director:director JavaDoc / mojo parameter description that this intentionally deviates from the behavior of the stand-alone director application invocation: eclipse -application org.eclipse.equinox.p2.director -destination <destination> ... * In DirectorMojo, add a consistency check that p2os/p2ws/p2arch must all be specified mutually. * The helper methods in DirectorRuntime are extended, to properly handle all three possible scenarios: 1) /path/without/app/bundle/layout --> /path/without/app/bundle/layout/Eclipse.app/Contents/Eclipse 2) /path/to/real.app/Contents/Eclipse --> /path/to/real.app/Contents/Eclipse 3) /path/to/real.app --> /path/to/real.app/Contents/Eclipse This allows us to remove redundant code in ProvisionedInstallationBuilder. * This also removes the <installAllPlatforms> option again which was introduced in #3091 (606a087). This is not used in production and was not having the desired effect. This simplifies the handling in AbstractEclipseTestMojo / ProvisionedInstallationBuilder even more. Fixes #3548. (cherry picked from commit b35d8e0)
1 parent e21610e commit 96143c8

File tree

8 files changed

+617
-73
lines changed

8 files changed

+617
-73
lines changed

‎tycho-core/src/main/java/org/eclipse/tycho/p2/tools/director/shared/DirectorRuntime.java

+31-5
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,35 @@ public interface Command {
7373

7474
/**
7575
* Computes the destination of a director install based on a target environment
76-
*
76+
* <p>
77+
* Currently, this implements special handling for macOS and behaves as follows:
78+
* <ul>
79+
* <li>If <code>baseLocation</code> already conforms to the full app bundle layout
80+
* (<code>/path/to/Foo.app/Contents/Eclipse</code>), <code>baseLocation</code> is returned
81+
* as-is.
82+
* <li>If <code>baseLocation</code> points to the root of an app bundle
83+
* (<code>/path/to/Foo.app</code>), <code>Contents/Eclipse</code> is appended and the path
84+
* <code>/path/to/Foo.app/Contents/Eclipse</code> is returned.
85+
* <li>Otherwise, i.e. if no app bundle path is given (<code>/path/to/work</code>), a valid app
86+
* bundle path is appended, and the path <code>/path/to/work/Eclipse.app/Contents/Eclipse</code>
87+
* is returned.
88+
* </ul>
89+
*
7790
* @param baseLocation
91+
* the base location
7892
* @param env
79-
* @return
93+
* the target environment
94+
* @return the adjusted location to conform to layouts required by the target environment
8095
*/
8196
public static File getDestination(File baseLocation, TargetEnvironment env) {
82-
if (PlatformPropertiesUtils.OS_MACOSX.equals(env.getOs()) && !hasRequiredMacLayout(baseLocation)) {
83-
return new File(baseLocation, "Eclipse.app/Contents/Eclipse/");
97+
if (PlatformPropertiesUtils.OS_MACOSX.equals(env.getOs())) {
98+
if (hasRequiredMacLayout(baseLocation)) {
99+
return baseLocation;
100+
} else if (isMacOsAppBundleRoot(baseLocation)) {
101+
return new File(baseLocation, "Contents/Eclipse/");
102+
} else {
103+
return new File(baseLocation, "Eclipse.app/Contents/Eclipse/");
104+
}
84105
}
85106
return baseLocation;
86107
}
@@ -96,9 +117,14 @@ private static boolean hasRequiredMacLayout(File folder) {
96117
File folder2 = folder.getParentFile();
97118
if (folder2 != null && "Contents".equals(folder2.getName())) {
98119
File parent = folder2.getParentFile();
99-
return parent != null && parent.getName().endsWith(".app");
120+
return parent != null && isMacOsAppBundleRoot(parent);
100121
}
101122
}
102123
return false;
103124
}
125+
126+
private static boolean isMacOsAppBundleRoot(File folder) {
127+
return folder.getName().endsWith(".app");
128+
}
129+
104130
}

‎tycho-its/projects/tycho-p2-director-plugin/director-goal-standalone/pom.xml

+201-23
Original file line numberDiff line numberDiff line change
@@ -7,28 +7,206 @@
77
<version>0.1.0-SNAPSHOT</version>
88
<packaging>pom</packaging>
99

10-
<build>
11-
<plugins>
12-
<plugin>
13-
<groupId>org.eclipse.tycho</groupId>
14-
<artifactId>tycho-p2-director-plugin</artifactId>
15-
<version>${tycho-version}</version>
16-
<executions>
17-
<execution>
18-
<id>run-director</id>
19-
<goals>
20-
<goal>director</goal>
21-
</goals>
22-
<phase>package</phase>
23-
<configuration>
24-
<repositories>${target-platform}</repositories>
25-
<installIUs>org.eclipse.osgi</installIUs>
26-
<destination>target/dummy</destination>
27-
</configuration>
28-
</execution>
29-
</executions>
30-
</plugin>
31-
</plugins>
32-
</build>
10+
<profiles>
11+
<profile>
12+
<id>director-windows</id>
13+
<build>
14+
<plugins>
15+
<plugin>
16+
<groupId>org.eclipse.tycho</groupId>
17+
<artifactId>tycho-p2-director-plugin</artifactId>
18+
<version>${tycho-version}</version>
19+
<executions>
20+
<execution>
21+
<id>run-director-windows</id>
22+
<goals>
23+
<goal>director</goal>
24+
</goals>
25+
<phase>package</phase>
26+
<configuration>
27+
<repositories>${target-platform}</repositories>
28+
<installIUs>org.eclipse.osgi</installIUs>
29+
<destination>target/productwindows</destination>
30+
<p2os>win32</p2os>
31+
<p2ws>win32</p2ws>
32+
<p2arch>x86_64</p2arch>
33+
</configuration>
34+
</execution>
35+
</executions>
36+
</plugin>
37+
</plugins>
38+
</build>
39+
</profile>
40+
<profile>
41+
<id>director-linux</id>
42+
<build>
43+
<plugins>
44+
<plugin>
45+
<groupId>org.eclipse.tycho</groupId>
46+
<artifactId>tycho-p2-director-plugin</artifactId>
47+
<version>${tycho-version}</version>
48+
<executions>
49+
<execution>
50+
<id>run-director-linux</id>
51+
<goals>
52+
<goal>director</goal>
53+
</goals>
54+
<phase>package</phase>
55+
<configuration>
56+
<repositories>${target-platform}</repositories>
57+
<installIUs>org.eclipse.osgi</installIUs>
58+
<destination>target/productlinux</destination>
59+
<p2os>linux</p2os>
60+
<p2ws>gtk</p2ws>
61+
<p2arch>x86_64</p2arch>
62+
</configuration>
63+
</execution>
64+
</executions>
65+
</plugin>
66+
</plugins>
67+
</build>
68+
</profile>
69+
<profile>
70+
<id>director-macos-destination-with-app-suffix</id>
71+
<build>
72+
<plugins>
73+
<plugin>
74+
<groupId>org.eclipse.tycho</groupId>
75+
<artifactId>tycho-p2-director-plugin</artifactId>
76+
<version>${tycho-version}</version>
77+
<executions>
78+
<execution>
79+
<id>run-director-macos-destination-with-app-suffix</id>
80+
<goals>
81+
<goal>director</goal>
82+
</goals>
83+
<phase>package</phase>
84+
<configuration>
85+
<repositories>${target-platform}</repositories>
86+
<installIUs>org.eclipse.osgi</installIUs>
87+
<destination>target/productmacos.app</destination>
88+
<p2os>macosx</p2os>
89+
<p2ws>cocoa</p2ws>
90+
<p2arch>x86_64</p2arch>
91+
</configuration>
92+
</execution>
93+
</executions>
94+
</plugin>
95+
</plugins>
96+
</build>
97+
</profile>
98+
<profile>
99+
<id>director-macos-destination-without-app-suffix</id>
100+
<build>
101+
<plugins>
102+
<plugin>
103+
<groupId>org.eclipse.tycho</groupId>
104+
<artifactId>tycho-p2-director-plugin</artifactId>
105+
<version>${tycho-version}</version>
106+
<executions>
107+
<execution>
108+
<id>run-director-macos-destinationout-with-app-suffix</id>
109+
<goals>
110+
<goal>director</goal>
111+
</goals>
112+
<phase>package</phase>
113+
<configuration>
114+
<repositories>${target-platform}</repositories>
115+
<installIUs>org.eclipse.osgi</installIUs>
116+
<destination>target/productmacos</destination>
117+
<p2os>macosx</p2os>
118+
<p2ws>cocoa</p2ws>
119+
<p2arch>x86_64</p2arch>
120+
</configuration>
121+
</execution>
122+
</executions>
123+
</plugin>
124+
</plugins>
125+
</build>
126+
</profile>
127+
<profile>
128+
<id>director-macos-destination-with-full-bundle-path</id>
129+
<build>
130+
<plugins>
131+
<plugin>
132+
<groupId>org.eclipse.tycho</groupId>
133+
<artifactId>tycho-p2-director-plugin</artifactId>
134+
<version>${tycho-version}</version>
135+
<executions>
136+
<execution>
137+
<id>run-director-macos-destination-with-full-bundle-path</id>
138+
<goals>
139+
<goal>director</goal>
140+
</goals>
141+
<phase>package</phase>
142+
<configuration>
143+
<repositories>${target-platform}</repositories>
144+
<installIUs>org.eclipse.osgi</installIUs>
145+
<destination>target/productmacos.app/Contents/Eclipse</destination>
146+
<p2os>macosx</p2os>
147+
<p2ws>cocoa</p2ws>
148+
<p2arch>x86_64</p2arch>
149+
</configuration>
150+
</execution>
151+
</executions>
152+
</plugin>
153+
</plugins>
154+
</build>
155+
</profile>
156+
<profile>
157+
<id>director-running-environment</id>
158+
<build>
159+
<plugins>
160+
<plugin>
161+
<groupId>org.eclipse.tycho</groupId>
162+
<artifactId>tycho-p2-director-plugin</artifactId>
163+
<version>${tycho-version}</version>
164+
<executions>
165+
<execution>
166+
<id>run-director-running-environment</id>
167+
<goals>
168+
<goal>director</goal>
169+
</goals>
170+
<phase>package</phase>
171+
<configuration>
172+
<repositories>${target-platform}</repositories>
173+
<installIUs>org.eclipse.osgi</installIUs>
174+
<destination>target/product</destination>
175+
</configuration>
176+
</execution>
177+
</executions>
178+
</plugin>
179+
</plugins>
180+
</build>
181+
</profile>
182+
<profile>
183+
<id>director-iconsistent-p2-arguments</id>
184+
<build>
185+
<plugins>
186+
<plugin>
187+
<groupId>org.eclipse.tycho</groupId>
188+
<artifactId>tycho-p2-director-plugin</artifactId>
189+
<version>${tycho-version}</version>
190+
<executions>
191+
<execution>
192+
<id>run-director-iconsistent-p2-arguments</id>
193+
<goals>
194+
<goal>director</goal>
195+
</goals>
196+
<phase>package</phase>
197+
<configuration>
198+
<repositories>${target-platform}</repositories>
199+
<installIUs>org.eclipse.osgi</installIUs>
200+
<destination>target/product</destination>
201+
<p2os>win32</p2os>
202+
<!-- other ones missing -->
203+
</configuration>
204+
</execution>
205+
</executions>
206+
</plugin>
207+
</plugins>
208+
</build>
209+
</profile>
210+
</profiles>
33211

34212
</project>

‎tycho-its/src/test/java/org/eclipse/tycho/test/P2DirectorPluginTest.java

+85-1
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,99 @@
11
package org.eclipse.tycho.test;
22

3+
import static org.junit.Assert.assertFalse;
4+
import static org.junit.Assert.assertTrue;
5+
import static org.junit.Assert.fail;
6+
7+
import java.nio.file.Files;
8+
import java.nio.file.Path;
9+
10+
import org.apache.maven.it.VerificationException;
311
import org.apache.maven.it.Verifier;
12+
import org.eclipse.tycho.TargetEnvironment;
413
import org.junit.Test;
514

615
public class P2DirectorPluginTest extends AbstractTychoIntegrationTest {
716

817
@Test
9-
public void testDirectorStandalone() throws Exception {
18+
public void testDirectorStandaloneWindows() throws Exception {
19+
Verifier verifier = getVerifier("tycho-p2-director-plugin/director-goal-standalone", true, true);
20+
verifier.addCliOption("-Pdirector-windows");
21+
verifier.executeGoal("package");
22+
verifier.verifyErrorFreeLog();
23+
24+
assertTrue(Files.isDirectory(Path.of(verifier.getBasedir(), "target", "productwindows", "plugins")));
25+
}
26+
27+
@Test
28+
public void testDirectorStandaloneLinux() throws Exception {
29+
Verifier verifier = getVerifier("tycho-p2-director-plugin/director-goal-standalone", true, true);
30+
verifier.addCliOption("-Pdirector-linux");
31+
verifier.executeGoal("package");
32+
verifier.verifyErrorFreeLog();
33+
34+
assertTrue(Files.isDirectory(Path.of(verifier.getBasedir(), "target", "productlinux", "plugins")));
35+
}
36+
37+
@Test
38+
public void testDirectorStandaloneMacOsDestinationWithAppSuffix() throws Exception {
39+
Verifier verifier = getVerifier("tycho-p2-director-plugin/director-goal-standalone", true, true);
40+
verifier.addCliOption("-Pdirector-macos-destination-with-app-suffix");
41+
verifier.executeGoal("package");
42+
verifier.verifyErrorFreeLog();
43+
44+
assertTrue(Files.isDirectory(
45+
Path.of(verifier.getBasedir(), "target", "productmacos.app", "Contents", "Eclipse", "plugins")));
46+
}
47+
48+
@Test
49+
public void testDirectorStandaloneMacOsDestinationWithoutAppSuffix() throws Exception {
1050
Verifier verifier = getVerifier("tycho-p2-director-plugin/director-goal-standalone", true, true);
51+
verifier.addCliOption("-Pdirector-macos-destination-without-app-suffix");
1152
verifier.executeGoal("package");
1253
verifier.verifyErrorFreeLog();
54+
55+
assertTrue(Files.isDirectory(Path.of(verifier.getBasedir(), "target", "productmacos", "Eclipse.app", "Contents",
56+
"Eclipse", "plugins")));
57+
}
58+
59+
@Test
60+
public void testDirectorStandaloneMacOsDestinationWithFullBundlePath() throws Exception {
61+
Verifier verifier = getVerifier("tycho-p2-director-plugin/director-goal-standalone", true, true);
62+
verifier.addCliOption("-Pdirector-macos-destination-with-full-bundle-path");
63+
verifier.executeGoal("package");
64+
verifier.verifyErrorFreeLog();
65+
66+
assertTrue(Files.isDirectory(
67+
Path.of(verifier.getBasedir(), "target", "productmacos.app", "Contents", "Eclipse", "plugins")));
68+
}
69+
70+
@Test
71+
public void testDirectorStandaloneUsingRunningEnvironment() throws Exception {
72+
Verifier verifier = getVerifier("tycho-p2-director-plugin/director-goal-standalone", true, true);
73+
verifier.addCliOption("-Pdirector-running-environment");
74+
verifier.executeGoal("package");
75+
verifier.verifyErrorFreeLog();
76+
77+
if ("macosx".equals(TargetEnvironment.getRunningEnvironment().getOs())) {
78+
assertTrue(Files.isDirectory(Path.of(verifier.getBasedir(), "target", "product", "Eclipse.app", "Contents",
79+
"Eclipse", "plugins")));
80+
} else {
81+
assertTrue(Files.isDirectory(Path.of(verifier.getBasedir(), "target", "product", "plugins")));
82+
}
83+
}
84+
85+
@Test
86+
public void testDirectorStandaloneInconsistentP2Options() throws Exception {
87+
Verifier verifier = getVerifier("tycho-p2-director-plugin/director-goal-standalone", true, true);
88+
verifier.addCliOption("-Pdirector-iconsistent-p2-arguments");
89+
try {
90+
verifier.executeGoal("package");
91+
fail(VerificationException.class.getName() + " expected");
92+
} catch (VerificationException e) {
93+
}
94+
verifier.verifyTextInLog(
95+
"p2os / p2ws / p2arch must be mutually specified, p2os=win32 given, p2ws missing, p2arch missing");
96+
assertFalse(Files.isDirectory(Path.of(verifier.getBasedir(), "target", "product")));
1397
}
1498

1599
}

0 commit comments

Comments
 (0)