Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Implement CoCreateGuid using uuid_generate on Unix #1738

Merged
merged 1 commit into from
Oct 13, 2015

Conversation

janvorli
Copy link
Member

@janvorli janvorli commented Oct 9, 2015

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.

@janvorli
Copy link
Member Author

janvorli commented Oct 9, 2015

@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()
Copy link
Member

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?

@janvorli
Copy link
Member Author

janvorli commented Oct 9, 2015

Ah, we will need to install libuuid to our Linux / FreeBSD / CentOS CI machines.

@jkotas
Copy link
Member

jkotas commented Oct 9, 2015

LGTM

@jkotas
Copy link
Member

jkotas commented Oct 9, 2015

And also update "getting started" instructions...

@janvorli
Copy link
Member Author

janvorli commented Oct 9, 2015

@jkotas It was installed on my OSX, but not on my Ubuntu and obviously from the CI neither on CentOS and FreeBSD.

@janvorli
Copy link
Member Author

janvorli commented Oct 9, 2015

It fixes #1602

@janvorli
Copy link
Member Author

janvorli commented Oct 9, 2015

@ellismg Could you please install uuid-dev package on our CI FreeBSD / CentOS machines?

@mmitche
Copy link
Member

mmitche commented Oct 9, 2015

@janvorli I'll look into it

@janvorli
Copy link
Member Author

janvorli commented Oct 9, 2015

@mmitche thanks!

@mmitche
Copy link
Member

mmitche commented Oct 9, 2015

@janvorli Installed for centos and SuSE. FreeBSD does not have an obvious package for this. Can you find out what it is?

@mmitche
Copy link
Member

mmitche commented Oct 9, 2015

@janvorli Also did you update the build documentation and cmake detection to indicate that this needs to be installed.

@stephentoub
Copy link
Member

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.

@janvorli
Copy link
Member Author

janvorli commented Oct 9, 2015

@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.

@akoeplinger
Copy link
Member

Looks like freebsd already has a similar function built-in: http://www.freebsd.org/cgi/man.cgi?query=uuid_create

@janvorli
Copy link
Member Author

janvorli commented Oct 9, 2015

@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

@adityamandaleeka
Copy link
Member

LGTM modulo others' comments.

@janvorli
Copy link
Member Author

janvorli commented Oct 9, 2015

@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?

@janvorli
Copy link
Member Author

@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.
So I guess that when you've installed uuid for CentOS, you have installed something else, since from the error log on the CI machines, I can see that it has not found the /usr/include/uuid/uuid.h.
I've dumped all of the installed packages on my CentOS that contain uuid in the name using rpm -qa | grep uuid command. I can see that I have these packages installed:
libuuid-devel-2.32.2-21.el7.x86_64
libuuid-2.32.2-21.el7.x86_64

@mmitche
Copy link
Member

mmitche commented Oct 12, 2015

Here's what got installed: Installed Packages
uuid-devel.x86_64 1.6.2-26.el7 @base

@mmitche
Copy link
Member

mmitche commented Oct 12, 2015

Well..hmm....libuuid-devel is different apparently.

@janvorli
Copy link
Member Author

@dotnet-bot test this please

@mmitche
Copy link
Member

mmitche commented Oct 12, 2015

Should be good now

@mmitche
Copy link
Member

mmitche commented Oct 12, 2015

(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.
janvorli added a commit that referenced this pull request Oct 13, 2015
Implement CoCreateGuid using uuid_generate on Unix
@janvorli janvorli merged commit eefbaaa into dotnet:master Oct 13, 2015
@janvorli janvorli deleted the fix-cocreate-guid branch October 13, 2015 14:57
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Implement CoCreateGuid using uuid_generate on Unix

Commit migrated from dotnet/coreclr@eefbaaa
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants