-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Implement CoCreateGuid using uuid_generate on Unix #1738
Conversation
@jkotas Can you take a look please? |
if(NOT HAVE_LIBUUID_H) | ||
unset(HAVE_LIBUUID_H CACHE) | ||
message(FATAL_ERROR "Cannot find libuuid. Try installing uuid-dev or the appropriate packages for your platform") | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is libuuid installed by default in common distros?
Ah, we will need to install libuuid to our Linux / FreeBSD / CentOS CI machines. |
LGTM |
And also update "getting started" instructions... |
@jkotas It was installed on my OSX, but not on my Ubuntu and obviously from the CI neither on CentOS and FreeBSD. |
It fixes #1602 |
@ellismg Could you please install uuid-dev package on our CI FreeBSD / CentOS machines? |
@janvorli I'll look into it |
@mmitche thanks! |
@janvorli Installed for centos and SuSE. FreeBSD does not have an obvious package for this. Can you find out what it is? |
@janvorli Also did you update the build documentation and cmake detection to indicate that this needs to be installed. |
This also contributes to https://github.com/dotnet/corefx/issues/3573. Before this change, Guid.NewGuid() on my Linux VM is ~200x slower than on my Windows machine. After this change, it's "only" ~60x slower. Still a significant difference, but quite a nice improvement. |
@mmitche I am trying to figure the FreeBSD out. CMake detection is updated, I have locally updated the build doc for Linux, but want to update the FreeBSD too before sending in another checkin. |
Looks like freebsd already has a similar function built-in: http://www.freebsd.org/cgi/man.cgi?query=uuid_create |
@akoeplinger thanks! That's great. I am just installing FreeBSD here so I'll try to use it instead of the lib uuid for FreeBSD |
LGTM modulo others' comments. |
41f7aaf
to
d44a84f
Compare
@mmitche It looks like either has CentOS a different uuid lib or the header is in a different place. Could you please find the uuid.h somewhere below /usr/include and see if it contains uuid_generate or uuid_create? |
d44a84f
to
0d64897
Compare
@mmitche I've installed a brand new installation of CentOS7.1 into a VM on my machine and it contained the /usr/include/uuid/uuid.h header like I have on Ubuntu. |
Here's what got installed: Installed Packages |
Well..hmm....libuuid-devel is different apparently. |
@dotnet-bot test this please |
Should be good now |
(assuming I got it in time) |
We were generating GUIDs on Unix as random numbers. But that is not correct since GUIDs have defined structure with bits having specific meanings. For example, there are four bits that represent the type of the GUID, which means whether the guid is randomly generated, name based on SHA1, time based etc. This change changes the CoCreateGuid to use the uuid_generate function from the uuid library that should generate well-formed GUIDs on Linux and OSX and the uuid_create on FreeBSD.
0d64897
to
dbd046b
Compare
Implement CoCreateGuid using uuid_generate on Unix
Implement CoCreateGuid using uuid_generate on Unix Commit migrated from dotnet/coreclr@eefbaaa
We were generating GUIDs on Unix as random numbers. But that is not
correct since GUIDs have defined structure with bits having specific
meanings. For example, there are four bits that represent the type of
the GUID, which means whether the guid is randomly generated,
name based on SHA1, time based etc.
This change changes the CoCreateGuid to use the uuid_generate function
from the uuid library that should generate well-formed GUIDs.