Re: [tor-bugs] #30558 [Applications/Tor Browser]: Namecoin support for onion sites in Tor Browser

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#30558: Namecoin support for onion sites in Tor Browser
--------------------------------------+--------------------------------
 Reporter:  arthuredelstein           |          Owner:  JeremyRand
     Type:  defect                    |         Status:  needs_revision
 Priority:  Medium                    |      Milestone:
Component:  Applications/Tor Browser  |        Version:
 Severity:  Normal                    |     Resolution:
 Keywords:  TorBrowserTeam201911      |  Actual Points:
Parent ID:                            |         Points:
 Reviewer:                            |        Sponsor:
--------------------------------------+--------------------------------
Changes (by gk):

 * keywords:  TorBrowserTeam201911R => TorBrowserTeam201911
 * status:  needs_review => needs_revision


Comment:

 Okay, let's start. Alas, I don't have time to review everything at once.
 My current plan is to go commit by commit and let you fix up things as we
 go. I hope this works for you.

 1d46514c6be75dadcf9201961b1252edafcfa443:
 {{{
 +    "proxy": "socks5:127.0.0.1:9150::",
 }}}
 I guess that hard-coding is done by taking the Tor Browser default values?
 One thing to keep in mind is that not every user is using 9150 as the
 SOCKS port. I don't know how hard it is to make this more dynamic but it
 would be nice if it were possible (but it's not necessary for a nightly
 inclusion).
 {{{
 +    - patch
 }}}
 Are you sure this is needed? We are applying a lot of other patches
 without requiring `patch` being explicitly installed (because it seems to
 come at least in newer OSes installed by default).
 {{{
 +  #- name: electrum-nmc
 +  #  project: electrum-nmc
 +  #  enable: '[% c("var/namecoin") %]'
 +  #- name: ncprop279
 +  #  project: ncprop279
 +  #  enable: '[% c("var/namecoin") %]'
 +  #- name: stemns
 +  #  project: stemns
 +  #  enable: '[% c("var/namecoin") %]'
 }}}
 Please remove those comments here and just include those projects when it
 is time.
 {{{
 ++        python3 TorBrowser/StemNS/poc.py &
 }}}
 I think we can't require users just having `python3` available yet. Please
 add a check and somewhere so users get a notice when they need to install
 `python3`.
 {{{
 +      namecoin: '[% c("var/nightly") %]'
 }}}
 Please add a `namecoin: 0` to all the other platforms given that you are
 not only checking for Namecoin support when dealing with Linux.

 Some general remarks:
 1) We try to make it possible to bisect issues in `tor-browser-build` in
 the sense that resulting builds are still running. It's hard sometimes and
 sometimes even not really possible, but that should be the goal anyways.
 With that in mind I think the commit I looked at above should be (to a
 large extend (that is without the changes in `rbm.conf`)) the *last* in
 your series of patches. Usually one is adding commits for all the projects
 and then in the final commit(s) one is "activating" those by getting them
 actually build once one sets the proper flags/or builds for the respective
 platforms/architectures. It would be nice if you could follow this general
 idea.

 2) Please don't use the submodule approach you had in mind first and then
 later on discard it in favor of including all the needed projects
 directly. Just include those projects directly. This makes the patch set
 smaller, is easier to review, and is the right thing to do anyway.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/30558#comment:16>
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