Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#19450 closed enhancement (fixed)

Rip out --enable-bufferevents

Reported by: arma Owned by: U+039b
Priority: Medium Milestone: Tor: 0.2.9.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: review-group-7 TorCoreTeam201608
Cc: *@… Actual Points:
Parent ID: Points:
Reviewer: U+039b Sponsor:

Description

Periodically we run across packagers that provide the bufferevents shotgun and aim it at the user's foot: first was gentoo that turned it on by default, and today was freebsd that gives you a tantalizing option to click without any hint about how you'll be missing a foot afterwards.

I proposed in #18189 that we rip out the ability to set --enable-bufferevents, so we end the steady trickle of users who lose feet. We should totally do this.

The next question then is: should we also remove all the code that will effectively become dead code then? That one I want Nick's opinion on.

Child Tickets

Attachments (1)

0001-Fix-19450-Rip-out-enable-bufferevents.patch (3.3 KB) - added by U+039b 3 years ago.
Proposed patch

Download all attachments as: .zip

Change History (16)

comment:1 Changed 3 years ago by U+039b

Cc: U+039b removed

comment:2 Changed 3 years ago by U+039b

Owner: set to U+039b
Status: newassigned

Changed 3 years ago by U+039b

Proposed patch

comment:3 Changed 3 years ago by U+039b

Cc: *@… added
Status: assignedneeds_review

comment:4 Changed 3 years ago by yawning

The next question then is: should we also remove all the code that will effectively become dead code then? That one I want Nick's opinion on.

Yes. Revision control history is a thing if we ever need it, and it simplifies the existing code (or at least gets rid of rather large ifdefed codeblocks).

comment:5 Changed 3 years ago by U+039b

I have removed the dead code related to bufferevents. A patch is available here: https://gitlab.s1.0x39b.fr/lambda/tor/commit/c735220a0b4c2c7852145fe8e9bf2584d4901d1f

The compilation is OK, ./src/test/test are OK, ./src/or/tor is OK 

comment:6 Changed 3 years ago by nickm

Milestone: Tor: 0.2.???Tor: 0.2.9.x-final

I am +1 on doing this; can somebody give it a quick look-over?

comment:7 Changed 3 years ago by nickm

Keywords: review-group-6 added

comment:8 Changed 3 years ago by nickm

Reviewer: nickm

comment:9 Changed 3 years ago by nickm

Reviewer: nickm

+.2 points.

Okay, this looks good, except that I found more things to remove. So now the branch (based on your branch) is "Fix_19450" in my public repository. Since I've touched it so much, it needs an extra reviewer now. (That could be you!)

comment:10 Changed 3 years ago by nickm

Reviewer: U+039b

U+039b has volunteered to look at my changes.

comment:11 Changed 3 years ago by nickm

Keywords: review-group-7 added; review-group-6 removed

All review-group-6 tickets have had at least one review; moving them to 7.

comment:12 Changed 3 years ago by U+039b

Status: needs_reviewmerge_ready

comment:13 Changed 3 years ago by U+039b

I did the review of Nickm's branch (Fix_19450). Everything is ok for me.

comment:14 Changed 3 years ago by nickm

Resolution: fixed
Status: merge_readyclosed

Merging. Thank you! (Making sure it compiles and passes tests first.)

comment:15 Changed 3 years ago by nickm

Keywords: TorCoreTeam201608 added
Note: See TracTickets for help on using tickets.