From 361fd9881e7bbc58046ea3c791fb5679cb1e6f2e Mon Sep 17 00:00:00 2001 From: Sebastian Ratz Date: Wed, 6 Mar 2024 16:04:32 +0000 Subject: [PATCH] tycho-p2-director:director: Fix handling of destination on macOS Partially revert changes to destination handling from 3f41779dbae8c50dfa46b6d19127b3d5d74dda1a and dc99997b8e2ebd08592d30c9634a4a12baaef59d The helper methods in DirectorRuntime are removed again and the special handling in put back into AbstractEclipseTestMojo like before 3f41779dbae8c50dfa46b6d19127b3d5d74dda1a. 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 ... 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. --- .../director/shared/DirectorRuntime.java | 31 --- .../director-goal-standalone/pom.xml | 224 ++++++++++++++++-- .../tycho/test/P2DirectorPluginTest.java | 91 ++++++- .../plugins/p2/director/DirectorMojo.java | 34 ++- .../surefire/AbstractEclipseTestMojo.java | 6 +- .../ProvisionedInstallationBuilder.java | 4 +- 6 files changed, 325 insertions(+), 65 deletions(-) diff --git a/tycho-core/src/main/java/org/eclipse/tycho/p2/tools/director/shared/DirectorRuntime.java b/tycho-core/src/main/java/org/eclipse/tycho/p2/tools/director/shared/DirectorRuntime.java index f87bdab8a1..b9161d7202 100644 --- a/tycho-core/src/main/java/org/eclipse/tycho/p2/tools/director/shared/DirectorRuntime.java +++ b/tycho-core/src/main/java/org/eclipse/tycho/p2/tools/director/shared/DirectorRuntime.java @@ -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; /** @@ -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//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; - } } diff --git a/tycho-its/projects/tycho-p2-director-plugin/director-goal-standalone/pom.xml b/tycho-its/projects/tycho-p2-director-plugin/director-goal-standalone/pom.xml index 9577921acf..ad63103197 100644 --- a/tycho-its/projects/tycho-p2-director-plugin/director-goal-standalone/pom.xml +++ b/tycho-its/projects/tycho-p2-director-plugin/director-goal-standalone/pom.xml @@ -7,28 +7,206 @@ 0.1.0-SNAPSHOT pom - - - - org.eclipse.tycho - tycho-p2-director-plugin - ${tycho-version} - - - run-director - - director - - package - - ${target-platform} - org.eclipse.osgi - target/dummy - - - - - - + + + director-windows + + + + org.eclipse.tycho + tycho-p2-director-plugin + ${tycho-version} + + + run-director-windows + + director + + package + + ${target-platform} + org.eclipse.osgi + target/productwindows + win32 + win32 + x86_64 + + + + + + + + + director-linux + + + + org.eclipse.tycho + tycho-p2-director-plugin + ${tycho-version} + + + run-director-linux + + director + + package + + ${target-platform} + org.eclipse.osgi + target/productlinux + linux + gtk + x86_64 + + + + + + + + + director-macos-destination-with-app-suffix + + + + org.eclipse.tycho + tycho-p2-director-plugin + ${tycho-version} + + + run-director-macos-destination-with-app-suffix + + director + + package + + ${target-platform} + org.eclipse.osgi + target/productmacos.app + macosx + cocoa + x86_64 + + + + + + + + + director-macos-destination-without-app-suffix + + + + org.eclipse.tycho + tycho-p2-director-plugin + ${tycho-version} + + + run-director-macos-destinationout-with-app-suffix + + director + + package + + ${target-platform} + org.eclipse.osgi + target/productmacos + macosx + cocoa + x86_64 + + + + + + + + + director-macos-destination-with-full-bundle-path + + + + org.eclipse.tycho + tycho-p2-director-plugin + ${tycho-version} + + + run-director-macos-destination-with-full-bundle-path + + director + + package + + ${target-platform} + org.eclipse.osgi + target/productmacos.app/Contents/Eclipse + macosx + cocoa + x86_64 + + + + + + + + + director-running-environment + + + + org.eclipse.tycho + tycho-p2-director-plugin + ${tycho-version} + + + run-director-running-environment + + director + + package + + ${target-platform} + org.eclipse.osgi + target/product + + + + + + + + + director-iconsistent-p2-arguments + + + + org.eclipse.tycho + tycho-p2-director-plugin + ${tycho-version} + + + run-director-iconsistent-p2-arguments + + director + + package + + ${target-platform} + org.eclipse.osgi + target/product + win32 + + + + + + + + + \ No newline at end of file diff --git a/tycho-its/src/test/java/org/eclipse/tycho/test/P2DirectorPluginTest.java b/tycho-its/src/test/java/org/eclipse/tycho/test/P2DirectorPluginTest.java index 9844723f5c..664732f4ac 100644 --- a/tycho-its/src/test/java/org/eclipse/tycho/test/P2DirectorPluginTest.java +++ b/tycho-its/src/test/java/org/eclipse/tycho/test/P2DirectorPluginTest.java @@ -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 + // ... + // 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 + // ... + // 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-environment"); + verifier.executeGoal("package"); + verifier.verifyErrorFreeLog(); + + // to be consistent with the classic + // eclipse -application org.eclipse.equinox.p2.director -destination + // ... + // API, we also do not want to prep-process 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"))); } } diff --git a/tycho-p2-director-plugin/src/main/java/org/eclipse/tycho/plugins/p2/director/DirectorMojo.java b/tycho-p2-director-plugin/src/main/java/org/eclipse/tycho/plugins/p2/director/DirectorMojo.java index 617939ccbf..f2101bb260 100644 --- a/tycho-p2-director-plugin/src/main/java/org/eclipse/tycho/plugins/p2/director/DirectorMojo.java +++ b/tycho-p2-director-plugin/src/main/java/org/eclipse/tycho/plugins/p2/director/DirectorMojo.java @@ -39,12 +39,10 @@ import org.eclipse.equinox.p2.core.IProvisioningAgentProvider; import org.eclipse.equinox.p2.core.ProvisionException; import org.eclipse.equinox.p2.repository.metadata.IMetadataRepositoryManager; -import org.eclipse.tycho.TargetEnvironment; import org.eclipse.tycho.TychoConstants; import org.eclipse.tycho.core.shared.StatusTool; import org.eclipse.tycho.p2.CommandLineArguments; import org.eclipse.tycho.p2.resolver.BundlePublisher; -import org.eclipse.tycho.p2.tools.director.shared.DirectorRuntime; import org.eclipse.tycho.p2maven.tmp.BundlesAction; import org.eclipse.tycho.p2tools.MavenDirectorLog; import org.eclipse.tycho.p2tools.copiedfromp2.DirectorApplication; @@ -101,6 +99,9 @@ public class DirectorMojo extends AbstractMojo { /** * The folder in which the targeted product is located. + *

+ * Note: When targeting {@link #p2os} 'macosx' and this value ends in '.app', the director will + * automatically detect this and append 'Contents/Eclipse' to the path. */ @Parameter(property = "destination", required = true) private File destination; @@ -258,18 +259,32 @@ public class DirectorMojo extends AbstractMojo { /** * The OS to use when the profile is created. + *

+ * If this is specified, {@link #p2ws} and {@link #p2arch} must also be specified for + * consistency. + *

+ * If none of them are specified, the values are derived from the running environment. */ @Parameter(property = "p2.os") private String p2os; /** * The windowing system to use when the profile is created. + *

+ * If this is specified, {@link #p2os} and {@link #p2arch} must also be specified for + * consistency. + *

+ * If none of them are specified, the values are derived from the running environment. */ @Parameter(property = "p2.ws") private String p2ws; /** * The architecture to use when the profile is created. + *

+ * If this is specified, {@link #p2os} and {@link #p2ws} must also be specified for consistency. + *

+ * If none of them are specified, the values are derived from the running environment. */ @Parameter(property = "p2.arch") private String p2arch; @@ -360,8 +375,7 @@ public class DirectorMojo extends AbstractMojo { @Override public void execute() throws MojoExecutionException, MojoFailureException { CommandLineArguments args = new CommandLineArguments(); - args.addNonNull("-destination", - DirectorRuntime.getDestination(destination, TargetEnvironment.getRunningEnvironment())); + args.addNonNull("-destination", destination); args.addNonNull("-metadatarepository", metadatarepositories); args.addNonNull("-artifactrepository", artifactrepositories); args.addNonNull("-repository", getRepositories()); @@ -378,9 +392,15 @@ public void execute() throws MojoExecutionException, MojoFailureException { args.addNonNull("-iuProfileproperties", iuProfileproperties); args.addNonNull("-flavor", flavor); args.addNonNull("-bundlepool", bundlepool); - args.addNonNull("-p2.os", p2os); - args.addNonNull("-p2.ws", p2ws); - args.addNonNull("-p2.arch", p2arch); + if (p2os != null || p2ws != null || p2arch != null) { + if (p2os == null || p2ws == null || p2arch == null) { + throw new MojoFailureException( + "If any of p2os / p2ws / p2arch is specified, all of them must be specified."); + } + args.add("-p2.os", p2os); + args.add("-p2.ws", p2ws); + args.add("-p2.arch", p2arch); + } args.addNonNull("-p2.nl", p2nl); args.addFlagIfTrue("-roaming", roaming); args.addNonNull("-trustedAuthorities", trustedAuthorities); diff --git a/tycho-surefire/tycho-surefire-plugin/src/main/java/org/eclipse/tycho/surefire/AbstractEclipseTestMojo.java b/tycho-surefire/tycho-surefire-plugin/src/main/java/org/eclipse/tycho/surefire/AbstractEclipseTestMojo.java index e357d68ede..ef42bf8c85 100644 --- a/tycho-surefire/tycho-surefire-plugin/src/main/java/org/eclipse/tycho/surefire/AbstractEclipseTestMojo.java +++ b/tycho-surefire/tycho-surefire-plugin/src/main/java/org/eclipse/tycho/surefire/AbstractEclipseTestMojo.java @@ -690,9 +690,13 @@ private EquinoxInstallation createProvisionedInstallation() throws MojoExecution File workingDir = new File(project.getBuild().getDirectory(), "p2temp"); workingDir.mkdirs(); installationBuilder.setWorkingDir(workingDir); - installationBuilder.setDestination(work); List list = getTestTargetEnvironments(); TargetEnvironment testEnvironment = list.get(0); + if (PlatformPropertiesUtils.OS_MACOSX.equals(testEnvironment.getOs()) && !work.getName().endsWith(".app")) { + installationBuilder.setDestination(new File(work, "Eclipse.app/Contents/Eclipse/")); + } else { + installationBuilder.setDestination(work); + } if (installAllEnvironments) { TargetPlatformConfiguration configuration = projectManager.getTargetPlatformConfiguration(project); List targetEnvironments = configuration.getEnvironments(); diff --git a/tycho-surefire/tycho-surefire-plugin/src/main/java/org/eclipse/tycho/surefire/provisioning/ProvisionedInstallationBuilder.java b/tycho-surefire/tycho-surefire-plugin/src/main/java/org/eclipse/tycho/surefire/provisioning/ProvisionedInstallationBuilder.java index 7be66e98b8..3aaf3e80e3 100644 --- a/tycho-surefire/tycho-surefire-plugin/src/main/java/org/eclipse/tycho/surefire/provisioning/ProvisionedInstallationBuilder.java +++ b/tycho-surefire/tycho-surefire-plugin/src/main/java/org/eclipse/tycho/surefire/provisioning/ProvisionedInstallationBuilder.java @@ -99,7 +99,7 @@ public EquinoxInstallation install(TargetEnvironment main) throws Exception { validate(); publishPlainBundleJars(); executeDirector(main); - return new ProvisionedEquinoxInstallation(DirectorRuntime.getDestination(effectiveDestination, main)); + return new ProvisionedEquinoxInstallation(effectiveDestination); } private void publishPlainBundleJars() throws Exception { @@ -126,7 +126,7 @@ private void executeDirector(TargetEnvironment env) throws MojoFailureException for (String iu : ius) { command.addUnitToInstall(iu); } - command.setDestination(DirectorRuntime.getDestination(effectiveDestination, env)); + command.setDestination(effectiveDestination); command.setProfileName(profileName); command.setInstallFeatures(installFeatures); command.setEnvironment(env);