Opened 4 years ago

Closed 4 years ago

#19285 closed enhancement (fixed)

Parse "package" lines in consensuses using method 19 or later

Reported by: karsten Owned by: karsten
Priority: Medium Milestone:
Component: Metrics/Library Version:
Severity: Normal Keywords:
Cc: iwakeh Actual Points:
Parent ID: #19398 Points:
Reviewer: Sponsor:


From dir-spec.txt:

    "package" SP PackageName SP Version SP URL SP DIGESTS NL

        [Any number of times.]

        For this element:

        URL = NONSPACE
        NONSPACE = one or more non-space printing characters
        DIGESTVAL = DIGESTTYPE = one or more non-=, non-" " characters.

        Indicates that a package called "package" of version VERSION may be
        found at URL, and its digest as computed with DIGESTTYPE is equal to
        DIGESTVAL.  In consensuses, these lines are sorted lexically by
        "PACKAGENAME VERSION" pairs, and DIGESTTYPES must appear in ascending
        order.  A consensus must not contain the same "PACKAGENAME VERSION"
        more than once.  If a vote contains the same "PACKAGENAME VERSION"
        more than once, all but the last is ignored.

        Included in consensuses only for method 19 and later.

Found in the wild here, though looks mostly like testing:

$ grep -C1 "^package " 23/2016-04-23-03-00-00-vote-D586D18309DED4CD6D57C18FDB97EFA96D330566-BA14F96AB22EA5117C2D29B79BABD06ECAE451FF 
package shouldbesecond 0 http digest=digest
package outoforder 0 http digest=digest
known-flags Authority BadExit Exit Fast Guard HSDir Running Stable V2Dir Valid

We might get away by just providing package lines without parsing them, as in: getPackageLines().

Child Tickets

Change History (7)

comment:1 Changed 4 years ago by karsten

Cc: iwakeh added
Status: newneeds_review

Please review my branch task-19285.

comment:2 Changed 4 years ago by karsten

Parent ID: #19398

comment:3 Changed 4 years ago by iwakeh

All fine, only the @Override annotation is missing in the new (and old) methods.

And, out of curiosity: Is this if (line.length() > "package ".length()) a precaution? An incomplete line is not allowed according to the protocol above (afaict)?
There is no test with an incomplete line.

comment:4 Changed 4 years ago by karsten

Wow, that was quick. Thanks! I think I fixed the second issue by being a little stricter about the format and throwing an exception if a "package" line contains fewer than 4 arguments. Please take a look at the updated branch if you can.

I'd like to fix the @Override annotations for all code in metrics-lib at once. My Eclipse setup is currently broken, so I'd want to fix that first and then add annotations with Eclipse's help. I'll do that in a separate ticket. Sound okay?

comment:5 Changed 4 years ago by karsten

Please find another commit in the same branch with @Override annotations and some minor changes while putting them in.

comment:6 Changed 4 years ago by iwakeh

Status: needs_reviewmerge_ready

The clean-up looks good and checking for the number of parts is a good compromise between
checking nothing and parsing every field.
Great to have the @Override annotations! That really helps (me) when reading/adapting the code.

comment:7 Changed 4 years ago by karsten

Resolution: fixed
Status: merge_readyclosed

Great, thanks for looking! Squashed that fixup-commit and pushed to master. Closing.

Note: See TracTickets for help on using tickets.