hackerb9 / lsix

Like "ls", but for images. Shows thumbnails in terminal using sixel graphics.
GNU General Public License v3.0
3.97k stars 129 forks source link

Windows PATH order workaround #42

Closed StaticPH closed 2 years ago

StaticPH commented 3 years ago

On Windows, there exists a convert.exe in the Windows System32 folder, which is used for converting FAT volumes to NTFS. If System32 is earlier in the user's PATH than the directory containing the ImageMagick executables, the convert.exe in System32 will be used, which, understandably, wont work as intended. To resolve this, all calls to executables from ImageMagick should use the magick executable, which is not name-shadowed by any default Windows executable, as they would have prior to ImageMagick 6. See https://imagemagick.org/script/command-line-tools.php While Windows does not have its own montage executable, it seems better to use the availability of the primary magick tool as an indicator of the presence of ImageMagick, and to call magick montage rather than montage simply to be consistent with magick convert.

hackerb9 commented 3 years ago

Your patch breaks lsix on my Debian box. Is there a fix that doesn't use "magick"?

StaticPH commented 3 years ago

Your patch breaks lsix on my Debian box. Is there a fix that doesn't use "magick"?

Breaks how, exactly? And what version of ImageMagick are you running? And yes, there is a solution: checking the OS and adjusting the call accordingly. But the documentation indicates that shouldn't be necessary :thinking:

hackerb9 commented 3 years ago

Breaks how, exactly? And what version of ImageMagick are you running? And yes, there is a solution: checking the OS and adjusting the call accordingly. But the documentation indicates that shouldn't be necessary

Sadly, documentation and reality diverge. When I run 'lsix' from your branch, I get "Please install ImageMagick". I am running the current stable version of Debian GNU/Linux (10.9 AKA "Buster") with its default ImageMagick package, version 6.9.10.

$ ./lsix
Please install ImageMagick

$ convert --version | head -1
Version: ImageMagick 6.9.10-23 Q16 x86_64 20190101 

$ magick
Command 'magick' not found, did you mean:

  command 'magic' from deb magic

Try: sudo apt install magic

$ cat /etc/debian_version 
10.9

Note that the magic package suggested by command-not-found is a VLSI layout tool. There is no package that provides "magick" in Debian stable.

How much impact does this issue have on Microsoft Windows users? Is the default PATH in Windows 10 set so system executables come before locally installed tools for typical (non-admin) users?

hackerb9 commented 3 years ago

By the way, adjusting the call shouldn't be too difficult as we already have bash. Perhaps something like,

if [[ "$WSLENV" ]]; then
    # Kludge so we don't run Microsoft's SYSTEM32\CONVERT.EXE
    alias convert="magick convert"
fi

You'd have to find an environment variable Microsoft Windows always sets; WSLENV is just a guess. Oh, and I guess this would break for anybody trying to use Microsoft Windows if they have an older version of ImageMagick that doesn't include the MAGICK.EXE.

StaticPH commented 3 years ago

Indeed, we could do something like that, though it would need to be a non-WSL related variable for my own purposes. I don't run Windows 10 myself, so I can't easily check to be sure, but I would like to believe that WSL sessions would inherit the COMSPEC and/or WINDIR environment variables from the Windows host; I know that at least COMSPEC is always set by Windows since way back in DOS, and that its picked up by Git-Bash, MSYS2, and Cygwin.

At least for me, when I installed ImageMagick as a standalone set of portable tools, I had to manually add it to the path. Once again, I can't speak definitively, but from what my PATH looks like without my user-specific modifications, it appears that the majority of programs that add themselves to the path do so by appending; it's possible that using an installer instead would have prepended instead, but I wouldn't count on it.

Bizarre that the Debian package doesn't provide the magick executable though. I know for a fact that the ImageMagick package on Arch certainly does, but Arch definitely isn't Debian. A brief glimpse in the source tarball from ImageMagick's site also seems to indicate that building directly from source with configure && make && sudo make install && sudo ldconfig /usr/local/lib should create the magick executable, so I can only assume that the package as provided by the regular Debian Buster repository must be stripping the file out for some reason.

hackerb9 commented 3 years ago

COMPSPEC sounds good to me.

I don't think Debian is removing magick. They freeze software versions when they declare a "stable release". Their ImageMagick is from two years ago (with a bunch of security patches, of course).

hackerb9 commented 3 years ago

By the way: what is the sixel compatible terminal you're using for Microsoft Windows? I want to make sure I document it in the README.

StaticPH commented 3 years ago

Their ImageMagick is from two years ago

Yeah, I can imagine that might be the reason, though it still doesn't explain why a version newer than 6.0.x lacks the magick binary... I'll switch to checking for COMSPEC and setting an alias accordingly.

By the way: what is the sixel compatible terminal you're using for Microsoft Windows? I want to make sure I document it in the README.

I use mintty (which you already have listed in the README), as provided by MSYS2, with TERM=xterm-256colorand COLORTERM=truecolor, though I tested and it seems to also work with TERM set to xterm, mintty, mintty-direct.

StaticPH commented 2 years ago

On a side note, I just learned that Intel® Embree also has its own convert executable (although at least when installing the mingw64 package for it, the executable is renamed to avoid the conflict with ImageMagick, and consequently the Windows utility as well).

hackerb9 commented 2 years ago

I have simplified your solution, but hopefully not too much. Please check my "windowsconvert" branch and tell me if it works. Also, please let me know which versions of Microsoft Windows you are able to test it under. Thanks!

StaticPH commented 2 years ago

I left a comment on the commit.

SonaliBendre commented 2 years ago

I use mintty (which you already have listed in the README), as provided by MSYS2, with TERM=xterm-256colorand COLORTERM=truecolor, though I tested and it seems to also work with TERM set to xterm, mintty, mintty-direct.

How to set COLORTERM? I don't find it in mintty(git bash) settings. Environment Variable in windows or .bashrc?

SonaliBendre commented 2 years ago

@StaticPH Also can you please write a guide how to use sixel in Windows?

hackerb9 commented 2 years ago

@SonaliBendre wrote:

@StaticPH wrote:

I use mintty (which you already have listed in the README), as provided by MSYS2, with TERM=xterm-256colorand COLORTERM=truecolor, though I tested and it seems to also work with TERM set to xterm, mintty, mintty-direct.

How to set COLORTERM? I don't find it in mintty(git bash) settings. Environment Variable in windows or .bashrc?

lsix does not use COLORTERM. It asks the user's terminal directly if it supports sixel, so there should be no need to change any environment variables.

@SonaliBendre wrote:

@StaticPH Also can you please write a guide how to use sixel in Windows?

@j4james: Does there already exist a Microsoft Windows specific How To guide that we can point this budding sixel neophyte to? I figure, if such a resource exists, you would surely know of it.

j4james commented 2 years ago

@j4james: Does there already exist a Microsoft Windows specific How To guide that we can point this budding sixel neophyte to?

Not that I'm aware of. But if you're using Mintty with wsltty, it should just work out of the box. As a simple test, something like img2sixel should be able to display an image. Or executing lsix in a directory of image files should display a list of their thumbnails. There's nothing special you need to do to enable sixel support.

If you're not using WSL, though, I'm not sure how well it would work. That's not something I've tried.