Skip to content
This repository has been archived by the owner on Jan 18, 2021. It is now read-only.

Make license used for pom configurable #760

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make license used for pom configurable #760

wants to merge 1 commit into from

Conversation

epeee
Copy link
Contributor

@epeee epeee commented Sep 26, 2018

Very first draft (#755).

This pr extends shipkit configuration in a way that we have to configure the license used (incl url now).

shipkit {
    ...

    licenseInfo.license = "Eclipse Public License v2.0"
    licenseInfo.url = "http://www.eclipse.org/legal/epl-v20.html"
}

This will end up in a pom like

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  ...
  <licenses>
    <license>
      <name>Eclipse Public License v2.0</name>
      <url>http://www.eclipse.org/legal/epl-v20.html</url>
      <distribution>repo</distribution>
    </license>
  </licenses>
  ...

The downside of this approach is that we still have to configure the license used by gradle bintray plugin, e.g.:

        bintray {
            pkg {
                ...
                licenses = ['EPL-2.0']
        ...

We could add this one also to shipkit config and do the configuration in our gradle plugin. As a result we'd have the license config in one place.

Note: this is a breaking change and will cause builds without a shipkit.licenseInfo configuration to fail:

Execution failed for task ':api:generatePomFileForJavaLibraryPublication'.
> Could not apply withXml() to generated POM
   > Please configure 'shipkit.licenseInfo.license' value (String).

```

@mstachniuk
Copy link
Contributor

@epeee Maybe you can read somehow Bintray plugin property to avoid duplication?
@mockitoguy any hint?

@jpkrohling
Copy link
Contributor

Any updates on this one?

@jpkrohling
Copy link
Contributor

Bump?

@mockitoguy
Copy link
Member

Looking at it. I met @epeee at CodeOne conference and he mentioned that his PR needs bump. @jpkrohling, sorry for slow turnaround! I'm on it.

@mockitoguy
Copy link
Member

Hey @epeee, I will review and provide my feedback shortly. In the meantime, I commented on the ticket so that @jpkrohling is unblocked and can customize the pom.

@mstachniuk
Copy link
Contributor

I found a hack how to get license info from bintray configuration. @epeee @mockitoguy WDYT?

        Object bintray = project.getExtensions().findByName("bintray");

        try {
            Method getPkg = bintray.getClass().getDeclaredMethod("getPkg");
            getPkg.setAccessible(true);
            Object pkg = getPkg.invoke(bintray);
            Field licensesField = pkg.getClass().getDeclaredField("licenses");
            licensesField.setAccessible(true);
            String[] licenses = (String[]) licensesField.get(pkg);
            for (String license : licenses) {
                LOG.lifecycle(" lic " + license);
            }
        } catch (Exception e) {
            e.printStackTrace();
        }

@mockitoguy
Copy link
Member

I found a hack how to get license info from bintray configuration

Please no :) That's a lot of complexity for little benefit.

@mstachniuk
Copy link
Contributor

It's a simple reflection ;-) But as you wish!

Copy link
Member

@mockitoguy mockitoguy left a comment

Choose a reason for hiding this comment

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

I like it. Let's update the defaults and ship it.

@@ -76,8 +76,8 @@ static void customizePom(Node root, ShipkitConfiguration conf,
}

Node license = root.appendNode("licenses").appendNode("license");
license.appendNode("name", "The MIT License");
license.appendNode("url", repoLink + "/blob/master/LICENSE");
license.appendNode("name", conf.getLicenseInfo().getLicense());
Copy link
Member

Choose a reason for hiding this comment

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

Can we add defaults, Apache 2.0 and blob/master/LICENSE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, good feedback, thx!

Copy link
Member

Choose a reason for hiding this comment

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

Cool. Let me know when you're ready.

MIT is 30y old and lawyers no longer like it. Let's default to Apache 2.0.

@mockitoguy
Copy link
Member

Once we merge it, can we get the license value populated to the Bintray extension, too? Thanks!!! Nice change!

@mockitoguy
Copy link
Member

Before merging, we need to add defaults (as my comment) or make it "lenient". I suggest to add defaults because it is simpler and more useful. Then we can merge :) Let me know when you're ready!

Copy link
Contributor

@mstachniuk mstachniuk left a comment

Choose a reason for hiding this comment

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

See my and Szczepan's comments. For me, it's not ready yet.

@@ -231,6 +236,24 @@ public void setWriteAuthToken(String writeAuthToken) {
}
}

public class LicenseInfo {
Copy link
Contributor

Choose a reason for hiding this comment

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

what about distribution property in pom?

@@ -95,6 +96,10 @@ public ReleaseNotes getReleaseNotes() {
return releaseNotes;
}

public LicenseInfo getLicenseInfo() {
Copy link
Contributor

Choose a reason for hiding this comment

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

How to now configure a few licenses? E.g. one open source and one commercial?

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

Successfully merging this pull request may close these issues.

4 participants