[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#663916: New phonetisaurus package available



Hi!
	Thank you for your review.

Il 13/10/2012 00:02, Jakub Wilk ha scritto:
> * Giulio Paci <giuliopaci@gmail.com>, 2012-10-11, 03:52:
>>>> git://git.debian.org/git/collab-maint/phonetisaurus.git.

>>> Last but not least, why do you need to recover this file? It looks like it shouldn't have been included in the upstream tarball in the first place.
>> Just because it was in the original tarball and I want that a "debian/rules clean" results in the same content of the original tarball.
> 
> Now I seem to recall that you told me that your workflow depends on such restoration. Sorry for the noise.

No problem.

>>> Oh, and Google sparse hash implemented is already packaged in Debian. Please build-depend on libsparsehash-dev and make sure that the system-wide copy is used, not the
>>> bundled one.
>>>
>>> Packaging the UTF-8 library (currently in src/3rdparty/utf8/) might be also worth considering, in order to comply with Policy §4.13.
>>
>> As we are not talking about shared libraries, but about a few headers files, I do not understand the benefits of doing so.
> 
> The main benefit is the same: you can fix bugs in one place, instead of doing it N places (where N is usually >> 1).

Yes, I understand the goal, but I am worried that using external source code would make it harder to replicate possible issues (I will need the exact version of the
libsparsehash-dev binary package that was used to compile the library).
However this is probably true for many other C++ libraries. It is just more evident here, where the libraries are just simple header files.

>> I see only disadvantages:
>> 1) using system wide files will prevent me to easily know the source code used to compile the phonetisaurus debian package;
> (Sometimes we need to keep exact source for license reasons; that's what Built-Using field is for.)

How can I set Built-Using field? Should I set it by hand? Is it possible to set it automatically?

>> 2) fixes in sparsehash will not be available to phonetisaurus unless phonetisaurus is recompiled;
> 
> That's not worse than status quo. Also: binNMUs are cheap.

Maybe I am missing something in the upload process (well, I am missing almost everything to be honest).
Can you point some reference that will help me understanding what will happen when new releases of libsparsehash-dev will be released (and phonetisaurus will need
recompilation)?

>> 3) I will need to maintain patches to use the system-wide copy;

Created, applied and forwarded a patch for this.

>> 4) an additional dependency is introduced;
> 
> Again, convince upstream to drop the embedded copy, and these problems will go away. :)

Additional dependency introduced. :-) If the patch will be accepted upstream, the patch will let upstream to compile embedded copy of the libraries and us to compile using
the system-wide dependency.

>> 5) If I will package UTF-8 I will need to invest time maintaining a new package that I do not care about and that contains just 4 headers files.
> 
> I checked that there are at least 14 source packages in Debian that bundle UTF8-CPP:
> 
> drizzle fife gdcm gource librime libvoikko love md5deep megaglest mkvtoolnix paraview ruby-passenger supertuxkart vtk
> 
> Hopefully one of their maintainers would be interested in packaging it separately. Maybe file an RFP, CCing them all?

If you think it is useful, I will do this. However I would like to understand how I am supposed to deal with this kind of libraries before doing this.
With the patch above, it will be very easy to use the system-wide utfcpp library once it is packaged.

>> Do you think that policy §4.13 apply in this case? I seems to me that it is more related to shared libraries than static ones and headers.
> 
> No, §4.13 it's not only about shared library. It does apply here.

Ok, using libsparsehash-dev then.

>> Moreover I think that the last part of the following sentence applies:
>> "Debian packages should not make use of these convenience copies unless the included package is explicitly intended to be used in this way".
> 
> Do you have any evidence that this is the case (e.g. links to upstream documentation saying this is the preferred way of using the libraries)?

No, I have no evidence about it, but the documentation is scarce in this sense. Both libraries report something like:
"You just need to put the .h files somewhere your compiler can see this."
I just thought that sparsehash and utf8 are similar enough to gnulib that people would use them in the same way.

> FWIW, I'm personally not fond of this exception to §4.13. I think we would be better without it. Fixing autotools bugs is definitely not fun.

I see your point of view.
But not being able to understand if autotools files are not working on others' systems because of different version of tools or because problems in the environment is also
not fun. :-)

> Now the promised review of d/copyright:
> 
>> Files: *
>> Copyright: 2011-2012, Josef Robert Novak
>> License: GPL-3+
> 
> As far as I can see, the code has been relicensed to 2-clause BSD.

Right, fixed.

>> Files: debian/phonetisaurus-g2p.1
> 
> No such file or directory.

Also fixed.

>> Files: FstPathFinder.cpp FstPathFinder.hpp
>> Copyright: Chris Taylor
>> 2011, Josef Novak
>> License: Apache-2.0 and GPL-3+
> 
> These now contain 2-clause BSD headers, though README.md says the "code was licensed under the Apache license". Could you clarify this with upstream?

This code is partially covered by Apache 2.0 license and partially by the BSD-2-clause license.
Upstream tried to contact Chris Taylor in the past, but he was not able to talk to him.
He is thinking about rewriting these files from scratch.

> Copyright/license information for src/3rdparty/google/ is missing.

Added.

Bests,
	Giulio.


Reply to: