Hi Brock,
Thanks for taking a look.
On Tue, 2010-04-27 at 14:24 -0700, Brock Pytlik wrote:
> The integration with the existing structure looks good to me in general.
Cool.
> The main bit that bothers me a bit is the pkg_attrs. I think it's a
> little strange to have pkgdep able to add arbitrary set actions to the
> manifest, can you elaborate a bit why these need to be in a set action?
It felt like a natural place to put it - automatically generated
metadata that applies to the package as a whole.
An alternative would be to add more attributes to the file action for
the SMF manifest itself, describing the SMF FMRIs that the file
delivers, but I was under the impression there was a consensus that
adding set actions was preferable to adding more attributes to 'file'?
In terms of doing it in pkgdepend vs. elsewhere, we're already examining
the package contents to produce dependency information at this point,
doing some analysis on every file in the package. We expect developers
to go through the pkgdepend phase prior to publication anyway - we could
add a third phase of automatic file-content sniffing, but that'd be more
work for the developer imho.
The alternative would be to add this auto-generated information during
the package publishing step, but that'd slow down package publishing on
subsequent attempts to publish tweaked versions of a package where the
developer is happy with the dependencies we've already worked out.
> If we do need to go down this road though, then I think it's probably
> worth making it so that more than one set action can be added, instead
> of only adding to the existing set action.
I agree - I've made that change in the latest webrev, in src/pkgdep.py
> Other than that, I didn't see anything else that needed changing.
I've got some sample text for pkgdepend(1) that tries to explain the
methods we use for dependency analysis. I'm not happy with the text for
python dependencies, but wasn't sure how else to summarise the
conditions under which we gather python dependency information (short of
reproducing the excellent block comment in
src/modules/flavor/python.py :-)
I'll reproduce that my suggested man page additions here for
convenience:
---
Several aspects of delivered files are used as sources of
dependency information:
ELF ELF headers in delivered files are analyzed for
dependency information, with the -k and -D options
modifying how that information is obtained. For
more details on ELF dependencies, please see ldd(1)
and the Solaris Linker and Libraries Guide.
Scripts Shell scripts that contain '#!' lines referencing
an interpreter will result in a dependency on the
package that delivers that interpreter.
Python Python scripts are analyzed as above. In addition,
any imports the script declares may also serve as a
source of dependency information.
Hard links Hard links in manifests will result in a
dependency on the package that delivers the link
target.
SMF SMF service manifests delivered that include
"require_all" dependencies will result in a
dependency on the package that delivers the SMF
manifest that provides that FMRI.
---
The updated webrev is at
http://cr.opensolaris.org/~timf/pkgdepend-smf
Thanks again for the comments, they're much appreciated.
cheers,
tim
> On 04/26/10 02:00 PM, Tim Foster wrote:
> > Hi all,
> >
> > Sorry this is so late - I had been hoping to get this request for a code
> > review sent out last week.
> >
> > I've got a code review at
> > http://cr.opensolaris.org/~timf/pkgdepend-smf
> >
> > that I'd really appreciate a review of please.
> >
> > This change looks for dependencies declared in SMF manifests delivered
> > by a package during the 'pkgdepend generate' phase of publication. It
> > resolves those SMF FMRIs to the SMF manifests that deliver them.
> >
> > As a by-product, it sets an attribute on those packages that deliver SMF
> > manifests, 'smf.fmri' so you can do a pkg search using SMF FMRIs.
> >
> > t...@tcx2200m2-02[1278] pkg search -l ':::svc\:/network/echo\:dgram'
> > INDEX ACTION VALUE PACKAGE
> > smf.fmri set svc:/network/echo:dgram
> > pkg:/service/network/[email protected]
> >
> >
> > It works by building internal dictionaries of all known SMF FMRIs at
> > publication time, and which files deliver those FMRIs. It uses those
> > dictionaries to lookup the file dependencies and generate depend
> > actions.
> >
> > The 'pkgdepend resolve' phase is untouched by this changeset, and as
> > usual, resolves file names to package names.
> >
> > I've included changes to importer.py to compute the same dependencies,
> > though this needs either the publishing build machine to include SMF
> > packages to satisfy those dependencies at publication time (unless
> > they're in the package for publication).
> >
> > importer.py will try to seed those dictionaries using a search for SMF
> > manifests against the repository we're publishing to, pulling those
> > manifests directly from the repository. For now, that requires a http
> > depot - it currently reports a warning and continues if we're publishing
> > to a file-url. (I can change that once we're able to search against
> > file-based repositories).
> >
> > I've done full ON builds on a test machine and have installed both the
> > built bits, as well as some ipkg zones using those published bits. I've
> > also added unit tests to exercise these changes.
> >
> > Defect 15305 has an attachment that shows pkgdiff output of the resolved
> > manifests on an onnv-gate build before and after this changeset was
> > applied on the build machine.
> >
> > I'll be available to work on feedback anytime between today and Friday,
> > but will be out of the office from Friday 30th, back on May 10th
> > assuming we survive the flight and initial few days in NZ :-)
> >
> >
> > cheers,
> > tim
> >
> >
> > _______________________________________________
> > pkg-discuss mailing list
> > [email protected]
> > http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
> >
>
> _______________________________________________
> pkg-discuss mailing list
> [email protected]
> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
