updates to tools/docker-format/Dockerfile : container builds again - #449
updates to tools/docker-format/Dockerfile : container builds again#449alichnewsky wants to merge 1 commit into
Conversation
Sorry, something went wrong.
| @@ -1,8 +1,28 @@ | |||
| FROM ubuntu:cosmic | |||
|
|
|||
| # repositories are obsolete. the image need to be changed. | |||
There was a problem hiding this comment.
If we update to something newer than cosmic, do we also get golang>=1.12?
Sorry, something went wrong.
There was a problem hiding this comment.
Upgrading from cosmic to focal ( 20.04 LTS ) gives you exactly golang 1.13.
It requires python3 but the same cmake-format package version works.
The default clang-format is 10.0 and needs to be downgraded not to have side effects on the current c++ code.
On other hand, installing golang manually makes the upgrade path clear should buildifier requirements change again.
currently in this PR
docker run -v $PWD:/opencensus-cpp -it opencensus-cpp/format:cosmic /opencensus-cpp/tools/docker-format/run.sh clang-format version 7.0.0-3 (tags/RELEASE_700/final) Running buildifier. Running cmake-format 0.6.10.
potential "focal" version
docker run -v $PWD:/opencensus-cpp -it opencensus-cpp/format:focal /opencensus-cpp/tools/docker-format/run.sh clang-format version 7.0.1-12 (tags/RELEASE_701/final) Running buildifier. Running cmake-format 0.6.10.
I can submit the latter if you'd rather.
Sorry, something went wrong.
There was a problem hiding this comment.
-
Is the "focal" version simpler?
-
Instead of installing golang in "cosmic" is it easier to just install a release of buildifier? e.g. https://github.com/bazelbuild/buildtools/releases/download/3.3.0/buildifier
Sorry, something went wrong.
| # brings golang to 1.10 by default.bazelbuilds wants 1.12 minimum.... | ||
| #RUN apt install -y golang | ||
|
|
||
| RUN apt install -y curl |
There was a problem hiding this comment.
Please merge all of the RUNs together into a single one to avoid a long chain of docker overlays.
Sorry, something went wrong.
lgruen
left a comment
There was a problem hiding this comment.
Sorry, I'm not working on OpenCensus anymore, but can't remove myself as a reviewer from this PR. Dismissing this review using "request changes" might remove it from my PR queue though.
Sorry, something went wrong.
Reviewers
At least 1 approving review is required to merge this pull request.
current docker file fails to build. Should be linked to issue #448
As a result, can't run pre-commit formatting before PR fail in CI builds.
to validate rebuild the container as per instructions in the README.md file.