Skip to content

Fix reinstall loop when tool has both binaryPath and desiredBinaryName#74

Merged
albertodebortoli merged 2 commits intomainfrom
fix/desired-binary-name-reinstall
Apr 19, 2026
Merged

Fix reinstall loop when tool has both binaryPath and desiredBinaryName#74
albertodebortoli merged 2 commits intomainfrom
fix/desired-binary-name-reinstall

Conversation

@albertodebortoli
Copy link
Copy Markdown
Member

Pull Request Title

Fix reinstall loop when tool has both binaryPath and desiredBinaryName

Description

  • Root cause: isToolInstalled checked binaryPath before desiredBinaryName. After first install, the archive binary is renamed to desiredBinaryName on disk, so binaryPath no longer exists. On subsequent runs isToolInstalled returned false, triggered a full re-download, re-extracted the archive, then failed with "item with the same name already exists" when trying to rename again.
  • Secondary fix: reinstall now also checks desiredBinaryName before binaryPath when resolving the on-disk binary, making the intent explicit and consistent with the install path.
  • No related issue.

Type of Change

  • Feature
  • Bug fix
  • Maintenance / Refactor
  • Documentation
  • CI / Tooling
  • Other (specify)

How Has This Been Tested?

  • Added / updated unit tests
  • Manually tested locally (describe)
  • Tested on macOS (arch: arm64)
  • Other

Two new regression tests:

  • InstallerTests/test_reinstall_withDesiredBinaryName_doesNotRedownload — installs a tool twice and asserts the downloader is called exactly once (second run takes the reinstall path, not re-download).
  • ToolInstallerTests/test_reinstall_withDesiredBinaryName_usesDesiredNameForSymlink — calls reinstall on a tool whose binary is stored under desiredBinaryName and verifies the symlink is created correctly.
swift test   # 375 tests, 0 failures

Checklist

  • Swift code builds locally (swift build)
  • Tests pass locally (swift test)
  • Code style / formatting respected
  • Documentation updated (README / comments)
  • Version / tag alignment considered (if release related)
  • PR title follows conventional style (optional)

CI Considerations

  • Affects build time notably
  • Requires new secrets / env vars
  • Alters release process

Breaking Changes?

  • No
  • Yes (describe impact and migration path)

Additional Notes

Reproducer (Lucafile entry that triggered the bug):

- name: Phrase
  binaryPath: phrase_macosx_arm64
  desiredBinaryName: phrase
  version: 2.59.0
  url: https://github.com/phrase/phrase-cli/releases/download/2.59.0/phrase_macosx_arm64.zip
  ignoreArchCheck: true

@albertodebortoli albertodebortoli added this to the 0.17.0 milestone Apr 19, 2026
@albertodebortoli albertodebortoli added the bugfix Something isn't working label Apr 19, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@albertodebortoli albertodebortoli merged commit d169c20 into main Apr 19, 2026
3 checks passed
@albertodebortoli albertodebortoli deleted the fix/desired-binary-name-reinstall branch April 19, 2026 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant