[kudu-CR] Do not run (g)addr2line translator on MacOS X

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: Do not run (g)addr2line translator on MacOS X
..


Patch Set 2: Code-Review+2

Ran the tests in my mac. When running on ctest all tests that injected failures 
were failing due to this. now they pass. lgtm (if jenkins agrees)

-- 
To view, visit http://gerrit.cloudera.org:8080/3360
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf8a81ae7f2e1b938734a4b66bec82cb731d90cc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Do not run (g)addr2line translator on MacOS X

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Do not run (g)addr2line translator on MacOS X
..


Patch Set 2:

> Thanks for making those changes. Looks good to me, though
 > ultimately I'll defer to someone who actually uses Mac OS.
 > 
 > If you do have some time, I'd love to know whether translation
 > works in sanitizer builds on Mac OS. No worries if you've got other
 > thing to look at, though.

Yep, sure -- I would like to get some feedback from Dan Burkert.  Yes, I'll try 
those sanitizer builds: I'm also curious how that works for those builds.

-- 
To view, visit http://gerrit.cloudera.org:8080/3360
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf8a81ae7f2e1b938734a4b66bec82cb731d90cc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Do not run (g)addr2line translator on MacOS X

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Do not run (g)addr2line translator on MacOS X
..


Patch Set 2: Code-Review+1

Thanks for making those changes. Looks good to me, though ultimately I'll defer 
to someone who actually uses Mac OS.

If you do have some time, I'd love to know whether translation works in 
sanitizer builds on Mac OS. No worries if you've got other thing to look at, 
though.

-- 
To view, visit http://gerrit.cloudera.org:8080/3360
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf8a81ae7f2e1b938734a4b66bec82cb731d90cc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Do not run (g)addr2line translator on MacOS X

[Alexey Serbin (Code Review)]

Hello Mike Percy, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/3360

to look at the new patch set (#2).

Change subject: Do not run (g)addr2line translator on MacOS X
..

Do not run (g)addr2line translator on MacOS X

When running tests on MacOS X, omit using the
stacktrace_addr2line.pl utility to translate addresses to line numbers.
For current builds on MacOS X, neither (g)addr2line nor atos
translate stack address into line number without extra-hassle.
This is so even for binaries linked without PIE
(see the -no_pie linker's option).

As for the extra-hassle part: there are at least two ways
to address that, but neither looks like a good candidate
in the context of the script.

  Option A:
Provide the atos utility with reference to running test process
using '-p ' option.  This, however, triggers a pop-up
requesting for password if it's run the very first time.

  Option B:
Take a snapshot of running test process using MacOS X tools
and provide the atos utility with the load address
using '-l ' option.

Change-Id: Idf8a81ae7f2e1b938734a4b66bec82cb731d90cc
---
M build-support/run-test.sh
1 file changed, 13 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/3360/2
-- 
To view, visit http://gerrit.cloudera.org:8080/3360
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idf8a81ae7f2e1b938734a4b66bec82cb731d90cc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Do not run (g)addr2line translator on MacOS X

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: Do not run (g)addr2line translator on MacOS X
..


Patch Set 1:

(2 comments)

> (2 comments)
 > 
 > My understanding is that sanitizer-enabled Kudu builds on Mac OS
 > will report correct line numbers since they use LLVM's built-in
 > symbolizer, so translation is only actually missing on the
 > addresses emitted by a CHECK() or equivalent crash. Is this
 > correct?

Thank you for review!

I haven't tried to run sanitizer-enabled builds.  Yes, as I understand the 
translation is needed for crash-related stacktrace printouts.  Anyways, it 
seems safe to remove addr2line translator since it does no do much on MacOS X 
anyways, just takes some to run.

http://gerrit.cloudera.org:8080/#/c/3360/1/build-support/run-test.sh
File build-support/run-test.sh:

PS1, Line 139: UNAME=$(uname -s)
 :   if [ "$UNAME" = "Darwin" ]; then
> How about using the bash built-in $OSTYPE? I think we typically use that in
Done


Line 143: # into line number.  The atos utility is able to do that only if
> Nit: can you fix this line's formatting a bit? Looks like you can squeeze a
Sure -- actually, I found these lines were too long (more than 80 chars).  I'll 
make them shorter and more uniformly filled.


-- 
To view, visit http://gerrit.cloudera.org:8080/3360
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf8a81ae7f2e1b938734a4b66bec82cb731d90cc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] Do not run (g)addr2line translator on MacOS X

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Do not run (g)addr2line translator on MacOS X
..


Patch Set 1:

(2 comments)

My understanding is that sanitizer-enabled Kudu builds on Mac OS will report 
correct line numbers since they use LLVM's built-in symbolizer, so translation 
is only actually missing on the addresses emitted by a CHECK() or equivalent 
crash. Is this correct?

http://gerrit.cloudera.org:8080/#/c/3360/1/build-support/run-test.sh
File build-support/run-test.sh:

PS1, Line 139: UNAME=$(uname -s)
 :   if [ "$UNAME" = "Darwin" ]; then
How about using the bash built-in $OSTYPE? I think we typically use that in 
lieu of uname.


Line 143: # into line number.  The atos utility is able to do that only if
Nit: can you fix this line's formatting a bit? Looks like you can squeeze a 
little more text into the end, which would give the multi-line comment a more 
uniform look.


-- 
To view, visit http://gerrit.cloudera.org:8080/3360
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf8a81ae7f2e1b938734a4b66bec82cb731d90cc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: Yes

[kudu-CR] Do not run (g)addr2line translator on MacOS X

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Do not run (g)addr2line translator on MacOS X
..


Patch Set 1: Code-Review+1

lgtm but Dan should take a look too, since he develops on a Mac

-- 
To view, visit http://gerrit.cloudera.org:8080/3360
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf8a81ae7f2e1b938734a4b66bec82cb731d90cc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Do not run (g)addr2line translator on MacOS X

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Do not run (g)addr2line translator on MacOS X
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/1806/

-- 
To view, visit http://gerrit.cloudera.org:8080/3360
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idf8a81ae7f2e1b938734a4b66bec82cb731d90cc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Do not run (g)addr2line translator on MacOS X

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/3360

Change subject: Do not run (g)addr2line translator on MacOS X
..

Do not run (g)addr2line translator on MacOS X

When running tests on MacOS X, omit using the
stacktrace_addr2line.pl utility to translate addresses to line numbers.
For current builds on MacOS X, neither (g)addr2line nor atos
translate stack address into line number without extra-hassle.
This is so even for binaries linked without PIE
(see the -no_pie linker's option).

As for the extra-hassle part: there are at least two ways
to address that, but neither looks a good candidate in the context
of the script.

  Option A:
Provide the atos utility with reference to running test process
using '-p ' option.  This, however, triggers a pop-up
requesting for password if it's run the very first time.

  Option B:
Take a snapshot of running test process using MacOS X tools
and provide the atos utility with the load address
using '-l ' option.

Change-Id: Idf8a81ae7f2e1b938734a4b66bec82cb731d90cc
---
M build-support/run-test.sh
1 file changed, 13 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/3360/1
-- 
To view, visit http://gerrit.cloudera.org:8080/3360
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idf8a81ae7f2e1b938734a4b66bec82cb731d90cc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin