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

Add additional plugin manifest entries #436

Merged
merged 6 commits into from
Apr 12, 2023
Merged

Conversation

jtnord
Copy link
Member

@jtnord jtnord commented Feb 17, 2023

Adds the following Manifest entries

Artifact-Id

whilst there was Short-Name that used artifactId it could be changed (however unlikely) and it is best to have something that won't ever change.
Could have been obtained from the pom.xml or pom.properties - but that requires you to go looking

Plugin-ScmConnection

currently the PCT is forcing plugins to specify this in mulit module projects which is convoluted.
Also moving here could simplify in the future by removing the need to parse any poms.

Plugin-ScmTag

a tag as specified by ${project.scm.tag} which may be HEAD if the project is is neither a release nor an incremental version.

Plugin-GitHash

put the Git SHA used for the HEAD of the plugin build.
Allows the (best effort) recording of a commit used for a SNAPSHOT build. (no longer needs to be extracted from the private build information)

Plugin-Module

the path of the project relative to the git project.

Regardless of any future PCT work to use these additions Plugin-Scm-Git-Hash is useful when looking at a deployed SNAPSHOT today and the others do no harm.

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

Adds the following Manifest entries

Artifact-Id : whilst there was Short-Name that used artifactId it could
	be changed and it is best to have somrthing that wont ever change.
	Could have been obtained from the pom.xml or pom.properties - but
	that requires you to go looking
Plugin-Scm-Connection : currently the PCT is forcing plugins to specify
	this in mulit module projects which is convoluted.  Also moving here can
	simplify in the future by removing the need to parse any poms.
Plugin-Scm-Git-Hash :  put the Git SHA used for the HEAD of the plugin
	build. Allows the recording of a commit for a SNAPSHOT build.
Plugin-Scm-Git-Module-Path : the path of the project relative to the git
	project.
public String getGitHeadSha1() {
// we want to allow the plugin that's not sitting at the root (such as findbugs plugin),
// but we don't want to go up too far and pick up unrelated repository.
File git = new File(project.getBasedir(), ".git");
Copy link
Member Author

Choose a reason for hiding this comment

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

this was questionable to begin with.
it implies that

  1. you can only ever have a 1 level structure
  2. it would be valid and useful to checkout a non git project into a clone of a git projects directory structure (but only at 2 levels or lower!)

}

protected String getGitRevParseOutput(String revparseArg) {
if (!project.getScm().getConnection().startsWith("scm:git")) {
Copy link
Member Author

Choose a reason for hiding this comment

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

rather than looking for a .git folder in the hierachy we now just ask the project if its connection starts with scm:git

This does not need to be set for every plugin in a reactor, as the model we have contains inherited data.

try {
Process p = new ProcessBuilder("git", "-C", git.getAbsolutePath(), "rev-parse", "HEAD").redirectErrorStream(true).start();
Process p = new ProcessBuilder("git", "rev-parse", revparseArg).directory(project.getBasedir()).redirectErrorStream(true).start();
Copy link
Member Author

Choose a reason for hiding this comment

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

switch from -C thePath to running git in the actual path.

@@ -124,6 +124,8 @@ protected void setAttributes(Manifest.ExistingSection mainSection) throws MojoEx
}

mainSection.addAttributeAndCheck(new Manifest.Attribute("Group-Id",project.getGroupId()));
mainSection.addAttributeAndCheck(new Manifest.Attribute("Artifact-Id",project.getArtifactId()));
Copy link
Member Author

Choose a reason for hiding this comment

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

There is no harm in adding this value explicitly.

addAttributeIfNotNull(mainSection, "Plugin-ScmUrl", getScmUrl());
addAttributeIfNotNull(mainSection, "Plugin-Scm-Connection", getSCMConnection());
Copy link
Member Author

Choose a reason for hiding this comment

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

so that it could be obtained in the future without projects having to pollute their own pom.

jenkinsci/plugin-compat-tester#453

addAttributeIfNotNull(mainSection, "Plugin-ScmUrl", getScmUrl());
addAttributeIfNotNull(mainSection, "Plugin-Scm-Connection", getSCMConnection());
addAttributeIfNotNull(mainSection, "Plugin-Scm-Git-Hash", getGitHeadSha1(false));
Copy link
Member Author

Choose a reason for hiding this comment

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

useful for tracking SNAPSHOT deployments.

Possibly helpful for testing SNAPSHOT builds in the PCT.

addAttributeIfNotNull(mainSection, "Plugin-ScmUrl", getScmUrl());
addAttributeIfNotNull(mainSection, "Plugin-Scm-Connection", getSCMConnection());
addAttributeIfNotNull(mainSection, "Plugin-Scm-Git-Hash", getGitHeadSha1(false));
addAttributeIfNotNull(mainSection, "Plugin-Scm-Git-Module-Path", getGitModulePath());
Copy link
Member Author

Choose a reason for hiding this comment

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

does no harm as is.

// normalize to Unix style
return path.replace(File.separatorChar, '/');
} catch (IOException e) {
getLog().warn("failed to obtain projects relative location in the Git repository", e);
Copy link
Member Author

Choose a reason for hiding this comment

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

goes against the fine level if git rev-parse HEAD fails in the existing code - but really this should not happen - we are a git project.

Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

For new APIs and extension points, there should be a link to at least one consumer.

@daniel-beck daniel-beck changed the title Add additional plugin manifiest entries Add additional plugin manifest entries Feb 21, 2023
@jtnord jtnord marked this pull request as draft March 3, 2023 17:32
jtnord added a commit to jtnord/aws-java-sdk-plugin that referenced this pull request Mar 24, 2023
testing jenkinsci/maven-hpi-plugin#436 to add additional manifest entries
@basil basil marked this pull request as ready for review April 12, 2023 02:46
Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

I simplified this considerably.

@basil basil merged commit 94061cd into jenkinsci:master Apr 12, 2023
basil added a commit to jtnord/plugin-compat-tester that referenced this pull request Apr 12, 2023
jtnord added a commit to jenkinsci/plugin-compat-tester that referenced this pull request Apr 17, 2023
This changes the way the PCT clones from per plugin to per repository.

Once the repository is cloned it then exercises each plugin in the repo in turn (serially).
plugins that are excluded (or implicitly if an include list is provided) will not be tested even if some other plugin in the same repository is tested. It is assumed that all plugins in the same repository will use the same tag (however this is not explicitly prevented - but can be if deemed necessary)

The pct command is no longer a root command but is now a sub command test-plugins and any caller of the PCT will need to be adjusted to use this sub command.

There is a new command list-plugins that will list all plugins in the war (excluding detached) that can also filter by includes or excludes.

Tooling can call this in order to group plugin tests for the same repository. (as above this does not group by repository and tag - it is assumed that the version of plugins from the same repository will be the same - this can be added if required, or we can group by repository and commit)

All AbstractMultiParentHook have been removed and similar to the git url workaround has been inlined into code that should become dead once all plugins have been updated to include jenkinsci/maven-hpi-plugin#436
The support is currently via a hook like MetadataExtractor - all implkementations of this are expected to be removed when plugins are migrated, simplifying the implementation.

---------

Co-authored-by: Basil Crow <me@basilcrow.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants