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

Upgrade metadata #7

Closed
wants to merge 2 commits into from
Closed

Upgrade metadata #7

wants to merge 2 commits into from

Conversation

timja
Copy link
Member

@timja timja commented Jan 26, 2021

cc @jglick @oleg-nenashev

also is there a reason this is in Cloudbees org? seems very specific to Jenkins

Tested with Jenkins core, all seems the same except some minor whitespace difference in XML, and displayed in core all looks fine

@@ -2,9 +2,10 @@
<modelVersion>4.0.0</modelVersion>

<parent>
<groupId>org.sonatype.oss</groupId>
<artifactId>oss-parent</artifactId>
Copy link
Member Author

Choose a reason for hiding this comment

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

pom.xml Outdated

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<sonatypeOssDistMgmtSnapshotsUrl>https://oss.sonatype.org/content/repositories/snapshots/</sonatypeOssDistMgmtSnapshotsUrl>
Copy link
Member Author

Choose a reason for hiding this comment

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

I basically inlined most of what was in the last oss-parent release

Copy link
Member

Choose a reason for hiding this comment

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

Well if we are going to use the Jenkins parent then delete this.

<groupId>org.jenkins-ci</groupId>
<artifactId>jenkins</artifactId>
<version>1.62</version>
<relativePath />
Copy link
Member

Choose a reason for hiding this comment

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

This will cease to publish to OSSRH and thus to Central. Probably OK for our purposes? I am not exactly sure what this plugin is for. If it is to appear in a plugin POM, then using our Artifactory only would be OK, since pluginRepositories should be defined.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the thing that extracts licenses from dependencies to show on Jenkins about page: http://localhost:8080/jenkins/about/

it allows running groovy to filter licenses / choose one etc:

https://github.com/jenkinsci/jenkins/blob/e7ca5dbc4473206b7e90f8ea612493a04c57817c/war/pom.xml#L441
https://github.com/jenkinsci/jenkins/blob/master/licenseCompleter.groovy

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 will cease to publish to OSSRH and thus to Central

yeah sounds fine

@@ -13,17 +14,20 @@
<packaging>maven-plugin</packaging>

<name>maven-license-plugin Maven Plugin</name>
<url>http://maven.apache.org</url>
<url>https://maven.apache.org</url>
Copy link
Member

Choose a reason for hiding this comment

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

???

Copy link
Member Author

Choose a reason for hiding this comment

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

well one step better than before =/ this plugin has no docs

pom.xml Outdated

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<sonatypeOssDistMgmtSnapshotsUrl>https://oss.sonatype.org/content/repositories/snapshots/</sonatypeOssDistMgmtSnapshotsUrl>
Copy link
Member

Choose a reason for hiding this comment

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

Well if we are going to use the Jenkins parent then delete this.

<repositories>
<repository>
<id>sonatype-nexus-snapshots</id>
<name>Sonatype Nexus Snapshots</name>
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@@ -104,7 +104,6 @@ public void accept(String name) {
return;
}
}
IllegalStateException error = new IllegalStateException("Expecting " + name + " but found " + toString(licenses) + " for dependency " + toString(dependency));
Copy link
Member

Choose a reason for hiding this comment

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

Just cleanup?

Copy link
Member Author

Choose a reason for hiding this comment

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

it didn't do anything, spotbugs marked this as a 'high' bug

comp.add((LicenseScript) shell.parse(getClass().getResourceAsStream("xmlgen.groovy"),"xmlgen.groovy"));
if (generateLicenseXml!=null) {
try {
comp.add((LicenseScript) shell.parse(getClass().getResource("xmlgen.groovy").toURI()));
Copy link
Member

Choose a reason for hiding this comment

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

Is using getResource the fix here?

Copy link
Member Author

Choose a reason for hiding this comment

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

the method parameters changed, InputStream isn't valid anymore, I chose the URI method

@jglick
Copy link
Member

jglick commented Jan 26, 2021

is there a reason this is in Cloudbees org?

Not sure.

Where did you come across this plugin?

@timja
Copy link
Member Author

timja commented Jan 26, 2021

is there a reason this is in Cloudbees org?

Not sure.

Where did you come across this plugin?

Jenkins core, every time I compile I get multiple IllegalReflectiveAccess warnings because of this plugin -.-

@timja
Copy link
Member Author

timja commented Jan 26, 2021

So should I just remove all sonatype and publish to Jenkins artifactory?

@jglick
Copy link
Member

jglick commented Jan 26, 2021

Seems to be used mainly from https://github.com/jenkinsci/pom/blob/b39188baf69a4c4a2d8dadfee2281ddedd9190b5/pom.xml#L593-L597 + https://github.com/jenkinsci/plugin-pom/blob/655257b6dc9a14b44e096ed5af8d142711068069/pom.xml#L418-L422. The risk is that someone outside the Jenkins org was using it, in which case they will cease to see updates if we switch to Artifactory. I am inclined to try to get it transferred to @jenkinsci though I am not sure who owns it (I have write access only). Seems to be Apache-licensed though it is unclear. Will ask around.

@timja
Copy link
Member Author

timja commented Jan 29, 2021

Will ask around.

Did you get anywhere @jglick?

@jglick
Copy link
Member

jglick commented Jan 29, 2021

Not really.

Can you split out the actual Groovy update into a minimal PR I can release safely?

@timja
Copy link
Member Author

timja commented Jan 29, 2021

Will try do something less invasive, think the Pom needed updates for the new groovy version or maven on my machine

@jglick
Copy link
Member

jglick commented Jan 29, 2021

Mm, OK. Would just prefer something I can continue to release to OSSRH. Typically we use https://github.com/kohsuke/pom for that; not sure why this one is using org.sonatype.oss:oss-parent directly.

@timja
Copy link
Member Author

timja commented Jan 29, 2021

Because it’s ancient 😂

@timja
Copy link
Member Author

timja commented Jan 29, 2021

#8

@jglick jglick changed the title Upgrade groovy Upgrade metadata Jan 30, 2021
@timja timja closed this Jan 30, 2021
@timja timja deleted the upgrade-groovy branch January 30, 2021 07:56
@jglick
Copy link
Member

jglick commented Feb 23, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants