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

Sanitise web fonts #6030

Closed
wants to merge 1 commit into from
Closed

Sanitise web fonts #6030

wants to merge 1 commit into from

Conversation

@kmcallister
Copy link
Contributor

kmcallister commented May 13, 2015

Fixes #3030.

r? @pcwalton.

If you can take a look at the fontsan repo as well, that'd be great.

Review on Reviewable

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented May 13, 2015

Critic review: https://critic.hoppipolla.co.uk/r/4983

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@pcwalton
Copy link
Contributor

pcwalton commented May 13, 2015

Both this and servo/fontsan seem reasonable.

@bors-servo: r+

@bors-servo
Copy link
Contributor

bors-servo commented May 13, 2015

📌 Commit 52ef9b5 has been approved by pcwalton

@pcwalton
Copy link
Contributor

pcwalton commented May 13, 2015

@bors-servo
Copy link
Contributor

bors-servo commented May 13, 2015

📌 Commit 52ef9b5 has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented May 13, 2015

Testing commit 52ef9b5 with merge 86d3321...

bors-servo pushed a commit that referenced this pull request May 13, 2015
Fixes #3030.

r? @pcwalton.

If you can take a look at [the fontsan repo](https://github.com/servo/fontsan) as well, that'd be great.

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

bors-servo commented May 13, 2015

💔 Test failed - mac2

@jdm
Copy link
Member

jdm commented May 13, 2015

Build failed, waiting for other jobs to finish...
failed to run custom build command for `fontsan v0.1.0 (https://github.com/servo/fontsan#9c37c78c)`
Process didn't exit successfully: `/Users/servo/buildbot/slave/mac2/build/components/servo/target/debug/build/fontsan-94a84b8df1707525/build-script-build` (exit code: 101)
--- stderr
thread '<main>' panicked at 'called `Result::unwrap()` on an `Err` value: "`\"pkg-config\" \"--libs\" \"--cflags\" \"zlib\"` did not exit successfully: exit code: 1\n--- stderr\nPackage zlib was not found in the pkg-config search path.\nPerhaps you should add the directory containing `zlib.pc\'\nto the PKG_CONFIG_PATH environment variable\nNo package \'zlib\' found\n"', /Users/larsberg/rust/src/libcore/result.rs:729
stack backtrace:
   1:        0x10a16a4ef - sys::backtrace::write::h6990067d88907b48zVr
   2:        0x10a172ec4 - panicking::on_panic::he976cdbadffd9e1bEVv
   3:        0x10a12eed5 - rt::unwind::begin_unwind_inner::ha519446d83876aa8nDv
   4:        0x10a12fd0c - rt::unwind::begin_unwind_fmt::h15c2d4adf8e8ecdatCv
   5:        0x10a1726bc - rust_begin_unwind
   6:        0x10a1c5495 - panicking::panic_fmt::haa553fbfab6eff2cNKy
   7:        0x10a08d8b3 - result::Result<T, E>::unwrap::h8250000126647709007
   8:        0x10a08befe - main::he4d0aadffe357b93iaa
   9:        0x10a1f8278 - rust_try_inner
  10:        0x10a1f8265 - rust_try
  11:        0x10a173948 - rt::lang_start::h62b79b4f7043bc7a9Pv
  12:        0x10a09318e - main

This will need a README.md and https://github.com/servo/saltfs/ change.

@kmcallister
Copy link
Contributor Author

kmcallister commented May 13, 2015

Surely we already have at least one copy of zlib among all our dependencies. I'd rather not add a duplicate. So I'll look into what the situation is.

@kmcallister kmcallister force-pushed the kmcallister:fontsan branch from 52ef9b5 to c844c72 May 14, 2015
@kmcallister
Copy link
Contributor Author

kmcallister commented May 14, 2015

I modified fontsan to use the libz-sys crate.

@kmcallister
Copy link
Contributor Author

kmcallister commented May 14, 2015

@bors-servo: r=pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented May 14, 2015

📌 Commit c844c72 has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented May 14, 2015

Testing commit c844c72 with merge dfff928...

bors-servo pushed a commit that referenced this pull request May 14, 2015
Fixes #3030.

r? @pcwalton.

If you can take a look at [the fontsan repo](https://github.com/servo/fontsan) as well, that'd be great.

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

bors-servo commented May 14, 2015

💔 Test failed - mac2

@jdm
Copy link
Member

jdm commented May 14, 2015

failed to run custom build command for `fontsan v0.1.0 (https://github.com/servo/fontsan#b7eadbc8)`
Process didn't exit successfully: `/Users/servo/buildbot/slave/mac2/build/components/servo/target/debug/build/fontsan-94a84b8df1707525/build-script-build` (exit code: 101)
--- stderr
thread '<main>' panicked at 'called `Result::unwrap()` on an `Err` value: "`\"pkg-config\" \"--libs\" \"--cflags\" \"zlib\"` did not exit successfully: exit code: 1\n--- stderr\nPackage zlib was not found in the pkg-config search path.\nPerhaps you should add the directory containing `zlib.pc\'\nto the PKG_CONFIG_PATH environment variable\nNo package \'zlib\' found\n"', /Users/larsberg/rust/src/libcore/result.rs:729
stack backtrace:
   1:        0x10d4f94ef - sys::backtrace::write::h6990067d88907b48zVr
   2:        0x10d501ec4 - panicking::on_panic::he976cdbadffd9e1bEVv
   3:        0x10d4bded5 - rt::unwind::begin_unwind_inner::ha519446d83876aa8nDv
   4:        0x10d4bed0c - rt::unwind::begin_unwind_fmt::h15c2d4adf8e8ecdatCv
   5:        0x10d5016bc - rust_begin_unwind
   6:        0x10d554495 - panicking::panic_fmt::haa553fbfab6eff2cNKy
   7:        0x10d4188d3 - result::Result<T, E>::unwrap::h14041122334415785811
   8:        0x10d416f1e - main::h7949b7c817f4fff5iaa
   9:        0x10d587278 - rust_try_inner
  10:        0x10d587265 - rust_try
  11:        0x10d502948 - rt::lang_start::h62b79b4f7043bc7a9Pv
  12:        0x10d41e1ae - main
@kmcallister
Copy link
Contributor Author

kmcallister commented May 14, 2015

Maybe I'll modify it to use miniz since we already build and link that.

@kmcallister kmcallister force-pushed the kmcallister:fontsan branch from c844c72 to 3e21c82 May 14, 2015
@kmcallister
Copy link
Contributor Author

kmcallister commented May 14, 2015

@bors-servo: r=pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented May 14, 2015

📌 Commit 3e21c82 has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented May 15, 2015

Testing commit 3e21c82 with merge 087038b...

bors-servo pushed a commit that referenced this pull request May 15, 2015
Fixes #3030.

r? @pcwalton.

If you can take a look at [the fontsan repo](https://github.com/servo/fontsan) as well, that'd be great.

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

bors-servo commented May 15, 2015

💔 Test failed - mac2

@jdm
Copy link
Member

jdm commented May 15, 2015

failed to run custom build command for `fontsan v0.1.0 (https://github.com/servo/fontsan#9a49a339)`
Process didn't exit successfully: `/Users/servo/buildbot/slave/mac2/build/components/servo/target/debug/build/fontsan-94a84b8df1707525/build-script-build` (exit code: 101)
--- stderr
+ fake_zlib=/Users/servo/buildbot/slave/mac2/build/components/servo/target/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install
+ mkdir -p /Users/servo/buildbot/slave/mac2/build/components/servo/target/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install/include /Users/servo/buildbot/slave/mac2/build/components/servo/target/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install/lib
+ cp /Users/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/src/fake-zlib.h /Users/servo/buildbot/slave/mac2/build/components/servo/target/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install/include/zlib.h
+ cp /Users/servo/buildbot/slave/mac2/build/components/servo/target/debug/build/miniz-sys-d7e62e66665f9648/out/libminiz.a /Users/servo/buildbot/slave/mac2/build/components/servo/target/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install/lib/libz.a
+ cd ots/
+ git submodule init
+ git submodule update
+ autoreconf --force --install --verbose
./scripts/build-ots.sh: line 17: autoreconf: command not found
thread '<main>' panicked at 'assertion failed: Command::new("./scripts/build-ots.sh").status().unwrap().success()', /Users/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/build.rs:9
stack backtrace:
   1:        0x1040da4ef - sys::backtrace::write::h6990067d88907b48zVr
   2:        0x1040e2ec4 - panicking::on_panic::he976cdbadffd9e1bEVv
   3:        0x10409eed5 - rt::unwind::begin_unwind_inner::ha519446d83876aa8nDv
   4:        0x10403218e - rt::unwind::begin_unwind::h7948925239011312833
   5:        0x10402ed8c - main::h8f6fc4e2a6c6102dhaa
   6:        0x104168278 - rust_try_inner
   7:        0x104168265 - rust_try
   8:        0x1040e3948 - rt::lang_start::h62b79b4f7043bc7a9Pv
   9:        0x104036ace - main
Fixes #3030.
@kmcallister kmcallister force-pushed the kmcallister:fontsan branch from 3e21c82 to 74ddd33 May 15, 2015
@kmcallister
Copy link
Contributor Author

kmcallister commented May 15, 2015

@bors-servo: r=pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented May 15, 2015

📌 Commit 74ddd33 has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented May 15, 2015

Testing commit 74ddd33 with merge 4873623...

bors-servo pushed a commit that referenced this pull request May 15, 2015
Fixes #3030.

r? @pcwalton.

If you can take a look at [the fontsan repo](https://github.com/servo/fontsan) as well, that'd be great.

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

bors-servo commented May 15, 2015

💔 Test failed - gonk

@jdm
Copy link
Member

jdm commented May 15, 2015

failed to run custom build command for `fontsan v0.1.0 (https://github.com/servo/fontsan#7915f5be)`
Process didn't exit successfully: `/home/servo/buildbot/slave/gonk/build/ports/gonk/target/debug/build/fontsan-94a84b8df1707525/build-script-build` (exit code: 101)
--- stdout

WARNING: Using a snapshot of autoconf output for ots.

checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking whether make supports nested variables... (cached) yes
checking for gcc... arm-linux-androideabi-gcc
checking whether the C compiler works... no

--- stderr
+ fake_zlib=/home/servo/buildbot/slave/gonk/build/ports/gonk/target/arm-linux-androideabi/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install
+ mkdir -p /home/servo/buildbot/slave/gonk/build/ports/gonk/target/arm-linux-androideabi/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install/include /home/servo/buildbot/slave/gonk/build/ports/gonk/target/arm-linux-androideabi/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install/lib
+ cp /home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/src/fake-zlib.h /home/servo/buildbot/slave/gonk/build/ports/gonk/target/arm-linux-androideabi/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install/include/zlib.h
+ cp /home/servo/buildbot/slave/gonk/build/ports/gonk/target/arm-linux-androideabi/debug/build/miniz-sys-d7e62e66665f9648/out/libminiz.a /home/servo/buildbot/slave/gonk/build/ports/gonk/target/arm-linux-androideabi/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install/lib/libz.a
+ cd ots/
+ git submodule init
+ git submodule update
+ which autoreconf
+ echo
+ echo WARNING: Using a snapshot of autoconf output for ots.
+ echo
+ cp /home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/scripts/ots-autoconf-snapshot/Makefile.in /home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/scripts/ots-autoconf-snapshot/README.ots-autoconf-snapshot /home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/scripts/ots-autoconf-snapshot/aclocal.m4 /home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/scripts/ots-autoconf-snapshot/compile /home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/scripts/ots-autoconf-snapshot/config.guess /home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/scripts/ots-autoconf-snapshot/config.h.in /home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/scripts/ots-autoconf-snapshot/config.sub /home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/scripts/ots-autoconf-snapshot/configure /home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/scripts/ots-autoconf-snapshot/depcomp /home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/scripts/ots-autoconf-snapshot/install-sh /home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/scripts/ots-autoconf-snapshot/missing /home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/scripts/ots-autoconf-snapshot/test-driver .
+ FLAGS=-fPIC -include /home/servo/buildbot/slave/gonk/build/ports/gonk/target/arm-linux-androideabi/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install/include/zlib.h
+ ./configure --with-zlib=/home/servo/buildbot/slave/gonk/build/ports/gonk/target/arm-linux-androideabi/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install CFLAGS=-fPIC -include /home/servo/buildbot/slave/gonk/build/ports/gonk/target/arm-linux-androideabi/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install/include/zlib.h CXXFLAGS=-fPIC -include /home/servo/buildbot/slave/gonk/build/ports/gonk/target/arm-linux-androideabi/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install/include/zlib.h LDFLAGS=-L /home/servo/buildbot/slave/gonk/build/ports/gonk/target/arm-linux-androideabi/debug/build/fontsan-94a84b8df1707525/out/fake-zlib-install/lib
configure: error: in `/home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/ots':
configure: error: C compiler cannot create executables
See `config.log' for more details
thread '<main>' panicked at 'assertion failed: Command::new("./scripts/build-ots.sh").status().unwrap().success()', /home/servo/.cargo/git/checkouts/fontsan-91a22877debaa5f7/master/build.rs:9
stack backtrace:
   1:     0x7fe9a7ef1019 - sys::backtrace::write::h6dd465a29c00292cqYr
   2:     0x7fe9a7ef90c1 - panicking::on_panic::hce7ca60568ec6ad9low
   3:     0x7fe9a7ebaa62 - rt::unwind::begin_unwind_inner::h2ea6b65af87d2482v3v
   4:     0x7fe9a862a7ee - rt::unwind::begin_unwind::h12393341848838316507
                        at /home/larsberg/rust/src/libstd/rt/unwind.rs:522
   5:     0x7fe9a862749e - main::h9d7316832bfdb171haa
                        at /home/servo/buildbot/slave/gonk/build/ports/gonk/<std macros>:3
   6:     0x7fe9a7f6b918 - rust_try_inner
   7:     0x7fe9a7f6b905 - rust_try
   8:     0x7fe9a7efa9ef - rt::lang_start::h09e0526b407d67dbQiw
   9:     0x7fe9a862f164 - main
  10:     0x7fe9a762fec4 - __libc_start_main
  11:     0x7fe9a8627238 - <unknown>
  12:                0x0 - <unknown>
@bors-servo
Copy link
Contributor

bors-servo commented May 26, 2015

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

@metajack
Copy link
Contributor

metajack commented Jun 1, 2015

@kmc Let's try and get this landed this week. Let me know if you need help.

@metajack
Copy link
Contributor

metajack commented Jul 29, 2015

@glennw Can you take this over?

@nox
Copy link
Member

nox commented Sep 14, 2015

Superseded by #7634.

@nox nox closed this Sep 14, 2015
@nox nox removed the S-needs-new-owner label Sep 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

8 participants
You can’t perform that action at this time.