Ticket #209 (new defect)

Opened 10 months ago

Last modified 10 months ago

FileUpload/FileDownload do not support setDefaultWorkdir()

Reported by: gward Assigned to:
Priority: minor Milestone: undecided
Component: buildprocess Version: 0.7.6
Keywords: Cc:

Description

For most build steps, it appears to be possible to set a default workdir for the whole build by ensuring that build.workdir is set. (E.g. you could subclass Build, override __init__() set self.workdir, and tell your BuildFactory about your Build subclass.)

However, FileUpload and FileDownload don't play nice with this scheme. From a quick read of BuildStep, it sounds like subclasses with a workdir are expected to implement setDefaultWorkdir(), and they do not. Thus, FileUpload and FileDownload steps do not respect the Build's workdir.

Attachments

bug209.0001.support-default-workdir.patch (3.4 kB) - added by gward on 03/25/08 10:18:36.
patch 1/3: fix FileUpload? and FileDownload? to support setDefaultWorkdir()
bug209.0002.add-tests.patch (2.2 kB) - added by gward on 03/25/08 10:19:12.
patch 2/3: add tests to ensure that calling setDefaultWorkdir() actually affects 'workdir' of FileUpload? and FileDownload? objects
bug209.0003.refactor.patch (3.0 kB) - added by gward on 03/25/08 10:19:37.
patch 3/3: factor out _TransferBuildStep for common workdir logic.

Change History

03/25/08 10:18:36 changed by gward

  • attachment bug209.0001.support-default-workdir.patch added.

patch 1/3: fix FileUpload? and FileDownload? to support setDefaultWorkdir()

03/25/08 10:19:12 changed by gward

  • attachment bug209.0002.add-tests.patch added.

patch 2/3: add tests to ensure that calling setDefaultWorkdir() actually affects 'workdir' of FileUpload? and FileDownload? objects

03/25/08 10:19:37 changed by gward

  • attachment bug209.0003.refactor.patch added.

patch 3/3: factor out _TransferBuildStep for common workdir logic.

03/25/08 10:21:43 changed by gward

The above patches work for me. There's probably room for more improvement though:

  • where else is "build" hardcoded as the default workdir?
  • it looks like there is other functionality in transfer.py that is common to FileUpload and FileDownload and could be factored out to _TransferBuildStep ... but I was concentrating on workdir here

03/25/08 10:29:03 changed by gward

Oops, forgot to mention: those patches are relative to Buildbot 0.7.6 with various patches applied. Patches 1 and 3 apply fine to the trunk, but patch 2 (tests) fails. But it's a really easy and obvious conflict. I can regenerate the patches relative to the trunk if someone asks, but for now I will assume that whoever reviews/applies these patches can figure things out. It's not hard, honest!