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

add docker config #166

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yancyribbens
Copy link

Add docker setup option for installing locally

Copy link
Contributor

@bitschmidty bitschmidty left a comment

Choose a reason for hiding this comment

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

Concept ACK. Couple comments. Thanks much @yancyribbens , very cool.

Built, ran, and tested a few notebooks without issue. Im not a docker expert so cannot evaluate the docker file structure other than at a rudimentary level, but it worked right away for me!

config.ini Outdated
@@ -3,4 +3,4 @@
[path]
# Set this to the source directory for your Optech Taproot Bitcoin Core
# eg SOURCE_DIRECTORY=/Users/Optech/bitcoin
SOURCE_DIRECTORY=
Copy link
Contributor

Choose a reason for hiding this comment

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

Will want to keep this empty as default. Perhaps a note in the comment about this config for the docker setup?

Copy link
Author

Choose a reason for hiding this comment

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

@bitschmidty I used a regex in the dockerfile to make this change during build:
RUN sed -i.bak 's\^SOURCE_DIRECTORY=\SOURCE_DIRECTORY=/optech/bitcoin\g' config.ini

Dockerfile Outdated
RUN ../dist/configure --disable-shared --enable-cxx --with-pic --prefix=$BDB_PREFIX
RUN make install

# install bitcoin 0.16.3
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps misleading since we are on the Optech Taproot_V0.1.4 branch

Copy link
Author

Choose a reason for hiding this comment

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

fixed

@yancyribbens yancyribbens force-pushed the add-docker-file branch 2 times, most recently from 1254b86 to 74aec8a Compare February 12, 2020 18:17
@yancyribbens
Copy link
Author

@bitschmidty updated based on your feedback. I also changed the maintainer label since I may not be able to provide ongoing support and maintenance.

Copy link
Contributor

@bitschmidty bitschmidty left a comment

Choose a reason for hiding this comment

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

ACK

Rebuilt, ran, and smoke tested a couple notebooks

@jnewbery
Copy link
Contributor

Concept ACK. I plan to review this soon. Thanks for the contribution, @yancyribbens!

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