Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use steps.Compile for `mach package` on Buildbot #372

Closed
aneeshusa opened this issue May 13, 2016 · 3 comments
Closed

Use steps.Compile for `mach package` on Buildbot #372

aneeshusa opened this issue May 13, 2016 · 3 comments

Comments

@aneeshusa
Copy link
Member

@aneeshusa aneeshusa commented May 13, 2016

We use ./mach package -r to package Android APKs on the android-nightly builder; the next step is then to upload the built package via S3.

Currently, the packaging step is broken (should be fixed by #290); however, even though it fails, Buildbot still tries to run the next step of uploading to S3. This is because there is no heuristic for ./mach package and it is run as a regular ShellCommand.

Instead, we should add a heuristic in the make_step function in the buildbot/master/files/config/factories.py file that checks for ./mach package and uses steps.Compile to create that step. This a) is a closer semantic match (we're "compiling" the APK), and b) will stop the build if the packaging fails, so Buildbot doesn't try the S3 upload step.

This can simply check for package as an argument to mach instead of using a more complicated regex as the other heuristics do, because currently there is only one type of packaging: Android APK.

@lucasloisp
Copy link
Contributor

@lucasloisp lucasloisp commented Oct 4, 2016

I would like to implement this. If I understand correctly, we should check that mach_arg re.matches "package" and assign step_class = steps.Compile. Right?
I'll get to it later today

@aneeshusa aneeshusa added the C-assigned label Oct 4, 2016
@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented Oct 4, 2016

@lucasloisp that's correct! I've assigned this to you, leave a comment if you have any questions.

Note: there are currently two copies of make_step, make sure to update both of them.

lucasloisp added a commit to lucasloisp/saltfs that referenced this issue Oct 5, 2016
This command is used during the Android APK build, but even though it
failed, Buildbot still went on to run the next step, because it used to
be run as a simple ShellCommand (as per issue servo#372).

Instead, we now check for it on the `make_step` function and use
`steps.Compile` to create the step as it is not only a closer semantic
match (we're "compiling" the APK) but it also makes it so when it fails
Builbot doesn't ignore it or move on to the next step.
bors-servo added a commit that referenced this issue Oct 12, 2016
…husa

`mach package` treated as a steps.Compile

@aneeshusa: as per issue #372, both `make_step` now have a heuristic for checking that the argument passed to mach (`mach_arg`) is `'package'` and assigns `step_class = steps.Compile`

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/503)
<!-- Reviewable:end -->
@jdm
Copy link
Member

@jdm jdm commented Oct 23, 2016

Fixed by #503.

@jdm jdm closed this Oct 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.