Bug 31744

Summary: BuildQueue should check if the tree is currently buildable
Product: WebKit Reporter: Adam Barth <abarth>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
2009-11-20 Adam Barth <abarth@webkit.org> none

Description Adam Barth 2009-11-20 14:08:05 PST
This will reduce false rejections.
Comment 1 Adam Barth 2009-11-20 14:09:02 PST
Created attachment 43612 [details]
Patch
Comment 2 Eric Seidel (no email) 2009-11-20 14:21:10 PST
Comment on attachment 43612 [details]
Patch

"clean" build is overloaded.  Not what you want.

Probably just "build"

 152         Command.__init__(self, "Updates working copy and does a clean build.", "tt", options)


 790             self.run_bugzilla_tool(["clean-build", self.port.flag(), "--force-clean", "--quiet", "--ignore-builders"])

shouldn't need --ignore builders, no?  why would "build" ever look at builders?

I'm not sure that build-attachment needs to look at builders either.  Maybe it should.  Although it seems like you don't really want "build-attachment" here so much as you want an apply step, followed by a build step.  We seem to be conflating two problems.  1. the need to run in a separate process.  2.  the want to control whcih specific steps are run.  Let's talk in person.

error is evil!
 39 from modules.logging import error, log, tee

There is already a --no-update flag elsewhere:
 81             make_option("--no-update", action="store_false", dest="update", default=True, help="Do not update the working copy."),

perhaps we should just copy or move that one?
Comment 3 Adam Barth 2009-11-20 14:33:01 PST
Created attachment 43613 [details]
Patch
Comment 4 Adam Barth 2009-11-20 14:34:26 PST
(In reply to comment #2)
> (From update of attachment 43612 [details])
> "clean" build is overloaded.  Not what you want.
>
> Probably just "build"

Fixed.

>  152         Command.__init__(self, "Updates working copy and does a clean
> build.", "tt", options)

Fixed.

>  790             self.run_bugzilla_tool(["clean-build", self.port.flag(),
> "--force-clean", "--quiet", "--ignore-builders"])
> 
> shouldn't need --ignore builders, no?  why would "build" ever look at builders?

Removed.  I'm inclined to solve the "looking at the builders too much" problem in a separate patch.

> error is evil!
>  39 from modules.logging import error, log, tee

This actually an unrelated bug fix.

> There is already a --no-update flag elsewhere:
>  81             make_option("--no-update", action="store_false", dest="update",
> default=True, help="Do not update the working copy."),
> 
> perhaps we should just copy or move that one?

I copied the text.  I'll unify this soon.
Comment 5 Eric Seidel (no email) 2009-11-20 14:44:49 PST
Comment on attachment 43613 [details]
Patch

 792             return (False, "Unable to perform a clean build.", None)

 152         Command.__init__(self, "Updates working copy and does a clean build.", "", options)

You and clean.

OK.  Needs fix for landing.
Comment 6 Adam Barth 2009-11-20 14:46:57 PST
Created attachment 43615 [details]
2009-11-20  Adam Barth  <abarth@webkit.org>
Comment 7 Adam Barth 2009-11-20 14:52:58 PST
Comment on attachment 43615 [details]
2009-11-20  Adam Barth  <abarth@webkit.org>

Clearing flags on attachment: 43615

Committed r51260: <http://trac.webkit.org/changeset/51260>
Comment 8 Adam Barth 2009-11-20 14:53:06 PST
All reviewed patches have been landed.  Closing bug.