Copilot commented on code in PR #8844:
URL: https://github.com/apache/hadoop/pull/8844#discussion_r4316607782
##########
hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/CMakeLists.txt:
##########
@@ -151,8 +151,8 @@ endif (NOT THREAD_LOCAL_SUPPORTED)
unset (PROTOC_IS_COMPATIBLE CACHE)
set (CMAKE_REQUIRED_INCLUDES ${PROTOBUF_INCLUDE_DIRS})
set (CMAKE_REQUIRED_LIBRARIES
- ${PROTOBUF_LIBRARY}
- ${PROTOBUF_PROTOC_LIBRARY}
+ protobuf::libprotobuf
+ protobuf::libprotoc
${protobuf_ABSL_USED_TARGETS})
Review Comment:
This check now links against `protobuf::libprotobuf` / `protobuf::libprotoc`
imported targets, but still relies on `${PROTOBUF_INCLUDE_DIRS}` for headers.
With Config-mode Protobuf, `${PROTOBUF_INCLUDE_DIRS}` may be unset (the target
carries include dirs instead), which can break `check_cxx_source_compiles()` by
failing to find `<google/protobuf/...>` headers. Prefer deriving required
include dirs from the imported target (e.g., via `get_target_property(...
INTERFACE_INCLUDE_DIRECTORIES ...)`) or using the correct variable for how
Protobuf is found in this build.
##########
dev-support/docker/Dockerfile_windows_10:
##########
@@ -50,54 +50,56 @@ RUN powershell Copy-Item -Recurse -Path 'C:\Program
Files\Git' -Destination C:\G
RUN powershell git clone https://github.com/microsoft/vcpkg.git \
&& cd vcpkg \
&& git fetch --all \
- && git checkout 2025.03.19 \
+ && git checkout 2026.07.29 \
&& .\bootstrap-vcpkg.bat
ADD vcpkg/vcpkg.json .
RUN powershell .\vcpkg\vcpkg.exe install --x-install-root .\vcpkg\installed
-# Install Azul Java 8 JDK.
-RUN powershell Invoke-WebRequest -URI
https://cdn.azul.com/zulu/bin/zulu8.62.0.19-ca-jdk8.0.332-win_x64.zip -OutFile
$Env:TEMP\zulu8.62.0.19-ca-jdk8.0.332-win_x64.zip
-RUN powershell Expand-Archive -Path
$Env:TEMP\zulu8.62.0.19-ca-jdk8.0.332-win_x64.zip -DestinationPath "C:\Java"
+# Install Microsoft Build of OpenJDK 17.
+RUN powershell Invoke-WebRequest -URI
https://aka.ms/download-jdk/microsoft-jdk-17.0.20.1-windows-x64.msi -OutFile
C:\jdk17.msi
+RUN powershell Start-Process msiexec.exe -ArgumentList
'/i','C:\jdk17.msi','INSTALLDIR=C:\Java\jdk17\','/quiet','/norestart' -Wait
+RUN powershell if (-not (Test-Path 'C:\Java\jdk17\bin\javac.exe')) { throw
'JDK 17 installation failed' }
+RUN powershell Remove-Item C:\jdk17.msi -Force
# Install Apache Maven.
RUN powershell Invoke-WebRequest -URI
https://archive.apache.org/dist/maven/maven-3/3.9.15/binaries/apache-maven-3.9.15-bin.zip
-OutFile $Env:TEMP\apache-maven-3.9.15-bin.zip
RUN powershell Expand-Archive -Path $Env:TEMP\apache-maven-3.9.15-bin.zip
-DestinationPath "C:\Maven"
-# Install CMake 3.19.0.
-RUN powershell Invoke-WebRequest -URI
https://cmake.org/files/v3.19/cmake-3.19.0-win64-x64.zip -OutFile
$Env:TEMP\cmake-3.19.0-win64-x64.zip
-RUN powershell Expand-Archive -Path $Env:TEMP\cmake-3.19.0-win64-x64.zip
-DestinationPath "C:\CMake"
+# Install CMake 3.30.2.
+RUN powershell Invoke-WebRequest -URI
https://cmake.org/files/v3.30/cmake-3.30.2-windows-x86_64.zip -OutFile
$Env:TEMP\cmake-3.30.2-windows-x86_64.zip
Review Comment:
These downloads/install steps don’t verify integrity (e.g., SHA-256) before
executing installers/unzipping. This increases supply-chain risk for the build
image. Consider adding checksum verification (or signature validation where
applicable) for the JDK MSI and CMake ZIP before installation/extraction.
##########
dev-support/docker/vcpkg/vcpkg.json:
##########
@@ -1,16 +1,239 @@
{
"$schema":
"https://raw.githubusercontent.com/microsoft/vcpkg-tool/main/docs/vcpkg.schema.json";,
"dependencies": [
- "boost",
+ "boost-asio",
+ "boost-date-time",
+ "boost-system",
+ "boost-program-options",
"protobuf",
"openssl",
"zlib"
],
- "builtin-baseline": "289a69379604112a433874fe8b9812dad3103341",
+ "builtin-baseline": "9e593bb18ea69cc5095e012465dcd675a822ed0d",
"overrides": [
{
"name": "protobuf",
- "version": "3.25.5"
+ "version": "4.25.1"
Review Comment:
This pins Windows Protobuf to `4.25.1`, while the PR description notes Linux
pins Protobuf `3.25.5` in-repo. Having different Protobuf major versions across
platforms can lead to divergent build behavior and harder-to-debug
inconsistencies in native components. Consider aligning the Protobuf version
across platforms (or explicitly documenting why Windows must use a newer major
and validating that the resulting native artifacts remain compatible with
project expectations).
##########
dev-support/docker/vcpkg/vcpkg.json:
##########
@@ -1,16 +1,239 @@
{
"$schema":
"https://raw.githubusercontent.com/microsoft/vcpkg-tool/main/docs/vcpkg.schema.json";,
"dependencies": [
- "boost",
+ "boost-asio",
+ "boost-date-time",
+ "boost-system",
+ "boost-program-options",
Review Comment:
The manifest now pins Boost by enumerating dozens of `boost-*` overrides,
which is hard to maintain as vcpkg port relationships evolve. If feasible,
consider selecting a `builtin-baseline` commit where Boost is already 1.86.0,
so you can drop most/all Boost overrides and only keep overrides for ports that
truly must diverge from that baseline (reducing long-term churn and review
surface).
##########
dev-support/docker/vcpkg/vcpkg.json:
##########
@@ -1,16 +1,239 @@
{
"$schema":
"https://raw.githubusercontent.com/microsoft/vcpkg-tool/main/docs/vcpkg.schema.json";,
"dependencies": [
- "boost",
+ "boost-asio",
+ "boost-date-time",
+ "boost-system",
+ "boost-program-options",
"protobuf",
"openssl",
"zlib"
],
- "builtin-baseline": "289a69379604112a433874fe8b9812dad3103341",
+ "builtin-baseline": "9e593bb18ea69cc5095e012465dcd675a822ed0d",
"overrides": [
{
"name": "protobuf",
- "version": "3.25.5"
+ "version": "4.25.1"
+ },
+ {
+ "name": "boost-algorithm",
+ "version": "1.86.0"
+ },
Review Comment:
The manifest now pins Boost by enumerating dozens of `boost-*` overrides,
which is hard to maintain as vcpkg port relationships evolve. If feasible,
consider selecting a `builtin-baseline` commit where Boost is already 1.86.0,
so you can drop most/all Boost overrides and only keep overrides for ports that
truly must diverge from that baseline (reducing long-term churn and review
surface).
##########
dev-support/docker/Dockerfile_windows_10:
##########
@@ -50,54 +50,56 @@ RUN powershell Copy-Item -Recurse -Path 'C:\Program
Files\Git' -Destination C:\G
RUN powershell git clone https://github.com/microsoft/vcpkg.git \
&& cd vcpkg \
&& git fetch --all \
- && git checkout 2025.03.19 \
+ && git checkout 2026.07.29 \
&& .\bootstrap-vcpkg.bat
ADD vcpkg/vcpkg.json .
RUN powershell .\vcpkg\vcpkg.exe install --x-install-root .\vcpkg\installed
-# Install Azul Java 8 JDK.
-RUN powershell Invoke-WebRequest -URI
https://cdn.azul.com/zulu/bin/zulu8.62.0.19-ca-jdk8.0.332-win_x64.zip -OutFile
$Env:TEMP\zulu8.62.0.19-ca-jdk8.0.332-win_x64.zip
-RUN powershell Expand-Archive -Path
$Env:TEMP\zulu8.62.0.19-ca-jdk8.0.332-win_x64.zip -DestinationPath "C:\Java"
+# Install Microsoft Build of OpenJDK 17.
+RUN powershell Invoke-WebRequest -URI
https://aka.ms/download-jdk/microsoft-jdk-17.0.20.1-windows-x64.msi -OutFile
C:\jdk17.msi
+RUN powershell Start-Process msiexec.exe -ArgumentList
'/i','C:\jdk17.msi','INSTALLDIR=C:\Java\jdk17\','/quiet','/norestart' -Wait
Review Comment:
These downloads/install steps don’t verify integrity (e.g., SHA-256) before
executing installers/unzipping. This increases supply-chain risk for the build
image. Consider adding checksum verification (or signature validation where
applicable) for the JDK MSI and CMake ZIP before installation/extraction.
##########
dev-support/docker/Dockerfile_windows_10:
##########
@@ -125,6 +127,13 @@ RUN powershell -Command Start-Process -FilePath
.\vcredist_x64.exe -ArgumentList
RUN powershell -Command Remove-Item vcredist_x64.exe
# Create a user HadoopBuilder with basic privileges and use it for building
Hadoop on Windows.
+# Windows Server Core enables password complexity by default (unlike the
+# Windows client image), which rejects the passwordless build account below.
+RUN powershell secedit /export /cfg "C:\pwpol.cfg"
+RUN powershell "(Get-Content C:\pwpol.cfg).Replace('PasswordComplexity = 1',
'PasswordComplexity = 0') | Out-File C:\pwpol.cfg"
+RUN powershell secedit /configure /db "C:\windows\security\local.sdb" /cfg
"C:\pwpol.cfg"
+RUN powershell Remove-Item -Force "C:\pwpol.cfg" -Confirm:$false
+
RUN powershell New-LocalUser -Name 'HadoopBuilder' -Description 'User account
for building Apache Hadoop' -Password ([securestring]::new())
-AccountNeverExpires -PasswordNeverExpires
Review Comment:
Disabling system-wide password complexity in the image weakens the
container’s local security posture beyond just the build user. A safer
alternative is to keep the policy enabled and create `HadoopBuilder` with a
compliant strong random password (the `USER` directive does not require the
password at runtime), avoiding the need to relax the policy.
##########
BUILDING.txt:
##########
@@ -603,12 +603,29 @@ still be immensely useful as a raw guide for all the
steps involved in creating
needed to build Hadoop on Windows 10.
Building using the Docker:
+Docker Desktop must first be switched to Windows containers. In the default
Linux container mode
+every command below fails immediately with "no matching manifest for
linux/amd64".
+> "%ProgramFiles%\Docker\Docker\DockerCli.exe" -SwitchWindowsEngine
+
+Wait for the engine to restart, then confirm - this must print "windows"
before proceeding.
+> docker info --format "{{.OSType}}"
+
We first need to build the Docker image for building Hadoop on Windows 10. Run
this command from
the root of the Hadoop repository.
-> docker build -t hadoop-windows-10-builder -f
.\dev-support\docker\Dockerfile_windows_10 .\dev-support\docker\
+> docker build -t hadoop-windows-10-builder -m 8g -f
.\dev-support\docker\Dockerfile_windows_10 .\dev-support\docker\
+
+The -m 8g is needed because the vcpkg step compiles Boost, Protocol Buffers,
OpenSSL and Zlib from
+source and the default container memory is too small. Expect the image build
to take a few hours
+and the resulting image to be around 23 GB. Note that -m and --isolation are
options of the classic
+builder, so leave DOCKER_BUILDKIT unset; BuildKit does not accept them.
Review Comment:
The instruction to 'leave DOCKER_BUILDKIT unset' is easy to misapply if
users already have it set globally. Consider adding an explicit command example
for both cmd.exe and PowerShell to disable it (e.g., setting it to 0 / clearing
it) before running `docker build`, to reduce copy/paste failures.
##########
hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/CMakeLists.txt:
##########
@@ -171,6 +171,13 @@ if (NOT PROTOC_IS_COMPATIBLE)
compiler are compatible with each other.")
endif (NOT PROTOC_IS_COMPATIBLE)
+# The checks above leave CMAKE_REQUIRED_INCLUDES and CMAKE_REQUIRED_LIBRARIES
+# pointing at the Protobuf and Abseil targets. Abseil carries C++ feature
+# requirements, which a C-language check_symbol_exists() below cannot satisfy
+# under the Visual Studio generator. Reset them so later checks run clean.
+unset (CMAKE_REQUIRED_INCLUDES)
+unset (CMAKE_REQUIRED_LIBRARIES)
Review Comment:
Unsetting `CMAKE_REQUIRED_*` after the compatibility check changes global
state for the rest of this CMakeLists. To avoid unintended interactions with
other checks that might have set these variables earlier, consider saving the
previous values before the Protobuf check and restoring them afterward (rather
than unsetting unconditionally).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
