Repository: netty/netty
Issue: Netty NOTICE doesn't cover Apache Harmony's notice
Labels: cleanup

... which is required by the ASLv2. 

https://www.apache.org/licenses/LICENSE-2.0 (section 4d.)



Issue comments:
@ivankelly ups... this most be an oversight... you want to do a PR ?
Can do, though it would probably be better for the person who originally pulled the harmony code to do so, so that you know what to bubble up. The original change was by @trustin (https://github.com/netty/netty/commit/681d46). From what I can tell it came from one of the last harmony releases, but I'm just guessing. 
@ivankelly what do you mean by it doesn't cover the notice?
From ASLv2 4d

> If the Work includes a "NOTICE" text file as part of its distribution, then any Derivative Works that You distribute must include a readable copy of the attribution notices contained within such NOTICE file, excluding those notices that do not pertain to any part of the Derivative Works, in at least one of the following places: within a NOTICE text file distributed as part of the Derivative Works; within the Source form or documentation, if provided along with the Derivative Works; or, within a display generated by the Derivative Works, if and wherever such third-party notices normally appear. The contents of the NOTICE file are for informational purposes only and do not modify the License. You may add Your own attribution notices within Derivative Works that You distribute, alongside or as an addendum to the NOTICE text from the Work, provided that such additional attribution notices cannot be construed as modifying the License. 

Apache harmony has a NOTICE file (as all ASF projects have), and as such this must be included as a readable copy in the derivative work (netty).
@ivankelly Does this mean this issue will be resolved once we add Harmony's NOTICE file somewhere in our repository? Do we also need to include that file in JAR files?
@trustin I don't think you need, nor should, pull in the whole notice file. From ASLv2 4d, "excluding those notices that do not pertain to any part of the Derivative Works". The parts you pulled into AsciiString seem to be only from java.lang.String, which is code which IBM contributed to harmony, but they assigned copyright to ASF when they did so. I had to dig into this when getting our own NOTICE up to scratch, which is when I found this issue.
@trustin but this also depends on where you pulled the harmony code from. Do you remember?
@ivankelly I think I pulled it in from the official repository. It was a very long time ago, though. Looking into the NOTICE file at: http://svn.apache.org/viewvc/harmony/enhanced/java/trunk/NOTICE?view=markup

> The parts you pulled into AsciiString seem to be only from java.lang.String, which is code which IBM contributed to harmony,

Yes, that's correct.

> but they assigned copyright to ASF when they did so. I had to dig into this when getting our own NOTICE up to scratch, which is when I found this issue.

I'm not really sure how our 'partial NOTICE file' has to look like, then. What about this:

```
Apache Harmony

Copyright 2006, 2010 The Apache Software Foundation.

This product includes software developed at
The Apache Software Foundation (http://www.apache.org/).

(C) Copyright 1997,2004 International Business Machines Corporation.
All rights reserved.

(C) Copyright IBM Corp. 2003. 
```

.. or should I remove the IBM part since IBM assigned copyright to the ASF when they contribute? If IBM part can be removed, this leaves merely the first six lines which seem redundant to me. Do I still have to keep this file?

@trustin AFAIK, regarding the code pulled in, the IBM lines are not needed. 

why do you say that the first 6 lines are redundant? It's informing the end user that the project contains code which is copyrighted to the ASF
Note that AsciiString has been reworked (IIUC it was removed and brought back) since its original introduction in Netty. @trustin - can you verify if the attribution is still necessary?
@Scottmitch possibly, though the latest asciistring code contains some methods which are strikingly similar to harmony, so i think the need for attribution still stands.
> why do you say that the first 6 lines are redundant?

Because we mentioned that in our NOTICE.txt file although it's not verbatim. I can add it anyway, though. Will send a PR this week.
Are you sure? I don't see it.
@ivankelly https://github.com/netty/netty/blob/4.1/NOTICE.txt#L61 It's just an implicit mention, so let me just make it clear by adding the file.
@ivankelly - out of curiosity can you point out which methods?
@trustin ah, ya, i think that's a little too implicit :)
@Scottmitch 
one is "int indexOf(CharSequence subString, int start)"

From apache-harmony-6.0-src-r991881 String.java:
```
   public int indexOf(String subString, int start) {
        if (start < 0) {
            start = 0;
        }
        int subCount = subString.count;
        if (subCount > 0) {
            if (subCount + start > count) {
                return -1;
            }
            char[] target = subString.value;
            int subOffset = subString.offset;
            char firstChar = target[subOffset];
            int end = subOffset + subCount;
            while (true) {
                int i = indexOf(firstChar, start);
                if (i == -1 || subCount + i > count) {
                    return -1; // handles subCount > count || start >= count                                                                                                                                                                                                              
                }
                int o1 = offset + i, o2 = subOffset;
                while (++o2 < end && value[++o1] == target[o2]) {
                    // Intentionally empty                                                                                                                                                                                                                                                
                }
                if (o2 == end) {
                    return i;
                }
                start = i + 1;
            }
        }
        return start < count ? start : count;
    }
```

From AsciiString.java:
```
    public int indexOf(CharSequence subString, int start) {
        if (start < 0) {
            start = 0;
        }

        final byte[] value = this.value;
        final int thisLen = value.length;

        int subCount = subString.length();
        if (subCount <= 0) {
            return start < thisLen ? start : thisLen;
        }
        if (subCount > thisLen - start) {
            return -1;
        }

        char firstChar = subString.charAt(0);
        for (;;) {
            int i = indexOf(firstChar, start);
            if (i == -1 || subCount + i > thisLen) {
                return -1; // handles subCount > count || start >= count                                                                                                                                                                                                                  
            }
            int o1 = i, o2 = 0;
            while (++o2 < subCount && (value[++o1] & 0xFF) == subString.charAt(o2)) {
                // Intentionally empty                                                                                                                                                                                                                                                    
            }
            if (o2 == subCount) {
                return i;
            }
            start = i + 1;
        }
    }
```

The comments are a giveaway
tbh it looks like the code was rewritten (with flow changes and improvements, not just changing variable names, the produced bytecode varies greatly) the comments could easily be rewritten too if that's a problem.
@johnou It's still a derivative work.
we should make what ever attribution to Harmony (despite the rework) is required in the NOTICE file.

@trustin - can you take care?
👍

> Am 18.01.2018 um 19:35 schrieb Scott Mitchell <notifications@github.com>:
> 
> we should make what ever attribution to Harmony (despite the rework) is required in the NOTICE file.
> 
> @trustin - can you take care?
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 

@trustin poke

Pull request review comments:
Would be more accurate to put it under a NOTICE: header, but this is fine too.
Sure. Will update.
yeah but it's still in the license directory.
Yeah, but I'm not sure I want to add another top-level directory just for this. Perhaps when we have more of these?

Patch that resolved the issue:
diff --git a/NOTICE.txt b/NOTICE.txt
index 26b5374398b..f973663670b 100644
--- a/NOTICE.txt
+++ b/NOTICE.txt
@@ -61,6 +61,8 @@ facade for Java, which can be obtained at:
 This product contains a modified portion of 'Apache Harmony', an open source
 Java SE, which can be obtained at:
 
+  * NOTICE:
+    * license/NOTICE.harmony.txt
   * LICENSE:
     * license/LICENSE.harmony.txt (Apache License 2.0)
   * HOMEPAGE:
diff --git a/license/NOTICE.harmony.txt b/license/NOTICE.harmony.txt
new file mode 100644
index 00000000000..036ba0d4bb2
--- /dev/null
+++ b/license/NOTICE.harmony.txt
@@ -0,0 +1,6 @@
+Apache Harmony
+
+Copyright 2006, 2010 The Apache Software Foundation.
+
+This product includes software developed at
+The Apache Software Foundation (http://www.apache.org/).


Test patch: