Skip to content

std::net: Ipv4Addr and Ipv6Addr improvements #51832

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

Closed
wants to merge 6 commits into from
Closed

std::net: Ipv4Addr and Ipv6Addr improvements #51832

wants to merge 6 commits into from

Conversation

little-dude
Copy link
Contributor

This is work in progress, but I'm opening the PR because I already have a few questions.

This PR:

As I said I have a few questions:

  • I'd like to add tests, but for that I need to be able to run them. I tried ./x.py -i test src/libstd --test-args std::net and a few variants, but on my laptop, those commands take more than an hour, so it's not viable. So how can I run the tests in src/libstd/net/ip.rs?
  • I added a few methods, and I think I need to mark them as unstable somehow. I see some methods are decorated with #[stable(since = "1.7.0", feature = "ip_17")], but I'm not sure what ip_17 stands for. Should I add #[unstable]?

@rust-highfive
Copy link
Contributor

r? @TimNN

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 27, 2018
@rust-highfive
Copy link
Contributor

The job x86_64-gnu-llvm-3.9 of 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.

[00:05:03] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:05:04] tidy error: /checkout/src/libstd/net/ip.rs:512: line longer than 100 chars
[00:05:04] tidy error: /checkout/src/libstd/net/ip.rs:1177: line longer than 100 chars
[00:05:05] some tidy checks failed
[00:05:05] 
[00:05:05] 
[00:05:05] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:05:05] 
[00:05:05] 
[00:05:05] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:05:05] Build completed unsuccessfully in 0:01:49
[00:05:05] Build completed unsuccessfully in 0:01:49
[00:05:05] make: *** [tidy] Error 1
[00:05:05] Makefile:79: recipe for target 'tidy' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:02174ee0
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:145bcdbb:start=1530072386266320854,finish=1530072386273396528,duration=7075674
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:3a4bd6ee
$ head -30 ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
head: cannot open ‘./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers’ for reading: No such file or directory
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:037306b0
$ dmesg | grep -i kill

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)

@therealbstern
Copy link
Contributor

ISTR from when I worked on is_unspecified that you can leave the unstable markings alone (neither adding nor deleting them). I think that when someone approves the stability, they add the correct feature flags and delete the unstable decoration.

ip17 is "We put this in std::net::ip in Rust 1.7.0" and we don't know which Rust will get this, so I think you should leave it alone too.

@TimNN
Copy link
Contributor

TimNN commented Jul 3, 2018

@rust-lang/libs: This PR adds several new methods Ipv4Addr and, IIUC, changes the behavior of some methods. Could you take a look at this PR and also, if possible, assign someone familiar with this topic?

@alexcrichton
Copy link
Member

It looks like these methods aren't stable yet so there shouldn't be any stability worries here, in which case seems fine to me to land!

@little-dude
Copy link
Contributor Author

@alexcrichton this is not totally done yet sorry, I'll push one more change for Ipv6Addr. Probably tomorrow, but maybe only this week-end if you don't mind waiting.

@alexcrichton
Copy link
Member

Sure thing!

@pietroalbini pietroalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 9, 2018
@TimNN
Copy link
Contributor

TimNN commented Jul 17, 2018

Ping from triage, @little-dude: We haven't heard from you in a while, what is the status of this PR?

@pietroalbini
Copy link
Member

Thank you for this PR @little-dude! Unfortunately we haven't heard from you on this in a while, so I'm closing the PR to keep things tidy. Don't worry though, if you'll have time again in the future please reopen this PR, we'll be happy to review it again!

@pietroalbini pietroalbini added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants