Skip to content

MDEV-39258 Make THIRD_PARTY_DOWNLOAD_LOCATION settable #4898

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vaintroub wants to merge 1 commit into
base: 10.6
Choose a base branch
from
+2 −2
Open

MDEV-39258 Make THIRD_PARTY_DOWNLOAD_LOCATION settable #4898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion win/packaging/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ ENDIF()
SET(THIRD_PARTY_FEATURE_CONDITION "")

IF(WITH_THIRD_PARTY)
SET(THIRD_PARTY_DOWNLOAD_LOCATION "$ENV{TEMP}")
SET(THIRD_PARTY_DOWNLOAD_LOCATION "$ENV{TEMP}" CACHE STRING "Download location for third party")
IF(THIRD_PARTY_DOWNLOAD_LOCATION)
FILE(TO_CMAKE_PATH "${THIRD_PARTY_DOWNLOAD_LOCATION}" THIRD_PARTY_DOWNLOAD_LOCATION)
ELSE()
Expand Down
2 changes: 1 addition & 1 deletion win/packaging/heidisql.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ SET(HEIDISQL_DOWNLOAD_DIR ${THIRD_PARTY_DOWNLOAD_LOCATION}/${HEIDISQL_BASE_NAME}
IF(NOT EXISTS ${HEIDISQL_DOWNLOAD_DIR}/${HEIDISQL_ZIP})
MAKE_DIRECTORY(${HEIDISQL_DOWNLOAD_DIR})
MESSAGE(STATUS "Downloading ${HEIDISQL_URL} to ${HEIDISQL_DOWNLOAD_DIR}/${HEIDISQL_ZIP}")
FILE(DOWNLOAD ${HEIDISQL_URL} ${HEIDISQL_DOWNLOAD_DIR}/${HEIDISQL_ZIP} TIMEOUT 60)
FILE(DOWNLOAD ${HEIDISQL_URL} ${HEIDISQL_DOWNLOAD_DIR}/${HEIDISQL_ZIP} TIMEOUT 300)
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The download step ignores FILE(DOWNLOAD ...) result/status, so a network failure can fall through to the extract step and produce a confusing tar error (or extract a partial file). Capture the download status/log and message(FATAL_ERROR ...) on failure so CI fails with a clear cause.

Suggested change
FILE(DOWNLOAD ${HEIDISQL_URL} ${HEIDISQL_DOWNLOAD_DIR}/${HEIDISQL_ZIP} TIMEOUT 300)
FILE(DOWNLOAD ${HEIDISQL_URL} ${HEIDISQL_DOWNLOAD_DIR}/${HEIDISQL_ZIP}
TIMEOUT 300
STATUS HEIDISQL_DOWNLOAD_STATUS
LOG HEIDISQL_DOWNLOAD_LOG
)
LIST(GET HEIDISQL_DOWNLOAD_STATUS 0 HEIDISQL_DOWNLOAD_STATUS_CODE)
LIST(GET HEIDISQL_DOWNLOAD_STATUS 1 HEIDISQL_DOWNLOAD_STATUS_MESSAGE)
IF(NOT HEIDISQL_DOWNLOAD_STATUS_CODE EQUAL 0)
MESSAGE(FATAL_ERROR
"Failed to download ${HEIDISQL_URL} to ${HEIDISQL_DOWNLOAD_DIR}/${HEIDISQL_ZIP}: "
"${HEIDISQL_DOWNLOAD_STATUS_MESSAGE}\n${HEIDISQL_DOWNLOAD_LOG}"
)
ENDIF()

Copilot uses AI. Check for mistakes.
EXECUTE_PROCESS(COMMAND ${CMAKE_COMMAND} -E chdir ${HEIDISQL_DOWNLOAD_DIR}
Comment on lines 6 to 10
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FILE(DOWNLOAD ...) arguments are unquoted. Since THIRD_PARTY_DOWNLOAD_LOCATION is now user-settable, it may contain spaces/semicolons and break argument parsing (e.g., producing multiple args / wrong path). Quote the URL and destination path (and related path usages in this block) to make the download/extract robust on Windows paths with spaces.

Copilot uses AI. Check for mistakes.
Comment on lines 8 to 10
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This downloads a third-party ZIP from the public internet without any integrity verification. Since the downloaded content is packaged into the MSI, this is a supply-chain risk if the download is tampered with or the upstream is compromised. Consider pinning an EXPECTED_HASH (or equivalent signature/checksum verification) and failing the build if it doesn’t match.

Copilot uses AI. Check for mistakes.
${CMAKE_COMMAND} -E tar xfz ${HEIDISQL_DOWNLOAD_DIR}/${HEIDISQL_ZIP}
)
Comment on lines 6 to 12
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IF(NOT EXISTS .../${HEIDISQL_ZIP}) guard skips both download and extraction when the ZIP already exists. That means a CI “pre-download” that only places the ZIP in the cache won’t trigger extraction, and packaging will later fail because ${HEIDISQL_DOWNLOAD_DIR}\\heidisql.exe (and other files) aren’t present. Consider separating the conditions: download only if the ZIP is missing, but extract if the extracted payload (e.g., heidisql.exe) is missing.

Copilot uses AI. Check for mistakes.
Expand Down