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

Remove dependency on regex_macros #8653

Merged
merged 2 commits into from Nov 25, 2015
Merged

Remove dependency on regex_macros #8653

merged 2 commits into from Nov 25, 2015

Conversation

@SimonSapin
Copy link
Member

SimonSapin commented Nov 23, 2015

This reduces the amount of code using unstable features that we depend on.
The hand-written IP address parser is probably just as fast.

Review on Reviewable
@jdm
Copy link
Member

jdm commented Nov 23, 2015 

./components/net/hsts.rs:7: use statement is not in alphabetical order

    expected: std::net::{Ipv4Addr, Ipv6Addr}

    found: std::str::{from_utf8}

./components/net/hsts.rs:8: use statement is not in alphabetical order

    expected: std::str::{from_utf8}

    found: std::net::{Ipv4Addr, Ipv6Addr}
@jdm
Copy link
Member

jdm commented Nov 23, 2015

Otherwise this is fine with me.
@jdm jdm added S-fails-tidy and removed S-awaiting-review labels Nov 23, 2015
@jdm jdm self-assigned this Nov 23, 2015
@SimonSapin SimonSapin force-pushed the no-regex-macros branch from a2ab91d to acde3a1 Nov 23, 2015
@SimonSapin
Copy link
Member Author

SimonSapin commented Nov 23, 2015

Fixed
@jdm
Copy link
Member

jdm commented Nov 23, 2015

@bors-servo: r+
@bors-servo
Copy link
Contributor

bors-servo commented Nov 23, 2015

📌 Commit acde3a1 has been approved by jdm
@bors-servo
Copy link
Contributor

bors-servo commented Nov 23, 2015

⌛ Testing commit acde3a1 with merge ff152b7...
bors-servo added a commit that referenced this pull request Nov 23, 2015
@bors-servo
Auto merge of #8653 - servo:no-regex-macros, r=jdm
ff152b7 
Remove dependency on regex_macros

This reduces the amount of code using unstable features that we depend on.
The hand-written IP address parser is probably just as fast.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8653)
<!-- Reviewable:end -->
@jdm jdm removed the S-fails-tidy label Nov 23, 2015
@bors-servo
Copy link
Contributor

bors-servo commented Nov 23, 2015

💔 Test failed - mac-dev-ref-unit
@jdm
Copy link
Member

jdm commented Nov 23, 2015 

---- resource_task::test_parse_hostsfile_with_invalid_ipv6_addresses stdout ----
    thread 'resource_task::test_parse_hostsfile_with_invalid_ipv6_addresses' panicked at 'assertion failed: `(left == right)` (left: `0`, right: `2`)', /Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/net/resource_task.rs:140
stack backtrace:
   1:        0x10b3d14d8 - sys::backtrace::tracing::imp::write::h662793599f713c74T5s
   2:        0x10b3d2fdf - panicking::log_panic::_<closure>::closure.38923
   3:        0x10b3d2aac - panicking::log_panic::h87af6096e0b6fa7c4Uw
   4:        0x10b3c3ef6 - sys_common::unwind::begin_unwind_inner::h1e338bc64cf56d9dr8r
   5:        0x10b3c42de - sys_common::unwind::begin_unwind_fmt::he992719329bdd2c6x7r
   6:        0x10ab92d67 - resource_task::test_parse_hostsfile_with_invalid_ipv6_addresses::h0833a4853336be88vQb
   7:        0x10b3627cb - boxed::_<impl>::call_box::call_box::h1439439458556714188
   8:        0x10b364f1d - boxed::_<impl>::call_box::call_box::h8069092559954270103
   9:        0x10b363122 - sys_common::unwind::try::try_fn::try_fn::h11324081254561251759
  10:        0x10b3d09a8 - __rust_try
  11:        0x10b3ce32e - sys_common::unwind::try::inner_try::hc13d8e198528cd0fZ4r
  12:        0x10b3632d2 - boxed::_<impl>::call_box::call_box::h6022582708213645161
  13:        0x10b3d23ad - sys::thread::_<impl>::new::thread_start::h906ddce3a93d4852ksw
  14:     0x7fff91449059 - _pthread_body
  15:     0x7fff91448fd6 - _pthread_start

---- resource_task::test_parse_hostsfile_with_valid_ipv6_addresses stdout ----
    thread 'resource_task::test_parse_hostsfile_with_valid_ipv6_addresses' panicked at 'assertion failed: `(left == right)` (left: `12`, right: `9`)', /Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/net/resource_task.rs:129
stack backtrace:
   1:        0x10b3d14d8 - sys::backtrace::tracing::imp::write::h662793599f713c74T5s
   2:        0x10b3d2fdf - panicking::log_panic::_<closure>::closure.38923
   3:        0x10b3d2aac - panicking::log_panic::h87af6096e0b6fa7c4Uw
   4:        0x10b3c3ef6 - sys_common::unwind::begin_unwind_inner::h1e338bc64cf56d9dr8r
   5:        0x10b3c42de - sys_common::unwind::begin_unwind_fmt::he992719329bdd2c6x7r
   6:        0x10ab92b17 - resource_task::test_parse_hostsfile_with_valid_ipv6_addresses::h5ef6137ba1c2dfbdVOb
   7:        0x10b3627cb - boxed::_<impl>::call_box::call_box::h1439439458556714188
   8:        0x10b364f1d - boxed::_<impl>::call_box::call_box::h8069092559954270103
   9:        0x10b363122 - sys_common::unwind::try::try_fn::try_fn::h11324081254561251759
  10:        0x10b3d09a8 - __rust_try
  11:        0x10b3ce32e - sys_common::unwind::try::inner_try::hc13d8e198528cd0fZ4r
  12:        0x10b3632d2 - boxed::_<impl>::call_box::call_box::h6022582708213645161
  13:        0x10b3d23ad - sys::thread::_<impl>::new::thread_start::h906ddce3a93d4852ksw
  14:     0x7fff91449059 - _pthread_body
  15:     0x7fff91448fd6 - _pthread_start


failures:
    resource_task::test_parse_hostsfile_with_invalid_ipv6_addresses
    resource_task::test_parse_hostsfile_with_valid_ipv6_addresses
SimonSapin referenced this pull request Nov 23, 2015
@gilles-leblanc
Add more robust hostsfile parsing
0faa55d 
Adds hostsfile parsing support for:
* Tabs
* Comments (line and end of line)
* IPv4 address validation
* Basic IPv6 address validation
* End of line whitespaces
* Host name alias (multiple host names per address)

Fixes #5063
@SimonSapin
Copy link
Member Author

SimonSapin commented Nov 23, 2015

The original regex for IPv6 includes a /\d{1,3} suffix which looks like a subnet notation (and unit tests test accordingly), but the rest of the code doesn’t seem to do any address range computation. I’m inclined to change the test here.

CC @gilles-leblanc 0faa55d#commitcomment-14562450
@SimonSapin SimonSapin force-pushed the no-regex-macros branch from acde3a1 to 0619900 Nov 24, 2015
@Ms2ger Ms2ger assigned Ms2ger and unassigned jdm Nov 24, 2015
@SimonSapin SimonSapin force-pushed the no-regex-macros branch from 0619900 to f21b2f6 Nov 24, 2015
@Ms2ger
Copy link
Contributor

Ms2ger commented Nov 24, 2015

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Nov 24, 2015

📌 Commit f21b2f6 has been approved by Ms2ger
@bors-servo
Copy link
Contributor

bors-servo commented Nov 24, 2015

💔 Test failed - mac-rel-css
@SimonSapin
Copy link
Member Author

SimonSapin commented Nov 24, 2015

@bors-servo retry
@jdm
Copy link
Member

jdm commented Nov 24, 2015

It's more likely #7437.
@bors-servo
Copy link
Contributor

bors-servo commented Nov 24, 2015

☔ The latest upstream changes (presumably #8609) made this pull request unmergeable. Please resolve the merge conflicts.
SimonSapin added 2 commits Nov 23, 2015
@SimonSapin
Remove IPv6 subnet parsing in host files like ::1/64
0dfdc94 
The host replacement code doesn’t do anything useful with it.
(It only compares strings.)
@SimonSapin
Remove dependency on regex_macros
Loading status checks…
45ec900 
This reduces the amount of code using unstable features that we depend on.
The hand-written IP address parser is probably just as fast.
@SimonSapin SimonSapin force-pushed the no-regex-macros branch from f21b2f6 to 45ec900 Nov 24, 2015
@SimonSapin
Copy link
Member Author

SimonSapin commented Nov 24, 2015

@bors-servo r=Ms2ger

Rebased, conflict resolved automatically by kdiff3
@bors-servo
Copy link
Contributor

bors-servo commented Nov 24, 2015

📌 Commit 45ec900 has been approved by Ms2ger
@bors-servo
Copy link
Contributor

bors-servo commented Nov 24, 2015

⌛ Testing commit 45ec900 with merge 6f35b86...
bors-servo added a commit that referenced this pull request Nov 24, 2015
@bors-servo
Auto merge of #8653 - servo:no-regex-macros, r=Ms2ger
Loading status checks…
6f35b86 
Remove dependency on regex_macros

This reduces the amount of code using unstable features that we depend on.
The hand-written IP address parser is probably just as fast.

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

bors-servo commented Nov 24, 2015

💔 Test failed - mac-rel-wpt
@jdm
Copy link
Member

jdm commented Nov 24, 2015

@bors-servo: retry
@bors-servo
Copy link
Contributor

bors-servo commented Nov 24, 2015

⚡ Previous build results for android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css are reusable. Rebuilding only mac-rel-wpt...
@bors-servo
Copy link
Contributor

bors-servo commented Nov 25, 2015

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt
@bors-servo bors-servo merged commit 45ec900 into master Nov 25, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Nov 25, 2015
Merged
@Ms2ger Ms2ger deleted the no-regex-macros branch Nov 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Ms2ger Ms2ger

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@SimonSapin @jdm @bors-servo @Ms2ger @highfive
You can’t perform that action at this time.