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.

Sign up for GitHub

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

Jump to bottom

Update Docker for build #3879

Closed
wants to merge 5 commits into from
Closed

Update Docker for build #3879

wants to merge 5 commits into from

Conversation

homdx
Copy link
Contributor

@homdx homdx commented May 29, 2018

  • Fix Docker for build v7
  • Update version deps packages

This was referenced Jun 11, 2018
Closed
Closed
Closed
Closed
moneromooo-monero
moneromooo-monero reviewed Jun 20, 2018
View reviewed changes
Dockerfile Outdated
RUN set -ex \
&& curl -s -L -o boost_${BOOST_VERSION}.tar.bz2 https://dl.bintray.com/boostorg/release/${BOOST_VERSION_DOT}/source/boost_${BOOST_VERSION}.tar.bz2 \
&& echo "${BOOST_HASH} boost_${BOOST_VERSION}.tar.bz2" | sha256sum -c \
&& echo "${BOOST_HASH} boost_${BOOST_VERSION}.tar.bz2" | sha256sum -c \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Spurious whitespace changes (and these are actually needed apparently)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added space in next commit

Dockerfile Outdated
@@ -93,6 +109,8 @@ RUN set -ex \
&& make check \
&& make install



Copy link
Collaborator

Choose a reason for hiding this comment

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

Spurious

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed to enter in next commit

README.md
@@ -448,6 +448,9 @@ Then you can run make as usual.
# Get binaries
docker cp monero-android:/opt/android/monero/build/release/bin .




Copy link
Collaborator

Choose a reason for hiding this comment

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

Spurious

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in next commit

README.md Outdated
@@ -498,6 +501,10 @@ Installing a snap is very quick. Snaps are secure. They are isolated with all of
# or in background
docker run -it -d -v /monero/chain:/root/.bitmonero -v /monero/wallet:/wallet -p 18080:18080 monero

* need 3 Gb space
Copy link
Collaborator

Choose a reason for hiding this comment

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

The build needs 3 GB space.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in next commit

Dockerfile Outdated
@@ -1,7 +1,7 @@
# Multistage docker build, requires docker 17.05

# builder stage
FROM ubuntu:16.04 as builder
FROM debian:jessie as builder
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checked under Ubuntu. In next commit revert to 16.04

@homdx homdx changed the title Fix Docker for build v7 Update Docker for build Jun 20, 2018
homdx
homdx commented Jun 20, 2018
View reviewed changes
Copy link
Contributor Author

@homdx homdx left a comment

Choose a reason for hiding this comment

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

Ready for review. You can sea build this docker:

https://hub.docker.com/r/homdx/monero/builds/

moneromooo-monero
moneromooo-monero reviewed Jun 20, 2018
View reviewed changes
Dockerfile Outdated
RUN set -ex \
&& curl -s -O https://www.openssl.org/source/openssl-${OPENSSL_VERSION}.tar.gz \
&& echo "${OPENSSL_HASH} openssl-${OPENSSL_VERSION}.tar.gz" | sha256sum -c \
&& echo "${OPENSSL_HASH} openssl-${OPENSSL_VERSION}.tar.gz" | sha256sum -c \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Still here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added spaces

Dockerfile Outdated
ARG CMAKE_HASH=8f864e9f78917de3e1483e256270daabc4a321741592c5b36af028e72bff87f5
RUN set -ex \
&& curl -s -O https://cmake.org/files/${CMAKE_VERSION_DOT}/cmake-${CMAKE_VERSION}.tar.gz \
&& echo "${CMAKE_HASH} cmake-${CMAKE_VERSION}.tar.gz" | sha256sum -c \
Copy link
Collaborator

Choose a reason for hiding this comment

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

That one will also need two spaces to work with some tools.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please, write tools name

@moneromooo-monero
Copy link
Collaborator

There's just the remaining added lines without any other change, then please squash and it's good to go.

homdx
homdx commented Jun 21, 2018
View reviewed changes
Copy link
Contributor Author

@homdx homdx left a comment

Choose a reason for hiding this comment

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

You can check again

@moneromooo-monero
Copy link
Collaborator

They're still here, but whatever. Not super important at this point. Just rebase/squash and we're good.

homdx
homdx commented Jun 22, 2018
View reviewed changes
Copy link
Contributor Author

@homdx homdx left a comment

Choose a reason for hiding this comment

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

I think it's worth updating all the dependent components to stable versions. that I realized

@moneromooo-monero
Copy link
Collaborator

It would be nice if you could rebase/squash very soon, there'll be a 0.12.3.0 and this could go in if ready.

@homdx
Copy link
Contributor Author

homdx commented Jul 3, 2018

I w merge latest 3 commits in one

@moneromooo-monero
Copy link
Collaborator

There's a "Merge remote-tracking branch 'upstream/master' " in there. Rebaseing (rather than merging) should fix it.

@homdx
Copy link
Contributor Author

homdx commented Jul 3, 2018

I pull a remote branch master to my fork

@moneromooo-monero
Copy link
Collaborator

Even worse now, there are two.
If you can't get them out, an easier way is to create another branch, then cherry-pick the good commits. Then push -f to the PR branch.

@homdx
Copy link
Contributor Author

homdx commented Jul 3, 2018

Updated this
#4094

@moneromooo-monero
Copy link
Collaborator

Thanks, I approved the other, this can can be closed now.

@moneromooo-monero
Copy link
Collaborator

+duplicate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@moneromooo-monero moneromooo-monero moneromooo-monero left review comments

Assignees
No one assigned
Labels
duplicate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@homdx @moneromooo-monero @issue-helper