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

Add a bisection command that uses nightly builds #19505

Open
jdm opened this issue Dec 6, 2017 · 28 comments
Open

Add a bisection command that uses nightly builds #19505

jdm opened this issue Dec 6, 2017 · 28 comments
Labels
A-mach B-interesting-project Represents work that is expected to be interesting in some fashion E-candidate-for-mentoring L-python Python is required

Comments

@jdm
Copy link
Member

jdm commented Dec 6, 2017

http://servo-builds.s3.amazonaws.com/ hosts a long list of historical nightly builds. We should write a command that allows several modes of operation:

  • ./mach bisect tests/wpt/web-platform-tests/foo.html - runs WPT tests using downloaded nightly builds, suggests good/bad bisection based on test result
  • ./mach bisect path/to/file.html - runs downloaded nightlies and loads file:// url for easy testing
  • ./mach bisect http://foo.com - runs downloaded nightlies and loads url for easy testing
  • ./mach bisect -b - runs downloaded nightlies with browser.html for easy testing

We should cache the downloaded nightlies to make it easier to do future bisections, too.

@jdm jdm added A-mach B-interesting-project Represents work that is expected to be interesting in some fashion labels Dec 6, 2017
@highfive
Copy link

highfive commented Dec 6, 2017

cc @wafflespeanut

@jdm jdm added the L-python Python is required label Dec 6, 2017
@jdm
Copy link
Member Author

jdm commented Dec 6, 2017

git rev-list -n 1 --before="2017-01-01" master is a useful command to figure out a commit associated with a particular nightly.

@jdm
Copy link
Member Author

jdm commented Dec 21, 2017

Curiously, http://servo-builds.s3.amazonaws.com/ only shows mac builds up until 2016-10-25. There are plenty of linux and android builds for recent days, however.

@jdm
Copy link
Member Author

jdm commented Dec 21, 2017

Ah, <MaxKeys>1000</MaxKeys> is hiding the entries that I care about.

@jdm
Copy link
Member Author

jdm commented Dec 21, 2017

I use http://servo-builds.s3.amazonaws.com/?list-type=2&prefix=nightly/mac/2017 when investigating regression ranges for macOS builds from 2017.

@o0Ignition0o
Copy link
Contributor

This one looks very interesting, could I please pick it? I think I can start working on it next week :)

@jdm
Copy link
Member Author

jdm commented Jan 5, 2018

That would be great!

@jdm jdm added the C-assigned There is someone working on resolving the issue label Jan 5, 2018
@jdm
Copy link
Member Author

jdm commented Jan 16, 2018

@o0Ignition0o Have you made any progress with this? If not, I would like to turn this into a project for some students this term.

@o0Ignition0o
Copy link
Contributor

Hi @jdm :) I haven't made too much progress with this yet, so you can assign it to someone else if you want to. I'll pick up an other task

@jdm
Copy link
Member Author

jdm commented Jan 16, 2018

That's fine; you're welcome to keep working on it!

@atouchet
Copy link
Contributor

atouchet commented Jan 16, 2018

@jdm are Windows builds available with http://servo-builds.s3.amazonaws.com/? I see every other platform on there aside from Windows. Having historical nightly builds would be useful for testing purposes.

@jdm
Copy link
Member Author

jdm commented Jan 16, 2018

Note my comment earlier with the prefix argument. If you specify the right prefix, they should appear.

@atouchet
Copy link
Contributor

@o0Ignition0o
Copy link
Contributor

o0Ignition0o commented Feb 3, 2018

Hey there :) I've been able to write a rough try to download a (hardcoded for now) nightly version and extract it.

I put the bisect command as a "mach wrapper" for now so I can run any other mach command, eg :
./mach bisect run https://www.mozilla.org
./mach bisect help
./mach bisect test --all

I'm now looking for a way to set the downloaded servo bin as the target I'm supposed to use.

I have a feeling this should be an environment variable, but I haven't found it quite yet ^^'
Next steps for me would be parsing s3's XML files (with beautifulsoup or something similar) and binary searching my way through the ./mach commands exit codes, but I need to be able to set the servo target first.
If you have an idea please let me know :)

@jdm
Copy link
Member Author

jdm commented Feb 4, 2018

https://github.com/servo/servo/blob/master/python/servo/post_build_commands.py#L99 is the implementation for ./mach run, and it relies on get_binary_path. There's no built-in way to override the binary that is selected right now; adding an additional argument to the run command that passes a specific binary to use would probably make sense.

@o0Ignition0o
Copy link
Contributor

Hi, thanks a lot for your response.

So here's what I'm going to do :

1 - Add a --bin flag to ./mach run to allow a user to specify the path to the executable to run

2 - Add a --nightly | -n flag to allow a user to specify a YYYY-MM-DD date. This flag will :
Figure out which OS the user is running
Check the nightlies folder to find out if the nightly version has already been downloaded and extracted
if not :
Try to fetch it from the S3 server
Raise an exception and print a message if there is no nightly for the specified date and platform
run the specified command with the nightly version
The default nightly folder will be PROJECT_ROOT/target/nightlies/ and should be customizable through an environment variable

3 - go back to the bisect command, which should be easier to implement once these steps are performed.

Each step will be a separate Pull Request, which I'll link to this issue.
Should I file separate issues for each step ?
Are there things I should reconsider before working on the implementation ?

@jdm
Copy link
Member Author

jdm commented Feb 4, 2018

That sounds great!

bors-servo pushed a commit that referenced this issue Feb 4, 2018
Add a --bin flag to the |mach run and rr-record commands

Add a --bin flag to the |mach run and rr-record commands to specify which servo binary to run

<!-- Please describe your changes on the following line: -->

Step 1 for #19505.
This flag allows to specify a downloaded servo binary for the ./mach run and ./mach rr-record commands.
The base issue is mentored by @jdm

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because | I would love to write tests on this, but I'm not really sure I can, since it's on ./mach commands

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

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

o0Ignition0o commented Feb 4, 2018

Just pushed a rough try to handle the --nightly flag.
I really don't like the way I let the -n flag silently fail if an empty string is provided, I'll amend my commit asap ^^'
EDIT : This is better, it will explicitely fail if an empty date is provided.

@o0Ignition0o
Copy link
Contributor

Ok so I chose to split my merge requests depending on the platforms, because I don't know how yo extract a .msi and a .dmg archive with python ^^'

If you have any hint pease let me know, I'll keep on digging :)

@o0Ignition0o
Copy link
Contributor

Ok so it seems like passing the -qi flags to msiexec.exe (on windows) should allow me to silently install a servo build, but I'm not sure at all I wouldn't mess any registry keys / already installed versions of servo.
Could anyone please point me to someone knowledgeable on windows installers so I can get more insights ?

PS : In the mozilla-unified repository, we seem to use executables so it seems easier to handle, but I'm not quite sure how I should handle .msi files :/

@jdm
Copy link
Member Author

jdm commented Feb 14, 2018

According to https://superuser.com/questions/307678/how-do-i-extract-files-from-an-msi-package you should be able to use msiexec /a c:\testfile.msi /qb TARGETDIR=c:\temp\test to extract the contents of the MSI to the target directory, which avoids the issues with installing the package.

@o0Ignition0o
Copy link
Contributor

Thanks a lot, I ll give it a try :)

bors-servo pushed a commit that referenced this issue Mar 6, 2018
Add a --nightly | -n flag to mach run commands for linux

First tries to download and extract a specific nightly version to run mach commands against.

<!-- Please describe your changes on the following line: -->
I chose to split the Pull requests for each platform to avoid submitting a huge one, and to make sure I get the logic right.
I'm able to download / extract a nightly version, and I keep nightly versions in the target folder.
Windows and Mac OS support will be filed in separate PRs.
This is part of step two for #19505

The mentor on the issue is jdm

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because it is part of a ./mach command.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19947)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this issue Mar 6, 2018
Add a --nightly | -n flag to mach run commands for linux

First tries to download and extract a specific nightly version to run mach commands against.

<!-- Please describe your changes on the following line: -->
I chose to split the Pull requests for each platform to avoid submitting a huge one, and to make sure I get the logic right.
I'm able to download / extract a nightly version, and I keep nightly versions in the target folder.
Windows and Mac OS support will be filed in separate PRs.
This is part of step two for #19505

The mentor on the issue is jdm

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because it is part of a ./mach command.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19947)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this issue Mar 7, 2018
Add a --nightly | -n flag to mach run commands for linux

First tries to download and extract a specific nightly version to run mach commands against.

<!-- Please describe your changes on the following line: -->
I chose to split the Pull requests for each platform to avoid submitting a huge one, and to make sure I get the logic right.
I'm able to download / extract a nightly version, and I keep nightly versions in the target folder.
Windows and Mac OS support will be filed in separate PRs.
This is part of step two for #19505

The mentor on the issue is jdm

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because it is part of a ./mach command.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

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

o0Ignition0o commented Mar 11, 2018

Just ran into github issues claiming running servo in a VM is not that easy, so I think the fact I can't run servo.exe might be related to my virtualbox setup.
I'll just remove the print occurrences and submit it for review.
I don't really know whether extracting only the executable is relevant or not, maybe as a followup bug if no other file is needed ?

Edit : I also don't have any IOS device so I might be stuck on providing mac os support for the flag :/
But I sure could help someone else building the feature ! (Maybe I could help someone ? :) )

@jdm
Copy link
Member Author

jdm commented Mar 11, 2018

If you add your ssh key to https://github.com/servo/saltfs/tree/master/admin/files/ssh and your username to https://github.com/servo/saltfs/blob/master/admin/map.jinja, you could get ssh access to our mac build machines if you wanted :)

@o0Ignition0o
Copy link
Contributor

Oh this is great !
I'll file to get access then, thanks a lot !

bors-servo pushed a commit to servo/saltfs that referenced this issue Mar 11, 2018
Add my SSH key

As discussed [here](servo/servo#19505 (comment)) and suggested by @jdm, I would like to have access to the mac build machines.

<!-- 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/813)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/saltfs that referenced this issue Mar 11, 2018
Add my SSH key

As discussed [here](servo/servo#19505 (comment)) and suggested by @jdm, I would like to have access to the mac build machines.

<!-- 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/813)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this issue Mar 16, 2018
Windows support for the --nightly | -n flag to mach run commands.

<!-- Please describe your changes on the following line: -->
Add windows support to the -n flag.
---
Followup to #19947 , this PR will add windows support to the -n flag.
 This is part of step two for #19505

<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

The feature does not work just yet, I'm able to download and extract the archive, but it's not running the executable yet.
@tigercosmos might be a good reviewer on this one :)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because they're part of a ./mach command.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

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

It looks like windows builds now produce an executable instead of a .msi archive, sothe -n flag doesnt work on windows anymore :( .

I'm trying to find a way to extract the executable without installing the application, but couldn't manage so far.

Also https://firefox-source-docs.mozilla.org/mozbase/mozinstall.html doesn't allow me to just extract a portable version, maybe I could submit a pr to mozinstall to allow people to just extract contents ?

@jdm
Copy link
Member Author

jdm commented Dec 8, 2018

Interesting point; that's an unfortunate consequence. If you want to change mozinstall, you probably need to submit a patch to mozilla-central, since I suspect that https://searchfox.org/mozilla-central/source/testing/mozbase/mozinstall is the authoritative source.

@o0Ignition0o
Copy link
Contributor

Yeah, plus that would maybe allow us to use exclusively the mozinstall API on Windows Linux and macos, which would be an improvement. I'll try to submit a patch :)

bors-servo pushed a commit that referenced this issue Dec 22, 2018
./mach run -n on mac os.

<!-- Please describe your changes on the following line: -->
Add macos support to the -n flag.

Followup to #20122 , this PR will add macos support to the -n flag.
 This is part of step two for #19505

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [X] These changes do not require tests because this mach command does not build anything

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22387)
<!-- Reviewable:end -->
@jdm jdm removed the C-assigned There is someone working on resolving the issue label Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mach B-interesting-project Represents work that is expected to be interesting in some fashion E-candidate-for-mentoring L-python Python is required
Projects
None yet
Development

No branches or pull requests

4 participants