On 25 Aug 2012, at 23:16, [email protected] wrote:

>> Webrev:
>> http://jurassic.us.oracle.com/~ebeasant/webrevs/7183731-v2/
> 
> components/mlocate/Makefile
> 
>       Line 42 - Just to verify, this setting of localstatedir will
>       result in the local data ending up in /var/cache/mlocate and
>       not just /var/cache?
> 

Thats correct - it will end up in /var/cache/mlocate (double checked !)

> components/mlocate/mlocate.p5m
> 
>       Line 27 - I'd suggest a richer pkg.description. When I updated
>       many of the ones in ON, I ended up looking at the initial
>       DESCRIPTION field in the man page for inspiration. So for
>       mlocate(1), that might result in something like
> 
>       set name=pkg.description \
>           value="locate(1) reads one or more databases prepared by 
> updatedb(1M) and writes file names matching at least one of user specified 
> patterns to standard output, one per line."
> 
>       (An aside, although the upstream name of the project and the
>       package name are "mlocate", do any distributions actually
>       deliver a binary called "mlocate"?)

A good point. I'll fatten up the description.

> 
>       Lines 38-44 - Is there a reason to put the binaries under
>       /usr/bin/$(MACH64) rather than just /usr/bin?

I guess I should have asked on that one - They're compiled 64 bit, I was 
originally intending to follow the convention of delivering 32 and 64 bit 
binaries,
but with the later intention to ship only 64 bit, I see no reason they need to 
be in /usr/bin/mach64. For simplicity, I can move them across.

> 
>       Lines 41, 44 - Just verifying that other distributions put
>       updatedb in /usr/bin? I would have expected it to be under
>       /usr/sbin or perhaps /usr/libexec on Linux.
> 
> components/mlocate/patches/mlocate-0.25-getmnt.patch

I've seen both, but /usr/sbin is far more sensible for an executable that is 
designed only for root.

> 
>       Lines 18-19 - Is there a reason you're not changing
>       PATH_MOUNTED to just MNTTAB (the latter is defined in
>       <sys/mnttab.h>?)
> 
> components/mlocate/updatedb.conf

That should definitely be MNTAB.

> 
>       Where did this particular file come from? It seems to not be
>       from the distribution but it also includes some things that
>       don't apply to Solaris such as AFS references and /usr/tmp (the
>       latter is already covered by /var/tmp being part of
>       PRUNEPATHS.)

Its actually from the old slocate package - I was aiming to avoid changing 
behaviors, so preserved the original installed settings.
As it happens, it appears this needs the trash taking out...

> 
> Finally, what's the story with file/slocate? Will the Desktop
> consolidation be doing a coordinated delivery of that package since it
> seems both packages cannot coexist at the same time.

I have submitted a patch to Ada in Desktop that removes the slocate build and 
delivers an obsoleting manifest - that will be coordinated with the Userland 
delivery of mlocate.


Cheers,
Edwin




_______________________________________________
userland-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/userland-discuss

Reply via email to