This is an archive of the discontinued LLVM Phabricator instance.

[SystemZ][z/OS] Fix handling of dirs with filesystem tests
ClosedPublic

Authored by muiez on Aug 18 2021, 12:54 PM.

Details

Summary

The aim of this patch is to fix the post processing that is happening on the temporary test directories upon scope exit. In particular, ~scoped_test_env aims to chmod and remove the temporary directories; however,

  • bad symlinks are followed and we get "No such file or directory". FIX: use find as alternative to chmod and avoid -follow option.
  • Attempting to remove read-only files on z/OS prompts a message asking for confirmation. FIX: use the -f option to delete read-only files immediately without asking for confirmation.
  • Some libcxx tests such as libcxx/test/std/input.output/filesystems/cl ass.directory_entry/directory_entry.cons/path.pass.cpp set the dir permissions to none. In turn, recursively doing chmod (-R) does not set the file permissions needed to be able to remove the dir on z/OS only. FIX: use find as alternative to chmod -R, which does not run into this issue on z/OS.

Diff Detail

Event Timeline

muiez requested review of this revision.Aug 18 2021, 12:54 PM
muiez created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptAug 18 2021, 12:54 PM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
muiez updated this revision to Diff 367565.Aug 19 2021, 11:39 AM

No change. Uploaded diff with full context

ldionne requested changes to this revision.Aug 24 2021, 12:55 PM

Also, you're submitting a patch for a platform that isn't supported. I very much want to support it, but in order to do so properly, I need something very simple: a build bot. Would it be possible to add one?

libcxx/test/support/filesystem_test_helper.h
145

Can we unconditionally make this change?

152

Why add f?

This revision now requires changes to proceed.Aug 24 2021, 12:55 PM
muiez added inline comments.Aug 24 2021, 1:42 PM
libcxx/test/support/filesystem_test_helper.h
145

I tried the following test on macOS and find doesn't work for dir's with no permission, so restricting the change to z/OS could be the way to go.

// macOS

> mkdir -m 0 dir 
> find dir -exec chmod 777 {} \;  
find: dir: Permission denied
152

Attempting to remove read-only files on z/OS prompts a message asking for confirmation. The -f option is to delete read-only files immediately without asking for confirmation.

muiez marked 2 inline comments as done.Aug 26 2021, 7:58 AM

Also, you're submitting a patch for a platform that isn't supported. I very much want to support it, but in order to do so properly, I need something very simple: a build bot. Would it be possible to add one?

I understand; however, it seems like support for a z/OS buildbot is not in IBM's plan now. I will try to push for this and update if anything changes.

muiez added a comment.Oct 6 2021, 12:21 PM

Any feedback @ldionne? The comments have been addressed.

FWIW, I think this is fine (all parts being scoped to __MVS__ and/or harmless). @ldionne +1?

nilayvaish accepted this revision.Dec 3 2021, 8:02 AM
nilayvaish added a subscriber: nilayvaish.

Looks fine to me.

ldionne accepted this revision.Dec 6 2021, 7:10 AM
This revision is now accepted and ready to land.Dec 6 2021, 7:10 AM
This revision was automatically updated to reflect the committed changes.