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

Re: Bug#762228: RFS: ufoai-music review



Control: tags -1 moreinfo

On Sat, 2014-09-20 at 01:22 +0200, Markus Koschany wrote:
> Hi Tobias,
> 
> thanks for taking your time for this.
> 
> On 19.09.2014 23:51, Tobias Frost wrote:
> > Control: -1 moreoinfo
> > 
> > Hi Markus,
> > 
> > so, let's start with -music
> 
> First of all I'd like to suggest that you start with the ufoai source
> package first because it contains the ufoai_copyright.py script and
> other information that are useful to understand the packaging of
> UFO:AI's data packages.

My reasoning is, that because of every data package has its own
orig.tar, they need to be crafted in a way to so that they will
be -- individually looked at -- reach Debian quality requirements. 

> > -> d/copyright contains *many* files not in this package.
> > Please clean up the file. (Also, please use wildcards;
> > this makes it far easier to review)
> 
> The debian/copyright file is identical for ufoai-data, ufoai-music and
> ufoai-maps. I did this on purpose because upstream does not distinguish
> between those files. In fact they maintain everything in one Git
> repository and the LICENSE file contains all copyright information for
> the game data. Thus I decided to use a script to parse all license
> information and then I generated a machine-readable debian/copyright
> file out of them.

> This makes it far easier to review the packages IMO because you only
> have to check and run the script on LICENSES. It also comes with the
> advantage that all files are machine-readable now. Thus wildcards,
> except for the Files: * paragraph, aren't necessary and the whole
> copyright information are more precise.

> > Seems that a "base/" prefix slipped in the -music part of d/copyright?
> 
> Nope, I think the base prefix is correct in d/copyright but the music
> and sounds directory should have been placed under the base directory in
> src:ufoai-music. At least that would have been more consistent. I can
> change that. 

Yes, this will fix it too.

> > Nitpick*: It looks like you are autogenerating the copyright file from
> > LICENSES. In this case, it would probably make sense (even if the
> > copyright-format-1.0 permits it to combine) to be more accurate and not
> > combine so many authors in one big block. 
> 
> Please see above. The script transforms upstream's LICENSES file into a
> machine-readable copyright format 1.0 file. I think the benefits are
> obvious and having the same information about authors listed as in
> LICENSES seems like a good thing to me.

Admitting, upstream is exemplary in case of tracking of its licenses
(also with the scope for Debian!), and it really helps to automate this
to get a skeleton dep5 file. 

However -- as with the output of licensecheck of the devscripts -- the
output needs to be checked and compared to *every* files in the source.
The LICENSE file may (and have) also errors: For example there are files
in this files with no copyright holder attributed. Or, there are URLs
attributed as copyright owners. How does the script handle this? In the
end this leads to wrongly attributed files that will either go unnoticed
(so Debian is violating copyright law) or lead to an FTP Master reject. 

To make it clear: I require an 100% accurate d/copyright and this is one
of the few points that are not subject to negotiations.

> 
> > "License: GPL-2
> >  On Debian systems, the complete text of the GNU General Public License
> >  version 2 can be found in "/usr/share/common-licenses/GPL-2".
> > "
> > This is not enough -- you need to add the first 3 paragraphs of the
> > license -- see the dep5' examples section.
> 
> https://www.debian.org/doc/debian-policy/ch-docs.html
> 
> "Packages distributed under the Apache license (version 2.0), the
> Artistic license, the GNU GPL (versions 1, 2, or 3), the GNU LGPL
> (versions 2, 2.1, or 3), and the GNU FDL (versions 1.2 or 1.3) should
> *refer* to the corresponding files under
> /usr/share/common-licenses,[119] *rather than quoting them* in the
> copyright file. "

> I believe we shouldn't make the process of creating debian/copyright
> even more painful and I think that a reference to
> /usr/share/common-licenses is more than enough for the most widely used
> free software license.

I disagree. As said above, d/copyright is important. Yes, it is tedious
to create it the first time and there are more exciting things to do,
but it is a necessity to be done and to do it right.

The policy means you should not quote the verbatim license, but it is
common practice to quote the first 3 paragraphs. 
Otherwise we'll introduce fuzziness.  Consider License GPL-2+
You refer to the GPL-2 file, which makes it non-obvious that you have
the "or later" option in place. For the causal user, its not
self-explaining what the "+" means.

Please add the few lines, I consider it not enough to just have the
reference. 

> > -> d/README.Source is refering to src:ufoai -- but this has no
> > README.Source, but should (actually a point of uifoai)
> > I think you need to tell in this file how to get the music files,
> > and you'll need to move the get-orig-source target from ufoai's
> > d/control here... (I prefer to have self-containing src packages,
> > which does not say "look in yyy to do get the source for xxx ..."
> 
> All information how to get the original sources are documented in
> src:ufoai + the package provides convenient get-orig-source targets.
> 
> The reasons therefor are:
> 
> - It saves us from duplicating the same information
> - It is easier to work with src:ufoai because we have all information
>   in one place
> - src:ufoai is small (only 9 MB) thus it saves bandwidth and time for
>   those who want to recreate the original source tarballs..
> > -> d/watch does not produce the source file but a file that does not
> > even have the -music files. This might be unexpected behavior, so this
> > needs to be documented in README.Source and in the watchfile.
> 
> d/watch just checks for new releases. It is far more convenient to work
> with the upstream Git repository hence I have created get-orig-source
> targets in src:ufoai.

As said in the beginning, I disagree. Each package needs to induvidually
fulfill the quality requirments.

If the watchfile does not retrieve the source, you need to have a
get-orig-source in d/rules and document who to get source for that
package in README.Source. You can recommend there to go ufoai and 
execute get-orig-source there to avoid double downloads, but I expect 
that this will be possible also without downloading another source
package.

(BTW, As you are repacking, so you should add the suffix +repack to the
upstream version; this makes clear that you won't find the tarball
upstream)

> Regards,
> 
> Markus
> 

--
tobi


Reply to: