Reapply "[lit] Implement builtin umask (#94621)" #155850
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit faa4e35. This was originally reverted because it was using a Python 3.9 feature (umask in subprocess.Popen) when LLVM only requires Python 3.8. This patch uses os.umask instead, which has been around for longer.
|
@llvm/pr-subscribers-debuginfo @llvm/pr-subscribers-llvm-binary-utilities Author: Aiden Grossman (boomanaiden154) ChangesThis reverts commit faa4e35. This was originally reverted because it was using a Python 3.9 feature (umask in subprocess.Popen) when LLVM only requires Python 3.8. This patch uses os.umask instead, which has been around for longer. Full diff: https://github.com/llvm/llvm-project/pull/155850.diff 9 Files Affected:
diff --git a/llvm/test/tools/llvm-dwarfutil/ELF/X86/mirror-permissions-unix.test b/llvm/test/tools/llvm-dwarfutil/ELF/X86/mirror-permissions-unix.test
index a95d1c0aafa21..fdcba4dcd666b 100644
--- a/llvm/test/tools/llvm-dwarfutil/ELF/X86/mirror-permissions-unix.test
+++ b/llvm/test/tools/llvm-dwarfutil/ELF/X86/mirror-permissions-unix.test
@@ -3,7 +3,6 @@
## Setting the umask to 0 ensures deterministic permissions across
## test environments.
# UNSUPPORTED: system-windows
-# REQUIRES: shell
# RUN: touch %t
# RUN: chmod 0777 %t
diff --git a/llvm/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test b/llvm/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test
index 8f4993f4f3d29..66a481a2230d1 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test
@@ -6,7 +6,6 @@
## Setting the umask to 0 ensures deterministic permissions across
## test environments.
# UNSUPPORTED: system-windows
-# REQUIRES: shell
# RUN: touch %t
# RUN: chmod 0777 %t
diff --git a/llvm/test/tools/llvm-objcopy/ELF/respect-umask.test b/llvm/test/tools/llvm-objcopy/ELF/respect-umask.test
index 376e33a217819..02e9b93f5376f 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/respect-umask.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/respect-umask.test
@@ -1,10 +1,8 @@
## This tests that the umask is respected when
## assigning permissions of output files.
-## Windows has no umask so this test makes no sense, nor would
-## it work because there is no umask(1) in a Windows environment
+## Windows has no umask so this test makes no sense.
# UNSUPPORTED: system-windows
-# REQUIRES: shell
# RUN: rm -f %t
# RUN: touch %t
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 31e3ed679d431..36c19c1c86c75 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -98,9 +98,10 @@ class ShellEnvironment(object):
we maintain a dir stack for pushd/popd.
"""
- def __init__(self, cwd, env):
+ def __init__(self, cwd, env, umask=-1):
self.cwd = cwd
self.env = dict(env)
+ self.umask = umask
self.dirStack = []
def change_dir(self, newdir):
@@ -582,6 +583,20 @@ class SHFILEOPSTRUCTW(Structure):
return ShellCommandResult(cmd, "", stderr.getvalue(), exitCode, False)
+def executeBuiltinUmask(cmd, shenv):
+ """executeBuiltinUmask - Change the current umask."""
+ if os.name != "posix":
+ raise InternalShellError(cmd, "'umask' not supported on this system")
+ if len(cmd.args) != 2:
+ raise InternalShellError(cmd, "'umask' supports only one argument")
+ try:
+ # Update the umask in the parent environment.
+ shenv.umask = int(cmd.args[1], 8)
+ except ValueError as err:
+ raise InternalShellError(cmd, "Error: 'umask': %s" % str(err))
+ return ShellCommandResult(cmd, "", "", 0, False)
+
+
def executeBuiltinColon(cmd, cmd_shenv):
"""executeBuiltinColon - Discard arguments and exit with status 0."""
return ShellCommandResult(cmd, "", "", 0, False)
@@ -736,6 +751,7 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
"popd": executeBuiltinPopd,
"pushd": executeBuiltinPushd,
"rm": executeBuiltinRm,
+ "umask": executeBuiltinUmask,
":": executeBuiltinColon,
}
# To avoid deadlock, we use a single stderr stream for piped
@@ -757,7 +773,7 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
# env FOO=1 llc < %s | env BAR=2 llvm-mc | FileCheck %s
# env FOO=1 %{another_env_plus_cmd} | FileCheck %s
if cmd_shenv is shenv:
- cmd_shenv = ShellEnvironment(shenv.cwd, shenv.env)
+ cmd_shenv = ShellEnvironment(shenv.cwd, shenv.env, shenv.umask)
args = updateEnv(cmd_shenv, args)
if not args:
# Return the environment variables if no argument is provided.
@@ -902,6 +918,13 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
args = quote_windows_command(args)
try:
+ # TODO(boomanaiden154): We currently wrap the subprocess.Popen with
+ # os.umask as the umask argument in subprocess.Popen is not
+ # available before Python 3.9. Once LLVM requires at least Python
+ # 3.9, this code should be updated to use umask argument.
+ old_umask = -1
+ if cmd_shenv.umask != -1:
+ old_umask = os.umask(cmd_shenv.umask)
procs.append(
subprocess.Popen(
args,
@@ -916,6 +939,8 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
errors="replace",
)
)
+ if old_umask != -1:
+ os.umask(old_umask)
proc_not_counts.append(not_count)
# Let the helper know about this process
timeoutHelper.addProcess(procs[-1])
diff --git a/llvm/utils/lit/tests/Inputs/shtest-umask/lit.cfg b/llvm/utils/lit/tests/Inputs/shtest-umask/lit.cfg
new file mode 100644
index 0000000000000..52e18b1d5c2b3
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-umask/lit.cfg
@@ -0,0 +1,7 @@
+import lit.formats
+
+config.name = "shtest-umask"
+config.suffixes = [".txt"]
+config.test_format = lit.formats.ShTest(execute_external=False)
+if sys.platform.startswith("win") or sys.platform.startswith("cygwin"):
+ config.available_features.add("system-windows")
diff --git a/llvm/utils/lit/tests/Inputs/shtest-umask/umask-bad-arg.txt b/llvm/utils/lit/tests/Inputs/shtest-umask/umask-bad-arg.txt
new file mode 100644
index 0000000000000..639bfd4b7b4f3
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-umask/umask-bad-arg.txt
@@ -0,0 +1 @@
+# RUN: umask bad
diff --git a/llvm/utils/lit/tests/Inputs/shtest-umask/umask-ok.txt b/llvm/utils/lit/tests/Inputs/shtest-umask/umask-ok.txt
new file mode 100644
index 0000000000000..9d43efbddf2cf
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-umask/umask-ok.txt
@@ -0,0 +1,11 @@
+## Windows has no umask so this test makes no sense.
+# UNSUPPORTED: system-windows
+
+# RUN: touch %t
+# RUN: chmod 644 %t && ls -l %t | cut -f 1 -d ' ' > %t.644
+# RUN: chmod 600 %t && ls -l %t | cut -f 1 -d ' ' > %t.600
+# RUN: chmod 666 %t && ls -l %t | cut -f 1 -d ' ' > %t.666
+
+# RUN: umask 022 && rm %t && touch %t && ls -l %t | cut -f 1 -d ' ' | cmp - %t.644
+# RUN: umask 177 && rm %t && touch %t && ls -l %t | cut -f 1 -d ' ' | cmp - %t.600
+# RUN: umask 000 && rm %t && touch %t && ls -l %t | cut -f 1 -d ' ' | cmp - %t.666
diff --git a/llvm/utils/lit/tests/Inputs/shtest-umask/umask-too-many-args.txt b/llvm/utils/lit/tests/Inputs/shtest-umask/umask-too-many-args.txt
new file mode 100644
index 0000000000000..0b7461b5bc3fe
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-umask/umask-too-many-args.txt
@@ -0,0 +1 @@
+# RUN: umask 0 0
diff --git a/llvm/utils/lit/tests/shtest-umask.py b/llvm/utils/lit/tests/shtest-umask.py
new file mode 100644
index 0000000000000..daec3cf479fc7
--- /dev/null
+++ b/llvm/utils/lit/tests/shtest-umask.py
@@ -0,0 +1,18 @@
+# Check the umask command
+
+# RUN: not %{lit} -a -v %{inputs}/shtest-umask | FileCheck -match-full-lines %s
+
+# CHECK: -- Testing: 3 tests{{.*}}
+
+# CHECK-LABEL: FAIL: shtest-umask :: umask-bad-arg.txt ({{[^)]*}})
+# CHECK: umask bad
+# CHECK: # | Error: 'umask': invalid literal {{.*}}
+
+# CHECK-LABEL: FAIL: shtest-umask :: umask-too-many-args.txt ({{[^)]*}})
+# CHECK: umask 0 0
+# CHECK: # | 'umask' supports only one argument
+
+# CHECK: Total Discovered Tests: 3
+# CHECK: {{Passed|Unsupported}}: 1 {{\([0-9]*\.[0-9]*%\)}}
+# CHECK: Failed{{ *}}: 2 {{\([0-9]*\.[0-9]*%\)}}
+# CHECK-NOT: {{.}}
|
|
@llvm/pr-subscribers-testing-tools Author: Aiden Grossman (boomanaiden154) ChangesThis reverts commit faa4e35. This was originally reverted because it was using a Python 3.9 feature (umask in subprocess.Popen) when LLVM only requires Python 3.8. This patch uses os.umask instead, which has been around for longer. Full diff: https://github.com/llvm/llvm-project/pull/155850.diff 9 Files Affected:
diff --git a/llvm/test/tools/llvm-dwarfutil/ELF/X86/mirror-permissions-unix.test b/llvm/test/tools/llvm-dwarfutil/ELF/X86/mirror-permissions-unix.test
index a95d1c0aafa21..fdcba4dcd666b 100644
--- a/llvm/test/tools/llvm-dwarfutil/ELF/X86/mirror-permissions-unix.test
+++ b/llvm/test/tools/llvm-dwarfutil/ELF/X86/mirror-permissions-unix.test
@@ -3,7 +3,6 @@
## Setting the umask to 0 ensures deterministic permissions across
## test environments.
# UNSUPPORTED: system-windows
-# REQUIRES: shell
# RUN: touch %t
# RUN: chmod 0777 %t
diff --git a/llvm/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test b/llvm/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test
index 8f4993f4f3d29..66a481a2230d1 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/mirror-permissions-unix.test
@@ -6,7 +6,6 @@
## Setting the umask to 0 ensures deterministic permissions across
## test environments.
# UNSUPPORTED: system-windows
-# REQUIRES: shell
# RUN: touch %t
# RUN: chmod 0777 %t
diff --git a/llvm/test/tools/llvm-objcopy/ELF/respect-umask.test b/llvm/test/tools/llvm-objcopy/ELF/respect-umask.test
index 376e33a217819..02e9b93f5376f 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/respect-umask.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/respect-umask.test
@@ -1,10 +1,8 @@
## This tests that the umask is respected when
## assigning permissions of output files.
-## Windows has no umask so this test makes no sense, nor would
-## it work because there is no umask(1) in a Windows environment
+## Windows has no umask so this test makes no sense.
# UNSUPPORTED: system-windows
-# REQUIRES: shell
# RUN: rm -f %t
# RUN: touch %t
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 31e3ed679d431..36c19c1c86c75 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -98,9 +98,10 @@ class ShellEnvironment(object):
we maintain a dir stack for pushd/popd.
"""
- def __init__(self, cwd, env):
+ def __init__(self, cwd, env, umask=-1):
self.cwd = cwd
self.env = dict(env)
+ self.umask = umask
self.dirStack = []
def change_dir(self, newdir):
@@ -582,6 +583,20 @@ class SHFILEOPSTRUCTW(Structure):
return ShellCommandResult(cmd, "", stderr.getvalue(), exitCode, False)
+def executeBuiltinUmask(cmd, shenv):
+ """executeBuiltinUmask - Change the current umask."""
+ if os.name != "posix":
+ raise InternalShellError(cmd, "'umask' not supported on this system")
+ if len(cmd.args) != 2:
+ raise InternalShellError(cmd, "'umask' supports only one argument")
+ try:
+ # Update the umask in the parent environment.
+ shenv.umask = int(cmd.args[1], 8)
+ except ValueError as err:
+ raise InternalShellError(cmd, "Error: 'umask': %s" % str(err))
+ return ShellCommandResult(cmd, "", "", 0, False)
+
+
def executeBuiltinColon(cmd, cmd_shenv):
"""executeBuiltinColon - Discard arguments and exit with status 0."""
return ShellCommandResult(cmd, "", "", 0, False)
@@ -736,6 +751,7 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
"popd": executeBuiltinPopd,
"pushd": executeBuiltinPushd,
"rm": executeBuiltinRm,
+ "umask": executeBuiltinUmask,
":": executeBuiltinColon,
}
# To avoid deadlock, we use a single stderr stream for piped
@@ -757,7 +773,7 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
# env FOO=1 llc < %s | env BAR=2 llvm-mc | FileCheck %s
# env FOO=1 %{another_env_plus_cmd} | FileCheck %s
if cmd_shenv is shenv:
- cmd_shenv = ShellEnvironment(shenv.cwd, shenv.env)
+ cmd_shenv = ShellEnvironment(shenv.cwd, shenv.env, shenv.umask)
args = updateEnv(cmd_shenv, args)
if not args:
# Return the environment variables if no argument is provided.
@@ -902,6 +918,13 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
args = quote_windows_command(args)
try:
+ # TODO(boomanaiden154): We currently wrap the subprocess.Popen with
+ # os.umask as the umask argument in subprocess.Popen is not
+ # available before Python 3.9. Once LLVM requires at least Python
+ # 3.9, this code should be updated to use umask argument.
+ old_umask = -1
+ if cmd_shenv.umask != -1:
+ old_umask = os.umask(cmd_shenv.umask)
procs.append(
subprocess.Popen(
args,
@@ -916,6 +939,8 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper):
errors="replace",
)
)
+ if old_umask != -1:
+ os.umask(old_umask)
proc_not_counts.append(not_count)
# Let the helper know about this process
timeoutHelper.addProcess(procs[-1])
diff --git a/llvm/utils/lit/tests/Inputs/shtest-umask/lit.cfg b/llvm/utils/lit/tests/Inputs/shtest-umask/lit.cfg
new file mode 100644
index 0000000000000..52e18b1d5c2b3
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-umask/lit.cfg
@@ -0,0 +1,7 @@
+import lit.formats
+
+config.name = "shtest-umask"
+config.suffixes = [".txt"]
+config.test_format = lit.formats.ShTest(execute_external=False)
+if sys.platform.startswith("win") or sys.platform.startswith("cygwin"):
+ config.available_features.add("system-windows")
diff --git a/llvm/utils/lit/tests/Inputs/shtest-umask/umask-bad-arg.txt b/llvm/utils/lit/tests/Inputs/shtest-umask/umask-bad-arg.txt
new file mode 100644
index 0000000000000..639bfd4b7b4f3
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-umask/umask-bad-arg.txt
@@ -0,0 +1 @@
+# RUN: umask bad
diff --git a/llvm/utils/lit/tests/Inputs/shtest-umask/umask-ok.txt b/llvm/utils/lit/tests/Inputs/shtest-umask/umask-ok.txt
new file mode 100644
index 0000000000000..9d43efbddf2cf
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-umask/umask-ok.txt
@@ -0,0 +1,11 @@
+## Windows has no umask so this test makes no sense.
+# UNSUPPORTED: system-windows
+
+# RUN: touch %t
+# RUN: chmod 644 %t && ls -l %t | cut -f 1 -d ' ' > %t.644
+# RUN: chmod 600 %t && ls -l %t | cut -f 1 -d ' ' > %t.600
+# RUN: chmod 666 %t && ls -l %t | cut -f 1 -d ' ' > %t.666
+
+# RUN: umask 022 && rm %t && touch %t && ls -l %t | cut -f 1 -d ' ' | cmp - %t.644
+# RUN: umask 177 && rm %t && touch %t && ls -l %t | cut -f 1 -d ' ' | cmp - %t.600
+# RUN: umask 000 && rm %t && touch %t && ls -l %t | cut -f 1 -d ' ' | cmp - %t.666
diff --git a/llvm/utils/lit/tests/Inputs/shtest-umask/umask-too-many-args.txt b/llvm/utils/lit/tests/Inputs/shtest-umask/umask-too-many-args.txt
new file mode 100644
index 0000000000000..0b7461b5bc3fe
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-umask/umask-too-many-args.txt
@@ -0,0 +1 @@
+# RUN: umask 0 0
diff --git a/llvm/utils/lit/tests/shtest-umask.py b/llvm/utils/lit/tests/shtest-umask.py
new file mode 100644
index 0000000000000..daec3cf479fc7
--- /dev/null
+++ b/llvm/utils/lit/tests/shtest-umask.py
@@ -0,0 +1,18 @@
+# Check the umask command
+
+# RUN: not %{lit} -a -v %{inputs}/shtest-umask | FileCheck -match-full-lines %s
+
+# CHECK: -- Testing: 3 tests{{.*}}
+
+# CHECK-LABEL: FAIL: shtest-umask :: umask-bad-arg.txt ({{[^)]*}})
+# CHECK: umask bad
+# CHECK: # | Error: 'umask': invalid literal {{.*}}
+
+# CHECK-LABEL: FAIL: shtest-umask :: umask-too-many-args.txt ({{[^)]*}})
+# CHECK: umask 0 0
+# CHECK: # | 'umask' supports only one argument
+
+# CHECK: Total Discovered Tests: 3
+# CHECK: {{Passed|Unsupported}}: 1 {{\([0-9]*\.[0-9]*%\)}}
+# CHECK: Failed{{ *}}: 2 {{\([0-9]*\.[0-9]*%\)}}
+# CHECK-NOT: {{.}}
|
|
Thanks! LGTM but I will defer to anyone who didn't write the original patch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts commit faa4e35.
This was originally reverted because it was using a Python 3.9 feature (umask in subprocess.Popen) when LLVM only requires Python 3.8. This patch uses os.umask instead, which has been around for longer.