Hi Erik,

Thanks for the comments. Yes, I copied an existing file, and -DUSE_MMAP seems unnecessary. I'll implement your suggestions.

Thanks,
   Jan

On 22.6.2015 10:25, Erik Joelsson wrote:
Hello Jan,

Build changes look mostly ok. Some notes:

* "$(CFLAGS_WARNINGS_ARE_ERRORS)", which should not be used anymore.
Warnings are already treated as errors globally.

* There should not be a need to add "-I$(INCLUDEDIR)
-I$(JDK_OUTPUTDIR)/include/$(OPENJDK_TARGET_OS)" to cflags. INCLUDEDIR
is undefined AFAIK, and the other dir is already in CFLAGS_JDKLIB.

I'm guessing that you copied Lib-jdk.jdi.gmk as a template. Is the
-DUSE_MMAP just an artifact of that copy or do you know that it is needed?

/Erik

On 2015-06-18 16:25, Jan Lahoda wrote:
Hello,

I am proposing to add JLine 2.12.1 into the jdk repository for use by
the Java and Nashorn REPLs. Full patch is available here:
http://cr.openjdk.java.net/~jlahoda/8080679/webrev.00/full/

To aid the review, I've split this patch into to smaller patches:
-a patch that only adds unmodified jline sources at appropriate places
in the jdk repository:
http://cr.openjdk.java.net/~jlahoda/8080679/webrev.00/clean-jline/

-a patch that shows the additional changes I've done:
http://cr.openjdk.java.net/~jlahoda/8080679/webrev.00/additional/

This split is intended solely to simplify reviewing, my plan is to
integrate this as a single patch.

The main additional changes are:
-plugging the new module, jdk.jline, into the JDK build. Currently,
the JLine packages are exported only to jdk.scripting.nashorn (the
plan is to also export them to the future jdk.jshell module). (The
patch is not adding the dependency from jdk.scripting.nashorn to
jdk.jline, though - I expect that to be added when needed.)
-the sources are re-packaged from package "jline" to "jdk.internal.jline"
-removing trailing whitespace, adding newlines at the end of the
files, encoding characters that are not ASCII
-avoiding the dependency on another library, jansi, by reimplementing
two elements that were used from the other library. These are mainly
the changes in WindowsTerminal and ConsoleReader.java. This also
includes the WindowsTerminal.cpp native library. The native part is
heavily inspired by:
http://cr.openjdk.java.net/~sherman/rl/src/java.base/windows/native/libjava/Console_md.c.html

As I am not experienced in native programming, comments to the native
part would be particularly useful.
-changes to resolve javac warnings in JLine.
-tests for some of the added functionality.

Any comments are welcome!

Thanks,
    Jan

Reply via email to