Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

brew alias automatically re-symlinks missing aliases #58

Merged
merged 2 commits into from
Jan 26, 2024
Merged

brew alias automatically re-symlinks missing aliases #58

merged 2 commits into from
Jan 26, 2024

Conversation

kabel
Copy link
Contributor

@kabel kabel commented Dec 28, 2023

Allows aliases saved from a backup of BASE_DIR to be rewritten and symlinked into Homebrew install.

Allows aliases saved from a backup of `BASE_DIR` to be rewritten and symlinked into Homebrew install.
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, thanks for the PR! Wondering if it might be worth doing this automatically on all/any other commands instead/as well? Could you try to benchmark with hyperfine?

@kabel
Copy link
Contributor Author

kabel commented Jan 20, 2024

Thanks. It might be useful to just bake this in automatically to the standard brew alias listing command, but that would seem to add side-effects to an otherwise read-only operation. Having an explicit command to re-write the alias file and symlink seems the most appropriate for users to invoke as desired to fix an alias that exists but doesn't execute (missing symlink in path).

I haven't done any benchmark with hyperfine before, but it seems pretty straightforward. The command will likely scale with the number of file-defined aliases. The slowest part of my script was just setting up 50 test aliases, which is slower than you'd expect.

#!/bin/bash

# setup test aliases, removing bin
for i in {1..50}
do
    NAME="test$i"
    brew alias "$NAME=info"
    rm -f "$HOMEBREW_PREFIX/bin/brew-$NAME"
done

# measure repair
hyperfine "brew alias --repair"

function finish {
    # cleanup
    for i in {1..50}
    do
        NAME="test$i"
        brew unalias "$NAME"
    done
}

trap finish EXIT

Results are pretty typical:

Benchmark 1: brew alias --repair
  Time (mean ± σ):      1.212 s ±  0.020 s    [User: 0.572 s, System: 0.365 s]
  Range (min … max):    1.186 s …  1.252 s    10 runs

@MikeMcQuaid
Copy link
Member

Thanks. It might be useful to just bake this in automatically to the standard brew alias listing command, but that would seem to add side-effects to an otherwise read-only operation.

Would that operation not fail (or at least fail to produce the expected output) without running --repair?

@kabel
Copy link
Contributor Author

kabel commented Jan 21, 2024

Would that operation not fail (or at least fail to produce the expected output) without running --repair?

Not exactly, which is the confusing part. brew alias will still be able to read and list the file that have been saved in the $HOME/.brew-aliases. However, invoking those commands will not work until --repair puts them back into $HOMEBREW_PREFIX/bin with the appropriate brew- bin prefix.

Example problem:

% brew alias test1=info
% rm -f $HOMEBREW_PREFIX/bin/brew-test1
% brew alias
brew alias test1='info'
% brew test1
Error: Unknown command: test1
% brew alias --repair
% brew test1
255 kegs, 132,498 files, 4.2GB

@MikeMcQuaid
Copy link
Member

However, invoking those commands will not work until --repair puts them back into $HOMEBREW_PREFIX/bin with the appropriate brew- bin prefix.

I feel like brew alias commands would ideally just do the repair automatically given how relatively fast it is. This feels more useful to users rather than learning another flag. Thoughts?

@kabel kabel changed the title brew alias learns --repair brew alias automatically re-symlinks missing aliases Jan 26, 2024
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh, this makes a lot more sense to me, thanks @kabel!

@MikeMcQuaid MikeMcQuaid merged commit a20493d into Homebrew:master Jan 26, 2024
1 check passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants