[GitHub] bigtop pull request #310: BIGTOP-2949. Add gradle task which leverage bigtop...
Github user c0s commented on a diff in the pull request: https://github.com/apache/bigtop/pull/310#discussion_r166417032 --- Diff: build.gradle --- @@ -425,6 +425,24 @@ task "bigtop-slaves"(type:Exec, commandLine command } +task "docker-package"(type:Exec, +description: 'Build packages via docker build.\n' + +'Usage:\n $ ./gradlew -POS=[centos-7|fedora-26|debian-9|ubuntu-16.04|opensuse-42.3] -Pprefix=TAG_PREFIX -Ptarget=GRADLE_TARGET docker-package\n' + +'Example:\n $ ./gradlew -POS=debian-9 -Pprefix=1.2.1 -Ptarget=tez-pkg docker-package\n' + --- End diff -- Cool, thanks! ---
[GitHub] bigtop pull request #310: BIGTOP-2949. Add gradle task which leverage bigtop...
Github user evans-ye commented on a diff in the pull request: https://github.com/apache/bigtop/pull/310#discussion_r165055895 --- Diff: build.gradle --- @@ -425,6 +425,24 @@ task "bigtop-slaves"(type:Exec, commandLine command } +task "docker-package"(type:Exec, +description: 'Build packages via docker build.\n' + +'Usage:\n $ ./gradlew -POS=[centos-7|fedora-26|debian-9|ubuntu-16.04|opensuse-42.3] -Pprefix=TAG_PREFIX -Ptarget=GRADLE_TARGET docker-package\n' + +'Example:\n $ ./gradlew -POS=debian-9 -Pprefix=1.2.1 -Ptarget=tez-pkg docker-package\n' + --- End diff -- Sounds good for pre-requisite check. Let's do it in another PR. I'll commit this if no object. ---
[GitHub] bigtop pull request #310: BIGTOP-2949. Add gradle task which leverage bigtop...
Github user c0s commented on a diff in the pull request: https://github.com/apache/bigtop/pull/310#discussion_r157172119 --- Diff: build.gradle --- @@ -425,6 +425,24 @@ task "bigtop-slaves"(type:Exec, commandLine command } +task "docker-package"(type:Exec, +description: 'Build packages via docker build.\n' + +'Usage:\n $ ./gradlew -POS=[centos-7|fedora-26|debian-9|ubuntu-16.04|opensuse-42.3] -Pprefix=TAG_PREFIX -Ptarget=GRADLE_TARGET docker-package\n' + +'Example:\n $ ./gradlew -POS=debian-9 -Pprefix=1.2.1 -Ptarget=tez-pkg docker-package\n' + --- End diff -- I presume "ind" stands for "in docker"? Sounds good to me! As for the docker per-requisite: perhaps we can run a sanity check (i.e. an exec and check the exit status) for docker and issue the error message if one isn't available. Thanks! ---
[GitHub] bigtop pull request #310: BIGTOP-2949. Add gradle task which leverage bigtop...
Github user evans-ye commented on a diff in the pull request: https://github.com/apache/bigtop/pull/310#discussion_r156964821 --- Diff: build.gradle --- @@ -425,6 +425,24 @@ task "bigtop-slaves"(type:Exec, commandLine command } +task "docker-package"(type:Exec, +description: 'Build packages via docker build.\n' + +'Usage:\n $ ./gradlew -POS=[centos-7|fedora-26|debian-9|ubuntu-16.04|opensuse-42.3] -Pprefix=TAG_PREFIX -Ptarget=GRADLE_TARGET docker-package\n' + +'Example:\n $ ./gradlew -POS=debian-9 -Pprefix=1.2.1 -Ptarget=tez-pkg docker-package\n' + --- End diff -- Valuable inputs @c0s! I love the idea of generating targets. Speaking of the name, how about tez-pkg-ind? Although dockerized build is our implementation detail, it can tell user what exactly the different is between tez-pkg and tez-pkg-ind. Anyhow user need to have docker installed before running any dockerized target bigtop provides. Therefore I think we should explicitly expose the info. ---
[GitHub] bigtop pull request #310: BIGTOP-2949. Add gradle task which leverage bigtop...
Github user c0s commented on a diff in the pull request: https://github.com/apache/bigtop/pull/310#discussion_r154011483 --- Diff: build.gradle --- @@ -425,6 +425,24 @@ task "bigtop-slaves"(type:Exec, commandLine command } +task "docker-package"(type:Exec, +description: 'Build packages via docker build.\n' + +'Usage:\n $ ./gradlew -POS=[centos-7|fedora-26|debian-9|ubuntu-16.04|opensuse-42.3] -Pprefix=TAG_PREFIX -Ptarget=GRADLE_TARGET docker-package\n' + +'Example:\n $ ./gradlew -POS=debian-9 -Pprefix=1.2.1 -Ptarget=tez-pkg docker-package\n' + --- End diff -- Any input about the target naming? Thanks! ---
[GitHub] bigtop pull request #310: BIGTOP-2949. Add gradle task which leverage bigtop...
Github user c0s commented on a diff in the pull request: https://github.com/apache/bigtop/pull/310#discussion_r153704226 --- Diff: build.gradle --- @@ -425,6 +425,24 @@ task "bigtop-slaves"(type:Exec, commandLine command } +task "docker-package"(type:Exec, +description: 'Build packages via docker build.\n' + +'Usage:\n $ ./gradlew -POS=[centos-7|fedora-26|debian-9|ubuntu-16.04|opensuse-42.3] -Pprefix=TAG_PREFIX -Ptarget=GRADLE_TARGET docker-package\n' + +'Example:\n $ ./gradlew -POS=debian-9 -Pprefix=1.2.1 -Ptarget=tez-pkg docker-package\n' + --- End diff -- I think it is more in the gradle style if we just generate a bunch of targets to build packages in the docker environment, instead of passing in the package name with a property. Something like: ``` ./gradlew -POS=debian-9 -Pprefix=1.2.1 tez-pkg-indocker ``` Look at the logic around packages.gradle:563-573 for the illustration of my thinking, if you will. Besides, the fact that we are building "in a dockerized environment" is just an implementation detail. We are helping people to use some standard environment we guarantee is working properly. Perhaps it make sense to change the name of these targets to something like `tez-pkg-stdenv` or something? What do you think? I am just a bit concerned that people will confuse this `docker-package` with producing a container for the component. Thanks! ---
[GitHub] bigtop pull request #310: BIGTOP-2949. Add gradle task which leverage bigtop...
Github user evans-ye commented on a diff in the pull request: https://github.com/apache/bigtop/pull/310#discussion_r153703568 --- Diff: build.gradle --- @@ -425,6 +425,24 @@ task "bigtop-slaves"(type:Exec, commandLine command } +task "docker-package"(type:Exec, +description: 'Build packages via docker build.\n' + --- End diff -- Good suggestion! Let me refine the patch. ---
[GitHub] bigtop pull request #310: BIGTOP-2949. Add gradle task which leverage bigtop...
Github user c0s commented on a diff in the pull request: https://github.com/apache/bigtop/pull/310#discussion_r153702692 --- Diff: build.gradle --- @@ -425,6 +425,24 @@ task "bigtop-slaves"(type:Exec, commandLine command } +task "docker-package"(type:Exec, +description: 'Build packages via docker build.\n' + --- End diff -- You can use triple-quotes for multi-line strings instead of doing string concatenations Java-style. Something like ```"""Build packages via docker build. Usage: $ ./gradlew""" ```and so on. ---
[GitHub] bigtop pull request #310: BIGTOP-2949. Add gradle task which leverage bigtop...
GitHub user evans-ye opened a pull request: https://github.com/apache/bigtop/pull/310 BIGTOP-2949. Add gradle task which leverage bigtop-ci/build.sh to build packages You can merge this pull request into a Git repository by running: $ git pull https://github.com/evans-ye/bigtop BIGTOP-2949 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/bigtop/pull/310.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #310 commit 88079efd80865f8a2db106e98144e7997c59d50b Author: Evans Ye Date: 2017-11-28T16:22:14Z BIGTOP-2949. Add gradle task which leverage bigtop-ci/build.sh to build packages ---
