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

Update cssparser. #9380

Merged
merged 1 commit into from
Jan 21, 2016
Merged

Update cssparser. #9380

merged 1 commit into from
Jan 21, 2016

Conversation

SimonSapin
Copy link
Member

@highfive
Copy link

Heads up! This PR modifies the following files:

  • @bholley: components/style/Cargo.toml, components/style/stylesheets.rs, components/style/lib.rs, components/style/values.rs, components/style/media_queries.rs, components/style/properties.mako.rs, components/style/font_face.rs

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jan 19, 2016
@SimonSapin
Copy link
Member Author

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit cac6a13 has been approved by SimonSapin

@bors-servo
Copy link
Contributor

⌛ Testing commit cac6a13 with merge 8d7f3be...

@highfive highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Jan 19, 2016
bors-servo pushed a commit that referenced this pull request Jan 19, 2016
Update cssparser.

servo/rust-cssparser#91

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9380)
<!-- Reviewable:end -->
@highfive highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jan 19, 2016
@bors-servo
Copy link
Contributor

💔 Test failed - mac-dev-unit

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jan 19, 2016
@SimonSapin
Copy link
Member Author

failed to select a version for `selectors` (required by `util`):
all possible versions conflict with previously selected versions of `selectors`
  version 0.2.2 in use by selectors v0.2.2
  possible versions to select: 0.2.3

I can’t reproduce locally and don’t understand how this happens with 0.2.3 in the lock file, but I’ll try to bump the requirement in Cargo.toml.

@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Jan 19, 2016
@SimonSapin
Copy link
Member Author

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 529d7d9 has been approved by SimonSapin

@bors-servo
Copy link
Contributor

⌛ Testing commit 529d7d9 with merge 0c11512...

bors-servo pushed a commit that referenced this pull request Jan 19, 2016
Update cssparser.

servo/rust-cssparser#91

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9380)
<!-- Reviewable:end -->
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jan 19, 2016
@bors-servo
Copy link
Contributor

💔 Test failed - mac-dev-unit

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jan 19, 2016
@SimonSapin
Copy link
Member Author

I haven’t managed to reproduce this issue on the builder. This worked fine:

ssh servo-mac3.servo.org
su servo
cd /Users/servo/buildbot/slave/mac-dev-unit/build
CARGO_HOME=/Users/servo/.cargo SERVO_CACHE_DIR=/Users/servo/.servo ./mach build-geckolib

@SimonSapin
Copy link
Member Author

Retry just in case something has changed, and also to make buildbot spin up EC2 builders so that I can investigate another issue.

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 529d7d9 with merge e4522b9...

bors-servo pushed a commit that referenced this pull request Jan 20, 2016
Update cssparser.

servo/rust-cssparser#91

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9380)
<!-- Reviewable:end -->
@highfive highfive removed the S-tests-failed The changes caused existing tests to fail. label Jan 20, 2016
@alexcrichton
Copy link
Contributor

Oh oops I realize now when I said "I'm not sure other machines are failing" I meant to say "I'm not sure why the other machines are failing", sorry if that caused any confusion!

@SimonSapin
Copy link
Member Author

This looks like you just need to rebase the branch, the Cargo.lock patch applies but there has been changes in the meantime which means that it's basically a corrupt Cargo.lock

This makes sense. When trying to reproduce I was using the branch, never a merge of the branch with master. I’ll try that tomorrow.

@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. S-needs-rebase There are merge conflict errors. labels Jan 21, 2016
@SimonSapin
Copy link
Member Author

Here is what I did:

  • Checkout this PR’s branch, geckolib builds fine
  • Fetch upstream and rebase unto upstream/master. Now I can reproduce the issue.
  • Run ./mach cargo-update -p selectors. The same error message shows up for ports/geckolib.
  • Look at ports/geckolib/Cargo.lock. It declares a dependency on selectors 0.2.2, but there is not [[package]] section for it. Change it manually to 0.2.3.
  • Build again, the problem seems to be fixed.

@bors-servo r+

So the moral of the story is:

  • CI doesn’t test your PR, it tests a merge of your PR into master. Thanks Alex for pointing this out! When something only happens on CI, rebasing may help.
  • A git merge of branches that both have changes to Cargo.lock can result in an inconsistent Cargo.lock file. I’ll file an issue about making Cargo more robust in that situation.

@bors-servo
Copy link
Contributor

📌 Commit 0013fce has been approved by SimonSapin

@bors-servo
Copy link
Contributor

⌛ Testing commit 0013fce with merge fe0fc90...

bors-servo pushed a commit that referenced this pull request Jan 21, 2016
Update cssparser.

servo/rust-cssparser#91

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9380)
<!-- Reviewable:end -->
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jan 21, 2016
@bors-servo
Copy link
Contributor

💔 Test failed - linux-dev

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jan 21, 2016
@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Jan 21, 2016
@SimonSapin
Copy link
Member Author

Fixed duplicate dependency introduced in the rebase.

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 6fd46b5 has been approved by SimonSapin

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jan 21, 2016
@SimonSapin
Copy link
Member Author

Cargo bug: rust-lang/cargo#2302. This needs at least a better error message, if not automatic resolution.

@bors-servo
Copy link
Contributor

⌛ Testing commit 6fd46b5 with merge 1ba1fb0...

bors-servo pushed a commit that referenced this pull request Jan 21, 2016
Update cssparser.

servo/rust-cssparser#91

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9380)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt

@bors-servo bors-servo merged commit 6fd46b5 into master Jan 21, 2016
@SimonSapin SimonSapin deleted the cssparserup branch January 26, 2016 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants