Opened 15 months ago

Closed 10 months ago

Last modified 10 months ago

#22110 closed defect (fixed)

Defining TOR_BUILD_TAG and tor_git_revision violates the version spec

Reported by: teor Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version:
Severity: Minor Keywords: easy, tor-spec, spec
Cc: Actual Points:
Parent ID: Points: 0.5
Reviewer: Sponsor:

Description

When we removed the git revision in #2988, we allowed vendors to specify TOR_BUILD_TAG instead. But if we specify both TOR_BUILD_TAG and tor_git_revision, get_version() returns a string like this:

Tor 0.2.9.9 (TOR_BUILD_TAG) (git-tor_git_revision)

This violates the version spec, which only allows one set of brackets for EXTRA_INFO.
https://gitweb.torproject.org/torspec.git/tree/version-spec.txt#n22

So instead, we should use:

Tor 0.2.9.9 (TOR_BUILD_TAG,git-tor_git_revision)

(We can't use spaces in the EXTRA_INFO.)

We should also write a unit test that checks that our own version passes the directory authority checks.

This isn't serious, because the only programmatic interface that uses this is GETINFO version.

Child Tickets

Change History (12)

comment:1 Changed 15 months ago by teor

(See #22109 for testing our own version.)

comment:2 Changed 15 months ago by cypherpunks

Somewhat related to #20982 which is also about the version string.

comment:3 Changed 12 months ago by teor

Keywords: tor-spec added

comment:4 Changed 10 months ago by nickm

Keywords: spec added

Add 'spec' keyword to items that are just spec fixes. These can land after the feature-freeze.

comment:5 Changed 10 months ago by nickm

Status: newneeds_review

I've tried doing a simple change here, to ratify the existing behavior, in torspec branch bug22110. What do you think?

comment:6 Changed 10 months ago by nickm

another option here might be to remove TOR_BUILD_TAG. Does anyone use it?

comment:7 Changed 10 months ago by nickm

Owner: set to nickm
Status: needs_reviewassigned

setting owner

comment:8 Changed 10 months ago by nickm

Status: assignedneeds_review

comment:9 in reply to:  6 Changed 10 months ago by teor

Replying to nickm:

I've tried doing a simple change here, to ratify the existing behavior, in torspec branch bug22110. What do you think?

This looks good to me. Let's get it merged, to document the behaviour up to at least 0.3.2.

Replying to nickm:

another option here might be to remove TOR_BUILD_TAG. Does anyone use it?

I have no idea.

comment:10 Changed 10 months ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merging the torspec branch; calling this good. :)

comment:11 Changed 10 months ago by atagar

Hi Nick. Just a quick head's up that I'm adjusting Stem to account for this. The next release (1.6.0) will permit multiple extra entries but according to prior versions such version strings are invalid.

Note: See TracTickets for help on using tickets.