autoPatchelfHook: Fixes/improvements for Android SDK emulator #50802

aszlig · 2018-11-19T22:57:25Z

This is related to #50596, which refactors the androidenv implementation.

For patching various ELF files, a similar implementation to autoPatchelfHook was introduced there and after talking with @svanderburg, I went on to improve the hook in a way so that it's possible to be also used for androidenv, which also makes it less confusing for users having two things doing roughly the same but are named similarly.

Essentially, these changes make it easier to use only the autoPatchelf command without automatically patching all the build outputs.

For example:

with import <nixpkgs> {}; stdenv.mkDerivation {
  name = "foo";
  ...
  nativeBuildInputs = [ autoPatchelfHook ];
  dontAutoPatchelf = true;
  postPatch = ''
    autoPatchelf foo
    autoPatchelf --no-recurse bar
  '';
}

The --no-recurse flag here only patches files within the bar directory but not files in subdirectories like bar/a/b/c.

In order to make sure these changes do not break existing users of this hook, I've tested it using the following Nix expression and looking for symbol lookup errors:

with import ./. { config.allowUnfree = true; };

runCommand "test-executables" {
  drvs = [
    anydesk cups-kyodialog3 elasticsearch franz gurobi
    masterpdfeditor oracle-instantclient powershell reaper
    sourcetrail teamviewer unixODBCDrivers.msodbcsql17 virtlyst
    vk-messenger wavebox zoom-us nomachine-client polar-bookshelf
  ];
} ("for i in $drvs; do for b in $i/bin/*; do " +
   "[ -x \"$b\" ] && timeout 10 \"$b\" || :; done; done")

The autoPatchelf main function which is run against all of the outputs was pretty much tailored towards this specific setup-hook and was relying on $prefix to be set globally. So if you wanted to run autoPatchelf manually - let's say during buildPhase - you would have needed to run it like this: prefix=/some/directory autoPatchelf This is now more intuitive and all you need to do is run the following: autoPatchelf /some/directory Signed-off-by: aszlig <aszlig@nix.build>

If you want to only run autoPatchelf on a specific path and leave everything else alone, we now have a $dontAutoPatchelf environment variable, which causes the postFixup hook to not run at all. The name "dontAutoPatchelf" probably is a bit weird in conjunction with putting "autoPatchelfHook" in nativeBuildInputs, but unless someone comes up with a better name I keep it that way because it's consistent with all the other dontStrip, dontPatchShebangs, dontPatchELF and whatnot. A specific example where this is needed is when building the Android SDK emulator, which contains a few ARM binaries in subdirectories that should not be patched. If we were to run autoPatchelf on all outputs unconditionally we'd run into errors because some ARM libraries couldn't be found. Signed-off-by: aszlig <aszlig@nix.build>

GrahamcOfBorg · 2018-11-19T23:10:49Z

Success on x86_64-linux (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

  /nix/store/kyclvsckyzsf85m8pq1y68ci1gr158a0-auto-patchelf-hook.drv
these paths will be fetched (0.06 MiB download, 0.17 MiB unpacked):
  /nix/store/3h4j4qk4996cch5xsxldng4r6a5i4cwb-stdenv-linux
  /nix/store/l2i9gmhz0rsi91mz4pvvvnvzkvi5wl6b-bzip2-1.0.6.0.1-bin
  /nix/store/pmvnf35kg6k2p0z91ac663akazlf7wz6-bzip2-1.0.6.0.1
copying path '/nix/store/pmvnf35kg6k2p0z91ac663akazlf7wz6-bzip2-1.0.6.0.1' from 'https://nix-cache.s3.amazonaws.com'...
copying path '/nix/store/l2i9gmhz0rsi91mz4pvvvnvzkvi5wl6b-bzip2-1.0.6.0.1-bin' from 'https://nix-cache.s3.amazonaws.com'...
copying path '/nix/store/3h4j4qk4996cch5xsxldng4r6a5i4cwb-stdenv-linux' from 'https://nix-cache.s3.amazonaws.com'...
building '/nix/store/kyclvsckyzsf85m8pq1y68ci1gr158a0-auto-patchelf-hook.drv'...
/nix/store/b7aky5y002ipvj34nv6k3dv40z1hlryp-auto-patchelf-hook

GrahamcOfBorg · 2018-11-19T23:10:50Z

Success on x86_64-darwin (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

these derivations will be built:
  /nix/store/lm8kfbw38i9m5w0byh2fp3750yjxdpca-auto-patchelf-hook.drv
building '/nix/store/lm8kfbw38i9m5w0byh2fp3750yjxdpca-auto-patchelf-hook.drv'...
/nix/store/0hjvkix1nvp6slr0r9j37sw5lsshrj7f-auto-patchelf-hook

GrahamcOfBorg · 2018-11-19T23:10:55Z

Success on aarch64-linux (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

these derivations will be built:
  /nix/store/303i8gzpa9128a85a2bwahi8cg914kii-auto-patchelf-hook.drv
building '/nix/store/303i8gzpa9128a85a2bwahi8cg914kii-auto-patchelf-hook.drv'...
/nix/store/80jmgld7lkbi1mg5ms3c7hky2f697l4y-auto-patchelf-hook

@svanderburg

This is to be used with the autoPatchelf command and allows to only patch a specific file or directory without recursing into subdirectories. Apart from being able to run the command in a standalone way, as detailled in the previous commit this is also needed for the Android SDK emulator, because according to @svanderburg there are subdirectories we don't want to patch. The reason why I didn't use GNU getopt is that it might not be available on all operating systems and the getopts bash builtin doesn't support long arguments. Apart from that, the implementation for recognizing the flag is pretty trivial and it's also using bash builtins only, so if we want to do something really fancy someday, we can still change it. Signed-off-by: aszlig <aszlig@nix.build>

@svanderburg

First of all, this makes the existing documentation a bit more clear on what autoPatchelfHook is all about, because after discussing with @svanderburg - who wrote a similar implementation - the rationale about autoPatchelfHook wasn't very clear in the documentation. I also added the recent changes around being able to use autoPatchelf manually and the new --no-recurse flag. Signed-off-by: aszlig <aszlig@nix.build>

GrahamcOfBorg · 2018-11-19T23:37:51Z

Success on x86_64-linux (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

/nix/store/b7aky5y002ipvj34nv6k3dv40z1hlryp-auto-patchelf-hook

GrahamcOfBorg · 2018-11-19T23:37:56Z

Success on x86_64-darwin (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

/nix/store/0hjvkix1nvp6slr0r9j37sw5lsshrj7f-auto-patchelf-hook

GrahamcOfBorg · 2018-11-19T23:37:56Z

Success on aarch64-linux (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

/nix/store/80jmgld7lkbi1mg5ms3c7hky2f697l4y-auto-patchelf-hook

Mic92

I have not tested it, but the code and the documentation make sense to me.
The example from your description would also fit well into the documentation.

svanderburg · 2018-11-20T08:35:43Z

Great! I'll try to adjust my androidenv pull request later today and see if it works with the new implementation. I'll report back asap!

svanderburg · 2018-11-20T20:15:18Z

I believe I am still having a practical issue while trying to migrate the Android SDK expressions to the autopatchelf hook implementation.

Apparently, to use autoPatchelf, you must refer to the libraries that you want to use (through buildInputs or runtimeDependencies) by providing the Nix store paths of the packages that you want to use. The autopatchelf hook seems to scan the lib/ sub folders of every Nix store path for the libraries that it needs.

I don't think there is a way to directly refer to library paths (without making an assumption that there is a lib/ sub folder)? The problem is that the Android SDK also bundles shared libraries residing in weird sub folders inside the same base directory as the executables, e.g. <basedir>/lib64, /lib/qt` etc.

The autopatchelf implementation that I created uses colon separated library paths (as opposed to white space delimited Nix store paths) and can also inspect library sub folders in the same package. Is there also a way to add this kind of flexibility to the autopatchelf hook?

(As a sidenote: my original motivation to use colon separated library paths is that you can use LD_LIBRARY_PATH outside a Nix builder environment to automatically make dynamic library references static).

My biggest struggle is to port the expression that patches the Android emulator. It needs external packages, as well as libraries that are bundled with the emulator package itself, as you may probably see while inspecting contents of this file: https://github.com/svanderburg/nix-androidenvtests/blob/nextgen/androidenv/emulator.nix

In the expression, I refer to Nix packages (and use lib.makeLibraryPath to compose the paths), but I also specify paths referring to sub directories inside the same package. autopatchelf will also inspect these folders for libraries that an executable may need.

Would it also be possible to do something similar with the autoPatchelf command or adjust it in such a way that it can?

aszlig · 2018-11-20T22:30:39Z

The autopatchelf implementation that I created uses colon separated library paths (as opposed to white space delimited Nix store paths) and can also inspect library sub folders in the same package. Is there also a way to add this kind of flexibility to the autopatchelf hook?

The library subdirectories for the same package are already inspected by autoPatchelfHook:

nixpkgs/pkgs/build-support/setup-hooks/auto-patchelf.sh

Lines 174 to 177 in 503b41b

    
               cachedDependencies+=( 
        
                   $(find "$@" "${findOpts[@]}" \! -type d \ 
        
                          \( -name '*.so' -o -name '*.so.*' \)) 
        
               )

However, when running autoPatchelf manually without the postFixup hook, it will only work on the path you've specified.

Maybe it makes sense to add shell function like addAutoPatchelfSearchPath (maybe with a shorter), which allows to add the patch to the dependency cache.

Does that sound good for you?

And would such a function then also recurse into subdirectories or even have the same --no-recurse flag?

My biggest struggle is to port the expression that patches the Android emulator. It needs external packages, as well as libraries that are bundled with the emulator package itself, as you may probably see while inspecting contents of this file: https://github.com/svanderburg/nix-androidenvtests/blob/nextgen/androidenv/emulator.nix

Seperately specifying libs_i386 and libs_x86_64 here shouldn't be necessary because autoPatchelf will only find compatible libraries or do you see a problem in that approach?

svanderburg · 2018-11-20T22:53:12Z

Seperately specifying libs_i386 and libs_x86_64 here shouldn't be necessary because autoPatchelf will only find compatible libraries or do you see a problem in that approach?

I don't need the separation, although I would find it nicer to write it separately, but that's just a matter of taste. I can live without it.

Basically, the only problem I have is converting this line in the emulator expression:

export libs_x86_64=$packageBaseDir/lib64:$packageBaseDir/lib64/qt/lib:$libs_x86_64

I want autopatchelf to inspect these two directories shown above in addition to the packages that I provide as buildInputs. These two paths translate to: $out/libexec/android-sdk/emulator/lib64 and $out/libexec/android-sdk/emulator/lib64/qt/lib (as you may see, these paths are quite unconventional).

If there's any environment variable (or function) allowing me to provide these paths, so that they get added to the RPATH of the right executables, that will basically fix my problem.

aszlig · 2018-11-20T23:09:07Z

The example you mentioned can be added via:

cachedDependencies+=( 
  $(find "$out/libexec/android-sdk/emulator/lib64" \
    \! -type d \( -name '*.so' -o -name '*.so.*' \))
)

That's a lot of churn, so that's why I mentioned factoring this out into a dedicated function, but this should work (haven't tested it though).

svanderburg · 2018-11-21T09:00:08Z

@aszlig Would be nice if you could implement a function wrapper so that I can conveniently add these paths

This function is useful if autoPatchelf is invoked during some of the phases of a build and allows to add arbitrary shared objects to the search path. So far the same functionality was in autoPatchelf itself, but not available as a separate function, so when adding shared objects to the dependency cache one would have to do so manually. The function also has the --no-recurse flag, which prevents recursing into subdirectories. Signed-off-by: aszlig <aszlig@nix.build>

aszlig · 2018-11-25T15:23:48Z

@svanderburg: Added the mentioned addAutoPatchelfSearchPath function.

GrahamcOfBorg · 2018-11-25T15:39:20Z

Success on x86_64-darwin (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

these derivations will be built:
  /nix/store/g3axkjvf3yhl39qj7n7h3cii37kgdbsf-auto-patchelf-hook.drv
building '/nix/store/g3axkjvf3yhl39qj7n7h3cii37kgdbsf-auto-patchelf-hook.drv'...
/nix/store/n5wgv0z2kq6p6c7myn3inr66pa6qvx1m-auto-patchelf-hook

GrahamcOfBorg · 2018-11-25T15:39:26Z

Success on aarch64-linux (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

these derivations will be built:
  /nix/store/hsc3qgyxax4pkxcb3lvq1y4n9xndsipl-auto-patchelf-hook.drv
building '/nix/store/hsc3qgyxax4pkxcb3lvq1y4n9xndsipl-auto-patchelf-hook.drv'...
/nix/store/dwamgiyrw9b0pj9qdrqjfnmwdkh5yhp4-auto-patchelf-hook

GrahamcOfBorg · 2018-11-25T15:39:44Z

Success on x86_64-linux (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

these derivations will be built:
  /nix/store/ksvgg8h435yfbxbb446v8382rsbxy8v6-auto-patchelf-hook.drv
building '/nix/store/ksvgg8h435yfbxbb446v8382rsbxy8v6-auto-patchelf-hook.drv'...
/nix/store/m2rrwzyd3nz21avx5l40953yzzvxx2lp-auto-patchelf-hook

svanderburg · 2018-11-25T23:20:44Z

Great now it seems to work a lot better. I have managed to make the basic test cases work (deploying an SDK installation with some plugins, building an APK, and launching the emulator).

Unfortunately, when try to compile the hello-jni example (that depends on the Android NDK), I'm still running into a problem:

setting RPATH to: /nix/store/1fd7ybkplw7cjx6hyiiyvb03abx0l0iz-ncurses-6.1-abi5-compat/lib:/nix/store/1fd7ybkplw7cjx6hyiiyvb03abx0l0iz-ncurses-6.1-abi5-compat/lib
searching for dependencies of prebuilt/linux-x86_64/bin/yasm
searching for dependencies of prebuilt/linux-x86_64/bin/python2
searching for dependencies of prebuilt/linux-x86_64/lib/python2.7/config/python.o
wrong ELF type
builder for '/nix/store/9zvwb63r9pcv4az6q1fsvlj8s5hymv26-ndk-bundle-18.1.5063045.drv' failed with exit code 1

Apparently, it recurses into a directory that provides an object file (python.o). autoPatchelf tries to patch it. With my autopatchelf implementation this file is skipped because I use the file command to determine whether a file is an appropriate ELF binary or not. Yours apparently works different. Would it be possible to exclude ordinary .o files so that the patch process succeeds?

(As a sidenote: it's really stupid of the Android package maintainers to include this file, but this something we have to live with)

aszlig · 2018-11-25T23:44:54Z

autoPatchelfHook was using file as well, see comments in #47222, the problem with file however is that it is very slow if you have a large number of files other than ELF files. I'll fix the object files case in a minute.

aszlig · 2018-11-26T00:01:39Z

~~@svanderburg: Can you provide an output of readelf -h -l prebuilt/linux-x86_64/lib/python2.7/config/python.o? I tested with a bunch of object files but couldn't reproduce it.~~

Never mind, got the ndk-bundle by myself and investigating...

While declaring it as an array doesn't do any harm in our usage, it might be a bit confusing when reading the code. Signed-off-by: aszlig <aszlig@nix.build>

GrahamcOfBorg · 2018-11-26T00:37:30Z

Success on aarch64-linux (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

these derivations will be built:
  /nix/store/31yxzbhl3sjbmjymrzyrfqsqd2k51b29-auto-patchelf-hook.drv
building '/nix/store/31yxzbhl3sjbmjymrzyrfqsqd2k51b29-auto-patchelf-hook.drv'...
/nix/store/a3priyk68x48s55q7b81s8mmrmh36s9v-auto-patchelf-hook

GrahamcOfBorg · 2018-11-26T00:49:20Z

Success on x86_64-linux (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

these derivations will be built:
  /nix/store/5v95psss0kbm65zkiajmwp3sxwhx2i6h-auto-patchelf-hook.drv
building '/nix/store/5v95psss0kbm65zkiajmwp3sxwhx2i6h-auto-patchelf-hook.drv'...
/nix/store/w5wy21k8c6ncchlrpikhxmf3ms697n8f-auto-patchelf-hook

GrahamcOfBorg · 2018-11-26T00:51:39Z

Success on x86_64-darwin (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

these derivations will be built:
  /nix/store/dwz8z4clzb5f37arljbhd34ifmn9010z-clang-5.0.2.drv
  /nix/store/q6iw6cww7baxq43zmwkxj491k8a07s3l-stdenv-darwin.drv
  /nix/store/m04xsjnajcngzy93svpnkvqqxp2qnkyl-auto-patchelf-hook.drv
waiting for locks or build slots...
building '/nix/store/q6iw6cww7baxq43zmwkxj491k8a07s3l-stdenv-darwin.drv'...
building '/nix/store/m04xsjnajcngzy93svpnkvqqxp2qnkyl-auto-patchelf-hook.drv'...
/nix/store/larwcz9jdjriaj9q3drwc5q0k1fb5sii-auto-patchelf-hook

If the file in question is not a shared object file but an ELF, we really want to skip the file, because we won't have anything to patch there. For example if the file is created via "gcc -c -o foo.o foo.c", we don't get a segment header and so far autoPatchelf was trying to patch such a file. By checking for missing segment headers, we're now no longer going to attempt patching such a file. Signed-off-by: aszlig <aszlig@nix.build> Reported-by: Sander van der Burg <svanderburg@gmail.com>

aszlig · 2018-11-26T01:12:26Z

@svanderburg: Fixed, thanks.

aszlig · 2018-11-26T01:22:58Z

@svanderburg: Btw. regarding the ability to run autoPatchelf outside of the setup hook, I might do that in the near future because I have a test suite I'd like to use here but don't want to dump into nixpkgs right now because it includes a whole bunch of binaries I gathered from random places around the web. Putting the test suite along with autoPatchelf into a separate repository will then not clutter up nixpkgs with all those binaries. But that's something for another PR because I have limited time right now.

GrahamcOfBorg · 2018-11-26T01:27:11Z

Success on aarch64-linux (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

these derivations will be built:
  /nix/store/y1m11p1zbh96vig2py99q649xiarj0dw-auto-patchelf-hook.drv
building '/nix/store/y1m11p1zbh96vig2py99q649xiarj0dw-auto-patchelf-hook.drv'...
/nix/store/dxddqfr0irzinz6zyjzqd5g6yhhx7p1c-auto-patchelf-hook

GrahamcOfBorg · 2018-11-26T01:27:59Z

Success on x86_64-linux (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

these derivations will be built:
  /nix/store/hdf0a18pri85rnkppzixp9phjqnxdkp3-auto-patchelf-hook.drv
building '/nix/store/hdf0a18pri85rnkppzixp9phjqnxdkp3-auto-patchelf-hook.drv'...
/nix/store/77l050wjvddcipln4ay0frpwkvygl1gi-auto-patchelf-hook

GrahamcOfBorg · 2018-11-26T02:30:24Z

Timed out, unknown build status on x86_64-darwin (full log)

Attempted: autoPatchelfHook

Partial log (click to expand)

[ 68%] Updating PPCGenFastISel.inc...
building of '/nix/store/plp1iag0842vjjzlzv7ynn6xahrapvx1-llvm-5.0.2.drv' timed out after 3600 seconds
cannot build derivation '/nix/store/hv8vrp6shlb2lnmg8s9jfgdq56gp61pq-cctools-port-895.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/dwz8z4clzb5f37arljbhd34ifmn9010z-clang-5.0.2.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/gakqwwlvhgnb6y7xp5qxvrjczz7ql4yy-compiler-rt-5.0.2.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/y01mnj37vk51my6yxyjdrzgv2wb00sfx-cctools-binutils-darwin.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/gjgmbdh04sfv3kmhdlajm23vsgm0six7-cctools-binutils-darwin-wrapper.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/q6iw6cww7baxq43zmwkxj491k8a07s3l-stdenv-darwin.drv': 6 dependencies couldn't be built
cannot build derivation '/nix/store/0i897mrh9syp4k2qjh2mimnzvbjzk340-auto-patchelf-hook.drv': 1 dependencies couldn't be built
error: build of '/nix/store/0i897mrh9syp4k2qjh2mimnzvbjzk340-auto-patchelf-hook.drv' failed

svanderburg · 2018-11-27T09:26:05Z

Hi,

I just did another test and the good news is: it seems to work. I now managed to successfully compile the hello-jni example and run it in the emulator.

svanderburg · 2018-11-27T09:26:52Z

This change can be integrated into the upstream branch. I probably have to rebase my Android pull request since it's already conflicting with the current Nixpkgs master branch.

aszlig added 0.kind: enhancement 8.has: documentation labels Nov 19, 2018

aszlig self-assigned this Nov 19, 2018

aszlig requested review from Mic92 and svanderburg November 19, 2018 22:57

aszlig requested a review from Ericson2314 as a code owner November 19, 2018 22:57

aszlig mentioned this pull request Nov 19, 2018

Mobile updates #50596

Merged

25 tasks

GrahamcOfBorg added 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 labels Nov 19, 2018

aszlig added 2 commits November 20, 2018 00:11

aszlig force-pushed the autopatchelf-improvements branch from 2c73283 to 503b41b Compare November 19, 2018 23:11

Mic92 approved these changes Nov 20, 2018

View reviewed changes

autoPatchelfHook: Fix type of norecurse variable

9f23a63

While declaring it as an array doesn't do any harm in our usage, it might be a bit confusing when reading the code. Signed-off-by: aszlig <aszlig@nix.build>

svanderburg approved these changes Nov 27, 2018

View reviewed changes

Mic92 merged commit afbdeb7 into NixOS:master Nov 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

autoPatchelfHook: Fixes/improvements for Android SDK emulator #50802

autoPatchelfHook: Fixes/improvements for Android SDK emulator #50802

aszlig commented Nov 19, 2018 •

edited

GrahamcOfBorg commented Nov 19, 2018

GrahamcOfBorg commented Nov 19, 2018

GrahamcOfBorg commented Nov 19, 2018

GrahamcOfBorg commented Nov 19, 2018

GrahamcOfBorg commented Nov 19, 2018

GrahamcOfBorg commented Nov 19, 2018

Mic92 left a comment

svanderburg commented Nov 20, 2018

svanderburg commented Nov 20, 2018

aszlig commented Nov 20, 2018

svanderburg commented Nov 20, 2018 •

edited

aszlig commented Nov 20, 2018

svanderburg commented Nov 21, 2018

aszlig commented Nov 25, 2018

GrahamcOfBorg commented Nov 25, 2018

GrahamcOfBorg commented Nov 25, 2018

GrahamcOfBorg commented Nov 25, 2018

svanderburg commented Nov 25, 2018

aszlig commented Nov 25, 2018

aszlig commented Nov 26, 2018 •

edited

GrahamcOfBorg commented Nov 26, 2018

GrahamcOfBorg commented Nov 26, 2018

GrahamcOfBorg commented Nov 26, 2018

aszlig commented Nov 26, 2018

aszlig commented Nov 26, 2018

GrahamcOfBorg commented Nov 26, 2018

GrahamcOfBorg commented Nov 26, 2018

GrahamcOfBorg commented Nov 26, 2018

svanderburg commented Nov 27, 2018

svanderburg commented Nov 27, 2018

autoPatchelfHook: Fixes/improvements for Android SDK emulator #50802

autoPatchelfHook: Fixes/improvements for Android SDK emulator #50802

Conversation

aszlig commented Nov 19, 2018 • edited

GrahamcOfBorg commented Nov 19, 2018

GrahamcOfBorg commented Nov 19, 2018

GrahamcOfBorg commented Nov 19, 2018

GrahamcOfBorg commented Nov 19, 2018

GrahamcOfBorg commented Nov 19, 2018

GrahamcOfBorg commented Nov 19, 2018

Mic92 left a comment

Choose a reason for hiding this comment

svanderburg commented Nov 20, 2018

svanderburg commented Nov 20, 2018

aszlig commented Nov 20, 2018

svanderburg commented Nov 20, 2018 • edited

aszlig commented Nov 20, 2018

svanderburg commented Nov 21, 2018

aszlig commented Nov 25, 2018

GrahamcOfBorg commented Nov 25, 2018

GrahamcOfBorg commented Nov 25, 2018

GrahamcOfBorg commented Nov 25, 2018

svanderburg commented Nov 25, 2018

aszlig commented Nov 25, 2018

aszlig commented Nov 26, 2018 • edited

GrahamcOfBorg commented Nov 26, 2018

GrahamcOfBorg commented Nov 26, 2018

GrahamcOfBorg commented Nov 26, 2018

aszlig commented Nov 26, 2018

aszlig commented Nov 26, 2018

GrahamcOfBorg commented Nov 26, 2018

GrahamcOfBorg commented Nov 26, 2018

GrahamcOfBorg commented Nov 26, 2018

svanderburg commented Nov 27, 2018

svanderburg commented Nov 27, 2018

aszlig commented Nov 19, 2018 •

edited

svanderburg commented Nov 20, 2018 •

edited

aszlig commented Nov 26, 2018 •

edited