Skip to content

[z/OS] treat text files as text files so auto-conversion is done #90128

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
merged 1 commit into from
May 1, 2024
Merged

[z/OS] treat text files as text files so auto-conversion is done #90128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 4 additions & 3 deletions clang/tools/clang-format/ClangFormat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,8 +413,9 @@ static bool format(StringRef FileName, bool ErrorOnIncompleteFormat = false) {
// On Windows, overwriting a file with an open file mapping doesn't work,
// so read the whole file into memory when formatting in-place.
ErrorOr<std::unique_ptr<MemoryBuffer>> CodeOrErr =
!OutputXML && Inplace ? MemoryBuffer::getFileAsStream(FileName)
: MemoryBuffer::getFileOrSTDIN(FileName);
!OutputXML && Inplace
? MemoryBuffer::getFileAsStream(FileName)
: MemoryBuffer::getFileOrSTDIN(FileName, /*IsText=*/true);
Copy link
Contributor

Choose a reason for hiding this comment

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

On windows won't this have an impact of potentially converting \n to \r\n? could a clang-format effectively become a unix2dos at the same time (for those using clang-format on cygwin is that what they want?) can we have a unit test to understand the impact?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could there be other impacts if the files that aren't utf-8 or contain internationalize characters in say comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe this will be a problem for Windows. This is a common problem that we have fixed in a few places already. I don't have a Windows platform to confirm. Would you or someone else be able to verify?

Copy link
Contributor

Choose a reason for hiding this comment

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

I tested it on Windows and didn't have a problem. I used LF only, CRLF only, and mixed line endings for the following:

int i;
int j;

Running clang-format through all values of the LineEnding option, with and without in-place, seemed ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I also was able to confirm there was no impact on windows i just wasn't sure so needed to prove it to myself, I'm ok if you want to commit this into clang-format, if you break stuff... we'll simply revert it ;-)

if (std::error_code EC = CodeOrErr.getError()) {
errs() << EC.message() << "\n";
return true;
Expand Down Expand Up @@ -558,7 +559,7 @@ static int dumpConfig() {
// Read in the code in case the filename alone isn't enough to detect the
// language.
ErrorOr<std::unique_ptr<MemoryBuffer>> CodeOrErr =
MemoryBuffer::getFileOrSTDIN(FileNames[0]);
MemoryBuffer::getFileOrSTDIN(FileNames[0], /*IsText=*/true);
if (std::error_code EC = CodeOrErr.getError()) {
llvm::errs() << EC.message() << "\n";
return 1;
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ static std::vector<StringRef> getSearchPaths(opt::InputArgList *Args,

// Opens a file. Path has to be resolved already. (used for def file)
std::unique_ptr<MemoryBuffer> openFile(const Twine &Path) {
ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> MB = MemoryBuffer::getFile(Path);
ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> MB =
MemoryBuffer::getFile(Path, /*IsText=*/true);

if (std::error_code EC = MB.getError()) {
llvm::errs() << "cannot open file " << Path << ": " << EC.message() << "\n";
Expand Down
9 changes: 6 additions & 3 deletions llvm/tools/llvm-cxxmap/llvm-cxxmap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,15 +144,18 @@ int main(int argc, const char *argv[]) {
cl::HideUnrelatedOptions({&CXXMapCategory, &getColorCategory()});
cl::ParseCommandLineOptions(argc, argv, "LLVM C++ mangled name remapper\n");

auto OldSymbolBufOrError = MemoryBuffer::getFileOrSTDIN(OldSymbolFile);
auto OldSymbolBufOrError =
MemoryBuffer::getFileOrSTDIN(OldSymbolFile, /*IsText=*/true);
if (!OldSymbolBufOrError)
exitWithErrorCode(OldSymbolBufOrError.getError(), OldSymbolFile);

auto NewSymbolBufOrError = MemoryBuffer::getFileOrSTDIN(NewSymbolFile);
auto NewSymbolBufOrError =
MemoryBuffer::getFileOrSTDIN(NewSymbolFile, /*IsText=*/true);
if (!NewSymbolBufOrError)
exitWithErrorCode(NewSymbolBufOrError.getError(), NewSymbolFile);

auto RemappingBufOrError = MemoryBuffer::getFileOrSTDIN(RemappingFile);
auto RemappingBufOrError =
MemoryBuffer::getFileOrSTDIN(RemappingFile, /*IsText=*/true);
if (!RemappingBufOrError)
exitWithErrorCode(RemappingBufOrError.getError(), RemappingFile);

Expand Down
2 changes: 1 addition & 1 deletion llvm/tools/yaml2obj/yaml2obj.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ int main(int argc, char **argv) {
}

ErrorOr<std::unique_ptr<MemoryBuffer>> Buf =
MemoryBuffer::getFileOrSTDIN(Input);
MemoryBuffer::getFileOrSTDIN(Input, /*IsText=*/true);
if (!Buf)
return 1;

Expand Down
18 changes: 16 additions & 2 deletions llvm/utils/lit/lit/builtin_commands/cat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,24 @@ def main(argv):
msvcrt.setmode(sys.stdout.fileno(), os.O_BINARY)
for filename in filenames:
try:
fileToCat = open(filename, "rb")
contents = fileToCat.read()
contents = None
is_text = False
try:
if sys.platform != "win32":
fileToCat = open(filename, "r")
contents = fileToCat.read()
is_text = True
except:
pass

if contents is None:
fileToCat = open(filename, "rb")
contents = fileToCat.read()

if show_nonprinting:
contents = convertToCaretAndMNotation(contents)
elif is_text:
contents = contents.encode()
writer.write(contents)
sys.stdout.flush()
fileToCat.close()
Expand Down
7 changes: 7 additions & 0 deletions llvm/utils/lit/lit/llvm/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ def __init__(self, lit_config, config):
self.lit_config.note("using lit tools: {}".format(path))
lit_path_displayed = True

if platform.system() == "OS/390":
self.with_environment("_BPXK_AUTOCVT", "ON")
self.with_environment("_TAG_REDIR_IN", "TXT")
self.with_environment("_TAG_REDIR_OUT", "TXT")
self.with_environment("_TAG_REDIR_ERR", "TXT")
self.with_environment("_CEE_RUNOPTS", "FILETAG(AUTOCVT,AUTOTAG) POSIX(ON)")

# Choose between lit's internal shell pipeline runner and a real shell.
# If LIT_USE_INTERNAL_SHELL is in the environment, we use that as an
# override.
Expand Down