Skip to content

Commit 0363867

Browse files
jschwejschwe
authored
Fix clippy lints for dist-server (#1119)
With the dist-server feature enabled, additional clippy warnings are emitted. Those are fixed by this commit. Lints were: - unneccessary borrow - string != "" should be !string.is_empty() - unwrap_or followed by a function call -> unwrap_or_else Signed-off-by: Jonathan Schwender <[email protected]>
1 parent dd87da9 commit 0363867

File tree

5 files changed

+37
-29
lines changed

5 files changed

+37
-29
lines changed

‎src/bin/sccache-dist/build.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@ fn docker_diff(cid: &str) -> Result<String> {
477477
// Force remove the container
478478
fn docker_rm(cid: &str) -> Result<()> {
479479
Command::new("docker")
480-
.args(&["rm", "-f", &cid])
480+
.args(&["rm", "-f", cid])
481481
.check_run()
482482
.context("Failed to force delete container")
483483
}
@@ -511,7 +511,7 @@ impl DockerBuilder {
511511
.args(&["ps", "-a", "--format", "{{.ID}} {{.Image}}"])
512512
.check_stdout_trim()
513513
.context("Unable to list all Docker containers")?;
514-
if containers != "" {
514+
if !containers.is_empty() {
515515
let mut containers_to_rm = vec![];
516516
for line in containers.split(|c| c == '\n') {
517517
let mut iter = line.splitn(2, ' ');
@@ -541,7 +541,7 @@ impl DockerBuilder {
541541
.args(&["images", "--format", "{{.ID}} {{.Repository}}"])
542542
.check_stdout_trim()
543543
.context("Failed to list all docker images")?;
544-
if images != "" {
544+
if !images.is_empty() {
545545
let mut images_to_rm = vec![];
546546
for line in images.split(|c| c == '\n') {
547547
let mut iter = line.splitn(2, ' ');
@@ -604,12 +604,12 @@ impl DockerBuilder {
604604
fn clean_container(&self, cid: &str) -> Result<()> {
605605
// Clean up any running processes
606606
Command::new("docker")
607-
.args(&["exec", &cid, "/busybox", "kill", "-9", "-1"])
607+
.args(&["exec", cid, "/busybox", "kill", "-9", "-1"])
608608
.check_run()
609609
.context("Failed to run kill on all processes in container")?;
610610

611-
let diff = docker_diff(&cid)?;
612-
if diff != "" {
611+
let diff = docker_diff(cid)?;
612+
if !diff.is_empty() {
613613
let mut lastpath = None;
614614
for line in diff.split(|c| c == '\n') {
615615
let mut iter = line.splitn(2, ' ');
@@ -643,17 +643,17 @@ impl DockerBuilder {
643643
}
644644
lastpath = Some(changepath);
645645
if let Err(e) = Command::new("docker")
646-
.args(&["exec", &cid, "/busybox", "rm", "-rf", changepath])
646+
.args(&["exec", cid, "/busybox", "rm", "-rf", changepath])
647647
.check_run()
648648
{
649649
// We do a final check anyway, so just continue
650650
warn!("Failed to remove added path in a container: {}", e)
651651
}
652652
}
653653

654-
let newdiff = docker_diff(&cid)?;
654+
let newdiff = docker_diff(cid)?;
655655
// See note about changepath == "/tmp" above
656-
if newdiff != "" && newdiff != "C /tmp" {
656+
if !newdiff.is_empty() && newdiff != "C /tmp" {
657657
bail!(
658658
"Attempted to delete files, but container still has a diff: {:?}",
659659
newdiff
@@ -814,7 +814,7 @@ impl DockerBuilder {
814814
cmd.arg("-e").arg(env);
815815
}
816816
let shell_cmd = "cd \"$1\" && shift && exec \"$@\"";
817-
cmd.args(&[cid, "/busybox", "sh", "-c", &shell_cmd]);
817+
cmd.args(&[cid, "/busybox", "sh", "-c", shell_cmd]);
818818
cmd.arg(&executable);
819819
cmd.arg(cwd);
820820
cmd.arg(executable);

‎src/bin/sccache-dist/main.rs‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ fn parse() -> Result<Command> {
215215
.value_of("config")
216216
.expect("missing config in parsed subcommand"),
217217
);
218-
check_init_syslog("sccache-scheduler", &matches);
218+
check_init_syslog("sccache-scheduler", matches);
219219
if let Some(config) = scheduler_config::from_path(config_path)? {
220220
Command::Scheduler(config)
221221
} else {
@@ -228,7 +228,7 @@ fn parse() -> Result<Command> {
228228
.value_of("config")
229229
.expect("missing config in parsed subcommand"),
230230
);
231-
check_init_syslog("sccache-buildserver", &matches);
231+
check_init_syslog("sccache-buildserver", matches);
232232
if let Some(config) = server_config::from_path(config_path)? {
233233
Command::Server(config)
234234
} else {
@@ -262,10 +262,10 @@ fn create_jwt_server_token(
262262
key: &[u8],
263263
) -> Result<String> {
264264
let key = jwt::EncodingKey::from_secret(key);
265-
jwt::encode(&header, &ServerJwt { server_id }, &key).map_err(Into::into)
265+
jwt::encode(header, &ServerJwt { server_id }, &key).map_err(Into::into)
266266
}
267267
fn dangerous_insecure_extract_jwt_server_token(server_token: &str) -> Option<ServerId> {
268-
jwt::dangerous_insecure_decode::<ServerJwt>(&server_token)
268+
jwt::dangerous_insecure_decode::<ServerJwt>(server_token)
269269
.map(|res| res.claims.server_id)
270270
.ok()
271271
}
@@ -333,7 +333,7 @@ fn run(command: Command) -> Result<i32> {
333333
scheduler_config::ServerAuth::Insecure => {
334334
warn!("Scheduler starting with DANGEROUSLY_INSECURE server authentication");
335335
let token = INSECURE_DIST_SERVER_TOKEN;
336-
Box::new(move |server_token| check_server_token(server_token, &token))
336+
Box::new(move |server_token| check_server_token(server_token, token))
337337
}
338338
scheduler_config::ServerAuth::Token { token } => {
339339
Box::new(move |server_token| check_server_token(server_token, &token))
@@ -395,7 +395,7 @@ fn run(command: Command) -> Result<i32> {
395395
let scheduler_auth = match scheduler_auth {
396396
server_config::SchedulerAuth::Insecure => {
397397
warn!("Server starting with DANGEROUSLY_INSECURE scheduler authentication");
398-
create_server_token(server_id, &INSECURE_DIST_SERVER_TOKEN)
398+
create_server_token(server_id, INSECURE_DIST_SERVER_TOKEN)
399399
}
400400
server_config::SchedulerAuth::Token { token } => {
401401
create_server_token(server_id, &token)
@@ -522,6 +522,12 @@ impl Scheduler {
522522
}
523523
}
524524

525+
impl Default for Scheduler {
526+
fn default() -> Self {
527+
Self::new()
528+
}
529+
}
530+
525531
impl SchedulerIncoming for Scheduler {
526532
fn handle_alloc_job(
527533
&self,
@@ -742,7 +748,7 @@ impl SchedulerIncoming for Scheduler {
742748
}
743749
Some(ref mut details) if details.server_nonce != server_nonce => {
744750
for job_id in details.jobs_assigned.iter() {
745-
if jobs.remove(&job_id).is_none() {
751+
if jobs.remove(job_id).is_none() {
746752
warn!(
747753
"Unknown job found when replacing server {}: {}",
748754
server_id.addr(),

‎src/config.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -762,7 +762,7 @@ pub mod scheduler {
762762
}
763763

764764
pub fn from_path(conf_path: &Path) -> Result<Option<Config>> {
765-
super::try_read_config_file(&conf_path).context("Failed to load scheduler config file")
765+
super::try_read_config_file(conf_path).context("Failed to load scheduler config file")
766766
}
767767
}
768768

@@ -817,7 +817,7 @@ pub mod server {
817817
}
818818

819819
pub fn from_path(conf_path: &Path) -> Result<Option<Config>> {
820-
super::try_read_config_file(&conf_path).context("Failed to load server config file")
820+
super::try_read_config_file(conf_path).context("Failed to load server config file")
821821
}
822822
}
823823

‎src/dist/http.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,7 @@ mod server {
608608
fn verify_token(&self, job_id: JobId, token: &str) -> Result<()> {
609609
let valid_claims = JobJwt { job_id };
610610
let key = jwt::DecodingKey::from_secret(&self.server_key);
611-
jwt::decode(&token, &key, &JWT_VALIDATION)
611+
jwt::decode(token, &key, &JWT_VALIDATION)
612612
.map_err(|e| anyhow!("JWT decode failed: {}", e))
613613
.and_then(|res| {
614614
fn identical_t<T>(_: &T, _: &T) {}
@@ -778,7 +778,7 @@ mod server {
778778
let alloc_job_res: AllocJobResult = try_or_500_log!(req_id, handler.handle_alloc_job(&requester, toolchain));
779779
let certs = server_certificates.lock().unwrap();
780780
let res = AllocJobHttpResponse::from_alloc_job_result(alloc_job_res, &certs);
781-
prepare_response(&request, &res)
781+
prepare_response(request, &res)
782782
},
783783
(GET) (/api/v1/scheduler/server_certificate/{server_id: ServerId}) => {
784784
let certs = server_certificates.lock().unwrap();
@@ -788,7 +788,7 @@ mod server {
788788
cert_digest: cert_digest.clone(),
789789
cert_pem: cert_pem.clone(),
790790
};
791-
prepare_response(&request, &res)
791+
prepare_response(request, &res)
792792
},
793793
(POST) (/api/v1/scheduler/heartbeat_server) => {
794794
let server_id = check_server_auth_or_err!(request);
@@ -807,7 +807,7 @@ mod server {
807807
num_cpus,
808808
job_authorizer
809809
));
810-
prepare_response(&request, &res)
810+
prepare_response(request, &res)
811811
},
812812
(POST) (/api/v1/scheduler/job_state/{job_id: JobId}) => {
813813
let server_id = check_server_auth_or_err!(request);
@@ -817,11 +817,11 @@ mod server {
817817
let res: UpdateJobStateResult = try_or_500_log!(req_id, handler.handle_update_job_state(
818818
job_id, server_id, job_state
819819
));
820-
prepare_response(&request, &res)
820+
prepare_response(request, &res)
821821
},
822822
(GET) (/api/v1/scheduler/status) => {
823823
let res: SchedulerStatusResult = try_or_500_log!(req_id, handler.handle_status());
824-
prepare_response(&request, &res)
824+
prepare_response(request, &res)
825825
},
826826
_ => {
827827
warn!("Unknown request {:?}", request);
@@ -968,7 +968,7 @@ mod server {
968968
trace!("Req {}: assign_job({}): {:?}", req_id, job_id, toolchain);
969969

970970
let res: AssignJobResult = try_or_500_log!(req_id, handler.handle_assign_job(job_id, toolchain));
971-
prepare_response(&request, &res)
971+
prepare_response(request, &res)
972972
},
973973
(POST) (/api/v1/distserver/submit_toolchain/{job_id: JobId}) => {
974974
job_auth_or_401!(request, &job_authorizer, job_id);
@@ -977,7 +977,7 @@ mod server {
977977
let body = request.data().expect("body was already read in submit_toolchain");
978978
let toolchain_rdr = ToolchainReader(Box::new(body));
979979
let res: SubmitToolchainResult = try_or_500_log!(req_id, handler.handle_submit_toolchain(&requester, job_id, toolchain_rdr));
980-
prepare_response(&request, &res)
980+
prepare_response(request, &res)
981981
},
982982
(POST) (/api/v1/distserver/run_job/{job_id: JobId}) => {
983983
job_auth_or_401!(request, &job_authorizer, job_id);
@@ -996,7 +996,7 @@ mod server {
996996
let outputs = outputs.into_iter().collect();
997997

998998
let res: RunJobResult = try_or_500_log!(req_id, handler.handle_run_job(&requester, job_id, command, outputs, inputs_rdr));
999-
prepare_response(&request, &res)
999+
prepare_response(request, &res)
10001000
},
10011001
_ => {
10021002
warn!("Unknown request {:?}", request);

‎tests/dist.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ fn basic_compile(tmpdir: &Path, sccache_cfg_path: &Path, sccache_cached_cfg_path
3434
write_source(tmpdir, source_file, "#if !defined(SCCACHE_TEST_DEFINE)\n#error SCCACHE_TEST_DEFINE is not defined\n#endif\nint x() { return 5; }");
3535
sccache_command()
3636
.args(&[
37-
std::env::var("CC").unwrap_or("gcc".to_string()).as_str(),
37+
std::env::var("CC")
38+
.unwrap_or_else(|_| "gcc".to_string())
39+
.as_str(),
3840
"-c",
3941
"-DSCCACHE_TEST_DEFINE",
4042
])

0 commit comments

Comments
 (0)