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 Azure Pipelines configuration #60777

Merged
merged 59 commits into from
May 24, 2019
Merged

Conversation

pietroalbini
Copy link
Member

@pietroalbini pietroalbini commented May 13, 2019

Huge thanks to @johnterickson and @willsmythe for writing the initial config! ❤️
I applied some changes to the initial config and disabled most of the builders since we're not going to run all of them during the initial step for the evaluation.

More details about our plans for the Azure Pipelines evaluation.

r? @alexcrichton @kennytm
cc @rust-lang/infra @ethomson @rylev

@pietroalbini pietroalbini added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label May 13, 2019
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 13, 2019
@pietroalbini
Copy link
Member Author

Azure Pipelines is not yet configured on the repository btw.

.azure-pipelines/auto.yml Outdated Show resolved Hide resolved
src/bootstrap/util.rs Outdated Show resolved Hide resolved
Co-Authored-By: Jake Goulding <shepmaster@mac.com>
@johnterickson
Copy link
Contributor

Looking good!

I do need to give credit to @willsmythe - he took some of my initial work and brought it a lot closer to being production ready.

One thing I noticed in my runs is that sometimes I would see OOM failures in some of the dist-* configs. These seems to work on retry. I know we're already chatting about some bigger VMs and I think that should help.

Both Azure Storage and S3 should work well, but just wanted to point out two things:

  1. You'll probably want to use separate containers/buckets just in case cached layers/sccache from one affects the other.
  2. If you see a network I/O perf regression here, then you might want to consider using Azure Storage accounts created in the same region as your builds are running in. This will put them in the same datacenter.

.azure-pipelines/steps/linux.yml Outdated Show resolved Hide resolved
steps:
- template: show-disk-usage.yml

- bash: |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider putting any bash script longer than a few lines in a standalone script that can be invoked instead of inlined into yml, perhaps under a /scripts folder.

This allows you to easily run & lint the script, whereas it takes extra steps to do so in yaml.

.azure-pipelines/steps/run-script.yml Outdated Show resolved Hide resolved
@pietroalbini
Copy link
Member Author

One thing I noticed in my runs is that sometimes I would see OOM failures in some of the dist-* configs. These seems to work on retry. I know we're already chatting about some bigger VMs and I think that should help.

That's good to know. Let's see how our first builds will go.

You'll probably want to use separate containers/buckets just in case cached layers/sccache from one affects the other.

Yep, that was the plan. I think there shouldn't be any issue merging the buckets (for example the Docker cache contains host machine data in the hash), but let's not risk it.

If you see a network I/O perf regression here, then you might want to consider using Azure Storage accounts created in the same region as your builds are running in. This will put them in the same datacenter.

At least for now let's keep things in AWS (mostly because it's one less cloud platform and sets of perms to manage), but we'll surely look into the Azure storage offering if the perf is too bad.


I'll try to address the other comments tomorrow, if I have some free time after the release is done.

@shepmaster
Copy link
Member

just in case cached layers/sccache from one affects the other.

One thing that CircleCI does that I appreciate is the ability to have "tiered" caches. For a Ruby app I help with, it looks like:

      - restore_cache:
          keys:
            - ruby-dependency-cache--v4--{{ arch }}-{{ .Branch }}-{{ checksum "Gemfile.lock" }}
            - ruby-dependency-cache--v4--{{ arch }}-{{ .Branch }}-
            - ruby-dependency-cache--v4--{{ arch }}-

When restoring a cache, it uses the first matching key. This means that new branches start from a cache from the master branch, but will create a differently-named cache to avoid unintentional cache busting.

@alexcrichton
Copy link
Member

Sorry I'm pretty slammed at the moment with a lot of things, but looking over this the non-azure configuration bits all look reasonable to me, and I'm happy to review the azure configuration bits on my own time when I get a chance. (I also trust y'all!)

Feel free to r=me when you're comfortable landing yourself.

@pietroalbini
Copy link
Member Author

@bors try

Note: this won't start Azure Pipelines yet.

@bors
Copy link
Contributor

bors commented May 15, 2019

⌛ Trying commit 4965ffd with merge 8ffb1f01b864abe7a25e0a86034407a103e08dd7...

@bors

This comment has been minimized.

@bors
Copy link
Contributor

bors commented May 24, 2019

⌛ Testing commit 2244ca3 with merge 8798cbc1646c5505b2f457b942929576d3f9c180...

@Centril
Copy link
Contributor

Centril commented May 24, 2019

@bors retry

@rust-highfive
Copy link
Collaborator

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
Cloning into 'rust-lang/rust'...
travis_time:end:0ae73e5a:start=1558719219386312087,finish=1558719225043283417,duration=5656971330
$ cd rust-lang/rust
$ git checkout -qf 8798cbc1646c5505b2f457b942929576d3f9c180
fatal: reference is not a tree: 8798cbc1646c5505b2f457b942929576d3f9c180
The command "git checkout -qf 8798cbc1646c5505b2f457b942929576d3f9c180" failed and exited with 128 during .
Your build has been stopped.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented May 24, 2019

⌛ Testing commit 2244ca3 with merge 0ae26befeb61b6dccc8a14c5e6ddaa661e46b3e9...

@pietroalbini
Copy link
Member Author

@bors retry

Yielding priority to the rollup.

@rust-highfive
Copy link
Collaborator

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
Cloning into 'rust-lang/rust'...
travis_time:end:0c225c27:start=1558720300440056536,finish=1558720306140163143,duration=5700106607
$ cd rust-lang/rust
$ git checkout -qf 0ae26befeb61b6dccc8a14c5e6ddaa661e46b3e9
fatal: reference is not a tree: 0ae26befeb61b6dccc8a14c5e6ddaa661e46b3e9
The command "git checkout -qf 0ae26befeb61b6dccc8a14c5e6ddaa661e46b3e9" failed and exited with 128 during .
Your build has been stopped.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented May 24, 2019

⌛ Testing commit 2244ca3 with merge dec4c52...

bors added a commit that referenced this pull request May 24, 2019
Add Azure Pipelines configuration

Huge thanks to @johnterickson and @willsmythe for writing the initial config! ❤️
I applied some changes to the initial config and disabled most of the builders since we're not going to run all of them during the initial step for the evaluation.

[More details about our plans for the Azure Pipelines evaluation.](https://internals.rust-lang.org/t/update-on-the-ci-investigation/10056)

r? @alexcrichton @kennytm
cc @rust-lang/infra @ethomson @rylev
@bors
Copy link
Contributor

bors commented May 24, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing dec4c52 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 24, 2019
@bors bors merged commit 2244ca3 into rust-lang:master May 24, 2019
@pietroalbini pietroalbini deleted the azure-pipelines branch May 25, 2019 08:34
@pietroalbini
Copy link
Member Author

Huge thanks to everyone involved in getting this PR landed! ❤️

Now that the core configuration is merged in the repo there are some (small and big) papercuts to fix, and I opened an issue for each of them with the azure-evaluation label. If you want to help us fixing them please claim am issue (typing @rustbot claim in a comment) and send a PR!

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jun 3, 2019
This was accidentally regressed in rust-lang#60777 by accident, and we've stopped
printing out step timings on AppVeyor recently reducing the ability for
us to track build times over time!
Centril added a commit to Centril/rust that referenced this pull request Jun 4, 2019
…etroalbini

ci: Reenable step timings on AppVeyor

This was accidentally regressed in rust-lang#60777 by accident, and we've stopped
printing out step timings on AppVeyor recently reducing the ability for
us to track build times over time!
Centril added a commit to Centril/rust that referenced this pull request Jun 4, 2019
…etroalbini

ci: Reenable step timings on AppVeyor

This was accidentally regressed in rust-lang#60777 by accident, and we've stopped
printing out step timings on AppVeyor recently reducing the ability for
us to track build times over time!
Centril added a commit to Centril/rust that referenced this pull request Jun 4, 2019
…etroalbini

ci: Reenable step timings on AppVeyor

This was accidentally regressed in rust-lang#60777 by accident, and we've stopped
printing out step timings on AppVeyor recently reducing the ability for
us to track build times over time!
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 15, 2020
…acrum

Remove vestigial CI job msvc-aux.

This CI job isn't really doing anything, so it seems prudent to remove it.

For some history:
* This was introduced in rust-lang#48809 when the msvc job was split in two to keep it under 2 hours (oh the good old days). At the time, this check-aux job did a bunch of things:
    * tidy
    * src/test/pretty
    * src/test/run-pass/pretty
    * src/test/run-fail/pretty
    * src/test/run-pass-valgrind/pretty
    * src/test/run-pass-fulldeps/pretty
    * src/test/run-fail-fulldeps/pretty
* Tidy was removed in rust-lang#60777.
* run-pass and run-pass-fulldeps moved to UI in rust-lang#63029
* src/test/pretty removed in rust-lang#58140
* src/test/run-fail moved to UI in rust-lang#71185
* run-fail-fulldeps removed in rust-lang#51285

Over time through attrition, the job was left with one lonely thing: `src/test/run-pass-valgrind/pretty`. And of course, this wasn't actually running the "pretty" tests. The normal `run-pass-valgrind` tests ran, and then when it tried to run in "pretty" mode, all the tests were ignored because compiletest thought nothing had changed (apparently compiletest isn't fingerprinting the mode?  Needs more investigation…). `run-pass-valgrind` is already being run as part of `x86_64-msvc-1`, so there's no need to run it here.

I've taken the liberty of removing `src/test/run-pass-valgrind/pretty` as a distinct test. I'm guessing from the other PR's that the pretty tests should now live in `src/test/pretty`, and that the team has moved away from doing pretty tests on other parts of the `src/test` tree.
RalfJung added a commit to RalfJung/rust that referenced this pull request Jun 15, 2020
…acrum

Remove vestigial CI job msvc-aux.

This CI job isn't really doing anything, so it seems prudent to remove it.

For some history:
* This was introduced in rust-lang#48809 when the msvc job was split in two to keep it under 2 hours (oh the good old days). At the time, this check-aux job did a bunch of things:
    * tidy
    * src/test/pretty
    * src/test/run-pass/pretty
    * src/test/run-fail/pretty
    * src/test/run-pass-valgrind/pretty
    * src/test/run-pass-fulldeps/pretty
    * src/test/run-fail-fulldeps/pretty
* Tidy was removed in rust-lang#60777.
* run-pass and run-pass-fulldeps moved to UI in rust-lang#63029
* src/test/pretty removed in rust-lang#58140
* src/test/run-fail moved to UI in rust-lang#71185
* run-fail-fulldeps removed in rust-lang#51285

Over time through attrition, the job was left with one lonely thing: `src/test/run-pass-valgrind/pretty`. And of course, this wasn't actually running the "pretty" tests. The normal `run-pass-valgrind` tests ran, and then when it tried to run in "pretty" mode, all the tests were ignored because compiletest thought nothing had changed (apparently compiletest isn't fingerprinting the mode?  Needs more investigation…). `run-pass-valgrind` is already being run as part of `x86_64-msvc-1`, so there's no need to run it here.

I've taken the liberty of removing `src/test/run-pass-valgrind/pretty` as a distinct test. I'm guessing from the other PR's that the pretty tests should now live in `src/test/pretty`, and that the team has moved away from doing pretty tests on other parts of the `src/test` tree.
@ehuss ehuss mentioned this pull request Dec 23, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 28, 2021
Remove VCVARS_BAT

This environment variable is no longer used.  It was used in the original Azure Pipelines configuration (rust-lang#60777). When GitHub Actions were added (rust-lang#70190), it was no longer used, and I suspect it was just an oversight while transitioning the configuration.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 28, 2021
Remove VCVARS_BAT

This environment variable is no longer used.  It was used in the original Azure Pipelines configuration (rust-lang#60777). When GitHub Actions were added (rust-lang#70190), it was no longer used, and I suspect it was just an oversight while transitioning the configuration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.