Uploaded image for project: 'Bigtop'
  1. Bigtop
  2. BIGTOP-1309 Gradle environment overhaul
  3. BIGTOP-2051

Get rid of hair-brain environment vars left after make-based build

    Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.0
    • Fix Version/s: 1.1.0
    • Component/s: build
    • Labels:
      None

      Description

      The last piece of the old make based build is the massive map of environment variables we had to keep around to avoid breaking everything in the builds. Let's get rid of it and start using the configuration object, build from the new DSL

      1. BIGTOP-2051.patch
        28 kB
        Konstantin Boudnik
      2. BIGTOP-2051.patch
        27 kB
        Konstantin Boudnik
      3. BIGTOP-2051.patch
        26 kB
        Konstantin Boudnik
      4. BIGTOP-2051.patch
        26 kB
        Konstantin Boudnik

        Issue Links

          Activity

          Hide
          cos Konstantin Boudnik added a comment -

          I believe this is the working version of the package build logic that is completely free of the make-like properties

          Show
          cos Konstantin Boudnik added a comment - I believe this is the working version of the package build logic that is completely free of the make-like properties
          Hide
          cos Konstantin Boudnik added a comment -

          Please review. This is the penultimate step to complete the build cleanup

          Show
          cos Konstantin Boudnik added a comment - Please review. This is the penultimate step to complete the build cleanup
          Hide
          rnowling RJ Nowling added a comment -

          +1

          Show
          rnowling RJ Nowling added a comment - +1
          Hide
          oflebbe Olaf Flebbe added a comment - - edited

          -1

          Please remove more envorinment variables

          --preserve-envvar JAVA32_HOME \
          --preserve-envvar JAVA64_HOME \
          --preserve-envvar FORREST_HOME \
          
          Show
          oflebbe Olaf Flebbe added a comment - - edited -1 Please remove more envorinment variables --preserve-envvar JAVA32_HOME \ --preserve-envvar JAVA64_HOME \ --preserve-envvar FORREST_HOME \
          Hide
          cos Konstantin Boudnik added a comment -

          These are actual variables that aren't coming from the BOM file. Arguably a separate issue.

          Show
          cos Konstantin Boudnik added a comment - These are actual variables that aren't coming from the BOM file. Arguably a separate issue.
          Hide
          oflebbe Olaf Flebbe added a comment - - edited

          Did you actually do a full rebuild on both RPM and DEB? Reading the patch I am not convinced it wont break.

          Show
          oflebbe Olaf Flebbe added a comment - - edited Did you actually do a full rebuild on both RPM and DEB? Reading the patch I am not convinced it wont break.
          Hide
          oflebbe Olaf Flebbe added a comment -

          Ok, will file a seperate issue. Revoking my -1

          Show
          oflebbe Olaf Flebbe added a comment - Ok, will file a seperate issue. Revoking my -1
          Hide
          rnowling RJ Nowling added a comment - - edited

          No, I haven't.

          I've started a full rebuild of the RPMs.

          Show
          rnowling RJ Nowling added a comment - - edited No, I haven't. I've started a full rebuild of the RPMs.
          Hide
          cos Konstantin Boudnik added a comment -

          Thanks, appreciate the thorough review.

          Show
          cos Konstantin Boudnik added a comment - Thanks, appreciate the thorough review.
          Hide
          cos Konstantin Boudnik added a comment -

          You need to change 'ignite' name in the bigtop.bom to 'ignite-hadoop': otherwise it won't work. I will upload the patch that enforces the requirement.

          Show
          cos Konstantin Boudnik added a comment - You need to change 'ignite' name in the bigtop.bom to 'ignite-hadoop': otherwise it won't work. I will upload the patch that enforces the requirement.
          Hide
          rnowling RJ Nowling added a comment -
          Show
          rnowling RJ Nowling added a comment - Thanks, Konstantin Boudnik
          Hide
          cos Konstantin Boudnik added a comment -

          This patch differs from the reviewed one by only enforcing component label to be equal to the component name.

          Show
          cos Konstantin Boudnik added a comment - This patch differs from the reviewed one by only enforcing component label to be equal to the component name.
          Hide
          oflebbe Olaf Flebbe added a comment - - edited

          Why use rpmbuild with environment

            '--define', "${NAME}_base_version $BASE_VERSION",
          

          and debuild with

          --set-envvar=${toOldStyleName(target)}_BASE_VERSION=$BASE_VERSION \
          

          ARghh forget it.

          Show
          oflebbe Olaf Flebbe added a comment - - edited Why use rpmbuild with environment '--define', "${NAME}_base_version $BASE_VERSION" , and debuild with --set-envvar=${toOldStyleName(target)}_BASE_VERSION=$BASE_VERSION \ ARghh forget it.
          Hide
          cos Konstantin Boudnik added a comment -

          I was intentionally not touching parts that aren't related to the task at hands, namely getting rid of the last shreds of make heritage. I am strictly sticking to the rule of doing JIRA/commit per logic change. Otherwise it is a nightmare to follow why certain changes were made.

          Show
          cos Konstantin Boudnik added a comment - I was intentionally not touching parts that aren't related to the task at hands, namely getting rid of the last shreds of make heritage. I am strictly sticking to the rule of doing JIRA/commit per logic change. Otherwise it is a nightmare to follow why certain changes were made.
          Hide
          oflebbe Olaf Flebbe added a comment -

          I checked some deb packages and checked the code a bit..

          LGTM +1

          Show
          oflebbe Olaf Flebbe added a comment - I checked some deb packages and checked the code a bit.. LGTM +1
          Hide
          cos Konstantin Boudnik added a comment - - edited

          Thanks. Will commit once RJ is back with his validation.

          Show
          cos Konstantin Boudnik added a comment - - edited Thanks. Will commit once RJ is back with his validation.
          Hide
          rnowling RJ Nowling added a comment - - edited

          I'm getting some errors:

          [ERROR] Could not find the selected project in the reactor: !modules/yarn -> [Help 1]
          [ERROR] 
          [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
          [ERROR] Re-run Maven using the -X switch to enable full debug logging.
          [ERROR] 
          [ERROR] For more information about the errors and possible solutions, please read the following articles:
          [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MavenExecutionException
          error: Bad exit status from /var/tmp/rpm-tmp.UWYOom (%build)
          

          Coming from the ignite-hadoop rpm build.

          And from the pig-rpm build, I get this:

              [javac] Compiling 981 source files to /home/rnowling/Projects/bigtop_2051/build/pig/rpm/BUILD/pig-0.14.0-src/build/classes
              [javac] javac: invalid target release: 
              [javac] Usage: javac <options> <source files>
              [javac] use -help for a list of possible options
          

          Error with the phoenix build:

          :phoenix-download
          Download http://apache.osuosl.org//phoenix/phoenix-4.4.0-HBase-0.98/src/phoenix-4.4.0-HBase-0.98-src.tar.gz
          :phoenix-tar
          Copy /home/rnowling/Projects/bigtop_2051/dl/phoenix-4.4.0-HBase-0.98-src.tar.gz to /home/rnowling/Projects/bigtop_2051/build/phoenix/tar/phoenix-4.4.0-HBase-0.98-src.tar.gz
          :phoenix-srpm
          error: File /home/rnowling/Projects/bigtop_2051/build/phoenix/rpm/SOURCES/phoenix-4.4.0-src.tar.gz: No such file or directory
          :phoenix-srpm FAILED
          
          Show
          rnowling RJ Nowling added a comment - - edited I'm getting some errors: [ERROR] Could not find the selected project in the reactor: !modules/yarn -> [Help 1] [ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch. [ERROR] Re-run Maven using the -X switch to enable full debug logging. [ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles: [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MavenExecutionException error: Bad exit status from /var/tmp/rpm-tmp.UWYOom (%build) Coming from the ignite-hadoop rpm build. And from the pig-rpm build, I get this: [javac] Compiling 981 source files to /home/rnowling/Projects/bigtop_2051/build/pig/rpm/BUILD/pig-0.14.0-src/build/classes [javac] javac: invalid target release: [javac] Usage: javac <options> <source files> [javac] use -help for a list of possible options Error with the phoenix build: :phoenix-download Download http://apache.osuosl.org//phoenix/phoenix-4.4.0-HBase-0.98/src/phoenix-4.4.0-HBase-0.98-src.tar.gz :phoenix-tar Copy /home/rnowling/Projects/bigtop_2051/dl/phoenix-4.4.0-HBase-0.98-src.tar.gz to /home/rnowling/Projects/bigtop_2051/build/phoenix/tar/phoenix-4.4.0-HBase-0.98-src.tar.gz :phoenix-srpm error: File /home/rnowling/Projects/bigtop_2051/build/phoenix/rpm/SOURCES/phoenix-4.4.0-src.tar.gz: No such file or directory :phoenix-srpm FAILED
          Hide
          cos Konstantin Boudnik added a comment -
          • ignite-hadoop issue is addressed in BIGTOP-2053
            Looking into other two...
          Show
          cos Konstantin Boudnik added a comment - ignite-hadoop issue is addressed in BIGTOP-2053 Looking into other two...
          Hide
          cos Konstantin Boudnik added a comment -

          The issue with phoenix is a real bug in the srpm code, introduced with this patch. Here's the fix

          diff --git packages.gradle packages.gradle
          index d2c0979..86b8068 100644
          --- packages.gradle
          +++ packages.gradle
          @@ -441,7 +441,7 @@ def genTasks = { target ->
               def final PKG_BUILD_DIR = config.bigtop.components[target].builddir
               def final SEED_TAR = config.bigtop.components[target].seedtar
               def final PKG_VERSION = config.bigtop.components[target].version.pkg
          -    def final BASE_VERSION = config.bigtop.components[target].version.pkg
          +    def final BASE_VERSION = config.bigtop.components[target].version.base
               def final PKG_OUTPUT_DIR = config.bigtop.components[target].outputdir
               delete ("$PKG_BUILD_DIR/rpm")
               ['INSTALL','SOURCES','BUILD','SRPMS','RPMS'].each { rpmdir ->
          

          will be applied to the next version of the patch. Still looking at Pig

          Show
          cos Konstantin Boudnik added a comment - The issue with phoenix is a real bug in the srpm code, introduced with this patch. Here's the fix diff --git packages.gradle packages.gradle index d2c0979..86b8068 100644 --- packages.gradle +++ packages.gradle @@ -441,7 +441,7 @@ def genTasks = { target -> def final PKG_BUILD_DIR = config.bigtop.components[target].builddir def final SEED_TAR = config.bigtop.components[target].seedtar def final PKG_VERSION = config.bigtop.components[target].version.pkg - def final BASE_VERSION = config.bigtop.components[target].version.pkg + def final BASE_VERSION = config.bigtop.components[target].version.base def final PKG_OUTPUT_DIR = config.bigtop.components[target].outputdir delete ( "$PKG_BUILD_DIR/rpm" ) ['INSTALL','SOURCES','BUILD','SRPMS','RPMS'].each { rpmdir -> will be applied to the next version of the patch. Still looking at Pig
          Hide
          cos Konstantin Boudnik added a comment -

          And have finally figured out Pig: I forgot to export JDK and SCALA version to list of versions written into build-time bigtop.bom file. New patch is getting uploaded and with that everything seems to be in order. Appreciate you keeping me honest, RJ.

          I will hold-off committing this in case there will be more nits. Although recalling my notes during this refactoring, that should be all.

          Show
          cos Konstantin Boudnik added a comment - And have finally figured out Pig: I forgot to export JDK and SCALA version to list of versions written into build-time bigtop.bom file. New patch is getting uploaded and with that everything seems to be in order. Appreciate you keeping me honest, RJ. I will hold-off committing this in case there will be more nits. Although recalling my notes during this refactoring, that should be all.
          Hide
          cos Konstantin Boudnik added a comment -

          Hopefully, the last one

          Show
          cos Konstantin Boudnik added a comment - Hopefully, the last one
          Hide
          rnowling RJ Nowling added a comment - - edited

          I confirmed that the most recent patch successfully builds RPMs for Pig and Phoenix. I also tested that BIGTOP-2053 successfully enables the ignite-hadoop RPM to be built.

          I'm +1 at this point.

          Thanks for all the great work, Cos, and the patience with the reviews.

          Show
          rnowling RJ Nowling added a comment - - edited I confirmed that the most recent patch successfully builds RPMs for Pig and Phoenix. I also tested that BIGTOP-2053 successfully enables the ignite-hadoop RPM to be built. I'm +1 at this point. Thanks for all the great work, Cos, and the patience with the reviews.
          Hide
          cos Konstantin Boudnik added a comment -

          Thanks everyone who spent time reviewing and testing. The build is a jerk. And while it is getting better we aren't out of the woods yet!

          Show
          cos Konstantin Boudnik added a comment - Thanks everyone who spent time reviewing and testing. The build is a jerk. And while it is getting better we aren't out of the woods yet!
          Hide
          cos Konstantin Boudnik added a comment -

          Pushed to the master. Thanks!

          Show
          cos Konstantin Boudnik added a comment - Pushed to the master. Thanks!

            People

            • Assignee:
              cos Konstantin Boudnik
              Reporter:
              cos Konstantin Boudnik
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development