GauthamBanasandra commented on code in PR #8844:
URL: https://github.com/apache/hadoop/pull/8844#discussion_r4426560161
##########
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:
Checked this one and it doesn't bite here, though you're right about the
premise.
We do resolve Protobuf in Config mode: the Windows build runs under vcpkg's
toolchain, and `share/protobuf/vcpkg-cmake-wrapper.cmake` forces
`_find_package(${ARGS} CONFIG)`, so CMake's own `FindProtobuf.cmake` never runs.
`PROTOBUF_INCLUDE_DIRS` is still populated, though. vcpkg's protobuf
portfile rewrites `if(protobuf_MODULE_COMPATIBLE)` to always-true in the
installed `protobuf-config.cmake`, which unconditionally loads
`protobuf-module.cmake` -- and that file does exactly what you suggest,
`get_target_property(Protobuf_INCLUDE_DIRS protobuf::libprotobuf
INTERFACE_INCLUDE_DIRECTORIES)`, then mirrors it to the uppercase name.
Independently of that, an imported target placed in
`CMAKE_REQUIRED_LIBRARIES` propagates its `INTERFACE_INCLUDE_DIRECTORIES` into
`check_cxx_source_compiles()`, so the check would still find the headers even
if the variable were empty. Confirmed by a full native build on CMake 3.30.2
with VS 2019. No change made.
##########
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:
Done in 46038bf1. All nine downloaded artifacts now verify SHA-512 with
`Get-FileHash` before they are used, following the pattern
`pkg-resolver/install-pip.ps1` already uses for `get-pip.py`.
Where the publisher distributes its own digest I cross-checked against it
before deriving the SHA-512, so the pinned values are not merely self-attested:
Microsoft publishes SHA-256 for the JDK, cmake.org for CMake, and the ASF
publishes SHA-512 for Maven. All three matched.
All nine gates were exercised in a full image rebuild.
One gap that cannot be closed the same way: `vs_buildtools.exe` comes from
`aka.ms/vs/16/release/`, a rolling bootstrapper with no stable content and no
published digest, so there is nothing to pin it to.
##########
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:
Fixed in 46038bf1, though not by the suggested route.
The policy is now restored to `PasswordComplexity = 1` once `HadoopBuilder`
has been created, so the shipped image is no longer left with a relaxed policy.
The restore folds into the `secedit` block that already runs immediately
afterwards, so it adds no extra layer.
I did not give the account a strong password instead. The load-bearing claim
in the suggestion -- that `USER` never needs the password at runtime -- does
not hold reliably for Windows containers: a non-builtin local account that
*has* a password can fail at process creation under `USER`, and the
passwordless account is what makes `USER HadoopBuilder` work here.
Verified by reading the policy back out of the finished image:
`PasswordComplexity = 1`, and `SeCreateSymbolicLinkPrivilege =
HadoopBuilder,*S-1-5-32-544` still granted.
##########
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:
I looked into moving `builtin-baseline` instead, and it does not work here.
The MSYS2 breakage this PR fixes comes from
`scripts/cmake/vcpkg_acquire_msys.cmake`, which ships with the checked-out
vcpkg **tag**, not with `builtin-baseline`. The older tags pin
`msys2-runtime-3.5.4-2`, which is now a 404 on the mirror. So the tag has to
move forward regardless, and keeping the new tag (2026.07.29) while pinning
Boost explicitly was the combination I could verify end to end.
That said, this comment found a real bug, so thank you. `boost-coroutine`
was missing from the override list, so it alone resolved to the baseline and
installed at **1.91.0** while the other 55 ports sat at 1.86.0 -- a mixed Boost
that happened to build anyway. It is now pinned, and I verified against vcpkg's
own package inventory in the rebuilt image that no Boost port is off 1.86.0.
##########
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:
These are not different major versions -- the two numbers are per-language
aliases for the same upstream release.
Protobuf publishes each release under several language-specific versions and
tags each of them. Upstream tag `v3.25.5` declares `cpp 4.25.5` and `java
3.25.5`, so the Linux build, which compiles that tag via `install-protobuf.sh`,
links **C++ runtime 4.25.5**; the `3.25.5` written there is that release's Java
alias. vcpkg names its `protobuf` port after the C++ runtime version and
resolves upstream as `REF v${VERSION}`.
So Windows 4.25.1 and Linux 4.25.5 are the same Protobuf 25.x generation,
one patch apart, not a major split.
Aligning them exactly is not available either: vcpkg publishes neither
`3.25.5` nor `4.25.5` for that port, so the nearest "alignment" would be a
genuinely different major.
##########
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:
I looked into moving `builtin-baseline` instead, and it does not work here.
The MSYS2 breakage this PR fixes comes from
`scripts/cmake/vcpkg_acquire_msys.cmake`, which ships with the checked-out
vcpkg **tag**, not with `builtin-baseline`. The older tags pin
`msys2-runtime-3.5.4-2`, which is now a 404 on the mirror. So the tag has to
move forward regardless, and keeping the new tag (2026.07.29) while pinning
Boost explicitly was the combination I could verify end to end.
That said, this comment found a real bug, so thank you. `boost-coroutine`
was missing from the override list, so it alone resolved to the baseline and
installed at **1.91.0** while the other 55 ports sat at 1.86.0 -- a mixed Boost
that happened to build anyway. It is now pinned, and I verified against vcpkg's
own package inventory in the rebuilt image that no Boost port is off 1.86.0.
--
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]
