Ticket #209 (closed defect: fixed)

Opened 5 years ago

Last modified 4 years ago

FileUpload/FileDownload do not support setDefaultWorkdir()

Reported by: gward Owned by:
Priority: minor Milestone: 0.7.10
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 Download (3.4 KB) - added by gward 5 years ago.
patch 1/3: fix FileUpload? and FileDownload? to support setDefaultWorkdir()
bug209.0002.add-tests.patch Download (2.2 KB) - added by gward 5 years ago.
patch 2/3: add tests to ensure that calling setDefaultWorkdir() actually affects 'workdir' of FileUpload? and FileDownload? objects
bug209.0003.refactor.patch Download (3.0 KB) - added by gward 5 years ago.
patch 3/3: factor out _TransferBuildStep for common workdir logic.

Change History

Changed 5 years ago by gward

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

Changed 5 years ago by gward

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

Changed 5 years ago by gward

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

comment:1 Changed 5 years ago 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

comment:2 Changed 5 years ago 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!

comment:3 Changed 4 years ago by dustin

  • Milestone changed from undecided to 0.7.10

comment:4 Changed 4 years ago by dustin

  • Status changed from new to closed
  • Resolution set to fixed 
commit d554bff93efb780f6ad09c559bad2402fa1fd217
Author: Greg Ward <gerg.ward+buildbot@gmail.com>
Date:   Tue Feb 3 10:42:37 2009 -0500

    (refs #209) patch 3/3: factor out _TransferBuildStep for common workdir logic.

commit 4210d6c33a31df925a75b1903f5435a833243e9d
Author: Greg Ward <gerg.ward+buildbot@gmail.com>
Date:   Tue Feb 3 10:41:35 2009 -0500

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

commit 5a42c47726a91efec0fad46c91865f02bc7c9d45
Author: Greg Ward <gerg.ward+buildbot@gmail.com>
Date:   Tue Feb 3 10:38:12 2009 -0500

    (refs #209) patch 1/3: fix FileUpload and FileDownload to support setDefaultWorkdir()
Note: See TracTickets for help on using tickets.