Skip to content

Warn about other rec commands instead of refusing to install - #53

Open
NopeCharon wants to merge 1 commit into
recursive-oss:mainfrom
NopeCharon:fix/install-rec-name-conflict
Open

NopeCharon wants to merge 1 commit into
recursive-oss:mainfrom
NopeCharon:fix/install-rec-name-conflict

Conversation

@NopeCharon

Copy link
Copy Markdown

Fixes #12.

install.sh stopped whenever any other rec was on PATH, so anyone with SoX installed through Homebrew could not install rec without deleting Homebrew's symlink. The same check blocked rec update on an existing install once another rec appeared.

As discussed in #12, this downgrades that check to a warning that prints the path of each conflicting command. It looks like this after a successful install:

Other rec commands on your PATH:
  /opt/homebrew/bin/rec
Whichever comes first on PATH runs as rec. This install is always at /Users/me/.local/bin/rec.
  • What still stops the install: a ~/.local/bin/rec that rec doesn't manage, because replacing it would overwrite another program.
  • Why the warning doesn't name a winner: the current shell and a new terminal can disagree. The installer's shell configuration line puts ~/.local/bin first, but only for new terminals.
  • Why it also warns when rec comes first: the other program's rec is now shadowed, which is worth knowing. This follows the "print all conflicting aliases" suggestion in install.sh cannot coexist with SoX's rec command #12, rather than staying silent as the reporter proposed.

The recursive fallback command also proposed in #12 is not included. It would add a second managed link for rec uninstall to handle, and it hasn't been agreed, so I'd rather leave it for a separate change if you want it.

Tests

  • tests/installer_test.py has new cases for:
    • another rec ahead of the install, on the first install, on a later update and with --serve, checking that the other program never runs
    • another rec behind the install, with a duplicated PATH entry listed once
    • no warning when the only rec is this install, including PATH spellings such as ~/.local/bin/ and ~/.local/bin/.
  • The new cases fail against install.sh on main with rec already resolves to .... They also fail if I remove the warning, the deduplication or the || true, send the warning to stdout, or replace -ef with a string comparison.
  • tests/installer_test.py passes with macOS's /bin/bash 3.2 and with bash 5.2 on Ubuntu. shellcheck and ruff pass.
  • I ran the script through bash on stdin in a PTY, using a local release server, a stand-in rec binary and a Homebrew-shaped symlink to a stand-in SoX rec:
    • The fresh install succeeds and prints the warning.
    • The other rec still wins in that shell until ~/.local/bin is added to PATH.
    • Reinstalling with ~/.local/bin first runs rec.
    • The symlink is unchanged.
  • I didn't run the full make test, as no Go or engine code changed. I also didn't test against a real Homebrew SoX install.

The installer stopped whenever any other `rec` was on PATH. On macOS that
name also belongs to the recorder in SoX, which Homebrew links at
/opt/homebrew/bin/rec, so anyone with SoX installed could not install rec
without deleting Homebrew's symlink, and `brew reinstall sox` restored the
problem. The check also ignored PATH order, and blocked `rec update` on an
existing install once another `rec` appeared.

The installer never writes outside ~/.local/bin, so another `rec` is no
reason to stop. Install as usual, then list the other commands and say how
to reach this one. Listing them last keeps the warning in view above the
shell prompt, and only reports it for an install that succeeded.

The warning does not say which command wins. The current shell and a new
terminal can disagree, because the installer's shell configuration line
puts ~/.local/bin first for new terminals only.

Compare with `-ef` rather than by name, so that PATH spellings such as
~/.local/bin/ are not reported as a conflict with this install. A
~/.local/bin/rec that rec does not manage still stops the install, since
replacing it would overwrite another program.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

install.sh cannot coexist with SoX's rec command

1 participant