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
| Actual Points: | → .1 |
|---|---|
| Owner: | set to nickm |
| Status: | new → accepted |
comment:2 Changed 7 weeks ago by
| Status: | accepted → needs_review |
|---|
comment:3 Changed 6 weeks ago by
| Reviewer: | → teor |
|---|
comment:4 Changed 6 weeks ago by
| Status: | needs_review → needs_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
| Status: | needs_revision → needs_review |
|---|
I've made the requested changes, and moved the dirclient-mode-checking functions in to the dirclient module.
comment:7 Changed 6 weeks ago by
| Status: | needs_review → needs_revision |
|---|
comment:8 Changed 6 weeks ago by
| Status: | needs_revision → needs_review |
|---|
Okay, now I've covered the last one. Please let me know if I missed anything?
comment:9 Changed 6 weeks ago by
(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
| 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
| Status: | needs_review → needs_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
| Status: | needs_revision → needs_review |
|---|
Whoops! I've pushed a couple more fixups. I'll keep an eye on CI.
comment:13 Changed 6 weeks ago by
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
| Status: | needs_review → merge_ready |
|---|
the updates branch passes, let's merge.
This went much faster than I had expected. See branch
ticket32487with PR at https://github.com/torproject/tor/pull/1644