You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Hao Hao (Code Review)" <ge...@cloudera.org> on 2020/04/15 21:49:25 UTC

[kudu-CR] mini-cluster: exclude libpcre for license check

Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15738


Change subject: mini-cluster: exclude libpcre for license check
......................................................................

mini-cluster: exclude libpcre for license check

While building mini cluster binaries on centos7, I saw error message
'unknown license: lib/libpcre.so.1' during license check on artifact.
By running lddtree on the binary, I found out libpcre is introduced
as transitive dependency

libcurl.so.4 => /root/kudu/thirdparty/installed/uninstrumented/lib/libcurl.so.4
        libk5crypto.so.3 => /lib64/libk5crypto.so.3
            libkrb5support.so.0 => /lib64/libkrb5support.so.0
                libselinux.so.1 => /lib64/libselinux.so.1
                    libpcre.so.1 => /lib64/libpcre.so.1

This patch excludes 'libpcre' for license check to account for licensing
information only relevant to the binary artifacts.

Change-Id: I0fc69eaf57025b1ce2f7857808e60218dc4f2771
---
M build-support/mini-cluster/relocate_binaries_for_mini_cluster.py
1 file changed, 1 insertion(+), 0 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0fc69eaf57025b1ce2f7857808e60218dc4f2771
Gerrit-Change-Number: 15738
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>

[kudu-CR] mini-cluster: exclude libpcre for license check

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

Change subject: mini-cluster: exclude libpcre for license check
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15738/1/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/15738/1/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py@54
PS1, Line 54: # Exclude libraries that are (L)GPL-licensed and libraries that are not
            : # portable across Linux kernel versions.
Is libpcre a beast of a different nature?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fc69eaf57025b1ce2f7857808e60218dc4f2771
Gerrit-Change-Number: 15738
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 15 Apr 2020 22:17:57 +0000
Gerrit-HasComments: Yes

[kudu-CR] mini-cluster: exclude libpcre for license check

Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke, 

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

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

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

Change subject: mini-cluster: exclude libpcre for license check
......................................................................

mini-cluster: exclude libpcre for license check

While building mini cluster binaries on centos7, I saw error message
'unknown license: lib/libpcre.so.1' during license check on artifact.
By running lddtree on the binary, I found out libpcre is introduced
as transitive dependency

libcurl.so.4 => /root/kudu/thirdparty/installed/uninstrumented/lib/libcurl.so.4
        libk5crypto.so.3 => /lib64/libk5crypto.so.3
            libkrb5support.so.0 => /lib64/libkrb5support.so.0
                libselinux.so.1 => /lib64/libselinux.so.1
                    libpcre.so.1 => /lib64/libpcre.so.1

This patch excludes 'libpcre' for license check to account for licensing
information only relevant to the binary artifacts.

Change-Id: I0fc69eaf57025b1ce2f7857808e60218dc4f2771
---
M build-support/mini-cluster/relocate_binaries_for_mini_cluster.py
1 file changed, 4 insertions(+), 1 deletion(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0fc69eaf57025b1ce2f7857808e60218dc4f2771
Gerrit-Change-Number: 15738
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] mini-cluster: exclude libpcre for license check

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

Change subject: mini-cluster: exclude libpcre for license check
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15738/1/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/15738/1/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py@54
PS1, Line 54: # Exclude libraries that are (L)GPL-licensed and libraries that are not
            : # portable across Linux kernel versions.
> Is libpcre a beast of a different nature?
libpcre is BSD licensed, which is GPL-licensed compatible (https://en.wikipedia.org/wiki/License_compatibility)? (Moreover as libpcre is a transitive dependency introduced by libselinux in this case) Do you think I need to call it out that it is BSD licensed?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fc69eaf57025b1ce2f7857808e60218dc4f2771
Gerrit-Change-Number: 15738
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 15 Apr 2020 22:30:43 +0000
Gerrit-HasComments: Yes

[kudu-CR] mini-cluster: exclude libpcre for license check

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

Change subject: mini-cluster: exclude libpcre for license check
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15738/1/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/15738/1/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py@54
PS1, Line 54: # Exclude libraries that are (L)GPL-licensed and libraries that are not
            : # portable across Linux kernel versions.
Sounds good to me, will update the comment.

> BTW, did you figure out why it has surfaced only now?  If it's a transitive dependency for libselinux, why didn't we see it before?

I am not 100% sure, but looking at the transitive dependencies chain:
libcurl.so.4 => /root/kudu/thirdparty/installed/uninstrumented/lib/libcurl.so.4
        libk5crypto.so.3 => /lib64/libk5crypto.so.3
            libkrb5support.so.0 => /lib64/libkrb5support.so.0
                libselinux.so.1 => /lib64/libselinux.so.1
                    libpcre.so.1 => /lib64/libpcre.so.1

It looks like is initiated by libcurl which we recently make it a run-time dependency?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fc69eaf57025b1ce2f7857808e60218dc4f2771
Gerrit-Change-Number: 15738
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 15 Apr 2020 23:59:56 +0000
Gerrit-HasComments: Yes

[kudu-CR] mini-cluster: exclude libpcre for license check

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

Change subject: mini-cluster: exclude libpcre for license check
......................................................................


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15738/1/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/15738/1/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py@54
PS1, Line 54: # Exclude libraries that are (L)GPL-licensed and libraries that are not
            : # portable across Linux kernel versions.
> Sounds good to me, will update the comment.
I think if libcurl were using pcre, libpcre would be linked directly to libcurl.  I don't see any references from libcurl to libpcre or from kudu-master to libpcre at any of my Linux build machines.

I guess it might be rather some specifics of a particular build machine with libselinux.

Thank you for updating the comment.  At least, it will be easier to track this later on (and maybe, there is a better way around appearance of libpcre in the dependency list, like using only first-level dependencies when using DependencyExtractor).



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fc69eaf57025b1ce2f7857808e60218dc4f2771
Gerrit-Change-Number: 15738
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 16 Apr 2020 00:32:51 +0000
Gerrit-HasComments: Yes

[kudu-CR] mini-cluster: exclude libpcre for license check

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

Change subject: mini-cluster: exclude libpcre for license check
......................................................................

mini-cluster: exclude libpcre for license check

While building mini cluster binaries on centos7, I saw error message
'unknown license: lib/libpcre.so.1' during license check on artifact.
By running lddtree on the binary, I found out libpcre is introduced
as transitive dependency

libcurl.so.4 => /root/kudu/thirdparty/installed/uninstrumented/lib/libcurl.so.4
        libk5crypto.so.3 => /lib64/libk5crypto.so.3
            libkrb5support.so.0 => /lib64/libkrb5support.so.0
                libselinux.so.1 => /lib64/libselinux.so.1
                    libpcre.so.1 => /lib64/libpcre.so.1

This patch excludes 'libpcre' for license check to account for licensing
information only relevant to the binary artifacts.

Change-Id: I0fc69eaf57025b1ce2f7857808e60218dc4f2771
Reviewed-on: http://gerrit.cloudera.org:8080/15738
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M build-support/mini-cluster/relocate_binaries_for_mini_cluster.py
1 file changed, 4 insertions(+), 1 deletion(-)

Approvals:
  Kudu Jenkins: Verified
  Alexey Serbin: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0fc69eaf57025b1ce2f7857808e60218dc4f2771
Gerrit-Change-Number: 15738
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] mini-cluster: exclude libpcre for license check

Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has removed a vote on this change.

Change subject: mini-cluster: exclude libpcre for license check
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/15738
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I0fc69eaf57025b1ce2f7857808e60218dc4f2771
Gerrit-Change-Number: 15738
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] mini-cluster: exclude libpcre for license check

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

Change subject: mini-cluster: exclude libpcre for license check
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15738/1/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/15738/1/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py@54
PS1, Line 54: # Exclude libraries that are (L)GPL-licensed and libraries that are not
            : # portable across Linux kernel versions.
> libpcre is BSD licensed, which is GPL-licensed compatible (https://en.wikip
The important point here is not GPL-compatibilty, but satisfying the requirements of distributing binaries under Apache Software Foundation terms: cannot distribute (L)GPL binaries.

I think we need to point out that libpcre is a special case here, not covered by this current comment.  Yes, I think it makes sense to point out that libpcre is covered by BSD license.  By my understanding, the point of this list is to contain libraries which we don't include into mini-cluster JARs because of the issues mentioned.  If libpcre is BSD-licenced, why don't we just include it into mini-cluster JARs?

Yes, I think it makes sense to point out that libpcre is a transitive dependency.  BTW, did you figure out why it has surfaced only now?  If it's a transitive dependency for libselinux, why didn't we see it before?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fc69eaf57025b1ce2f7857808e60218dc4f2771
Gerrit-Change-Number: 15738
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 15 Apr 2020 23:09:37 +0000
Gerrit-HasComments: Yes

[kudu-CR] mini-cluster: exclude libpcre for license check

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

Change subject: mini-cluster: exclude libpcre for license check
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15738/1/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/15738/1/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py@54
PS1, Line 54: # Exclude libraries that are (L)GPL-licensed and libraries that are not
            : # portable across Linux kernel versions.
> I think if libcurl were using pcre, libpcre would be linked directly to lib
Ack, thanks a lot for the review!



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fc69eaf57025b1ce2f7857808e60218dc4f2771
Gerrit-Change-Number: 15738
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 16 Apr 2020 00:46:32 +0000
Gerrit-HasComments: Yes

[kudu-CR] mini-cluster: exclude libpcre for license check

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

Change subject: mini-cluster: exclude libpcre for license check
......................................................................


Patch Set 1: Verified+1

Unrelated flaky test.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fc69eaf57025b1ce2f7857808e60218dc4f2771
Gerrit-Change-Number: 15738
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 15 Apr 2020 22:31:26 +0000
Gerrit-HasComments: No