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

Don't copy over all build artifacts #108

Merged
merged 4 commits into from
Feb 16, 2021

Conversation

CosmicHorrorDev
Copy link
Collaborator

@CosmicHorrorDev CosmicHorrorDev commented Feb 16, 2021

Description

Contributions

Nothing too exciting here. Just changing to copy over just the odin binary instead of the whole build folder. This should slim down the image by ~200MB. I'm assuming that the binary is copied over then symlinked later because of backwards compatibility reasons?

Also side note, but congratz on the soon to be 200 stars!

Checklist

  • I added one or multiple labels which best describes this PR.
  • I have tested the changes locally.
  • This PR has a reviewer on it.
  • I have validated my changes in a docker container and on Ubuntu. (Only needed for Odin or Docker Changes)

@mbround18 mbround18 added the docker Tag if its related to docker label Feb 16, 2021
@mbround18
Copy link
Owner

mbround18 commented Feb 16, 2021

Also side note, but congratz on the soon to be 200 stars!

Thank you <3

For the binary, you can remove the symlink too and just copy the binary directly into /usr/local/bin ill be honest, I did not know in rust land the other artifacts were not required for the binary to run.

@mbround18 mbround18 self-requested a review February 16, 2021 16:26
@CosmicHorrorDev
Copy link
Collaborator Author

All good, it's an easy mistake to make. Most of the stuff in target is to allow for incremental compilation so it will have compiled dependencies and all that jazz. Rust by default will statically link (vs dynamically linked) meaning that all the required dependencies are bundled up in the final executable

I think we should be all good now. If people made any custom scripts pointing to /usr/local/odin then they may have problems with these changes, but that should be an easy fix for everyone

@mbround18 mbround18 merged commit a554d28 into mbround18:main Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker Tag if its related to docker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants