You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mike Percy (Code Review)" <ge...@cloudera.org> on 2019/02/11 04:49:34 UTC

[kudu-CR] KUDU-2411: Update license information for binary test jars 

Hello Grant Henke,

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

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

to review the following change.


Change subject: KUDU-2411: Update license information for binary test jars
......................................................................

KUDU-2411: Update license information for binary test jars

This patch updates the license files to account for transitive
dependencies we ship with the binary jar files and updates the scripting
infrastructure to validate the licensing information at binary jar build
time.

This patch also introduces binary-jar-specific license and notice files
to account for licensing information only relevant to the binary
artifacts.

Change-Id: If864384e60013d592c5c7315e2d9a99c83f31449
---
A LICENSE-BINARY-JAR-LINUX.txt
A LICENSE-BINARY-JAR-OSX.txt
M LICENSE.txt
A NOTICE-BINARY-JAR-LINUX.txt
A NOTICE-BINARY-JAR-OSX.txt
M build-support/mini-cluster/build_mini_cluster_binaries.sh
A build-support/mini-cluster/check-license.pl
M build-support/mini-cluster/relocate_binaries_for_mini_cluster.py
M thirdparty/LICENSE.txt
9 files changed, 656 insertions(+), 11 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If864384e60013d592c5c7315e2d9a99c83f31449
Gerrit-Change-Number: 12431
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>

[kudu-CR] KUDU-2411: Update license information for binary test jars

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12431 )

Change subject: KUDU-2411: Update license information for binary test jars
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12431/3/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py
File build-support/mini-cluster/relocate_binaries_for_mini_cluster.py:

http://gerrit.cloudera.org:8080/#/c/12431/3/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py@64
PS3, Line 64:                                         libm|
One of the positive side effects of excluding libm is that running the binaries with LD_BIND_NOW=1 works (and I think that's a good test for whether there'll be any missing symbols at runtime). If libm is shipped, you get a runtime linking error about how the shipped libm is missing __get_cpu_features. That symbol was in libc on el6, but is in a different library on e.g. Ubuntu 18.

On el6 (/lib64):
  $ for so in *.so; do nm -D $so | grep __get_cpu_features && echo $so; done
  0000003ae0a1f2b0 T __get_cpu_features
  libc-2.12.so
                   U __get_cpu_features
  libm-2.12.so

On Ubuntu 18 (/lib/x86_64-linux-gnu):
  $ for so in *.so; do nm -D $so | grep __get_cpu_features && echo $so; done
  0000000000019a30 T __get_cpu_features
  ld-2.27.so



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If864384e60013d592c5c7315e2d9a99c83f31449
Gerrit-Change-Number: 12431
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 12 Feb 2019 20:03:28 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2411: Update license information for binary test jars

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Grant Henke, 

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

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

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

Change subject: KUDU-2411: Update license information for binary test jars
......................................................................

KUDU-2411: Update license information for binary test jars

This patch updates the license files to account for transitive
dependencies we ship with the binary jar files and updates the scripting
infrastructure to validate the licensing information at binary jar build
time.

This patch also introduces binary-jar-specific license and notice files
to account for licensing information only relevant to the binary
artifacts.

Change-Id: If864384e60013d592c5c7315e2d9a99c83f31449
---
M LICENSE.txt
A build-support/mini-cluster/LICENSE-BINARY-JAR-LINUX.txt
A build-support/mini-cluster/LICENSE-BINARY-JAR-OSX.txt
A build-support/mini-cluster/NOTICE-BINARY-JAR-LINUX.txt
A build-support/mini-cluster/NOTICE-BINARY-JAR-OSX.txt
M build-support/mini-cluster/build_mini_cluster_binaries.sh
A build-support/mini-cluster/check-license.pl
M build-support/mini-cluster/relocate_binaries_for_mini_cluster.py
M thirdparty/LICENSE.txt
9 files changed, 660 insertions(+), 12 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If864384e60013d592c5c7315e2d9a99c83f31449
Gerrit-Change-Number: 12431
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-2411: Update license information for binary test jars

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12431 )

Change subject: KUDU-2411: Update license information for binary test jars
......................................................................


Patch Set 1: Code-Review+1

(4 comments)

http://gerrit.cloudera.org:8080/#/c/12431/1/build-support/mini-cluster/build_mini_cluster_binaries.sh
File build-support/mini-cluster/build_mini_cluster_binaries.sh:

http://gerrit.cloudera.org:8080/#/c/12431/1/build-support/mini-cluster/build_mini_cluster_binaries.sh@188
PS1, Line 188: echo "Running license check on artifact..."
             : $SOURCE_ROOT/build-support/mini-cluster/check-license.pl $ARTIFACT_NAME
Should we care about cleaning up the LICENSE or NOTICE files if this fails?


http://gerrit.cloudera.org:8080/#/c/12431/1/build-support/mini-cluster/check-license.pl
File build-support/mini-cluster/check-license.pl:

http://gerrit.cloudera.org:8080/#/c/12431/1/build-support/mini-cluster/check-license.pl@30
PS1, Line 30: unpacked-prefix-dir
nit: mind adding a comment on what prefix this is referring to? Would "parent" directory also be an apt name?


http://gerrit.cloudera.org:8080/#/c/12431/1/thirdparty/LICENSE.txt
File thirdparty/LICENSE.txt:

http://gerrit.cloudera.org:8080/#/c/12431/1/thirdparty/LICENSE.txt@7
PS1, Line 7: in Kudu binary test jar
nit: "in the"


http://gerrit.cloudera.org:8080/#/c/12431/1/thirdparty/LICENSE.txt@8
PS1, Line 8: most of Some 
nit: "some"



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If864384e60013d592c5c7315e2d9a99c83f31449
Gerrit-Change-Number: 12431
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 11 Feb 2019 19:33:09 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2411: Update license information for binary test jars

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12431 )

Change subject: KUDU-2411: Update license information for binary test jars
......................................................................

KUDU-2411: Update license information for binary test jars

This patch updates the license files to account for transitive
dependencies we ship with the binary jar files and updates the scripting
infrastructure to validate the licensing information at binary jar build
time.

This patch also introduces binary-jar-specific license and notice files
to account for licensing information only relevant to the binary
artifacts.

Change-Id: If864384e60013d592c5c7315e2d9a99c83f31449
Reviewed-on: http://gerrit.cloudera.org:8080/12431
Reviewed-by: Grant Henke <gr...@apache.org>
Tested-by: Kudu Jenkins
---
M LICENSE.txt
A build-support/mini-cluster/LICENSE-BINARY-JAR-LINUX.txt
A build-support/mini-cluster/LICENSE-BINARY-JAR-OSX.txt
A build-support/mini-cluster/NOTICE-BINARY-JAR-LINUX.txt
A build-support/mini-cluster/NOTICE-BINARY-JAR-OSX.txt
M build-support/mini-cluster/build_mini_cluster_binaries.sh
A build-support/mini-cluster/check-license.pl
M build-support/mini-cluster/relocate_binaries_for_mini_cluster.py
M thirdparty/LICENSE.txt
9 files changed, 660 insertions(+), 12 deletions(-)

Approvals:
  Grant Henke: Looks good to me, approved
  Kudu Jenkins: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If864384e60013d592c5c7315e2d9a99c83f31449
Gerrit-Change-Number: 12431
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-2411: Update license information for binary test jars

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12431 )

Change subject: KUDU-2411: Update license information for binary test jars
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If864384e60013d592c5c7315e2d9a99c83f31449
Gerrit-Change-Number: 12431
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Mon, 11 Feb 2019 20:54:24 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2411: Update license information for binary test jars

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12431 )

Change subject: KUDU-2411: Update license information for binary test jars
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12431/1/LICENSE-BINARY-JAR-LINUX.txt
File LICENSE-BINARY-JAR-LINUX.txt:

PS1: 
Should we put these license files in the mini-cluster directory?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If864384e60013d592c5c7315e2d9a99c83f31449
Gerrit-Change-Number: 12431
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 11 Feb 2019 20:02:09 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2411: Update license information for binary test jars

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12431 )

Change subject: KUDU-2411: Update license information for binary test jars
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12431/1/thirdparty/LICENSE.txt
File thirdparty/LICENSE.txt:

http://gerrit.cloudera.org:8080/#/c/12431/1/thirdparty/LICENSE.txt@668
PS1, Line 668: TODO(KUDU-2696): gmock is unnecessarily linked into the kudu cli binary
Is it normal to have TODO's in a license file? Maybe we should just add this detail to the jira?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If864384e60013d592c5c7315e2d9a99c83f31449
Gerrit-Change-Number: 12431
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 11 Feb 2019 20:07:24 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2411: Update license information for binary test jars

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12431 )

Change subject: KUDU-2411: Update license information for binary test jars
......................................................................


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/12431/1/LICENSE-BINARY-JAR-LINUX.txt
File LICENSE-BINARY-JAR-LINUX.txt:

PS1: 
> Should we put these license files in the mini-cluster directory?
Done


http://gerrit.cloudera.org:8080/#/c/12431/1/build-support/mini-cluster/build_mini_cluster_binaries.sh
File build-support/mini-cluster/build_mini_cluster_binaries.sh:

http://gerrit.cloudera.org:8080/#/c/12431/1/build-support/mini-cluster/build_mini_cluster_binaries.sh@188
PS1, Line 188: echo "Running license check on artifact..."
             : $SOURCE_ROOT/build-support/mini-cluster/check-license.pl $ARTIFACT_NAME
> Should we care about cleaning up the LICENSE or NOTICE files if this fails?
Yes, it will fail this script due to -e


http://gerrit.cloudera.org:8080/#/c/12431/1/build-support/mini-cluster/check-license.pl
File build-support/mini-cluster/check-license.pl:

http://gerrit.cloudera.org:8080/#/c/12431/1/build-support/mini-cluster/check-license.pl@30
PS1, Line 30: unpacked-prefix-dir
> nit: mind adding a comment on what prefix this is referring to? Would "pare
It's in the script file comment, but I will also print it.


http://gerrit.cloudera.org:8080/#/c/12431/1/thirdparty/LICENSE.txt
File thirdparty/LICENSE.txt:

http://gerrit.cloudera.org:8080/#/c/12431/1/thirdparty/LICENSE.txt@7
PS1, Line 7: in Kudu binary test jar
> nit: "in the"
Done


http://gerrit.cloudera.org:8080/#/c/12431/1/thirdparty/LICENSE.txt@8
PS1, Line 8: most of Some 
> nit: "some"
Done


http://gerrit.cloudera.org:8080/#/c/12431/1/thirdparty/LICENSE.txt@668
PS1, Line 668: TODO(KUDU-2696): gmock is unnecessarily linked into the kudu cli binary
> Is it normal to have TODO's in a license file? Maybe we should just add thi
I don't think it hurts anything to leave it here.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If864384e60013d592c5c7315e2d9a99c83f31449
Gerrit-Change-Number: 12431
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Mon, 11 Feb 2019 20:28:05 +0000
Gerrit-HasComments: Yes