Consolidate search-file command into search - #788
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
54dbac2 to
80c8bdd
Compare
80c8bdd to
68d0471
Compare
|
@otherJL0 Needs a rebase. |
68d0471 to
c8e4912
Compare
c8e4912 to
0b4ebef
Compare
|
Please be aware that this may need another rebase soon. I'll poke you as soon as we're ready for it to be reviewed and landed. |
|
Ready to be rebased onto main now I believe. |
0b4ebef to
988806d
Compare
988806d to
dda1208
Compare
tarkah
left a comment
There was a problem hiding this comment.
Hey sorry for the late review! This makes sense, but isn't this a regression in that the search string now must contain a path separator?
I have no way to search for the file "hx".. it'll default now to doing a package search.
Should we add a "--file" flag that explicitly enables file search? At that point, how much better is this than the original "search-file" command?
Hey thanks for the review! The flag idea makes sense because the logic "treat this as a filepath if contains '/'" is a little flimsy. As for the direction we want to go, that might be a misunderstanding on my part. This PR is an attempt to address #739 (comment) and potentially close out #729. Based on the exchange with @ermo here. I think the # default: search for substring in package name or description
moss search hx
# default: '/' found in substring, search for files
moss search /usr/bin/hx
# default: search string matches provider syntax, search by provider
# currently requires exact match
moss search 'binary(hx)'
## Potential future work: fuzzy search with provider syntax
## NOT IMPLEMENTED YET, this will return nothing
moss search 'library(yaml)'
# file flag: search for file
moss search --file hx
# provides flag: search for binary file
moss search --provides hx
# provides flag with type: search for a specified provider
moss search --provides=lib hx # soname(hx)
moss search --provides=pkg hx # pkg(hx)Does this seem right so far? |
|
@otherJL0 That's almost what I had in mind, except: |
|
@otherJL0 I have clarified the UX I was aiming for. Hope it makes sense? The idea is basically that normal users will never need to use anything but Packagers (who are required to understand the provider concept), can then use provider syntax and the --provides flag as filters to narrow down searches. |
|
@ermo this kinda makes sense. I just want to clarify how the general search will work. So naively, all three searches can get lumped into something like which seems noisy. So for a simple search instead of showing every single file path that matches from a package, it could be something like show every package name and summary that has a file which contains the search term, something like This could be the simple path and then the search output with all the explicit file paths is shown when the |
Maybe make it Otherwise, it looks fine. 👍 EDIT: I wonder if we can make it use a more tree-like indent: Then we can sort on first the package name, then display the paths alphabetically beneath each? |
|
I like the tree layout idea for the # original format shows source package at end of line
> moss search --files ssh | grep 'config'
/usr/lib/helix/runtime/grammars/sshclientconfig.so from helix
/usr/lib/helix/runtime/queries/sshclientconfig/highlights.scm from helix
/usr/lib/pkgconfig/libssh2.pc from libssh2-devel
/usr/share/man/man3/libssh2_keepalive_config.3 from libssh2-devel
/usr/share/defaults/ssh/ssh_config.d/20-systemd-ssh-proxy.conf from systemd
/usr/share/defaults/ssh/ssh_config from openssh
/usr/share/defaults/ssh/ssh_config.d/ssh_config_aerynos.conf from openssh
/usr/share/man/man5/ssh_config.5 from openssh
# tree format organizes package name a few lines above, hard to grep
> moss search --files ssh | grep 'config'
├ /usr/lib/helix/runtime/grammars/sshclientconfig.so
├ /usr/lib/helix/runtime/queries/sshclientconfig/highlights.scm
├ /usr/lib/pkgconfig/libssh2.pc
├ /usr/share/man/man3/libssh2_keepalive_config.3
├ /usr/share/defaults/ssh/ssh_config.d/20-systemd-ssh-proxy.conf
├ /usr/share/defaults/ssh/ssh_config
├ /usr/share/defaults/ssh/ssh_config.d/ssh_config_aerynos.conf
├ /usr/share/man/man5/ssh_config.5I was thinking there could be another flag like I'm fighting my instinct to immediately start implementing these features and instead take a breath and plan out what should and shouldn't get added. |
|
I think JSON is the better deal for structured output here FWIW. |
|
For users, the output should be "nice". For machine parsing, we should be using JSON for everything. |
dda1208 to
dbd9563
Compare
|
@otherJL0 I'm not seeing P.S. if you want to experiment with making |
|
@ermo sorry I've been working on this feature locally and working on some bigger changes. I just pushed a current version and the code is a mess right now but just wanted to show off the current iteration. The I've made the I also added a # results truncated for brevity
cargo run -- search virt --json
Compiling tools_buildinfo v0.26.6 (/home/jlopez/vcs/git/gh/aerynos/os-tools/crates/tools_buildinfo)
Compiling moss v0.26.6 (/home/jlopez/vcs/git/gh/aerynos/os-tools/moss)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.91s
Running `target/debug/moss search virt --json`
[
{
"search_field": "name",
"description": "package name contains search string",
"packages": [
{
"name": "qemu-hw-display-virtio-gpu-pci-gl",
"summary": "QEMU virtio-gpu-gl-pci-gl display device"
},
...
]
},
{
"search_field": "summary",
"description": "package summary contains search string",
"packages": [
{
"name": "llvm-devel",
"summary": "Low Level Virtual Machine"
},
...
]
},
{
"search_field": "file",
"description": "at least one package file path contains search string",
"packages": [
{
"name": "bash-completion",
"summary": "Programmable completion functions for bash"
},
...
]
}
]Which means we can now use something like # show all package names that match by package name
cargo run -- search virt --json | jq 'map(select(.search_field == "name")) | first | .packages[] | .name'
Compiling tools_buildinfo v0.26.6 (/home/jlopez/vcs/git/gh/aerynos/os-tools/crates/tools_buildinfo)
Compiling moss v0.26.6 (/home/jlopez/vcs/git/gh/aerynos/os-tools/moss)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.95s
Running `target/debug/moss search virt --json`
"qemu-hw-display-virtio-gpu-pci-gl"
"qemu-hw-display-virtio-vga-gl"
"qemu-hw-display-virtio-gpu-gl"
"qemu-hw-display-virtio-gpu"
"qemu-hw-display-virtio-gpu-pci"
"qemu-hw-display-virtio-vga"
"qemu-hw-s390x-virtio-gpu-ccw"
"pkgset-aeryn-virtual"
"libvirt-dbginfo"
"libvirt"
"libvirt-devel"
"libvirt-docs"
"virt-manager"
"qt6-virtualkeyboard"
"qt6-virtualkeyboard-demos"
"qt6-virtualkeyboard-devel"
"qt6-virtualkeyboard-dbginfo"
"libvirt-python"
"libvirt-python-dbginfo"
"python-virtualenv"
"virtiofsd-dbginfo"
"virtiofsd"
"libvirt-glib"
"libvirt-glib-dbginfo"
"libvirt-glib-devel"Still to do include:
This should probably be a new PR with all the new changes. The code is very ugly too and I'm nowhere near done but let me know if this seems like it's moving in the right direction so far. |
|
Maybe consider adding a --no-headers option and a --no-tree option? That way, they act as "option filters" while the default looks "nice"? |
|
In terms of PR, this work is IMO still congruent with the PR title. We could potentially skip the json part and do that later, now that the output can potentially be controlled via the --files option? In reality, I am thinking that (in the future), we could add a "file" kind of provider that we capture at build time. That would make conflicts easier to hook up, because all conflicts are then provider conflicts...? 🧐 |
93851c9 to
0a1045f
Compare
|
@otherJL0 Do you need me to test anything? If you do, could you try rebasing? |
#729
This PR fixes some small issues with
moss searchand adds thesearch-filefunctionality to thesearchcommand. I've added a deprecation warning tosearch-fileto steer users towards the consolidatedsearchcommand.