msys2 / msys2-pacman

A friendly fork of https://gitlab.archlinux.org/pacman/pacman
GNU General Public License v2.0
21 stars 12 forks source link

Ownership check (-Qo) chokes on executable file extension #36

Open radioflash opened 4 months ago

radioflash commented 4 months ago

If I want to check which package an executable is from, then it only works if I fully specify the file extension (e.g. ".exe").

Works

❯ pacman -Qo ntldd.exe /ucrt64/bin/ntldd.exe is owned by mingw-w64-ucrt-x86_64-ntldd

Fails

❯ pacman -Qo ntldd error: No package owns /ucrt64/bin/ntldd

Why this should be fixed

This is highly misleading, because it basically pretends that the file in question did not come from a package at all; if it was not intended to work, then it should NOT do the path lookup, because currently it basically acknowledges to the user that the file was found (without the user specifying the full path NOR the extension) and THEN fails if the extension is missing.

TL;DR: It would be non-ideal, but acceptable if pacman -Qo ntldd would return "error: No package owns ntldd" in the above example (note: output name is the same as user query).

Possible fix

My approach to fixing this would be to add extension awareness (specifically: ".exe" only) to lrealpath (in https://github.com/msys2/msys2-pacman/blob/master/src/pacman/query.c#L101), which does path canonicalization already.

Questions

Biswa96 commented 4 months ago

This is highly misleading, because it basically pretends that the file in question did not come from a package at all

It is not misleading at all. ntldd and ntldd.exe are two different file and ntldd does not exist.

Does the suggested fix make sense, or should this be fixed differently?

No, it is the expected behavior. For example,

$ pacman -Qo mpv
/mingw64/bin/mpv is owned by mingw-w64-x86_64-mpv 0.37.0-1

$ pacman -Qo mpv.exe
/mingw64/bin/mpv.exe is owned by mingw-w64-x86_64-mpv 0.37.0-1

In the above example, mpv is a shell script whereas mpv.exe is a portable executable.

$ file mpv.exe
mpv.exe: PE32+ executable (GUI) x86-64 (stripped to external PDB), for MS Windows, 12 sections

$ file mpv
mpv: Bourne-Again shell script, ASCII text executable
radioflash commented 4 months ago

Just to clarify, I'd not want to change the behavior in your example (where there's a wrapper script AND an .exe file).

My problem is that when using "pacman -Qo" for commands, I typically don't know or care if they have an .exe extension, and MSYS behavior actively enables and reinforces this in basically all other places:

But this fails hard with pacman -Qo, and it fails deceptively because the output indicates that pacman -Qo did find the command in question (e.g. by resolving ntldd to /ucrt64/bin/ntldd)-- but it then pretends that the file does not belong to any package.

I used to believe that pacman -Qo was broken/unreliable on windows for a long time because of this issue...

Would you at least be willing to consider merging a PR for this?

Biswa96 commented 4 months ago

I understand your perspective but I am not sure if that pacman behavior need to be changed. Please feel free to create a pull request. Others may share their views.

radioflash commented 4 months ago

I have a fix that is not too ugly! It is modelled after cygwin_spelling in an MSYS2 coreutils patch.

If this is acceptable, I could also just PR the necessary changes in MSYS2-packages (because I have those for testing already anyways).

Note: Lots of tests fail for pacman right now, but these are currently ignored. Thus I have not looked into adding unit tests for the added .exe suffix checking.