[pkg-bacula-devel] Merging the 'development' branch: 01-3f141b4 -> 04-19ae64a (+ 11-123a8b6)

Jan Hauke Rahm jhr at debian.org
Mon May 7 20:02:04 UTC 2012


On Wed, May 02, 2012 at 04:37:01PM +0200, Luca Capello wrote:
> * commit 3f141b49834c96def9df8ed33ab2efb56eefea6e
>   Author: Jan Hauke Rahm <jhr at debian.org>
>   Date: Wed, 13 Apr 2011 16:52:29 +0200
>   Subject: [PATCH 01/39] Revert changes made without using patch system
> 
> * commit 8bea4b398094758a1cc7338bd3ccd8324e80ebce
>   Author: Jan Hauke Rahm <jhr at debian.org>
>   Date: Wed, 13 Apr 2011 15:24:49 +0200
>   Subject: [PATCH 02/39] Rework patching to use 3.0 (quilt)
> 
> These were highly needed, but it seems that the situation once the
> quilt patches are applied is not the same as before the first commit,
> which renders both patches difficult to be read.  Examples are
> scripts/logwatch/logfile.bacula.conf, which creation is commented out
> in debian/patches/fix-default-config, and s/hostname/debian_hostname/
> in debian/additions/postinst-common and subsequent scripts.
> 
> QUESTION: what is the rationale for using '@debian_hostname@' instead
>           of relying on upstream '@basename@' or '@hostname@'?  We
>           should diverge from upstream only if needed, and in this
>           case I do not see any real advantage.

I honestly don't remember. Thinking back, I assume that during later
commits @basename@ broke something and, to have less patches touch the
same lines over and over again, I made that change in an earlier commit.
But I really don't remember, sorry.

That left aside, please note that I consider anything like this broken
anyway. First replacing all @hostname@ with the building machine's
hostname and later on replacing that particular hostname with a default
for the config files is not only insane, it's also broken. My default
build host's name is 'ca'. You can imagine how many lines were messed up
when the scripts tried to replace 'ca' even within words.

One reason why I stopped working was that the build system was so broken
that I couldn't support any other upload with it. The work I did was to
restructure all of it. Taking singular patches will make it better but
it's still just crap as long as those hacks are even in it. There is no
way to assume a stable build environment if we rely on stupid build
hostnames.

> Hauke, please note that this is an example of why I do not like
> Git-styled debian/changelog [2]: if instead you write there an entry
> each time you modify a file in the debian/ folder, then you do not need
> to explain what your Git commit does.  The fact that there is no
> explanation in debian/changelog and in the Git commit itself either is a
> pity whenever someone wants to understand what is going on.  Just to be
> clear: I fully agree with the commit, it is just that I prefer
> self-contained and self-explaining commits :-)

I don't get how you figure my changelog entries would have been any
better or more-explaining than my git commits. I'm sorry for crappy
commit messages but I assure you, I would most definitely never have
written better one-liners in yet another file. :)

Having said that, I still think that changelog changes within the
technical commits produce, before anything, just a branch-and-merge hell
as *every* merge process will lead to conflicts in d/changelog. If I had
written proper git commit messages (And I'm sorry if you feel I didn't),
you could *at any point* in the commit history let git-dch produce a
changelog entry and release *that point* while still not having to deal
with conflicts in later commits. That's a feature of git. If you don't
want it, you could've used subversion. :)

> [2] <mid:20111030093228.GA23955 at co.mobile.jhr-online.de>
>     <http://lists.alioth.debian.org/pipermail/pkg-bacula-devel/2011-October/000093.html>
> 
> Nevertheless, I recorded your changes in debian/changelog:
> 
>   * commit 71390091a6a5709d6f8cd892d16fcf36c006bd53
>     Author: Luca Capello <luca at pca.it>
>     Date: Sat Mar 31 23:45:19 2012 +0200
>     Subject: debian/changelog: add Hauke's changes up to 8bea4b3
> 
> the output of debdiff seems OK and the list of files
> contained in all .deb [3] is identical to the one generated from
> commit 9e5f75495f791a1846c4c9932d3420f9134c72a4.  Each single file in
> /etc [4] was identical between the two versions as well.
> 
> [3] from the directory containing all your .deb:
>       $ mkdir $VERSION; cd $VERSION; find ./ -type f >../$VERSION.list
> [4] from the directory $VERSION_1:
>       $ for I in $(find etc/ -type f); do \
>             diff -u "$I" ../$VERSION_2/"$I"; \
>         done
> 
> However, there were differences at least in files in /usr/share:
> 
> a) SQL additions not included in any patch
> --8<---------------cut here---------------start------------->8---
> --- usr/share/bacula-director/update_postgresql_tables	2012-03-05 22:24:06.000000000 +0100
> +++ ../5.0.3-2.dev+2/usr/share/bacula-director/update_postgresql_tables	2012-03-08 20:50:08.000000000 +0100
> @@ -69,6 +69,8 @@
>  -- CREATE INDEX CONCURRENTLY file_jpf_idx ON File (JobId, PathId, FilenameId)
>  -- to make it without locks (require PostgreSQL 8.2 version)
>  
> +CREATE INDEX file_jpfid_idx on File (JobId, PathId, FilenameId);
> +
>  ANALYSE;
>  
>  END-OF-DATA
> --- usr/share/bacula-director/update_mysql_tables	2012-03-05 22:24:09.000000000 +0100
> +++ ../5.0.3-2.dev+2/usr/share/bacula-director/update_mysql_tables	2012-03-08 20:50:10.000000000 +0100
> @@ -15,6 +15,7 @@
>  if mysql $* -f <<END-OF-DATA
>  USE ${db_name};
>  
> +ALTER TABLE JobMedia DROP Stripe ;
>  ALTER TABLE JobMedia DROP Copy ;
>  ALTER TABLE Job ADD COLUMN HasCache tinyint default 0 after HasBase;
>  ALTER TABLE Job ADD COLUMN Reviewed tinyint default 0 after HasCache;
> --8<---------------cut here---------------end--------------->8---
> 
>   These missing changes were then recorded in the following commit:
> 
>     * commit 19ae64aebb0cbdf1710d079b530a0e8a108ba600
>       Author: Jan Hauke Rahm <jhr at debian.org>
>       Date: Wed, 13 Apr 2011 17:02:16 +0200
>       Subject: [PATCH 04/39] Disable unclear patch about SQL stuff
> 
>   The PostgreSQL fix is not in upstream 5.0.3, so you should have
>   removed that as well.  However, it is still needed because of:
> 
>     * commit 580680e3ea06bb1a8e73accddc950990a66ea1d4
>       Author: John Goerzen <jgoerzen at complete.org>
>       Date: Fri Aug 20 14:57:02 2010 -0500
>       Subject: Remove file_jpfid_idx creation on upgrade
>     
>       Closes #591293
>       See http://bugs.bacula.org/view.php?id=1623
> 
>   Which actually got fixed upstream with commit:
> 
>     * commit fc7990abb396b0f0bd488e948cf97f1444d63415
>       Author: John Goerzen <jgoerzen at complete.org>
>       Date: Tue Sep 21 10:25:27 2010 -0500
>       Subject: applied patch from http://bugs.bacula.org/view.php?id=1623#c5605
> 
>   But the next commit erroneously removed it:
> 
>     * commit 63e0842522cc31b10233b114ba747879cac57907
>       Author: gregor herrmann <gregoa at debian.org>
>       Date: Sat Nov 27 17:18:41 2010 +0100
>       Subject: Imported Debian patch 5.0.2-2.1
> 
>   Given that this has been included in upstream version 5.2.1, we need
>   it if we keep using older versions, re-added from Git format-patch as
>   debian/patches/upstream-1623_debian-591293___file_jpfid_idx.patch
>   (after having discovered that dpkg-source does not accept patches with
>   fuzz, see #666752):
> 
>     * commit a64dee5c2199af135e7f91978c61909829756d0b
>       Author: Luca Capello <luca at pca.it>
>       Date: Sun Apr 1 21:47:23 2012 +0200
>       Subject: debian/patches/upstream-1623_debian-591293___file_jpfid_idx.patch
>     
>       This restores fc7990a, a fix for PostgreSQL lost with 63e0842 and
>       partially restored with 8bea4b3, but still needed until upstream
>       version 5.2.1.
> 
>   Next one.  As you discovered later, the MySQL fix had a clear reason:
> 
>     * commit 123a8b6782486f34717e83a33044dd4aaa089db3
>       Author: Jan Hauke Rahm <jhr at debian.org>
>       Date: Mon Apr 18 13:17:29 2011 +0200
>       Subject: [PATCH 11/39] Re-enable mysql upgrade patch
>     
>       See #569285 and http://bugs.bacula.org/view.php?id=1498 for more info
>       why this is needed on upgrades 3.0 -> 5.0. Actually, since 5.0 is in
>       stable, this could be removed now but let's keep it until next bacula db
>       version to be safe.
> 
>   The related commit by John contained more than the single MySQL fix:
> 
>     * commit 0541ba6d8725f2d891b09934a227a835859be10c
>       Author: John Goerzen <jgoerzen at complete.org>
>       Date: Mon Feb 22 11:12:44 2010 -0600
>       Subject: Fix MySQL upgrade removing Stripe in JobMedia
> 
>       ---
>        debian/bacula-director-mysql.dirs         |    1 +
>        debian/bacula-director-mysql.script.5.0.0 |   62 +++++++++++++++++++++++++++++
>        debian/rules                              |    3 +
>        src/cats/update_mysql_tables.in           |    1 -
>        4 files changed, 66 insertions(+), 1 deletions(-)
>        create mode 100644 debian/bacula-director-mysql.script.5.0.0
> 
>   So, if we reinstate the MySQL fix we should do the same for the
>   upgrading script as well.  However, given that bacula_5.0.0 was
>   already in lenny and that Debian supports upgrades only to consecutive
>   releases, this fix is now useless.  Moreover, the upstream bug has
>   been closed with 'Resolution: not fixable' and the following comment
>   by the main upstream developer:
> 
>     kern (administrator) 2010-04-10 13:46
> 
>     Unfortunately, I cannot think of any fix other than modifying the
>     Debian installation process to ignore this particular error.
> 
>     If you have a solution, please let us know. 
> 
>   For the reasons above, I simply ignored the MySQL stuff.
> 
> b) missing changes for s/hostname/debian_hostname/
> --8<---------------cut here---------------start------------->8---
> --- usr/share/bacula-common/defconfig/bconsole.conf	2012-03-05 22:24:15.000000000 +0100
> +++ ../5.0.3-2.dev+2/usr/share/bacula-common/defconfig/bconsole.conf	2012-03-08 20:50:05.000000000 +0100
> @@ -3,8 +3,8 @@
>  #
>  
>  Director {
> -  Name = @hostname at -dir
> +  Name = gismo-dir
>    DIRport = 9101
>    address = localhost
> -  Password = "WfCX4v8r+QCvMbAyT/F3T6+ADlX2/xPEHHpMv/KDlROK"
> +  Password = "hqZxVIBhdCi+Z1WdgfQQgA5QDNLQT1N1qOdDIn218k3m"
>  }
> --- usr/share/bacula-common/defconfig/tray-monitor.conf	2012-03-05 22:24:15.000000000 +0100
> +++ ../5.0.3-2.dev+2/usr/share/bacula-common/defconfig/tray-monitor.conf	2012-03-08 20:50:05.000000000 +0100
> @@ -3,27 +3,27 @@
>  #
>  
>  Monitor {
> -  Name = @hostname at -mon
> +  Name = gismo-mon
>    Password = "XXX_MONDIRPASSWORD_XXX"         # password for the Directors   
>    RefreshInterval = 5 seconds
>  }
>     
>  Client {
> -  Name = @hostname at -fd
> +  Name = gismo-fd
>    Address = localhost
>    FDPort = 9102
>    Password = "XXX_MONFDPASSWORD_XXX"          # password for FileDaemon
>  }
>  
>  Storage {
> -  Name = @hostname at -sd
> +  Name = gismo-sd
>    Address = localhost
>    SDPort = 9103
>    Password = "XXX_MONSDPASSWORD_XXX"          # password for StorageDaemon
>  }
>  
>  Director {
> -  Name = @hostname at -dir
> +  Name = gismo-dir
>    DIRport = 9101
>    address = localhost
>  }
> --- usr/share/bacula-common/defconfig/bat.conf	2012-03-05 22:24:15.000000000 +0100
> +++ ../5.0.3-2.dev+2/usr/share/bacula-common/defconfig/bat.conf	2012-03-08 20:50:05.000000000 +0100
> @@ -3,8 +3,8 @@
>  #
>  
>  Director {
> -  Name = @hostname at -dir
> +  Name = gismo-dir
>    DIRport = 9101
>    address = localhost
> -  Password = "WfCX4v8r+QCvMbAyT/F3T6+ADlX2/xPEHHpMv/KDlROK"
> +  Password = "hqZxVIBhdCi+Z1WdgfQQgA5QDNLQT1N1qOdDIn218k3m"
>  }
> --8<---------------cut here---------------end--------------->8---
> 
> This means that the above files will not work out of the box, because
> the build hostname is hardcoded, thus sed command in bacula-console's
> postinst will not change anything.
> 
> These are caused by the fact the command in debian/rules:install-arch
> that fixed the above files were removed by the second commit (i.e.
> 8bea4b398094758a1cc7338bd3ccd8324e80ebce), which also removed
> debian/patches/fix_[config|director|scripts](.sed).
> 
> Again, this is more than the simple "Rework[ing] patching to use 3.0
> (quilt)": you changed the final result and there is no documentation
> about the rationale.  Never mind, I recorded the missing changes:

As said above, the build system is more than broken. Relying on it is
dangerous and wrong in my opinion. Whatever fix you apply to it, it'll
never be good. I wanted to get rid of it and thus later didn't care
anymore about these useless changes.

>   * commit ab5fc635a1fb5fbc44b48e84351e45e8ba44960c
>     Author: Luca Capello <luca at pca.it>
>     Date: Wed May 2 14:30:45 2012 +0200
>     Subject: debian/patches/fix-default-config: missing basename replacements
>     
>     This complements 8fd8ebd.
> 
> However, I see a problem with the old and new approaches, given that
> they mix basename and hostname and also because upstream configure
> permits specifying both:
> =====
> $ ./configure --help | grep RESNAME
>   --with-basename=RESNAME specify base resource name for daemons
>   --with-hostname=RESNAME specify host name for daemons
> $ 
> =====
> 
> Debian decided to always use localhost has the default hostname, so we
> should specify that to ./configure (in my ToDo list):
> 
>   * commit 7f9cf0d67b46e48722021bf83c81b5e614b5b799
>     Author: John Goerzen <jgoerzen at complete.org>
>     Date: Tue Mar 13 11:04:48 2007 -0500
>     Subject: Consoles connect to localhost by default
> 
>     Previously the result of `hostname` was being used
>     since the director only listens on 127.0.0.1 by default, this didn't work
>     so well by default
>     
>     closes deb#404868
> 
> 
> 
> * commit e87c61bde39e1b0ea9325e55363184108ccc84be
>   From: Jan Hauke Rahm <jhr at debian.org>
>   Date: Thu, 14 Apr 2011 12:11:48 +0200
>   Subject: [PATCH 03/39] Leave patches unapplied by dpkg-source
> 
> QUESTION: what is the rationale for unapply-patches in
>           debian/source/local-options?

When I put that in there, dpkg did not automatically unapply patches
when applied right before building. Patches were thus applied in the git
repo which was overly annoying. dpkg was changed. This commit is thus
rendered obsolete.

> Even after having read the dpkg-source manpage, I still fail why we need
> such an option:
> 
> --8<---------------cut here---------------start------------->8---
> dpkg-source(1) -- 2011-08-14 -- Debian Project -- dpkg utilities
> [...]
> SOURCE PACKAGE FORMATS
> [...]
>    Format: 3.0 (quilt)
> [...]
> 	Build options
> [...]
> 	--unapply-patches
> 		Unapply  the patches  in  the  --after-build hook.  You
> 		usually  don't need  this  option  as dpkg-source  will
> 		automatically unapply the patches  if it did apply them
> 		during --before-build.  This option is only  allowed in
> 		debian/source/local-options   so  that   all  generated
> 		source packages have the same behavior by default.
> --8<---------------cut here---------------end--------------->8---

Thank you and all those who recently jumped in here for taking care of
this. I'm truly sorry for having started enthusiastically and then just
stoped. Much changed at work, not only related to our use of bacula, and
I don't see how I can be of much help anymore. I'll stick around and try
to read the mail on the list as you're in part discussing my old
commits. I hope I can be of help with that at least -- even if I'm slow.

Hauke

-- 
 .''`.   Jan Hauke Rahm <jhr at debian.org>               www.jhr-online.de
: :'  :  Debian Developer                                 www.debian.org
`. `'`   Member of the Linux Foundation                    www.linux.com
  `-     Fellow of the Free Software Foundation Europe      www.fsfe.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-bacula-devel/attachments/20120507/cb25a179/attachment.pgp>


More information about the pkg-bacula-devel mailing list