Opened 3 months ago

Closed 5 weeks ago

#32487 closed enhancement (implemented)

Phase 1: Stop compiling "acting as a directory cache" in --disable-module-relay

Reported by: teor Owned by: nickm
Priority: Medium Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-design, network-team-roadmap-november, 043-can
Cc: nickm Actual Points: .1
Parent ID: #31851 Points: 1
Reviewer: teor Sponsor: Sponsor31-can

Description

We want to stop compiling the code that makes tor act as a directory cache, when the relay module is disabled.

(We already disable DirPort and DirCache, so the functionality is not available.)

At each stage, we should work to minimize layer-violations: there should generally not be calls from src/core/ into relay-specific code, and we should plan to refactor as needed to minimize them. We can reduce layer violations in parallel with the above.

Child Tickets

Change History (15)

comment:1 Changed 7 weeks ago by nickm

Actual Points: .1
Owner: set to nickm
Status: newaccepted

comment:2 Changed 7 weeks ago by nickm

Status: acceptedneeds_review

This went much faster than I had expected. See branch ticket32487 with PR at https://github.com/torproject/tor/pull/1644

comment:3 Changed 6 weeks ago by dgoulet

Reviewer: teor

comment:4 Changed 6 weeks ago by teor

Status: needs_reviewneeds_revision

Looks good overall, but there are a few things that need fixing.

I also had some design questions.

Please see my comments on the pull request.

comment:5 Changed 6 weeks ago by nickm

Status: needs_revisionneeds_review

I've made the requested changes, and moved the dirclient-mode-checking functions in to the dirclient module.

comment:6 Changed 6 weeks ago by nickm

Oh, wait, I still have one more to do.

comment:7 Changed 6 weeks ago by nickm

Status: needs_reviewneeds_revision

comment:8 Changed 6 weeks ago by nickm

Status: needs_revisionneeds_review

Okay, now I've covered the last one. Please let me know if I missed anything?

comment:9 Changed 6 weeks ago by nickm

(Fwiw, I expect this to conflict in include.am files with #32137 on merge; I'm happy to resolve the conflicts when we do the merge.)

comment:10 Changed 6 weeks ago by nickm

Keywords: 043-can added

tag all currently needs_review tickets with 043-can. (Since there's code before the feature freeze, maybe we can take it.)

comment:11 Changed 6 weeks ago by teor

Status: needs_reviewneeds_revision

Code changes look good, there's one minor mistake in a comment.

But CI is failing when we disable relay, so there's something wrong here.

comment:12 Changed 6 weeks ago by nickm

Status: needs_revisionneeds_review

Whoops! I've pushed a couple more fixups. I'll keep an eye on CI.

comment:13 Changed 6 weeks ago by nickm

Okay, it looks like CI is failing because of the merge conflict with #32137 I mentioned above.

I've made a new branch ticket32487_squashed_and_merged. The PR is https://github.com/torproject/tor/pull/1669

comment:14 Changed 5 weeks ago by teor

Status: needs_reviewmerge_ready

the updates branch passes, let's merge.

comment:15 Changed 5 weeks ago by nickm

Resolution: implemented
Status: merge_readyclosed

Merged it!

Note: See TracTickets for help on using tickets.