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.

Sign up for GitHub

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

Make checks for 'feature-source'/'p2-metadata' executions more precise #912

Merged
merged 1 commit into from
Apr 24, 2022
Merged

Make checks for 'feature-source'/'p2-metadata' executions more precise #912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions tycho-source-plugin/src/main/java/org/eclipse/tycho/source/OsgiSourceMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@
/**
* Goal to create a JAR-package containing all the source files of a osgi project.
*/
@Mojo(name = "plugin-source", defaultPhase = LifecyclePhase.PREPARE_PACKAGE, threadSafe = true)
Copy link
Member Author

Choose a reason for hiding this comment

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

Do you think it is in general a reasonable pattern to use a constant as mojo-name (if available)?

Copy link
Member

Choose a reason for hiding this comment

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

I think it doesn't hurt.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you think it would be a good pattern to be applied generally to have a public static final String GOAL field for each Mojo class that could be referenced wherever the goal id is required? It would avoid using 'magic' string literals in the code base.

Copy link
Member

Choose a reason for hiding this comment

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

I think it does not makes much of a difference if it is in a GOAL="..." or in a name = "..." unless we reference it somewhere else (in which case it might be more useful in TychoConstants or a new TychoMojos interface)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure it only makes a difference when the goal-id is used somewhere else.
The good think about such constants is that in Eclipse one can simply check the call-hierarchy to see where the goal is used and does not have to search for the String and sort out all false positive results.

I'm not sure where such constant would be placed best. My first thought was the mojo because it's a property of it.
But I'm not planning to apply this to the entire code base at the moment. So there is some time to keep this in the back of our minds.

Copy link
Member

Choose a reason for hiding this comment

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

The mojo ids are at least used in the compontents.xml and we cant reference the constants there.

@Mojo(name = OsgiSourceMojo.GOAL, defaultPhase = LifecyclePhase.PREPARE_PACKAGE, threadSafe = true)
public class OsgiSourceMojo extends AbstractSourceJarMojo {

private static final String GOAL = "plugin-source";
static final String GOAL = "plugin-source";

static final String MANIFEST_HEADER_ECLIPSE_SOURCE_BUNDLE = "Eclipse-SourceBundle";
private static final String MANIFEST_BUNDLE_LOCALIZATION_BASENAME = BUNDLE_LOCALIZATION_DEFAULT_BASENAME + "-src";
Expand Down
15 changes: 5 additions & 10 deletions tycho-source-plugin/src/main/java/org/eclipse/tycho/source/SourceFeatureMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.apache.maven.archiver.MavenArchiver;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.model.Plugin;
import org.apache.maven.model.PluginExecution;
import org.apache.maven.plugin.AbstractMojo;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugin.MojoFailureException;
Expand Down Expand Up @@ -90,9 +89,11 @@
* <code>&lt;originalFeature&gt;/feature.properties</code>.
*
*/
@Mojo(name = "feature-source", defaultPhase = LifecyclePhase.PACKAGE, threadSafe = true)
@Mojo(name = SourceFeatureMojo.GOAL, defaultPhase = LifecyclePhase.PACKAGE, threadSafe = true)
public class SourceFeatureMojo extends AbstractMojo {

static final String GOAL = "feature-source";

public enum MissingSourcesAction {
FAIL, WARN;
}
Expand Down Expand Up @@ -257,7 +258,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {
projectHelper.attachArtifact(project, outputJarFile, SOURCES_FEATURE_CLASSIFIER);
if (!isP2GenerationEnabled()) {
logger.warn(
"org.eclipse.tycho:tycho-p2-plugin seems not to be enabled but will be required if the generated feature is used in an update-site. You can add the following snippet to your pom: \n" //
"org.eclipse.tycho:tycho-p2-plugin seems not to be enabled but will be required if the generated source-feature is used in an update-site or another feature. You can add the following snippet to your pom: \n" //
+ " <plugin>\n"
+ " <groupId>org.eclipse.tycho</groupId>\n" //
+ " <artifactId>tycho-p2-plugin</artifactId>\n" //
Expand All @@ -282,13 +283,7 @@ public void execute() throws MojoExecutionException, MojoFailureException {

protected boolean isP2GenerationEnabled() {
Plugin plugin = project.getPlugin("org.eclipse.tycho:tycho-p2-plugin");
if (plugin != null) {
PluginExecution execution = plugin.getExecutionsAsMap().get("attach-p2-metadata");
if (execution != null) {
return execution.getGoals().contains("p2-metadata");
}
}
return false;
return plugin != null && plugin.getExecutions().stream().anyMatch(e -> e.getGoals().contains("p2-metadata"));
}

static File getSourcesFeatureOutputDir(MavenProject project) {
Expand Down
3 changes: 2 additions & 1 deletion ...source-plugin/src/main/java/org/eclipse/tycho/source/SourceFeatureP2MetadataProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ public Map<String, IDependencyMetadata> getDependencyMetadata(MavenSession sessi
}
Plugin plugin = project.getPlugin("org.eclipse.tycho:tycho-source-plugin");
if (plugin != null) {
PluginExecution execution = plugin.getExecutionsAsMap().get("feature-source");
PluginExecution execution = plugin.getExecutions().stream()
.filter(e -> e.getGoals().contains(SourceFeatureMojo.GOAL)).findFirst().orElse(null);
if (execution == null) {
return null;
}
Expand Down