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

Binary name detection for dependencies #1697

Closed
3 tasks done
sntp opened this issue Sep 28, 2024 · 5 comments
Closed
3 tasks done

Binary name detection for dependencies #1697

sntp opened this issue Sep 28, 2024 · 5 comments
Labels
feature New feature request good first issue Good for newcomers

Comments

@sntp
Copy link

sntp commented Sep 28, 2024

yazi --debug output

n/a

Please describe the problem you're trying to solve

While installing and configuring Yazi on Ubuntu, I noticed that Yazi doesn't recognize the installed ImageMagick and fd packages on my system. The issue is that APT provides binaries with different names. For ImageMagick, Yazi expects the binary to be called magick, but on Ubuntu, it's called convert. Similarly, fd is named fdfind.

After briefly checking Yazi's source code, I found that hardcoded names are used for all utilities.

writeln!(s, " ueberzugpp : {}", Self::process_output("ueberzugpp", "--version"))?;
writeln!(s, " ffmpegthumbnailer: {}", Self::process_output("ffmpegthumbnailer", "-v"))?;
writeln!(s, " magick : {}", Self::process_output("magick", "--version"))?;
writeln!(s, " fzf : {}", Self::process_output("fzf", "--version"))?;
writeln!(s, " fd : {}", Self::process_output("fd", "--version"))?;
writeln!(s, " rg : {}", Self::process_output("rg", "--version"))?;
writeln!(s, " chafa : {}", Self::process_output("chafa", "--version"))?;
writeln!(s, " zoxide : {}", Self::process_output("zoxide", "--version"))?;
writeln!(s, " 7z : {}", Self::process_output("7z", "i"))?;
writeln!(s, " 7zz : {}", Self::process_output("7zz", "i"))?;
writeln!(s, " jq : {}", Self::process_output("jq", "--version"))?;

local child, code = Command("magick"):args({
"-density",
"200",
tostring(self.file.url),
"-flatten",
"-resize",
string.format("%dx%d^", PREVIEW.max_width, PREVIEW.max_height),
"-quality",
tostring(PREVIEW.image_quality),
"-auto-orient",
"JPG:" .. tostring(cache),
}):spawn()

This seems to be a common issue across various distributions and operating systems since program names don't always match.

Would you be willing to contribute this feature?

  • Yes, I'll give it a shot

Describe the solution you'd like

Yazi should make its best effort to identify the correct binary name or allow plugin developers and users to configure it. There could be a registry for program names where developers and users can register the appropriate names for their systems.

For example, since dependency versions are printed in debug.rs, Yazi could come with a default registry:

let binaryNameRegistry = hashmap! {
  "magick" => hashmap! {
    "Ubuntu" => "convert",
    "Windows" => "magick.exe"
  },
  "fzf" => ...,
  "ffmpegthumbnailer" => ...
  ...
}

Then, plugin developers could add overrides like this:

-- allows to override, but if ImageMagick.exe is missing on the system, try to use magick.exe
local cmd = ya.resolve_binary("magick", { Mydistro = "mymagick", Windows ="ImageMagick.exe" })

Additionally, users could customize it for ad-hoc fixes:

[binaryNameRegistry.magick]
MyDistro = "my-magic"

This is just the first idea that came to mind.

Additional context

No response

Validations

  • I have searched the existing issues/discussions
  • The latest nightly build of Yazi doesn't already have this feature
@sntp sntp added the feature New feature request label Sep 28, 2024
@sxyazi
Copy link
Owner

sxyazi commented Sep 28, 2024

For ImageMagick, Yazi expects the binary to be called magick, but on Ubuntu, it's called convert.

No, convert is a deprecated command, if you run convert, you'll see a warning telling you to use the new magick command instead:

❯ convert
WARNING: The convert command is deprecated in IMv7, use "magick" instead of "convert" or "magick convert"

Also IIRC, magick and convert behave a bit differently, so simply replacing magick with convert might not work.

Please make sure you're using the latest version of ImageMagick.

Similarly, fd is named fdfind.

After a quick check, you're absolutely right - some systems call fd as fdfind, so introducing a name check makes sense.

Would you like to raise a PR for it? Here's the code:

pub fn fd(opt: FdOpt) -> Result<UnboundedReceiver<File>> {
let mut child = Command::new("fd")
.arg("--base-directory")
.arg(&opt.cwd)

@sxyazi sxyazi added the good first issue Good for newcomers label Sep 28, 2024
@xfzv
Copy link

xfzv commented Sep 28, 2024

bat is also available as batcat on Debian:

In this package the executable and its manpage have been renamed from ‘bat’ to ‘batcat’ because of a file name clash with another Debian package.

@sntp
Copy link
Author

sntp commented Sep 28, 2024

No, convert is a deprecated command, if you run convert, you'll see a warning telling you to use the new magick command instead:

Oh I see this in version 7. They still have version 6 in the repository.

❯ docker run --rm -it ubuntu:24.10

root@6b10945cccb4:/# apt update -qq && apt install imagemagick -y -qq

root@6b10945cccb4:/# magick
bash: magick: command not found

root@6b10945cccb4:/# convert
Version: ImageMagick 6.9.13-12 Q16 x86_64 18420 https://legacy.imagemagick.org
Copyright: (C) 1999 ImageMagick Studio LLC
License: https://imagemagick.org/script/license.php
Features: Cipher DPC Modules OpenMP(4.5) 
Delegates (built-in): bzlib djvu fftw fontconfig freetype heic jbig jng jp2 jpeg lcms lqr ltdl lzma openexr pangocairo png raw tiff webp wmf x xml zlib
Usage: convert-im6.q16 [options ...] file [ [options ...] file ...] [options ...] file
...

Do you think it's not worth creating an option to override the utility name and simply try known names in the code instead? There also an option to override find with environment variable: https://github.com/search?q=repo%3Asxyazi%2Fyazi+YAZI_FILE_ONE&type=code. How about introducing common variables for utilities?

@sxyazi
Copy link
Owner

sxyazi commented Sep 29, 2024

Do you think it's not worth creating an option to override the utility name and simply try known names in the code instead?

Yazi aims for maximum automation; it should be out-of-box, not reliant on user config. According to the fd doc, there are only two possible names: fdfind and fd. So, if fd isn't found, simply adding a fdfind fallback is sufficient.

There also an option to override find with environment variable: https://github.com/search?q=repo%3Asxyazi%2Fyazi+YAZI_FILE_ONE&type=code. How about introducing common variables for utilities?

No, this means user config is necessary, while it should be automatic. The role of the YAZI_FILE_ONE env variable is different; it addresses the issue where Windows users can't install file and must manually specify the Git installation path, rather than file having different filenames.

@sxyazi
Copy link
Owner

sxyazi commented Nov 6, 2024

Done in #1889, thank you @Integral-Tech !

@sxyazi sxyazi closed this as completed Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants