Thread: contrib/xml2 function name change and minor bitrot fix
Hi, I've done a bit of testing now and it seems that both sets of XML functions will coexist in the same database (they both load OK and I can use functions from both without any apparent problem). I have changed the name of the new parse function to xml_valid and fixed a reference to SortMem which meant that the code as supplied would work against 7.3 and 7.4 but wouldn't work in CVS. Oops! Patch is attached. Regards John
Attachment
John Gray wrote: > I have changed the name of the new parse function to xml_valid and fixed > a reference to SortMem which meant that the code as supplied would work > against 7.3 and 7.4 but wouldn't work in CVS. Oops! Patch is attached. Looks good -- I'll apply the patch by this evening. -Neil
John Gray wrote: > I have changed the name of the new parse function to xml_valid and fixed > a reference to SortMem which meant that the code as supplied would work > against 7.3 and 7.4 but wouldn't work in CVS. Oops! Patch is attached. Patch applied. A few comments on the module: - can you include some licensing information? "Distributed under the same terms as PostgreSQL, Copyright ..." should be sufficient unless you'd prefer another license - some information on how to configure the location of the libxml2 headers / libs would be good, I think - the readme for both 'contrib/xml' and 'contrib/xml2' is called 'README.pgxml', yet neither module is actually named 'pgxml' -Neil
Neil Conway <neilc@samurai.com> writes: > A few comments on the module: > - can you include some licensing information? "Distributed under the > same terms as PostgreSQL, Copyright ..." should be sufficient unless > you'd prefer another license Actually, it had better be "same terms"; we are going to start being picky about that for contrib modules. (I have a TODO I've been ignoring to try to bring the existing contrib modules into line.) > - the readme for both 'contrib/xml' and 'contrib/xml2' is called > 'README.pgxml', yet neither module is actually named 'pgxml' The readmes *must* have different filenames so that they can be installed without conflict. It'd probably be best to ensure that they match the contrib module names. Are there any other installable files in the two modules that have conflicting names? regards, tom lane
On Sun, 2004-03-07 at 17:54 -0500, Tom Lane wrote: > Neil Conway <neilc@samurai.com> writes: > > A few comments on the module: > > > - can you include some licensing information? "Distributed under the > > same terms as PostgreSQL, Copyright ..." should be sufficient unless > > you'd prefer another license > > Actually, it had better be "same terms"; we are going to start being > picky about that for contrib modules. (I have a TODO I've been ignoring > to try to bring the existing contrib modules into line.) > Both modules are intended to be on the same terms as PostgreSQL. I can submit a patch to the README files to make that clear. Question: should the copyright be in the name of PGDG or me? (I cannot envisage doing anything with the module that the PostgreSQL licence would prevent, so I don't think I need to maintain copyright ownership of it) > > - the readme for both 'contrib/xml' and 'contrib/xml2' is called > > 'README.pgxml', yet neither module is actually named 'pgxml' > Oops - I didn't catch that in testing because it's not needed for the module to work.... I am loathe to change the installed .so name (pgxml_dom) for the old module (it might be in people's dump files, I suppose) so I suggest that renaming the xml/README to README.pgxml_dom will at least associate the README with the library and sql files. There are no other files that conflict (the so and sql files have different names). The new one is called pgxml (it produces pgxml.so and pgxml.sql) but if we want to be consistent, perhaps it should be xml2 instead? I'm really not hung up on the name -it's only a Makefile setting. In response to Neil's earlier point about configuration of include and library paths - that's on my personal TODO for the module. The relevant flags can be retrieved using xml2-config --cflags and xml2-config --libs (assuming a sane install of libxml2), but I haven't worked out how to cleanly set these in a module Makefile. I'll have another look at it. Regards John
Patch applied by Neil (thanks). Thanks. --------------------------------------------------------------------------- John Gray wrote: > Hi, > > I've done a bit of testing now and it seems that both sets of XML > functions will coexist in the same database (they both load OK and I can > use functions from both without any apparent problem). > > I have changed the name of the new parse function to xml_valid and fixed > a reference to SortMem which meant that the code as supplied would work > against 7.3 and 7.4 but wouldn't work in CVS. Oops! Patch is attached. > > Regards > > John > > [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 7: don't forget to increase your free space map settings -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Tom Lane wrote: > Neil Conway <neilc@samurai.com> writes: > > A few comments on the module: > > > - can you include some licensing information? "Distributed under the > > same terms as PostgreSQL, Copyright ..." should be sufficient unless > > you'd prefer another license > > Actually, it had better be "same terms"; we are going to start being > picky about that for contrib modules. (I have a TODO I've been ignoring > to try to bring the existing contrib modules into line.) I have updated both README's to say it has the same BSD copyright as PostgreSQL. > > - the readme for both 'contrib/xml' and 'contrib/xml2' is called > > 'README.pgxml', yet neither module is actually named 'pgxml' > > The readmes *must* have different filenames so that they can be > installed without conflict. It'd probably be best to ensure that they > match the contrib module names. Are there any other installable files > in the two modules that have conflicting names? I have renamed the README's to README.xml and README.xml2 to match /contrib directory names. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073