Page MenuHomePhabricator

services: Remove gRPC source after installing
ClosedPublic

Authored by jim on Mar 10 2022, 11:36 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 20, 5:34 PM
Unknown Object (File)
Fri, Dec 20, 5:34 PM
Unknown Object (File)
Fri, Dec 20, 5:34 PM
Unknown Object (File)
Fri, Dec 20, 5:34 PM
Unknown Object (File)
Fri, Dec 20, 5:31 PM
Unknown Object (File)
Wed, Dec 4, 12:32 AM
Unknown Object (File)
Wed, Nov 27, 5:19 AM
Unknown Object (File)
Wed, Nov 27, 4:37 AM

Details

Summary

This reduces size of the Docker base image from 3.03GB to 1.29GB.

Also:

services: Pass gRPC build flags at build time
services: Better Docker layering in base image
services: Remove unused gRPC build variable

Test Plan

Run npm run build-all and run-all

Diff Detail

Repository
rCOMM Comm
Branch
grpc-shrink
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ashoat added inline comments.
services/base-image/contents/install_grpc.sh
30–31 ↗(On Diff #10274)

Thanks for adding this comment

This revision is now accepted and ready to land.Mar 10 2022, 8:05 PM

What is the reason behind having grpc flags passed to the cmake in the bash script instead of having them directly in the cmake file?

services/backup/docker-server/contents/build_server.sh
5 ↗(On Diff #10274)

What are these changes for?
I think it's safer with /s

In D3399#92777, @karol-bisztyga wrote:

What is the reason behind having grpc flags passed to the cmake in the bash script instead of having them directly in the cmake file?

Because that is when gRPC is built. In the CMake file, it's just using the pre-installed gRPC library.

See comments here https://phabricator.ashoat.com/D3198?id=10038#inline-19455 and here https://linear.app/comm/issue/ENG-824#comment-6555f66b.

services/backup/docker-server/contents/build_server.sh
5 ↗(On Diff #10274)

The changes are needed because I changed the WORKDIR in the base image from /. And I agree that it's preferable to change to an absolute path in this script OR in the backup service Dockerfile do RUN cd /transferred/server && ../scripts/build-server.sh. As in, if this script requires itself to be run from a certain place, that place should be the target directory, not the root directory.

Ah yes, sorry I get it now, it's alright!

services/backup/docker-server/contents/build_server.sh
5 ↗(On Diff #10274)

I just prefer an absolute path, it's a matter of design I believe. Either you make it more error-resistant in terms of where the script is being run from or in terms of where exactly is the target path in the file system.

This revision is now accepted and ready to land.Mar 15 2022, 8:16 PM