rmyorston / busybox-w32

WIN32 native port of BusyBox.
https://frippery.org/busybox
Other
674 stars 124 forks source link

`tar` doesn't support `$BB_OVERRIDE_APPLETS` #376

Closed Un1q32 closed 10 months ago

Un1q32 commented 10 months ago

This is a major problem if you want to create xz or lzma archives, since the builtin applets don't support compression with those formats, only decompression. You can put better versions in your $PATH but tar can't use them. Of course you can always just make an uncompressed tar and compress it after, but it would be nice for the -J and --lzma options to just work

rmyorston commented 10 months ago

I thought the problem with xz and lzma was fixed as a result of issue #222. The first release with that fix was FRP-4264. I've just tried the current release and was able to create an xz compressed tar file using an external binary.

Un1q32 commented 10 months ago

I tried with both the latest release and latest prerelease

Screenshot (4)

Un1q32 commented 10 months ago

The same thing happens in ash with BB_OVERRIDE_APPLETS="xz lzma"

rmyorston commented 10 months ago

It shouldn't be necessary to set BB_OVERRIDE_APPLETS in this situation. When a compressor is required the code checks whether it's xz or lzma. If it is, and if it would be run as an applet, it creates a command line to force CreateProcess() to do a path lookup.

I've tried to replicate your setup by extracting the external xv to ~/xv and copying busybox.exe to ~/bin/xv.exe. Then I set PATH to add those two directories, in that order, so the external xv will be found before the one in ~/bin. BB_OVERRIDE_APPLETS wasn't set.

~ $ export PATH="$PATH;c:/users/rmy/xz/bin_i686;c:/users/rmy/bin"
~ $ which -a xz
xz
c:/users/rmy/xz/bin_i686/xz.exe
c:/users/rmy/bin/xz.exe
~ $ tar cJvf test.txz  testfile
testfile
~ $

Even though which -a shows that the built-in xv takes precedence tar correctly runs the external one.

I've built a binary from current Git HEAD with a print statement to output the compressor command: busybox_compress.exe. When I run tar using this binary I get:

~ $ ./busybox_compressor tar cJvf test.txz testfile
fork compressor 'xz -cf -'
testfile
~ $

I've no idea why it's not working for you. Can you share the contents of PATH on your system?

Un1q32 commented 10 months ago
C:\Users\Joey\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Program Files\Git\cmd;C:\Windows\System32\OpenSSH\;C:\Users\Joey\AppData\Local\BusyBox;C:\Users\Joey\AppData\Local\Microsoft\WindowsApps; 
Un1q32 commented 10 months ago

unsetting BB_OVERRIDE_APPLETS does not fix the issue

rmyorston commented 10 months ago

Hmm. which -a tells us how the shell in busybox-w32 would find binaries. To discover how Windows would do the same we should use where.

If I do that, and if I have an xz.exe binary (as a copy of busybox.exe) in my current directory I get this behaviour:

~ $ where xz
C:\Users\rmy\xz.exe
c:\Users\rmy\xz\bin_i686\xz.exe
c:\Users\rmy\bin\xz.exe
~ $ ./busybox_compressor tar cJvf test.txz testfile
fork compressor 'xz -cf -'
testfile
BusyBox v1.37.0.git (2023-11-12 19:08:15 GMT)

Usage: xz -d [-cfk] [FILE]...

Decompress FILEs (or stdin)

        -d      Decompress
        -c      Write to stdout
        -f      Force
        -k      Keep input files
        -t      Test integrity
~ $

Do you, by any chance, have an xz.exe in your current directory? If so, Windows will run that before checking elsewhere on PATH.

Un1q32 commented 10 months ago

There is no xz.exe in my current directory when running the command. More info that might be useful: I am running Windows 10 LTSC IoT Enterprise 2021 on a Steam Deck, I am using the latest 64 bit prerelease as of creating the issue, The official release I mentioned testing is also the 64 bit version, I haven't tested with 32 bit versions, maybe tomorrow

Un1q32 commented 10 months ago

last 32 bit release has the same issue

rmyorston commented 10 months ago

What does where xz report? The current directory isn't the only place Windows looks before checking PATH.

Un1q32 commented 10 months ago

image

rmyorston commented 10 months ago

I still don't understand what's going on here. I've made new test binaries: busybox_compress.exe and busybox_compress64.exe. These print the path of the binary CreateProcess() actually runs and the command line.

When I use one of those to run tar with an external xz.exe on PATH (that is, the case where it works for me) I get:

~ $ ./busybox_compressor tar cJvf test.txz testfile
\Device\HarddiskVolume2\Users\rmy\xz\bin_i686\xz.exe (xz -cf -)
testfile
~ $

Could you try this on your system?

For the record, the patch to add the debug output is:

diff --git a/win32/popen.c b/win32/popen.c
index 2208aa6bb..09cf100ee 100644
--- a/win32/popen.c
+++ b/win32/popen.c
@@ -1,5 +1,7 @@
 #include <fcntl.h>
 #include "libbb.h"
+#include <psapi.h>
+#include "lazyload.h"
 #include "NUM_APPLETS.h"

 typedef struct {
@@ -190,6 +192,8 @@ static int mingw_popen_internal(pipe_data *p, const char *cmd,
    int success;
    int fd = -1;
    int ip, ic, flags;
+   DECLARE_PROC_ADDR(DWORD, GetProcessImageFileNameA, HANDLE,
+                       LPSTR, DWORD);

    if ( cmd == NULL || *cmd == '\0' || mode == NULL ) {
        return -1;
@@ -260,6 +264,14 @@ static int mingw_popen_internal(pipe_data *p, const char *cmd,
        goto finito;
    }

+   if(INIT_PROC_ADDR(psapi.dll, GetProcessImageFileNameA)) {
+       char exepath[PATH_MAX];
+       HANDLE proc = p->piProcInfo.hProcess;
+       if (GetProcessImageFileNameA(proc, exepath, sizeof(exepath))) {
+           fprintf(stderr, "%s (%s)\n", exepath, cmd);
+       }
+   }
+
    /* close child end of pipe */
    CloseHandle(p->pipe[ic]);
    p->pipe[ic] = INVALID_HANDLE_VALUE;
Un1q32 commented 10 months ago

\Device\HarddiskVolume3\Users\Joey\bin\xz.exe (xz -cf -)

I think I figured out the issue.

I noticed the issue didn't happen if I ran busybox from inside the current directory. Then I noticed it didn't happen if I moved my normal busybox.exe to a different directory in my PATH. Then I found the issue didn't happen if I deleted the xz.exe symlink inside my busybox directory. Finally if I have a busybox.exe and xz.exe symlink in one directory that is not in my PATH or the current directory, and I run that busybox by specifying the path then it will run that busyboxes xz. It seems if busybox has an xz.exe in the same directory as itself it will always run that.

rmyorston commented 10 months ago

Splendid! That seems to be the answer. When CreateProcess() tries to run a command without an explicit path being specified it first looks in the same directory as the current process. That seems to cover the situation you've described.

I'll consider what can be done to work around this in busybox-w32. I suspect more code will be required to deal with Microsoft's rather strange ideas.

rmyorston commented 10 months ago

The latest prerelease binaries should have a fix for this issue. busybox-w32 now performs its own search for the compressor binary instead of relying on the odd behaviour of CreateProcess().

Un1q32 commented 10 months ago

fixed

xparq commented 10 months ago

I think the lookup feature of CreateProces is reasonable: if your app bundles a bunch of child executables, you have nothing else to do for a working deployment.

It's also aligned with how you don't need any setup for application-bundled DLLs in the app exe dir to be always just found, no matter what.

(The lack of control to occasionally override an app can be annoying though, indeed.)