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

Minor feature: Add -a/--all option to brl which to list all strata that provide a given binary #212

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cptpcrd
Copy link
Contributor

@cptpcrd cptpcrd commented Nov 13, 2020

Rationale

Sometimes, when on a Bedrock system, I find myself wondering "which strata do I have X installed in?" What I'd like is a combination of the functionality of brl which -b (which prints the stratum that provides a given binary) and which -a (with GNU coreutils, this prints all the locations in which the given program can be found). I could probably implement this with a quick one-liner using strat -r and which, but to me this feels like a missing feature.

In a nutshell, here's what I'm aiming for:

$ brl which -a xbps-install
void
void-musl

This PR adds a -a/--all option to brl which which does this.

Notes

  1. This is mostly a proof-of-concept. I'm not sure if it will work in all scenarios, and I'm happy to completely rewrite it if necessary to fix bugs or make it follow better practice. I'm not in any rush to get this merged; this is just something I'd kind of like to see eventually that I think other people would find useful too.
  2. There's an argument to be made that this should also work like which_file if any of the arguments contain a /. I can implement that too.
  3. The interface might need changes; right now if multiple arguments are passed it's unclear which strata names correspond to which arguments.

@paradigm
Copy link
Member

A number of people have shown interest in something like this over the years (e.g. https://gist.github.com/NICHOLAS85/a01d5f64744e4924ec0a5d5fd112cbc1). I've been hesitant in the past, but at some point I should probably just accept the demand for this outweighs the associated complexity.

In the past, another option I've considered as an alternative to this would be a new brl subcommand which runs the provided arguments for all strata. You could then do something like

$ # list all strata that provide `ls`
$ brl all command -v ls
$ # read all /etc/os-release files
$ brl all cat /etc/os-release

However, in practice, I expect the vast majority of use cases to be equivalent to brl which -a, and in that case, it'd make more sense to group the functionality with brl which.

So I guess we'll go with brl which -a. Given this, some changes I'd like to see from your proof of concept before upstreaming something like this:

  • Look through /bedrock/share/common-code for helper functions like list_strata rather than writing your own.
  • It should probably support both functionality for both binaries in the $PATH and files, since there's obvious ways to interpret it for both.
  • The -a/--all flag could be something that can be combine with other flags.
    • If you run brl which -ab <binary> it'll find strata that provide the given binary
    • If you run brl which -af file it'll find all strata that provide the given file path if /bedrock/strata/<stratum> is prefixed
    • If you just run brl which -a <arg> it should auto-detect binary vs file vs something it doesn't know how to all-ify and error out
    • If we ever think of a way to do this with another identifier type where all is applicable, we can use it there then
  • You shouldn't have to search the $PATH yourself. Other strategies to consider:
    • Loop over the strata and strat "${strat}" /bedrock/libexec/busybox sh -c "command -v \"$cmd\""
    • Loop over the strata and strat "${strat}" /bedrock/libexec/busybox which "$cmd"
  • bash, zsh, and fish completion (see /bedrock/share/{bash,zsh,fish})

Prints the list of strata in which the given binary can be found.
@cptpcrd
Copy link
Contributor Author

cptpcrd commented Feb 1, 2021

@paradigm I've pushed changes that should address most of your concerns, though I have a few of my own. I'll leave comments at relevant points.

@@ -67,6 +68,19 @@ which_bin() {
done
}

which_bin_all() (
min_args "${#}" "1"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't test this before pushing, and it appears min_args doesn't work reliably from inside functions. Currently, this happens, and it's not immediately obvious to me how to fix it:

$ brl which -ab
ERROR: Insufficient arguments, see `--help`.
ERROR: Unexpected error occurred.

which_bin_all() (
min_args "${#}" "1"

while [ -n "${1:-}" ]; do
Copy link
Contributor Author

@cptpcrd cptpcrd Feb 1, 2021

Choose a reason for hiding this comment

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

The normal brl which interface doesn't really work properly with -a. For example, this is very ambiguous:

$ brl which -a cmake git
void
void-musl
arch

Some of the formats that leapt to mind include:

$ brl which -a cmake git
void: cmake
void-musl: cmake
arch: git

$ brl which -a cmake git
cmake: void
cmake: void-musl
git: arch

$ brl which -a cmake git
cmake: void void-musl
git: arch

$ brl which -a cmake git
void: cmake git
void-musl: cmake
arch: git

I'm not really sure which is best.

Copy link

@jake-87 jake-87 Jun 4, 2021

Choose a reason for hiding this comment

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

Personally

$ brl which -a cmake git
void: cmake git
void-musl: cmake
arch: git 

makes the most sense to me, as this quickly shows what has what, while still having a per distro view.


while [ -n "${1:-}" ]; do
for stratum in $(list_strata); do
if path="$(strat -r "${stratum}" /bedrock/libexec/busybox which "$1" 2>/dev/null)"; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can add handling for "file/binary not found" both here and in which_file_all, but what should be the behavior? Something like this?

$ brl which -a git 0ad cmake
arch: git
WARNING: 0ad not found in any strata
void: cmake
# exit with status 1

Copy link

Choose a reason for hiding this comment

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

I don't think it makes much sense to phrase it as a warning, as it's really just the absence of something. Perhaps something closer to just 0ad not found in any strata?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would probably depend on the exact format; see above. e.g. for some it could be 0ad not found; for others 0ad: not found might make more sense. Colorization might help too.

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.

3 participants