Rework Updating #65

tilosp · 2017-04-14T20:33:59Z

TODO:

version comparison
occ upgrade
show disabled apps
split the app folder
disable the updater
replace chown -R www-data /var/www/html For big installations, chown of data folder is taking a looong time #26

mkuron · 2017-04-15T07:51:28Z

Wouldn't it make sense to automatically execute occ upgrade after installing an update? Automatically performing any needed database migrations is standard on most other dockerized applications.

tilosp · 2017-04-15T08:02:38Z

You are right. This PR is WIP and i want to implement this pseudo code of @pierreozoux:

if version is different in /usr/src and /var/www/html then:
  tar cf - --one-file-system -C /usr/src/nextcloud . | tar xf -
  php occ app:list > /tmp/list_before
  for app in `ls /usr/src/nextcloud/apps/`; do rm -rf ./apps/$app;cp -TR /usr/src/nextcloud/apps/$app/ ./apps/$app/; chown -R www-data:www-data ./apps/$app/; done
  php occ upgrade --no-app-disable
  php occ app:list > /tmp/list_after
  echo "the following app have beed disabled:
  diff <(sed -n "/Enabled:/,/Disabled:/p" /tmp/list_before) <(sed -n "/Enabled:/,/Disabled:/p" /tmp/list_after) | grep '<' | cut -d- -f2 | cut -d: -f1
fi

mkuron · 2017-04-15T10:28:01Z

10.0/apache/docker-entrypoint.sh

@@ -18,7 +18,18 @@ if version_greater "$installed_version" "$image_version"; then
 fi

 if version_greater "$image_version" "$installed_version"; then
-    tar cf - --one-file-system -C /usr/src/nextcloud . | tar xf -
+    rsync -a --delete --exclude /config/ --exclude /data/ --exclude /apps/ /usr/src/nextcloud/ /var/www/html/


Is there any reason why the tarball isn't directly extracted to /var/www/html in the Dockerfile? Then you wouldn't need to copy here.

Yes because we need to use a volume to mount it into nginx. And a container on its own will not overwrite the content of a volume.

tilosp · 2017-04-15T19:47:22Z

docker-entrypoint.sh

+if version_greater "$installed_version" "$image_version"; then
+    echo "Downgrade not supported"
+    exit 1
+fi


This check will break the update app. This mean if we keep it we should disable the update app.

pierreozoux

This looks good to me \o/ Thanks for the work.

Looks like the app folder is already splitted?

Agree with the updater, then it is good if we have a consistent way.

pierreozoux · 2017-04-17T08:57:38Z

10.0/apache/apps.config.php

+              "writable" => false,
+      ),
+      1 => array (
+              "path"     => OC::$SERVERROOT."/apps2",


What about having a better naming than 2?
What about custom_apps or user_apps ?

cyphar

Aside from my comments this looks okay at the moment -- though we should disable the update app (as you said) and write some documentation to explain how this has to be used.

I recently figured out that my current NextCloud setup is broken because of this issue so I'll be reinstalling soon and I'll be able to tell you how well this new system works.

Also it'd be nice if you squashed some of your commits (the app2 and custom_app commits for example).

Though, can you explain what you mean in the issue description by "replace chown -R www-data"?

cyphar · 2017-04-17T14:40:20Z

docker-entrypoint.sh

+	[[ "$(printf '%s\n' "$@" | sort -V | head -n 1)" != "$1" ]];
+}
+
+installed_version="0.0.0"


Minor nit, it would be nice if we made this 0.0.0~unknown or something like that. But it's not necessary.

this could cause problems inside the version_greater function

No it won't -- I work with rpm-style versions quite a lot. sort -V does version checks against the leading digits first. In general you would use ~ for things like 1.0.0~rc1 which is a lower version number than 1.0.0 (or 1.0.0~rc2). However something like 0.0.0+rc1 is a higher version number than 1.0.0.

But it's up to you.

thanks for explaining, i changed it.

cyphar · 2017-04-17T14:40:43Z

docker-entrypoint.sh

+fi
+
+if version_greater "$image_version" "$installed_version"; then
+    if version_greater "$installed_version" "0.0.0"; then


Maybe we should do something like [[ "$installed_version" != "0.0.0" ]] in all of these cases to make it more clear which is the upgrade path and which is the fresh install path?

tilosp · 2017-04-17T20:32:52Z

Though, can you explain what you mean in the issue description by "replace chown -R www-data"?

I want to fix #26.
rsync -a should copy the owner and this mean we could just set the right owner in the source dir at build time

SnowMB

Great work so far!

I've one question regarding the volume declaration:
With your function to compare the versions we know what kind of installation we're dealing with (new one, existing one, upgrade, downgrade). Since you also split the app folder, is there any reason to have a volume for the /var/www/html folder?
The Dockerfile could just install the newest version directly to /var/www/html. The entrypoint script checks the possibilities (new installation does not contain a config.php):

no config.php: create new one with default content (e.g. split app folder)
existing config.php with matching version: do nothing
existing config.php with higher version: exit 1 and echo error
existing config.php with lower version: perfom occ:upgrade

This would save us a lot of scripting and the whole chown deal.

SnowMB · 2017-04-18T07:45:28Z

10.0/apache/docker-entrypoint.sh

+image_version=$(php -r 'require "/usr/src/nextcloud/version.php"; echo "$OC_VersionString";')
+
+if version_greater "$installed_version" "$image_version"; then
+    echo "Downgrade not supported"


Should probably be more specific. Something like

echo "Can't start Nextcloud because the version of the data ($installed_version) is higher than the docker image version ($image_version) and downgrading is not supported. Are you sure you have pulled the newest image version?"

ChessSpider · 2017-04-18T08:11:33Z

Hi Guys,
I'm wondering, why we're holding on to the extracting source-code into the directory?

If I pull version 11.0.2-fpm, or fpm, or latest, then I would reasonably expect to find that source-code in the source directory.

I suggest this:

Place the source-code directly into the /var/www/html directory with the right owner
1. I'd argue the owner of most files should be e.g. root, as to prevent any in-place upgrades/changes of source-code from a security perspective
2. This also means the automated updates of nextcloud should be disabled, and instead, the Docker way of upgrading should be used (docker --pull). This is good, as it means that any system security problems for the underlying OS and dependancies are also patched. (FROM php, FROM ubuntu,...)
3. You also don't need to install rsync..
Define VOLUMEs for all directories containing user-content or dynamic data. I believe this to be the /data/ directory, the /config directory and the /app directory?
1. The end-user can always override these VOLUMEs by redefining them in their docker-compose or docker run cmd
Define VOLUMEs for directories containing static data, like .js and .css files.
1. The end user could either use volumes_from, or preferably, override this VOLUME with a named-volume and mount that volume as read-only in their nginx
Make an entrypoint script which checks the installed version of nextcloud (should be possible via either config.php or DB), and if required, do an automated occ:upgrade.
Possibly also make the entrypoint-script enable automatic installation of nextcloud by reading ENV variables (though I'd personally not use it due to security concerns with exposing credentials like that, some might like it)

tilosp · 2017-04-18T08:31:23Z

@ChessSpider
but all of those volumes for static files had to be recreated if you want to update, or am i wrong?

ChessSpider · 2017-04-18T08:39:22Z

@tilosp When you do a docker --pull to update a container it should reuse the VOLUMEs of the previous version of the container, right? I'm assuming here honestly, as I always declare my volumes explicitly in the docker-compose..

You don't want to recreate the volumes containing dynamic/user data. when updating. As that means you'd lose all the uploaded files/apps/etc?

tilosp · 2017-04-18T08:45:38Z

@ChessSpider Yes but a newly created container will not overwrite files in a volume it will just add new ones.

ChessSpider · 2017-04-18T09:01:44Z

What I propose is something in the direction of:

FROM php:5.6-apache
RUN apt-get install........ etc
RUN tar -xjf nextcloud.tar.bz2 -C /var/www/html

# Volumes for dynamic data
VOLUME /var/www/html/config
VOLUME /var/www/html/data
VOLUME /var/www/html/custom_apps
# volumes for template purposes or static files which are to be served by nginx in a fpm environment
VOLUME /var/www/html/vendor
VOLUME /var/www/html/css
VOLUME /var/www/html/static all files...

wait, what, why are the static files all over the place? Are they not collected somewhere in one directory ?

root@1a83c9988257:/var/www/html# find . -name '*.css' 
./core/vendor/select2/select2.css
./core/vendor/strengthify/strengthify.css
./core/vendor/jcrop/css/jquery.Jcrop.css
./core/vendor/jquery-ui/themes/base/jquery-ui.css
./core/css/global.css
./core/css/mobile.css
./core/css/fixes.css
./core/css/tooltip.css
./core/css/inputs.css
./core/css/share.css
./core/css/jquery.ocdialog.css
./core/css/jquery-ui-fixes.css
./core/css/header.css
./core/css/fonts.css
./core/css/multiselect.css
./core/css/icons.css
./core/css/styles.css
./core/css/apps.css
./core/css/lostpassword/resetpassword.css
./core/css/systemtags.css
./core/css/update.css
./core/doc/user/_static/bootstrap-3.1.0/css/bootstrap-theme.css
./core/doc/user/_static/bootstrap-3.1.0/css/bootstrap.css
./core/doc/user/_static/bootstrap-3.1.0/css/bootstrap.min.css
./core/doc/user/_static/bootstrap-3.1.0/css/bootstrap-theme.min.css
./core/doc/user/_static/basic.css
./core/doc/user/_static/pygments.css
./core/doc/user/_static/bootstrap.css
./core/doc/user/_static/main.min.css
./core/doc/user/_static/bootstrap-sphinx.css
./core/doc/user/_static/styles.css
./core/doc/user/_static/bootstrap-responsive.css
./core/doc/user/_static/style.css
./core/doc/admin/_static/bootstrap-3.1.0/css/bootstrap-theme.css
./core/doc/admin/_static/bootstrap-3.1.0/css/bootstrap.css
./core/doc/admin/_static/bootstrap-3.1.0/css/bootstrap.min.css
./core/doc/admin/_static/bootstrap-3.1.0/css/bootstrap-theme.min.css
./core/doc/admin/_static/basic.css
./core/doc/admin/_static/pygments.css
./core/doc/admin/_static/bootstrap.css
./core/doc/admin/_static/main.min.css
./core/doc/admin/_static/bootstrap-sphinx.css
./core/doc/admin/_static/styles.css
./core/doc/admin/_static/bootstrap-responsive.css
./core/doc/admin/_static/style.css
./core/search/css/results.css
./settings/css/settings.css
./3rdparty/sabre/dav/lib/DAV/Browser/assets/openiconic/open-iconic.css
./3rdparty/sabre/dav/lib/DAV/Browser/assets/sabredav.css
./themes/example/core/css/styles.css
./apps/password_policy/css/settings-admin.css
./apps/encryption/css/settings-personal.css
./apps/encryption/css/settings-admin.css
./apps/files_pdfviewer/vendor/pdfjs/web/viewer.css
./apps/files_pdfviewer/css/viewer.css
./apps/files_pdfviewer/css/minmode.css
./apps/files_pdfviewer/css/style.css
./apps/gallery/css/mobile.css
./apps/gallery/css/github-markdown.css
./apps/gallery/css/slideshow.css
./apps/gallery/css/public.css
./apps/gallery/css/upload.css
./apps/gallery/css/gallerybutton.css
./apps/gallery/css/share.css
./apps/gallery/css/styles.css
./apps/gallery/css/authenticate.css
./apps/gallery/css/error.css
./apps/files_external/css/external.css
./apps/files_external/css/settings.css
./apps/files_videoplayer/css/style.css
./apps/files_videoplayer/videojs/src/video-js.css
./apps/workflowengine/css/admin.css
./apps/external/css/style.css
./apps/user_ldap/vendor/ui-multiselect/jquery.multiselect.css
./apps/user_ldap/css/settings.css
./apps/files_texteditor/css/mobile.css
./apps/files_texteditor/css/DroidSansMono/stylesheet.css
./apps/files_texteditor/css/style.css
./apps/files_sharing/css/mobile.css
./apps/files_sharing/css/public.css
./apps/files_sharing/css/sharetabview.css
./apps/files_sharing/css/authenticate.css
./apps/files_sharing/css/sharebreadcrumb.css
./apps/files_sharing/css/sharedfilelist.css
./apps/files_sharing/css/404.css
./apps/files/css/mobile.css
./apps/files/css/upload.css
./apps/files/css/files.css
./apps/files/css/detailsView.css
./apps/serverinfo/css/style.css
./apps/user_saml/css/personal.css
./apps/user_saml/css/admin.css
./apps/files_versions/css/versions.css
./apps/templateeditor/css/settings-admin.css
./apps/theming/css/settings-admin.css
./apps/files_retention/css/retention.css
./apps/logreader/css/app.css
./apps/logreader/build/main.css
./apps/federation/css/settings-admin.css
./apps/activity/css/settings.css
./apps/activity/css/style.css
./apps/survey_client/css/admin.css
./apps/comments/css/comments.css
./apps/twofactor_backupcodes/css/style.css
./apps/files_trashbin/css/trash.css
./apps/federatedfilesharing/css/settings-personal.css
./apps/systemtags/css/systemtagsfilelist.css
./apps/firstrunwizard/css/colorbox.css
./apps/firstrunwizard/css/firstrunwizard.css
./apps/notifications/css/styles.css

ChessSpider · 2017-04-18T09:18:34Z

just to clarify, the simplified docker-compose for my nginx and app of a project of mine is this:

  version: '2'

  networks:
    net:
      driver: bridge
  services:
    web:
      ports:
        - "8000:8000"
      image: nginx
      user: www-data
      volumes:
        - ./nginx.conf:/etc/nginx/nginx.conf:ro
        - ./app/static:/var/www/html/static/:ro
        - ./app/dynamic:/var/www/html/dynamic/:ro
      tmpfs:
        - /var/cache/nginx/:uid=33,rw,noexec,nosuid
        - /run:uid=33,rw,noexec,nosuid
      security_opt:
        - no-new-privileges
      cap_drop:
        - net_raw
        - mknod
      links:
        - app
      networks:
        - net
    app:
      image: app
      user: lowprivuser
      links:
        - db
      volumes:
        - ./app/static/:/usr/src/app/static/
        - ./app/dynamic/:/usr/src/app/dynamic/
      security_opt:
        - no-new-privileges
      cap_drop:
        - net_raw
        - mknod
      tmpfs:
        - /tmp:rw,noexec,nosuid
      networks:
        - net

The app on first-run collects all static files and puts them in the /static/ directory.

tilosp · 2017-04-19T07:39:07Z

@ChessSpider
if you run these commands you can see that the content of the volume is not getting updated.

$ sudo docker run --rm -v test:/usr/src/nextcloud nextcloud:9 bash -c exit
$ sudo docker run --rm -v test:/usr/src/nextcloud nextcloud:10 bash -c exit
$ sudo docker run --rm -v test:/test debian cat /test/version.php
<?php 
$OC_Version = array(9,0,57,2);
$OC_VersionString = '9.0.57';

Your static files volumes would have the same problem.

ChessSpider · 2017-04-19T07:51:04Z

Yeah but that's with the old VOLUME declaration on /var/www/html right? So that makes sense. The updater only runs if you first remove version.php to start the extraction process.

I agree that static files volume would have that behavior, but it would only be a problem while downgrading. Which we don't support anyway. When upgrading, old static files will be overwritten. Deleted static files will not be referenced in the HTML and thus should not be a problem.

Also static files should only have to be a VOLUME for the FPM-usecase.

I do worry about the static files being scattered across various folders. That does make it a lot less clear which folders should be a VOLUME and which ones should not. There's really no need at all to mount folders with php files in the nginx docker container when using fpm.

==
edit; or did you mean this is a problem?

FROM php:7.1-fpm
RUN apt-get install........ && tar -xjf nextcloud.tar.bz2 -C /var/www/html

# Volumes for dynamic data
VOLUME /var/www/html/config
VOLUME /var/www/html/data
VOLUME /var/www/html/custom_apps
# volumes for template purposes or static files which are to be served by nginx in a fpm environment
VOLUME /var/www/html/vendor
VOLUME /var/www/html/css
VOLUME /var/www/html/static all files...

because when upgrading with this dockerfile, it will not overwrite the contents of /var/www/html/css into the volume?
It should be ok on first-run as the contents of the folder should be copied into the volume, but it doesn't happen if the volume already exists.
Is that what you meant?

tilosp · 2017-04-19T08:18:10Z

@ChessSpider no the Dockerfile places the source-code directly into `/usr/src/nextcloud'. A container will not overwrite the files in a volume when started, this is the reason why we copy them into the volume in the entry-point script.

ChessSpider · 2017-04-19T08:25:39Z

@tilosp yeah, but that's why #23 and others are suggesting to remove the volume on /var/www/html right?
Im also argueing to remove the too broad volume on /var/www/html, and replace it with a volume with a smaller scope: the directory/directories with static files. And only for the fpm image.
Are you saying that is not feasible and we need to have a broad volume on the entire /var/www/html directory?

tilosp · 2017-04-19T08:34:00Z

@ChessSpider i'am saying if we use volumes with a smaller scope, we would still have to update the content of the small scope volumes. And this would then again require us to copy the files into the volume. I'am perfectly fine with removing the volume declaration completely and let the user decide which scope they want to use.

ChessSpider · 2017-04-19T10:01:40Z

@tilosp Right, we're finally aligned :). You're correct about the static files. I think the VOLUME on /var/www/html needs to be removed either way. You want users to upgrade using PULL instead of using the nextcloud updater, and you want users to get the correct version when using the docker updater.

A different solution has to be figured out for the fpm use-case though. Maybe some kind of asset management is possible? Previously I'd say to re-run the asset pipelining feature, and put the VOLUME on the assets/ folder. But that's gone.

Do you have a suggestion? Is there a feasible way to concentrate static files in one directory, or is there a limited number of directories which contain static files?

Alternatively, we can provide clear instructions which previous volumes has to be removed and recreated in case you want to use it in a php-fpm setup. The neighbors at Wonderfall unfortunately don't have this problem as they run php-fpm alongside nginx inside the container https://github.com/Wonderfall/dockerfiles/blob/master/nextcloud/11.0/Dockerfile

pierreozoux · 2017-04-19T10:59:47Z

10.0/apache/docker-entrypoint.sh

-    tar cf - --one-file-system -C /usr/src/nextcloud . | tar xf -
+# version_greater A B returns whether A > B
+function version_greater() {
+	[[ "$(printf '%s\n' "$@" | sort -V | head -n 1)" != "$1" ]];


I just tried this on an alpine box, and the busybox sort is complaining about the -V...

I ended up using this:
https://github.com/pierreozoux/docker-alpine/blob/03573c088fc71bc9bd6a6e0b4af262935289621c/builder/scripts/apk-install#L4-L6

Yeah, that's because sort -V is a GNU coreutils thing. I really am not a fan of that script you linked, GNU's version checking is much more sophisticated. Are we basing our images on alpine? I feel like we shouldn't because of the other issues that Alpine container images have...

Actually, we can install coreutils in alpine too.
https://wiki.alpinelinux.org/wiki/How_to_get_regular_stuff_working

What are the other issues?

I really like alpine, and as free software advocates, I believe it is our mission to make this IT world a better place. Alpine is a lot more ecological, that's it.

cyphar · 2017-04-19T12:24:38Z

@ChessSpider @tilosp Removing VOLUME isn't going to help users who want to use Docker's volume drivers, it'll mean that they can't use any such drivers. While I understand the worry about the copying code, I really don't agree with the idea that we should make the default usage of our images be unable to extract the data in a sane way so that it is persistent. If you look at how other images work, (for example mysql) you'll notice that they use VOLUME for its intended purpose -- defining which parts of a container's filesystem should be persistent across restarts.

From my experience, splitting up the VOLUME causes nothing but headaches. Right now I have a NextCloud install that I need to purge with fire because of the issues I've had with upgrading it -- some but not all of the files get deleted properly (for example). It's a mess.

ChessSpider · 2017-04-19T12:31:03Z

@cyphar A user can always create a volume on startup with docker run -v , regardless of any VOLUME declaration in the Dockerfile? Removing all VOLUME declarations should not limit the user in any way.

The MySQL Dockerfile you reference only put a VOLUME on the /var/lib/mysql folder. That is equivalent of the data, custom_app and config directory of Nextcloud. Nextcloud should probably do that too.

tilosp · 2017-04-19T17:54:14Z

should be ready for merge

tilosp · 2017-04-22T09:25:32Z

@pierreozoux @cyphar can we get this merged before 11.0.3 is released?

pierreozoux

Looks good to me!

Thanks for the amazing work done on this.

If @tianon or @yosifkit could take a look too, would be amazing.

tianon · 2017-04-24T16:20:39Z

Looks generally OK to me, although I'd feel better if @yosifkit took a look as well.

My only question after the look I did was whether there are any technical reasons downgrading isn't supported ATM? (not thinking anything here should change, more curious whether there's some indicator that could possibly be used in the future for determining data compatibility when doing upgrades/downgrades so that this could potentially be expanded 👍)

yosifkit · 2017-04-24T17:25:47Z

Seems fine to me! Seems like we should be able to do something similar in the WordPress image. 😍

SnowMB · 2017-04-25T07:20:10Z

My only question after the look I did was whether there are any technical reasons downgrading isn't supported ATM? (not thinking anything here should change, more curious whether there's some indicator that could possibly be used in the future for determining data compatibility when doing upgrades/downgrades so that this could potentially be expanded 👍)

I think this would be something for the guys working on the server as this is not docker specific 😉

cyphar · 2017-04-26T04:54:34Z

/me will test this out soon once I figure out how much of my current data might be FUBAR.

- `docker`: experimental multiarch (docker-library/docker#52) - `golang`: experimental multiarch (docker-library/golang#158) - `nextcloud`: 10.0.5, 11.0.3, 9.0.58, update via container updates (nextcloud/docker#65) - `postgres`: ensure `postgres` user's HOME has decent permissions (docker-library/postgres#277) - `python`: fix `pip` install to also install `setuptools` and `wheel` (docker-library/python#186, docker-library/python#187)

tilosp mentioned this pull request Apr 14, 2017

Dockerfile: use correct VOLUMEs #23

Closed

mkuron reviewed Apr 15, 2017

View reviewed changes

tilosp requested review from pierreozoux and cyphar April 15, 2017 11:28

tilosp commented Apr 15, 2017

View reviewed changes

pierreozoux reviewed Apr 17, 2017

View reviewed changes

cyphar reviewed Apr 17, 2017

View reviewed changes

tilosp added 5 commits April 17, 2017 22:49

add version comparison

8f2bc6b

use rsync for upgrade

055f577

run occ upgrade

87c6513

show disabled apps

ae5b15c

split app folder

fc384e6

tilosp force-pushed the volume branch from fc810d3 to fc384e6 Compare April 17, 2017 21:25

tilosp self-assigned this Apr 17, 2017

disable the updater

07a1082

SnowMB reviewed Apr 18, 2017

View reviewed changes

SnowMB mentioned this pull request Apr 18, 2017

"This version of Nextcloud is not compatible with PHP 7.1" #40

Closed

more specific downgrade message

201d934

pierreozoux reviewed Apr 19, 2017

View reviewed changes

pierreozoux mentioned this pull request Apr 19, 2017

alpine php-fpm images #54

Closed

strong permissions

d253cc1

tilosp changed the title ~~WIP: Rework Updating~~ Rework Updating Apr 19, 2017

pierreozoux approved these changes Apr 22, 2017

View reviewed changes

tilosp merged commit 71199bd into master Apr 24, 2017

tilosp deleted the volume branch April 24, 2017 18:58

tilosp mentioned this pull request Apr 24, 2017

For big installations, chown of data folder is taking a looong time #26

Closed

SnowMB mentioned this pull request Apr 25, 2017

WIP: Rework Readme file #53

Merged

tianon mentioned this pull request Apr 26, 2017

Update docker-library images docker-library/official-images#2896

Merged

ChessSpider mentioned this pull request Jun 13, 2017

Make use of NEXTCLOUD_CONFIG_DIR #102

Closed

tilosp mentioned this pull request Jun 29, 2017

Ensure config/apps.config.php get copied #118

Merged

Rework Updating #65

Rework Updating #65

Conversation

tilosp commented Apr 14, 2017 • edited Loading

mkuron commented Apr 15, 2017

tilosp commented Apr 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pierreozoux left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyphar left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyphar Apr 17, 2017 • 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

tilosp commented Apr 17, 2017

SnowMB left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChessSpider commented Apr 18, 2017

tilosp commented Apr 18, 2017 • edited Loading

ChessSpider commented Apr 18, 2017 • edited Loading

tilosp commented Apr 18, 2017

ChessSpider commented Apr 18, 2017

ChessSpider commented Apr 18, 2017 • edited Loading

tilosp commented Apr 19, 2017

ChessSpider commented Apr 19, 2017 • edited Loading

tilosp commented Apr 19, 2017

ChessSpider commented Apr 19, 2017

tilosp commented Apr 19, 2017

ChessSpider commented Apr 19, 2017

pierreozoux Apr 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cyphar commented Apr 19, 2017

ChessSpider commented Apr 19, 2017

tilosp commented Apr 19, 2017

tilosp commented Apr 22, 2017

pierreozoux left a comment

Choose a reason for hiding this comment

tianon commented Apr 24, 2017

yosifkit commented Apr 24, 2017

SnowMB commented Apr 25, 2017

cyphar commented Apr 26, 2017

tilosp commented Apr 14, 2017 •

edited

Loading

tilosp commented Apr 15, 2017 •

edited

Loading

cyphar left a comment •

edited

Loading

cyphar Apr 17, 2017 •

edited

Loading

SnowMB left a comment •

edited

Loading

tilosp commented Apr 18, 2017 •

edited

Loading

ChessSpider commented Apr 18, 2017 •

edited

Loading

ChessSpider commented Apr 18, 2017 •

edited

Loading

ChessSpider commented Apr 19, 2017 •

edited

Loading

pierreozoux Apr 19, 2017 •

edited

Loading