Conversation
|
I guess you saw that a bunch of jobs are failing |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2206 +/- ##
===========================================
+ Coverage 30.91% 54.41% +23.49%
===========================================
Files 53 55 +2
Lines 20112 20865 +753
Branches 9755 9866 +111
===========================================
+ Hits 6217 11353 +5136
- Misses 7922 8068 +146
+ Partials 5973 1444 -4529 ☔ View full report in Codecov by Sentry. |
Did you do any benchmark to confirm this ? thanks |
No, just theory analysis. By using generic, the generated machine code is almost the same as before. |
BusyJay
force-pushed
the
support-uds
branch
2 times, most recently
from
78b5737 to
3d02761
Compare
|
I don't think the gcc issue is your fault |
| env::var("SCCACHE_SERVER_PORT") | ||
| fn get_addr() -> crate::net::SocketAddr { | ||
| #[cfg(unix)] | ||
| if let Ok(addr) = env::var("SCCACHE_SERVER_UDS") { |
There was a problem hiding this comment.
I think rather than add another configuration know, SCCACHE_SERVER_PORT should be parsed as an int, and if that doesn't work, try as a UDS, and if that doesn't work, use the default TCP port.
There was a problem hiding this comment.
Or, actually, if parsing doesn't work, it should be an error, rather than fallback. Only fallback when nothing is explicitly given.
There was a problem hiding this comment.
I'm trying not to break compatibility here. And it seems confusing to me to parse something named "port" as a path. If we want to use one single environment variable, we should choose some name like "SCCACHE_SERVER_ADDR" instead.
src/commands.rs
Outdated
| .unwrap_or(DEFAULT_PORT) | ||
| .unwrap_or(DEFAULT_PORT); | ||
| crate::net::SocketAddr::Net(std::net::SocketAddr::new( | ||
| "127.0.0.1".parse().unwrap(), |
There was a problem hiding this comment.
Should we use the 127.0.0.1 address here & inside src/server.rs:567? Is it OK to duplicate the default address in a few places?
Maybe we should add a const &str for this or just use the result of get_addr() inside the src/server.rs, isn't it?
src/commands.rs
Outdated
| "sccache: Listening on port {} instead of {}", | ||
| actualport, | ||
| port | ||
| "sccache: Listening on port {actual_addr} instead of {addr}" |
There was a problem hiding this comment.
| "sccache: Listening on port {actual_addr} instead of {addr}" | |
| "sccache: Listening on address {actual_addr} instead of {addr}" |
|
Hi @Xuanwo Looks like this PR is ready. Should we merge it? |
A new module named `net` is added to unify TCP socket and UDS. By default, sccache server is still listen on local TCP port, user can choose to listen on UDS by setting environment variable `SCCACHE_SERVER_UDS`. Generic is used in server implementation for best performance, trait object is used in client implementation for simplicity and better readability. Close mozilla#933. Signed-off-by: Jay Lee <[email protected]>
Signed-off-by: Jay Lee <[email protected]>
Signed-off-by: Jay Lee <[email protected]>
Signed-off-by: Jay Lee <[email protected]>
Signed-off-by: Jay Lee <[email protected]>
Signed-off-by: Jay Lee <[email protected]>
Signed-off-by: Jay Lee <[email protected]>
Signed-off-by: Jay Lee <[email protected]>
Signed-off-by: Jay Lee <[email protected]>
sylvestre
force-pushed
the
support-uds
branch
from
c4066cb to
6039160
Compare
6 participants
A new module named
netis added to unify TCP socket and UDS. By default, sccache server is still listen on local TCP port, user can choose to listen on UDS by setting environment variableSCCACHE_SERVER_UDS.Generic is used in server implementation for best performance, trait object is used in client implementation for simplicity and better readability.
Close #933.