Skip to content

/servo

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

Get rustc commit hash from channel manifest #18420

Merged
merged 4 commits into from Sep 12, 2017

Conversation

Reviewers

@emilio emilio

Assignees

@nox nox

Labels
None yet
Projects
None yet
Milestone
No milestone
7 participants
@SimonSapin @nox @bors-servo @bholley @jdm @emilio @highfive
@SimonSapin
Copy link
Member

SimonSapin commented Sep 8, 2017 •
edited

… added in rust-lang/rust#44218, instead of using the GitHub API.

Also upgrade to rustc 1.22.0-nightly (d93036a04 2017-09-07).

This change is Reviewable
@nox

This comment has been minimized.

Sign in to view
Copy link
Member

nox commented Sep 8, 2017

@bors-servo r+
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Sep 8, 2017

📌 Commit 3c0680a has been approved by nox

@highfive highfive assigned nox Sep 8, 2017

@highfive highfive added the S-awaiting-merge label Sep 8, 2017

@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Sep 8, 2017

⌛️ Testing commit 3c0680a with merge da2d0a5...

bors-servo added a commit that referenced this pull request Sep 8, 2017

@bors-servo
Auto merge of #18420 - servo:toml, r=nox 
Get rustc commit hash from channel manifest

… added in rust-lang/rust#44218, instead of using the GitHub API.

Also upgrade to rustc 1.22.0-nightly (d93036a04 2017-09-07), and app_units 0.5.6 (for servo/app_units#37)

<!-- 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/18420)
<!-- Reviewable:end -->
Loading status checks…
da2d0a5
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Sep 8, 2017

💔 Test failed - mac-dev-unit

@highfive highfive added S-tests-failed and removed S-awaiting-merge labels Sep 8, 2017

@SimonSapin

This comment has been minimized.

Sign in to view
Copy link
Member Author

SimonSapin commented Sep 8, 2017

On buildbot:

error: failed to run custom build command for `fontsan v0.3.2 (https://github.com/servo/fontsan#ab68da17)`
could not execute process `/Users/servo/buildbot/slave/mac-dev-unit/build/target/debug/build/fontsan-c0e0814b33162e4e/build-script-build` (never executed)

On Travis:

error: linking with `cc` failed: exit code: 1
  |
  = note: "cc" "-Wl,--as-needed" "-Wl,-z,noexecstack" "-m64"   […]
  = note: collect2: error: ld returned 1 exit status
          
error: aborting due to previous error
error: Could not compile `servo`.

… neither with any more output that seems relevant to the error. I can’t reproduce locally, on either Linux or OS X. So… I’ve no idea. Let’s see if it’s reliable?

@bors-servo retry
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Sep 8, 2017

⌛️ Testing commit 3c0680a with merge 9aca316...

bors-servo added a commit that referenced this pull request Sep 8, 2017

@bors-servo
Auto merge of #18420 - servo:toml, r=nox 
Get rustc commit hash from channel manifest

… added in rust-lang/rust#44218, instead of using the GitHub API.

Also upgrade to rustc 1.22.0-nightly (d93036a04 2017-09-07), and app_units 0.5.6 (for servo/app_units#37)

<!-- 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/18420)
<!-- Reviewable:end -->
Loading status checks…
9aca316

@highfive highfive added S-awaiting-merge and removed S-tests-failed labels Sep 8, 2017

@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Sep 8, 2017

💔 Test failed - linux-rel-wpt

@highfive highfive added S-tests-failed and removed S-awaiting-merge labels Sep 8, 2017

@SimonSapin

This comment has been minimized.

Sign in to view
Copy link
Member Author

SimonSapin commented Sep 8, 2017

At least some of the failures are:

  │ ERROR:servo: assertion failed: distance_to_stack_bottom <= max_allowable_distance
  │ assertion failed: distance_to_stack_bottom <= max_allowable_distance (thread <unnamed>, at /home/servo/buildbot/slave/linux-rel-css/build/components/style/context.rs:658)

CC @bholley @julian-seward1, should we increase the stack size?
@bholley

This comment has been minimized.

Sign in to view
Copy link
Contributor

bholley commented Sep 8, 2017

I don't think stack size is the issue - we just bumped the limit, and those assertion stacks are very short in any case.

We need to understand what the various values are to understand why it isn't working.
@bholley

This comment has been minimized.

Sign in to view
Copy link
Contributor

bholley commented Sep 8, 2017

(Note that we could increase the slop from 10k, but if that fixed it, that would also be surprising enough to warrant figuring out why).
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Sep 8, 2017

☔️ The latest upstream changes (presumably #18413) made this pull request unmergeable. Please resolve the merge conflicts.

@highfive highfive added the S-needs-rebase label Sep 8, 2017

@SimonSapin SimonSapin force-pushed the toml branch from 3c0680a to 64eb2e0 Sep 9, 2017

@highfive highfive added S-awaiting-review and removed S-tests-failed labels Sep 9, 2017

@SimonSapin

This comment has been minimized.

Sign in to view
Copy link
Member Author

SimonSapin commented Sep 9, 2017

@bors-servo try
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Sep 9, 2017

⌛️ Trying commit 64eb2e0 with merge 69bac87...

bors-servo added a commit that referenced this pull request Sep 9, 2017

@bors-servo
Auto merge of #18420 - servo:toml, r=<try> 
Get rustc commit hash from channel manifest

… added in rust-lang/rust#44218, instead of using the GitHub API.

Also upgrade to rustc 1.22.0-nightly (d93036a04 2017-09-07), and app_units 0.5.6 (for servo/app_units#37)

<!-- 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/18420)
<!-- Reviewable:end -->
Loading status checks…
69bac87
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Sep 9, 2017

💔 Test failed - linux-rel-wpt

@highfive highfive added the S-tests-failed label Sep 9, 2017

@SimonSapin

This comment has been minimized.

Sign in to view
Copy link
Member Author

SimonSapin commented Sep 12, 2017

Bisection in #18445 shows that rust-lang/rust#43931 is responsible (though possibly indirectly). Maybe more inlining is making some stack frames larger? @bholley, @emilio, would that make sense?

@highfive highfive assigned bholley and unassigned nox Sep 12, 2017

@emilio

This comment has been minimized.

Sign in to view
Copy link
Member

emilio commented Sep 12, 2017

I think so, yeah.

We can confirm it marking StyleBloom::new and StyleSharingCandidateCache::new with #[inline(never)].

@SimonSapin SimonSapin force-pushed the toml branch from 64eb2e0 to a71fe3c Sep 12, 2017

SimonSapin added a commit that referenced this pull request Sep 12, 2017

@SimonSapin
Un-inline some methods initializing thread_local! … 
… in order to limit stack frame sizes after extra inlining from
rust-lang/rust#43931

See #18420 (comment)
3b26386

@highfive highfive removed the S-tests-failed label Sep 12, 2017

@SimonSapin

This comment has been minimized.

Sign in to view
Copy link
Member Author

SimonSapin commented Sep 12, 2017

@bors-servo try
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Sep 12, 2017

⌛️ Trying commit a71fe3c with merge 415e802...

bors-servo added a commit that referenced this pull request Sep 12, 2017

@bors-servo
Auto merge of #18420 - servo:toml, r=<try> 
Get rustc commit hash from channel manifest

… added in rust-lang/rust#44218, instead of using the GitHub API.

Also upgrade to rustc 1.22.0-nightly (d93036a04 2017-09-07).

<!-- 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/18420)
<!-- Reviewable:end -->
Loading status checks…
415e802
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Sep 12, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
State: approved= try=True
@emilio

emilio approved these changes Sep 12, 2017
View changes

components/style/bloom.rs Outdated
@@ -121,6 +121,7 @@ impl<E: TElement> StyleBloom<E> {
/// Create an empty `StyleBloom`. Because StyleBloom acquires the thread-
/// local filter buffer, creating multiple live StyleBloom instances at
/// the same time on the same thread will panic.
#[inline(never)]

This comment has been minimized.

Sign in to view
Copy link
@emilio

emilio Sep 12, 2017

Member

Maybe leave a comment on the reasoning for this here and below?

This comment has been minimized.

Sign in to view
Copy link
@SimonSapin

SimonSapin Sep 12, 2017

Author Member

Done.

@SimonSapin SimonSapin force-pushed the toml branch from 46909ce to 052a8b5 Sep 12, 2017

@SimonSapin

This comment has been minimized.

Sign in to view
Copy link
Member Author

SimonSapin commented Sep 12, 2017

@bors-servo r=nox,emilio
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Sep 12, 2017

📌 Commit 052a8b5 has been approved by nox,emilio

@highfive highfive removed S-awaiting-review S-needs-rebase labels Sep 12, 2017

@highfive highfive assigned nox and unassigned bholley Sep 12, 2017

@highfive highfive added the S-awaiting-merge label Sep 12, 2017

@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Sep 12, 2017

⌛️ Testing commit 052a8b5 with merge af077a7...

bors-servo added a commit that referenced this pull request Sep 12, 2017

@bors-servo
Auto merge of #18420 - servo:toml, r=nox,emilio 
Get rustc commit hash from channel manifest

… added in rust-lang/rust#44218, instead of using the GitHub API.

Also upgrade to rustc 1.22.0-nightly (d93036a04 2017-09-07).

<!-- 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/18420)
<!-- Reviewable:end -->
Loading status checks…
af077a7
@bors-servo

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors-servo commented Sep 12, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: nox,emilio
Pushing af077a7 to master...

@bors-servo bors-servo merged commit 052a8b5 into master Sep 12, 2017
3 of 4 checks passed

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
dependency-ci Dependencies checked
Details
homu Test successful
Details

@highfive highfive removed the S-awaiting-merge label Sep 12, 2017

@SimonSapin SimonSapin deleted the toml branch Sep 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.