[libcxx-commits] [PATCH] D87940: [SystemZ][ZOS] Porting the time functions within libc++ to z/OS

Fri Oct 9 10:40:45 PDT 2020

ldionne added a comment.

In D87940#2299436 <https://reviews.llvm.org/D87940#2299436>, @hubert.reinterpretcast wrote:

> @ldionne, does this patch look okay to you? In terms of changes being made for z/OS, it's mainly going to be:

I'm okay with the changes, but I don't think it makes sense to implement `nanosleep()` upstream. Are you willing to keep this workaround downstream until z/OS implements `nanosleep`? This also gives you a nice incentive to implement `nanosleep()`. :-)

> - lack of LSB and BSD or GNU extension APIs,
> - older UNIX conformance level,
> - EBCDIC/ASCII mode support, and
> - versioning for IEEE floating point.
>
> I understand that a lot of context would be needed for the latter two, but I believe that the first two is not particularly special and introduces only the usual amount of maintenance burden.
> Has it been a norm to request a full set of patches from the get-go before accepting patches that adjust for API differences?

There's been no norm so far, and libc++ has greatly suffered from that laxness. We now "support" several platforms that are not tested and have nobody working on them. That situation is bad for everyone: the platforms are not properly supported, and there's dead (or not dead?) code to account for these platforms everywhere. We can go one patch at a time, but be aware that we might push back on some changes if (1) they are too intrusive and (2) we don't have good ways to isolate those from the rest of the code, and hence they add a lot of unavoidable complexity. We'll try to find solutions to make it reasonable, of course.

I'm trying to start being more principled around new configurations/platform support. For example, are you able to provide a build bot to build on your system?

================
Comment at: libcxx/src/filesystem/filesystem_common.h:413
+
+// allow the utimes implementation to compile even it we're not going
+// to use it.
----------------
I don't understand this comment.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87940/new/

https://reviews.llvm.org/D87940