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

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

Open
radioflash opened this issue Feb 14, 2024 · 4 comments
Open

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

radioflash opened this issue Feb 14, 2024 · 4 comments

Comments

@radioflash
Copy link

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
Copy link
Member

Biswa96 commented Feb 14, 2024

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
Copy link
Author

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:

  • I never run have to run gcc.exe, it's just gcc
  • I can do which gcc or even file $(which gcc) and it all works, without the extension ever being mentioned back to me

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
Copy link
Member

Biswa96 commented Feb 15, 2024

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants