-
Notifications
You must be signed in to change notification settings - Fork 231
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
sh is borked on 2.0.0 #162
Comments
You probably need the unicode variant of busybox-w32 for that, but w64dk distributes a non-unicode build, because the unicode build doesn't run on windows XP. You probably can't use the unicode variant (from https://frippery.org/busybox/) as a drop-in replacement for your existing busybox.exe, because w64dk uses a custom build which disables some utilities in busybox (like But with some effort, you can replace busybox.exe with the unicode busybox variant, and exclude those utilities using the env var: BB_OVERRIDE_APPLETS=";ar make strings xxd" I'm not entirely sure which applets w64dk disables in busybox, but whatever they are, you should be able to add them to BB_OVERRIDE_APPLETS and then the unicode busybox.exe should work in w64dk. |
@avih |
I believe @avih is correct. Alternatively extract w64devkit to an ASCII-only real path. Even with Unicode-aware busybox-w32, other tools in the kit (e.g. Binutils) aren't wide-path capable and still may not work correctly on non-ASCII paths. |
@ale5000-git, see the discussion on 4f201b2. An external manifest works (with the usual SxS caching caveats), as does injecting it into the binary later, but I currently don't enable Unicode editing in the build, so you'll still have trouble editing non-ASCII commands. My comment on the MR that "interactive UTF-8 editing is still too broken" was incorrect, and addressed in the discussion. I just needed to enable an additional option to get the correct behavior. |
@skeeto maybe you should build a binary which is closer to upstream, at least as far as disabling applets go, and then exclude them using env vars as suggested above, so that it's easier for user to use different or newer busybox.exe binary? For instance if some critical issue is detected and upstream releases a new binary which fixes it - users can't use it as drop-in replacement for their current busybox.exe in w64dk, but it would be nice if they could. |
Why not provide an external UTF-8 manifest for a separate manual download? |
skeeto already replied to that, and additionally currently the busybox-w32 code needs to enable unicode at build time. I.e it doesn't support adding/removing the manifest with the same binary (though it should be possible, but currently the code doesn't do that). However, it should still be possible to distribute an additional unicode build. True that it's not fun, but it would be the simplest. Or just ensure that upstream unicode busybox.exe binary can be used as a drop-in replacement, and instruct users to do that if they need. |
I have only talked about an external manifest file, not to be merged but just placed near the exe. |
Having w64devkit define a suitable |
Sure, and I mentioned how to do that with existing setup. But it would need less patches in w64dk, which I consider to be better. |
Running something like this from an upstream
Using that information we can make export BB_OVERRIDE_APPLETS='[[ ar ascii dpkg dpkg-deb ftpget ftpput link make ma
n pdpmake rpm rpm2cpio strings tsort unlink vi xxd'
. "$W64DEVKIT_HOME/src/profile" Then replacing the w64devkit The overridden applets are unconditionally excluded to avoid unexpected surprises. They remain present, so if we want to use If @skeeto could be prevailed upon to set I'd certainly find that useful for testing purposes. |
What upstream support would be needed? to support a build-time embedded profile file? If yes, how would such upstream support help to use an upstream binary as a drop-in replacement? If no, do you mean w64dk-specific things in upstream busybox.exe? FWIW, I don't think that's appropriate. But mainly, w64dk already sets some env vars without any help of busybox-w32 that I know of (via the launcher), which I think would also work with upstream binary, so I don't think I'm seeing the need for any upstream suppport to add one more env var. |
To run
It wouldn't be necessary to provide a
Yes, and it sets some more via |
You mean to add an additional default relative path like this?
Yes, but this already works upstream, no? If relative I'd think both are equally simple to implement, but one of them keeps bb-w32 more generic than the other approach. |
As things stand w64devkit and busybox-w32 use different paths for their binary-relative profile scripts:
And the other requires w64devkit to put its script in a different place than it does now. Since I've already volunteered Chris to add the |
I don't think To me it looks like I'd think that If anything, you two should agree on a good generic relative path which w64dk can be comfortable to place init scripts, e.g. maybe |
I would prefer to set the If I switch to Regarding style and convention, I agree that |
Just an idea but it is also possible to create a shell file that set variables and load it from both w64devkit.exe and etc/profile, so it will work in every case. |
This will maybe fix one problem and generate 100 more, since it is global and the UTF-8 codepage doesn't work properly in many cases. But you can get a similar thing locally in a shell by running |
Today I realized a one-liner busybox-w32 patch would improve Unicode support without downsides: 3cacd96. On x64, GCC has had a UTF-8 manifest since w64dk 1.19 (first release with GCC 13), and this hasn't been a problem for Windows 7, the oldest target the x64 build supports. Unlike XP, Windows 7 (and earlier versions of Windows 10, and presumably Windows 8) loads the EXE but ignores the manifest. That's fine because ignoring the manifest means it gracefully degrades to the prior behavior (i.e. nothing lost). I can get the same from busybox-w32 by disabling the UTF-8 code page check. (However, I must again emphasize that parts of w64dk still do not understand Unicode paths and arguments: Binutils, GDB, Make, and Ctags.) |
IIRC not quite. While such mode is considered (utf8 manifest which falls back to ansi where the manifest is no-op, e.g. win7/8), the current code needs few changes before this can work, beyond the test to explicitly exit which you patched, because IIRC there's some code which hardcodes dependency on build time manifest. IIRC related to utf8 input/output (i.e. KB input, and unicode console output). So I expect this would not behave entirely correctly in ANSI mode on win7/8, mainly related to KB input and console output, but without recalling the specifics currently. To be more specific, I think pure ascii-7 input and output would work, but chars outside ascii-7 (in the OEM/ANSI codepage and console input/output codepage) which work-ish in non-unicode build might be broken in this auto-unicode build. |
Thanks for the heads up, @avih! I'll keep an eye out for issues. I didn't catch anything in the source, and the original commit (rmyorston/busybox-w32@9e2c3594) indicates the check is to "avoid raising false hopes." For me personally, mostly working beats flat refusal, and quiet is better than nagging, e.g. a hypothetical "warning: UTF8 is disabled!" on every start. (I've already accumulated four anti-nagging patches, and Cppcheck used to have the fifth.) |
I don't disagree, but as I mentioned, I think this will be broken for non-ascii chars I/O, which supposedly work correctly in the non-unicode build. It should not be too hard to fix, and I think we should do that (upstream) to support auto-unicode mode, but for now it's not yet supported fully. "supposedly" because IMO the non-unicode build is not perfect in this regard, but it still should be better than your patch. Also, without looking closely at the patch context, I think the unicode support report at the output of |
FYI current upstream discussion about "auto-unicode" starts here rmyorston/busybox-w32#423 (comment) |
Upstream: rmyorston/busybox-w32#447
If it helps, I'm running Windows 11 21H2.
The text was updated successfully, but these errors were encountered: