Update sysutils/docker-freebsd to somewhere circa v17.05.0

[lifanov]

Summary

Update to latest from https://github.com/freebsd-docker project.

Submitted by:	dteske

Test Plan

portlint - OK
build - OK
poudriere testport - OK
run - not tested yet

[lifanov]

list myself as maintainer per dteske

[lifanov]

s/post-install/do-install/ - it makes more sense this way

[lifanov]

include dteske's changes to the rc script and wrap them im prestart

I shuffled things around, so bugs are all mine.

[lifanov]

per dteske, it's fine to just use docker_dir

[dteske]

sysutils/docker-freebsd/files/docker.in

32–39

We should put "local" (without quotes) before each of these (non-blank) lines

40–48

We'll want a "local module" (w/o quotes) at the top of the function (first line after curly) to localize the variable used here in the for-loop

79–91

Let's reduce the indentation of these lines by a single tab

98

Let's remove this line -- it's not necessary unless using the dpv/cat pipe-through to redirect method

114–115

Since renaming docker_zfs_dir to docker_dir (reduction of 4 letters), is that enough to fit this all one one line that is 79 characters-or-less? (if no, retain line continuation)

[lifanov]

address some of dteske's feedback

[lifanov]

sysutils/docker-freebsd/files/docker.in

114–115

It still doesn't fit in 79 cols :(

[lifanov]

address dteske's feedback: localize "module" var

[dteske]

sysutils/docker-freebsd/files/docker.in

32–39

Initialization to NULL is required for all but module and for purposes of minimal diffs, prefer making these separate statements

[tab]local module
[tab]local fs_type=
[tab]local make_zfs=
[tab]local zfs_mountpoint=
[tab]local zfs_name=
[tab]local zpool_name=
[tab]local zpool_size=

40–48

Preferred for minimal diffs:

[tab]for module in \
[tab][tab]zfs \
[tab][tab]pf \
[tab][tab]linux \
[tab][tab]linux64 \
[tab]; do

114–115

Thanks for checking ;D

140

There should be a space after > and 2> but more importantly, this is preferred:

[tab][tab]pgrep -F /var/run/docker.pid > /dev/null 2>&1

140–144

Preferred:

[tab][tab]if pgrep -F /var/run/docker.pid > /dev/null 2>&1; then
[tab][tab][tab]echo "Docker already running? /var/run/docker.pid"
[tab][tab][tab]exit 1
[tab][tab]fi

[lifanov]

sysutils/docker-freebsd/files/docker.in

40–48

You don't need a semicolon here if do is on the next line.

140

This isn't portable, but our sh supports it.

[lifanov]

address dteske's feedback

[mat]

sysutils/docker-freebsd/Makefile

5–6

Why add the g here that forces you to bump PORTEPOCH ?
This is a bad idea, remove the g.

22

Use USE_GITHUB=yes.

23

default value, remove.

25–35

Remove this from the GH_TUPLE and add GH_SUBDIR=src/github.com/docker/docker instead.

[lifanov]

sysutils/docker-freebsd/Makefile

5–6

This is going to become 17.05.0 before this patch lands, which will involve a PORTEPOCH bump.

[mat]

sysutils/docker-freebsd/Makefile

5–6

Well, when this becomes 17.05.0, then PORTEPOCH will be needed, but right now, with a version of 20170907, there is no need to do it.

[lifanov]

address mat's feedback: clean up USE_GITHUB with default project

[dteske]

sysutils/docker-freebsd/files/docker.in

40–48

The reason why you should use:

[tab][tab]linux64 \
[tab]; do

Instead of:

[tab][tab]linux64
[tab]do

Is because list management (e.g., sorting with sort, moving items manually for ordered operation, etc.) is possible when all the elements have a trailing backslash. By making the last element not have a trailing backslash, a future diff that appends a new item to the end will not just be a diff with a single "+" line but instead a "-" and "+" pair where the backslash had to be added to the now-previous last-line in the list.

This also prevents mistakes.

140

What isn't portable? Space after the redirect operator? The duplication of fd1 to fd2 (to send stderr the same place as stdout)? Ordered file descriptor operations? All of those things are portable and no system I have touched in 20 years of using UNIX has ever balked at any of these. I would be shocked if you could find any variant of shell that balks at any of these (note: csh/tcsh/zsh don't count; things like ksh, dash, ash, do count -- it has to be a bourne shell variant, bash included since CentOS, Mac OS X, etc. use bash as /bin/sh).

[lifanov]

address mat's feedback: un-bump epoch until we get a real version
address dteske's feedback: make for list sortable by including a semicolon

[mat]

sysutils/docker-freebsd/Makefile

20

Remove, default value.

sysutils/docker-freebsd/files/docker.in

114–115

The 80's called, they're telling me they had wider terminals back then and we have not had that limitations since before FreeBSD was born.

[dteske]

sysutils/docker-freebsd/files/docker.in

114–115

Religion called, they want their hate back.

[dteske]

sysutils/docker-freebsd/Makefile

20

The port is named docker-freebsd, so wouldn't that mean the default value would be backward? The GitHub account is freebsd-docker, not docker-freebsd.

[dteske]

sysutils/docker-freebsd/files/docker.in

139–140

Been thinking a lot that we should make the pidfile able to be set in rc.conf(5). E.g., docker_pidfile

147

$docker_pidfile instead of hard-coded value repeated throughout

152

$docker_pidfile instead of hard-coded value repeated throughout

154

$docker_pidfile instead of hard-coded value repeated throughout

[lifanov]

address dteske's feedback: un-hardcode pid file

[mat]

sysutils/docker-freebsd/Makefile

20

Oh, yes, my brain rearranged the worlds in the same order, sorry about it.

[dteske]

Great work! (can't think of anything else other than known pending issues e.g., PORT_EPOCH and version which is blocked by need to generate a tag; getting this far we can totally leverage this for runtime testing to get to a tag-point where we are happy with the runtime performance).

[lifanov]

Cool beans! Once the new version is functional enough to replace existing 1.7.0, I'll open a PR and attempt to land this.