Opened 22 months ago

Closed 20 months ago

Last modified 20 months ago

#18331 closed task (fixed)

Update OS X toolchain to work with ESR 45

Reported by: gk Owned by: boklm
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Major Keywords: tbb-gitian, ff45-esr, TorBrowserTeam201604R
Cc: mcs, arlolra, boklm Actual Points:
Parent ID: #18226 Points:
Reviewer: Sponsor:

Description (last modified by gk)

It seems our cross-compiler crashes while trying to compile ESR 45:

   x86_64-apple-darwin10-clang++	 ...  /home/ubuntu/build/tor-browser/intl/icu/source/common/umutex.cpp
x86_64-apple-darwin10-clang: SemaChecking.cpp:4431: void {anonymous}::DiagnoseOutOfRangeComparison(clang::Sema&, clang::BinaryOperator*, clang::Expr*, clang::Expr*, llvm::APSInt, bool): Assertion `(OtherT->isIntegerType() && ConstantT->isIntegerType()) && "comparison with non-integer type"' failed.
0  x86_64-apple-darwin10-clang 0x00000000022b6392 llvm::sys::PrintStackTrace(_IO_FILE*) + 34
1  x86_64-apple-darwin10-clang 0x00000000022b67c9
2  libpthread.so.0             0x00002b2aba39bcb0
3  libc.so.6                   0x00002b2abb0700d5 gsignal + 53
4  libc.so.6                   0x00002b2abb07383b abort + 379
5  libc.so.6                   0x00002b2abb068d9e
6  libc.so.6                   0x00002b2abb068e42
7  x86_64-apple-darwin10-clang 0x0000000000b2a7d9
8  x86_64-apple-darwin10-clang 0x0000000000b3899e
9  x86_64-apple-darwin10-clang 0x0000000000b38d61 clang::Sema::CheckCompletedExpr(clang::Expr*, clang::SourceLocation, bool) + 33
10 x86_64-apple-darwin10-clang 0x0000000000c8f06a clang::Sema::ActOnFinishFullExpr(clang::Expr*, clang::SourceLocation, bool, bool) + 250
11 x86_64-apple-darwin10-clang 0x0000000000ad5cc5 clang::Parser::ParseWhileStatement(clang::SourceLocation*) + 469
12 x86_64-apple-darwin10-clang 0x0000000000ad108c clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*, clang::Parser::ParsedAttributesWithRange&) + 1036
13 x86_64-apple-darwin10-clang 0x0000000000ad159e clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*) + 142
14 x86_64-apple-darwin10-clang 0x0000000000ad2217 clang::Parser::ParseCompoundStatementBody(bool) + 1607
15 x86_64-apple-darwin10-clang 0x0000000000ad27dd clang::Parser::ParseCompoundStatement(bool, unsigned int) + 45
16 x86_64-apple-darwin10-clang 0x0000000000ad2832 clang::Parser::ParseCompoundStatement(bool) + 18
17 x86_64-apple-darwin10-clang 0x0000000000ad11fa clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*, clang::Parser::ParsedAttributesWithRange&) + 1402
18 x86_64-apple-darwin10-clang 0x0000000000ad159e clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*) + 142
19 x86_64-apple-darwin10-clang 0x0000000000ad7216 clang::Parser::ParseIfStatement(clang::SourceLocation*) + 1030
20 x86_64-apple-darwin10-clang 0x0000000000ad138b clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*, clang::Parser::ParsedAttributesWithRange&) + 1803
21 x86_64-apple-darwin10-clang 0x0000000000ad159e clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*) + 142
22 x86_64-apple-darwin10-clang 0x0000000000ad2217 clang::Parser::ParseCompoundStatementBody(bool) + 1607
23 x86_64-apple-darwin10-clang 0x0000000000ad26c3 clang::Parser::ParseFunctionStatementBody(clang::Decl*, clang::Parser::ParseScope&) + 211
24 x86_64-apple-darwin10-clang 0x0000000000a754cb clang::Parser::ParseFunctionDefinition(clang::ParsingDeclarator&, clang::Parser::ParsedTemplateInfo const&, clang::Parser::LateParsedAttrList*) + 2635
25 x86_64-apple-darwin10-clang 0x0000000000a8153e clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, unsigned int, bool, clang::SourceLocation*, clang::Parser::ForRangeInit*) + 1902
26 x86_64-apple-darwin10-clang 0x0000000000a70625 clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec&, clang::AccessSpecifier) + 245
27 x86_64-apple-darwin10-clang 0x0000000000a70d68 clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*, clang::AccessSpecifier) + 1016
28 x86_64-apple-darwin10-clang 0x0000000000a72280 clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) + 96
29 x86_64-apple-darwin10-clang 0x0000000000a932dd clang::Parser::ParseInnerNamespace(std::vector<clang::SourceLocation, std::allocator<clang::SourceLocation> >&, std::vector<clang::IdentifierInfo*, std::allocator<clang::IdentifierInfo*> >&, std::vector<clang::SourceLocation, std::allocator<clang::SourceLocation> >&, unsigned int, clang::SourceLocation&, clang::ParsedAttributes&, clang::BalancedDelimiterTracker&) + 381
30 x86_64-apple-darwin10-clang 0x0000000000a93e1e clang::Parser::ParseNamespace(unsigned int, clang::SourceLocation&, clang::SourceLocation) + 2670
31 x86_64-apple-darwin10-clang 0x0000000000a87d0e clang::Parser::ParseDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, unsigned int, clang::SourceLocation&, clang::Parser::ParsedAttributesWithRange&) + 558
32 x86_64-apple-darwin10-clang 0x0000000000a7241b clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) + 507
33 x86_64-apple-darwin10-clang 0x0000000000a98f2b clang::Parser::ParseLinkage(clang::ParsingDeclSpec&, unsigned int) + 1067
34 x86_64-apple-darwin10-clang 0x0000000000a70684 clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec&, clang::AccessSpecifier) + 340
35 x86_64-apple-darwin10-clang 0x0000000000a70d68 clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*, clang::AccessSpecifier) + 1016
36 x86_64-apple-darwin10-clang 0x0000000000a72280 clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) + 96
37 x86_64-apple-darwin10-clang 0x0000000000a729b8 clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&) + 184
38 x86_64-apple-darwin10-clang 0x0000000000a6c00d clang::ParseAST(clang::Sema&, bool, bool) + 493
39 x86_64-apple-darwin10-clang 0x00000000007650ea clang::FrontendAction::Execute() + 282
40 x86_64-apple-darwin10-clang 0x0000000000748ddd clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 349
41 x86_64-apple-darwin10-clang 0x000000000072fb52 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) + 1666
42 x86_64-apple-darwin10-clang 0x0000000000728c18 cc1_main(char const**, char const**, char const*, void*) + 1224
43 x86_64-apple-darwin10-clang 0x000000000070d9e1 main + 705
44 libc.so.6                   0x00002b2abb05b76d __libc_start_main + 237
45 x86_64-apple-darwin10-clang 0x00000000007285dd
Stack dump:
0.	Program arguments: /home/ubuntu/build/x-tools/x86_64-apple-darwin10/bin/x86_64-apple-darwin10-clang -cc1 -triple x86_64-apple-macosx10.6.0 -isysroot /home/ubuntu/build/MacOSX10.7.sdk -emit-obj -disable-free -main-file-name umutex.cpp -mrelocation-model pic -pic-level 2 -mdisable-fp-elim -relaxed-aliasing -masm-verbose -munwind-tables -target-cpu core2 -target-linker-version 2.22 -coverage-file /home/ubuntu/build/tor-browser/obj-macos/intl/icu/target/common/umutex.o -resource-dir /home/ubuntu/build/x-tools/x86_64-apple-darwin10/bin/../lib/clang/3.3 -dependency-file umutex.d -MT umutex.d umutex.o umutex.o -isysroot /home/ubuntu/build/MacOSX10.7.sdk -D U_ATTRIBUTE_DEPRECATED= -D U_COMMON_IMPLEMENTATION -D U_USING_ICU_NAMESPACE=0 -D U_NO_DEFAULT_INCLUDE_UTF_HEADERS=1 -D UCONFIG_NO_LEGACY_CONVERSION -D UCONFIG_NO_TRANSLITERATION -D UCONFIG_NO_REGULAR_EXPRESSIONS -D UCONFIG_NO_BREAK_ITERATION -D U_CHARSET_IS_UTF8 -D U_HAVE_ATOMIC=0 -D DEFAULT_ICU_PLUGINS="/usr/local/lib/icu"  -D NO_X11 -U DEBUG -D NDEBUG -D U_STATIC_IMPLEMENTATION -I /home/ubuntu/build/tor-browser/intl/icu/source/common -I /home/ubuntu/build/tor-browser/intl/icu/source/i18n -I /home/ubuntu/build/tor-browser/intl/icu/source/common -I/home/ubuntu/build/x-tools/x86_64-apple-darwin10/lib/gcc/x86_64-apple-darwin10/5666.3/../../../../x86_64-apple-darwin10/include/c++/5666.3 -I/home/ubuntu/build/x-tools/x86_64-apple-darwin10/lib/gcc/x86_64-apple-darwin10/5666.3/../../../../x86_64-apple-darwin10/include/c++/5666.3/x86_64-apple-darwin10 -I/home/ubuntu/build/x-tools/x86_64-apple-darwin10/lib/gcc/x86_64-apple-darwin10/5666.3/../../../../x86_64-apple-darwin10/include/c++/5666.3/backward -I/home/ubuntu/build/x-tools/x86_64-apple-darwin10/lib/gcc/x86_64-apple-darwin10/5666.3/include -I/home/ubuntu/build/x-tools/x86_64-apple-darwin10/lib/gcc/x86_64-apple-darwin10/5666.3/include-fixed -I/home/ubuntu/build/x-tools/x86_64-apple-darwin10/lib/gcc/x86_64-apple-darwin10/5666.3/../../../../x86_64-apple-darwin10/include -O3 -Wall -Wempty-body -Woverloaded-virtual -Wsign-compare -Wwrite-strings -Wno-invalid-offsetof -Wno-c++0x-extensions -Wno-extended-offsetof -Wno-unknown-warning-option -Wno-return-type-c-linkage -std=gnu++0x -fconst-strings -fdeprecated-macro -fdebug-compilation-dir /home/ubuntu/build/tor-browser/obj-macos/intl/icu/target/common -ferror-limit 19 -fmessage-length 0 -fvisibility hidden -pthread -stack-protector 1 -mstackrealign -fblocks -fobjc-runtime=macosx-10.6.0 -fobjc-dispatch-method=mixed -fobjc-default-synthesize-properties -fencode-extended-block-signature -fno-common -fdiagnostics-show-option -backend-option -vectorize-loops -o umutex.o -x c++ /home/ubuntu/build/tor-browser/intl/icu/source/common/umutex.cpp 
1.	/home/ubuntu/build/tor-browser/intl/icu/source/common/umutex.cpp:273:33: current parser token '{'
2.	/home/ubuntu/build/tor-browser/intl/icu/source/common/umutex.cpp:250:1 <Spelling=/home/ubuntu/build/tor-browser/intl/icu/source/common/unicode/uversion.h:117:45>: parsing namespace 'icu_56'
3.	/home/ubuntu/build/tor-browser/intl/icu/source/common/umutex.cpp:265:38: parsing function body 'umtx_initImplPreInit'
4.	/home/ubuntu/build/tor-browser/intl/icu/source/common/umutex.cpp:265:38: in compound statement ('{}')
5.	/home/ubuntu/build/tor-browser/intl/icu/source/common/umutex.cpp:272:12: in compound statement ('{}')
x86_64-apple-darwin10-clang: error: unable to execute command: Aborted
x86_64-apple-darwin10-clang: error: clang frontend command failed due to signal (use -v to see invocation)
clang version 3.3 (tags/RELEASE_33/final)
Target: x86_64-apple-darwin10
Thread model: posix
x86_64-apple-darwin10-clang: note: diagnostic msg: PLEASE submit a bug report to http://llvm.org/bugs/ and include the crash backtrace, preprocessed source, and associated run script.
x86_64-apple-darwin10-clang: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
x86_64-apple-darwin10-clang: note: diagnostic msg: /tmp/umutex-VR5zgw.cpp
x86_64-apple-darwin10-clang: note: diagnostic msg: /tmp/umutex-VR5zgw.sh
x86_64-apple-darwin10-clang: note: diagnostic msg: 

********************
*** Failed compilation command follows: ----------------------------------------------------------
/home/ubuntu/build/x-tools/x86_64-apple-darwin10/bin/x86_64-apple-darwin10-clang++ -arch x86_64 -isysroot /home/ubuntu/build/MacOSX10.7.sdk -DU_ATTRIBUTE_DEPRECATED= -DU_COMMON_IMPLEMENTATION -DU_USING_ICU_NAMESPACE=0 -DU_NO_DEFAULT_INCLUDE_UTF_HEADERS=1 -DUCONFIG_NO_LEGACY_CONVERSION -DUCONFIG_NO_TRANSLITERATION -DUCONFIG_NO_REGULAR_EXPRESSIONS -DUCONFIG_NO_BREAK_ITERATION -DU_CHARSET_IS_UTF8 -I/home/ubuntu/build/tor-browser/intl/icu/source/common -I/home/ubuntu/build/tor-browser/intl/icu/source/i18n -Qunused-arguments -DU_HAVE_ATOMIC=0 -I/home/ubuntu/build/tor-browser/intl/icu/source/common -DDEFAULT_ICU_PLUGINS="/usr/local/lib/icu"  -fPIC -Qunused-arguments -Wall -Wempty-body -Woverloaded-virtual -Wsign-compare -Wwrite-strings -Wno-invalid-offsetof -Wno-c++0x-extensions -Wno-extended-offsetof -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-exceptions -fno-strict-aliasing -frtti -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -DNO_X11 -pipe -UDEBUG -DNDEBUG -O3 -DU_STATIC_IMPLEMENTATION -fvisibility=hidden -fvisibility=hidden -fno-common -c -dynamic -MMD -MT umutex.d umutex.o umutex.o -o umutex.o /home/ubuntu/build/tor-browser/intl/icu/source/common/umutex.cpp
--- ( rebuild with "make VERBOSE=1 all" to show all parameters ) --------
make[7]: *** [umutex.o] Error 1

Trying to not compile ICU (using --without-intl-api) does not help. For some reason umutex.ccp still gets compiled.

Child Tickets

Attachments (1)

0001-Bug-18331-install-sudo-in-Debian-VMs-with-KVM-too.patch (809 bytes) - added by boklm 21 months ago.

Download all attachments as: .zip

Change History (25)

comment:1 Changed 22 months ago by gk

Description: modified (diff)

comment:2 Changed 22 months ago by mcs

Cc: mcs added

comment:3 Changed 22 months ago by gk

Testing with an experimental toolchain using clang 3.4.2 provided by Ray hits the same internal compiler error.

comment:4 Changed 22 months ago by gk

This actually seems to be https://llvm.org/bugs/show_bug.cgi?id=23638 which got fixed in r238083. The ICU patch responsible for this is https://bugs.icu-project.org/trac/changeset/37326. Hm, I wonder what Mozilla is doing here to avoid this problem...

comment:5 Changed 22 months ago by gk

The ICU code causing this made it into version 56 which got into Firefox 45 via: https://bugzilla.mozilla.org/show_bug.cgi?id=1210165.

comment:6 Changed 22 months ago by gk

I've been running a build with https://bugs.icu-project.org/trac/changeset/37326 backed out and that works fine it seems. Might be an option if we can't find a better solution in time.

comment:7 in reply to:  4 Changed 22 months ago by gk

Replying to gk:

This actually seems to be https://llvm.org/bugs/show_bug.cgi?id=23638 which got fixed in r238083. The ICU patch responsible for this is https://bugs.icu-project.org/trac/changeset/37326. Hm, I wonder what Mozilla is doing here to avoid this problem...

They updated clang to r247539 in https://bugzilla.mozilla.org/show_bug.cgi?id=1209930.

comment:8 Changed 21 months ago by arlolra

Cc: arlolra added

comment:9 Changed 21 months ago by boklm

Cc: boklm added

comment:10 Changed 21 months ago by gk

Keywords: TorBrowserTeam201603R added
Status: newneeds_review

Alright. bug_18331 (https://gitweb.torproject.org/user/gk/tor-browser.git/commit/?h=bug_18331&id=224788da1289b8084265cc7030bcf48c23246b4d) in my tor-browser repo has the .mozconfig changes. bug_18331_chicken_v2 (https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_18331_chicken_v2&id=b3f02ae85f9d303902ceb3752d16b73902965ada) in my tor-browser-bundle repo has the Gitian changes for review. They are producing a working .dmg file (tested on an old OS X 10.6 machine). You have to point the TORBROWSER_TAG to my bug_18331, though, and take care of #14970 manually while testing :).

That said I have a more elaborate patch in the pipeline (I am doing some final testing) which builds our own clang and fixes #18690 as well while we are at it. Chances are that it will finally fix #9711, too (currently cctools does not want as I would like). I think, even if the latter does not hold, we should try that one for Tor Browser, if possible.

Last edited 21 months ago by gk (previous) (diff)

comment:11 Changed 21 months ago by gk

Here comes the thing that I actually would like to see getting used. The tor-browser commit is the same as in comment:10. The tor-browser-bundle branch is in my public repo: bug_18331_v4 (https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_18331_v4&id=128bcfb6f781adb8cfaff2815a191432ce0d62b5).

comment:12 Changed 21 months ago by gk

Keywords: TorBrowserTeam201604R added; TorBrowserTeam201603R removed

No easy way to do reviews in March anymore.

comment:13 Changed 21 months ago by boklm

I tried to build the bug_18331_v4 branch, using KVM, and had an error building gitian-utils.yml because sudo is not found. sudo was installed by default on Ubuntu, but is not on Debian. After looking at gitian-builder/bin/make-base-vm, the sudo package is added for Debian images, but only for LXC builds. I am attaching a patch to gitian-builder that fix that.

After fixing the sudo issue, I had an other error caused by dpkg not finding ldconfig. Adding /usr/sbin and /sbin in the PATH before all dpkg commands fixed the issue:

diff --git a/gitian/descriptors/mac/gitian-pluggable-transports.yml b/gitian/descriptors/mac/gitian-pluggable-transports.yml
index e2b51c4..165ab67 100644
--- a/gitian/descriptors/mac/gitian-pluggable-transports.yml
+++ b/gitian/descriptors/mac/gitian-pluggable-transports.yml
@@ -79,6 +79,7 @@ script: |
   mkdir -p $PTDIR/
   mkdir -p $OUTDIR/
   #
+  export PATH="/usr/sbin:/sbin:$PATH"
   sudo dpkg -i *.deb
   tar xaf multiarch-darwin*tar.xz
   export PATH="$PATH:$HOME/build/apple-osx/bin/"
diff --git a/gitian/descriptors/mac/gitian-tor.yml b/gitian/descriptors/mac/gitian-tor.yml
index 94d3eb1..10d321d 100644
--- a/gitian/descriptors/mac/gitian-tor.yml
+++ b/gitian/descriptors/mac/gitian-tor.yml
@@ -43,6 +43,7 @@ script: |
   mkdir -p $TORCONFIGDIR/
   mkdir -p $OUTDIR/
   #
+  export PATH="/usr/sbin:/sbin:$PATH"
   sudo dpkg -i *.deb
   tar xaf multiarch-darwin*tar.xz
   export PATH="$PATH:$HOME/build/apple-osx/bin/"
diff --git a/gitian/descriptors/mac/gitian-utils.yml b/gitian/descriptors/mac/gitian-utils.yml
index 5288471..a22e5cf 100644
--- a/gitian/descriptors/mac/gitian-utils.yml
+++ b/gitian/descriptors/mac/gitian-utils.yml
@@ -71,6 +71,7 @@ script: |
     export FAKETIME=$REFERENCE_DATETIME
     cd ..
 
+    export PATH="/usr/sbin:/sbin:$PATH"
     sudo dpkg -i *.deb
     tar xaf multiarch-darwin*tar.xz
     export PATH="$PATH:$HOME/build/apple-osx/bin/"

After fixing this, I now have an other error while building gitian-tor.yml which I did not look at yet:

configure.ac:19: installing `./config.sub'
configure.ac:14: installing `./install-sh'
configure.ac:14: installing `./missing'
Makefile.am: installing `./depcomp'
+ find -type f -print0
+ xargs -0 touch '--date=2000-01-01 00:00:00'
+ ./configure --enable-static-openssl --disable-asciidoc --host=i686-apple-darwin11 --with-libevent-dir=/home/debian/install/libevent --with-openssl-dir=/home/debian/install/openssl --prefix=/home/debian/install
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... configure: error: newly created file is older than distributed files!
Check your system clock

comment:14 Changed 21 months ago by gk

Status: needs_reviewneeds_revision

comment:15 Changed 21 months ago by gk

Owner: changed from tbb-team to boklm
Status: needs_revisionassigned

comment:16 Changed 21 months ago by boklm

Status: assignedneeds_review

The error in gitian-tor.yml was caused by faketime. I removed faketime from mac/gitian-tor.yml and made builds on different days, always getting the same results, so it looks like faketime is no longer required in mac/gitian-tor.yml to make the build reproducible.

In the bug_18331_v5 branch in my user repo:
https://gitweb.torproject.org/user/boklm/tor-browser-bundle.git/log/?h=bug_18331_v5
I added two commits:

  • a fixup commit (to be squashed before merging) to add /usr/sbin and /sbin to the PATH
  • a commit to remove faketime from mac/gitian-tor.yml

comment:17 Changed 21 months ago by gk

Hm.. Are we really able to get rid of faketime here? Back then when I cleaned up the respective Linux descriptor I had the same idea but, after testing, realized that we still need it for zipping up the results. I just re-run the same tests on an LXC box and again preloading libfaketime is needed. I actually don't see a difference in this regard in the OS X related descriptor. Thus, I am inclined to do the same here. I can probably test whether that really matters later.

Regarding the fixup I wonder whether that is a gitian-builder issue as well and should be fixed there, too, (like the sudo thing). I mean both KVM and LXC are using clean Debian Wheezy VMs and there should be no reason this descriptor fixup is needed for KVM but not for LXC.

comment:18 in reply to:  17 ; Changed 21 months ago by boklm

Replying to gk:

Hm.. Are we really able to get rid of faketime here? Back then when I cleaned up the respective Linux descriptor I had the same idea but, after testing, realized that we still need it for zipping up the results. I just re-run the same tests on an LXC box and again preloading libfaketime is needed. I actually don't see a difference in this regard in the OS X related descriptor. Thus, I am inclined to do the same here. I can probably test whether that really matters later.

Are you talking about the tor-mac64-gbuilt.zip, or the final .mar and .dmg files?

I checked that the .mar and .dmg files we create are reproducible, but indeed the intermediary tor-mac64-gbuilt.zip includes timestamps.

We can maybe add a find $@ -exec touch --date="$REFERENCE_DATETIME" {} \; in build-helpers/dzip.sh to get rid of those timestamps without relying on faketime.

Regarding the fixup I wonder whether that is a gitian-builder issue as well and should be fixed there, too, (like the sudo thing). I mean both KVM and LXC are using clean Debian Wheezy VMs and there should be no reason this descriptor fixup is needed for KVM but not for LXC.

I think the reason is that on Debian wheezy, /sbin:/usr/sbin is not added to the PATH, except for login shells. In the case of LXC, the commands are run using something like lxc-execute -- sudo -u $TUSER -i -- [command], with sudo's -i option to use a login shell. In the case of KVM, the commands are run using ssh $TUSER@localhost [command] which doesn't use a login shell. I'm not sure how to change the ssh command to make it use a login shell.

comment:19 in reply to:  18 ; Changed 20 months ago by gk

Status: needs_reviewneeds_revision

Replying to boklm:

Replying to gk:

Hm.. Are we really able to get rid of faketime here? Back then when I cleaned up the respective Linux descriptor I had the same idea but, after testing, realized that we still need it for zipping up the results. I just re-run the same tests on an LXC box and again preloading libfaketime is needed. I actually don't see a difference in this regard in the OS X related descriptor. Thus, I am inclined to do the same here. I can probably test whether that really matters later.

Are you talking about the tor-mac64-gbuilt.zip, or the final .mar and .dmg files?

I checked that the .mar and .dmg files we create are reproducible, but indeed the intermediary tor-mac64-gbuilt.zip includes timestamps.

Just the .zip file. Ah, end we are not exposing that one to the public. So, we might be fine here. Could you add that to the commit message to explain why we treat OS X differently?

We can maybe add a find $@ -exec touch --date="$REFERENCE_DATETIME" {} \; in build-helpers/dzip.sh to get rid of those timestamps without relying on faketime.

That might be a good idea, generally, but I think this should be a different ticket. Mind to file one?

Regarding the fixup I wonder whether that is a gitian-builder issue as well and should be fixed there, too, (like the sudo thing). I mean both KVM and LXC are using clean Debian Wheezy VMs and there should be no reason this descriptor fixup is needed for KVM but not for LXC.

I think the reason is that on Debian wheezy, /sbin:/usr/sbin is not added to the PATH, except for login shells. In the case of LXC, the commands are run using something like lxc-execute -- sudo -u $TUSER -i -- [command], with sudo's -i option to use a login shell. In the case of KVM, the commands are run using ssh $TUSER@localhost [command] which doesn't use a login shell. I'm not sure how to change the ssh command to make it use a login shell.

Okay, then let's leave this fix in tor-browser-bundle for now. Could you add a comment/hint why we need this as well?

Last edited 20 months ago by gk (previous) (diff)

comment:20 in reply to:  18 Changed 20 months ago by cypherpunks

Replying to boklm:

I think the reason is that on Debian wheezy, /sbin:/usr/sbin is not added to the PATH, except for login shells. [...]

No, is not about wheezy in particular, is Debian policy in general. And is not about login shells, is about privileged accounts: regular user accounts do not get sbin in PATH. See: https://www.debian.org/doc/manuals/debian-reference/ch01.en.html#_the_literal_path_literal_variable

And no, what makes sudo work is not "-i", is sudo itself: sudo will override the PATH with the value in "secure_path" (sane security measure). That "secure_path" does include sbin directories.

comment:21 in reply to:  19 Changed 20 months ago by boklm

Replying to gk:

We can maybe add a find $@ -exec touch --date="$REFERENCE_DATETIME" {} \; in build-helpers/dzip.sh to get rid of those timestamps without relying on faketime.

That might be a good idea, generally, but I think this should be a different ticket. Mind to file one?

I opened #18845 for that.

Okay, then let's leave this fix in tor-browser-bundle for now. Could you add a comment/hint why we need this as well?

I updated the commit messages for both commits and added a comment before setting the PATH in branch bug_18331_v6.

Replying to cypherpunks:

Replying to boklm:

I think the reason is that on Debian wheezy, /sbin:/usr/sbin is not added to the PATH, except for login shells. [...]

No, is not about wheezy in particular, is Debian policy in general.

Thanks for the correction.

comment:22 Changed 20 months ago by boklm

Status: needs_revisionneeds_review

comment:23 Changed 20 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Fixed with commit ae90ac520dbdf6b1267b099b654b58352ee86ef0 (tor-browser-builder-4 in gitian-builder) and commits 128bcfb6f781adb8cfaff2815a191432ce0d62b5, 8c71dad38b58aa138872d1f8827ea2813763ab1b and da14737254245cfa6b849dc8ac94a52cb3d9a61d on master in tor-browser-bundle.

comment:24 Changed 20 months ago by gk

Seems I forgot to push the tor-browser changes. This is fixed with commit 833436c0e36649d47df22567b23343c76985243a on tor-browser-45.0.2esr-6.x-1.

Note: See TracTickets for help on using tickets.