Opened 9 months ago

Closed 9 months ago

#29703 closed defect (fixed)

Backport test-network.sh fixes to 0.2.9

Reported by: teor Owned by: teor
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version: Tor: 0.2.9.1-alpha
Severity: Normal Keywords: merge-after-0403-alpha, teor-merge, CI, PTs 029-backport network-team-roadmap-2019-Q1Q2
Cc: cohosh Actual Points: 0.4
Parent ID: #29280 Points: 0.5
Reviewer: nickm Sponsor:

Description

In our 0.3.4 chutney CI, we get the chutney version of test-network.sh:

test-network.sh: using CHUTNEY_DNS_CONF '/dev/null'
test-network.sh: $TOR_DIR not set, trying $PWD
test-network.sh: $TOR_DIR is a Tor 0.3.4 or earlier build directory

https://travis-ci.org/torproject/tor/jobs/499382754#L3067

But in 0.2.9, we get the tor version of test-network.sh:

test-network.sh: Parsing command-line arguments to find $CHUTNEY_PATH
test-network.sh: $TOR_DIR not set, trying $PWD

Launching chutney using Python 2.7.6

https://travis-ci.org/torproject/tor/jobs/499382330#L2859

But tor 0.2.9's test-network.sh is meant to call chutney's test-network.sh.

We should fix this bug to make sure that we're getting consistent results from our CI. There are a lot of extra bug fixes in the latest chutney's test-network.sh, that aren't in tor 0.2.9's test-network.sh.

Child Tickets

Change History (10)

comment:1 Changed 9 months ago by teor

Owner: set to teor
Status: newassigned

comment:2 Changed 9 months ago by teor

Actual Points: 0.2
Reviewer: nickm
Status: assignedneeds_review

To fix this issue, we need to backport the 0.3.0 test-network.sh fixes:

  • #21570 Add a quiet mode to test-network.sh
  • #21562 Use bash in src/test/test-network.sh
  • #21581 Restore support for test-network.sh on BSD and other systems without bash

The cherry-picks apply cleanly to maint-0.2.9.

I could try to remove #21570 from the backport, but I'd rather use exactly the same code that we tested in 0.3.0.

The 0.2.9 pull request is:
https://github.com/torproject/tor/pull/773

The 0.3.4 pull request is just an ours merge to maint-0.3.4, because these changes were already in 0.3.0 and later:
https://github.com/torproject/tor/pull/778

The 0.2.9 CI merged with chutney CI from #29280 is:
https://github.com/torproject/tor/pull/777

It calls chutney's test-network.sh:

test-network.sh: $TOR_DIR not set, trying $PWD
test-network.sh: Calling newer chutney script /home/travis/build/teor2345/tor/chutney/tools/test-network.sh
test-network.sh: using CHUTNEY_DNS_CONF '/dev/null'
test-network.sh: $TOR_DIR is a Tor 0.3.4 or earlier build directory

https://travis-ci.org/teor2345/tor/jobs/503853599#L2849

Some of the jobs failed due to #29706, which is a shared random unit test bug.

Assigning the review to nickm, because this backport is part of an ongoing design discussion on #29280.

comment:3 Changed 9 months ago by nickm

None of the changes terrify me greatly, but I do have a few questions about the underlying rationale here:

  • Why do we need to backport quiet mode to solve this bug?
  • Why do we need to backport exit status 2 to solve this bug?
  • Do we really want to be maintaining the 0.2.9 version of this script for the lifetime of 0.2.9? How bad would it be to simply copy the script from 0.3.4 into maint-0.2.9? (I realize that it's not our usual practice to consider a large backport to be better than a small one, but I think in this case, it might be.)

comment:4 in reply to:  3 Changed 9 months ago by teor

Status: needs_reviewneeds_revision

Replying to nickm:

None of the changes terrify me greatly, but I do have a few questions about the underlying rationale here:

  • Why do we need to backport quiet mode to solve this bug?
  • Why do we need to backport exit status 2 to solve this bug?

We don't, but it seemed easier to backport the same branch, rather than split it up.

  • Do we really want to be maintaining the 0.2.9 version of this script for the lifetime of 0.2.9? How bad would it be to simply copy the script from 0.3.4 into maint-0.2.9? (I realize that it's not our usual practice to consider a large backport to be better than a small one, but I think in this case, it might be.)

I agree - I think the script will be easier to maintain if it matches 0.3.4. I'll do a pull request some time today.

comment:5 Changed 9 months ago by teor

I wasn't able to do this pull request, I will try to get it done by the end of the week.

comment:6 Changed 9 months ago by teor

I won't merge this into 0.2.9 (and merge forward) until next week: nickm is doing an alpha.

comment:7 Changed 9 months ago by teor

Actual Points: 0.20.3
Keywords: merge-after-0403-alpha teor-merge added
Milestone: Tor: 0.4.1.x-finalTor: 0.2.9.x-final
Status: needs_revisionneeds_review

Ok, I have backported tor/src/tools/test-network.sh from 0.3.4 to 0.2.9.

Here is the pull request:
https://github.com/torproject/tor/pull/773

I'll need to do an "ours" merge into 0.3.4, because the changes are already in 0.3.4.
There's also no need for an initial mainline merge for this branch.

I won't merge this into 0.2.9 (and merge forward) until next week: nickm is doing an alpha.

comment:8 Changed 9 months ago by nickm

Status: needs_reviewmerge_ready

PR looks good to me, assuming you have tested it to make sure test-network.sh passes for you.

comment:9 Changed 9 months ago by teor

0.4.0.3-alpha was released on Friday, so these tickets can now be merged.

#29018 and #29806 can be merged by asn, and #29703 can be merged by teor as a backport.

comment:10 Changed 9 months ago by teor

Actual Points: 0.30.4
Resolution: fixed
Status: merge_readyclosed

Merged to maint-0.2.9, and merged forward (but the changes are already in 0.3.4 and later).

Note: See TracTickets for help on using tickets.