search

emk / rust-musl-builder

Docker images for compiling static Rust binaries using musl-libc and musl-gcc, with static versions of useful C libraries. Supports openssl and diesel crates.
Apache License 2.0
1.54k stars 193 forks source link

Updated the process of native libs builds to make it more reliable #33

Closed frol closed 6 years ago

frol commented 6 years ago

The main reason I touched this part was that postgres sources were left in the image and while I could fix it with a single /.. change, I decided to make the script more reliable overall.

emk commented 6 years ago

Could you please explain what your new strategy is, and why it's an improvement? Do you know whether /tmp is guaranteed to be large enough inside a Docker container for building a large C++ library, for example? Thank you!

frol commented 6 years ago
  1. During the build process, there is no difference between /tmp and any other location except the permissions (i.e. /tmp is not mounted separately, so if /home/rust/libs can fit all the libs then /tmp will also do). Thus, there is no point in a separate location for temporary files.
  2. Starting the builds from the absolute path (cd /tmp) just simplifies reading (if compare it to cd .. you will notice that you will need to scan for all the other cds to reason about the location you end up).
  3. You just clean the whole /tmp after you are done and that is it, so you don't need to list all the separate files and folders (you could have done the same with /home/rust/libs as well).
  4. Using explicit OPENSSL_VERSION variables is just more explicit way which is just future-proof (imagine you dropped the second VERS and that would just silently install zlib 1.0.1 instead of 1.2.11)
  5. Using double quotes around sh variables is just a best practice in this case (in general, it makes sure that whitespaces in a variable don't break the command you construct)
emk commented 6 years ago

Great! Thank you for the explanation of what you've changed. Have you also run the test-image script to make sure that everything still works? If so, I'll go ahead and merge this.

My apologies for not having a CONTRIBUTING.md suggesting what things to include with PRs. I should get around to that. :-) And thank you very much for the PR.

frol commented 6 years ago 
...
Successfully tagged rust-musl-builder-using-diesel:latest
Hello, world!
No DATABASE_URL set, so doing nothing
     Created binary (application) `testme` project
   Compiling testme v0.1.0 (file:///home/rust/src/testme)
    Finished dev [unoptimized + debuginfo] target(s) in 0.46 secs
Checking to make sure it is not a dynamic executable
        not a dynamic executable
OK. Tests passed.
emk commented 6 years ago

Thank you for the PR! I've merged it.

Docker Hub should rebuild the official images sometime in the next few hours.