Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-4152

Incremental builds should include previous builds' modules if previous build failed

    • Icon: Improvement Improvement
    • Resolution: Won't Fix
    • Icon: Major Major
    • maven-plugin
    • None
    • Platform: All, OS: All

      (pasted from message
      http://www.nabble.com/Problem-with-incremental-builds-td24799079.html )

      We're making use of the new incremental build feature, and I've noticed a problem.

      Let's say I have two modules, "library" and "app", with "app" depending on
      "library". If a commit fails a unit test in "library", Hudson correctly flags
      the build. However, if a subsequent commit occurs on "app" (and "app" only),
      Hudson will only execute the build of "app", and despite no one having committed
      code to fix the unit test, Hudson reports this subsequent build as "passed".
      This is because Hudson only built "app", and did not run library's unit tests.

      In my opinion, if a previous build has failed, Hudson should keep using this
      build's maven's "pl" argument, appending to it as necessary as more commits are
      made. So, in my example, the first build would be invoked "mvn -pl library"
      (since that's the module that has changed). The second build would be invoked
      "-ol library, app", which is "pl of last build + new changes.

          [JENKINS-4152] Incremental builds should include previous builds' modules if previous build failed

          hopfrog238 created issue -

          Andrew Bayer added a comment -

          Taking issue - still not entirely sure what the best resolution would be,
          though. While I definitely see your point, I don't know if we really want to
          keep building failed/unstable modules when nothing has changed in them since the
          previous build. The point of the incremental build is to only build those
          modules which have changed since the previous build - if module A has failed
          tests in build 1 but there are no changes in it for build 2, we already know
          what the result will be, so why run the build/tests again? It might make more
          sense to clarify the result of build 2 in that situation - if a module is
          skipped in a given build, reuse the result of its last build. I'll look into
          this more, but probably won't get much (if anything) done before next week.

          Andrew Bayer added a comment - Taking issue - still not entirely sure what the best resolution would be, though. While I definitely see your point, I don't know if we really want to keep building failed/unstable modules when nothing has changed in them since the previous build. The point of the incremental build is to only build those modules which have changed since the previous build - if module A has failed tests in build 1 but there are no changes in it for build 2, we already know what the result will be, so why run the build/tests again? It might make more sense to clarify the result of build 2 in that situation - if a module is skipped in a given build, reuse the result of its last build. I'll look into this more, but probably won't get much (if anything) done before next week.

          hopfrog238 added a comment -

          > so why run the build/tests again?

          So Hudson can accurately report that the build is still failing.

          I suppose you could simply assume that, if no commits occurred in the failing
          module, subsequent builds are still unstable/failed. This would cut-down on the
          build time. However, in most cases, I'd expect the optimization gained would be
          negligible, and it seems more prudent just to endure the extra build duration.
          This is an area where I think it would be wise to side-step any logic
          determining which modules should be built. Brute-force in the name of caution.

          Whatever the resolution, having a CI Engine report a pass when you're unable to
          checkout and compile should be regarded as an egregious error. At the very
          least, I think providing a warning about the use of this feature is necessary.

          hopfrog238 added a comment - > so why run the build/tests again? So Hudson can accurately report that the build is still failing. I suppose you could simply assume that, if no commits occurred in the failing module, subsequent builds are still unstable/failed. This would cut-down on the build time. However, in most cases, I'd expect the optimization gained would be negligible, and it seems more prudent just to endure the extra build duration. This is an area where I think it would be wise to side-step any logic determining which modules should be built. Brute-force in the name of caution. Whatever the resolution, having a CI Engine report a pass when you're unable to checkout and compile should be regarded as an egregious error. At the very least, I think providing a warning about the use of this feature is necessary.

          Andrew Bayer added a comment -
              • Issue 4243 has been marked as a duplicate of this issue. ***

          Andrew Bayer added a comment - Issue 4243 has been marked as a duplicate of this issue. ***
          Andrew Bayer made changes -
          Link New: This issue is duplicated by JENKINS-4243 [ JENKINS-4243 ]

          djrobo added a comment -

          I have to agree that addressing this issue is vital to the usability of the
          incremental builds feature. Development teams have to be able to trust that a
          current successful build on the CI server indicates that the build is not
          secretly in a broken state.

          I can think of three possible ways of dealing with this case. t
          If a build is broken, and a new check-in occurs,
          1. Ignore the incremental builds this build and instead build all modules. This
          certainly has simplicity on its side, and may just be the best answer.
          2. Get the normal module list to build from the SCM changes, but then append to
          this build list the broken modules from the previous build. (Or for simplicity,
          you could append all modules from the previous build, whether they failed or
          not). This is slightly more complex, but would address the issue while
          maintaining the build-only-what-is-necessary mentality of incremental builds.
          3. You can detect if the previous failure took place in a module that will not
          get rebuilt due to not being in the modules/dependency tree that was checked
          into this time. In this case, the build could go as normal, but once complete
          Hudson would sill mark the build as a failure due to the previous build failure.
          This is the most efficient method (in terms of eliminating the rebuilding of
          projects that should still be in failure), but if a module had previously failed
          due to some transient issue in a test (temporary network connectivity issue for
          instance), then you are not giving it another chance that it may succeed on.

          I think clearly option 1 or 2 is the right choice here.

          djrobo added a comment - I have to agree that addressing this issue is vital to the usability of the incremental builds feature. Development teams have to be able to trust that a current successful build on the CI server indicates that the build is not secretly in a broken state. I can think of three possible ways of dealing with this case. t If a build is broken, and a new check-in occurs, 1. Ignore the incremental builds this build and instead build all modules. This certainly has simplicity on its side, and may just be the best answer. 2. Get the normal module list to build from the SCM changes, but then append to this build list the broken modules from the previous build. (Or for simplicity, you could append all modules from the previous build, whether they failed or not). This is slightly more complex, but would address the issue while maintaining the build-only-what-is-necessary mentality of incremental builds. 3. You can detect if the previous failure took place in a module that will not get rebuilt due to not being in the modules/dependency tree that was checked into this time. In this case, the build could go as normal, but once complete Hudson would sill mark the build as a failure due to the previous build failure. This is the most efficient method (in terms of eliminating the rebuilding of projects that should still be in failure), but if a module had previously failed due to some transient issue in a test (temporary network connectivity issue for instance), then you are not giving it another chance that it may succeed on. I think clearly option 1 or 2 is the right choice here.

          Andrew Bayer added a comment -

          I'm leaning towards #2, and am hoping to have that implemented in time for
          1.321, but it may not make it in until 1.322 - today was spent writing test
          infrastructure for incremental builds. =)

          Andrew Bayer added a comment - I'm leaning towards #2, and am hoping to have that implemented in time for 1.321, but it may not make it in until 1.322 - today was spent writing test infrastructure for incremental builds. =)

          djrobo added a comment -

          That is great to hear. Option number 2 is certainly what I would have leaned
          towards myself.

          djrobo added a comment - That is great to hear. Option number 2 is certainly what I would have leaned towards myself.

          Andrew Bayer added a comment -

          Ok, this definitely won't make it in until 1.322 at the earliest - I've got a
          bunch of actual paid work to do, and I'm having trouble getting my test
          SCM-with-changelog working for the ITs for this feature. But I'll keep plugging
          away.

          Andrew Bayer added a comment - Ok, this definitely won't make it in until 1.322 at the earliest - I've got a bunch of actual paid work to do, and I'm having trouble getting my test SCM-with-changelog working for the ITs for this feature. But I'll keep plugging away.

          Code changed in hudson
          User: : abayer
          Path:
          trunk/hudson/main/core/src/main/java/hudson/model/Run.java
          trunk/hudson/main/maven-plugin/src/main/java/hudson/maven/MavenModuleSetBuild.java
          trunk/hudson/main/test/src/test/java/hudson/maven/MavenMultiModuleTest.java
          trunk/www/changelog.html
          http://fisheye4.cenqua.com/changelog/hudson/?cs=20953
          Log:
          [FIXED JENKINS-4152] Maven incremental build now will rebuild modules which were failed/unstable in previous build, even if there were no changes for those modules in this build.

          SCM/JIRA link daemon added a comment - Code changed in hudson User: : abayer Path: trunk/hudson/main/core/src/main/java/hudson/model/Run.java trunk/hudson/main/maven-plugin/src/main/java/hudson/maven/MavenModuleSetBuild.java trunk/hudson/main/test/src/test/java/hudson/maven/MavenMultiModuleTest.java trunk/www/changelog.html http://fisheye4.cenqua.com/changelog/hudson/?cs=20953 Log: [FIXED JENKINS-4152] Maven incremental build now will rebuild modules which were failed/unstable in previous build, even if there were no changes for those modules in this build.
          SCM/JIRA link daemon made changes -
          Resolution New: Fixed [ 1 ]
          Status Original: Open [ 1 ] New: Resolved [ 5 ]

            abayer Andrew Bayer
            hopfrog238 hopfrog238
            Votes:
            1 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: