[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

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



#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:  TorBrowserTeam201912      |  Actual Points:
Parent ID:                            |         Points:
 Reviewer:  gk                        |        Sponsor:
--------------------------------------+--------------------------------

Comment (by JeremyRand):

 > a) In gobtcd: Why is there build_go_lib_pre defined? What does that do?
 Building ncprop279 without it works perfectly fine for me.

 IIRC this is a historical relic that exists due to complexities involved
 in having Namecoin maintain a fork of btcd with minimal diff against
 upstream (meaning that a bunch of imports refer to upstream btcd's
 namespace).  I think it's probably no longer needed because it was
 introduced before the `gobtcd`/`gobtcd2` split, and `gobtcd2` now has the
 same effect as this hack since it installs to upstream's namespace.  I'll
 test to make sure that this hack is no longer needed, and remove it if
 testing confirms that it's okay to remove.

 > b) Why is gobtcd and gobtcd2 split? Is that because gobtcd depends on
 gobtcutil which depends on gobtcd2? How does the compilation of that work
 outside tor-browser-build as I can't imagine that this circular dependency
 is not an issue there as well? If you can't work around that (i.e. have
 only one gobtcd project) then please add a comment, probably in the
 gobtcd2 project, explaining the problem.

 `gobtcd` builds the `btcjson` and `rpcclient` subpackages of `btcd`.
 `gobtcd2` builds the `btcec`, `chaincfg`, `chaincfg/chainhash`, and `wire`
 subpackages of `btcd`.  The former set depends on another project
 (probably `gobtcutil` as you say), and that project depends on the latter
 set.  It's not a circular dependency in terms of Go packages (so it works
 fine outside of rbm since `go get` operates on the level of Go packages,
 not Git repos), but it is a circular dependency in terms of Git repos
 (meaning that it causes problems in rbm since rbm uses 1 project per Git
 repo).  I'll add a comment explaining this.

 > c) gowinsvc is not needed, please remove it.

 No objection.

 > d) ncdns: os_go_lib_deps is empty, please remove it and the respective
 for-loop in the build script.

 No objection.

 > e) ncdns: +#mkdir -p /var/tmp/build etc. is commented out but not
 needed, please remove it.

 No objection.

 > f) ncdns: what is [snip] doing at that place in the build script as you
 are not building after it anymore? The same goes for +# Build as library.

 Good point, I think I forgot to remove that while I was removing the
 conditional support for building ncdns as an executable.  I'll remove
 that.

 > g) ncdns: the goxlog dependency does not seem to be needed, please
 remove it.

 Interesting; this was definitely required when building ncdns as an
 executable with TLSA support enabled.  I'll check to make sure it can be
 safely removed when building ncdns as a library with TLSA support
 disabled, and remove it if that still works.  (I'll also fix that in
 Namecoin's version of the rbm scripts, where the conditional vars are
 still a thing.)

 > h) ncprop279: there is a bunch of comments in the build script which are
 not needed, starting with +#mkdir -p /var/tmp/build; please remove them.

 No idea how those got left in there; I'll remove them.

 > i) ncprop279: what does [snip] do?

 The `linux-x86_64` conditional is to handle the Go compiler putting
 binaries into a different folder depending on whether they're cross-
 compiled.  In this case, `linux-i686` binaries are considered cross-
 compiled.  I think the `ls` was left there by mistake; I'll remove that
 line.

 > j) +export CGO_ENABLED=[% c("var/cgo") %] <- no need for having that
 exported as cgo seems to be 0. Thus, you can remove all the cgo related
 things.

 IIRC the Go compiler defaults to CGO_ENABLED=1 in some cases, which I
 think includes linux targets in `tor-browser-build`.  The intent of that
 line is to make sure cgo stays disabled even if the default would be to
 enable it.  No objection to removing the variable though, since it will
 always be 0 here.

 I'll push an updated branch later today.

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