LPGhatguy / aftman

Aftman, the prodigal sequel to Foreman
MIT License
157 stars 16 forks source link

Weird behaviour depending on the capitalisation of executable suffix #54

Closed JohnnyMorganz closed 4 months ago

JohnnyMorganz commented 12 months ago

On Windows, sometimes the path to an aftman managed executable is returned by tools (such as node-which) with a capitalised suffix .EXE, e.g.: C:\Users\Development\.aftman\bin\stylua.EXE

I'm not sure exactly why its returned like this sometimes, but I've seen it often occur for users, and myself.

When attempting to run this executable directly using the path, there can be odd behaviour. For example, in cmd:

C:\Users\Development\Downloads\project>C:\Users\Development\.aftman\bin\stylua.EXE --version
stylua 0.18.1

But in powershell:

PS C:\Users\Development\Downloads\project> C:\Users\Development\.aftman\bin\stylua.EXE --version
Aftman error: Tried to run an Aftman-managed version of stylua.EXE, but no aftman.toml files list this tool.
To run stylua.EXE from this directory, add it to one of these files:
- C:\Users\Development\Downloads\project\aftman.toml
- C:\Users\Development\.aftman\aftman.toml

But C:\Users\Development\Downloads\project\aftman.toml contains the contents:

# This file lists tools managed by Aftman, a cross-platform toolchain manager.
# For more information, see https://github.com/LPGhatguy/aftman

# To add a new tool, add an entry to this table.
[tools]
rojo = "rojo-rbx/rojo@7.3.0"
selene = "Kampfkarren/selene@0.25.0"
wally = "UpliftGames/wally@0.3.2"
luau-lsp = "JohnnyMorganz/luau-lsp@1.23.0"
stylua = "JohnnyMorganz/StyLua@0.18.1"
wally-package-types = "JohnnyMorganz/wally-package-types@1.2.1"

I would expect the tool to always run fine, even with .EXE as a suffix.

Looking at the code, the offending issue seems to be current_exe_name() not stripping the .EXE suffix like it does for .exe, meaning aftman doesn't find it in manfiest.tools vector

https://github.com/LPGhatguy/aftman/blob/412d5b5a54971a881bfd99ebd7976b48cf5dedfe/src/main.rs#L73C1-L83C2

LPGhatguy commented 12 months ago

That's a funny bug! I wonder where those tools are conjuring a different extension from.

I think a reasonable fix would be to also try to strip the uppercase version of EXE_SUFFIX. I want to avoid introducing new bugs on platforms that tend to have case-sensitive filesystems if we can. What do you think about that solution?

JohnnyMorganz commented 8 months ago

Sorry for the delay in responding!

I wonder where those tools are conjuring a different extension from.

Looks to just be how node-which defines the extension on windows platform https://github.com/npm/node-which/blob/25c5191ec4e29d8f83028fae5f52c0182d006d96/lib/index.js#L33

What do you think about that solution?

Sounds reasonable to me. Maybe even gate it to only windows platforms?