Skip to content

Conversation

@ltfschoen
Copy link
Owner

@ltfschoen ltfschoen commented Jan 5, 2023

  • You added a brief description of the PR, e.g.:
  • followed these instructions https://stackoverflow.com/a/57774684/3208553 to create a custom ./docker/substrate_builder.Dockerfile.dockerignore that is dedicated to ./docker/substrate_builder.Dockerfile with fallback to ./.dockerignore

  • checked current disk usage:

$ du -hc --max-depth=1 .[^.]* * | grep [MG]
217M	.git/objects
218M	.git
16K	        .github/ISSUE_TEMPLATE
15G	        bin/node-template
4.6M	bin/node
5.4M	bin
7.6M	client/executor
1.2M	client/network
14M	        client
1.2M	frame/contracts
3.3M	frame/support
13M	        frame
4.0K	        HEADER-GPL3
36K	        LICENSE-GPL3
3.9M	primitives
4.0K	        README.md
2.8M	zombienet/0001-basic-warp-sync
2.8M	zombienet
  • added contents of .dockerignore to ./docker/substrate_builder.Dockerfile.dockerignore,
    then added extra folders like .git/ to it too

  • used a different approach to obtaining the PROJECT_ROOT that didn't involve using git command based on this stackoverflow suggestion https://stackoverflow.com/a/57774684/3208553

  • built the image

cd docker
./build.sh
  • inspect the size of the image
$ docker images

parity/substrate   latest    31c200b06ab8   15 minutes ago   277MB
parity/substrate   v3.0.0    31c200b06ab8   15 minutes ago   277MB

$ docker run --rm -it parity/substrate substrate --version
substrate 3.0.0-dev-unknown

$ docker run --rm -it parity/substrate subkey --version
subkey 2.0.2

$ docker run --rm -it parity/substrate node-template --version
node-template 4.0.0-dev-unknown

$ docker run --rm -it parity/substrate chain-spec-builder --help
<it worked and output help stuff>
  • You mentioned a related issue if this PR related to it, e.g. Fixes #228 or Related #1337.
  • You asked any particular reviewers to review. If you aren't sure, start with GH suggestions.
    TODO
  • Your PR adheres the style guide
    • In particular, mind the maximal line length.
    • There is no commented code checked in unless necessary.
    • Any panickers have a proof or removed.

@ltfschoen ltfschoen force-pushed the luke/2464-optimise-docker branch from 015cd59 to 0221536 Compare January 5, 2023 19:43
@ggwpez
Copy link

ggwpez commented Jan 5, 2023

Uhm why is this MR in your repo?

@ltfschoen
Copy link
Owner Author

Uhm why is this MR in your repo?

sorry i should have tagged reviewers later... this is still a WIP with a draft PR description...

because it's based on branch 'luke/13071-fix-ldd-error', which is only a branch in ltfschoen/substrate, and is still an active PR into 'master' of paritytech/substrate, so if i try to create a dedicated PR into paritytech/substrate 'master' from ltfschoen/substrate branch 'luke/2464-optimise-docker' then it shows changes from all commits from both branch 'luke/13071-fix-ldd-error' and 'luke/2464-optimise-docker' combined (instead of only showing 'luke/2464-optimise-docker' changes), but if 'luke/13071-fix-ldd-error' gets merged into 'master' of paritytech/substrate, then i can create a PR directly into 'master' of paritytech/substrate and it'll only show the 'luke/2464-optimise-docker' changes.

so my plan was to wait until 'luke/13071-fix-ldd-error' was merged into 'master' of paritytech/substrate, and then create a PR from branch 'luke/2464-optimise-docker' of ltfschoen/substrate into 'master' of paritytech/substrate, but in the interim while its still in my repo i wrote a draft PR description.... in hindsight i probably should have added tags for reviewers later on when i'd actually created the PR into 'master' of paritytech/substrate

@ltfschoen
Copy link
Owner Author

Uhm why is this MR in your repo?

FYI, i tried to get it to work with Alpine Linux instead of Ubuntu, but i could only reduce it to 236MB instead of 277MB, and i could only get it to work if i disabled the ldd ... command because there are some issues with the Alpine libraries.
so we can't consider using Alpine until that's resolved. i've included steps i took here in the PR with the changes i made so far #2

@ltfschoen ltfschoen changed the base branch from luke/13071-fix-ldd-error to master February 25, 2023 08:18
@ltfschoen ltfschoen changed the base branch from master to luke/13071-fix-ldd-error February 25, 2023 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants