[Toolchain-CR] IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.

2016-11-08 Thread Jim Apple (Code Review)
Jim Apple has abandoned this change.

Change subject: IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.
..


Abandoned

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ic1110b8c8dc5a9333143055afd49734fc336a1f0
Gerrit-PatchSet: 1
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 


[Toolchain-CR] IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.

2016-11-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.
..


Patch Set 1:

> I don't understand how this patch conflates them.
It's building code that's not C/C++ and isn't self-contained (since it pulls 
down binary artifacts rather than building from source). I don't think Kudu 
fully meets those criteria, unfortunately, but that's a separate issue.

> Not the system JDK? Why not?

We generally build Impala with a fixed JDK version on all platforms (Oracle JDK 
7). I think this makes more sense (and the compiled JARs should be the same 
independent of platform). 

> Is "it" dita-ot

Yeah. I mean it seems most useful if we have this helper script in the same 
place as the other doc build scripts (that would make it easier to automate doc 
builds I think).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1110b8c8dc5a9333143055afd49734fc336a1f0
Gerrit-PatchSet: 1
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Toolchain-CR] IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.

2016-11-02 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change.

Change subject: IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.
..


Patch Set 1:

> My feeling is that we need to clearly distinguish between the
 > native-toolchain as a system for building native dependencies from
 > source in a self-contained reproducible way, and the S3 buckets as
 > a delivery mechanism.

I don't understand how this patch conflates them.

 > It's just
 > coincidence that one was available on the systems that it was built
 > on because those VM images are prepopulated with various JDKs.

Agreed. And putting a JVM in them is a rabbit hole I'd rather not go down.

 > We need to have a JDK for the Impala build (which is not the system
 > JDK)

Not the system JDK? Why not?

 > , so I think that actually is an argument to build it in the
 > Impala repository, which is the only step of the build now that
 > requires a JDK to be present.

Is "it" dita-ot in this sentence?

 > I also think it would be a mistake to start treating the
 > impala-setup repository as an essential build step instead of a
 > convenience.

I don't think adding dita-ot to impala-setup treats impala-setup as an 
essential. Anyone who already had dita-ot installed could presumably continue 
to use it.

 > What's the vision for the docs build? Will it be part of the Impala
 > source repoo?

Yes, the docs will be part of the Impala source repo.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1110b8c8dc5a9333143055afd49734fc336a1f0
Gerrit-PatchSet: 1
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Toolchain-CR] IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.

2016-11-02 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.
..


Patch Set 1:

My feeling is that we need to clearly distinguish between the native-toolchain 
as a system for building native dependencies from source in a self-contained 
reproducible way, and the S3 buckets as a delivery mechanism. I don't think 
they're the same thing and I think conflating them will cause problems.

native-toolchain should not require any Java compilers. It's just coincidence 
that one was available on the systems that it was built on because those VM 
images are prepopulated with various JDKs.

We need to have a JDK for the Impala build (which is not the system JDK), so I 
think that actually is an argument to build it in the Impala repository, which 
is the only step of the build now that requires a JDK to be present.

I also think it would be a mistake to start treating the impala-setup 
repository as an essential build step instead of a convenience.

What's the vision for the docs build? Will it be part of the Impala source 
repoo?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1110b8c8dc5a9333143055afd49734fc336a1f0
Gerrit-PatchSet: 1
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Toolchain-CR] IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.

2016-11-02 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change.

Change subject: IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.
..


Patch Set 1:

> I think we can avoid making this part of native-toolchain since it
 > looks like pure Java code that doesn't depend on any other parts of
 > the toolchain and isn't platform-dependent.

I was adding this to the toolchain not only for platform-dependency issues, but 
also to make it easy for toolchain users to get all of the tools needed for 
build and test.

Also, I imagine it might depend on your JDK, which one might consider part of 
the platform.

 > This would also make the native toolchain dependent on Gradle,
 > which I believe will download things from the internet, rather than
 > just our S3 source buckets.

Yeah, that's not great. Once we decide where to put this, I will try to fix 
that with "./gradlew dist".

 > How about we just add the build script to the Impala repository? I
 > could see hosting the source on S3 but it might also be good to
 > support building directly from the original github tag.

Maybe it should be part of https://github.com/awleblang/impala-setup? It seems 
like that's where the rest of the scripts to download and install third-party 
dependencies are, other than this Toolchain repo.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1110b8c8dc5a9333143055afd49734fc336a1f0
Gerrit-PatchSet: 1
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Toolchain-CR] IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.

2016-11-01 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.
..


Patch Set 1:

I think we can avoid making this part of native-toolchain since it looks like 
pure Java code that doesn't depend on any other parts of the toolchain and 
isn't platform-dependent.

This would also make the native toolchain dependent on Gradle, which I believe 
will download things from the internet, rather than just our S3 source buckets.

How about we just add the build script to the Impala repository? I could see 
hosting the source on S3 but it might also be good to support building directly 
from the original github tag.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1110b8c8dc5a9333143055afd49734fc336a1f0
Gerrit-PatchSet: 1
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Toolchain-CR] IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.

2016-11-01 Thread Jim Apple (Code Review)
Jim Apple has posted comments on this change.

Change subject: IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.
..


Patch Set 1:

I have run buildall.sh on:

centos 5
centos 6
centos 7
debian 6
debian 7
debian 8
sles 11
sles 12
ubuntu 12.04
ubuntu 14.04
ubuntu 16.04

using Jenkins. They were all green.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1110b8c8dc5a9333143055afd49734fc336a1f0
Gerrit-PatchSet: 1
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Jim Apple 
Gerrit-Reviewer: Jim Apple 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-HasComments: No


[Toolchain-CR] IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.

2016-11-01 Thread Jim Apple (Code Review)
Jim Apple has uploaded a new change for review.

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

Change subject: IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.
..

IMPALA-3399: Add DITA Open Toolkit to build Impala user docs.

On the doc_prototype branch of the ASF Impala repo, the following
command successfully builds the docs (after build.sh dita-ot 2.3.3):

build/dita-ot-2.3.3/src/main/bin/dita \
  -i ${IMPALA_HOME}/docs/impala_sqlref.ditamap \
  -f html5 \
  -o $(mktemp -d)

Change-Id: Ic1110b8c8dc5a9333143055afd49734fc336a1f0
---
M buildall.sh
A source/dita-ot/build.sh
2 files changed, 44 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Toolchain refs/changes/02/4902/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4902
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic1110b8c8dc5a9333143055afd49734fc336a1f0
Gerrit-PatchSet: 1
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Jim Apple