We need fs::realpath

[lilyball]

After an aborted attempt at implementing this directly (#11734), we need to investigate how to provide fs::realpath() correctly.

The current thinking is that for libgreen, we should defer to libuv. For libnative, on POSIX, we should use realpath(). On Windows, I don't know, but we should avoid writing this ourselves at all costs because there's a lot of complexity involved.

[bnoordhuis]

The current thinking is that for libgreen, we should defer to libuv.

Minor correction: libuv doesn't have a realpath() implementation. The fs.realpath() and fs.realpathSync() functions in node.js are implemented in JavaScript.

[lilyball]

@bnoordhuis That's unfortunate. Given that realpath() includes a number of filesystem calls (e.g. stat, readlink), does this mean that we do need to reimplement it ourselves in order to use libuv's implementation of those filesystem calls, or is calling the "real" realpath() still acceptable even when using libgreen?

[bnoordhuis]

realpath() can block so I wouldn't call it from a green thread. You can use uv_queue_work() to offload the call to a thread from the thread pool. It would look something like this in C:

typedef struct {
  uv_work_t work_req;
  int errorno;
  char resolved_path[PATH_MAX];
  char path[1];
} realpath_req;

static void work_cb(uv_work_t *req);
static void done_cb(uv_work_t *req, int status);

void fs_realpath(const char *path) {
  size_t len = strlen(path);
  realpath_req *req = malloc(sizeof(*req) + len);
  memcpy(req->path, path, len + 1);
  uv_queue_work(uv_default_loop(), &req->work_req, work_cb, done_cb);
}

static void work_cb(uv_work_t *work_req) {
  realpath_req *req = container_of(work_req, realpath_req, work_req);
  req->errno = 0;
  if (realpath(req->path, req->resolved_path) == NULL)
    req->errorno = errno;
}

static void done_cb(uv_work_t *req, int status) {
  realpath_req *req = container_of(work_req, realpath_req, work_req);
  (void) &status;  // Only relevant when calling uv_cancel().
  // ...
  free(req);
}

[lilyball]

cc @alexcrichton

[wycats]

The current state is not acceptable.

For Cargo, we had to pull in the realpath that is used by rustc: rust-lang/cargo@4c3f9da#diff-42830b961b225436d4e616dcc6832330R153

[wycats]

It doesn't make sense to me to have an implementation that we use internally in the compiler but to be unwilling to ship it even temporarily as part of std. If it's broken, that will mean the compiler is broken!

[lilyball]

@wycats

That realpath() implementation looks broken to me. Notably, the MAX_LINKS_FOLLOWED really seems to limit the number of path components that can be links, but when resolving a link, it recursively runs realpath() on the link destination, despite that getting a fresh "links followed" counter.

It also isn't appending the resolved symlink path to the directory the symlink was in before calling the recursive realpath(), which is quite wrong. A relative symlink will then be considered relative to the cwd instead, which is of course not correct.

I would suggest that as a temporary workaround you use FFI to call POSIX realpath() instead (not sure about Windows, maybe just skip it there? The realpath() impl you linked to already short-circuits Windows).

[wycats]

Windows supports symlinks as of Vista, so that seems like a problem. I would suggest that getting this working correctly in the compiler is important.

[wycats]

In other words, either this is horribly broken in the compiler and needs to be fixed ASAP, or it's working well enough to include in the standard library for now.

[lilyball]

There's a grey area in between those two extremes. It's definitely broken enough that it's not appropriate to put in the standard library. But it apparently works well enough for the limited use-case of the compiler, or this would have been an issue before now. Which is to say, it should be fixed, but it's not an ASAP kind of fix.