Add support for macOS and allow building stacks-core from commit hash#39
Add support for macOS and allow building stacks-core from commit hash#39radu-stacks wants to merge 4 commits into
Conversation
Co-authored-by: Radu Bahmata <92028479+BowTiedRadone@users.noreply.github.com>
207ea2f to
b78fbfb
Compare
|
The @ASuciuX email address was mapping to @BowTiedDeployer which didn't sign the CLA. To allow this PR to be merged I rebased the commits, replacing the email address with the no-reply one and signed with my signature. Hope that's ok @wileyj. |
| RUN git clone --branch $STACKS_CORE_BASE_BRANCH --single-branch --depth=1 https://github.com/stacks-network/stacks-core.git /code/stacks-core | ||
| WORKDIR /code/stacks-core | ||
| RUN apt-get update && apt-get install -y git libclang-dev llvm | ||
| RUN apt-get update && apt-get install -y libclang-dev llvm |
There was a problem hiding this comment.
this ordering is weird (and is present in the default branch, so i'll take ownership for this lol).
i think we should move the apt-get commands to the top of the file before starting the git clone/init. i also think we should keep the git package install. it may be present in the current base image rust:bookworm, but there is no promise that it will always be present there.
| - &REWARD_RECIPIENT_2 ${REWARD_RECIPIENT_2:-ST2FW15NGB4H76FMVXKHYYSM865YVS6V3SA1GNABC} # priv: fe3087801196d8027008146b13e6d365920c2e4b7bc9969729ec2f0f22ef74fc01 | ||
| - &REWARD_RECIPIENT_3 ${REWARD_RECIPIENT_3:-ST2MES40ZEXTX9M4YXW9QSWHRVC9HYT419S198VPM} # priv: ed7eb063c61b8e892987228f1fcfb74eab5009568861613dc4b074b708a7893701 | ||
| - &STACKS_CORE_BASE_BRANCH ${STACKS_CORE_BASE_BRANCH:-3.4.0.0.1} | ||
| - &STACKS_CORE_BASE_BRANCH ${STACKS_CORE_BASE_BRANCH:-3.4.0.0.1} # branch, tag, or commit SHA |
There was a problem hiding this comment.
| - &STACKS_CORE_BASE_BRANCH ${STACKS_CORE_BASE_BRANCH:-3.4.0.0.1} # branch, tag, or commit SHA | |
| - &STACKS_CORE_BASE_BRANCH ${STACKS_CORE_BASE_BRANCH:-3.4.0.0.2} # branch, tag, or commit SHA |
bump to latest version
| # restores archive ownership and leaves bind-mount sources unwritable. | ||
| # Extract as the current user so everything lands user-owned. | ||
| TAR_EXTRACT := tar -xf | ||
| else |
There was a problem hiding this comment.
this should be sufficient, but we should add a disclaimer in the README that this Makefile will only work on linux/macos. if someone were to run this on windows for example, this else section would fail.
i think it's ok, but better to document that only linux/macos are supported here.
| $(CHAINSTATE_DIR): | ||
| @if [ ! -d "$(CHAINSTATE_DIR)" ]; then \ | ||
| mkdir -p $(CHAINSTATE_DIR); \ | ||
| if [ "$(TARGET)" = "up" ]; then \ | ||
| if [ -f "$(CHAINSTATE_ARCHIVE)" ]; then \ | ||
| $(TAR_EXTRACT) $(CHAINSTATE_ARCHIVE) -C $(CHAINSTATE_DIR) || exit 1; \ | ||
| else \ | ||
| echo "Chainstate archive ($(CHAINSTATE_ARCHIVE)) not found. Exiting"; \ | ||
| rm -rf $(CHAINSTATE_DIR); \ | ||
| exit 1; \ | ||
| fi; \ | ||
| fi; \ |
There was a problem hiding this comment.
can this section be changed to conform to the original syntax? requiring an escape on each line of the conditional is not ideal.
| # Boot the network from a local chainstate archive | ||
| up: check-not-running | build $(CHAINSTATE_DIR) | ||
| @echo "Starting $(PROJECT) network from chainstate archive" | ||
| @echo " OS: $(OS)" |
There was a problem hiding this comment.
nit, but i would use UNAME_S here since it's the data directly from the system vs a hardcoded value
| docker compose -f docker/docker-compose.yml --profile default -p $(PROJECT) down | ||
| @if [ -f .current-chainstate-dir ]; then \ | ||
| rm -f .current-chainstate-dir | ||
| rm -f .current-chainstate-dir; \ |
There was a problem hiding this comment.
same comment as before- is there a compelling reason to keep the ; \ chars here? does macos make handle the existing syntax differently?
adding these is not wrong necessarily, but it does add maintenance overhead and errors won't be apparent (e.g. you make a change and forget to add the ; \ where needed in the proposed syntax change.
| docker compose -f docker/docker-compose.yml --profile default -p $(PROJECT) down | ||
| @if [ -f .current-chainstate-dir ]; then \ | ||
| rm -f .current-chainstate-dir | ||
| rm -f .current-chainstate-dir; \ |
There was a problem hiding this comment.
to reduce spam - assume this applies to all other similar changes, as i see several more
i see @ASuciuX is credited as co-author so it's perfectly fine |
Continuation of #29 which became stale.