Bug#477751: tackling this bug
Hi Joey, sgml-base 1.26+nmu2 has been accepted in sid. Can you go ahead and upload debhelper? I talked to the release team and will take care of the binnmus. Helmut -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#477751: tackling this bug
On Mon, Apr 30, 2012 at 05:52:35PM +0200, Helmut Grohne wrote: On the debhelper side it should be enough to remove all remaining calls to update-catalog and introduce a dependency on the changed sgml-base. I did not test this thus far. I worked out the remaining bits and tested them. For convenience the very same sgml-base.debdiff as in the previous mail is attached. In addition a debhelper.debdiff fixing this issue is attached. I verified the following things (in pbuilder): 1) Building sgml-base NMU. 2) Building debhelper NMU. 3) Installing sgml-base NMU. 4) Upgrading sgml-base from sid to NMU. 5) Upgrading debhelper from sid to NMU. 6) Building python-docutils with debhelper NMU. 7) Upgrading docutils-common to binNMU. 8) Purging docutils-common binNMU. 9) Installing docutils-common binNMU. I did not observe any problems like obvious failures or even conffile questions. I looked at /etc/sgml/catalog and /etc/sgml/docutils-common.cat after each step and verified that the contents are sensible. Note that downgrading sgml-base leaves artifacts. Also note that if the debhelper.debdiff gets applied before the sgml-base.debdiff gets applied, packages built with the updated debhelper will be uninstallable. I ask for feedback on this combination of patches. Since the bug is assigned to debhelper now, I explicitly pull in the sgml-base maintainers (who seem to be MIA). Helmut diff -Nru sgml-base-1.26+nmu1/debian/changelog sgml-base-1.26+nmu2/debian/changelog --- sgml-base-1.26+nmu1/debian/changelog2010-07-18 14:39:38.0 +0200 +++ sgml-base-1.26+nmu2/debian/changelog2012-04-30 17:16:27.0 +0200 @@ -1,3 +1,13 @@ +sgml-base (1.26+nmu2) unstable; urgency=low + + * Non-maintainer upload. + * Generate the super catalog from /etc/sgml directory contents. +This does not solve #477751, but the upcoming debhelper changes will solve +that bug based on this work. + * Do not truncate the manual pages during build. + + -- Helmut Grohne hel...@subdivi.de Mon, 30 Apr 2012 17:15:48 +0200 + sgml-base (1.26+nmu1) unstable; urgency=low * Non-maintainer upload diff -Nru sgml-base-1.26+nmu1/debian/control sgml-base-1.26+nmu2/debian/control --- sgml-base-1.26+nmu1/debian/control 2010-07-18 14:37:50.0 +0200 +++ sgml-base-1.26+nmu2/debian/control 2012-04-30 13:15:14.0 +0200 @@ -11,7 +11,7 @@ Priority: optional Architecture: all Conflicts: sgml-data (= 0.02), sgmltools-2 (= 2.0.2-4) -Depends: ${perl:Depends} +Depends: ${perl:Depends}, dpkg (= 1.14.18) Suggests: sgml-base-doc Description: SGML infrastructure and SGML catalog file support This package creates the SGML infrastructure directories and provides diff -Nru sgml-base-1.26+nmu1/debian/copyright sgml-base-1.26+nmu2/debian/copyright --- sgml-base-1.26+nmu1/debian/copyright2004-06-07 05:18:28.0 +0200 +++ sgml-base-1.26+nmu2/debian/copyright2012-04-30 17:06:30.0 +0200 @@ -6,6 +6,7 @@ Copyright (C) 1997 Christian Schwarz schw...@debian.org. Copyright (C) 2001-2004 Ardo van Rangelrooij a...@debian.org +Copyright (C) 2012 Helmut Grohne hel...@subdivi.de This is free software; you may redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free diff -Nru sgml-base-1.26+nmu1/debian/sgml-base.NEWS sgml-base-1.26+nmu2/debian/sgml-base.NEWS --- sgml-base-1.26+nmu1/debian/sgml-base.NEWS 1970-01-01 01:00:00.0 +0100 +++ sgml-base-1.26+nmu2/debian/sgml-base.NEWS 2012-04-30 16:37:03.0 +0200 @@ -0,0 +1,13 @@ +sgml-base (1.26+nmu2) unstable; urgency=low + + Starting with this release the SGML super catalog /etc/sgml/catalog will be + replaced with a symbolic link to /var/lib/sgml-base/supercatalog. The latter + file can be regenerated from the contents of the /etc/sgml directory including + all files ending in .cat using the new update-catalog --update-super option. + This call will be (dpkg) triggered by packages placing files in /etc/sgml. The + transition to this way of handling the super catalog will loose user changes to + /etc/sgml/catalog. Further overwriting of user changes will happen until all + packages using dh_installcatalogs are built with a fixed version of debhelper. + Sorry for the inconvenience. + + -- Helmut Grohne hel...@subdivi.de Mon, 30 Apr 2012 16:37:01 +0200 diff -Nru sgml-base-1.26+nmu1/debian/sgml-base.dirs sgml-base-1.26+nmu2/debian/sgml-base.dirs --- sgml-base-1.26+nmu1/debian/sgml-base.dirs 1970-01-01 01:00:00.0 +0100 +++ sgml-base-1.26+nmu2/debian/sgml-base.dirs 2012-04-30 13:16:43.0 +0200 @@ -0,0 +1 @@ +var/lib/sgml-base diff -Nru sgml-base-1.26+nmu1/debian/sgml-base.postinst sgml-base-1.26+nmu2/debian/sgml-base.postinst --- sgml-base-1.26+nmu1/debian/sgml-base.postinst 2004-08-14 17:04:15.0 +0200 +++ sgml-base-1.26+nmu2/debian/sgml-base.postinst 2012-04-30 14:15:51.0 +0200 @@ -11,20 +11,11 @@ then
Bug#477751: tackling this bug
Helmut Grohne wrote: I ask for feedback on this combination of patches. Since the bug is assigned to debhelper now, I explicitly pull in the sgml-base maintainers (who seem to be MIA). Right. As I think I've posted before to this bug, I will move ahead with the debhelper changes as soon as sgml-base is in unstable (or perhaps testing). -- see shy jo signature.asc Description: Digital signature
Bug#477751: tackling this bug
On Fri, Apr 27, 2012 at 10:54:01AM -0400, Joey Hess wrote: It certianly seems feasible to convert it to a symlink into /var. I worked out the sgml-base part of the patch. It will turn the super catalog into a symbolic link from /etc/sgml/catalog to /var/lib/sgml-base/supercatalog and update the latter file using triggers. The loosing of user configuration will persist in all detail until packages are built with a fixed version of debhelper. I verified that upgrading to my sgml-base nmu and reinstalling docutils-common (a caller of update-catalog) works as expected. On the debhelper side it should be enough to remove all remaining calls to update-catalog and introduce a dependency on the changed sgml-base. I did not test this thus far. Can you comment on the approach taken in the sgml-base.debdiff? Helmut diff -Nru sgml-base-1.26+nmu1/debian/changelog sgml-base-1.26+nmu2/debian/changelog --- sgml-base-1.26+nmu1/debian/changelog2010-07-18 14:39:38.0 +0200 +++ sgml-base-1.26+nmu2/debian/changelog2012-04-30 17:16:27.0 +0200 @@ -1,3 +1,13 @@ +sgml-base (1.26+nmu2) unstable; urgency=low + + * Non-maintainer upload. + * Generate the super catalog from /etc/sgml directory contents. +This does not solve #477751, but the upcoming debhelper changes will solve +that bug based on this work. + * Do not truncate the manual pages during build. + + -- Helmut Grohne hel...@subdivi.de Mon, 30 Apr 2012 17:15:48 +0200 + sgml-base (1.26+nmu1) unstable; urgency=low * Non-maintainer upload diff -Nru sgml-base-1.26+nmu1/debian/control sgml-base-1.26+nmu2/debian/control --- sgml-base-1.26+nmu1/debian/control 2010-07-18 14:37:50.0 +0200 +++ sgml-base-1.26+nmu2/debian/control 2012-04-30 13:15:14.0 +0200 @@ -11,7 +11,7 @@ Priority: optional Architecture: all Conflicts: sgml-data (= 0.02), sgmltools-2 (= 2.0.2-4) -Depends: ${perl:Depends} +Depends: ${perl:Depends}, dpkg (= 1.14.18) Suggests: sgml-base-doc Description: SGML infrastructure and SGML catalog file support This package creates the SGML infrastructure directories and provides diff -Nru sgml-base-1.26+nmu1/debian/copyright sgml-base-1.26+nmu2/debian/copyright --- sgml-base-1.26+nmu1/debian/copyright2004-06-07 05:18:28.0 +0200 +++ sgml-base-1.26+nmu2/debian/copyright2012-04-30 17:06:30.0 +0200 @@ -6,6 +6,7 @@ Copyright (C) 1997 Christian Schwarz schw...@debian.org. Copyright (C) 2001-2004 Ardo van Rangelrooij a...@debian.org +Copyright (C) 2012 Helmut Grohne hel...@subdivi.de This is free software; you may redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free diff -Nru sgml-base-1.26+nmu1/debian/sgml-base.NEWS sgml-base-1.26+nmu2/debian/sgml-base.NEWS --- sgml-base-1.26+nmu1/debian/sgml-base.NEWS 1970-01-01 01:00:00.0 +0100 +++ sgml-base-1.26+nmu2/debian/sgml-base.NEWS 2012-04-30 16:37:03.0 +0200 @@ -0,0 +1,13 @@ +sgml-base (1.26+nmu2) unstable; urgency=low + + Starting with this release the SGML super catalog /etc/sgml/catalog will be + replaced with a symbolic link to /var/lib/sgml-base/supercatalog. The latter + file can be regenerated from the contents of the /etc/sgml directory including + all files ending in .cat using the new update-catalog --update-super option. + This call will be (dpkg) triggered by packages placing files in /etc/sgml. The + transition to this way of handling the super catalog will loose user changes to + /etc/sgml/catalog. Further overwriting of user changes will happen until all + packages using dh_installcatalogs are built with a fixed version of debhelper. + Sorry for the inconvenience. + + -- Helmut Grohne hel...@subdivi.de Mon, 30 Apr 2012 16:37:01 +0200 diff -Nru sgml-base-1.26+nmu1/debian/sgml-base.dirs sgml-base-1.26+nmu2/debian/sgml-base.dirs --- sgml-base-1.26+nmu1/debian/sgml-base.dirs 1970-01-01 01:00:00.0 +0100 +++ sgml-base-1.26+nmu2/debian/sgml-base.dirs 2012-04-30 13:16:43.0 +0200 @@ -0,0 +1 @@ +var/lib/sgml-base diff -Nru sgml-base-1.26+nmu1/debian/sgml-base.postinst sgml-base-1.26+nmu2/debian/sgml-base.postinst --- sgml-base-1.26+nmu1/debian/sgml-base.postinst 2004-08-14 17:04:15.0 +0200 +++ sgml-base-1.26+nmu2/debian/sgml-base.postinst 2012-04-30 14:15:51.0 +0200 @@ -11,20 +11,11 @@ then ## -- -## create SGML root catalog -[ ! -f /etc/sgml/catalog ] \ -cp -a /usr/share/sgml-base/catalog.super /etc/sgml/catalog - -## -- ## clean up /usr/lib/sgml if [ -d /usr/lib/sgml ] then ## -- -## remove nasty old circular catalog - update-catalog --remove --super /usr/lib/sgml/catalog || true - - ##
Bug#477751: tackling this bug
Helmut Grohne wrote: I worked out the sgml-base part of the patch. It will turn the super catalog into a symbolic link from /etc/sgml/catalog to /var/lib/sgml-base/supercatalog and update the latter file using triggers. The loosing of user configuration will persist in all detail until packages are built with a fixed version of debhelper. I verified that upgrading to my sgml-base nmu and reinstalling docutils-common (a caller of update-catalog) works as expected. On the debhelper side it should be enough to remove all remaining calls to update-catalog and introduce a dependency on the changed sgml-base. I did not test this thus far. Won't dh_installcatalogs also need to be modified to stop deleting the package's central catalog file on upgrade? Or does this do away with the central catalog thing and just use the super catalog? -- see shy jo signature.asc Description: Digital signature
Bug#477751: tackling this bug
On Mon, Apr 30, 2012 at 12:24:52PM -0400, Joey Hess wrote: Helmut Grohne wrote: On the debhelper side it should be enough to remove all remaining calls to update-catalog and introduce a dependency on the changed sgml-base. I did not test this thus far. Won't dh_installcatalogs also need to be modified to stop deleting the package's central catalog file on upgrade? Or does this do away with the central catalog thing and just use the super catalog? There are two places where the central catalog is deleted. I am not sure which of them you are talking about: 1) During postinst the old snippet removes and recreates the central catalog. This behaviour is removed by my debhelper.debdiff. 2) The transitional code I added removes the central catalog in preinst if the central catalog is not a conffile. This call could be disabled. That would cause a question about a supposedly user modified configuration file for every single central catalog that is shipped with a package being rebuild with an updated debhelper. I deemed this question unacceptable, because it would hit every installation (even those who never actively touched those files). By removing the file in preinst, dpkg will put the file back during unpack and everything is fine (except one more loss of user configuration). So we still have central catalogs, but they are proper conffiles now. Does this answer your question? Helmut -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#477751: tackling this bug
Helmut Grohne wrote: On Mon, Apr 30, 2012 at 12:24:52PM -0400, Joey Hess wrote: Helmut Grohne wrote: On the debhelper side it should be enough to remove all remaining calls to update-catalog and introduce a dependency on the changed sgml-base. I did not test this thus far. There are two places where the central catalog is deleted. I am not sure which of them you are talking about: 1) During postinst the old snippet removes and recreates the central catalog. This behaviour is removed by my debhelper.debdiff. Ok, so you still want that applied. Was not clear. -- see shy jo signature.asc Description: Digital signature
Bug#477751: tackling this bug
On Thu, Apr 26, 2012 at 06:18:40PM -0400, Joey Hess wrote: This is why I originally recommended that the registration process be converted to use triggers. A [directory full] of catalogs, and a root catalog file automatically generated from them (which need not be a config file in /etc) is a much cleaner approach. This change would be fairly intrusive, but it clearly has its advantages. update-catalog would be updated to turn any calls containing --super into no-ops. These configuration options are somewhat burnt by the current prerm and postinst invocations and can no longer be used by an administrator in a sane way. /etc/sgml/catalog would be regenerated using a new update-catalog --update-super. (I don't think moving the file elsewhere is feasible.) It would unconditionally overwrite /etc/sgml/catalog to include /etc/sgml/*.cat. The trigger interest would be declared in sgml-base. No trigger activation is necessary. The generated /etc/sgml/catalog would explain that to remove a catalog an administrator should call update-catalog --disable $package. This would mv /etc/sgml/$package.cat{,.disabled} and --update-super. Similarly --enable $package would revert this change. These file moves persist during upgrades, because removed conffiles are not readded. Does this method have any obvious problems? I can write a patch. Helmut -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#477751: tackling this bug
Helmut Grohne wrote: On Thu, Apr 26, 2012 at 06:18:40PM -0400, Joey Hess wrote: This is why I originally recommended that the registration process be converted to use triggers. A [directory full] of catalogs, and a root catalog file automatically generated from them (which need not be a config file in /etc) is a much cleaner approach. This change would be fairly intrusive, but it clearly has its advantages. update-catalog would be updated to turn any calls containing --super into no-ops. These configuration options are somewhat burnt by the current prerm and postinst invocations and can no longer be used by an administrator in a sane way. /etc/sgml/catalog would be regenerated using a new update-catalog --update-super. (I don't think moving the file elsewhere is feasible.) It certianly seems feasible to convert it to a symlink into /var. -- see shy jo signature.asc Description: Digital signature
Bug#477751: tackling this bug
Helmut Grohne wrote: So we seem to agree that both solutions (present vs. adding --transition to sgml-base) are doable and both have their own problems. Are you still interested in pushing the transitional code to sgml-base? Your arguments have convinced me that it could work. I have not yet updated the patch to do that, so please tell me whether you want that change (at least for evaluation). While I'm leaning toward just putting the code in debhelper, I am worried about another issue in the patch. It makes update-catalog be called only on new install, not upgrade ([-z $2]). But then, if a catalog is added to an existing package, update-catalog will not be run. There is a new preinst that runs update-catalog on upgrade, but only during the transition to conffiles, so it does not close this hole. -- see shy jo signature.asc Description: Digital signature
Bug#477751: tackling this bug
On Thu, Apr 26, 2012 at 01:57:33PM -0400, Joey Hess wrote: While I'm leaning toward just putting the code in debhelper, I am worried about another issue in the patch. It makes update-catalog be called only on new install, not upgrade ([-z $2]). But then, if a catalog is added to an existing package, update-catalog will not be run. There is a new preinst that runs update-catalog on upgrade, but only during the transition to conffiles, so it does not close this hole. Thanks for looking at it this close. This issue definitely needs to be addressed. It gets even worse. Consider the case where a maintainer removes a catalog from an existing package and stops calling dh_installcatalogs. Then the root catalog would contain a dangling reference and there really is no way to fix this anymore, because our code is never invoked again. This probably is precisely the reason for why we currently remove and add the catalog. To fix this, we will have to keep removing the package catalog from the root catalog in prerm, but remember that we did it. In the postinst we will have to add it again, iff we removed it before. The big question for me would be: How to transfer this state from prerm to postinst? Options would be: 1) Some state file in /etc. The apache2 upgrade to experimental takes this approach for instance. (Thanks to Arno Töll for pointing out.) 2) Some state file in /var/run. See sysv-rc for an example. A downside is that if our upgrade gets interrupted and the system is rebooted, our state file is gone. 3) Some state file in /var/lib. Seems ugly. 3) Abuse some debconf key for this. 4) Something else? Any opinion on this? Helmut -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#477751: tackling this bug
Helmut Grohne wrote: It gets even worse. Consider the case where a maintainer removes a catalog from an existing package and stops calling dh_installcatalogs. Then the root catalog would contain a dangling reference and there really is no way to fix this anymore, because our code is never invoked again. This probably is precisely the reason for why we currently remove and add the catalog. To fix this, we will have to keep removing the package catalog from the root catalog in prerm, but remember that we did it. In the postinst we will have to add it again, iff we removed it before. The big question for me would be: How to transfer this state from prerm to postinst? This is why I originally recommended that the registration process be converted to use triggers. A file fill of catalogs, and a root catalog file automatically generated from them (which need not be a config file in /etc) is a much cleaner approach. -- see shy jo signature.asc Description: Digital signature
Bug#477751: tackling this bug
Joey Hess wrote: This is why I originally recommended that the registration process be converted to use triggers. A file fill of catalogs, and a root catalog ^ directory full file automatically generated from them (which need not be a config file in /etc) is a much cleaner approach. -- see shy jo signature.asc Description: Digital signature
Bug#477751: tackling this bug
Helmut Grohne wrote: In the unlikely event that the admin called it, it would detect that the file was a conffile and not delete it. An admin could call update-catalog --transition for a package that was not rebuilt with the newer debhelper. In that case harm would still happen. Do you have an idea about how to prevent this? Since this is deleting possibly modified config files on upgrade anyway, this doesn't seem worth worrying about. I do not see any complexity in a versioned dependency; dh_installcatalogs already adds one. It must be a Pre-Depends, since we are using it in preinst. Ok, that's annoying as each package would need misc:PreDepends added. In that case, maybe this should be left in debhelper. I am pretty uncomfortable with it though, especially since it does delete config files. -- see shy jo signature.asc Description: Digital signature
Bug#477751: tackling this bug
On Tue, Apr 17, 2012 at 09:08:30AM -0400, Joey Hess wrote: Helmut Grohne wrote: An admin could call update-catalog --transition for a package that was not rebuilt with the newer debhelper. In that case harm would still happen. Do you have an idea about how to prevent this? Since this is deleting possibly modified config files on upgrade anyway, this doesn't seem worth worrying about. Thinking about it we could check the state of the package being transitioned. If an admin calls --transition that would most likely be ii. If preinst calls it, I guess it would be iF or at least not ii. Ok, that's annoying as each package would need misc:PreDepends added. In that case, maybe this should be left in debhelper. I am pretty uncomfortable with it though, especially since it does delete config files. So we seem to agree that both solutions (present vs. adding --transition to sgml-base) are doable and both have their own problems. Are you still interested in pushing the transitional code to sgml-base? Your arguments have convinced me that it could work. I have not yet updated the patch to do that, so please tell me whether you want that change (at least for evaluation). Helmut -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#477751: [ping] Re: Bug#477751: tackling this bug
Hi Joey, There is still no progress on this release critical issue. Given the number of affected packages that will need a rebuild and the freeze in June, I ask for action. Can you comment on my reasons against the proposed change to sgml-base or come up with a solution yourself? These were my points. On Sat, Jan 07, 2012 at 10:25:17PM +0100, Helmut Grohne wrote: On Sat, Jan 07, 2012 at 02:53:46PM -0400, Joey Hess wrote: But update-catalog can get new switches that handle the transition, and debhelper can update the code to use them. Ok. Let's evaulate what could be changed about update-catalog. 1) package catalog. As per Daniel's request the package catalogs are now created at build time, so update-catalog no longer touches them. The only place we still touch the package catalog is to remove it (being an unowned file in /etc) to transition to a proper configfile. So we would add some update-catalog --transition-catalog to the debhelper preinst. It would have do the magic to detect whether this transition is actually necessary. 2) root catalog. The package catalog is currently removed and readded to the root catalog during every upgrade. This is to change, but the next upgrade will still do the removal. So the --transition-catalog would do this as well. This --transition-catalog would do harm to the system when invoked by an administrator since it relies on the broken behaviour of debhelper's prerm and the creation of the conffile by the package upgrade. Essentially the transitional code that I put into preinst would be moved to update-catalog. I honestly do not see the value in this. In fact it the complexity is even larger since we now have to depend on a newer version of sgml-base and if we really need to apply further fixes we need to change two packages now. Not mentioning the combinatorial explosion of version combinations (of debhelper and sgml-base). Another argument against this move is that it makes removing the transitional code much harder. Helmut -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#477751: [ping] Re: Bug#477751: tackling this bug
Helmut Grohne wrote: These were my points. On Sat, Jan 07, 2012 at 10:25:17PM +0100, Helmut Grohne wrote: On Sat, Jan 07, 2012 at 02:53:46PM -0400, Joey Hess wrote: But update-catalog can get new switches that handle the transition, and debhelper can update the code to use them. Ok. Let's evaulate what could be changed about update-catalog. 1) package catalog. As per Daniel's request the package catalogs are now created at build time, so update-catalog no longer touches them. The only place we still touch the package catalog is to remove it (being an unowned file in /etc) to transition to a proper configfile. So we would add some update-catalog --transition-catalog to the debhelper preinst. It would have do the magic to detect whether this transition is actually necessary. This --transition-catalog would do harm to the system when invoked by an administrator since it relies on the broken behaviour of debhelper's prerm and the creation of the conffile by the package upgrade. Your patch already has the preinst calling update-catalog. AFAICS, update-catalog could check with dpkg-query if the file is not owned by a package, and not remove it unless this was the case, and it was called with --transition. In the unlikely event that the admin called it, it would detect that the file was a conffile and not delete it. Essentially the transitional code that I put into preinst would be moved to update-catalog. I honestly do not see the value in this. In fact it the complexity is even larger since we now have to depend on a newer version of sgml-base I do not see any complexity in a versioned dependency; dh_installcatalogs already adds one. and if we really need to apply further fixes we need to change two packages now. No, you just change sgml-base in a manner consistent with its new interface. debhelper does not enter this highly hypothetical scenario. Not mentioning the combinatorial explosion of version combinations (of debhelper and sgml-base) AFAICS the explosion results in 4 combinations. Another argument against this move is that it makes removing the transitional code much harder. Well, it's what, 3 lines? The difference is that it's 3 lines in one place. -- see shy jo -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#477751: tackling this bug
Hi Joey, Thanks for your quick response after the ping. On Sun, Apr 15, 2012 at 02:47:03PM -0400, Joey Hess wrote: Your patch already has the preinst calling update-catalog. AFAICS, update-catalog could check with dpkg-query if the file is not owned by a package, and not remove it unless this was the case, and it was called with --transition. Agreed. In the unlikely event that the admin called it, it would detect that the file was a conffile and not delete it. An admin could call update-catalog --transition for a package that was not rebuilt with the newer debhelper. In that case harm would still happen. Do you have an idea about how to prevent this? I do not see any complexity in a versioned dependency; dh_installcatalogs already adds one. It must be a Pre-Depends, since we are using it in preinst. So I could argue that it is even too complex for you to spot on the first look. and if we really need to apply further fixes we need to change two packages now. No, you just change sgml-base in a manner consistent with its new interface. debhelper does not enter this highly hypothetical scenario. I agree on your reasoning to keep the transition code at a single place. If things really go wrong though, I would assume that just changing the preinst hook is not enough. In that case we really need to touch debhelper as well. Not mentioning the combinatorial explosion of version combinations (of debhelper and sgml-base) AFAICS the explosion results in 4 combinations. If you were not planning on further fixes, there would be no need to move the transitional code to sgml-base, because it would just work. Assuming that we need to release another sgml-base and debhelper version we now have at least 9 combinations. The actual explosion resides in the relationships. As pointed out above, we already need Pre-Depend. The number of possible relationships you can declare really explodes. Another argument against this move is that it makes removing the transitional code much harder. Well, it's what, 3 lines? The difference is that it's 3 lines in one place. Almost. You need to remove both the caller and the callee. Even though concur with your line count, this is two places. If we can sort out the issue about the admin, I can change the patch to move the transitional code to sgml-base. Helmut -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#477751: tackling this bug
Helmut Grohne wrote: * preinst will do the tricky transition part. If it is called during an upgrade and /etc/sgml/$package.cat is not owned by any package (this is currently the case), then it fixes up the installation. The old prerm will have the package catalog removed from the root catalog, so it is readded here. The old postinst would recreate /etc/sgml/$package.cat. This file is removed during preinst. The advantage of this approach is that the conffile can be installed without asking the user. The disadvantage of this approach is that we are overwriting user changes one more time. I don't think it's appropriate to put tricky transition code into debhelper autoscripts, from where it is exploded out to lots of packages. If it's tricky, it's going to break, and it needs to be in a centralized location so the breakage can be fixed with one upload. Also, it's quite likely that other packages also use update-catalog, without using dh_installcatalogs. So this should be fixed in update-catalog. There is a debhelper.debdiff attached which implements the above description. I have rebuild xml-core using this patched debhelper and tried to upgrade and reinstall xml-core. However downgrading xml-core and upgrading it again results in a broken installation. Even when downgrading a package a conffile stays to be a conffile, so the preinst hook is only executed during the first upgrade. After the second upgrade the /etc/sgml/$package.cat is left untouched (being a conffile) and missing from /etc/sgml/catalog. It's not clear to me if this is a serious problem with this approach or not. I will let the maintainer of update-catalog deside when implementing their solution. -- see shy jo signature.asc Description: Digital signature
Bug#477751: tackling this bug
Hi Joey, thanks for your response. On Sat, Jan 07, 2012 at 01:01:56PM -0400, Joey Hess wrote: Helmut Grohne wrote: * preinst will do the tricky transition part. If it is called during an upgrade and /etc/sgml/$package.cat is not owned by any package (this is currently the case), then it fixes up the installation. The old prerm will have the package catalog removed from the root catalog, so it is readded here. The old postinst would recreate /etc/sgml/$package.cat. This file is removed during preinst. The advantage of this approach is that the conffile can be installed without asking the user. The disadvantage of this approach is that we are overwriting user changes one more time. I don't think it's appropriate to put tricky transition code into debhelper autoscripts, from where it is exploded out to lots of packages. If it's tricky, it's going to break, and it needs to be in a centralized location so the breakage can be fixed with one upload. I agree that the complexity should not reside in debhelper templates. However that is already the case. The entire code that is responsible for #88010 already is contained in debhelper. It is debhelper's prerm that removes the package catalog from the root catalog. And it is debhelper's postinst that removes the package catalog file. Those are two removals shipped in current debhelper and they are causing the harm. One of those removals is done using /bin/rm (out of scope of sgml-base) and the other removal is a call to update-catalog --quiet --remove --super /etc/sgml/$package.cat. Turning this into a no-op is error prone on its own as it might legitimately be invoked that way from a user. Please observe how my debdiff reduces this complexity. By generating the package catalog at build time the code that modifies the package catalog at installation time is removed altogether. All that remains are two calls to update-catalog in postinst and prerm to modify the root catalog. The preinst and postrm contents are only needed for the transition period to fix old stuff. Also, it's quite likely that other packages also use update-catalog, without using dh_installcatalogs. So this should be fixed in update-catalog. Unless those other packages use the same snippets as debhelper they are not affected. If they do, those packages need to be fixed separately, but this issue cannot be solved by a change to update-catalog. There is a debhelper.debdiff attached which implements the above description. I have rebuild xml-core using this patched debhelper and tried to upgrade and reinstall xml-core. However downgrading xml-core and upgrading it again results in a broken installation. Even when downgrading a package a conffile stays to be a conffile, so the preinst hook is only executed during the first upgrade. After the second upgrade the /etc/sgml/$package.cat is left untouched (being a conffile) and missing from /etc/sgml/catalog. It's not clear to me if this is a serious problem with this approach or not. I will let the maintainer of update-catalog deside when implementing their solution. If you can come up with a better approach to detect an old debhelper template, I can adapt the patch. For instance I could craft a regular expression for grepping $(dpkg-query --control-path $package postinst). I would expect a number of packages to break when you try to downgrade them. Also note that downgrading packages is not officially supported (especially when downgrading towards a broken package). As far as I can see sgml-base is unmaintained. It received its last maintainer upload in 2004, so it seems pointless to wait for the maintainer. Helmut -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#477751: tackling this bug
Helmut Grohne wrote: I agree that the complexity should not reside in debhelper templates. However that is already the case. The entire code that is responsible for #88010 already is contained in debhelper. It is debhelper's prerm that removes the package catalog from the root catalog. And it is debhelper's postinst that removes the package catalog file. Those are two removals shipped in current debhelper and they are causing the harm. One of those removals is done using /bin/rm (out of scope of sgml-base) and the other removal is a call to update-catalog --quiet --remove --super /etc/sgml/$package.cat. Turning this into a no-op is error prone on its own as it might legitimately be invoked that way from a user. I'm not suggesting that the existing code, as currently present in package maintainer scripts everywhere, should somehow implement this transition. But update-catalog can get new switches that handle the transition, and debhelper can update the code to use them. -- see shy jo signature.asc Description: Digital signature
Bug#477751: tackling this bug
Hi Joey, On Sat, Jan 07, 2012 at 02:53:46PM -0400, Joey Hess wrote: But update-catalog can get new switches that handle the transition, and debhelper can update the code to use them. Ok. Let's evaulate what could be changed about update-catalog. 1) package catalog. As per Daniel's request the package catalogs are now created at build time, so update-catalog no longer touches them. The only place we still touch the package catalog is to remove it (being an unowned file in /etc) to transition to a proper configfile. So we would add some update-catalog --transition-catalog to the debhelper preinst. It would have do the magic to detect whether this transition is actually necessary. 2) root catalog. The package catalog is currently removed and readded to the root catalog during every upgrade. This is to change, but the next upgrade will still do the removal. So the --transition-catalog would do this as well. This --transition-catalog would do harm to the system when invoked by an administrator since it relies on the broken behaviour of debhelper's prerm and the creation of the conffile by the package upgrade. Essentially the transitional code that I put into preinst would be moved to update-catalog. I honestly do not see the value in this. In fact it the complexity is even larger since we now have to depend on a newer version of sgml-base and if we really need to apply further fixes we need to change two packages now. Not mentioning the combinatorial explosion of version combinations (of debhelper and sgml-base). Another argument against this move is that it makes removing the transitional code much harder. Helmut -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#477751: tackling this bug
Hi Daniel and Joey, I took some more time to look at Daniel's proposal and managed to come up with an implementation which consists of one debdiff to only debhelper (no sgml-base changes). On Mon, Dec 05, 2011 at 12:05:26AM +0100, Daniel Leidert wrote: (1) Register the catalog, if it exists (and unregister any registered catalog, if it doesn't exist anymore). So users can remove the package catalog file. (2) Register the catalog only during installation, but not during upgrade. Usually we only add a catalog reference to the super catalog. (3) Catalog files should be written at build time not during installation. Instead of creating /etc/sgml/package.cat during installation, this should be created during package build. So the user can edit /etc/sgml/package.cat and /etc/sgml/catalog and we preserve these changes. If the user now changes /etc/sgml/package.cat and we need to ship an updated file, he should usually be asked, if he wishes to update the file during installation. I implemented the above description. The details are: * prerm will no longer remove the package catalog from the root catalog during upgrade. * postrm will only remove the .old file on purge (dpkg remove the conffile /etc/sgml/$package.cat). * postinst will no longer regenerate /etc/sgml/$package.cat and only add the package catalog to the root catalog during installation (as it is no longer removed during upgrade). * dh_installcatalogs will create a /etc/sgml/$package.cat containing the same contents (without the comment header). * preinst will do the tricky transition part. If it is called during an upgrade and /etc/sgml/$package.cat is not owned by any package (this is currently the case), then it fixes up the installation. The old prerm will have the package catalog removed from the root catalog, so it is readded here. The old postinst would recreate /etc/sgml/$package.cat. This file is removed during preinst. The advantage of this approach is that the conffile can be installed without asking the user. The disadvantage of this approach is that we are overwriting user changes one more time. There is a debhelper.debdiff attached which implements the above description. I have rebuild xml-core using this patched debhelper and tried to upgrade and reinstall xml-core. However downgrading xml-core and upgrading it again results in a broken installation. Even when downgrading a package a conffile stays to be a conffile, so the preinst hook is only executed during the first upgrade. After the second upgrade the /etc/sgml/$package.cat is left untouched (being a conffile) and missing from /etc/sgml/catalog. What do you think about this approach? Helmut diff -Nru debhelper-8.9.13/autoscripts/postinst-sgmlcatalog debhelper-8.9.13+nmu1/autoscripts/postinst-sgmlcatalog --- debhelper-8.9.13/autoscripts/postinst-sgmlcatalog 2011-08-07 02:46:45.0 +0200 +++ debhelper-8.9.13+nmu1/autoscripts/postinst-sgmlcatalog 2011-12-12 13:26:14.0 +0100 @@ -1,7 +1,3 @@ -if [ $1 = configure ]; then - rm -f #CENTRALCAT# - for ordcat in #ORDCATS#; do - update-catalog --quiet --add #CENTRALCAT# ${ordcat} - done +if [ $1 = configure ] [ -z $2 ]; then update-catalog --quiet --add --super #CENTRALCAT# fi diff -Nru debhelper-8.9.13/autoscripts/postrm-sgmlcatalog debhelper-8.9.13+nmu1/autoscripts/postrm-sgmlcatalog --- debhelper-8.9.13/autoscripts/postrm-sgmlcatalog 2011-08-07 02:46:45.0 +0200 +++ debhelper-8.9.13+nmu1/autoscripts/postrm-sgmlcatalog2011-12-12 12:12:48.0 +0100 @@ -1,3 +1,3 @@ if [ $1 = purge ]; then - rm -f #CENTRALCAT# #CENTRALCAT#.old + rm -f #CENTRALCAT#.old fi diff -Nru debhelper-8.9.13/autoscripts/preinst-sgmlcatalog debhelper-8.9.13+nmu1/autoscripts/preinst-sgmlcatalog --- debhelper-8.9.13/autoscripts/preinst-sgmlcatalog1970-01-01 01:00:00.0 +0100 +++ debhelper-8.9.13+nmu1/autoscripts/preinst-sgmlcatalog 2011-12-12 14:04:43.0 +0100 @@ -0,0 +1,7 @@ +if [ $1 = upgrade ] ! dpkg-query -S #CENTRALCAT# /dev/null 21; then + # If the dpkg-query command returns non-zero, the central catalog is + # not owned by any package. This is due to an old behaviour of + # debhelper and needs to be cleaned up. + rm -f #CENTRALCAT# + update-catalog --quiet --add --super #CENTRALCAT# +fi diff -Nru debhelper-8.9.13/autoscripts/prerm-sgmlcatalog debhelper-8.9.13+nmu1/autoscripts/prerm-sgmlcatalog --- debhelper-8.9.13/autoscripts/prerm-sgmlcatalog 2011-08-07 02:46:45.0 +0200 +++ debhelper-8.9.13+nmu1/autoscripts/prerm-sgmlcatalog 2011-12-12 12:12:07.0 +0100 @@ -1,3 +1,3 @@ -if [ $1 = remove ] || [ $1 = upgrade ]; then +if [ $1 = remove ]; then update-catalog --quiet --remove --super #CENTRALCAT# fi diff -Nru debhelper-8.9.13/debian/changelog debhelper-8.9.13+nmu1/debian/changelog ---
Bug#477751: tackling this bug
Hi Daniel, On Mon, Dec 05, 2011 at 12:05:26AM +0100, Daniel Leidert wrote: My thoughts on this are pretty easy. There are IMO three mechanisms to use: (1) Register the catalog, if it exists (and unregister any registered catalog, if it doesn't exist anymore). So users can remove the package catalog file. (2) Register the catalog only during installation, but not during upgrade. Usually we only add a catalog reference to the super catalog. This is what I proposed. It can be done today with a simple change to debhelper and no changes to sgml-base. (3) Catalog files should be written at build time not during installation. Instead of creating /etc/sgml/package.cat during installation, this should be created during package build. So the user can edit /etc/sgml/package.cat and /etc/sgml/catalog and we preserve these changes. Initially I thought about this as well. The problem with this is that currently /etc/sgml/package.cat is not owned by any package. So by switching a package to this model, each installation would prompt the user for those files. If the user now changes /etc/sgml/package.cat and we need to ship an updated file, he should usually be asked, if he wishes to update the file during installation. IMO we don't need to check, what has been disabled or not. Or does this have any advantages IYO? It works without asking the user questions during upgrade. This applies both to a transitioning period and to the long term when a package.cat changes. I don't really care whether the user is asked on package upgrades after he changes those files. But installations where no user changed those files should definitely not ask the user. So I propose that either you come up with a method to cleanly take over ownership of those configuration files or we use my approach. In any case this bug should be fixed some way or another. Helmut -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#477751: tackling this bug
Helmut Grohne wrote: Good. This would also necessitate a versioned dependency on sgml-base. Yes, easy since it already uses misc:Depends. Do you want a diff for debhelper? Would be appreciated. -- see shy jo signature.asc Description: Digital signature
Bug#477751: tackling this bug
tags 477751 +patch thanks Hi, I finally took the opportunity to work on this bug. As Joey Hess pointed out the first thing to change is update-catalog. On the other hand surely the debhelper snipped *will* have to change, because it unconditionally removes a file in /etc. So let us have a look at the current snippet: $ cat /usr/share/debhelper/autoscripts/postinst-sgmlcatalog if [ $1 = configure ]; then rm -f #CENTRALCAT# for ordcat in #ORDCATS#; do update-catalog --quiet --add #CENTRALCAT# ${ordcat} done update-catalog --quiet --add --super #CENTRALCAT# fi $ So there are two places where we do not preserve user changes. 1) Changes to the root catalog. This is due to the update-catalog --quiet --add --super. It is actually easy to solve, because it is a no-op if the catalog is already added. Thus it should only be invoked when installing the package. It should not be invoked when upgrading the package. A simple check on $2 being empty solves this issue. 2) Changes to the central catalog of the package. This is more tricky and requires changes to update-catalog. There needs to be some way to remember what catalogs the user disabled. To achieve this I changed the behaviour of --remove (see attached debdiff). It will now comment out catalogs to be removed. Now removing that catalog is no longer a good thing to do. Instead updatew-catalog needs to do something more clever. This is where update-catalog --update #CENTRALCAT# #ORDCATS# comes in. It will walk over the central catalog removing any (disabled or not) entries not found in the passed #ORDCATS#. It will not touch entries already present, but add new ones. So this should solve the issue. The new snipped would look like this: $ cat postinst-sgmlcatalog.new if [ $1 = configure ]; then update-catalog --quiet --update #CENTRALCAT# #ORDCATS# if [ -z $2 ]; then update-catalog --quiet --add --super #CENTRALCAT# fi fi $ So what are your thoughts on this? Helmut diff -Nru sgml-base-1.26+nmu1/debian/changelog sgml-base-1.26+nmu2/debian/changelog --- sgml-base-1.26+nmu1/debian/changelog2010-07-18 14:39:38.0 +0200 +++ sgml-base-1.26+nmu2/debian/changelog2011-12-04 12:50:15.0 +0100 @@ -1,3 +1,10 @@ +sgml-base (1.26+nmu2) unstable; urgency=low + + * Non-maintainer upload. + * Rework update-catalog in a way that empowers debhelper to fix #477751. + + -- Helmut Grohne hel...@subdivi.de Sun, 04 Dec 2011 12:49:07 +0100 + sgml-base (1.26+nmu1) unstable; urgency=low * Non-maintainer upload diff -Nru sgml-base-1.26+nmu1/tools/update-catalog sgml-base-1.26+nmu2/tools/update-catalog --- sgml-base-1.26+nmu1/tools/update-catalog2004-06-21 00:04:49.0 +0200 +++ sgml-base-1.26+nmu2/tools/update-catalog2011-12-04 12:41:06.0 +0100 @@ -30,6 +30,7 @@ use vars qw( $super ); use vars qw( $template ); use vars qw( $type ); +use vars qw( $update ); ## -- while ( $ARGV[0] =~ m/^--/ ) @@ -44,6 +45,10 @@ { $remove = 1; } +elsif ( $_ eq '--update' ) +{ + $update = 1; +} elsif ( $_ eq '--quiet' ) { $quiet = 1; @@ -83,10 +88,14 @@ } ## -- -if ( $add || $remove ) +if ( $add || $remove || $update ) { if ( $super ) { + if ( $update ) { + print Updating the super catalog is not supported.\n; + exit 1; + } $catalog = '/etc/sgml/catalog'; } else @@ -104,6 +113,21 @@ } ## -- +if ( $add + $remove + $update != 1) +{ +print Huh? You have to use precisely one out of --add --remove and --update.\n; +exit 1; +} +if ( $update ) +{ +print Updating $catalog...\n + unless $quiet; +read_catalog_updating_argv; +write_catalog; +exit 0; +} + +## -- $entry = shift( @ARGV ); ## -- @@ -115,13 +139,6 @@ } ## -- -if ( $add == $remove ) -{ -print Huh? You have to use --add or --remove (not both).\n; -exit 1; -} - -## -- print STDERR $name: test mode - catalog file will not be updated\n if $debug ! $quiet; @@ -131,8 +148,7 @@ print Adding entry $entry to catalog $catalog...\n unless $quiet; -read_catalog_without_entry; -add_entry; +read_catalog_enabling_entry; write_catalog; } elsif ( $remove ) @@ -140,7 +156,7 @@ print Removing entry $entry from catalog $catalog...\n unless $quiet; -read_catalog_without_entry; +
Bug#477751: tackling this bug
Helmut Grohne wrote: So what are your thoughts on this? I haven't considered all the implications... Will the new sgml-base work ok with the old postinst? With mixtures of the new and old postinsts? I'm happy moving ahead with the debhelper changes as soon as sgml-base is in unstable. -- see shy jo signature.asc Description: Digital signature
Bug#477751: tackling this bug
Hi Joey, thanks for your quick answer. On Sun, Dec 04, 2011 at 05:25:42PM -0400, Joey Hess wrote: I haven't considered all the implications... Will the new sgml-base work ok with the old postinst? With mixtures of the new and old postinsts? Good question! Let's look at them individually. The old postinst just uses --add which is idempotent (old and new). So this should all work out. The old postrm just does rm and the old prerm does --remove on the root catalog. This should work as well (even though it now leaves a comment). The prerm however needs updating as well, as it should no longer --remove on upgrade (as we can no longer --add on upgrade). I'm happy moving ahead with the debhelper changes as soon as sgml-base is in unstable. Good. This would also necessitate a versioned dependency on sgml-base. Do you want a diff for debhelper? Helmut -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#477751: tackling this bug
Am Sonntag, den 04.12.2011, 13:06 +0100 schrieb Helmut Grohne: [..] So what are your thoughts on this? My thoughts on this are pretty easy. There are IMO three mechanisms to use: (1) Register the catalog, if it exists (and unregister any registered catalog, if it doesn't exist anymore). So users can remove the package catalog file. (2) Register the catalog only during installation, but not during upgrade. Usually we only add a catalog reference to the super catalog. (3) Catalog files should be written at build time not during installation. Instead of creating /etc/sgml/package.cat during installation, this should be created during package build. So the user can edit /etc/sgml/package.cat and /etc/sgml/catalog and we preserve these changes. If the user now changes /etc/sgml/package.cat and we need to ship an updated file, he should usually be asked, if he wishes to update the file during installation. IMO we don't need to check, what has been disabled or not. Or does this have any advantages IYO? Regards, Daniel -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org