Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
.DS_Store
build
./build
21 changes: 13 additions & 8 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@
# github.com/docker/cli
#

# build the CLI
.PHONY: build
build: clean
@./scripts/build/binary
all: binary

# remove build artifacts
.PHONY: clean
Expand All @@ -18,24 +15,32 @@ clean:
test:
@go test -tags daemon -v $(shell go list ./... | grep -v /vendor/)

# run linters
.PHONY: lint
lint:
@gometalinter --config gometalinter.json ./...


.PHONY: binary
binary:
@./scripts/build/binary

# build the CLI for multiple architectures
.PHONY: cross
cross: clean
cross:
@./scripts/build/cross

.PHONY: dynbinary
dynbinary:
@./scripts/build/dynbinary

# download dependencies (vendor/) listed in vendor.conf
.PHONY: vendor
vendor: vendor.conf
@vndr 2> /dev/null
@script/validate/check-git-diff vendor
@scripts/validate/check-git-diff vendor

cli/compose/schema/bindata.go: cli/compose/schema/data/*.json
go generate github.com/docker/cli/cli/compose/schema

compose-jsonschema: cli/compose/schema/bindata.go
@script/validate/check-git-diff cli/compose/schema/bindata.go
@scripts/validate/check-git-diff cli/compose/schema/bindata.go
44 changes: 13 additions & 31 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,60 +6,42 @@ docker/cli
This repository is the home of the cli used in the Docker CE and
Docker EE products.

It's composed of 3 main folders

* `/cli` - all the commands code.
* `/cmd/docker` - the entrypoint of the cli, aka the main.

Development
===========

### Build locally
`docker/cli` is developed using Docker.

```
$ make build
```
Build a linux binary:

```
$ make clean
$ make -f docker.Makefile binary
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If building with docker is the default as specified in the beginning of that section, then I would make docker.Makefile the default Makefile and the in container one the secondary one.

i.e.: as an user I just want to do make, make binary or make cross directly. This is the most common behavior and doesn't force someone to have to read the README if they just want the binary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I understand that.

My concern is that docker.Makefile is not really a good use of make. Every target is effectively .PHONY. It is not really taking advantage of any of what Make provides. It could just as easily be a bash script, or ideally a tool that is actually designed for this use case.

We should be looking to remove docker.Makefile and replace it with something else, which is why it's named this way. We could very easily drop in a dobi.yaml for this purpose and it would provide a lot of additional benefits.

It should also (eventually) be merged with what is now circle.yml so we don't have a separate copy for CI tasks. This is not really possible with circleci, just yet.

We could, for now, create a script that wraps this make -f docker.Makefile so the instructions would be something like ./task binary ./task lint, etc. This would give us a shim to transition to new build tools and would probably be a lot more concise when what we have in docker.Makefile right now.

Since this PR provides some necessary support for building cross, I'd like to get this merged first and address that in a separate PR.

WDTY?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is blocking quite a few things, I'm fine with merging it as it is.

But I'm afraid we'll end up breaking people relying on the cli downstream if we don't update quickly afterwards.

make is the standard, but I agree it is ill fitted to deal with output depending on docker images (depending on the Dockerfiles themselves would be an improvement though).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

I agree make is pretty standard for some languages, but I don't see it widely used by many golang projects. If it is a standard it's not very widely adopted. I think there are a lot more issues with make beyond just handling docker images, but Ill leave that for another thread.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be fine with having a build.sh script instead. Not for this PR, just my 2¢.

```

You will need [gox](https://github.com/mitchellh/gox) for this one:

```
$ make cross
```

If you don't have [gox](https://github.com/mitchellh/gox), you can use the "in-container" version of `make cross`, listed below.

### Build inside container
Build binaries for all supported platforms:

```
$ make -f docker.Makefile build
$ make -f docker.Makefile cross
```

```
$ make -f docker.Makefile clean
```
Run all linting:

```
$ make -f docker.Makefile cross
$ make -f docker.Makefile lint
```
`

### In-container development environment

```
$ make -f docker.Makefile dev
```

Then you can use the [build locally](#build-locally) commands:
Start an interactive development environment:

```
$ make build
$ make -f docker.Makefile shell
```

In the development environment you can run many tasks, including build binaries:

```
$ make clean
$ make binary
```

Legal
Expand Down
16 changes: 12 additions & 4 deletions circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,34 @@ jobs:
name: "Lint"
command: |
if [ "$CIRCLE_NODE_INDEX" != "0" ]; then exit; fi
docker build -f dockerfiles/Dockerfile.lint --tag cli-linter .
dockerfile=dockerfiles/Dockerfile.lint
echo "COPY . ." >> $dockerfile
docker build -f $dockerfile --tag cli-linter .
docker run cli-linter
- run:
name: "Cross"
command: |
if [ "$CIRCLE_NODE_INDEX" != "1" ]; then exit; fi
docker build -f dockerfiles/Dockerfile.ci --tag cli-builder .
dockerfile=dockerfiles/Dockerfile.cross
echo "COPY . ." >> $dockerfile
docker build -f $dockerfile --tag cli-builder .
docker run --name cross cli-builder make cross
docker cp cross:/go/src/github.com/docker/cli/build /work/build
- run:
name: "Unit Test"
command: |
if [ "$CIRCLE_NODE_INDEX" != "2" ]; then exit; fi
docker build -f dockerfiles/Dockerfile.ci --tag cli-builder .
dockerfile=dockerfiles/Dockerfile.dev
echo "COPY . ." >> $dockerfile
docker build -f $dockerfile --tag cli-builder .
docker run cli-builder make test
- run:
name: "Validate Vendor and Code Generation"
command: |
if [ "$CIRCLE_NODE_INDEX" != "3" ]; then exit; fi
docker build -f dockerfiles/Dockerfile.ci --tag cli-builder .
dockerfile=dockerfiles/Dockerfile.dev
echo "COPY . ." >> $dockerfile
docker build -f $dockerfile --tag cli-builder .
docker run cli-builder make -B vendor compose-jsonschema

- store_artifacts:
Expand Down
26 changes: 19 additions & 7 deletions docker.Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,30 @@

DEV_DOCKER_IMAGE_NAME = docker-cli-dev
LINTER_IMAGE_NAME = docker-cli-lint
CROSS_IMAGE_NAME = docker-cli-cross
MOUNTS = -v `pwd`:/go/src/github.com/docker/cli

# build docker image (dockerfiles/Dockerfile.build)
.PHONY: build_docker_image
build_docker_image:
@docker build -q -t $(DEV_DOCKER_IMAGE_NAME) -f ./dockerfiles/Dockerfile.build .
@docker build -t $(DEV_DOCKER_IMAGE_NAME) -f ./dockerfiles/Dockerfile.dev .

# build docker image having the linting tools (dockerfiles/Dockerfile.lint)
.PHONY: build_linter_image
build_linter_image:
@docker build -q -t $(LINTER_IMAGE_NAME) -f ./dockerfiles/Dockerfile.lint .
@docker build -t $(LINTER_IMAGE_NAME) -f ./dockerfiles/Dockerfile.lint .

.PHONY: build_cross_image
build_cross_image:
@docker build -t $(CROSS_IMAGE_NAME) -f ./dockerfiles/Dockerfile.cross .


# build executable using a container
.PHONY: build
build: build_docker_image
binary: build_docker_image
@echo "WARNING: this will drop a Linux executable on your host (not a macOS or Windows one)"
@docker run --rm $(MOUNTS) $(DEV_DOCKER_IMAGE_NAME) make build
@docker run --rm $(MOUNTS) $(DEV_DOCKER_IMAGE_NAME) make binary

build: binary

# clean build artifacts using a container
.PHONY: clean
Expand All @@ -36,14 +43,16 @@ test: build_docker_image

# build the CLI for multiple architectures using a container
.PHONY: cross
cross: build_docker_image
@docker run --rm $(MOUNTS) $(DEV_DOCKER_IMAGE_NAME) make cross
cross: build_cross_image
@docker run --rm $(MOUNTS) $(CROSS_IMAGE_NAME) make cross

# start container in interactive mode for in-container development
.PHONY: dev
dev: build_docker_image
@docker run -ti $(MOUNTS) -v /var/run/docker.sock:/var/run/docker.sock $(DEV_DOCKER_IMAGE_NAME) ash

shell: dev

# run linters in a container
.PHONY: lint
lint: build_linter_image
Expand All @@ -53,3 +62,6 @@ lint: build_linter_image
.PHONY: vendor
vendor: build_docker_image vendor.conf
@docker run -ti --rm $(MOUNTS) $(DEV_DOCKER_IMAGE_NAME) make vendor

dynbinary: build_cross_image
@docker run -ti --rm $(MOUNTS) $(CROSS_IMAGE_NAME) make dynbinary
29 changes: 0 additions & 29 deletions dockerfiles/Dockerfile.ci

This file was deleted.

22 changes: 22 additions & 0 deletions dockerfiles/Dockerfile.cross
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@

FROM golang:1.8.1

# allow replacing httpredir or deb mirror
ARG APT_MIRROR=deb.debian.org
RUN sed -ri "s/(httpredir|deb).debian.org/$APT_MIRROR/g" /etc/apt/sources.list

RUN apt-get update -qq && apt-get install -y -q \
libltdl-dev \
gcc-mingw-w64 \
parallel \
;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ; necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's necessary because of the \ on the previous line. I like this style because it lets you add items to the list, and keep it sorted, without worrying about the special case of no \ on the last line.

It's a minor thing though, and I'm fine to change it if you prefer I drop the \ and ;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷‍♂️ , I'm more used to the other way around, I was just wondering if this was adding any side effect I wasn't aware off.


RUN go get github.com/mitchellh/gox && \
cp /go/bin/gox /usr/bin && \
rm -rf /go/src/* /go/pkg/* /go/bin/*

COPY dockerfiles/osx-cross.sh /tmp/
RUN /tmp/osx-cross.sh
ENV PATH /osxcross/target/bin:$PATH

WORKDIR /go/src/github.com/docker/cli
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

FROM golang:1.8-alpine

RUN apk add -U git make
RUN apk add -U git make bash coreutils

RUN go get github.com/LK4D4/vndr && \
cp /go/bin/vndr /usr/bin && \
Expand Down
1 change: 0 additions & 1 deletion dockerfiles/Dockerfile.lint
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,5 @@ RUN go get -u gopkg.in/alecthomas/gometalinter.v1 && \
gometalinter --install

WORKDIR /go/src/github.com/docker/cli
COPY . .
ENTRYPOINT ["/usr/local/bin/gometalinter"]
CMD ["--config=gometalinter.json", "./..."]
29 changes: 29 additions & 0 deletions dockerfiles/osx-cross.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/usr/bin/env bash
#
# Install dependencies required to cross compile osx, then cleanup
#
# TODO: this should be a separate build stage when CI supports it


set -eu -o pipefail

PKG_DEPS="patch xz-utils clang"

apt-get update -qq
apt-get install -y -q $PKG_DEPS

OSX_SDK=MacOSX10.11.sdk
OSX_CROSS_COMMIT=a9317c18a3a457ca0a657f08cc4d0d43c6cf8953
OSXCROSS_PATH="/osxcross"

echo "Cloning osxcross"
time git clone https://github.com/tpoechtrager/osxcross.git $OSXCROSS_PATH
cd $OSXCROSS_PATH
git checkout -q $OSX_CROSS_COMMIT

echo "Downloading OSX SDK"
time curl -sSL https://s3.dockerproject.org/darwin/v2/${OSX_SDK}.tar.xz \
-o "${OSXCROSS_PATH}/tarballs/${OSX_SDK}.tar.xz"

echo "Buidling osxcross"
UNATTENDED=yes OSX_VERSION_MIN=10.6 ${OSXCROSS_PATH}/build.sh > /dev/null
18 changes: 18 additions & 0 deletions scripts/build/.variables
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/usr/bin/env bash

set -eu

VERSION=${VERSION:-"unknown-version"}
GITCOMMIT=${GITCOMMIT:-$(git rev-parse --short HEAD 2> /dev/null || true)}
BUILDTIME=${BUILDTIME:-$(date --utc --rfc-3339 ns 2> /dev/null | sed -e 's/ /T/')}

export LDFLAGS="\
-w \
-X github.com/docker/cli/cli.GitCommit=${GITCOMMIT} \
-X github.com/docker/cli/cli.BuildTime=${BUILDTIME} \
-X github.com/docker/cli/cli.Version=${VERSION} \
${LDFLAGS:-} \
"

export TARGET="build/docker-$(go env GOHOSTOS)-$(go env GOHOSTARCH)"
export SOURCE="github.com/docker/cli/cmd/docker"
15 changes: 12 additions & 3 deletions scripts/build/binary
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
#!/usr/bin/env sh
#!/usr/bin/env bash
#
# Build a static binary for the host OS/ARCH
#

source ./scripts/build/ldflags
set -eu -o pipefail

go build -o ./build/docker --ldflags "${LDFLAGS}" github.com/docker/cli/cmd/docker
source ./scripts/build/.variables

echo "Building statically linked $TARGET"
export CGO_ENABLED=0
go build -o "${TARGET}" --ldflags "${LDFLAGS}" "${SOURCE}"

ln -sf "$(basename ${TARGET})" build/docker
16 changes: 10 additions & 6 deletions scripts/build/cross
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
#!/usr/bin/env sh
#!/usr/bin/env bash

source ./scripts/build/ldflags
set -eu -o pipefail

gox -output build/docker-{{.OS}}-{{.Arch}} \
-osarch="linux/arm linux/amd64 darwin/amd64 windows/amd64" \
--ldflags "${LDFLAGS}" \
github.com/docker/cli/cmd/docker
export BUILDDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

echo "Building all binaries"
SHELL=/bin/bash parallel ::: \
"$BUILDDIR/linux-cross" \
"$BUILDDIR/windows" \
"$BUILDDIR/osx" \
;
14 changes: 14 additions & 0 deletions scripts/build/dynbinary
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/usr/bin/env bash
#
# Build a dynamically linked binary for the host OS/ARCH
#

set -eu -o pipefail

source ./scripts/build/.variables

echo "Building dynamically linked $TARGET"
export CGO_ENABLED=1
go build -o "${TARGET}" -tags pkcs11 --ldflags "${LDFLAGS}" "${SOURCE}"

ln -sf "$(basename ${TARGET})" build/docker
Loading