Opened 5 months ago

Closed 4 months ago

#26455 closed defect (fixed)

use correct CARGO_HOME in test_rust.sh

Reported by: catalyst Owned by: catalyst
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: rust, tor-ci, tor-test, fast-fix
Cc: Actual Points:
Parent ID: Points: 1
Reviewer: isis Sponsor: SponsorM-can

Description

make check currently doesn't seem to work with --enable-rust without --enable-cargo-online-mode.

It looks like cargo expects CARGO_HOME to point to the .cargo directory itself, not the directory containing it. Builds seem to work anyway because we cd to a directory where cargo can find a .cargo directory, even though we set CARGO_HOME incorrectly.

This might be making Jenkins builds fail too:

https://jenkins.torproject.org/job/tor-ci-linux-master-rust/ARCHITECTURE=i386,SUITE=sid/lastBuild/console

20:04:05 FAIL: src/test/test_rust.sh
20:04:05 ===========================
20:04:05 
20:04:05 error: attempting to make an HTTP request, but --frozen was specified
20:04:05 error: attempting to make an HTTP request, but --frozen was specified

This incarnation of this bug seems to be in the fix for #26258, but similar problems seem to exist in releases as far back as 0.3.1.

Child Tickets

Change History (8)

comment:1 Changed 5 months ago by catalyst

Status: assignedneeds_review

comment:2 Changed 5 months ago by chelseakomlo

Good catch. We also set CARGO_HOME in Makefile.in so this should likely be updated as well?

comment:3 in reply to:  2 ; Changed 5 months ago by catalyst

Replying to chelseakomlo:

Good catch. We also set CARGO_HOME in Makefile.in so this should likely be updated as well?

Maybe not in a backported patch? I think the cargo build invocations happen to work by accident right now because they cd into the directory that contains the (autoconf-generated) .cargo directory in the build tree.

I think it's probably better to clean up the other build instances of this problem on a separate ticket, because it's fortuitously working right now.

comment:4 Changed 5 months ago by asn

Reviewer: isis

comment:5 in reply to:  3 ; Changed 5 months ago by chelseakomlo

Replying to catalyst:

Replying to chelseakomlo:

Good catch. We also set CARGO_HOME in Makefile.in so this should likely be updated as well?

Maybe not in a backported patch? I think the cargo build invocations happen to work by accident right now because they cd into the directory that contains the (autoconf-generated) .cargo directory in the build tree.

I think it's probably better to clean up the other build instances of this problem on a separate ticket, because it's fortuitously working right now.

It is good to keep like commits together, so if the complexity of backporting is too much to do in this patch, please make sure this is fixed in a future ticket and the ticket number noted here so we can track it.

comment:6 in reply to:  5 Changed 5 months ago by catalyst

Replying to chelseakomlo:

It is good to keep like commits together, so if the complexity of backporting is too much to do in this patch, please make sure this is fixed in a future ticket and the ticket number noted here so we can track it.

This is a good idea. New ticket for the broader issue is #26497.

comment:7 Changed 5 months ago by isis

Points: 1
Sponsor: SponsorM-can
Status: needs_reviewmerge_ready

LGTM! I've always wondered why we were setting it to /src/rust and how that was even working, but I guess it wasn't!

comment:8 Changed 4 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

Merged to 0.3.3 and forward. Resolved a simple conflict in 75d9db9e5b6d70002.

Note: See TracTickets for help on using tickets.