Skip to content

UUID.uuidString should return an upper String like on Darwin #1792

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 3 commits into from
Jan 12, 2019
Merged

UUID.uuidString should return an upper String like on Darwin #1792

merged 3 commits into from
Jan 12, 2019

Conversation

pvieito
Copy link
Contributor

@pvieito pvieito commented Dec 3, 2018

@xwu
Copy link
Contributor

xwu commented Dec 9, 2018

Could you please write a more descriptive comment above as to what's being fixed? The link is to a bug that's already been fixed for several years.

This PR also needs a test that fails without the patch and passes with the patch. Also, the same change should be committed to the Darwin Foundation overlay.

@pvieito
Copy link
Contributor Author

pvieito commented Dec 10, 2018

It seems that now Foundation builds against the system libuuid instead of the included version at swift-corelibs-foundation/uuid/.

As uuid_unparse is not guaranteed to be uppercase, in Linux (at least in Ubuntu 16.04) it returns a lowercase string. We should be using uuid_unparse_upper (_cf_uuid_unparse_upper) to ensure it returns an uppercase string.

In the Darwin overlay it is OK to use _cf_uuid_unparse as in Apple Darwin.uuid uuid_unparse already returns an uppercase string (although the maintainers should probably update it to maintain the same codebase in both implementations).

@compnerd
Copy link
Member

I believe that it is almost guaranteed to be lowercase. The RFC 4122 expects the UUID to be lower case. This was something that I had to fix in the compiler as well for Windows as the UUID generation code in OLE also generated it lower case.

@compnerd
Copy link
Member

CC: @millenomi

@compnerd
Copy link
Member

@swift-ci please test

@pvieito
Copy link
Contributor Author

pvieito commented Jan 1, 2019

@swift-ci please test

1 similar comment
@compnerd
Copy link
Member

compnerd commented Jan 2, 2019

@swift-ci please test

@compnerd compnerd merged commit f033eaa into swiftlang:master Jan 12, 2019
@pvieito pvieito deleted the patch-1 branch April 11, 2019 20:16
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.

3 participants