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

Port Piped{Input,Output}Stream from Apache Harmony #2691

Merged

Conversation

WojciechMazur
Copy link
Contributor

@WojciechMazur WojciechMazur commented Jun 25, 2022

Fixes #2631
Direct port of Apache Harmony implementation of:

  • PipedInputStream
  • PipedOutputStream
  • PipedReader
  • PipedWriter

Since Piped classes are dedicated for usage from multiple threads and might lead to deadlocks when used in single thread (as pointed in the Java API) it should be merged after implementing multithreading support in Scala Native

TODO:

  • Port remaining tests from Apache Harmony (converted from Java to Scala and commented out)

Copy link
Member

@tanishiking tanishiking left a comment

Choose a reason for hiding this comment

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

I guess porting from Apache Harmony applies the "redistribution" https://www.apache.org/licenses/LICENSE-2.0 (even though we have a modification from Java to Scala)

In that case,

a. You must give any other recipients of the Work or Derivative Works a copy of this License; and

We already have one https://github.com/scala-native/scala-native/blob/main/LICENSE.md#license-notice-for-apache-harmony 👍


b. You must cause any modified files to carry prominent notices stating that You changed the files; and

I guess we have to keep these part in the source code https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/luni/src/main/java/java/io/PipedInputStream.java#L1-L16 am I correct?


d. 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; ...

Looks like we need to copy the NOTICE file into our repository NOTICE file?

@WojciechMazur
Copy link
Contributor Author

I guess we have to keep these part in the source code https://github.com/apache/harmony/blob/02970cb7227a335edd2c8

Probably, we point in the License notices that we either include information that given code is ported from Apache Harmony or we include the whole original header. Typically the short notice is good enough, I think Scala.js does the same.

Looks like we need to copy the NOTICE file into our repository NOTICE file?

Good point, we should add such file. Probably we can do that in similar format as Scala.js does: https://github.com/scala-js/scala-js/blob/037fa855a636a521721f6b126f915744136cd47e/NOTICE

@WojciechMazur WojciechMazur marked this pull request as ready for review July 18, 2023 21:36
@ekrich
Copy link
Member

ekrich commented Jul 18, 2023

I think we remove all the javadoc/scaladoc from the code as we publish that for reference but we want the Oracle javadoc to be the reference.

@WojciechMazur WojciechMazur merged commit 9ef3b11 into scala-native:main Jul 20, 2023
79 checks passed
@WojciechMazur WojciechMazur deleted the javalib/IpedInputStream branch July 20, 2023 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing implementation for piped streams
3 participants