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

TCR-46 mod-linked-data #83

Merged

Conversation

maccabeelevine
Copy link
Member

No description provided.

@craigmcnally craigmcnally changed the title Evaluate TCR-46 mod-linked-data TCR-46 mod-linked-data Dec 16, 2024
Comment on lines 32 to 35
- Three new library dependencies, these are also in the TCR process currently:
- lib-linked-data-dictionary
- lib-linked-data-fingerprint
- lib-linked-data-marc4ld
Copy link
Contributor

Choose a reason for hiding this comment

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

A library dependency is not a FOLIO interface.

A FOLIO interface is a set of API endpoints. Examples: https://dev.folio.org/reference/api/endpoints/

Required and optional interfaces have to be declared in the "requires" and "optional" section of the module descriptor. Example: https://github.com/folio-org/mod-linked-data/blob/master/descriptors/ModuleDescriptor-template.json#L163-L176

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a fair point, you are correct. But I think the intent of this criterion and the one directly above (about Officially Supported Technologies) is to avoid approving things with new, unapproved dependencies. Which the dev team acknowledges in their submission that these libraries are.

Dependent on: lib-linked-data-marc4ld, lib-linked-data-fingerprint and lib-linked-data-dictionary

Perhaps we need to tweak this criterion (or else to add a new criterion) about dependent libraries also having to be already approved.

In practice, we tend to review backend modules before paired front-end modules to avoid this specific timing situation. I think we should just review backend libraries before paired backend modules, and then we avoid the same issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

A module can use any software library, I don't see why only approved FOLIO libraries can be used whereas non-FOLIO libraries can always be used. Software developers can simply fork a non-approved FOLIO library to circumvent this restriction.

Copy link
Member Author

Choose a reason for hiding this comment

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

A module can use any software library, I don't see why only approved FOLIO libraries can be used whereas non-FOLIO libraries can always be used. Software developers can simply fork a non-approved FOLIO library to circumvent this restriction.

This is a fair criticism of our current process, but I'm not sure what solution you are suggesting. If we reverse the decision to review & approve FOLIO libraries, then we incentivize developers to put anything controversial into a library. So I don't think we should do that. If you are suggesting that we treat non-FOLIO libraries like any other third-party tech to consider (as in the OST process), that's going to be a big conversation :-) I'm guessing you have a better suggestion.

If you would like to do a suggested edit for TCR process improvements below, I would be happy to merge it into the PR.

Copy link
Member

Choose a reason for hiding this comment

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

A module can use any software library, I don't see why only approved FOLIO libraries can be used whereas non-FOLIO libraries can always be used

My understanding of this is slightly different

The officially supported technologies page includes non-FOLIO libraries. This was intended to reflect decisions where the community had chosen a particular tool, e.g. JUnit for java based unit testing, and it was expected that folks should choose to use that for their FOLIO modules

This was a compromise between expecting development teams to seek approval for every new third party dependency and trying to constrain the choices to a degree. Much like many areas of FOLIO policies, it relies on good faith actions by the folks involved

* [X] Uses Apache 2.0 license
* [X] Module build MUST produce a valid module descriptor
* [X] Module descriptor MUST include interface requirements for all consumed APIs
* [X] Third party dependencies use an Apache 2.0 compatible license
Copy link
Contributor

Choose a reason for hiding this comment

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

List of third party dependencies: https://julianladisch.github.io/folio-dependencies/mod-linked-data-1.0.1-SNAPSHOT.html
Can you elaborate how you came to the conclusion that they are compatible?

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 used one of those npx tools to generate the list of dependencies, and then compared it against the list at https://apache.org/legal/resolved.html, and there were no issues. This was a month ago so I don't remember the specifics of each license used.

Copy link
Contributor

Choose a reason for hiding this comment

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

npx is for JavaScript, not for maven pom.xml.
Can you specify which tool you've used?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for the catch -- my not noticing that the method we use for UI modules would not apply.

As a different approach, seeing we already use MojoHaus plugins, I added the license-maven-plugin to the pom

      <plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>license-maven-plugin</artifactId>
        <configuration>
          <outputDirectory>${project.build.directory}/reports</outputDirectory>
        </configuration>
      </plugin>

Then executed the third-party-report goal which generated an HTML page with the results -- 321 total dependencies. The HTML is verbose (no consolidated list of licenses), so I ran this cludgy one-liner on in the page's dev console to generated a sorted, unique list of the licenses.

new Set(Array.from(document.querySelectorAll('#third-parties-list + table > tbody > tr > td:last-child')).map((a) => a.innerText).sort())

The resulting list of 46 "unique" licenses (really a lot fewer, things are spelled differently) is

    "Apache 2",
    "Apache 2.0",
    "Apache License 2.0",
    "Apache License 2.0\nLGPL 2.1\nMPL 1.1",
    "Apache License, 2.0\nBSD 2-Clause\nEDL 1.0\nEPL 2.0\nGPL2 w/ CPE\nMIT license\nModified BSD\nPublic Domain\nW3C license\njQuery license",
    "Apache License, 2.0\nEPL 2.0\nPublic Domain\nThe GNU General Public License (GPL), Version 2, With Classpath Exception",
    "Apache License, 2.0\nEPL 2.0\nThe GNU General Public License (GPL), Version 2, With Classpath Exception",
    "Apache License, Version 2.0",
    "Apache Public License 2.0",
    "Apache Software License, version 2.0\nLesser General Public License, version 3 or greater",
    "Apache-2.0",
    "Apache-2.0\nLGPL-2.1-or-later",
    "BSD",
    "BSD 2-Clause License",
    "BSD License 3",
    "BSD-2-Clause",
    "BSD-2-Clause\nPublic Domain, per Creative Commons CC0",
    "BSD-3-Clause",
    "Bouncy Castle Licence",
    "CDDL\nGPLv2+CE",
    "CDDL 1.1\nGPL2 w/ CPE",
    "EDL 1.0",
    "EPL 2.0\nGPL2 w/ CPE",
    "Eclipse Distribution License - v 1.0",
    "Eclipse Distribution License v. 1.0\nEclipse Public License v. 2.0",
    "Eclipse Public License - v 1.0\nThe Apache Software License, Version 2.0",
    "Eclipse Public License - v 2.0",
    "Eclipse Public License - v 2.0\nThe Apache Software License, Version 2.0",
    "Eclipse Public License 1.0",
    "Eclipse Public License v. 2.0\nGNU General Public License, version 2 with the GNU Classpath Exception",
    "Eclipse Public License v2.0",
    "Eclipse Public License, Version 2.0",
    "GNU Lesser General Public License, Version 2.1",
    "GNU Library General Public License v2.1 or later",
    "LGPL",
    "MIT",
    "MIT License",
    "Mozilla Public License, Version 2.0",
    "Public Domain",
    "Public Domain, per Creative Commons CC0",
    "The (New) BSD License",
    "The 2-Clause BSD License",
    "The Apache License, Version 2.0",
    "The Apache Software License, Version 2.0",
    "The MIT License",
    "The MIT License (MIT)"

Comparing this list to https://apache.org/legal/resolved.html, the only questions I see would be

  • Category B licenses - ok if labelled
    • CDDL
    • Eclipse
    • Mozilla
  • EDL -- what's that? But it's used by jakarta.activation so probably every module?
  • GNU LGPL - prohibited in the apache document, but almost every dependency licensed under LGPL is also separately licensed under something above. The exception is org.z3950.zing:cql-java:1.13

Copy link
Contributor

Choose a reason for hiding this comment

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

EDL -- what's that?

Please click on the link I've posted on 2024-12-16.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi Julian, that's a great analysis, thank you.

@maccabeelevine maccabeelevine marked this pull request as ready for review January 3, 2025 22:44
@maccabeelevine maccabeelevine requested a review from a team as a code owner January 3, 2025 22:44
@maccabeelevine maccabeelevine merged commit 4eb4664 into folio-org:master Jan 20, 2025
1 check passed
@maccabeelevine maccabeelevine deleted the TCR-46-mod-linked-data branch January 20, 2025 18:12
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.

3 participants