Opened 7 months ago

Closed 3 weeks ago

#26038 closed enhancement (worksforme)

Misc Rust/Cargo improvements (incl. use global cargo cache)

Reported by: Hello71 Owned by: Hello71
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: rust, 035-removed-20180711
Cc: Sebastian, chelseakomlo Actual Points:
Parent ID: Points:
Reviewer: isis Sponsor:

Description

https://cgit.alxu.ca/tor.git/log/?h=misc-rust

for discussion (since nobody cared on #tor-dev): should we use the global cargo cache? I think most C+Rust projects still use the global cache. I tried searching GitHub (https://github.com/search?q=%22CARGO_HOME%22+extension%3Aam&type=Code). I found that tor is the only project that does not. for users who do not care, using the global cache will save download time and bandwidth on repeat builds, and for those who do care, my patch prints a warning so they will know. (maybe it should be downgraded to NOTICE?)

Child Tickets

Change History (17)

comment:1 Changed 7 months ago by Hello71

Status: assignedneeds_review

comment:2 Changed 7 months ago by nickm

Milestone: Tor: 0.3.4.x-final

comment:3 Changed 7 months ago by Hello71

Status: needs_reviewneeds_revision

hm, warning for relative CARGO_HOME doesn't work right now, must fix...

comment:4 Changed 6 months ago by Hello71

Status: needs_revisionneeds_review

missing ; before \\n. why make gotta make shell harder than it already is

comment:5 Changed 6 months ago by asn

Reviewer: isis

comment:6 Changed 6 months ago by isis

Cc: Sebastian added
Status: needs_reviewneeds_information

I think I'd like to hear from Sebastian, if possible, about the reasoning for using a different CARGO_HOME originally? Perhaps this had something to do with capturing build artifacts in Jenkins?

comment:7 Changed 6 months ago by isis

Milestone: Tor: 0.3.4.x-finalTor: 0.3.5.x-final

Triaging more non-vital tickets into 0.3.5, but we're happy to take this in 0.3.4 if we get to it.

comment:8 Changed 6 months ago by isis

FWIW, CI passes.

comment:9 Changed 4 months ago by nickm

Keywords: 035-removed-20180711 added
Milestone: Tor: 0.3.5.x-finalTor: unspecified

These tickets are being triaged out of 0.3.5. The ones marked "035-roadmap-proposed" may return.

comment:10 Changed 3 months ago by teor

Parent ID: #24629

comment:11 Changed 3 months ago by teor

Status: needs_informationneeds_review

These fixes will be in my #24629 branch.

comment:12 Changed 3 months ago by chelseakomlo

Cc: chelseakomlo added

comment:13 Changed 3 months ago by chelseakomlo

Can this ticket be closed if the fixes will be in #24629? Otherwise it would be helpful to have a clear summary of the additional intended changes and improvements for reviewing.

comment:14 Changed 3 months ago by chelseakomlo

Status: needs_reviewneeds_information

Please provide a list of the intended changes that this patch introduces, if there are more than changing to use a global Cargo cache.

comment:15 Changed 3 months ago by Hello71

  1. remove source.crates-io.registry = 'https://github.com/rust-lang/crates.io-index'. I think this is obsolete.
  1. set target-dir = './target' instead of CARGO_TARGET_DIR, so we don't have to set that with cargo build and cargo test
  1. stop setting CARGO_HOME. warn if it is set for backwards compat, and error if it is a relative path. now that I think about it, maybe CARGO_HOME should be AC_ARG_VAR, but I don't think it really matters.

comment:16 Changed 3 months ago by teor

Parent ID: #24629
Status: needs_informationneeds_revision

Based on the feedback in:
https://trac.torproject.org/projects/tor/ticket/24629#comment:29
https://trac.torproject.org/projects/tor/ticket/24629#comment:30

I'm not going to try and merge this patch set into #24629.

Once #24629 merges, please rebase this patch set on top of maint-0.3.2.

comment:17 Changed 3 weeks ago by Hello71

Resolution: worksforme
Status: needs_revisionclosed

the major thing here was CARGO_HOME, everything else isn't worth pursuing

Note: See TracTickets for help on using tickets.