[kudu-CR] [thirdparty]: added include-what-you-use

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged.

Change subject: [thirdparty]: added include-what-you-use
..


[thirdparty]: added include-what-you-use

Build the include-what-you-use utility along with the LLVM toolchain.

The project URL:
  https://include-what-you-use.org/

Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
Reviewed-on: http://gerrit.cloudera.org:8080/7593
Reviewed-by: Adar Dembo 
Tested-by: Alexey Serbin 
---
M thirdparty/download-thirdparty.sh
A thirdparty/patches/llvm-add-iwyu.patch
A thirdparty/patches/llvm-iwyu-include-picker.patch
A thirdparty/patches/llvm-iwyu-nocurses.patch
M thirdparty/vars.sh
5 files changed, 39 insertions(+), 2 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Alexey Serbin: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [thirdparty]: added include-what-you-use

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [thirdparty]: added include-what-you-use
..


Patch Set 6: Verified+1

Unrelated flake in 
org.apache.kudu.client.TestScannerMultiTablet.org.apache.kudu.client.TestScannerMultiTablet

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [thirdparty]: added include-what-you-use

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [thirdparty]: added include-what-you-use
..


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [thirdparty]: added include-what-you-use

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [thirdparty]: added include-what-you-use
..

[thirdparty]: added include-what-you-use

Build the include-what-you-use utility along with the LLVM toolchain.

The project URL:
  https://include-what-you-use.org/

Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
---
M thirdparty/download-thirdparty.sh
A thirdparty/patches/llvm-add-iwyu.patch
A thirdparty/patches/llvm-iwyu-include-picker.patch
A thirdparty/patches/llvm-iwyu-nocurses.patch
M thirdparty/vars.sh
5 files changed, 39 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/7593/6
-- 
To view, visit http://gerrit.cloudera.org:8080/7593
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [thirdparty]: added include-what-you-use

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [thirdparty]: added include-what-you-use
..

[thirdparty]: added include-what-you-use

Build the include-what-you-use utility along with the LLVM toolchain.

The project URL:
  https://include-what-you-use.org/

Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
---
M thirdparty/download-thirdparty.sh
A thirdparty/patches/llvm-add-iwyu.patch
A thirdparty/patches/llvm-iwyu-include-picker.patch
A thirdparty/patches/llvm-iwyu-nocurses.patch
M thirdparty/vars.sh
5 files changed, 47 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/7593/4
-- 
To view, visit http://gerrit.cloudera.org:8080/7593
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [thirdparty]: added include-what-you-use

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [thirdparty]: added include-what-you-use
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7593/3/thirdparty/patches/llvm-iwyu-nocurses.patch
File thirdparty/patches/llvm-iwyu-nocurses.patch:

> What does IWYU use curses for? What is the effect of removing it?
Some of our build machines used by Jenkins do not have ncurses libraries and 
headers files installed.  On those the IWYU tool could not be built.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [thirdparty]: added include-what-you-use

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change.

Change subject: [thirdparty]: added include-what-you-use
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7593/3/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:

PS3, Line 55:   local URL_PREFIX=$3
:   if [ -z "$URL_PREFIX" ]; then
: URL_PREFIX="${CLOUDFRONT_URL_PREFIX}"
:   fi
> is this used?
It's no longer used: that's a remnant from the version where the source file 
was not uploaded into the cloudsource repo.

I'll remove it.


PS3, Line 264:   fetch_and_expand 
include-what-you-use-${IWYU_VERSION}.src.tar.gz \
 : ${LLVM_SOURCE}/tools/clang/tools
> Right now updating LLVM involves extracting the LLVM tarball, adding clang 
Thank you for pointing that out.  Indeed -- this is kind of hackish way to 
achieve what I needed.

I think the former option (uploading a new LLVM tarball which would include 
IWYU) is my best bet in the short term.  The include-what-you-use source does 
not need to be updated every release of LLVM, it just needs to be compiled 
along with it.


http://gerrit.cloudera.org:8080/#/c/7593/3/thirdparty/patches/llvm-iwyu-nocurses.patch
File thirdparty/patches/llvm-iwyu-nocurses.patch:

> What does IWYU use curses for? What is the effect of removing it?
You could find more details at 
https://github.com/include-what-you-use/include-what-you-use/pull/465

It seems somehow they compile LlvmSupport lib with ncurses support, and when 
linking the include-what-you-use binary (which links LlvmSupport lib), it's 
necessary to add ncurses library for some symbols in LlvmSupport library itself.

However, I found that in our environment we don't need that (probably, because 
we don't use all bells and whistles).

But I might be some bug -- I haven't looked at that yet.  I will need to do 
that to continue with that pull request I submitted, though.


http://gerrit.cloudera.org:8080/#/c/7593/3/thirdparty/vars.sh
File thirdparty/vars.sh:

PS3, Line 148: IWYU_NAME=llvm-$LLVM_VERSION.src
 : IWYU_SOURCE=$TP_SOURCE_DIR/$IWYU_NAME
> are these actually used, then?
Nope, it's not.  It seems it's not needed.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [thirdparty]: added include-what-you-use

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [thirdparty]: added include-what-you-use
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7593/3/thirdparty/patches/llvm-iwyu-nocurses.patch
File thirdparty/patches/llvm-iwyu-nocurses.patch:

What does IWYU use curses for? What is the effect of removing it?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [thirdparty]: added include-what-you-use

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [thirdparty]: added include-what-you-use
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7593/3/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:

PS3, Line 264:   fetch_and_expand 
include-what-you-use-${IWYU_VERSION}.src.tar.gz \
 : ${LLVM_SOURCE}/tools/clang/tools
Right now updating LLVM involves extracting the LLVM tarball, adding clang and 
a bunch of other stuff, retarring it, and uploading the result. The new 
EXTRACT_SUBDIR functionality you added to fetch_and_expand means we could 
perhaps get away with uploading the various LLVM and clang tarballs to S3 and 
piecing the directory tree together on the client machine, like you did for 
IWYU.

In any case, adding IWYU like this represents an unusual middle ground: the 
LLVM tarball is not fully preprocessed, nor is it fully postprocessed. Could 
you either adhere to the status quo and upload a new LLVM tarball to S3 
containing IWYU, or redo this to fully componentize LLVM and piece the 
directory tree together locally?

Which approach is preferred may depend on whether IWYU needs to be 
patched/updated with every LLVM release. Is there some semblance of 
compatibility? Or is there a 1-1 relationship between each release?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [thirdparty]: added include-what-you-use

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [thirdparty]: added include-what-you-use
..


Patch Set 3:

(2 comments)

any idea what the incremental compilation time cost is for thirdparty on this?

http://gerrit.cloudera.org:8080/#/c/7593/3/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:

PS3, Line 55:   local URL_PREFIX=$3
:   if [ -z "$URL_PREFIX" ]; then
: URL_PREFIX="${CLOUDFRONT_URL_PREFIX}"
:   fi
is this used?


http://gerrit.cloudera.org:8080/#/c/7593/3/thirdparty/vars.sh
File thirdparty/vars.sh:

PS3, Line 148: IWYU_NAME=llvm-$LLVM_VERSION.src
 : IWYU_SOURCE=$TP_SOURCE_DIR/$IWYU_NAME
are these actually used, then?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] [thirdparty]: added include-what-you-use

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: [thirdparty]: added include-what-you-use
..

[thirdparty]: added include-what-you-use

Build the include-what-you-use utility along with the LLVM toolchain.

The project URL:
  https://include-what-you-use.org/

Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
---
M thirdparty/download-thirdparty.sh
A thirdparty/patches/llvm-add-iwyu.patch
A thirdparty/patches/llvm-iwyu-nocurses.patch
M thirdparty/vars.sh
4 files changed, 44 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/7593/3
-- 
To view, visit http://gerrit.cloudera.org:8080/7593
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I041ad9e4bbff410f1760c7abd8cd173e5e9cc564
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon