Skip to content

[NETBEANS-1074] Module Review libs.glassfish_logging#828

Merged
matthiasblaesing merged 3 commits intoapache:masterfrom
juneau001:review-libs.glassfish-logging
Sep 4, 2018
Merged

[NETBEANS-1074] Module Review libs.glassfish_logging#828
matthiasblaesing merged 3 commits intoapache:masterfrom
juneau001:review-libs.glassfish-logging

Conversation

@juneau001
Copy link
Contributor

  • Added license header to binaries-list
  • Repaired license.txt
  • Scanned module and no other issues found

- Added license header to binaries-list
- Repaired license.txt
- Scanned module and no other issues found
Name: GlassFish Logging
License: CDDL-GPL-2-CP
License: Apache-2.0
OSR: 4101
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove the OSR entry - it is an artifact from the Sun times.

@junichi11
Copy link
Member

I don't know this anything, but why can we change the license to Apache-2.0?

@junichi11 junichi11 changed the title [NETBEANS-1074] Module review libs.glassfish-logging [NETBEANS-1074] Module Review libs.glassfish_logging Sep 1, 2018
@juneau001
Copy link
Contributor Author

I am not quite sure if we should change the license since it was Oracle or not. It was an Oracle license, and they are now transferring GlassFish to Eclipse...so I wasn't quite sure what to do here. Therefore, I submitted the PR to open dialog on this. Just let me know what is the best approach to take and I'll make the modifications. Thanks for your time and assistance!

@junichi11
Copy link
Member

If Oracle donated it to the Apache, we can change the license, I suppose.
I could not find the source code for com.sun.org.apache.commons.logging.

@matthiasblaesing
Should we just choose CDDL-1.0?

@matthiasblaesing
Copy link
Contributor

The ALv2 license looks correct. If you look at the package structure, this is a shaded version of commons logging. I checked glassfish 2.1.1 and indeed, it contains the jar, but that jar correctly holds a LICENSE ant NOTICE file.

That file can also be found on maven central:
https://search.maven.org/artifact/com.sun.commons/logging-api/1.0.4/jar

From an API perspective they are identical to the glassfish-logging-2.0.jar. I suspect, that the version number in the jar refers to the glassfish version that it was contained in.

I suggest we switch to the maven central version, fix the version number and are done. I don't think this artifact is used often, as glassfish is AFAIK currently using java.util.logging.

@junichi11
Copy link
Member

junichi11 commented Sep 2, 2018

My doubt is cleared now. Thanks, Matthias!

@juneau001
Copy link
Contributor Author

Thanks for the assistance, I appreciate it. I will change the version number in the license.txt and remove the OSR entry. Can you please inform me what needs to change in order to switch to the Maven central version of commons logging 1.0.4? I've not modified dependencies on internal NetBeans modules before and I don't want to break the build.

@matthiasblaesing
Copy link
Contributor

No problem:
The first think that needs to be done is switching the reference in the binaries-list. In the beginning of the transition phase the Downloader task was modified to recognize the pattern group:name:version:classifier@extension (classifier + extension are optional). If the pattern is found, the file is downloaded from maven central. See for example libs.amazon/external/binaries-list.

The external files are referenced from the containing project in different ways, as the file name will probably change, references need to be updated. Check project.properties. The properties, that begin with release. specify filemapping from the module into the release. So:

release.external/glassfish-logging-2.0.jar=modules/ext/glassfish-logging-2.0.jar

means, that the file external/glassfish-logging-2.0.jar (left-hand) will end up in <cluster>/modules/ext/glassfish-logging-2.0.jar (right-hand).

You will also want to look in project.xml. The file you are changing might be referenced and in this case it is so:

            <class-path-extension>
                <runtime-relative-path>ext/glassfish-logging-2.0.jar</runtime-relative-path>
                <binary-origin>external/glassfish-logging-2.0.jar</binary-origin>
            </class-path-extension>

I think you are able to deduce where the paths come from (hint: match them to the project.properties).

If other code refers to files, this potentially needs to be adjusted - that needs decisions case-by-case. Grep is your fried :-)

Hope that helps a bit.

@juneau001
Copy link
Contributor Author

Thanks @matthiasblaesing I will give it a try soon. I appreciate the great explanation.

- Changed external library from glassfish-logging-2.0.jar to logging-api-1.0.4.jar
- Modified license to reflect the change from glassfish logging to commons logging
- Modified hash and maven coordinates in binaries-list
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
1821ad7489a0a3d9241ccffdd39fa734ff01305d com.sun.commons:logging-api:1.0.4
Copy link
Contributor

Choose a reason for hiding this comment

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

The SHA1 sums are expected to be in upper case.

@@ -0,0 +1,206 @@
Name: Commons Logging
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not Apache Commons Logging, but a shaded version of it. I would stay with the original name, as it was distributed as part of glassfish, same goes then for the description.

@@ -0,0 +1,206 @@
Name: Commons Logging
Copy link
Contributor

Choose a reason for hiding this comment

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

The name of the file need to be logging-api-1.0.4-license.txt, as the downloaded artifact gets the name logging-api-1.0.4.jar. The prefix of the license text needs to either match the artifact name or the Files header of the license file must explicitly reference the files.

Name: Commons Logging
License: Apache-2.0
Description: Commons Logging
Version: 1.0.4
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a Origin header - it is required. I would let it be: Distributed with Sun GlassFish Enterprise Server v2.1. You can run license verification by invoking: ant verify-all-libs-and-licenses and look for the module you are checking.

- Modified SHA1 sum to upper case in binaries-list
- Renamed commons-logging-1.0.4-license.txt to logging-api-1.0.4-license.txt
- Added Origin header to logging-api-1.0.4-license.txt and modified name and description
@juneau001
Copy link
Contributor Author

Thanks for the feedback and very useful information. I've modified as requested.

@matthiasblaesing matthiasblaesing merged commit 3505422 into apache:master Sep 4, 2018
@matthiasblaesing
Copy link
Contributor

Merged - thank you!

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