You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2017/06/21 20:14:44 UTC

[kudu-CR] Fix gmock link errors on macOS 

Hello Adar Dembo, Alexey Serbin,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: Fix gmock link errors on macOS
......................................................................

Fix gmock link errors on macOS

For reasons unknown, using 'make install' to install the gmock/gtest
libraries was causing the lib name to be malformed on macOS. For
example, before this change:

```
$ otool -L thirdparty/installed/uninstrumented/lib/libgmock.dylib
thirdparty/installed/uninstrumented/lib/libgmock.dylib:
        libgmock.dylib (compatibility version 0.0.0, current version 0.0.0)
        /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 120.0.0)
        /usr/lib/libc++abi.dylib (compatibility version 1.0.0, current version 125.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1213.0.0)
```

and with the change:

```
$ otool -L thirdparty/installed/uninstrumented/lib/libgmock.dylib
thirdparty/installed/uninstrumented/lib/libgmock.dylib:
        /Users/dan/src/cpp/kudu-gtest/thirdparty/build/gmock-1.7.0.shared/libgmock.dylib (compatibility version 0.0.0, current version 0.0.0)
        /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 120.0.0)
        /usr/lib/libc++abi.dylib (compatibility version 1.0.0, current version 125.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1213.0.0)
```

The result was that tests failed with runtime linker errors:

```
$ bin/index-test
dyld: Library not loaded: libgmock.dylib
  Referenced from: /Users/dan/src/cpp/kudu/build/debug/bin/index-test
    Reason: image not found
    fish: 'and bin/index-test' terminated by signal SIGTRAP (Trace or breakpoint trap)
```

I didn't get as far as identifying why the built libraries don't have
the same lib name issue (as opposed to the installed libraries).

Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
---
M thirdparty/build-definitions.sh
1 file changed, 11 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>

[kudu-CR] Fix gmock link errors on macOS

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: Fix gmock link errors on macOS
......................................................................


Patch Set 2: Code-Review+1

Failure doesn't look related.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Fix gmock link errors on macOS

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: Fix gmock link errors on macOS
......................................................................


Patch Set 2:

right, that's a fair assesment

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Fix gmock link errors on macOS

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: Fix gmock link errors on macOS
......................................................................


Patch Set 1:

(1 comment)

woops, meant that to be a reply to the comment.

http://gerrit.cloudera.org:8080/#/c/7251/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

PS1, Line 339:   echo Installing gmock...
             :   cp -a $GMOCK_SHARED_BDIR/libgmock.$DYLIB_SUFFIX $PREFIX/lib/
             :   cp -a $GMOCK_STATIC_BDIR/libgmock.a $PREFIX/lib/
             :   rsync -av $GMOCK_SOURCE/include/ $PREFIX/include/
             :   rsync -av $GMOCK_SOURCE/gtest/include/ $PREFIX/include/
> So, playing with additional
I tried applying the patch here: https://github.com/google/googletest/pull/829, but that had no effect.  I believe the two things you mentioned are equivalent.  I can't spend any more time on this issue, so I'm going to suggest we merge as is and not investigate more.  The rsync install method has worked reliably for the entire life of the project to date.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Fix gmock link errors on macOS

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: Fix gmock link errors on macOS
......................................................................

Fix gmock link errors on macOS

For reasons unknown, using 'make install' to install the gmock/gtest
libraries was causing the lib name to be malformed on macOS. For
example, before this change:

```
$ otool -L thirdparty/installed/uninstrumented/lib/libgmock.dylib
thirdparty/installed/uninstrumented/lib/libgmock.dylib:
        libgmock.dylib (compatibility version 0.0.0, current version 0.0.0)
        /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 120.0.0)
        /usr/lib/libc++abi.dylib (compatibility version 1.0.0, current version 125.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1213.0.0)
```

and with the change:

```
$ otool -L thirdparty/installed/uninstrumented/lib/libgmock.dylib
thirdparty/installed/uninstrumented/lib/libgmock.dylib:
        /Users/dan/src/cpp/kudu-gtest/thirdparty/build/gmock-1.7.0.shared/libgmock.dylib (compatibility version 0.0.0, current version 0.0.0)
        /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 120.0.0)
        /usr/lib/libc++abi.dylib (compatibility version 1.0.0, current version 125.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1213.0.0)
```

The result was that tests failed with runtime linker errors:

```
$ bin/index-test
dyld: Library not loaded: libgmock.dylib
  Referenced from: /Users/dan/src/cpp/kudu/build/debug/bin/index-test
    Reason: image not found
    fish: 'and bin/index-test' terminated by signal SIGTRAP (Trace or breakpoint trap)
```

I didn't get as far as identifying why the built libraries don't have
the same lib name issue (as opposed to the installed libraries).

Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
---
M thirdparty/build-definitions.sh
1 file changed, 10 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Fix gmock link errors on macOS

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: Fix gmock link errors on macOS
......................................................................


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Fix gmock link errors on macOS

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: Fix gmock link errors on macOS
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7251/1/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

PS1, Line 339:   echo Installing gmock...
             :   cp -a $GMOCK_SHARED_BDIR/libgmock.$DYLIB_SUFFIX $PREFIX/lib/
             :   cp -a $GMOCK_STATIC_BDIR/libgmock.a $PREFIX/lib/
             :   rsync -av $GMOCK_SOURCE/include/ $PREFIX/include/
             :   rsync -av $GMOCK_SOURCE/gtest/include/ $PREFIX/include/
So, playing with additional

-DCMAKE_MACOSX_RPATH=xxx

for the 'cmake' command above didn't help?

And CMP0042 didn't work either?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Fix gmock link errors on macOS

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: Fix gmock link errors on macOS
......................................................................


Patch Set 2:

I tried applying the patch here: https://github.com/google/googletest/pull/829, but that had no effect.  I believe the two things you mentioned are equivalent.  I can't spend any more time on this issue, so I'm going to suggest we merge as is and not investigate more.  The rsync install method has worked reliably for the entire life of the project to date.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Fix gmock link errors on macOS

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: Fix gmock link errors on macOS
......................................................................


Patch Set 2: Code-Review+2

If f329e089f6ac1ee2fcb4a4e65fb34ef0d3d374d5 caused some regression and we have no time to investigate, let's merge this change.  and this patch is kind of rolling back of that. f329e089f6ac1ee2fcb4a4e65fb34ef0d3d374d5 

LGTM -- this patch is kind of rolling back of the corresponding modifications introduced in f329e089f6ac1ee2fcb4a4e65fb34ef0d3d374d5

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Fix gmock link errors on macOS

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.

Change subject: Fix gmock link errors on macOS
......................................................................


Fix gmock link errors on macOS

For reasons unknown, using 'make install' to install the gmock/gtest
libraries was causing the lib name to be malformed on macOS. For
example, before this change:

```
$ otool -L thirdparty/installed/uninstrumented/lib/libgmock.dylib
thirdparty/installed/uninstrumented/lib/libgmock.dylib:
        libgmock.dylib (compatibility version 0.0.0, current version 0.0.0)
        /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 120.0.0)
        /usr/lib/libc++abi.dylib (compatibility version 1.0.0, current version 125.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1213.0.0)
```

and with the change:

```
$ otool -L thirdparty/installed/uninstrumented/lib/libgmock.dylib
thirdparty/installed/uninstrumented/lib/libgmock.dylib:
        /Users/dan/src/cpp/kudu-gtest/thirdparty/build/gmock-1.7.0.shared/libgmock.dylib (compatibility version 0.0.0, current version 0.0.0)
        /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 120.0.0)
        /usr/lib/libc++abi.dylib (compatibility version 1.0.0, current version 125.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1213.0.0)
```

The result was that tests failed with runtime linker errors:

```
$ bin/index-test
dyld: Library not loaded: libgmock.dylib
  Referenced from: /Users/dan/src/cpp/kudu/build/debug/bin/index-test
    Reason: image not found
    fish: 'and bin/index-test' terminated by signal SIGTRAP (Trace or breakpoint trap)
```

I didn't get as far as identifying why the built libraries don't have
the same lib name issue (as opposed to the installed libraries).

Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Reviewed-on: http://gerrit.cloudera.org:8080/7251
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Dan Burkert <da...@apache.org>
Tested-by: Dan Burkert <da...@apache.org>
---
M thirdparty/build-definitions.sh
1 file changed, 10 insertions(+), 2 deletions(-)

Approvals:
  Dan Burkert: Looks good to me, but someone else must approve; Verified
  Alexey Serbin: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I41e6d4bfcabb36efbf83fd4969e9f35c16b74f3d
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>