Conversation
The asset regex accepted "x86" and "mac" whatever architecture was detected, so on Apple Silicon both macOS packages matched: pandoc-3.7.0.2-arm64-macOS.pkg matched via "arm" pandoc-3.7.0.2-x86_64-macOS.pkg matched via "x86" and "mac" The loop assigns pandoc_urls["darwin"] for every match, so the later asset wins and download_pandoc() installed the Intel build on arm64 and aarch64 machines. Linux was unaffected, because neither "x86" nor "mac" appears in pandoc-*-arm64.deb. Move the alternatives into the architecture branch so each one only matches its own builds: "amd|x86" for Intel, "arm|aarch" for ARM. Windows keeps working. Its only installer is windows-x86_64.msi, and Windows on ARM reports "ARM64" from platform.uname()[4], which does not match the lowercase startswith checks and so takes the Intel branch. An ARM Linux or macOS host no longer gets a win32 entry, which download_pandoc never reads: it looks up sys.platform only, and guards with "if pf not in pandoc_urls".
for more information, see https://pre-commit.ci
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
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.
The problem
The asset regex in
_get_pandoc_urlsputsx86andmacoutside the architecture branch, so they match whatever architecture was detected:On Apple Silicon both macOS packages match:
The collect loop assigns
pandoc_urls[ext2platform[ext]]on every match, so the later asset wins anddownload_pandoc()installs the Intel build onarm64andaarch64.Running the current matcher over a real release's asset list:
Linux is unaffected, neither
x86normacappears inpandoc-*-arm64.deb, so it already picks correctly.It runs under Rosetta, which is presumably why this has gone unnoticed, but it is the wrong binary and it fails outright where Rosetta is not installed. CI is affected too, since the workflow calls
download_pandoc()directly and GitHub now offers ARM runners.The change
Two lines. Move the alternatives inside the architecture branch so each only matches its own builds:
After, over the same asset list:
Linux and Windows picks are unchanged on Intel hosts.
Two things worth checking in review
Windows still works. Its only installer is
windows-x86_64.msi, matched byx86on the Intel branch. Windows on ARM reports"ARM64"fromplatform.uname()[4], and the existingstartswithchecks are lowercase, so it takes the Intel branch and gets that msi which is the only one that exists.An ARM Linux or macOS host no longer gets a
win32entry.download_pandocnever reads it: it looks upsys.platformonly, and already guards withif pf not in pandoc_urls._get_pandoc_urlsis private, so nothing public depends on the map being complete for other platforms.Tests
TestArchitectureSelectionadded totests/test_pandoc_download.py, using the sameMagicMock+patchstyle as the existing tests and a stubbed release payload, so it needs no network and no pandoc install.