Uploaded image for project: 'Bigtop'
  1. Bigtop
  2. BIGTOP-1384

Implement Gradle Wrapper for smoke tests and cleanup.

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.8.0
    • Fix Version/s: 1.0.0
    • Component/s: build
    • Labels:

      Description

      By adding a the infamous gradle wrapper script (this is the idiom in gradle - to use the wrapper instead of a local gradle install) into our VCS, we gaurantee that the gradle tricks we implement will be running the exact same on all systems, no matter what.

      • Also, it opens the tests up to be runnable by anyone, even those who don't have gradle installed.
      • Finally, gradle wrapper will embolden us to be able to use more sophisticated gradle tricks newer features, b/c we will know that they run the same in all environments.

      So, this task consists of :

      1) Adding gradle wrapper to the bigtop-smoke-tests
      2) Implementing cleanup for the build.gradle files also, possibly allowing for version specific features (i.e. advanced dependency inheritance for subprojects etc)
      3) Update README with new instructions for people running the tests.

      1. BIGTOP-1384.patch
        15 kB
        David Capwell
      2. BIGTOP-1384.2.patch
        22 kB
        David Capwell
      3. BIGTOP-1384.3.patch
        19 kB
        David Capwell

        Issue Links

          Activity

          Hide
          cos Konstantin Boudnik added a comment -

          Thanks David! It was on my to-do list for this evening, but @rvs has beat me to it, taking advantage of being +11 hours

          Show
          cos Konstantin Boudnik added a comment - Thanks David! It was on my to-do list for this evening, but @rvs has beat me to it, taking advantage of being +11 hours
          Hide
          rvs Roman Shaposhnik added a comment -

          +1 and committed! Thanks a lot for the patch!

          Show
          rvs Roman Shaposhnik added a comment - +1 and committed! Thanks a lot for the patch!
          Hide
          dcapwell David Capwell added a comment -

          Konstantin Boudnik, jay vyas. Updated based off feedback.

          gradlew is no longer the one provided by gradle, but really a fork that merges all the logic that the java code had done. This will mean that we don't need a binary in the source, but for users its the same experience.

          Show
          dcapwell David Capwell added a comment - Konstantin Boudnik , jay vyas . Updated based off feedback. gradlew is no longer the one provided by gradle, but really a fork that merges all the logic that the java code had done. This will mean that we don't need a binary in the source, but for users its the same experience.
          Hide
          dcapwell David Capwell added a comment -

          I'll try to fix this tomorrow
          On Oct 29, 2014 9:23 PM, "Konstantin Boudnik (JIRA)" <jira@apache.org>

          Show
          dcapwell David Capwell added a comment - I'll try to fix this tomorrow On Oct 29, 2014 9:23 PM, "Konstantin Boudnik (JIRA)" <jira@apache.org>
          Hide
          cos Konstantin Boudnik added a comment -

          BTW, guys - the patch doesn't apply cleanly on the master - it complains about existing files.

          Show
          cos Konstantin Boudnik added a comment - BTW, guys - the patch doesn't apply cleanly on the master - it complains about existing files.
          Hide
          jayunit100 jay vyas added a comment - - edited

          okay great it works by just running gradle-bootstrap ++ gradlew .

          Id like to suggest we add one line to our custom gradlew which runs the gradle-bootstrap.

          • the reason is, that it will make the bigtop user experience awesome.
          • it will save us time knowing we dont have to maintain "install gradle" instructions in every damn readme .

          Ive not used gradlew much, so i could be wrong. but id like feedback b4 we put this in about wether we should just go all out and make gradlew a fully bootstarapping wrapper to gradle tasks in bigtop

          David and cos and i have gone back and forth on the idea. i think it will make for the best user experience, so my vote is to add that one extra line of bootstrapping magic, unless there is a serious technical reason not to do so.

          Show
          jayunit100 jay vyas added a comment - - edited okay great it works by just running gradle-bootstrap ++ gradlew . Id like to suggest we add one line to our custom gradlew which runs the gradle-bootstrap . the reason is, that it will make the bigtop user experience awesome. it will save us time knowing we dont have to maintain "install gradle" instructions in every damn readme . Ive not used gradlew much, so i could be wrong. but id like feedback b4 we put this in about wether we should just go all out and make gradlew a fully bootstarapping wrapper to gradle tasks in bigtop David and cos and i have gone back and forth on the idea. i think it will make for the best user experience, so my vote is to add that one extra line of bootstrapping magic , unless there is a serious technical reason not to do so.
          Hide
          dcapwell David Capwell added a comment -

          jay vyas, updated the patch.

          Show
          dcapwell David Capwell added a comment - jay vyas , updated the patch.
          Hide
          jayunit100 jay vyas added a comment - - edited

          so, back on the subject:
          1) Is there an advantage to having gradlew in the top level of bigtop.? I think there can be:

          • bigtop smoke tests have no need for additional dependencies
          • running bigtop build doesnt require downloading puppet or gradle or ...

          2) If thats the case, is it okay to hardcode the version in gradlew ? I think so. If it causes a problem, we can parameterize it. David Capwell I presume you are okay to help maintain gradlew as well ?

          Show
          jayunit100 jay vyas added a comment - - edited so, back on the subject: 1) Is there an advantage to having gradlew in the top level of bigtop.? I think there can be: bigtop smoke tests have no need for additional dependencies running bigtop build doesnt require downloading puppet or gradle or ... 2) If thats the case, is it okay to hardcode the version in gradlew ? I think so. If it causes a problem, we can parameterize it. David Capwell I presume you are okay to help maintain gradlew as well ?
          Hide
          jayunit100 jay vyas added a comment -

          i agree - definetly weird. instead at some point lets make sure that the puppet bootstrapper and gradlew both read gradle url from the same source : to avoid divergent gradle versions in bigtop

          Show
          jayunit100 jay vyas added a comment - i agree - definetly weird. instead at some point lets make sure that the puppet bootstrapper and gradlew both read gradle url from the same source : to avoid divergent gradle versions in bigtop
          Hide
          cos Konstantin Boudnik added a comment -

          .... puppet right now does a download of gradle in the puppet toolchain code

          this is very true. But at the moment toolchain is capable of bootstrapping the environment without relying on anything in the bigtop source code or build. So I agree with David - it'd feel a bit weird if we will be calling gradlew from a recipe down below. May be this is just me?

          Show
          cos Konstantin Boudnik added a comment - .... puppet right now does a download of gradle in the puppet toolchain code this is very true. But at the moment toolchain is capable of bootstrapping the environment without relying on anything in the bigtop source code or build. So I agree with David - it'd feel a bit weird if we will be calling gradlew from a recipe down below. May be this is just me?
          Hide
          dcapwell David Capwell added a comment -

          Ah! Thanks for explaining

          Show
          dcapwell David Capwell added a comment - Ah! Thanks for explaining
          Hide
          jayunit100 jay vyas added a comment - - edited

          well.... puppet right now does a download of gradle in the puppet toolchain code . I guess we can ignore that fact for now........... lets take a look at the final patch, and then we can confirm the idea of how gradlew will be used w/ bigtop w / cos

          Show
          jayunit100 jay vyas added a comment - - edited well.... puppet right now does a download of gradle in the puppet toolchain code . I guess we can ignore that fact for now........... lets take a look at the final patch, and then we can confirm the idea of how gradlew will be used w/ bigtop w / cos
          Hide
          dcapwell David Capwell added a comment -

          Puppet calling gradle feel weird to me. Where would this be?

          Show
          dcapwell David Capwell added a comment - Puppet calling gradle feel weird to me. Where would this be?
          Hide
          jayunit100 jay vyas added a comment -

          If we are all okay with gradlew in top level, Can we also modify puppet tool chain simple use his gradlew as well ?

          Show
          jayunit100 jay vyas added a comment - If we are all okay with gradlew in top level, Can we also modify puppet tool chain simple use his gradlew as well ?
          Hide
          dcapwell David Capwell added a comment -

          The idea from gradles side is that it forces everyone to use the same
          binary.

          If someone wants to update gradle they do it for the project and everyone
          uses that new version.

          Because we can't add a binary, this causes the user to call the bootstrap
          script when anyone updates gradle version. This is going to be the biggest
          issue with gradle updating.
          On Oct 28, 2014 10:57 AM, "Konstantin Boudnik (JIRA)" <jira@apache.org>

          Show
          dcapwell David Capwell added a comment - The idea from gradles side is that it forces everyone to use the same binary. If someone wants to update gradle they do it for the project and everyone uses that new version. Because we can't add a binary, this causes the user to call the bootstrap script when anyone updates gradle version. This is going to be the biggest issue with gradle updating. On Oct 28, 2014 10:57 AM, "Konstantin Boudnik (JIRA)" <jira@apache.org>
          Hide
          cos Konstantin Boudnik added a comment -

          Agree on checking-in the source code. As for the hard-coded version: I am uneasy about it because this is sort of things that tends to be forgotten later and come back to bite you in the back when an incompatibility is introduced or something. I might be overly cautious here though.

          Show
          cos Konstantin Boudnik added a comment - Agree on checking-in the source code. As for the hard-coded version: I am uneasy about it because this is sort of things that tends to be forgotten later and come back to bite you in the back when an incompatibility is introduced or something. I might be overly cautious here though.
          Hide
          hsaputra Henry Saputra added a comment - - edited

          Hi Konstantin Boudnik, it will be easier to check in the gradlew file and we also need ASF license header so committing it to source repo should be the easiest way to go.
          Putting specific version in the gradlew file should be ok which allow uniform usage of same version.

          Show
          hsaputra Henry Saputra added a comment - - edited Hi Konstantin Boudnik , it will be easier to check in the gradlew file and we also need ASF license header so committing it to source repo should be the easiest way to go. Putting specific version in the gradlew file should be ok which allow uniform usage of same version.
          Hide
          dcapwell David Capwell added a comment -

          Thanks for the heads up!

          Show
          dcapwell David Capwell added a comment - Thanks for the heads up!
          Hide
          jayunit100 jay vyas added a comment - - edited

          David Capwell also you have 2 do a quick rebase tomorrow as well ! commiting BIGTOP-1498 now. sorry about the (minor) collision . then we can discuss the cos suggestions as well. ...

          Show
          jayunit100 jay vyas added a comment - - edited David Capwell also you have 2 do a quick rebase tomorrow as well ! commiting BIGTOP-1498 now. sorry about the (minor) collision . then we can discuss the cos suggestions as well. ...
          Hide
          dcapwell David Capwell added a comment -

          Konstantin Boudnik, thanks for the feedback.

          "I don't like that exact version, e.g. gradle-2.1-bin.zip, is hardcoded. Is it a requirement?"

          This is gradle's configuration; this tells gradle where to pull the binaries from. Normally auto-generated by running

          gradle wrapper

          (which is how I created it).

          "do we really need to check-in gradlew script? My impressions was that the damn thing got generated on every run of the gradle build, isn't it?"

          gradlew gets generated by calling

          gradle wrapper

          . The gradlew file knows how to work with different versions of gradle and will install it for you (assuming gradle-wrapper.jar is local). Only way I can think to remove gradlew is to do what sbt has done, a global shell script that reads the configs and does all the logic the wrapper does (https://github.com/paulp/sbt-extras/blob/master/sbt).

          "can we specify that the version needs to be 'greater than' instead of 'can equal' (whatever it means)"

          I can make the change but the largest value that worked for me was 0.8.0. I assume that I need to populate the gradle artifact cache in order to get something like 0.9.0-SNAPSHOT to work. Ill make this change tomorrow as well.

          Show
          dcapwell David Capwell added a comment - Konstantin Boudnik , thanks for the feedback. "I don't like that exact version, e.g. gradle-2.1-bin.zip, is hardcoded. Is it a requirement?" This is gradle's configuration; this tells gradle where to pull the binaries from. Normally auto-generated by running gradle wrapper (which is how I created it). "do we really need to check-in gradlew script? My impressions was that the damn thing got generated on every run of the gradle build, isn't it?" gradlew gets generated by calling gradle wrapper . The gradlew file knows how to work with different versions of gradle and will install it for you (assuming gradle-wrapper.jar is local). Only way I can think to remove gradlew is to do what sbt has done, a global shell script that reads the configs and does all the logic the wrapper does ( https://github.com/paulp/sbt-extras/blob/master/sbt ). "can we specify that the version needs to be 'greater than' instead of 'can equal' (whatever it means)" I can make the change but the largest value that worked for me was 0.8.0. I assume that I need to populate the gradle artifact cache in order to get something like 0.9.0-SNAPSHOT to work. Ill make this change tomorrow as well.
          Hide
          jayunit100 jay vyas added a comment - - edited

          thanks for the feedback cos .... david please let us know your thoughtson these ^ also david you will need to do git pull --rebase because smoke-tests.sh just changed .

          Show
          jayunit100 jay vyas added a comment - - edited thanks for the feedback cos .... david please let us know your thoughtson these ^ also david you will need to do git pull --rebase because smoke-tests.sh just changed .
          Hide
          cos Konstantin Boudnik added a comment -

          A couple of comments:

          • I don't like that exact version, e.g. gradle-2.1-bin.zip, is hardcoded. Is it a requirement?
          • do we really need to check-in gradlew script? My impressions was that the damn thing got generated on every run of the gradle build, isn't it?
          • can we specify that the version needs to be 'greater than' instead of 'can equal' (whatever it means)
            -  * itest can equal = 0.7.0, 0.8.0-SNAPSHOT,
            +  * itest can equal = 0.7.0, 0.8.0,
            

            I will wait for the updated version of the patch before digging deeper. Thanks for doing this

          Show
          cos Konstantin Boudnik added a comment - A couple of comments: I don't like that exact version, e.g. gradle-2.1-bin.zip, is hardcoded. Is it a requirement? do we really need to check-in gradlew script? My impressions was that the damn thing got generated on every run of the gradle build, isn't it? can we specify that the version needs to be 'greater than' instead of 'can equal' (whatever it means) - * itest can equal = 0.7.0, 0.8.0-SNAPSHOT, + * itest can equal = 0.7.0, 0.8.0, I will wait for the updated version of the patch before digging deeper. Thanks for doing this
          Hide
          dcapwell David Capwell added a comment -

          I'll update tomorrow

          Show
          dcapwell David Capwell added a comment - I'll update tomorrow
          Hide
          jayunit100 jay vyas added a comment - - edited

          David Capwell thanks for the patch.

          To get the gradle wrapper to work w/ smoke tests, two further things to be done. we willl need to:

          1) chmod gradlew to executable
          2) in the smoke-tests, change ./gradlew to /bigtop-home/gradlew

          FYI, i remember you mentioning that you needed root password. heres the easy workaround: you can just use "vagrant" as root password to become root. then run tests as root.

          After making chanes (1) and (2) above, im pretty sure vagrant up will work perfectly.

          Can you resubmit with those changes and (3) either use git format-patch to make a commit with your author attribution or tell me the author user name to use .

          Thanks again ... looks pretty close . Im running a final test to confirm if any other mods are needed.

          .... UPDATE .... confirmed that, with the above two modifications, the smoke-tests.sh script runs perfectly fine in the vagrant created boxes, w/ your gradlew replacement. ! So feel free to make those 2 changes, and resubmit - and ill commit tomorrow.

          Show
          jayunit100 jay vyas added a comment - - edited David Capwell thanks for the patch. To get the gradle wrapper to work w/ smoke tests, two further things to be done. we willl need to: 1) chmod gradlew to executable 2) in the smoke-tests, change ./gradlew to /bigtop-home/gradlew FYI, i remember you mentioning that you needed root password. heres the easy workaround : you can just use "vagrant" as root password to become root. then run tests as root. After making chanes (1) and (2) above, im pretty sure vagrant up will work perfectly. Can you resubmit with those changes and (3) either use git format-patch to make a commit with your author attribution or tell me the author user name to use . Thanks again ... looks pretty close . Im running a final test to confirm if any other mods are needed. .... UPDATE .... confirmed that, with the above two modifications, the smoke-tests.sh script runs perfectly fine in the vagrant created boxes, w/ your gradlew replacement. ! So feel free to make those 2 changes, and resubmit - and ill commit tomorrow.
          Hide
          dcapwell David Capwell added a comment -

          Thanks. Will keep that in mind

          Show
          dcapwell David Capwell added a comment - Thanks. Will keep that in mind
          Hide
          jayunit100 jay vyas added a comment -

          David Capwell FYI, normally we encourage use of git format-patch like this : https://cwiki.apache.org/confluence/display/BIGTOP/How+to+Contribute ... Just something to keep in mind ! Applied with patch -p1 and now testing............

          Show
          jayunit100 jay vyas added a comment - David Capwell FYI, normally we encourage use of git format-patch like this : https://cwiki.apache.org/confluence/display/BIGTOP/How+to+Contribute ... Just something to keep in mind ! Applied with patch -p1 and now testing............
          Hide
          jayunit100 jay vyas added a comment -

          Thanks David. Good idea to split them up. I will test it out shortly and let you know .

          Show
          jayunit100 jay vyas added a comment - Thanks David. Good idea to split them up. I will test it out shortly and let you know .
          Hide
          dcapwell David Capwell added a comment -

          This patch adds gradle wrapper (w/o binary) for top level and smoke tests.

          I plan to split all the gradle improvements into different JIRAs

          Show
          dcapwell David Capwell added a comment - This patch adds gradle wrapper (w/o binary) for top level and smoke tests. I plan to split all the gradle improvements into different JIRAs
          Hide
          jayunit100 jay vyas added a comment -

          looks like David Capwell is now working this angle and has some interesting ideas on how to get around the ASF limitations. looking forward to upcoming patch.

          Show
          jayunit100 jay vyas added a comment - looks like David Capwell is now working this angle and has some interesting ideas on how to get around the ASF limitations. looking forward to upcoming patch. davids additions will install gradle on the fly so smoke-tests have zero system expectations Also i think this patch will update the https://github.com/apache/bigtop/blob/master/bigtop-deploy/vm/vagrant-puppet/smoke-tests.sh recipes to use the gradle bash wrapper instead of the puppet based toolchain installer
          Hide
          cos Konstantin Boudnik added a comment -

          Just a point of reference: ASF release can not include jar files - only the source code. And the Gradle wrapper requires a bootstrap jar file to be checked-in along with the rest of an application source code. Here's the proof-link http://www.apache.org/dev/release-publishing.html#valid

          Show
          cos Konstantin Boudnik added a comment - Just a point of reference: ASF release can not include jar files - only the source code. And the Gradle wrapper requires a bootstrap jar file to be checked-in along with the rest of an application source code. Here's the proof-link http://www.apache.org/dev/release-publishing.html#valid
          Hide
          jayunit100 jay vyas added a comment -

          We need to finish 1222 first, obviously.

          Show
          jayunit100 jay vyas added a comment - We need to finish 1222 first, obviously.

            People

            • Assignee:
              dcapwell David Capwell
              Reporter:
              jayunit100 jay vyas
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development