[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #32962 [Core Tor/Tor]: add_c_file: bug when adding to end of include.am
#32962: add_c_file: bug when adding to end of include.am
--------------------------+------------------------------------
Reporter: nickm | Owner: nickm
Type: defect | Status: merge_ready
Priority: Medium | Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor | Version:
Severity: Normal | Resolution:
Keywords: 043-should | Actual Points: .3
Parent ID: | Points: .1
Reviewer: catalyst | Sponsor:
--------------------------+------------------------------------
Changes (by catalyst):
* status: needs_review => merge_ready
Comment:
Replying to [comment:5 teor]:
> I found the path handling in this script really hard to understand. So I
created a PR that distinguishes `tor`-relative (topdir, renamed to tordir)
and `tor/src`-relative (srcdir) paths. I also made the canonicalisation
more reliable.
>
> What do you think?
> * master: https://github.com/torproject/tor/pull/1667
>
> I had to rebase to avoid conflicts with the include.am split from
#32137.
Thanks! This is a lot easier to understand now. Looks good to me.
Nits:
* Maybe we should avoid using `assert()` for invalid user input? I prefer
to reserve assertions for internal inconsistencies. On the other hand,
this is a maintainer tool, and I suspect it's not especially confusing if
there's an assertion failure?
* I ran this in a build directory, whereupon it created the files but
couldn't find the include.am to modify. Maybe we should also look for a
`src/Makefile.am`?
These are fairly minor and I'm also ok with merging this as is.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/32962#comment:6>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs