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

Re: Working with gbp and older releases



On sob, lut 22, 2014 at 11:02:12 +0100, Tobias Frost wrote:
> Am Mittwoch, den 19.02.2014, 20:00 +0100 schrieb Dariusz Dwornikowski
> > I uploaded my version to mentors. Would you be so nice to review it ? 
> > http://mentors.debian.net/package/maradns
> 
> Hi Dariusz,
> 
> Sure, I'll take a look.  (I'm not a DD, so I cannot sponsor.)  
> 
> A suggestion: You should the RFS Bug to get a broader audience, maybe
> someone who can indeed sponsor.

Yes, I thought it would be better to share first with you, since you
seemed interesed. 

> Ok, lets jump into the package: (Note that I do not sort the items, I
> just write as I see them; so no ordering, severity, ... implied. And
> don't get shocked by the length of the remarks, thats normal for the
> first shot.)
> 
> -> Please file a bug for your changelog entry "Several security bugs" to
> document in the BTS that the current version in xxx has problems. 
> (One bug is enough, just quote your 5-lines in the changelog as bug
> report)

Did that, thank you. 
> 
> -> For the old changelogs, where you added for example
> 
>   [ Dariusz Dwornikowski ]
>   * Security bugfix for CVE-2012-1570
> 
> This is somehow misleading as it implies that you actively did smth on
> the pckageing, but leave it unclear "what has been done". I think you
> should not add your name here and hadd the "CVE-" to the (existing)
> changelog entry. In this special case I would just update the first line
> to

I was not sure about it either. Now I know that [] indicate job done,
not the author of particular changelog entry. 

> 
> maradns (2.0.06-1) experimental; urgency=low
> 
>   * New upstream release, fixes CVE-2012-1570
> 
> (And then document the change in d/changelog for the to-be-uploaded
> version, for example
>  
>   * Updated old d/changelog entries: Added information when the CVEs   
>     where fixed: (adding Debian-Versions which where changed=  
> 

Done. Thank you. 

> If you bump the SV, you should not if there have been any changes
> necessary and if not document that too.
> 
> Generally, d/changelog entries should reflect *what* have been changed
> on not focus on the *why* something has changed. For example, you write
> "We no longer modify the config (Closes: #710903)": 
> I would write "updated d/postinst to no longer modifiy conffiles.
> (Closes: #...)
> (and in the patch for it, do not just comment out the lines, remove them
> to be clear that this is not acciedentially commented out)

Done. Thank you. 
> 
> -> Regarding the usage of your repository. I would suggest to have "one
> change - one commit" and also have the commit messages speak for the
> changes (ideally, they are identical to the d/changelog entry); There
> were at least some commit which changed more than the git log says.
> (the commit for the conffile fix is a examples - it also changes
> d/control but does not mention it)

Seems reasonable. I will stick to this from now on. 

> (BTW, in this commit you change the dependency of duende to an *older*
> version? As this looks weird, I *suppose* this is wrong... Maybe you
> wanted to enforce the current version, then use ${binary:Version})

Fixed that. Thank you. 

> 
> I see also that there are sometimes undocumented changes, please
> document them. For example you updated the watchfile, add gpg signatures
> but this is not documented in the d/changelog. But this is just an
> example, there are more than this one. 

I got rid of that because it did not work. 

> 
> d/control:
> Why is the Breaks / Replaces necessary for maradns-zoneserver and other
> packages? Why does the docs breaks an older version of maradns?

This
Breaks/Replaces was some technique of the old maintainer. I removed it
and it works fine without it. I think it was because of him updating
conffiles in d/postinst, which got the package to violate the policy. 

> 
> In Debian there is a file DwRandPrime.h which seems somehow
> autogenerated. What is its purpose?

This is a weirdo. While building maradns generated a random prime
number and writes it to DwRandPrime.h, I keep the upstream original in
the debian repository to avoid "upstream changed" problems. After
every build the DwRandPrime.h would be changed and gbp and debuild
would complain. This assures reproductible builds.  
> 
> 
> ((note, I had to stop the review here due to time constraints)

Thank you. It helped a lot. 
> 
> 
> Best regards, 
> Tobias
> 

-- 
Pozdrawiam,
Dariusz Dwornikowski, Assistant at Institute of Computing Science, Poznań University of Technology
www.cs.put.poznan.pl/ddwornikowski/ 
room 2.7.2 BTiCW | tel. +48 61 665 29 41


Reply to: