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

Handle deletion of symlinks: update gradle

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.0
    • Fix Version/s: 1.2.1
    • Component/s: build, toolchain
    • Labels:
      None

      Description

      We get various errors in CI because gradle < 2.13 does not handle deletion of symlinks correctly.

      Please compare https://docs.gradle.org/2.13/dsl/org.gradle.api.tasks.Delete.html (2.13) with https://docs.gradle.org/2.12/dsl/org.gradle.api.tasks.Delete.html (2.12)

      Looks like 2.14.1 is a suitable quick win.

      Errors are for instance:

      Execution failed for task ':clean'.
      > Unable to delete file: /ws/build/hue/rpm/BUILD/hue-release-3.11.0/build/env/include/python2.7/pyconfig.h
      
      1. 0001-BIGTOP-2808.-Handle-deletion-of-symlinks-update-grad.patch
        6 kB
        Konstantin Boudnik
      2. 0001-BIGTOP-2808.-Handle-deletion-of-symlinks-update-grad.patch
        6 kB
        Konstantin Boudnik
      3. BIGTOP-2808.1.patch
        14 kB
        Olaf Flebbe
      4. BIGTOP-2808.2.patch
        75 kB
        Olaf Flebbe
      5. BIGTOP-2808.3.patch
        75 kB
        Olaf Flebbe

        Issue Links

          Activity

          Hide
          oflebbe Olaf Flebbe added a comment - - edited

          Problems to be solved:

          The buildSrc groovy files did not compile correctly because of failed dependencies, reworked by two lines in packages.gradle.

          removed buildSrc cruft

          The download task was broken by an intern gradle change, upgrade it to a recent version

          updated gradlew

          updated gradle in toolchain

          Show
          oflebbe Olaf Flebbe added a comment - - edited Problems to be solved: The buildSrc groovy files did not compile correctly because of failed dependencies, reworked by two lines in packages.gradle. removed buildSrc cruft The download task was broken by an intern gradle change, upgrade it to a recent version updated gradlew updated gradle in toolchain
          Hide
          cos Konstantin Boudnik added a comment -

          A couple of things that went missing:

          • the ALv2 header in the gradlew script
          • exclusion of buildSrc/build/** in the RAT configuration for build.gradle. Not sure why is that.
          • when I run ./gradlew with this patch in place I am getting the following
            Error: Could not find or load main class org.gradle.wrapper.GradleWrapperMain
            

          The rest of the changes looks ok, with one more comment.

          Now, I assume new version of gradlew script is what you got from the gradle wrapper, right? It comes out a bit different from what we have in the source tree. The reason we have a customized version of the script, is not to download the gradle-wrapper.jar on every run, IIRC.

          Show
          cos Konstantin Boudnik added a comment - A couple of things that went missing: the ALv2 header in the gradlew script exclusion of buildSrc/build/** in the RAT configuration for build.gradle. Not sure why is that. when I run ./gradlew with this patch in place I am getting the following Error: Could not find or load main class org.gradle.wrapper.GradleWrapperMain The rest of the changes looks ok, with one more comment. Now, I assume new version of gradlew script is what you got from the gradle wrapper , right? It comes out a bit different from what we have in the source tree. The reason we have a customized version of the script, is not to download the gradle-wrapper.jar on every run, IIRC.
          Hide
          oflebbe Olaf Flebbe added a comment -

          I would vote not to to manually edit a machine generated script and exclude gradlew from rat. That would solve (1

          Removed buildSrc altogether, it is not needed any more. The buildSrc you are seeing are from previous runs. (2)

          Regenerated gradlew with "gradle wrapper". So I presume any change not to download gradle-wrapper.jar is futile. (3)

          Gradle does not download gradle-wrapper.jar every run. Evans added logic to bigtop_toolchain to download everything necessary from gradle. Changes to gradlew are not necessary.

          Show
          oflebbe Olaf Flebbe added a comment - I would vote not to to manually edit a machine generated script and exclude gradlew from rat. That would solve (1 Removed buildSrc altogether, it is not needed any more. The buildSrc you are seeing are from previous runs. (2) Regenerated gradlew with "gradle wrapper". So I presume any change not to download gradle-wrapper.jar is futile. (3) Gradle does not download gradle-wrapper.jar every run. Evans added logic to bigtop_toolchain to download everything necessary from gradle. Changes to gradlew are not necessary.
          Hide
          cos Konstantin Boudnik added a comment -

          Well, I guess I can see that. However, with the patch in place (on a clean workspace), after running toolchain update I still see the error message about the missing class I just mentioned above, because the jar file would be missing in the clean git workspace.

          And now I remember why we did the custom script: with the generated one we have to keep the binary jar file in the source code (under gradle/wrapper) and we cannot do that because we cannot keep the binaries as a part of our source release (see the discussion on BIGTOP-1384).

          Show
          cos Konstantin Boudnik added a comment - Well, I guess I can see that. However, with the patch in place (on a clean workspace), after running toolchain update I still see the error message about the missing class I just mentioned above, because the jar file would be missing in the clean git workspace. And now I remember why we did the custom script: with the generated one we have to keep the binary jar file in the source code (under gradle/wrapper ) and we cannot do that because we cannot keep the binaries as a part of our source release (see the discussion on BIGTOP-1384 ).
          Hide
          oflebbe Olaf Flebbe added a comment -

          Oh gradle now needs a (binary) jar file included into the distribution
          https://docs.gradle.org/2.14/userguide/gradle_wrapper.html

          Show
          oflebbe Olaf Flebbe added a comment - Oh gradle now needs a (binary) jar file included into the distribution https://docs.gradle.org/2.14/userguide/gradle_wrapper.html
          Hide
          oflebbe Olaf Flebbe added a comment -

          with included gradle-wrapper.jar

          Show
          oflebbe Olaf Flebbe added a comment - with included gradle-wrapper.jar
          Hide
          oflebbe Olaf Flebbe added a comment -

          Updated rat test. Does not make sense to include ASL Licenses in a machine-generated files.

          Show
          oflebbe Olaf Flebbe added a comment - Updated rat test. Does not make sense to include ASL Licenses in a machine-generated files.
          Hide
          evans_ye Evans Ye added a comment - - edited

          Anyone with the expertise can help? Konstantin Boudnik Roman Shaposhnik?

          Show
          evans_ye Evans Ye added a comment - - edited Anyone with the expertise can help? Konstantin Boudnik Roman Shaposhnik ?
          Hide
          cos Konstantin Boudnik added a comment -

          Unfortunately, we can not include the binary jar into the source code because of the release requirements.
          Let me play with that and see if I can come up with some workaround perhaps (I have no idea though...)

          Show
          cos Konstantin Boudnik added a comment - Unfortunately, we can not include the binary jar into the source code because of the release requirements. Let me play with that and see if I can come up with some workaround perhaps (I have no idea though...)
          Hide
          oflebbe Olaf Flebbe added a comment -

          If we cannot distribute the wrapper, policywise: Then I would propose to remove the wrapper.
          Since we already have the gradle distro within our docker toolchain the wrapper is technically not even necessary.

          Everyone else has to download gradle and recreate the wrapper, which is compareable to running autoconf to create "configure".

          Show
          oflebbe Olaf Flebbe added a comment - If we cannot distribute the wrapper, policywise: Then I would propose to remove the wrapper. Since we already have the gradle distro within our docker toolchain the wrapper is technically not even necessary. Everyone else has to download gradle and recreate the wrapper, which is compareable to running autoconf to create "configure".
          Hide
          cos Konstantin Boudnik added a comment - - edited

          Can we do this instead? I kept most of your patch intact, just worked around the gradlew part.
          Custom gradlew isn't inherently evil? In fact, I like the fact that everything I need is there no matter if I use the official docker image or not. And on some platforms (read MacOS) Puppet isn't readily available (which unfortunately is my PITA at my current job).

          As a part of the patch I also took the liberty to bump the Gradle to the latest 4.0 version, if no one minds. Thanks!

          Show
          cos Konstantin Boudnik added a comment - - edited Can we do this instead? I kept most of your patch intact, just worked around the gradlew part. Custom gradlew isn't inherently evil? In fact, I like the fact that everything I need is there no matter if I use the official docker image or not. And on some platforms (read MacOS) Puppet isn't readily available (which unfortunately is my PITA at my current job). As a part of the patch I also took the liberty to bump the Gradle to the latest 4.0 version, if no one minds. Thanks!
          Hide
          oflebbe Olaf Flebbe added a comment -

          Urgh. Does that really work ?

          Did you really tested toolchain run, packaging and provisioner with gradle 4.0 ? I only tried 2.14.1.
          Please keep in mind that the target of the fix is 1.2.1, not 1.3.

          Show
          oflebbe Olaf Flebbe added a comment - Urgh. Does that really work ? Did you really tested toolchain run, packaging and provisioner with gradle 4.0 ? I only tried 2.14.1. Please keep in mind that the target of the fix is 1.2.1, not 1.3.
          Hide
          cos Konstantin Boudnik added a comment -

          Fair enough, keeping it at 2.14.1
          Will open a separate JIRA for upgrade

          Show
          cos Konstantin Boudnik added a comment - Fair enough, keeping it at 2.14.1 Will open a separate JIRA for upgrade
          Hide
          oflebbe Olaf Flebbe added a comment -

          Thanks! Your latest patch LGTM +1

          Show
          oflebbe Olaf Flebbe added a comment - Thanks! Your latest patch LGTM +1
          Hide
          cos Konstantin Boudnik added a comment -

          Thanks, I have committed and pushed this. Closing now.
          Also I accidentally pushed (and immediately reverted) BIGTOP-2825.

          Show
          cos Konstantin Boudnik added a comment - Thanks, I have committed and pushed this. Closing now. Also I accidentally pushed (and immediately reverted) BIGTOP-2825 .
          Hide
          cos Konstantin Boudnik added a comment -

          Committed and pushed to the master.

          Show
          cos Konstantin Boudnik added a comment - Committed and pushed to the master.

            People

            • Assignee:
              oflebbe Olaf Flebbe
              Reporter:
              oflebbe Olaf Flebbe
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development