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 configure JRE container to be the highest environment from #843

Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.stream.Collectors;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -172,7 +174,8 @@ public void configure(ProjectConfigurationRequest request, IProgressMonitor moni

String executionEnvironmentId = getExecutionEnvironmentId(options);
String buildEnvironmentId = getMinimumJavaBuildEnvironmentId(request, monitor);
addJREClasspathContainer(classpath, buildEnvironmentId != null ? buildEnvironmentId : executionEnvironmentId);
// use the higher environment id as JREs are backwards compatible
addJREClasspathContainer(classpath, getHighestEnvironmentId(buildEnvironmentId, executionEnvironmentId));

addMavenClasspathContainer(classpath);

Expand All @@ -198,6 +201,24 @@ public void configure(ProjectConfigurationRequest request, IProgressMonitor moni
MavenJdtPlugin.getDefault().getBuildpathManager().updateClasspath(project, monitor);
}

static String getHighestEnvironmentId(String... environmentIds) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where could I put a unit test for this (no IT necessary, no bundling into an Eclipse plugin either)? m2e.jdt.tests also only contains package o.e.m.jdt.tests and this test would need to be located in the same package.

Copy link
Member Author

@kwin kwin Jul 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach from https://stackoverflow.com/a/44926613 seems reasonable to me but as only the reactor has a pom.xml one would need to generally enable that....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

o.e.m.jdt.tests would be the best place. As the class is internal you could make the method static.
Another approach, that m2e is not leveraging yet so it may require more work to get started, is https://github.com/eclipse/tycho/blob/master/RELEASE_NOTES.md#execute-unit-tests-with-eclipse-plugin-packaging

List<String> environmentIdList = Arrays.stream(environmentIds).filter(Objects::nonNull)
.collect(Collectors.toList());
if(environmentIdList.size() == 1) {
return environmentIdList.get(0);
}
// ENVIRONMENTS map is sorted from lowest first to highest last
List<String> sortedExecutionEnvironmentIds = new ArrayList<>(ENVIRONMENTS.values());
Collections.reverse(sortedExecutionEnvironmentIds); // reverse order to see highest id first
for(String executionEnvironmentId : sortedExecutionEnvironmentIds) {
if(environmentIdList.contains(executionEnvironmentId)) {
return executionEnvironmentId;
}
}
throw new IllegalArgumentException(
"None of the environment ids from '" + String.join(",", environmentIds) + "' found in ENVIRONMENTS");
}

protected IContainer getOutputLocation(ProjectConfigurationRequest request, IProject project) {
MavenProject mavenProject = request.mavenProject();
return getFolder(project, mavenProject.getBuild().getOutputDirectory());
Expand Down