search

mozilla / sccache

Sccache is a ccache-like tool. It is used as a compiler wrapper and avoids compilation when possible. Sccache has the capability to utilize caching in remote storage environments, including various cloud storage options, or alternatively, in local storage.
Apache License 2.0
5.85k stars 552 forks source link

Add ability to specify server port from cli #2239

Open just-an-engineer opened 3 months ago

just-an-engineer commented 3 months ago

It now looks to see if a port number was specified on the command line, and tries to use that before looking at the config file or environment variables Since --start-server calls InternalStartServer under the hood, I have it starting it with the port number specified as the environment variable, although I can change it to also take it from the cli. Added tests and updated the readme for usages

codecov-commenter commented 3 months ago

Codecov Report

Attention: Patch coverage is 32.82443% with 88 lines in your changes missing coverage. Please review.

Project coverage is 51.79%. Comparing base (0cc0c62) to head (f07dc9b). Report is 72 commits behind head on main.

Files with missing lines Patch % Lines
src/util.rs 27.27% 13 Missing and 43 partials :warning:
src/commands.rs 36.36% 2 Missing and 12 partials :warning:
src/config.rs 43.75% 1 Missing and 8 partials :warning:
src/cmdline.rs 42.85% 0 Missing and 8 partials :warning:
tests/oauth.rs 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2239 +/- ## =========================================== + Coverage 30.91% 51.79% +20.87% =========================================== Files 53 54 +1 Lines 20112 20550 +438 Branches 9755 9540 -215 =========================================== + Hits 6217 10643 +4426 - Misses 7922 7956 +34 + Partials 5973 1951 -4022 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sylvestre commented 3 months ago

What about configuring it in a configuration file?

just-an-engineer commented 3 months ago

Yeah, I can do that. I'll add that as well

just-an-engineer commented 3 months ago

Alright, just added that in. Currently, this is the order of precedence in the code of where it looks for the port: CLI, config, env var, and lastly, the hardcoded default port of 4226. No particular reason for that order, so that can be changed up if desired.

just-an-engineer commented 3 months ago

@sylvestre, I'm a bit new to the whole process. At this point, do I select "Update branch"? Do I want for someone else to do something?

just-an-engineer commented 3 months ago

I realized I hadn't done anything for commands outside of start or stop. So I added in basic functionality for a client cache (which we can expand to other things), and added tests for that. I also corrected a prior test that was incorrect, I just wasn't thinking properly for that one. I also expanded it to test that a compile request works for a server started on a non-default port. The client cache will also help it be more ergonomic for users because they won't have to specify an environment variable alongside every sccache command they run, because if they started a server on a different port, they'd have to use the env vars to see its stats, run a compile request, etc. This can make it a bit more streamlined by assuming that you'll have a single compile server you primarily use (if that's not the case, this may break down, but it's not like it was easier beforehand), and once you specify it, that's the one you want to see the stats for, run compile jobs from, and shutdown