Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always use full-named o.e.equinox.launcher artifact #3860

Conversation

HannesWell
Copy link
Member

For for Mac OS X there was no equinox-launcher fragment for CARBON with full name, i.e. with .. suffix, but only a org.eclipse.equinox.launcher.carbon.macosx fragment. But since Eclipse 4.2, released 2012, CARBON is not supported anymore at all [1]. Therefore the code adjust to that special naming schema, which also imposed difficulties or new supported architectures on Macos should be removed.

See also eclipse-equinox/equinox#622

[1] - https://download.eclipse.org/eclipse/updates/4.2/R-4.2-201206081400

For for Mac OS X there was no equinox-launcher fragment for CARBON with
full name, i.e. with <ws>.<os>.<arch> suffix, but only a
org.eclipse.equinox.launcher.carbon.macosx fragment.
But since Eclipse 4.2, released 2012, CARBON is not supported anymore at
all [1]. Therefore the code adjust to that special naming schema, which
also imposed difficulties or new supported architectures on Macos should
be removed.

See also eclipse-equinox/equinox#622

[1] - https://download.eclipse.org/eclipse/updates/4.2/R-4.2-201206081400
@HannesWell HannesWell added the backport-to-tycho-4.0.x Can be added to a PR to trigger an automatic backport of the change label May 19, 2024
Copy link

Test Results

  594 files    594 suites   4h 4m 26s ⏱️
  422 tests   413 ✅  7 💤 1 ❌ 1 🔥
1 266 runs  1 242 ✅ 22 💤 1 ❌ 1 🔥

For more details on these failures and errors, see this check.

Results for commit d2ffb78.

@laeubi
Copy link
Member

laeubi commented May 20, 2024

@HannesWell before being too much concerned you should check if the method is actually of any use today, as the DepedencyWalker is to be removed and beside one place should be unused anyways.

@HannesWell
Copy link
Member Author

From the callers it looks like only BuildQualifierAggregatorMojo uses it at:

projectType.getDependencyWalker(thisProject).walk(new ArtifactDependencyVisitor() {

While I'm not so much concerned about the qualifier of a feature, I wonder if it would still break a build if the short-named launcher fragment is removed one day? Because without this, when build a product for mac the computation of its qualifier will probably fail, given it uses dependency-walker/visitor:

public void missingPlugin(PluginRef ref, List<ArtifactDescriptor> walkback) {
throw newRuntimeException("Could not resolve plugin", ref.toString(), walkback);
}

But I'm not sure if that is really used.

@HannesWell
Copy link
Member Author

Btw. are the test failures related to this change? They looks somewhat unrelated.

@HannesWell
Copy link
Member Author

HannesWell commented May 20, 2024

Thinking again about the current only usage (determine the qualifier of repositories/products/features), I think we should just remove the special treatment of the equinox-launcher fragments here.
Other parts of Tycho or P2 already determine what's contained in a product so we don't have to replicate that logic here again, IMHO.
@laeubi what do you think about just removing the following code entirely?
This would also help if for example in the future no launcher fragments are required at all anymore.

if (environments != null && product.includeLaunchers()) {
for (TargetEnvironment environment : environments) {
String os = environment.getOs();
String ws = environment.getWs();
String arch = environment.getArch();
String id;
// for Mac OS X there is no org.eclipse.equinox.launcher.carbon.macosx.x86 or org.eclipse.equinox.launcher.carbon.macosx.ppc folder,
// only a org.eclipse.equinox.launcher.carbon.macosx folder.
// see https://jira.codehaus.org/browse/MNGECLIPSE-1075
if (PlatformPropertiesUtils.OS_MACOSX.equals(os) && (PlatformPropertiesUtils.ARCH_X86.equals(arch)
|| PlatformPropertiesUtils.ARCH_PPC.equals(arch))) {
id = "org.eclipse.equinox.launcher." + ws + "." + os;
} else {
id = "org.eclipse.equinox.launcher." + ws + "." + os + "." + arch;
}
if (!bundles.contains(id)) {
PluginRef ref = new PluginRef("plugin");
ref.setId(id);
ref.setOs(os);
ref.setWs(ws);
ref.setArch(arch);
traversePlugin(ref, visitor, visited);
}
}
}

@HannesWell
Copy link
Member Author

@laeubi what do you think about just removing the following code entirely?
This would also help if for example in the future no launcher fragments are required at all anymore.

See #3862. If we decide for the other PR, this one can be closed.

@HannesWell HannesWell closed this May 20, 2024
@HannesWell HannesWell deleted the fully-named-equinox-launchers branch May 20, 2024 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-tycho-4.0.x Can be added to a PR to trigger an automatic backport of the change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants