On Mon, 21 Apr 2025 18:59:26 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Jay Bhaskar has updated the pull request incrementally with one additional >> commit since the last revision: >> >> remove unused files > > modules/javafx.web/src/main/native/Source/ThirdParty/libxml/linux/include/libxml/xmlversion.h > line 322: > >> 320: * the string suffix used by dynamic modules (usually shared libraries) >> 321: */ >> 322: #define LIBXML_MODULE_EXTENSION ".so > > This looks wrong. It would almost certainly fail to compile if the code path > were active. You might want to fix it, although it is unused. ok , i miss " > modules/javafx.web/src/main/native/Source/ThirdParty/libxml/src/CMakeLists.txt > line 1: > >> 1: cmake_minimum_required(VERSION 3.18) > > I don't think we use this file in our build. We should remove it if not. This file is not required , deleted > modules/javafx.web/src/main/native/Source/ThirdParty/libxml/src/dict.c line > 933: > >> 931: #else >> 932: // This block will compile on macOS (and any non-Linux system) if >> HAVE_GETENTROPY is defined >> 933: #if defined(HAVE_GETENTROPY) && !defined(__linux__) > > Can you check whether this is needed if `HAVE_GETENTROPY` is left undefined? > I suspect it will no longer be necessary, and it would be better not to have > local mods to upstream files. > > If a modification _is_ needed, then we will need a clear comment with the > changes, noting that this is a JavaFX-specific addition. removed custom change > modules/javafx.web/src/main/native/Source/ThirdParty/libxslt/src/CMakeLists.txt > line 1: > >> 1: cmake_minimum_required(VERSION 3.18) > > I don't think we use this file in our build. We should remove it if not. this [CMakeLists.txt] not required , i have removed > modules/javafx.web/src/main/native/Source/ThirdParty/libxslt/src/configure.ac > line 24: > >> 22: LIBEXSLT_MAJOR_VERSION=0 >> 23: LIBEXSLT_MINOR_VERSION=8 >> 24: LIBEXSLT_MICRO_VERSION=25 > > This should be `24` (not `25`). this file is not required, removed now ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2053826221 PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2053827228 PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2053827841 PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2053829026 PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2053829815
