Skip to content

[LLDB][test] Improve SHELL detection on Windows in Makefile.rules #99532

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

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

dzhidzhoev
Copy link
Member

@dzhidzhoev dzhidzhoev commented Jul 18, 2024

In MinGW make, the %windir% variable has a lowercase name $(windir) when launched from cmd.exe, and $(WINDIR) name when launched from MSYS2, since MSYS2 represents standard Windows environment variables names in upper case.

This commit makes Makefile.rules consider both run variants.

@llvmbot
Copy link
Member

llvmbot commented Jul 18, 2024

@llvm/pr-subscribers-lldb

Author: Vladislav Dzhidzhoev (dzhidzhoev)

Changes

Since Windows 10 the case of 'windir' env variable was changed. Such error appears without that change:

make: \system32\cmd.exe: Command not found

Makefile.rules:628: recipe for target 'main.o' failed

Full diff: https://github.com/llvm/llvm-project/pull/99532.diff

1 Files Affected:

  • (modified) lldb/packages/Python/lldbsuite/test/make/Makefile.rules (+3-2)
diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index 3d562285ce9cc..4ade8c16b8e6f 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -80,8 +80,9 @@ endif
 # Also reset BUILDDIR value because "pwd" returns cygwin or msys path
 # which needs to be converted to windows path.
 #----------------------------------------------------------------------
-ifeq "$(OS)" "Windows_NT"
-	SHELL = $(WINDIR)\system32\cmd.exe
+ifeq "$(HOST_OS)" "Windows_NT"
+	# Windows 10 and later has the lower-case 'windir' env variable.
+	SHELL := $(or $(windir),$(WINDIR),C:\WINDOWS)\system32\cmd.exe
 	BUILDDIR := $(shell echo %cd%)
 endif
 

In MinGW make, %windir% variable has a lowercase name $(windir) when
launched from cmd.exe, and $(WINDIR) name when launched from MSYS2,
since MSYS2 represents standard Windows environment variables in upper
case.

This commit makes Makefile.rules consider both run variants.
@dzhidzhoev dzhidzhoev force-pushed the lldb/win-shell-detection branch from 3ed6eec to 17f92b9 Compare July 18, 2024 20:18
@dzhidzhoev dzhidzhoev changed the title [LLDB][test] Fix cmd.exe detection on recent Windows versions [LLDB][test] Improve SHELL detection on Windows in Makefile.rules Jul 18, 2024
@dzhidzhoev
Copy link
Member Author

Sorry, the initial issue this commit resolves was identified wrongly, fixed the commit description.

Copy link
Collaborator

@labath labath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@dzhidzhoev dzhidzhoev merged commit 243af2f into llvm:main Jul 19, 2024
6 checks passed
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
…9532)

Summary:
In MinGW make, the `%windir%` variable has a lowercase name `$(windir)`
when launched from cmd.exe, and `$(WINDIR)` name when launched from
MSYS2, since MSYS2 represents standard Windows environment variables
names in upper case.

This commit makes Makefile.rules consider both run variants.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251450
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants