Bug 31744 – BuildQueue should check if the tree is currently buildable

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Adam Barth]

This will reduce false rejections.

[Adam Barth]

Created attachment 43612 [details]
Patch

[Eric Seidel (no email)]

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?

[Adam Barth]

Created attachment 43613 [details]
Patch

[Adam Barth]

(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.

[Eric Seidel (no email)]

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.

[Adam Barth]

Created attachment 43615 [details]
2009-11-20  Adam Barth  <[e-mail]>

[Adam Barth]

Comment on attachment 43615 [details]
2009-11-20  Adam Barth  <[e-mail]>

Clearing flags on attachment: 43615

Committed r51260: <http://trac.webkit.org/changeset/51260>

[Adam Barth]

All reviewed patches have been landed.  Closing bug.