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 git shallow clones #52

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Daniel-Worrall
Copy link
Contributor

Optimises git cloning with shallow clone.

Reduces processing time & file size.

@RX14
Copy link
Contributor

RX14 commented Jan 16, 2020

Unfortunately this doesn't allow checking out a specific sha1 revision, unless it happens to be tagged.

If you ask for a specific revision from github it will refuse, unless the revision corresponds to an existing ref, such as a tag or a branch.

So, yes this works great for building tagged releases but breaks in the general case :(

@straight-shoota
Copy link
Member

I guess then there's nothing to do here.

@Daniel-Worrall
Copy link
Contributor Author

Daniel-Worrall commented Jan 16, 2020

Good catch

I can remove the changes with references to the crystal sha and keep the ones that use a tagged version like bdwgc, libatomic and shards. If it changes in the future that we need to use a specific sha, we could use a conditional.

@RX14
Copy link
Contributor

RX14 commented Jan 16, 2020

@Daniel-Worrall there's scope for wanting to build with a custom GC version in the future but I think that would be fine

@Daniel-Worrall
Copy link
Contributor Author

Done.

Also, I noticed that there could be further optimizations such as in the Linux Dockerfile, various things are cloned twice. These could be copied over to the 2nd build stage before altered so that it saves on cloning. The only problem with this is that currently all the cloning/configuration is done on a single RUN and splitting that up might just end up removing any optimizations.
Anyway, I'm not going to delve into it any further.

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