OSX Catalina (10.15) Issues/Fixes

[DavidMChan]

While OSX is not officially supported - I wanted to point out the issues and solutions I found necessary to get Deepmind lab running on 10.15 on the macos branch with python3 for those who are interested. I'm not sure if any of these should be merged, but the documentation could be adapted.

Issues (Or at least, those I was able to find)

1. RESOLVED (AS OF 12/6): ~~The "//:deepmind/support:realpath" script no longer functions on 10.15. Fix: Install the coreutils brew package (which includes a functioning realpath) and replace all lines in BUILD of the form ln -s $$($(location //deepmind/support:realpath) ... with ln -s $$(realpath .... The patch file is here: https://gist.github.com/DavidMChan/6654a82c658480f8b983b9afabe4502c~~

2. SDL 2.0.10 renders the window in only 1/4 of the screen. Fix: Use SDL 2.0.12 by installing with --HEAD on brew. This is related to high DPI support in the upstream (See: https://hg.libsdl.org/SDL/rev/46b094f7d20e and https://github.com/ioquake/ioq3/issues/422)

[tkoeppe]

Have you considered the macos branch? https://github.com/deepmind/lab/tree/macos

It addresses some of those issues.

[DavidMChan]

I had these issues specific to 10.15 on the Macos branch as specified in the post - everything is working well prior to 10.15 on the macos branch.

[tkoeppe]

Ah, I see, sorry! Is 10.15 available on Travis?

[DavidMChan]

No worries! I don't believe it is yet - at least according to https://docs.travis-ci.com/user/reference/osx/

[tkoeppe]

What are the actual errors with my replacement realpath?

[DavidMChan]

The realpath implementation isn't locating the asset files - that is, it produces a file not found/cannot be resolved error. I'll get the actual bazel debug logs when I'm next in front of my home PC.

I believe the issue may have to do with the realpath not resolving symlinks correctly - I experimented some with it, and it seemed that when the path of a file is hidden behind a symlink, the realpath implementation provided could not find the file.

[DavidMChan]

The short form:

INFO: Writing tracer profile to '/private/var/tmp/_bazel_davidchan/1ed2868fb4e7e440ee2a2775754846e4/command.profile.gz'
INFO: Analyzed target //:deepmind_lab.so (33 packages loaded, 3221 targets configured).
INFO: Found 1 target...
INFO: From Executing genrule //:non_pk3_assets:
Error resolving path 'assets/default.cfg', error was: 'No such file or directory'
ERROR: /Users/davidchan/Projects/lab/BUILD:681:1: declared output 'baselab/default.cfg' was not created by genrule. This is probably because the genrule actually didn't create this output, or because the output was a directory and the genrule was run remotely (note that only the contents of declared file outputs are copied from genrules run remotely)
ERROR: /Users/davidchan/Projects/lab/BUILD:681:1: not all outputs were created or valid
Target //:deepmind_lab.so failed to build
Use --verbose_failures to see the command lines of failed build steps.
ERROR: /Users/davidchan/Projects/lab/BUILD:961:1 not all outputs were created or valid
INFO: Elapsed time: 21.543s, Critical Path: 11.40s
INFO: 44 processes: 44 darwin-sandbox.
FAILED: Build did NOT complete successfully

Verbose Failure here: https://gist.github.com/DavidMChan/ca1c9b443d6d6056c35916968be98972

[tkoeppe]

I'd like to understand why the realpath behaviour has changed on 10.15. Just to be sure, could you try out whether it has to do with the extension that allows passing a null pointer for the output, by instead using the following (non-extension) variation:

diff --git a/deepmind/support/BUILD b/deepmind/support/BUILD
index c369673..860ba3a 100644
--- a/deepmind/support/BUILD
+++ b/deepmind/support/BUILD
@@ -32,5 +32,6 @@ cc_binary(
         "-std=c99",
         "-D_DEFAULT_SOURCE",
         "-D_POSIX_C_SOURCE",
+        "-D_DARWIN_BETTER_REALPATH",
     ],
 )
diff --git a/deepmind/support/realpath.c b/deepmind/support/realpath.c
index aad9512..5522dcc 100644
--- a/deepmind/support/realpath.c
+++ b/deepmind/support/realpath.c
@@ -21,24 +21,27 @@
 // step. This implements the equivalent of "realpath -e" on Linux.
 
 #include <errno.h>
+#include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
+char p_storage[PATH_MAX] = {};
+
 int main(int argc, char* argv[]) {
   int num_errors = 0;
   errno = 0;
 
   for (int i = 1; i < argc; ++i) {
-    char* p = realpath(argv[i], NULL);
+    char* p = realpath(argv[i], p_storage);
+    p_storage[PATH_MAX - 1] = '\0';
     if (p == NULL) {
-      fprintf(stderr, "Error resolving path '%s', error was: '%s'\n",
-              argv[i], strerror(errno));
+      fprintf(stderr, "Error resolving path '%s', error was: '%s', details: '%s'\n",
+              argv[i], strerror(errno), p_storage);
       errno = 0;
       ++num_errors;
     } else {
       fprintf(stdout, "%s\n", p);
-      free(p);
     }
   }
 

[DavidMChan]

After testing this morning, I can confirm, this patch does seem to fix the realpath issues.

[tkoeppe]

Wonderful, thanks! That's a very trivial fix then :-) I'll get that committed.

[tkoeppe]

(It's hard to know who's being portable and who isn't here. The linux manual says that the use of NULL is Posix-2008 compliant, and that the previous Posix-2001 version is "broken by design", but macos apparently seems to have regressed in Posix compliance and no longer supports the NULL form? I'd welcome further information, if anyone can dig that up.)

[tkoeppe]

I committed this change in 760ab54c166667413e476cf991a06167523339dc, and I've cherrypicked that onto the macos branch (for now, until I next rebase that). Does that work for you?

[DavidMChan]

Yep! The realpath implementation is working now on 10.15. There are still issues with SDL2 2.0.10, but there's nothing really that we can do about that until they release a new stable version.

[tkoeppe]

Great :-)

Yes, SDL isn't under our control at all, I just set some reasonable defaults for Travis to work. But you need to adapt it to whatever you have locally.