upnp-sdk-dev

[upnp-sdk-dev] release 1.3.1, rpm, ixmlPrintDocument

[John D. <jd...@re...>]

First of all Remi, thank you for your work and releasing version 1.3.1.
I've downloaded it and did some testing. I also reviewed the work you
did to convert building with automake, libtool, pkgconfig etc. It looks
excellent and is a huge improvement!

Eric, I was not aware you had done an RPM for extras, thank you. Back in
January, which is when I looked, it was not there, it looks like you
added it in the interim. Now that Remi has released 1.3.1 we'll need to
update the rpm to use it. Because of the work Remi did supporting the
standard build/packaging tools the spec file will need some rework.
Fortunately it should get simpler :-) Do you want to do this or would
you like me to? I ask because I'm not sure how familiar you are with
spec files which utilize this build/install paradigm. 

On the issue of installation, I see that the libraries for threadutil
and ixml are installed in the system in parallel with upnp. I wonder if
in the long run that's a good idea. These two libraries I imagine are
only ever going to be used by upnp, there are probably better
independent libraries for threads and xml and we probably don't want
folks independently using and linking against these two libraries in the
belief they are supported interfaces. At some point it might be
worthwhile to link these libraries into libupnp and hide their symbols.

Remi, I do have a fix which needs to be folded in, I wish I had known
you were about to prepare a release so we didn't have to respin again.
Here is the issue:

The problem is only evident with devices and only if the device uses the
UpnpRegisterRootDevice2 entry point. It appears as if the SDK authors
realized that UpnpRegisterRootDevice was simplistic and inflexible and
added UpnpRegisterRootDevice2 later in development. Perhaps at some
point these two functions could be collapsed into one.

With UpnpRegisterRootDevice2 it is possible to generate the device
description document dynamically and have the internal web server serve
an in-memory copy of the document as opposed to a file. This is actually
a very useful technique and ixmlPrintDocument is the means to get an
in-memory copy of the XML description document given a DOM tree. Just
one problem. ixmlPrintDocument does not produce a valid XML document
because it leaves out the XML document prolog. I discovered this when
testing with control points based on Microsoft Windows UPnP framework.
The Microsoft implementation will silently ignore any UPnP devices whose
description document is malformed and that includes omitting the XML
prolog.

In the UPnP SDK ixmlPrintDocument had been #define'd to be ixmlPrintNode
which prints XML elements. The fix I believe is to have to independent
functions, ixmlPrintDocument and ixmlPrintNode (same for the toString
versions) where the Document functions include the prolog and the Node
functions omit it. The attached patch (which I'll also post on the SF
site) creates the new Document functions which prepends the prolog and
replaces every instance of ixmlPrintDocument with ixmlPrintNode because
the two are now no longer interchangeable (except in configure_urlbase()
in the file urlconfig.c, because here you do want a document).

I imagine the reason neither of you ran into this problem is because
you've been doing control point code and not device code.

This is an interface change and as such the libtool versioning will have
to be bumped. BTW, the patch is against the 1.3.1 version.

-- 
John Dennis <jd...@re...>
Red Hat Inc.

[upnp-sdk-dev] Re: release 1.3.1, rpm, ixmlPrintDocument

[John D. <jd...@re...>]

On Tue, 2006-03-07 at 22:42 +0100, Eric Tanguy wrote:
> John, the rpms for release 1.3.1 are already built for FC-3, FC-4 and
> devel as well as new version of ushare using it. If you want to own the
> package or submit a patch, don't hesitate! The only thing is that this
> lib have to be compliant with ushare (the package which need it and
> that's why i did rpm for libupnp). Another question : redhat will
> include it in core or prefer to let it in extras ?

Excellent Eric! You're one step ahead of me again. The rpm looks
fabulous. The only thing that appears to be missing is the documentation
and sample programs. These should probably go
in /usr/share/doc/libupnp-*

I don't think auto generated doc is terribly important, it would be nice
but not critical, but I do think including UPnP_Programming_Guide.pdf is
essential. 

Off the top of my head without any testing (I feel like I'm going to
regret my caviler suggestions which weren't tested :-) Something like
this should do the trick (Remi: since these seem to be tweaks to the
configure stuff perhaps it should be added at your end, or you may
prefer a different approach):

% in configure.ac add:

AC_ARG_WITH([docdir], AC_HELP_STRING([--with-docdir=DIR],
			             [where documentation is installed]),
            [docdir="$with_docdir"],
            [docdir="${datadir}/doc/${PACKAGE_NAME}-${PACKAGE_VERSION}"])

AC_SUBST(docdir)

% in upnp/doc/Makefile.am add:

docdir = @DOCDIR@
doc_DATA = UPnP_Programming_Guide.pdf

% in upnp/Makefile.am add:

docdir = @DOCDIR@
examplesdir = $(docdir)/examples
examples_DATA =  $(upnp_tv_ctrlpt_SOURCES) $(upnp_tv_device_SOURCES)

(Note: examples_DATA probably also needs a Makefile utilizing
pkg-config)

Once all this is done Eric then I think all that needs to be done is to
just add the doc files to the %files section in the spec file.

If either of you prefers I do the work I'm happy to.

With respect to moving libupnp from extras to core that's a decision
which will be out there in the future a bit. At the moment our use of
this technology is experimental and not yet fundamental to the core
product (all of which is subject to change :-)


-- 
John Dennis <jd...@re...>
Red Hat Inc.

[upnp-sdk-dev] Re: release 1.3.1, rpm, ixmlPrintDocument

[Eric T. <eri...@un...>]

Le mardi 07 mars 2006 =C3=A0 18:17 -0500, John Dennis a =C3=A9crit :
> On Tue, 2006-03-07 at 22:42 +0100, Eric Tanguy wrote:
> > John, the rpms for release 1.3.1 are already built for FC-3, FC-4 and
> > devel as well as new version of ushare using it. If you want to own t=
he
> > package or submit a patch, don't hesitate! The only thing is that thi=
s
> > lib have to be compliant with ushare (the package which need it and
> > that's why i did rpm for libupnp). Another question : redhat will
> > include it in core or prefer to let it in extras ?
>=20
> Excellent Eric! You're one step ahead of me again. The rpm looks
> fabulous. The only thing that appears to be missing is the documentatio=
n
> and sample programs. These should probably go
> in /usr/share/doc/libupnp-*
>=20
> I don't think auto generated doc is terribly important, it would be nic=
e
> but not critical, but I do think including UPnP_Programming_Guide.pdf i=
s
> essential.=20
>=20
> Off the top of my head without any testing (I feel like I'm going to
> regret my caviler suggestions which weren't tested :-) Something like
> this should do the trick (Remi: since these seem to be tweaks to the
> configure stuff perhaps it should be added at your end, or you may
> prefer a different approach):
>=20
> % in configure.ac add:
>=20
> AC_ARG_WITH([docdir], AC_HELP_STRING([--with-docdir=3DDIR],
> 			             [where documentation is installed]),
>             [docdir=3D"$with_docdir"],
>             [docdir=3D"${datadir}/doc/${PACKAGE_NAME}-${PACKAGE_VERSION=
}"])
>=20
> AC_SUBST(docdir)
>=20
> % in upnp/doc/Makefile.am add:
>=20
> docdir =3D @DOCDIR@
> doc_DATA =3D UPnP_Programming_Guide.pdf
>=20
> % in upnp/Makefile.am add:
>=20
> docdir =3D @DOCDIR@
> examplesdir =3D $(docdir)/examples
> examples_DATA =3D  $(upnp_tv_ctrlpt_SOURCES) $(upnp_tv_device_SOURCES)
>=20
> (Note: examples_DATA probably also needs a Makefile utilizing
> pkg-config)
>=20
> Once all this is done Eric then I think all that needs to be done is to
> just add the doc files to the %files section in the spec file.
>=20
> If either of you prefers I do the work I'm happy to.
>=20
> With respect to moving libupnp from extras to core that's a decision
> which will be out there in the future a bit. At the moment our use of
> this technology is experimental and not yet fundamental to the core
> product (all of which is subject to change :-)
>=20
Ok but i prefer to wait this will be included directly in the tarball. Wh=
at do you think about this R=C3=A9mi ?
Cheers, Eric

[upnp-sdk-dev] Re: release 1.3.1, rpm, ixmlPrintDocument

[Eric T. <eri...@un...>]

Le mardi 07 mars 2006 =C3=A0 15:46 -0500, John Dennis a =C3=A9crit :
> First of all Remi, thank you for your work and releasing version 1.3.1.
> I've downloaded it and did some testing. I also reviewed the work you
> did to convert building with automake, libtool, pkgconfig etc. It looks
> excellent and is a huge improvement!
>=20
> Eric, I was not aware you had done an RPM for extras, thank you. Back i=
n
> January, which is when I looked, it was not there, it looks like you
> added it in the interim. Now that Remi has released 1.3.1 we'll need to
> update the rpm to use it. Because of the work Remi did supporting the
> standard build/packaging tools the spec file will need some rework.
> Fortunately it should get simpler :-) Do you want to do this or would
> you like me to? I ask because I'm not sure how familiar you are with
> spec files which utilize this build/install paradigm.=20

John, the rpms for release 1.3.1 are already built for FC-3, FC-4 and
devel as well as new version of ushare using it. If you want to own the
package or submit a patch, don't hesitate! The only thing is that this
lib have to be compliant with ushare (the package which need it and
that's why i did rpm for libupnp). Another question : redhat will
include it in core or prefer to let it in extras ?

>=20
> On the issue of installation, I see that the libraries for threadutil
> and ixml are installed in the system in parallel with upnp. I wonder if
> in the long run that's a good idea. These two libraries I imagine are
> only ever going to be used by upnp, there are probably better
> independent libraries for threads and xml and we probably don't want
> folks independently using and linking against these two libraries in th=
e
> belief they are supported interfaces. At some point it might be
> worthwhile to link these libraries into libupnp and hide their symbols.
>=20
> Remi, I do have a fix which needs to be folded in, I wish I had known
> you were about to prepare a release so we didn't have to respin again.
> Here is the issue:
>=20
> The problem is only evident with devices and only if the device uses th=
e
> UpnpRegisterRootDevice2 entry point. It appears as if the SDK authors
> realized that UpnpRegisterRootDevice was simplistic and inflexible and
> added UpnpRegisterRootDevice2 later in development. Perhaps at some
> point these two functions could be collapsed into one.
>=20
> With UpnpRegisterRootDevice2 it is possible to generate the device
> description document dynamically and have the internal web server serve
> an in-memory copy of the document as opposed to a file. This is actuall=
y
> a very useful technique and ixmlPrintDocument is the means to get an
> in-memory copy of the XML description document given a DOM tree. Just
> one problem. ixmlPrintDocument does not produce a valid XML document
> because it leaves out the XML document prolog. I discovered this when
> testing with control points based on Microsoft Windows UPnP framework.
> The Microsoft implementation will silently ignore any UPnP devices whos=
e
> description document is malformed and that includes omitting the XML
> prolog.
>=20
> In the UPnP SDK ixmlPrintDocument had been #define'd to be ixmlPrintNod=
e
> which prints XML elements. The fix I believe is to have to independent
> functions, ixmlPrintDocument and ixmlPrintNode (same for the toString
> versions) where the Document functions include the prolog and the Node
> functions omit it. The attached patch (which I'll also post on the SF
> site) creates the new Document functions which prepends the prolog and
> replaces every instance of ixmlPrintDocument with ixmlPrintNode because
> the two are now no longer interchangeable (except in configure_urlbase(=
)
> in the file urlconfig.c, because here you do want a document).
>=20
> I imagine the reason neither of you ran into this problem is because
> you've been doing control point code and not device code.
>=20
> This is an interface change and as such the libtool versioning will hav=
e
> to be bumped. BTW, the patch is against the 1.3.1 version.
>=20

Re: [upnp-sdk-dev] release 1.3.1, rpm, ixmlPrintDocument

[<tur...@ea...>]

John,

> First of all Remi, thank you for your work and releasing version 1.=
3.1.
> I've downloaded it and did some testing. I also reviewed the work y=
ou
> did to convert building with automake, libtool, pkgconfig etc. It l=
ooks
> excellent and is a huge improvement!
>  =20
thanks !

> On the issue of installation, I see that the libraries for threadut=
il
> and ixml are installed in the system in parallel with upnp. I wonde=
r if
> in the long run that's a good idea. (...)
>  At some point it might be
> worthwhile to link these libraries into libupnp and hide their symb=
ols.
>  =20
except that ixml has its own documented interface (see the libupnp-do=
c=20
package), and normally is used directly by programs interfacing with=
=20
libupnp.I think it would not be practical to hide it. May be merge it=
,=20
but that would break programs linking with the 3 libraries.

>
> In the UPnP SDK ixmlPrintDocument had been #define'd to be ixmlPrin=
tNode
> which prints XML elements. The fix I believe is to have to independ=
ent
> functions, ixmlPrintDocument and ixmlPrintNode (same for the toStri=
ng
> versions) where the Document functions include the prolog and the N=
ode
> functions omit it. The attached patch (which I'll also post on the =
SF
> site) creates the new Document functions which prepends the prolog =
and
> replaces every instance of ixmlPrintDocument with ixmlPrintNode bec=
ause
> the two are now no longer interchangeable (except in configure_urlb=
ase()
> in the file urlconfig.c, because here you do want a document).
>  =20
you are right, that's the way ixmlPrintDocument should have been=20
implemented from the beginning. However, I have seen programs using t=
his=20
existing interface as it is, and manually adding the prolog.
This fix would silently break this interface. To keep backward=20
compatibility : wouldn't it be preferable to add a new function (e.g.=
=20
ixmlPrintDocumentWithProlog or something like that), and document the=
=20
deficiency in the old function?

Also : another user suggests to implement a check in the libupnp, in=
=20
order to add the prolog in the device description document if it is=
=20
missing :
   =20
http://sourceforge.net/tracker/index.php?func=3Ddetail&aid=3D931724&g=
roup_id=3D7189&atid=3D107189
I don't know if this other patch is correct or not, but we could=20
implement this idea too, in addition the the ixml fix.

What's your view ?

R=E9mi

Re: [upnp-sdk-dev] release 1.3.1, rpm, ixmlPrintDocument

[John D. <jd...@re...>]

On Thu, 2006-03-09 at 23:02 +0100, R=C3=A9mi wrote:
> John,
>=20
> > First of all Remi, thank you for your work and releasing version 1.3.=
1.
> > I've downloaded it and did some testing. I also reviewed the work you
> > did to convert building with automake, libtool, pkgconfig etc. It loo=
ks
> > excellent and is a huge improvement!
> >  =20
> thanks !
>=20
> > On the issue of installation, I see that the libraries for threadutil
> > and ixml are installed in the system in parallel with upnp. I wonder =
if
> > in the long run that's a good idea. (...)
> >  At some point it might be
> > worthwhile to link these libraries into libupnp and hide their symbol=
s.
> >  =20
> except that ixml has its own documented interface (see the libupnp-doc=20
> package), and normally is used directly by programs interfacing with=20
> libupnp.I think it would not be practical to hide it. May be merge it,=20
> but that would break programs linking with the 3 libraries.

Good points, sounds fine to me.

>=20
> >
> > In the UPnP SDK ixmlPrintDocument had been #define'd to be ixmlPrintN=
ode
> > which prints XML elements. The fix I believe is to have to independen=
t
> > functions, ixmlPrintDocument and ixmlPrintNode (same for the toString
> > versions) where the Document functions include the prolog and the Nod=
e
> > functions omit it. The attached patch (which I'll also post on the SF
> > site) creates the new Document functions which prepends the prolog an=
d
> > replaces every instance of ixmlPrintDocument with ixmlPrintNode becau=
se
> > the two are now no longer interchangeable (except in configure_urlbas=
e()
> > in the file urlconfig.c, because here you do want a document).
> >  =20
> you are right, that's the way ixmlPrintDocument should have been=20
> implemented from the beginning. However, I have seen programs using thi=
s=20
> existing interface as it is, and manually adding the prolog.
> This fix would silently break this interface. To keep backward=20
> compatibility : wouldn't it be preferable to add a new function (e.g.=20
> ixmlPrintDocumentWithProlog or something like that), and document the=20
> deficiency in the old function?
>=20
> Also : another user suggests to implement a check in the libupnp, in=20
> order to add the prolog in the device description document if it is=20
> missing :
>    =20
> http://sourceforge.net/tracker/index.php?func=3Ddetail&aid=3D931724&gro=
up_id=3D7189&atid=3D107189
> I don't know if this other patch is correct or not, but we could=20
> implement this idea too, in addition the the ixml fix.
>=20
> What's your view ?

I'm not much in favor of codifying broken behavior or per user
implementation of hacks. I would much prefer to see the library work
cleanly out of the box. I don't think the libupnp SDK is in such wide
spread use yet that fear of breaking previous usage justifies retaining
buggy behavior and thus forcing new users of the library to learn the
work-around, surely a time waste for them (as it was for me). I believe
a new release could draw attention via release notes and/or a message on
the download page about the change in behavior and thus accommodate the
earlier adopters.

Also, I've cc'ed Jason Vas Dias on this email. Jason sits near me here
at Red Hat and it looks like he is going to pick up ownership of this
package. Jason tells me there has been some discussion of moving libupnp
into core from extras, but I'll leave that discussion to Eric and Jason
to pursue.

Also, as a side note, I've implemented a few upnp programs using the
SDK. I started with the TV samples whose coding style I found a bit
awkward and in the process rewrote and reformatted much of the samples.
It also includes new utilities to set search criteria, handle command
line input, etc. If anyone is interested I'm happy to donate the code.
--=20
John Dennis <jd...@re...>
Red Hat Inc.

[upnp-sdk-dev] Re: release 1.3.1, rpm, ixmlPrintDocument

[<r3...@us...>]

Eric Tanguy wrote:
>  What do you think about this R=C3=A9mi ?
>  =20
that it'll have to wait until after 20th March, when I am back :-)


Regards,
    R=C3=A9mi

[upnp-sdk-dev] Re: release 1.3.1, rpm, ixmlPrintDocument

[<tur...@ea...>]

Hello,

> The only thing that appears to be missing is the documentation
> and sample programs. These should probably go
> in /usr/share/doc/libupnp-*
>
> I don't think auto generated doc is terribly important, it would be nice
> but not critical, but I do think including UPnP_Programming_Guide.pdf is
> essential.
>

I have added the documentation stuff (--with-docdir as you suggested , but=
=20
also --without-docdir works if you need to skip doc install).
Most of it is in the CVS (I can't add the rest now because it is down).

I also put back the UPnP_Programming_Guide.pdf from the libupnp-doc-1.2.1=20
package, because the one in the CVS seemed corrupted.

> (Note: examples_DATA probably also needs a Makefile utilizing
> pkg-config)

not done yet ...


=2D-=20
R=E9mi