[Bug 32679] New: cmd.exe Add support for || and &&
http://bugs.winehq.org/show_bug.cgi?id=32679 Bug #: 32679 Summary: cmd.exe Add support for || and && Product: Wine Version: 1.5.21 Platform: x86 OS/Version: Linux Status: NEW Severity: normal Priority: P2 Component: cmd AssignedTo: wine-bugs(a)winehq.org ReportedBy: us(a)edmeades.me.uk Classification: Unclassified Add support to cmd.exe for the && and || functionality - They are currently parsed and handled ok, but not correctly processed -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=32679 Austin English <austinenglish(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |download -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 Mikhail Bystryantsev <ho-rr-or(a)mail.ru> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ho-rr-or(a)mail.ru -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #1 from Mikhail Bystryantsev <ho-rr-or(a)mail.ru> --- Still not implemented in 1.7.33. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 Ken Sharp <imwellcushtymelike(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |source -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #2 from Mikhail Bystryantsev <ho-rr-or(a)mail.ru> --- Created attachment 52201 --> https://bugs.winehq.org/attachment.cgi?id=52201 Possible fix Some time ago a tried to contribute this patch, but it has not been reviewed. I hope it will be helpful at least here. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 Qian Hong <fracting(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fracting(a)gmail.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 super_man(a)post.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |super_man(a)post.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #3 from super_man(a)post.com --- (In reply to Mikhail Bystryantsev from comment #2)
Created attachment 52201 [details] Possible fix
Some time ago a tried to contribute this patch, but it has not been reviewed. I hope it will be helpful at least here.
"Qian Hong 2016-03-20 12:46:30 CET BTW, after applying the patch, Qt5 seems builds fine. Qt4 doesn't need this patch to build." So your patch is tested against at least 1 application. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #4 from Qian Hong <fracting(a)gmail.com> --- (In reply to Mikhail Bystryantsev from comment #2)
Created attachment 52201 [details] Possible fix
Some time ago a tried to contribute this patch, but it has not been reviewed. I hope it will be helpful at least here.
Hi Mikhail, thanks a lot for the patch, it still applies on today's Wine, but the test seems doesn't pass: --- snip --- ../../../tools/runtest -q -P wine -T ../../.. -M cmd.exe -p cmd.exe_test.exe.so batch && touch batch.ok batch.c:313: Test succeeded inside todo block: unexpected char 0x0 position -1 in line 414 (got 'foo3 not created', wanted 'foo3 not created') batch.c:313: Test failed: unexpected char 0x2d position 0 in line 416 (got '--- on failure conditional or', wanted 'foo4') --- snip --- Would you please have a look? Thanks! -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #5 from Qian Hong <fracting(a)gmail.com> --- (In reply to Qian Hong from comment #4)
--- snip --- ../../../tools/runtest -q -P wine -T ../../.. -M cmd.exe -p cmd.exe_test.exe.so batch && touch batch.ok batch.c:313: Test succeeded inside todo block: unexpected char 0x0 position -1 in line 414 (got 'foo3 not created', wanted 'foo3 not created') batch.c:313: Test failed: unexpected char 0x2d position 0 in line 416 (got '--- on failure conditional or', wanted 'foo4') --- snip ---
Simple case to reproduce: 1. Apply the patch 2. Run cmd 3. both `echo foo && echo bar` and `echo foo || echo bar` works fine 4. raise some error, for example, run `nonexistentcommand` in cmd 5. both `echo foo && echo bar` and `echo foo || echo bar` behaviors wrong this time, the results are inverse. `echo foo && echo bar` prints "foo", `echo foo || echo bar` prints "foo" and "bar" -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #6 from Qian Hong <fracting(a)gmail.com> --- Looks like we need to use a "local ERRORLEVEL" instead of "global ERRORLEVEL" in order to support || and && correctly. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #7 from Qian Hong <fracting(a)gmail.com> --- (In reply to Qian Hong from comment #6)
Looks like we need to use a "local ERRORLEVEL" instead of "global ERRORLEVEL" in order to support || and && correctly.
Update: some builtin commands like `echo` does not change ERRORLEVEL, those commands should always be treated as success, we need to change the logic to ignore ERRORLEVEL from previous command when handling these kinds of special builtin commands. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #8 from Qian Hong <fracting(a)gmail.com> --- Created attachment 54016 --> https://bugs.winehq.org/attachment.cgi?id=54016 Improved fix -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 winetest(a)luukku.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |winetest(a)luukku.com --- Comment #9 from winetest(a)luukku.com --- I dont personally need this, but it would be nice to get this included in wine. This bug reprot is missing keyword patch. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #10 from Bruno Jesus <00cpxxx(a)gmail.com> --- (In reply to Qian Hong from comment #8)
Created attachment 54016 [details] Improved fix
Hi, Qian. What about sending your patch upstream? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 alexcoffin1999(a)gmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alexcoffin1999(a)gmail.com --- Comment #11 from alexcoffin1999(a)gmail.com --- Created attachment 59159 --> https://bugs.winehq.org/attachment.cgi?id=59159 A path that adds support for "||" and "&&" with errorlevel checking Adds the support for correct error checking and should work with multiple layers of "call"s with "&&"s and "||"s. Also I didn't fix a Windows bug I noticed where it resets the errorlevel to "1" after a specific set of operational conditions. Feel free to add that. This is my first path for Wine so sorry if I make any mistakes :) Also here is the link to the Wine TestBot job if that helps: https://testbot.winehq.org/JobDetails.pl?Key=33077 The following is copied from the commit msg: Added support for "&&" and "||" condition checking Before "&&" and "||" were supported, but they also didn't check if past commands had failed. This commit removes direct access to errorlevel. Now errorlevel can only be accessed using getters and setters, but the it seemed like the most efficient solution. Also please note I added one TODO task. Currently this commit doesn't replicate a certain windows bug. See test_builtins.cmd for more details. Also tests in test_builtins.cmd were added in order to validate these changes. Finally I think I changed some random tabs to spaces in one file. Hopefully that doesn't cause commit issues -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 joaopa <jeremielapuree(a)yahoo.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jeremielapuree(a)yahoo.fr --- Comment #12 from joaopa <jeremielapuree(a)yahoo.fr> --- Does the bug still occur with wine-4.0-rc7? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #13 from alexcoffin1999(a)gmail.com --- (In reply to joaopa from comment #12)
Does the bug still occur with wine-4.0-rc7?
Yep just try "echo expected && echo expected || echo unexpected" It was more complicated to properly implement than I had initially thought and then I became busy. It turns out that batch followed the usual programming language rules somewhat (if I recall correctly), I only note the usual part since it usually doesn't, and had operator precedence and parentheses correctly handled for "&&" and "||" along with somehow handling "&" and "|" (I forget how). My *very old* patch had only dealt with the section on global ERRORLEVEL as opposed to operator precedence, parentheses, etc. and was rejected (rightly so) for those reasons. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 Zebediah Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fiendishx(a)gmail.com --- Comment #14 from Zebediah Figura <z.figura12(a)gmail.com> --- *** Bug 39808 has been marked as a duplicate of this bug. *** -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 Lois Di Qual <lois.diqual(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lois.diqual(a)gmail.com --- Comment #15 from Lois Di Qual <lois.diqual(a)gmail.com> --- *** Bug 47803 has been marked as a duplicate of this bug. *** -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #16 from joaopa <jeremielapuree(a)yahoo.fr> --- windows XP: expected expected Wine:expected expected unexpected. So I assume the bug still occurs with wine-5.14. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #17 from alexcoffin1999(a)gmail.com --- (In reply to joaopa from comment #16)
windows XP: expected expected Wine:expected expected unexpected.
So I assume the bug still occurs with wine-5.14.
I haven't checked but I almost guarentee it. Yeah the issue is really that the whole cmd.exe needs to be rewritten. I have some notes I could send you, but they mostly are covered by this stackoverflow post. WARNING: the post is incorrect about a few parts of the processing, and order (I think I noticed because if you break a command across multiple lines with new lines in variable expansion or other edges cases it doesn't quite process how he claims it would). https://stackoverflow.com/questions/4094699/how-does-the-windows-command-int... If people actually care about this issue I could bite the bullet and do it, but it seemed like a lot of work for something no one cares about. I would recommend checking if ReactOS has a better implementation of cmd.exe before starting from scratch though. Let me know if you want my other notes, and test programs. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #18 from joaopa <jeremielapuree(a)yahoo.fr> --- At first, you could try to send your test to wine-devel. It would be a good start. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 Temtaime <temtaime(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |temtaime(a)gmail.com --- Comment #19 from Temtaime <temtaime(a)gmail.com> --- So in almost 2k21 wine still doesn't support || operator? When can we expect this fixed? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #20 from Zebediah Figura <z.figura12(a)gmail.com> --- (In reply to Temtaime from comment #19)
So in almost 2k21 wine still doesn't support || operator? When can we expect this fixed?
When someone sends a patch to fix it. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #21 from Temtaime <temtaime(a)gmail.com> --- I successfully run windows xp's cmd.exe under wine and it works like charm. I can run it using absolute path, however, when i call cmd.exe wine uses its built-in version of cmd.exe I already had repace c:\windows\(system32|syswow64)\cmd.exe with xp's one How can i stop wine from using its built-it version of cmd.exe? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 Gijs Vermeulen <gijsvrm(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |AlvinSeville7cf(a)gmail.com --- Comment #22 from Gijs Vermeulen <gijsvrm(a)gmail.com> --- *** Bug 51402 has been marked as a duplicate of this bug. *** -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #23 from Austin English <austinenglish(a)gmail.com> --- Still present in 8.10. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 Alex Henrie <alexhenrie24(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mr.dmitry.sokolov(a)gmail.com --- Comment #24 from Alex Henrie <alexhenrie24(a)gmail.com> --- *** Bug 55489 has been marked as a duplicate of this bug. *** -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 Eric Pouech <eric.pouech(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nemos200408(a)gmail.com --- Comment #25 from Eric Pouech <eric.pouech(a)gmail.com> --- *** Bug 56820 has been marked as a duplicate of this bug. *** -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 Eric Pouech <eric.pouech(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |eric.pouech(a)gmail.com Status|NEW |ASSIGNED --- Comment #26 from Eric Pouech <eric.pouech(a)gmail.com> --- support for these operators has been added to Wine 9.12 Still require to update all the internal commands to report success/failure. So not closing yet. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 Eric Pouech <eric.pouech(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |RESOLVED Resolution|--- |FIXED --- Comment #28 from Eric Pouech <eric.pouech(a)gmail.com> --- closing as implemented in Wine 9.13 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 --- Comment #27 from Eric Pouech <eric.pouech(a)gmail.com> --- support for || and && operators should be fully operational in wine 9.13 I'll close this bug if someone finds issues from wine 9.13 & upwards, please file a new bug report for the failing case -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=32679 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #29 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 9.14. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (2)
-
wine-bugs@winehq.org -
WineHQ Bugzilla