Bug 31744 - BuildQueue should check if the tree is currently buildable
Summary: BuildQueue should check if the tree is currently buildable
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-20 14:08 PST by Adam Barth
Modified: 2009-11-20 14:53 PST (History)
1 user (show)

See Also:


Attachments
Patch (4.66 KB, patch)
2009-11-20 14:09 PST, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (4.59 KB, patch)
2009-11-20 14:33 PST, Adam Barth
no flags Details | Formatted Diff | Diff
2009-11-20 Adam Barth <abarth@webkit.org> (4.65 KB, patch)
2009-11-20 14:46 PST, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.