Re: Review of controllib-removal branch requested
On Thu, 17 Jan 2008, Guillem Jover wrote: What would you suggest then ? Shall I add $substvar-set(Format, $changes_format) in dpkg-genchanges ? Sure, and also override the Format field if such substvar is present as documented in the man page. Ok, done. In this case I think the documentation is bogus. $varlistfile has never been initialized since that option was added (fb269aa4). And I don't think it makes sense to initalize it by default. Reverted and fixed the documentation. As you say, if it's not initialised, we might as well drop the substvars support in dpkg-source. One can always explicitely enable substvar processing by passing that option, so it's not completely useless (another thing is if that feature is desirable at all, but if it's disabled by default it does not hurt having it). Indeed. Thanks for the review! Cheers, -- Raphaël Hertzog Le best-seller français mis à jour pour Debian Etch : http://www.ouaza.com/livre/admin-debian/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Re: Review of controllib-removal branch requested
On Wed, 2008-01-16 at 08:18:12 +0100, Raphael Hertzog wrote: On Wed, 16 Jan 2008, Guillem Jover wrote: The Format substvar is not set anymore. Indeed, that was on purpose. It wasn't used by the code as a substitution but only like a general purpose variable (which I really created as is in dpkg-genchanges) and I have never seen any package use it. And it didn't make sense to store the version of the format of the changes file in all substvar objects that are used to make substitutions also in binary packages and source packages. Well, it's a published interface (deb-substvars(5)), so I'd consider this a regression. Exporting Format allows packages to extend it by changing the value w/o needed to modify dpkg itself, and adding or changing current field values for example. What would you suggest then ? Shall I add $substvar-set(Format, $changes_format) in dpkg-genchanges ? Sure, and also override the Format field if such substvar is present as documented in the man page. In dpkg-source.pl, $varlistfile should not be initialized, this is a functional change. I've actually been considering for some time removing the substvar support from dpkg-source. It is a functional change indeed but it only brings it in line with the documentation, cf dpkg-source.1: -Tsubstvarsfile Read substitution variables in substvarsfile; the default is debian/substvars. In this case I think the documentation is bogus. $varlistfile has never been initialized since that option was added (fb269aa4). And I don't think it makes sense to initalize it by default. As you say, if it's not initialised, we might as well drop the substvars support in dpkg-source. One can always explicitely enable substvar processing by passing that option, so it's not completely useless (another thing is if that feature is desirable at all, but if it's disabled by default it does not hurt having it). regards, guillem -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Re: Review of controllib-removal branch requested
Hi, On Tue, 2008-01-01 at 21:15:02 +0100, Raphael Hertzog wrote: I just finished the necessary work to get rid off controllib.pl. I pushed it in a new branch controllib-removal: http://git.debian.org/?p=dpkg/dpkg.git;a=shortlog;h=controllib-removal I welcome some review on the changes (it amounts to 35 changesets of which half comes from djpig's parsechangelog branch). The Format substvar is not set anymore. In dpkg-source.pl, $varlistfile should not be initialized, this is a functional change. I've actually been considering for some time removing the substvar support from dpkg-source. Also this is minor and it's done already, but while you were at it you could have fixed indentation and missing spaces for the touched lines. regards, guillem -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Re: Review of controllib-removal branch requested
Hi, On Wed, 16 Jan 2008, Guillem Jover wrote: On Tue, 2008-01-01 at 21:15:02 +0100, Raphael Hertzog wrote: I just finished the necessary work to get rid off controllib.pl. I pushed it in a new branch controllib-removal: http://git.debian.org/?p=dpkg/dpkg.git;a=shortlog;h=controllib-removal I welcome some review on the changes (it amounts to 35 changesets of which half comes from djpig's parsechangelog branch). The Format substvar is not set anymore. Indeed, that was on purpose. It wasn't used by the code as a substitution but only like a general purpose variable (which I really created as is in dpkg-genchanges) and I have never seen any package use it. And it didn't make sense to store the version of the format of the changes file in all substvar objects that are used to make substitutions also in binary packages and source packages. What would you suggest then ? Shall I add $substvar-set(Format, $changes_format) in dpkg-genchanges ? In dpkg-source.pl, $varlistfile should not be initialized, this is a functional change. I've actually been considering for some time removing the substvar support from dpkg-source. It is a functional change indeed but it only brings it in line with the documentation, cf dpkg-source.1: -Tsubstvarsfile Read substitution variables in substvarsfile; the default is debian/substvars. As you say, if it's not initialised, we might as well drop the substvars support in dpkg-source. Also this is minor and it's done already, but while you were at it you could have fixed indentation and missing spaces for the touched lines. I usually do that except when it's just a big copy/paste or a simple reindentation (because I removed an enclosing if, or something like that). I'll try to keep that in mind for the future. -- Raphaël Hertzog Le best-seller français mis à jour pour Debian Etch : http://www.ouaza.com/livre/admin-debian/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
Review of controllib-removal branch requested
Hello, I just finished the necessary work to get rid off controllib.pl. I pushed it in a new branch controllib-removal: http://git.debian.org/?p=dpkg/dpkg.git;a=shortlog;h=controllib-removal This branch also includes the parsechangelog branch of djpig and does one more change on top of it by adding a parse_changelog function to Dpkg::Changelog which was needed to replace controllib.pl's parsechangelog. I welcome some review on the changes (it amounts to 35 changesets of which half comes from djpig's parsechangelog branch). My plan is to merge this branch this week-end/early next week so that we'll run this code long enough to catch potential problems before the next upload. I'm running this version already here but a few more users/testers would be good. BTW, while we're doing important changes that need testing, it might also be a good idea to merge the work of Ian Jackson on triggers. And then maybe we can have an intermediary experimental upload. Cheers, -- Raphaël Hertzog Le best-seller français mis à jour pour Debian Etch : http://www.ouaza.com/livre/admin-debian/ -- To UNSUBSCRIBE, email to [EMAIL PROTECTED] with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
