qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alan Conway" <acon...@redhat.com>
Subject Re: Review Request 35214: PROTON-902 Remove the dependency on uuid
Date Tue, 09 Jun 2015 13:54:17 GMT


> On June 8, 2015, 6:11 p.m., Kenneth Giusti wrote:
> > proton-c/bindings/python/setup.py, line 209
> > <https://reviews.apache.org/r/35214/diff/3/?file=980771#file980771line209>
> >
> >     We might be better off just checking for the existence of the proper header
files here, since the libuuid library may be installed without them.
> >     
> >     From looking at the platform.c file, we could simply map:
> >     
> >     if 'uuid/uuid.h' -> USE_UUID_GENERATE
> >     else if 'uuid.h' -> USE_UUID_CREATE ...
> >     
> >     the truely paranoid would check for both headers and the symbol in the library.
> 
> Alan Conway wrote:
>     If the proposed code is a valid UUID generator then why don't we just drop using
libuuid entirely? Lets just have one implementation to debug...
> 
> Kenneth Giusti wrote:
>     ".. and then the emperor realized he was naked ..."
>     
>     That's a very good point Alan - the uuid is used merely as a fallback name for the
messenger container if the user doesn't supply a name on creation.
>     
>     I'd vote for removing all the conditional stuff (except for the windows calls) and
use Flavio's version of pn_i_genuuid() in all cases.
> 
> Flavio Percoco wrote:
>     Honestly, my first - local - proposal was to remove everything and use the proposed
uuid implementation. However, I tried not to be so agressive and take one step at a time.
First propose the fallback and then see how it goes and remove the rest.
>     
>     
>     
>     That said, if the team is happy to just stick with the uuid4 generator proposed in
this patch, I'll happily update the diff.
> 
> Alan Conway wrote:
>     "Be agressive, Be-ee agressive" (You have to imagine the pom-poms)

Nitpick: if we drop libuuid this probably goes in uuid.c/uuid.h rather than platform.c.h and
is called pni_uuid. (I have no idea why the platform stuff uses `pn_i_` instead of `pni_`
like everything else but that's a different story...)


- Alan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35214/#review87053
-----------------------------------------------------------


On June 8, 2015, 9:55 p.m., Flavio Percoco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35214/
> -----------------------------------------------------------
> 
> (Updated June 8, 2015, 9:55 p.m.)
> 
> 
> Review request for qpid and Rafael Schloming.
> 
> 
> Repository: qpid-proton-git
> 
> 
> Description
> -------
> 
> Instead of relying on libuuid for uuid generation, let proton-c have a built-in uuid4
generator as a fallback when no uuid generator is present in the system.
>     
> Eventually, proton-c could simply rely on the built-in generator but lets give this some
time to mature and test.
> 
> 
> Diffs
> -----
> 
>   proton-c/CMakeLists.txt b534e86 
>   proton-c/bindings/python/setup.py 79168d2 
>   proton-c/src/platform.c 8f8ac5f 
> 
> Diff: https://reviews.apache.org/r/35214/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Flavio Percoco
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message