Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docker: fix build, upgrade Go #1516

Merged
merged 1 commit into from
Apr 8, 2020
Merged

docker: fix build, upgrade Go #1516

merged 1 commit into from
Apr 8, 2020

Conversation

jsign
Copy link
Contributor

@jsign jsign commented Apr 6, 2020

The basic-miner-busybox Dockerfile is currently broken because of two issues.

Issue 1: missing jq dep now required to build extern/filecoin-ffi:

touch build/.update-modules
make -C extern/filecoin-ffi/ .install-filcrypto
make[1]: Entering directory '/lotus/extern/filecoin-ffi'
./install-filcrypto
+ auth_header=()
+ '[' -n '' ']'
++ dirname ./install-filcrypto
+ cd .
+ rust_sources_dir=rust
++ jq -r '.[].rustc_target_feature'
./install-filcrypto: line 23: jq: command not found
+ optimized_release_rustc_target_features=
make[1]: Leaving directory '/lotus/extern/filecoin-ffi'
make[1]: *** [Makefile:11: .install-filcrypto] Error 127
make: *** [Makefile:28: build/.filecoin-install] Error 2
The command '/bin/sh -c cd $SRC_DIR   && mkdir $SRC_DIR/build   && . $HOME/.cargo/env   && make clean   && make deps' returned a non-zero code: 2

Isse 2: failing extern/filecoin-ffi because of a “missing” C header file:

---> 90009b27540b
Step 18/40 : RUN cd $SRC_DIR   && . $HOME/.cargo/env   && make $MAKE_TARGET
 ---> Running in 704f07386d7e
rm -f lotus
go build -ldflags=-X="github.com/filecoin-project/lotus/build".CurrentCommit="+gitc9dae720.dirty" -o lotus ./cmd/lotus
# github.com/filecoin-project/filecoin-ffi/generated
extern/filecoin-ffi/generated/cgo_helpers.go:11:10: fatal error: cgo_helpers.h: No such file or directory
 #include "cgo_helpers.h"
          ^~~~~~~~~~~~~~~
compilation terminated.

This PR fixes both things making the Dockerfile build successfully.

Signed-off-by: Ignacio Hagopian <jsign.uy@gmail.com>
@jsign
Copy link
Contributor Author

jsign commented Apr 7, 2020

Uhm, realized now that .dockerignore is a symlink to .gitignore... my original edit while doing the PR was editing .dockerignore, not .gitignore.

I don't like that symlink, since this change to .gitignore it isn't good. Also, might be a bit dangerous to think about doing a change on one file and really modify another one in a codebase. (Like happened now)

Maybe avoid the symlink? Or find other solution that still ignores .h but lets you build the Dockefile?
Opinions?

Copy link
Contributor

@placer14 placer14 left a comment

Choose a reason for hiding this comment

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

I proved the error existed and then proved the fix removed the error as described. Some further discussion about other improvement to be consider will follow in a separate PR.

Specifically:

  • decouple .dockerignore from .gitignore
  • optimize Dockerfile for caching and reduced git noise
  • internalize extern depednencies within the docker image instead of COPYing from host

MAINTAINER ldoublewood <ldoublewood@gmail.com>

ENV SRC_DIR /lotus

RUN apt-get update && apt-get install -y && apt-get install -y ca-certificates llvm clang mesa-opencl-icd ocl-icd-opencl-dev
RUN apt-get update && apt-get install -y ca-certificates llvm clang mesa-opencl-icd ocl-icd-opencl-dev jq
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch on the duplicated install line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants