On Tue, 17 May 2022 17:06:51 GMT, Hima Bindu Meda <hm...@openjdk.org> wrote:

>> Updated libxml to version 2.9.14.As mentioned in UPDATING.txt, configured 
>> for windows , linux and Mac platforms and updated the files accordingly.
>> 
>> Updated libxslt to version 1.1.35. Generated the config.h files for windows 
>> , Mac and linux platforms and updated accordingly.
>> 
>> Verified the build on all the platforms and sanity testing looks fine.No 
>> regressions observed.
>
> Hima Bindu Meda has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Add UPDATING.txt for libxslt update

The code changes look good. I tested it on 3 platforms. All good.

I left a few inline comments, including a couple questions about whitespace 
diffs. I notice that there are 27 files that only have whitespace changes and 
no other diffs, as well as one more where the whitespace diffs are the majority 
of the changes. Is this intentional?

modules/javafx.web/src/main/native/Source/ThirdParty/libxml/src/valid.c line 30:

> 28: 
> 29: static xmlElementPtr xmlGetDtdElementDesc2(xmlDtdPtr dtd, const xmlChar 
> *name,
> 30:                                    int create);

This file has many whitespace-only changes (more than 2,800 lines). There is 
only one actual diff for this file. Can you double check the tab conversion?

modules/javafx.web/src/main/native/Source/ThirdParty/libxslt/UPDATING.txt line 
15:

> 13:         >  cscript configure.js compiler=msvc xslt_debug=no  iconv=no 
> mem_debug=no modules=no  zlib=no profiler=no trio=no
> 14:         - Above command generates a header file 'src/config.h' file (on 
> all platforms) and libxslt\src\libxslt\xsltconfig.h.
> 15: 4.1 Copy `libxslt\src\config.h` to `libxslt\win32\config.h'. config.h 
> file defines several macros to control libxslt features. We do not require 
> all of the features to be enabled. 

Minor: trailing whitespace (since this is a text file, jcheck won't complain, 
but since you will be editing this file anyway, maybe you can fix this).

modules/javafx.web/src/main/native/Source/ThirdParty/libxslt/UPDATING.txt line 
36:

> 34: 9.1 Copy `libxslt/src/config.h` to `libxslt/linux/config.h` and follow 
> same guidelines as Windows to retain changes from our repo.
> 35: 
> 36: 10. Remove tabs and trailing whitespaces from source files(.h and .c).

Can you add a step to update the version in 
`modules/javafx.web/src/main/legal/libxslt.md`. Please also add a similar step 
to `modules/javafx.web/src/main/native/Source/ThirdParty/libxml/UPDATING.txt` 
(which is otherwise unmodified).

modules/javafx.web/src/main/native/Source/ThirdParty/libxslt/linux/libexslt/exsltconfig.h
 line 21:

> 19:  * the version string like "1.2.3"
> 20:  */
> 21: #define LIBEXSLT_DOTTED_VERSION "0.8.20"

Shouldn't this be `1.1.35`?

modules/javafx.web/src/main/native/Source/ThirdParty/libxslt/src/libxslt/attributes.c
 line 67:

> 65:                      ((c) == 0x0D))
> 66: 
> 67: #define IS_BLANK_NODE(n)                                                \

This file has only whitespace changes and no other changes. other than 
whitespace changes. Is this intentional?

-------------

PR: https://git.openjdk.java.net/jfx/pull/797

Reply via email to