Skip to content

Add various strcase* functions and getline #1130

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

Merged
merged 16 commits into from
Nov 27, 2018

Conversation

palfrey
Copy link
Contributor

@palfrey palfrey commented Nov 20, 2018

Adds

  • strcasestr
  • strcasecmp
  • strncasecmp
  • getline

I think they're semi-universal, but shall see what CI pops up...

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 20, 2018

It appears that the freebsd image used in CI lacks getline. Maybe a header is missing in the libc-test/build.rs for freebsd ? Otherwise you might want to take a look at whether more recent freebsd versions expose this.

@palfrey
Copy link
Contributor Author

palfrey commented Nov 20, 2018

@gnzlbg https://www.freebsd.org/cgi/man.cgi?query=getline&apropos=0&sektion=0&manpath=FreeBSD+8.2-RELEASE&format=html indicates FreeBSD 8.0+ has getline, but only if you define _WITH_GETLINE, so let's try that...

@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 20, 2018

r? @gnzlbg

@palfrey please ping me when CI is green

@rust-highfive rust-highfive assigned gnzlbg and unassigned alexcrichton Nov 20, 2018
@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 21, 2018

@bors: r+

@bors
Copy link
Contributor

bors commented Nov 21, 2018

📋 Looks like this PR is still in progress, ignoring approval.

Hint: Remove [WIP] from this PR's title when it is ready for review.

@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 21, 2018

The windows msvc toolchains (the gnu ones are ok) are failing with:

C:\projects\libc\target\x86_64-pc-windows-msvc\debug\build\libc-test-9926ed016ab2f41f\out\main.c(591): error C2065: 'strcasecmp': undeclared identifier
C:\projects\libc\target\x86_64-pc-windows-msvc\debug\build\libc-test-9926ed016ab2f41f\out\main.c(591): warning C4047: 'return': 'int (__cdecl *)(const char *,const char *)' differs in levels of indirection from 'int'
C:\projects\libc\target\x86_64-pc-windows-msvc\debug\build\libc-test-9926ed016ab2f41f\out\main.c(596): error C2065: 'strncasecmp': undeclared identifier
C:\projects\libc\target\x86_64-pc-windows-msvc\debug\build\libc-test-9926ed016ab2f41f\out\main.c(596): warning C4047: 'return': 'int (__cdecl *)(const char *,const char *,std::size_t)' differs in levels of indirection from 'int'

Could you take a look?

@gnzlbg gnzlbg mentioned this pull request Nov 21, 2018
@bors
Copy link
Contributor

bors commented Nov 21, 2018

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

@palfrey palfrey changed the title [WIP] Add various strcase* functions and getline Add various strcase* functions and getline Nov 21, 2018
@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 22, 2018

I think it would be good to add these to redox, cloudabi, etc. as well.

@palfrey
Copy link
Contributor Author

palfrey commented Nov 23, 2018

I dug through https://github.com/redox-os/relibc and https://github.com/NuxiNL/cloudlibc and added all of these functions that I could find there, along with what I think is the right header support...

@palfrey
Copy link
Contributor Author

palfrey commented Nov 23, 2018

Uh, so I think all the Travis failures are nothing to do with this branch e.g.

+docker build -t libc -f ci/docker/sparc64-unknown-linux-gnu/Dockerfile ci/
Sending build context to Docker daemon    107kB
Step 1/7 : FROM debian:stretch
stretch: Pulling from library/debian
age configuration: unknown blob

Can we kick it in some way to rebuild and try again?

@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 24, 2018

@bors: r+

@bors
Copy link
Contributor

bors commented Nov 24, 2018

💔 Test failed - status-travis

@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 24, 2018

@bors: try

@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 24, 2018

@bors: retry

@bors
Copy link
Contributor

bors commented Nov 24, 2018

⌛ Trying commit 75071fe with merge 6ef0a8e...

bors added a commit that referenced this pull request Nov 24, 2018
Add various strcase* functions and getline

Adds
* `strcasestr`
* `strcasecmp`
* `strncasecmp`
*  `getline`

I *think* they're semi-universal, but shall see what CI pops up...
@bors
Copy link
Contributor

bors commented Nov 24, 2018

💔 Test failed - status-travis

@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 24, 2018

@bors: retry

@bors
Copy link
Contributor

bors commented Nov 24, 2018

⌛ Trying commit 75071fe with merge e4a81c7...

bors added a commit that referenced this pull request Nov 24, 2018
Add various strcase* functions and getline

Adds
* `strcasestr`
* `strcasecmp`
* `strncasecmp`
*  `getline`

I *think* they're semi-universal, but shall see what CI pops up...
@bors
Copy link
Contributor

bors commented Nov 24, 2018

💔 Test failed - status-travis

@palfrey
Copy link
Contributor Author

palfrey commented Nov 24, 2018

b05e058 is a bit of a hack, but I managed to hit (temporarily) one of the Travis issues locally and manually pulling the Docker images before building seemed to fix the problem

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Nov 26, 2018

📌 Commit 0942070 has been approved by alexcrichton

@palfrey
Copy link
Contributor Author

palfrey commented Nov 26, 2018

Good news: down to just 1 failure ;)
Bad news: Failure on sparc64-unknown-linux-gnu

+ curl -LO https://cdimage.debian.org/cdimage/ports/9.0/sparc64/iso-cd/debian-9.0-sparc64-NETINST-1.iso
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   362  100   362    0     0    456      0 --:--:-- --:--:-- --:--:--   456
  0     0    0     0    0     0      0      0 --:--:--  0:05:01 --:--:--     0
curl: (28) Operation timed out after 300808 milliseconds with 0 out of 0 bytes received

Pretty sure this still isn't due to this PR...

@bors
Copy link
Contributor

bors commented Nov 26, 2018

⌛ Testing commit 0942070 with merge 717d77e...

bors added a commit that referenced this pull request Nov 26, 2018
Add various strcase* functions and getline

Adds
* `strcasestr`
* `strcasecmp`
* `strncasecmp`
*  `getline`

I *think* they're semi-universal, but shall see what CI pops up...
@bors
Copy link
Contributor

bors commented Nov 26, 2018

💔 Test failed - status-travis

@palfrey
Copy link
Contributor Author

palfrey commented Nov 26, 2018

@bors: retry

@bors
Copy link
Contributor

bors commented Nov 26, 2018

@palfrey: 🔑 Insufficient privileges: not in try users

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Nov 27, 2018

⌛ Testing commit 0942070 with merge aee584c...

bors added a commit that referenced this pull request Nov 27, 2018
Add various strcase* functions and getline

Adds
* `strcasestr`
* `strcasecmp`
* `strncasecmp`
*  `getline`

I *think* they're semi-universal, but shall see what CI pops up...
@bors
Copy link
Contributor

bors commented Nov 27, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing aee584c to master...

@bors bors merged commit 0942070 into rust-lang:master Nov 27, 2018
@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 27, 2018

Thank you!

@palfrey palfrey deleted the strcase-various branch November 27, 2018 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants