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

Re: [tor-bugs] #24629 [Core Tor/Tor]: Activate osx builds on travis, at low priority



#24629: Activate osx builds on travis, at low priority
-------------------------------------------------+-------------------------
 Reporter:  teor                                 |          Owner:  teor
     Type:  enhancement                          |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.5.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  not-just-linux, tor-ci, teor-was-    |  Actual Points:
  assigned, 034-triage-20180328,                 |
  034-removed-20180328, 034-backport,            |
  035-removed-20180711, fast-fix                 |
Parent ID:                                       |         Points:  0.5
 Reviewer:  catalyst                             |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by Hello71):

 Replying to [comment:29 catalyst]:

 (I think these comments should go in #26038, but I'm responding to them
 here because you made them here.)

 > Please explicitly delete the `CARGO_HOME` settings from
 src/rust/tor_rust/include.am and src/test/test_rust.sh instead of merging
 the changes from #26038.  There are other changes in #26038 that I think
 we don't want, like:
 > * deleting the `registry` setting in `[source.crates-io]` in
 src/rust/.cargo/config.in; I'm not sure whether this breaks anything, but
 I'd rather play it safe and leave it there until we know why we should
 delete it.

 I believe cargo vendor used to print this line a long time ago, and now it
 has been the default for some time, so they stopped printing it. I could
 be remembering wrong though.

 > * warning if `CARGO_HOME` is set; I'm not sure the added complexity is
 worth it, especially given that the checks are added in at least four
 places.

 This is not correct. My patch only checks if CARGO_HOME is set once. It is
 supposed to check separately if CARGO_HOME is a relative path at use time,
 on the basis that the user may have changed their CARGO_HOME since they
 used configure. However, it does this wrong, since it should actually
 error instead of just warn, and also there is more than one place that
 CARGO_HOME is used. I think I do agree that checking for relative path can
 be done a single time in configure with AC_ARG_VAR though.

 > * explicit setting of `target-dir` in `[build]` in
 src/rust/.cargo/config.in; this seems redundant with the explicit setting
 of `CARGO_TARGET_DIR` in various cargo invocations.
 >
 > We could use `target-dir` in the cargo config instead of explicitly
 setting `CARGO_TARGET_DIR` for each cargo invocation, but in that case we
 should make a comment that it assumes that cargo is always invoked from a
 specific directory (which it should be after this change set).

 This is also not correct, although it is my fault for writing a poor
 comment. Paths in cargo configuration are interpreted relative to the
 parent of the .cargo directory, so as long as the correct cargo config is
 read, the target will always be set to the same place. Therefore,
 "./target" is equivalent to writing "@abs_top_builddir@/src/rust/target".
 Whether it is read or not, however, is unrelated to CARGO_TARGET_DIR, but
 in fact the CARGO_HOME. I assume however that if you can set CARGO_HOME
 correctly then you can also set the current directory correctly.

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