rework the API layers between the standard library and the operating system #2527

andrewrk · 2019-05-20T16:38:12Z

This is a very breaking change. I apologize in advance - everybody is going to have to update their code, and the untested regions of the standard library probably have compile errors lurking in them. The good news is the new APIs are more stable and future-proof than before.

The main driving issue behind this pull request is #2380. However it solves a few other issues, which I will note below.

All of the high-level API functionality of std.os has been moved to other places, as suggested by @jayschwa. These are the APIs that are intended to be used by applications.
- all the file system functions have moved to a new module, std.fs.
- std.os.path is now std.fs.path.
- std.os.ChildProcess is now std.ChildProcess
- environment variable stuff and process argument stuff has been moved to a new module, std.process. this is also where the concept of the current working directory lives.
- std.os.Thread is moved to std.Thread. std.os.spawnThread is moved to std.Thread.spawn.
- std.os.time is moved to std.time.
- std.os.getRandomBytes is moved to std.crypto.randomBytes.
- The deprecated functions std.cstr.toSlice, std.cstr.toSliceConst, and std.cstr.len are removed. Instead use, respectively, std.mem.toSlice, std.mem.toSliceConst, and std.mem.len. The idea here is that after proposal: type for null terminated pointer #265, null-terminated byte buffers does not necessarily mean it's a "C string". It's just a different way of arranging memory.
Integration with libc is now more robust and complete.
- std.os.system when linking libc is aliased to std.c. When not linking libc, which applies to targets linux, windows, wasi, std.os.system is aliased to std.os.linux, std.os.windows, std.os.wasi respectively.
- std.os is thin wrappers around operating system API that provide zig-style parameters and error sets. These wrappers are cross platform where applicable, for example posix functions, and std.os.exit. These wrappers call into libc when linking libc.
- std.os.linux and std.os.windows etc can be used directly to bypass libc, even when linking libc.
- std.os also has all the OS constants and types, such as iovec. These are declared in std/os/bits.zig with target-specific additions in std/os/bits/*. The linux ones are compatible when compiling with and without libc.
When not linking libc, Linux now uses @hasDecl to check for the existence of syscalls to make the appropriate syscall. For example, some systems do not have rename and some systems do not have renameat. Zig standard library now does the correct thing for both. This fixes zig fmt failure in Window Subsystem for Linux #2481.
All the std.os.windows functions have been moved to be namespaced under the DLL they are found in. std.os.windows still has all the values and types, however.
- std.os.windows has new functions which are thin wrappers around Windows API that provide Zig-style parameters and error sets.
Many places that had inferred error sets that diverged across platforms have been unified, making the standard library more cross-platform friendly.

I changed my mind about "glue code to work around kernel API footguns". I think this belongs in the OS-specific API layer.

I also changed my mind about "Make the OS-specific files do exactly what they say. E.g. if you call std.os.linux.fork, that is going to do the fork syscall when you observe the program with strace, guaranteed." Upon close inspection I believe the way @shawnl did it makes sense - it just needed the @hasDecl improvement, which Shawn has been trying to get added into Zig. I apologize for my wavering on this issue; I believe the code in this PR gets it right.

This pull request also fixes #2415.

It also fixes #2425.

What's left to do

all tests passing on linux
all tests passing on macos
all tests passing on windows
all tests passing on freebsd
do some manual testing for wasi

daurnimator · 2019-05-24T04:41:09Z

Recommendations:

std/c.zig at the top level would contain things defined in the C standard. But there would be submodules that contain operating system/libc specific defines e.g. std/c/macosx.zig for things in the OSX libc.
- All functions under the std/c namespace should be defined as extern "c".
- If libc defines a function that e.g. "takes a pointer to a native struct sockaddr" then you may use type definitions from std/os/*
std/os/* should be where all interfaces to the OS ABI are kept.
There should be a submodule per builtin.os enum. The top level std/os.zig shouldn't contain any types or functions.
Some operating systems don't have much of a well defined ABI and instead defer to libc. This may cause a OS's std/os/* to be quite small!
It is not limited by glibc/musl C conventions. And should use zig types where possible. e.g.:
- syscall numbers could be a non-exhaustive enum rather than SYS_*
- the write syscall wrapper might take a slice instead of separate buf, count arguments.
- to keep return types sensible, the syscall wrappers could return actual zig errors. pub fn close(fd: i32) !void
std/posix* should be reserved for things actually defined in POSIX.
- POSIX is a superset of C, so std/posix.zig could use @import("c.zig");.
- A mmap function under here would only accept flags defined in POSIX. If you want to use extensions like MAP_FIXED_NOREPLACE then you should be using std/os/linux or std/c/linux directly instead.
- e.g. std/posix/macosx.zig would be mostly wrappers around std/c/macosx.zig as OSX is a libc-non-optional OS.
- e.g. std/posix/linux.zig could contain wrappers around either of std/os/linux or std/c/linux.zig, depending on if libc is linked.

Much of this can be thought of as "separating the (2) from the (3)" (this is a quote that stuck in my head from somewhere, but now I can't find a source for it), which is a reference to man page sections "2: System calls (functions provided by the kernel)" and "3: Library calls". Though in practice man pages sometimes end up documenting glibc things in section 2 🤦‍♂️

andrewrk · 2019-05-24T17:40:02Z

Thanks for the recommendations. I'm doing most of them, except for the non-exhaustive enum thing.

I'm also taking all of @jayschwa's suggestions as outlined in #2380.

Hopefully can have this wrapped up today.

See #2380

see #2380

shritesh · 2019-05-27T04:18:21Z

std/os/bits/wasi.zig

-    buf: [*]const u8,
-    buf_len: usize,
-};
+pub const ciovec_t = iovec_const;


use @import("../bits.zig"); is needed for this to work

@andrewrk patch to fix WASI failing:

diff --git a/std/os/bits/wasi.zig b/std/os/bits/wasi.zig index a6f96de3..93d2a82f 100644 --- a/std/os/bits/wasi.zig +++ b/std/os/bits/wasi.zig @@ -1,2 +0,0 @@ -use @import("../bits.zig"); - @@ -15,2 +12,0 @@ pub const ADVICE_NOREUSE: advice_t = 5; -pub const ciovec_t = iovec_const; - @@ -185,2 +180,0 @@ pub const inode_t = u64; -pub const iovec_t = iovec; - diff --git a/std/os/wasi.zig b/std/os/wasi.zig index 1ce0fe0c..adfe9e82 100644 --- a/std/os/wasi.zig +++ b/std/os/wasi.zig @@ -8 +8 @@ pub const is_the_target = builtin.os == .wasi; -pub use @import("bits/wasi.zig"); +pub use @import("bits.zig"); @@ -20,0 +21,3 @@ comptime { +pub const iovec_t = iovec; +pub const ciovec_t = iovec_const; +

Ahh that's much better, incorporated, thanks!

And here’s for when the docs start to fail:

diff --git a/doc/langref.html.in b/doc/langref.html.in index 19839904..402e5365 100644 --- a/doc/langref.html.in +++ b/doc/langref.html.in @@ -9011,0 +9012 @@ export fn add(a: i32, b: i32) void { + {#code_release_fast#} @@ -9015,2 +9016,2 @@ pub fn main() !void { - const args = try std.os.argsAlloc(std.heap.wasm_allocator); - defer std.os.argsFree(std.heap.wasm_allocator, args); + const args = try std.process.argsAlloc(std.heap.wasm_allocator); + defer std.process.argsFree(std.heap.wasm_allocator, args);

what's the release fast for?

Building WASI in debug mode fails with Unsupported OS in debug.zig. I think this is the correct behavior as we don’t have support for debugging symbols for WASM.

Ah. Well that's a separate issue but we can easily make it work, sans debug symbols. And then it can go from "debug mode isn't yet supported" to "debug symbols aren't yet supported", which I think is a preferable state to be in. There are lots of benefits to debug mode besides debug info parsing.

Agreed. It works on master. I didn’t want CI to fail due to WAS{M,I}.

daurnimator

I only reviewed a little bit of the code; huge PR!

daurnimator · 2019-05-27T04:24:10Z

std/os.zig

-            // See #397
-            const errno = posix.getErrno(posix.getrandom(buf.ptr, buf.len, 0));
-            switch (errno) {
+pub fn getrandom(buf: []u8) GetRandomError!void {


I think this function should be in std/crypto?

std.crypto.randomBytes calls (or is aliased to) this function. This exists in std.os because it's a Linux API and so this is the code that bridges libc and non-libc. Since it's a useful API to make cross platform, this is also the layer where it is made so, just like the other posix functions. You can think of it somewhat like kevent in that it's not POSIX but it is cross-platform.

Since it's a useful API to make cross platform, this is also the layer where it is made so, just like the other posix functions

I don't follow why it belongs std/os instead of std/crypto though?

You can think of it somewhat like kevent in that it's not POSIX but it is cross-platform.

kevent is very not cross platform; there are many kevent semantics that are impossible to implement on linux, windows or wasi.

You can abstract over the top of kevent(bsds+osx)/epoll(linux)/ports(solaris), but that wouldn't be kevent, it would be something else (and I would put it in a new module, std/kpoll or similar)

The OS provides some assurances about random numbers that a pure user-space implementation cannot (most importantly that the PRNG will be seeded).

daurnimator · 2019-05-27T04:26:37Z

std/os.zig

+    }
+    switch (errno(system.close(fd))) {
+        EBADF => unreachable, // Always a race condition.
+        EINTR => return, // This is still a success. See https://github.com/ziglang/zig/issues/2425


Comment here isn't exactly correct. As mentioned in linked issue, some OSes you should retry, others your don't.

Also for OSX we should be calling close_nocancel here instead (that can be in another PR, but I'd leave a TODO here)

I looked into close_nocancel via experimentation and looking for man pages on OSX and concluded that it's not a thing that exists anymore. Can you point to documentation for it?

That's because it's called close$NOCANCEL, and it seems to be used at least by Rust and Chrome.

daurnimator · 2019-05-27T04:33:04Z

std/os.zig

+    return inotify_add_watchC(inotify_fd, &pathname_c, mask);
+}
+
+/// Same as `inotify_add_watch` except pathname is null-terminated.


Shouldn't inotify be under linux?

daurnimator · 2019-05-27T04:33:38Z

std/os.zig

+    flags: u32,
+    fd: fd_t,
+    offset: isize,
+) MMapError![]align(mem.page_size) u8 {


daurnimator · 2019-05-27T04:34:59Z

std/mem.zig

@@ -8,6 +8,11 @@ const meta = std.meta;
 const trait = meta.trait;
 const testing = std.testing;

+pub const page_size = switch (builtin.arch) {
+    .wasm32, .wasm64 => 64 * 1024,
+    else => 4 * 1024,


There is no longer just one page size; huge pages are getting used more and more.

that's going to have to be a separate and even-more-breaking change if page size stops being comptime known. out of scope for this PR

Created #2564 to track.

daurnimator · 2019-05-27T04:39:08Z

std/os.zig

+    }
+}
+
+pub fn gettimeofday(tv: ?*timeval, tz: ?*timezone) void {


Move to std.time? Or possibly remove all-together? (on posixy operating systems we should be preferring clock_gettime, on windows GetSystemTimeAsFileTime or similar)

It's called from std.time for the darwin target. I'm not sure why darwin is treated specially; it does seem that clock_gettime should work there too. maybe @tgschultz knows why

I'm not sure why darwin is treated specially; it does seem that clock_gettime should work there too. maybe @tgschultz knows why

From another project I work on https://github.com/wahern/cqueues/blob/36b4b8cf1d8d3e3224cab04357dda3a633c7f0ff/src/cqueues.c#L128 :

OS X didn't implement the clock_gettime() POSIX interface until macOS 10.12 (Sierra). But it did provide a monotonic clock through mach_absolute_time(). On i386 and x86_64 architectures this clock is in nanosecond units, but not so on other devices. mach_timebase_info() provides the conversion parameters.

daurnimator's info is indeed why I didn't just call clock_gettime. I thought I left a comment about it in the code.

daurnimator · 2019-05-27T04:40:52Z

std/os.zig

+    const prefix = []u16{ '\\', '\\', '?', '\\' };
+    const start_index = if (mem.startsWith(u16, wide_slice, prefix)) prefix.len else 0;
+
+    // Trust that Windows gives us valid UTF-16LE.


this PR probably isn't the place for it; but windows allows unpaired surrogates in paths; which are not valid UTF-16LE.

This has come up before. Yes this should not trust, even if we do not support using such files.

OK, definitely want to change this, but it also affects master branch. Out of scope for this PR

Created #2565 to track

daurnimator · 2019-05-27T04:41:28Z

std/os.zig

+    const wide_slice = wide_len[0..wide_len];
+
+    // Windows returns \\?\ prepended to the path.
+    // We strip it to make this function consistent across platforms.


I'm not sure I understand this comment. Why do we strip \\?\? that would be like stripping / from the start of unix paths.

This is a like a magic way to pass a flag to the windows syscall. It makes it look at the path-name is a very different way.

There are many different types of windows paths; the \\?\ prefix is the way to say "I know what I'm doing", and allows you avoid things like windows blowing up on files named CON.
See https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide-on-win32-to-nt.html?m=1 for more info

daurnimator · 2019-05-27T04:41:41Z

std/os.zig

+}
+
+/// Spurious wakeups are possible and no precision of timing is guaranteed.
+pub fn nanosleep(seconds: u64, nanoseconds: u64) void {


move to std.time?

std.time and std.crypto should use these routines. these are the low-level versions.

I understand your general preference @daurnimator to move stuff out of std/os.zig. I'd like to handle that proposal separately. I think there is something to it, but I also think that the premise of this PR as it stands is sound. The comment at the top says:

// This file contains thin wrappers around OS-specific APIs, with these // specific goals in mind: // * Convert "errno"-style error codes into Zig errors. // * When null-terminated byte buffers are required, provide APIs which accept // slices as well as APIs which accept null-terminated byte buffers. Same goes // for UTF-16LE encoding. // * Where operating systems share APIs, e.g. POSIX, these thin wrappers provide // cross platform abstracting. // * When there exists a corresponding libc function and linking libc, the libc // implementation is used. Exceptions are made for known buggy areas of libc. // On Linux libc can be side-stepped by using `std.os.linux` directly. // * For Windows, this file represents the API that libc would provide for // Windows. For thin wrappers around Windows-specific APIs, see `std.os.windows`. // Note: The Zig standard library does not support POSIX thread cancellation, and // in general EINTR is handled by trying again.

In particular 3 of these points apply to nanosleep:

nanosleep is a libc function, available on linux, freebsd, macos, netbsd, and probably more. os.nanosleep provides cross platform abstraction over these.

on linux when not linking libc, os.nanosleep provides abstraction over calling the libc version or the linux syscall directly.

the possible error of nanosleep is converted to void meaning that it cannot fail (by adding "spurious wakeups are possible" to the documented behavior)

On the other hand, the decision to make it work for Windows is questionable. nanosleep is not a Windows API function, either directly or in msvcrt. os.nanosleep could have Windows support removed and that logic can go directly in std.time.sleep, and become simpler. That's probably a good idea.

Lets continue in #2571

shawnl

Looks good to me (only reviewed Linux changes).

shawnl · 2019-05-27T04:43:30Z

std/fs.zig

+/// in the same directory as dest_path.
+/// Destination file will have the same mode as the source file.
+pub fn copyFile(source_path: []const u8, dest_path: []const u8) !void {
+    var in_file = try File.openRead(source_path);


This could use copy_file_path, but that is another bug (i just opened). #2563

shawnl · 2019-05-27T04:59:27Z

std/fs.zig

+/// Returned value is a slice of out_buffer.
+///
+/// On Linux, depends on procfs being mounted. If the currently executing binary has
+/// been deleted, the file path looks something like `/a/b/c/exe (deleted)`.


This is a funky API, because it is ambiguous.

Yes. That's one reason to make these 3 functions different than each other:

openSelfExe (always correct on linux even if the currently executing exe has been deleted) See https://stackoverflow.com/questions/28953307/how-to-handle-readlink-of-proc-self-exe-when-executable-is-replaced-during/39001204

selfExeDirPath (always correct on linux even if the currently executing exe has been deleted, because taking the dirname gets rid of (deleted))

selfExePath (ambiguous on linux)

always correct on linux even if the currently executing exe has been deleted

I'm not so sure. e.g. what if you create an in-memory file memfd_create() and fexecve() it?

@daurnimator check that stack overflow link 😉

check that stack overflow link wink

I didn't see the answer in there.
FWIW I tried it with the following C program:

#define _GNU_SOURCE #include <sys/mman.h> #include <unistd.h> #include <fcntl.h> char elf[] = { #include "elf.xxd" }; int main() { int fd = memfd_create("foo", MFD_CLOEXEC); if (fd==-1) return 1; if (write(fd, elf, sizeof elf) < 0) return 2; if (fd != 3) return 3; fd = open("/proc/self/fd/3", O_RDONLY); if (-1 == fd) return 4; if (-1 == close(3)) return 5; char *cmd[] = { "ls", "-l", "/proc/self/fd/", "/proc/self/exe", NULL }; char *env[] = { NULL }; if (-1 == fexecve(fd, cmd, env)) return 6; return 7; }

Where elf.xxd is created with xxd -i < /usr/bin/ls > elf.xxd

And I see:

lrwxrwxrwx 1 root root 0 May 27 16:15 /proc/self/exe -> '/memfd:foo (deleted)' /proc/self/fd: total 0 lrwx------ 1 root root 64 May 27 16:15 0 -> /dev/pts/1 lrwx------ 1 root root 64 May 27 16:15 1 -> /dev/pts/1 lrwx------ 1 root root 64 May 27 16:15 2 -> /dev/pts/1 lr-x------ 1 root root 64 May 27 16:15 3 -> /proc/18364/fd lr-x------ 1 root root 64 May 27 16:15 4 -> '/memfd:foo (deleted)'

I'm working on a Linux patch to make this unambiguous. It will be a hack, but it will not require another syscall.

Ahh apologies. I didn't understand what you were getting at, but now I do. (The winky face was just because the stack overflow answer was authored by me a few years ago and I thought that was amusing.) This is a good find - something that also affects master branch. I believe @shawnl is working on a solution.

Created #2570 to track

shawnl · 2019-05-27T05:05:09Z

std/os/bits/linux/arm64.zig

@@ -0,0 +1,405 @@
+// arm64-specific declarations that are intended to be imported into the POSIX namespace.


kinda annoying that github doesn't recognize renames the way git does. (and yes i see you added new syscalls)

I don't think I added new syscalls. I don't remember doing that, and I just did a diff with this file and the corresponding one in master branch. I only tried to fix rename/renameat & friends.

shawnl · 2019-05-27T05:16:35Z

std/os/bits/linux.zig

+    return sum;
+}
+
+// TODO port these over


sigset support is similarly lacking.

daurnimator · 2019-05-27T06:19:50Z

doc/langref.html.in

@@ -9923,6 +9923,7 @@ keyword &lt;- KEYWORD_align / KEYWORD_and / KEYWORD_allowzero / KEYWORD_anyerror
        <li>Avoid local maximums.</li>
        <li>Reduce the amount one must remember.</li>
        <li>Minimize energy spent on coding style.</li>
+        <li>Resource deallocation must succeed.</li>


Need to update the zen in src-self-hosted/main.zig?

emekoi · 2019-05-27T19:00:44Z

std/os.zig

-    OutOfMemory,
+/// Used to convert a slice to a null terminated slice on the stack.
+/// TODO https://github.com/ziglang/zig/issues/287
+pub fn toPosixPath(file_path: []const u8) ![PATH_MAX]u8 {


PATH_MAX doesn't exist on windows and should be MAX_PATH instead.

emekoi · 2019-05-27T19:02:40Z

std/fs/file.zig

+            const path_w = try windows.cStrToPrefixedFileW(path);
+            return openWriteNoClobberW(&path_w, file_mode);
+        }
+        const flags = os.O_LARGEFILE | os.O_WRONLY | os.O_CREAT | os.O_CLOEXEC | os.O_EXCL;


these are not defined on windows for some reason.

emekoi · 2019-05-27T19:04:25Z

std/os.zig

-pub fn posixDup2(old_fd: i32, new_fd: i32) !void {
+        const rc = system.open(file_path, flags, perm);
+        switch (errno(rc)) {
+            0 => return @intCast(fd_t, rc),


fd_t is *c_void on windows.

emekoi · 2019-05-27T19:07:39Z

std/os.zig

+    Unexpected,
+};
+
+pub fn fstat(fd: fd_t) FStatError!Stat {


Stat doesn't exist on windows.

andrewrk · 2019-05-27T20:37:32Z

Thanks everybody for your comments. Tests are passing now and I'm going to merge this into master. If this breaks something for you and simple std lib changes are needed to fix the problem please submit PRs and I will merge them immediately.

If your comments didn't get addressed in this PR please open separate issues for them so that they don't get lost.

fengb mentioned this pull request May 24, 2019

List out posix compatibles #2548

Closed

daurnimator mentioned this pull request May 24, 2019

Add sigaltstack syscall for Linux #2546

Merged

andrewrk force-pushed the posix-layer branch from c93b51d to 57c0579 Compare May 26, 2019 17:37

andrewrk added 10 commits May 26, 2019 18:32

extract posix functions from std/os.zig to std/os/posix.zig

67726e3

See #2380

more progress on posix API layer

daae7e1

see #2380

more progress. moving windows API layer to its own file

2def230

do Jay's suggestion with posix/os API naming & layout

17b0166

rename "posix" to "bits"

3d61e42

starting to fix the regressions

ca6debc

clean up references to posix

7cb6279

clean up references to os

2f040a2

more cleanup. down to just the @hasDecl builtin

44a049e

behavior tests passing on Linux

2b42e91

andrewrk changed the title ~~work-in-progress posix layer~~ rework the API layers between the standard library and the operating system May 27, 2019

andrewrk added the breaking Implementing this issue could cause existing code to no longer compile or have different behavior. label May 27, 2019

tests passing on linux

0c6ab61

andrewrk force-pushed the posix-layer branch from 57c0579 to 0c6ab61 Compare May 27, 2019 03:35

andrewrk marked this pull request as ready for review May 27, 2019 03:37

shritesh reviewed May 27, 2019

View reviewed changes

daurnimator suggested changes May 27, 2019

View reviewed changes

fixes for Windows and WASI

6be79d7

shawnl reviewed May 27, 2019

View reviewed changes

daurnimator mentioned this pull request May 27, 2019

design flaw: operating system page size is not always comptime known, but std lib assumes it is #2564

Closed

andrewrk added 3 commits May 27, 2019 01:35

more fixes for windows and wasi

129714d

std lib fixes for zig build on windows

fda7e0b

fix debug builds of WASI

abf959a

daurnimator reviewed May 27, 2019

View reviewed changes

daurnimator mentioned this pull request May 27, 2019

Windows paths allow unpaired surrogates #2565

Closed

andrewrk added 5 commits May 27, 2019 12:16

nanosleep: move windows logic to std.time

f1610f6

fixes for darwin

0643553

std.fs: fix error set regressions on linux

13265cf

use close$NOCANCEL on darwin

b3dc1c3

darwin: add missing error handling

db0a5e7

emekoi suggested changes May 27, 2019

View reviewed changes

andrewrk added 3 commits May 27, 2019 15:48

revert hello world examples. macos tests passing

5de07ab

fixes for windows to build self hosted compiler

86bb7e5

windows does not integrate cleanly with libc

af4b8eb

freebsd fixes

3640303

andrewrk merged commit 6a56091 into master May 27, 2019

andrewrk deleted the posix-layer branch May 27, 2019 23:21

This was referenced May 28, 2019

selfExeDir doesn't always work #2570

Open

Minimal std/os #2571

Closed

jdalton mentioned this pull request Feb 9, 2024

Add win32 path.toNamespacedPath and align rest of node:path with Node oven-sh/bun#8469

Merged

1 task

		@@ -0,0 +1,405 @@
		// arm64-specific declarations that are intended to be imported into the POSIX namespace.

rework the API layers between the standard library and the operating system #2527

rework the API layers between the standard library and the operating system #2527

Conversation

andrewrk commented May 20, 2019 • edited Loading

What's left to do

daurnimator commented May 24, 2019 • edited Loading

andrewrk commented May 24, 2019

Choose a reason for hiding this comment

shritesh May 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewrk May 27, 2019 • edited Loading

Choose a reason for hiding this comment

shritesh May 27, 2019 • edited Loading

Choose a reason for hiding this comment

daurnimator left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daurnimator May 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daurnimator May 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shawnl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewrk commented May 27, 2019

andrewrk commented May 20, 2019 •

edited

Loading

daurnimator commented May 24, 2019 •

edited

Loading

shritesh May 27, 2019 •

edited

Loading

andrewrk May 27, 2019 •

edited

Loading

shritesh May 27, 2019 •

edited

Loading

daurnimator May 27, 2019 •

edited

Loading

daurnimator May 27, 2019 •

edited

Loading