#1248 closed defect (wontfix)

move logic for build steps from buildmaster config to misc/build_helpers

Reported by: davidsarah Owned by: warner
Priority: normal Milestone: soon
Component: dev-infrastructure Version: 1.8.0
Keywords: buildbot build-helpers cleanup install setuptools pycryptopp zfec Cc:
Launchpad Bug:

Description (last modified by zooko)

There is some near-duplicated code in the buildmaster's TestFromEgg and TestFromPrefixDir classes (in bbsupport.py). There is also duplication between setup.py, misc/build_helpers/run_trial.py, and some of the tests in src/allmydata/test/test_runner.py, for things like setting up PYTHONPATH, getting the Tahoe-LAFS version, and checking that we are running code from the right directory.

Code in the buildmaster is more difficult to debug than code in the Tahoe-LAFS distribution; changes require a buildmaster restart to take effect, and interfere with the reproducibility of builds. So we should simplify these classes by having them just call scripts (or a single script with different arguments) in misc/build_helpers.

The reason why the code that is already in the distribution isn't better-factored, is probably that in each case we need to run it before importing any non-stdlib modules. It seems like there should be some way to work around that.

Change History (11)

comment:1 Changed at 2010-11-04T01:37:10Z by davidsarah

  • Owner changed from somebody to davidsarah
  • Status changed from new to assigned

comment:2 Changed at 2010-12-01T19:55:39Z by davidsarah

  • Keywords install setuptools pycryptopp zfec added
  • Milestone changed from 1.9.0 to soon
  • Priority changed from minor to major
  • Summary changed from refactor buildmaster Test classes and build_helper scripts to move logic for build steps from buildmaster config to misc/build_helpers

From #1275, which was a duplicate:

Buildmaster config is a bad place to keep logic—almost nobody can see it except for a privileged few, and it tends to be ill-documented and ill-organized. Move the install-to-prefix step (below) and the test-from-prefix step (below) from buildmaster config into a script in the tahoe-lafs source tree. Also add a test to it that goes red if the install puts anything other than allmydata into the Python lib dir (currently the install puts buildtest into there, which is not intended and will be fixed as soon as we have a test of it).

install-to-prefix:

class InstallToPrefixDir(PythonCommand):
    """
    Step to install into a temporary install directory using --prefix.
    """

    flunkOnFailure = False
    description = ["install-to-prefix"]
    name = "install-to-prefix"

    def __init__(self, prefixinstalldir="prefixinstalldir", *args, **kwargs):
        python_command = ["-c", ("import subprocess, sys;"
                             "sys.exit(subprocess.call([sys.executable, 'setup.py', 'install', '--single-version-externally-managed', '--record=record.txt', '--prefix', '"+prefixinstalldir+"']))")]
        kwargs['python_command'] = python_command
        PythonCommand.__init__(self, *args, **kwargs)
        self.addFactoryArguments(prefixinstalldir=prefixinstalldir)

example: http://tahoe-lafs.org/buildbot/builders/Kyle%20OpenBSD%20amd64/builds/24/steps/install-to-prefix/logs/stdio

test-from-prefix:

class TestFromPrefixDir(PythonCommand):
    """
    Step to run the Tahoe-LAFS tests from a --prefix=installdir installation.
    """
    flunkOnFailure = True
    description = ["test-from-prefixdir"]
    name = "test-from-prefixdir"

    def __init__(self, testsuite=None, prefixinstalldir="prefixinstalldir", srcbasedir=".", *args, **kwargs):
        if testsuite:
            assert isinstance(testsuite, basestring)
            pcmd = (
                    "import copy,os,subprocess,sys;"
                    "trial=os.path.join(os.getcwd(), 'misc', 'build_helpers', 'run_trial.py');"
                    "os.chdir('"+srcbasedir+"');"
                    "testsuite='"+testsuite+"';"
                    "prefixinstdir=os.path.join(os.getcwd(), 'prefixinstalldir');"
                    "libdir=(('win32' in sys.platform.lower()) and os.path.join(os.getcwd(), 'prefixinstalldir', 'Lib', 'site-packages') or os.path.join(os.getcwd(), 'prefixinstalldir', 'lib', 'python%(ver)s' % {'ver': sys.version[:3]}, 'site-packages'));"
                    "bindir=('win32' in sys.platform.lower()) and 'Scripts' or 'bin';"
                    "env=copy.copy(os.environ);"
                    "env['PATH']=bindir+os.pathsep+env.get('PATH','');"
                    "env['PYTHONPATH']=libdir+os.pathsep+env.get('PYTHONPATH','');"
                    "os.chdir('prefixinstalldir');"
                    "sys.exit(subprocess.call([sys.executable, trial, '--reporter=bwverbose', '--rterror', testsuite], env=env))")
        else:
            pcmd = (
                    "import copy,os,subprocess,sys;"
                    "trial=os.path.join(os.getcwd(), 'misc', 'build_helpers', 'run_trial.py');"
                    "os.chdir('"+srcbasedir+"');"
                    "testsuite=subprocess.Popen([sys.executable, 'setup.py', '--name'], stdout=subprocess.PIPE).communicate()[0].strip()+'.test';"
                    "prefixinstdir=os.path.join(os.getcwd(), 'prefixinstalldir');"
                    "libdir=(('win32' in sys.platform.lower()) and os.path.join(os.getcwd(), 'prefixinstalldir', 'Lib', 'site-packages') or os.path.join(os.getcwd(), 'prefixinstalldir', 'lib', 'python%(ver)s' % {'ver': sys.version[:3]}, 'site-packages'));"
                    "bindir=('win32' in sys.platform.lower()) and 'Scripts' or 'bin';"
                    "env=copy.copy(os.environ);"
                    "env['PATH']=bindir+os.pathsep+env.get('PATH','');"
                    "env['PYTHONPATH']=libdir+os.pathsep+env.get('PYTHONPATH','');"
                    "os.chdir('prefixinstalldir');"
                    "sys.exit(subprocess.call([sys.executable, trial, '--reporter=bwverbose', '--rterror', testsuite], env=env))")
        python_command = ["-c", pcmd]
        logfiles = {"test.log": prefixinstalldir+"/_trial_temp/test.log"}
        kwargs['python_command'] = python_command
        kwargs['logfiles'] = logfiles
        PythonCommand.__init__(self, *args, **kwargs)
        self.addFactoryArguments(testsuite=testsuite, prefixinstalldir=prefixinstalldir, srcbasedir=srcbasedir)

example: http://tahoe-lafs.org/buildbot/builders/Kyle%20OpenBSD%20amd64/builds/24/steps/test-from-prefixdir/logs/stdio

(Note: we also need to copy the in-source-tree buildstep script into zfec and pycryptopp source trees.)

comment:3 Changed at 2011-01-02T23:37:13Z by francois

  • Owner changed from davidsarah to francois
  • Status changed from assigned to new

comment:4 follow-up: Changed at 2011-01-14T23:42:02Z by zooko

François: want to work on this, this weekend? We could work on it together.

comment:5 in reply to: ↑ 4 Changed at 2011-01-15T16:21:34Z by francois

Replying to zooko:

François: want to work on this, this weekend? We could work on it together.

My current priority is #755, but I'll be glad to work on this one as soon as #755 is fixed.

comment:6 Changed at 2012-04-01T05:04:01Z by davidsarah

  • Priority changed from major to normal

comment:7 Changed at 2012-05-31T22:33:43Z by davidsarah

  • Owner changed from francois to warner

<warner>: have I mentioned recently that the buildbot config is way too big, and that things like this should be in a Makefile or a shell script or a helper program inside the source tree, so they can be updated at the same time?

comment:8 Changed at 2013-08-27T12:13:59Z by zooko

  • Description modified (diff)
  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #2049.

comment:9 Changed at 2015-01-13T17:51:29Z by warner

  • Resolution duplicate deleted
  • Status changed from closed to reopened

On second thought, #2049 doesn't really cover this (it's about where the packaging tests should live, not about where the subroutines used by those tests should live). We decided to reopen this.

comment:10 Changed at 2015-01-13T18:01:16Z by zooko

Here is the set of packaging tests which are currently written into the buildmaster config and which ought to be moved into separate scripts:

comment:11 Changed at 2019-07-25T12:59:26Z by exarkun

  • Resolution set to wontfix
  • Status changed from reopened to closed

Buildbot has been decommissioned.

Note: See TracTickets for help on using tickets.