buildhistory: Drop BUILDHISTORY_RESET due to reliability issues

The implementation of BUILDHISTORY_RESET is problematic, particlarly given that
people are trying to create an API with it alongside BUILDHISTORY_PRESERVE
which simply doesn't exist and can't work reliably. Worse, the code paths with
this bolted on implementation are convoluted and near impossible to follow.

BUILDHISTORY_PRESERVE is effectively internal API, used to stop buildhistory
removing some files which are needed for data, or are created at different
parts of the build. Add a comment to explain what it is doing and why these files
are listed.

Commit 9f68a45aa238ae5fcdfaca71ba0e7015e9cb720e tried to "fix" preserve support
with the reset functionality but it didn't fully work and has just exposed futher
issues. There is a further fix however I can brely follow the code and in reviewing
it, I've concluded we shouldn't be doing this at all.

Due to the way BUILDHISTORY_RESET was implemented, horrible races were introduced
making it unclear what happens to the data if builds fail for example, or how sstate
interacts with the build since things get reset but stamps do not and tasks may not
rerun. It also interacts badly with any additions to the preserve list, due to
misunderstandings on what that variable does.

Having stared long and hard at the code, and really struggled to understand it, I',
of the view that "reset" for CI purposes should be done by the CI itself. The CI can
choose to remove some files or all files and decide how to handle failures. It has
to handle the buildhistory directory anyway.

Therefore drop BUILDHISTORY_RESET support, allowing the "old" codepaths to be dropped.
BUILDHISTORY_PRESERVE is better documented to hint that it is internal API and to
show what it is really for.

If we really do want some functionality list this, it needs to be implemented in a
way you can follow the code, and have tests.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
index e9701826205864c08acfed68dc37e639627fdebc..4a380c10c6db263625eab63c0e77c70ffb816c76 100644 (file)
--- a/meta/classes/buildhistory.bbclass
+++ b/meta/classes/buildhistory.bbclass
@@ -16,28 +16,6 @@ BUILDHISTORY_DIR ?= "${TOPDIR}/buildhistory"
 BUILDHISTORY_DIR_IMAGE = "${BUILDHISTORY_DIR}/images/${MACHINE_ARCH}/${TCLIBC}/${IMAGE_BASENAME}"
 BUILDHISTORY_DIR_PACKAGE = "${BUILDHISTORY_DIR}/packages/${MULTIMACH_TARGET_SYS}/${PN}"
 
-# Setting this to non-empty will remove the old content of the buildhistory as part of
-# the current bitbake invocation and replace it with information about what was built
-# during the build.
-#
-# This is meant to be used in continuous integration (CI) systems when invoking bitbake
-# for full world builds. The effect in that case is that information about packages
-# that no longer get build also gets removed from the buildhistory, which is not
-# the case otherwise.
-#
-# The advantage over manually cleaning the buildhistory outside of bitbake is that
-# the "version-going-backwards" check still works. When relying on that, be careful
-# about failed world builds: they will lead to incomplete information in the
-# buildhistory because information about packages that could not be built will
-# also get removed. A CI system should handle that by discarding the buildhistory
-# of failed builds.
-#
-# The expected usage is via auto.conf, but passing via the command line also works
-# with: BB_ENV_PASSTHROUGH_ADDITIONS=BUILDHISTORY_RESET BUILDHISTORY_RESET=1
-BUILDHISTORY_RESET ?= ""
-
-BUILDHISTORY_OLD_DIR = "${BUILDHISTORY_DIR}/${@ "old" if "${BUILDHISTORY_RESET}" else ""}"
-BUILDHISTORY_OLD_DIR_PACKAGE = "${BUILDHISTORY_OLD_DIR}/packages/${MULTIMACH_TARGET_SYS}/${PN}"
 BUILDHISTORY_DIR_SDK = "${BUILDHISTORY_DIR}/sdk/${SDK_NAME}${SDK_EXT}/${IMAGE_BASENAME}"
 BUILDHISTORY_IMAGE_FILES ?= "/etc/passwd /etc/group"
 BUILDHISTORY_SDK_FILES ?= "conf/local.conf conf/bblayers.conf conf/auto.conf conf/locked-sigs.inc conf/devtool.conf"
@@ -70,9 +48,10 @@ SSTATEPOSTUNPACKFUNCS[vardepvalueexclude] .= "| buildhistory_emit_outputsigs"
 # necessary because some of these items (package directories, files that
 # we no longer emit) might be obsolete.
 #
-# When extending build history, derive your class from buildhistory.bbclass
-# and extend this list here with the additional files created by the derived
-# class.
+# The files listed here are either written by tasks that aren't do_package (e.g.
+# latest_srcrev from do_fetch) so do_package must not remove them, or, they're
+# used to read values in do_package before always being overwritten, e.g. latest,
+# for version backwards checks.
 BUILDHISTORY_PRESERVE = "latest latest_srcrev sysroot"
 
 PATCH_GIT_USER_EMAIL ?= "buildhistory@oe"
@@ -108,7 +87,6 @@ python buildhistory_emit_pkghistory() {
         return 0
 
     pkghistdir = d.getVar('BUILDHISTORY_DIR_PACKAGE')
-    oldpkghistdir = d.getVar('BUILDHISTORY_OLD_DIR_PACKAGE')
 
     class RecipeInfo:
         def __init__(self, name):
@@ -203,7 +181,7 @@ python buildhistory_emit_pkghistory() {
 
     def getlastpkgversion(pkg):
         try:
-            histfile = os.path.join(oldpkghistdir, pkg, "latest")
+            histfile = os.path.join(pkghistdir, pkg, "latest")
             return readPackageInfo(pkg, histfile)
         except EnvironmentError:
             return None
@@ -219,20 +197,6 @@ python buildhistory_emit_pkghistory() {
         items.sort()
         return ' '.join(items)
 
-    def preservebuildhistoryfiles(pkg, preserve):
-        if os.path.exists(os.path.join(oldpkghistdir, pkg)):
-            listofobjs = os.listdir(os.path.join(oldpkghistdir, pkg))
-            for obj in listofobjs:
-                if obj not in preserve:
-                    continue
-                try:
-                    bb.utils.mkdirhier(os.path.join(pkghistdir, pkg))
-                    shutil.copyfile(os.path.join(oldpkghistdir, pkg, obj), os.path.join(pkghistdir, pkg, obj))
-                except IOError as e:
-                    bb.note("Unable to copy file. %s" % e)
-                except EnvironmentError as e:
-                    bb.note("Unable to copy file. %s" % e)
-
     pn = d.getVar('PN')
     pe = d.getVar('PE') or "0"
     pv = d.getVar('PV')
@@ -260,14 +224,6 @@ python buildhistory_emit_pkghistory() {
     if not os.path.exists(pkghistdir):
         bb.utils.mkdirhier(pkghistdir)
     else:
-        # We need to make sure that all files kept in
-        # buildhistory/old are restored successfully
-        # otherwise next block of code wont have files to
-        # check and purge
-        if d.getVar("BUILDHISTORY_RESET"):
-            for pkg in packagelist:
-                preservebuildhistoryfiles(pkg, preserve)
-
         # Remove files for packages that no longer exist
         for item in os.listdir(pkghistdir):
             if item not in preserve:
@@ -887,25 +843,7 @@ END
 
 python buildhistory_eventhandler() {
     if (e.data.getVar('BUILDHISTORY_FEATURES') or "").strip():
-        reset = e.data.getVar("BUILDHISTORY_RESET")
-        olddir = e.data.getVar("BUILDHISTORY_OLD_DIR")
-        if isinstance(e, bb.event.BuildStarted):
-            if reset:
-                import shutil
-                # Clean up after potentially interrupted build.
-                if os.path.isdir(olddir):
-                    shutil.rmtree(olddir)
-                rootdir = e.data.getVar("BUILDHISTORY_DIR")
-                bb.utils.mkdirhier(rootdir)
-                entries = [ x for x in os.listdir(rootdir) if not x.startswith('.') ]
-                bb.utils.mkdirhier(olddir)
-                for entry in entries:
-                    bb.utils.rename(os.path.join(rootdir, entry),
-                              os.path.join(olddir, entry))
-        elif isinstance(e, bb.event.BuildCompleted):
-            if reset:
-                import shutil
-                shutil.rmtree(olddir)
+        if isinstance(e, bb.event.BuildCompleted):
             if e.data.getVar("BUILDHISTORY_COMMIT") == "1":
                 bb.note("Writing buildhistory")
                 bb.build.exec_func("buildhistory_write_sigs", d)