fix: Arrow extraction on Windows without admin privileges#179
Merged
timosachsenberg merged 1 commit intomasterfrom Mar 25, 2026
Merged
fix: Arrow extraction on Windows without admin privileges#179timosachsenberg merged 1 commit intomasterfrom
timosachsenberg merged 1 commit intomasterfrom
Conversation
…rrors Arrow 23.0.0's archive contains symbolic links (e.g., python/cmake_modules -> ../cpp/cmake_modules). 7-Zip cannot create symlinks on Windows without admin privileges, causing extraction to fail for non-elevated users. Replace 7-Zip with cmake -E tar for Arrow extraction on Windows, which handles symlinks gracefully by copying target content instead of creating actual symlinks. Fixes #177 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThe ARROW contrib extraction on Windows/MSVC is refactored from 7-Zip-based extraction with symlink-unsafe flags to CMake's tar extraction with explicit error handling and skip-if-exists logic. Non-MSVC platforms now use the original extraction method in an Changes
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
libraries.cmake/arrow.cmake (2)
13-13: Minor: Checkfile differs between platforms.The MSVC path checks for
CMakeLists.txtwhile the non-MSVC path (line 35) usesREADMEas the checkfile. This works since both files exist after extraction, but for consistency, you could use the same file on both paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libraries.cmake/arrow.cmake` at line 13, The check for extraction success uses different marker files per platform: the MSVC branch checks for ${ARROW_DIR}/CMakeLists.txt while the non-MSVC branch checks for ${ARROW_DIR}/README; make them consistent by using the same checkfile in both branches (e.g., use ${ARROW_DIR}/CMakeLists.txt or ${ARROW_DIR}/README) so the existence test in arrow.cmake is identical for both paths and references the same symbol ARROW_DIR.
16-29: Missing log file output for extraction.The rest of the Arrow build process writes output to
${LOGFILE}(e.g., lines 81-82, 98-99), and theOPENMS_SMARTEXTRACTmacro also logs extraction output. The new MSVC extraction path doesn't logZIP_OUT/ZIP_ERRto the logfile, which could make debugging extraction issues harder.♻️ Proposed fix to add logging
if(NOT EXTRACT_SUCCESS EQUAL 0) message(STATUS "Extracting ARROW .. failed") message(STATUS "${ZIP_ERR}") message(FATAL_ERROR "${ZIP_OUT}") else() + # output to logfile + file(APPEND ${LOGFILE} ${ZIP_OUT}) + file(APPEND ${LOGFILE} ${ZIP_ERR}) message(STATUS "Extracting ARROW .. done") endif()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libraries.cmake/arrow.cmake` around lines 16 - 29, The ARROW extraction block using execute_process (variables ZIP_OUT, ZIP_ERR, RESULT_VARIABLE EXTRACT_SUCCESS) doesn't write its output to ${LOGFILE}; update the block so extraction stdout/stderr are recorded to the same logfile used elsewhere (matching OPENMS_SMARTEXTRACT behavior). For example, either set execute_process to use OUTPUT_FILE and ERROR_FILE pointing at ${LOGFILE} (or append ZIP_OUT/ZIP_ERR into ${LOGFILE} with file(APPEND ${LOGFILE} "...\\n")) and ensure both success and failure branches write ZIP_OUT and ZIP_ERR into ${LOGFILE} alongside the current message() calls so MSVC and non-MSVC extraction paths produce consistent logs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@libraries.cmake/arrow.cmake`:
- Line 13: The check for extraction success uses different marker files per
platform: the MSVC branch checks for ${ARROW_DIR}/CMakeLists.txt while the
non-MSVC branch checks for ${ARROW_DIR}/README; make them consistent by using
the same checkfile in both branches (e.g., use ${ARROW_DIR}/CMakeLists.txt or
${ARROW_DIR}/README) so the existence test in arrow.cmake is identical for both
paths and references the same symbol ARROW_DIR.
- Around line 16-29: The ARROW extraction block using execute_process (variables
ZIP_OUT, ZIP_ERR, RESULT_VARIABLE EXTRACT_SUCCESS) doesn't write its output to
${LOGFILE}; update the block so extraction stdout/stderr are recorded to the
same logfile used elsewhere (matching OPENMS_SMARTEXTRACT behavior). For
example, either set execute_process to use OUTPUT_FILE and ERROR_FILE pointing
at ${LOGFILE} (or append ZIP_OUT/ZIP_ERR into ${LOGFILE} with file(APPEND
${LOGFILE} "...\\n")) and ensure both success and failure branches write ZIP_OUT
and ZIP_ERR into ${LOGFILE} alongside the current message() calls so MSVC and
non-MSVC extraction paths produce consistent logs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e54388ba-d4ab-44f7-99b2-8124b3757c92
📒 Files selected for processing (1)
libraries.cmake/arrow.cmake
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
python/cmake_modules -> ../cpp/cmake_modules) that 7-Zip cannot create on Windows without admin privileges, causing extraction to failcmake -E tarfor Arrow extraction on Windows, which handles symlinks gracefully by copying target content instead of creating actual symlinksTest plan
Fixes #177
🤖 Generated with Claude Code
Summary by CodeRabbit