Opened 9 months ago

Closed 5 months ago

#28806 closed defect (fixed)

checkIncludes.py does not like code in src/ext/timeouts

Reported by: rl1987 Owned by: nickm
Priority: Low Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: asn-merge
Cc: Actual Points: 0.1
Parent ID: Points: 0.1
Reviewer: cohosh Sponsor: Sponsor31-can

Description

At the end of make check, we can see:

python3 ./scripts/maint/checkIncludes.py
Unusual pattern timeout-bitops.c in src/ext/timeouts
Unusual pattern timeout-debug.h in src/ext/timeouts
Unusual pattern timeout.h in src/ext/timeouts

For example, see: https://travis-ci.org/torproject/tor/jobs/465497113

Child Tickets

Change History (8)

comment:1 Changed 9 months ago by rl1987

Priority: MediumLow

These are just warnings, but it seems mentioned lines of src/ext/timeouts/.may_include do not match any of the following regular expressions:

 54 ALLOWED_PATTERNS = [                                                                              
 55     re.compile(r'^.*\*\.(h|inc)$'),                                                               
 56     re.compile(r'^.*/.*\.h$'),                                                                    
 57     re.compile(r'^ext/.*\.c$'),                                                                   
 58     re.compile(r'^orconfig.h$'),                                                                  
 59     re.compile(r'^micro-revision.i$'),                                                            
 60 ]  

What should we do? Add one or more entries to this list? I would like to hear from Nick or someone else who worked on checkIncludes script.

comment:2 Changed 9 months ago by nickm

Let's clean up the includes in timeouts, so that checkIncludes.py can make sure we haven't introduced any circularity.

comment:3 Changed 6 months ago by nickm

Owner: set to nickm
Status: newaccepted

comment:4 Changed 6 months ago by nickm

Actual Points: 0
Milestone: Tor: unspecifiedTor: 0.4.1.x-final
Status: acceptedneeds_review

See branch ticket28806; PR at https://github.com/torproject/tor/pull/866

I suggest no backport here, since this is a completely cosmetic change.

comment:5 Changed 6 months ago by asn

Reviewer: cohosh

comment:6 Changed 5 months ago by cohosh

Status: needs_reviewmerge_ready

Looks good!

comment:7 Changed 5 months ago by nickm

Keywords: asn-merge added

comment:8 Changed 5 months ago by teor

Actual Points: 00.1
Points: 0.1
Resolution: fixed
Sponsor: Sponsor31-can
Status: merge_readyclosed

Looks like refactoring/modularity support to me, so it can be Sponsor 31.

This change doesn't have a changes file, please add one later if you'd like one.

Merged to master.

Merged #29121, #30004 to 0.4.0, and #29926, #28806 to master, then merged forward.

Note: See TracTickets for help on using tickets.