[PATCH v2 0/1] MR6891: cmd: Use CD /D to change directory in POPD (same as PUSHD).

[huangqinjin (＠huangqinjin)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57137

--
  v2: cmd: Use CD /D to change directory in POPD (same as PUSHD).

https://gitlab.winehq.org/wine/wine/-/merge_requests/6891

[PATCH v2 1/1] cmd: Use CD /D to change directory in POPD (same as PUSHD).

[huangqinjin]

From: huangqinjin

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57137
---
 programs/cmd/builtins.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c
index d7c5090d17f..a57055a6d97 100644
--- a/programs/cmd/builtins.c
+++ b/programs/cmd/builtins.c
@@ -1806,6 +1806,8 @@ RETURN_CODE WCMD_pushd(const WCHAR *args)

RETURN_CODE WCMD_popd(void)
 {
+    RETURN_CODE return_code;
+    int errorlevel_backup;
     struct env_stack *temp = pushd_directories;

if (!pushd_directories)
@@ -1813,10 +1815,16 @@ RETURN_CODE WCMD_popd(void)

/* pop the old environment from the stack, and make it the current dir */
     pushd_directories = temp->next;
-    SetCurrentDirectoryW(temp->strings);
+    /* Change directory using CD code with /D parameter */
+    lstrcpyW(quals, L"/D");
+    errorlevel_backup = errorlevel;
+    return_code = WCMD_setshow_default(temp->strings);
+    if (return_code == NO_ERROR) {
+      errorlevel = errorlevel_backup;
+    }
     free(temp->strings);
     free(temp);
-    return NO_ERROR;
+    return return_code;
 }

/****************************************************************************

[PATCH v2 1/1] cmd: Use CD /D to change directory in POPD (same as PUSHD).

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=149920

Your paranoid android.

=== debian11b (64 bit WoW report) ===

user32:
win.c:4070: Test failed: Expected active window 0000000003E40168, got 0000000000000000.
win.c:4071: Test failed: Expected focus window 0000000003E40168, got 0000000000000000.

[eric pouech (＠epo)]

the use of /D option for popd looks ok to me

I would like to see tests for popd in case the directory saved in the pushd command no longer exists (to validate that the return_code value is the correct one)

* to be added to both test_builtins.bat and test_builtins.cmd in the success/failure PUSHD/POPD part

quick local test here show that the return_code is not the same as using CD to a non existing directory

also saving & restoring errorlevel in popd looks quite ugly ; it's likely preferable to introduce a helper to do the actual work of changing directory, and handle errorlevel outside of the helper