[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: [e-mail]
        ReportedBy: [e-mail]
    Classification: Unclassified

Add support to cmd.exe for the && and || functionality - They are currently
parsed and handled ok, but not correctly processed

[Bug 32679] cmd.exe Add support for || and &&

http://bugs.winehq.org/show_bug.cgi?id=32679

Austin English  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |download

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

Mikhail Bystryantsev  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #1 from Mikhail Bystryantsev  ---
Still not implemented in 1.7.33.

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

Ken Sharp  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |source

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #2 from Mikhail Bystryantsev  ---
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.

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

Qian Hong  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #3 from [e-mail] ---
(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.

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #4 from Qian Hong  ---
(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!

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #5 from Qian Hong  ---
(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"

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #6 from Qian Hong  ---
Looks like we need to use a "local ERRORLEVEL" instead of "global ERRORLEVEL"
in order to support || and && correctly.

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #7 from Qian Hong  ---
(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.

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #8 from Qian Hong  ---
Created attachment 54016
  --> https://bugs.winehq.org/attachment.cgi?id=54016
Improved fix

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #9 from [e-mail] ---
I dont personally need this, but it would be nice to get this included in wine.
This bug reprot is missing keyword patch.

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #10 from Bruno Jesus  ---
(In reply to Qian Hong from comment #8)
> Created attachment 54016 [details]
> Improved fix

Hi, Qian. What about sending your patch upstream?

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #11 from [e-mail] ---
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

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

joaopa  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #12 from joaopa  ---
Does the bug still occur with wine-4.0-rc7?

[Bug 32679] cmd.exe Add support for || and &&

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #13 from [e-mail] ---
(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.

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

Zebediah Figura  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #14 from Zebediah Figura  ---
*** Bug 39808 has been marked as a duplicate of this bug. ***

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

Lois Di Qual  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #15 from Lois Di Qual  ---
*** Bug 47803 has been marked as a duplicate of this bug. ***

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #16 from joaopa  ---
windows XP: expected expected
Wine:expected expected unexpected.

So I assume the bug still occurs with wine-5.14.

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #17 from [e-mail] ---
(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.

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #18 from joaopa  ---
At first, you could try to send your test to wine-devel. It would be a good
start.

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

Temtaime  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #19 from Temtaime  ---
So in almost 2k21 wine still doesn't support || operator?
When can we expect this fixed?

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #20 from Zebediah Figura  ---
(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.

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #21 from Temtaime  ---
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?

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

Gijs Vermeulen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #22 from Gijs Vermeulen  ---
*** Bug 51402 has been marked as a duplicate of this bug. ***

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #23 from Austin English  ---
Still present in 8.10.

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

Alex Henrie  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #24 from Alex Henrie  ---
*** Bug 55489 has been marked as a duplicate of this bug. ***

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

Eric Pouech  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #25 from Eric Pouech  ---
*** Bug 56820 has been marked as a duplicate of this bug. ***

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

Eric Pouech  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
             Status|NEW                         |ASSIGNED

--- Comment #26 from Eric Pouech  ---
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.

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

Eric Pouech  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #28 from Eric Pouech  ---
closing as implemented in Wine 9.13

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

--- Comment #27 from Eric Pouech  ---
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

[Bug 32679] cmd.exe Add support for || and &&

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=32679

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #29 from Alexandre Julliard  ---
Closing bugs fixed in 9.14.