Skip to content

Add text file support for z/OS back in #209259

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.

Sign up for GitHub

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
perry-ca merged 3 commits into from
Jul 14, 2026
+23 −8
Merged

Add text file support for z/OS back in #209259

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
43f21cd
Add text file support for z/OS back in
perry-ca Jul 13, 2026
efcc338
only open as text on z/OS
perry-ca Jul 13, 2026
a0b390a
only open as text on z/OS
perry-ca Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions llvm/utils/lit/lit/builtin_commands/cat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import getopt
import os
import platform
import sys
from io import StringIO

Expand Down Expand Up @@ -65,18 +66,32 @@ def run(argv, stdin, stdout, stderr, cwd):

for filename in filenames:
path = filename
contents = None
if not os.path.isabs(path):
path = os.path.join(cwd, path)
try:
with open(path, "rb") as fileToCat:
contents = fileToCat.read()
except IOError as error:
error.filename = filename
stderr.write(str(error).encode())
return 1

contents = None
is_text = False
if platform.system() == "OS/390":
try:
with open(path, "r") as fileToCat:
contents = fileToCat.read()
is_text = True
except:
pass

if contents is None:
try:
with open(path, "rb") as fileToCat:
contents = fileToCat.read()
except IOError as error:
error.filename = filename
stderr.write(str(error).encode())
return 1

if show_nonprinting:
contents = convertToCaretAndMNotation(contents)
elif is_text:
contents = contents.encode()

@boomanaiden154 boomanaiden154 Jul 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we can avoid is_text entirely if we just do this where we currently set is_text = True.

@perry-ca perry-ca Jul 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll have to dig out my notes, but I believe the --show-nonprinting option fails if you do that. It looks like this would double encode the text (once at the file read and then again inside of the convert function). That would produce gibberish.

Still good?

@boomanaiden154 boomanaiden154 Jul 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's unclear to me how this proposal would cause a semantic difference from what the code does now. When we set is_text, we're reading in text node, so .read() does not read an encoded form and we still only encode once.

@perry-ca perry-ca Jul 14, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

With the way the code is now the sequence of operations for a text file on z/OS with the --show-nonprinting option is:

  1. open the file
  2. read the file
  3. call the convert function (this calls encode())
  4. print the contents

If we called encode() where the is_text flag is set, the sequence would be

  1. open the file
  2. read the file
  3. encode the contents
  4. call the convert function (which calls encode() again)
  5. print the contents.

I'm thinking of a command like python cat.py --show-nonprinting text-file. I'm pretty sure we came across this when first making this change.

@prasoon054 prasoon054 Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Didn't think about that interaction with --show-nonprinting when I made this change. 😓

stdout.write(contents)
return 0

Expand Down
Loading