Conversation
…s out When run_with_timeout exits 124 (timeout) inside a command substitution under `set -euo pipefail`, the whole shell dies before the filesystem fallback can run. Same for any non-zero mdfind exit. Adding `|| true` to the two `hit=$(...)` assignments lets the resolver continue to the slow-path scan, which is the whole point of gating mdfind behind a timeout in the first place. This was surfacing as a deterministic CI failure on macos-latest for the 'bundle_has_installed_app finds an app by CFBundleIdentifier (Spotlight miss)' test, and blocked PR tw93#770.
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.
Summary
Under
set -euo pipefail,hit=$(run_with_timeout 2 mdfind ... | head -1)kills the shell whenrun_with_timeoutreturns 124 (timeout). That's exactly the case the timeout was supposed to survive — the resolver should fall through to its direct filesystem scan, not die.Appending
|| trueto bothhit=$(...)assignments inbundle_has_installed_apppreserves the fast-path attempt but lets both mdfind failures and timeout-driven 124 exits fall through to the Info.plist walk below.Why this showed up now
This has been surfacing as a CI failure on
macos-latestfor PR #770 (and blocking #772 / #773 rebases). The failing test isbundle_has_installed_app finds an app by CFBundleIdentifier (Spotlight miss)— test #9 in theUnit & Integration Testsjob. It's not a flake:mdfindcan legitimately time out on hosted runners and trip exactly this path.Repro without the fix:
Test plan
bundle_has_installed_app falls back after run_with_timeout returns 124 (set -e + pipefail regression)totests/bundle_resolver.bats. Mocksrun_with_timeoutto return 124 and asserts the filesystem fallback still finds the app.upstream/main(reproducing the CI failure) and passes with the fix.bats tests/bundle_resolver.batsgreen (8/8) locally.