[Piuparts-devel] Bug#652934: Bug#652934: status

[Dave Steele]

On Fri, Jun 1, 2012 at 4:31 AM, Holger Levsen <holger at layer-acht.org> wrote:
>
> Hi Dave,
>
> Thanks, so I've merged the next one... but then I looked at
> e6444734909a6074f9f768f0c32ffc56d3a2017e and found several points I disliked,
> want improved or plainly reject:
>

OK, I think I can return to this productively now.

Holger, you asked about background on ht_root. It is a loose reference
to 'http root'.  Apache defaults to the convention that
http:///~<user>/ is served out of the directory ~<user>/ht_root. As
Andreas hinted, the analogy to my usage is imperfect. In any case, the
issue is moot. I had changed the variable name per your direction
before the discussion came up, and I have a one
large-variable-renaming-per-customer-per-variable limit.

You asked about what "missing templates" I was referring to in an
earlier email on packaging. I have had many "analysis template *.tpl
does not exist" errors in my piuparts-report output, and no issue
summaries in the web output, and chalked that up to cause and effect.
I see now that they originate elsewhere. Lately, the web summaries
have started showing up. I don't know why. (the latest run has
summaries, but no summary counts - odd)

The new summaries revealed 6b24129 - detect-well-known-errors - use
relative addressing for log links.

>
> - please keep the package in 1.0 format. 3.0 is more complicated and buys us
> nothing as we dont have any patches.

I don't understand why 3.0 (native) is more complicated.

    http://www.debian.org/doc/manuals/developers-reference/pkgs.html#sourcelayout

7bc9779 - Use '1.0' format for the packages

The change added a .git lintian warning for me.

>
> - the README-server.txt (yay for writing it!) says that piuparts stores its
> logs in /var/lib/piuparts. Please add a sentence explaining that those logs
> are basically the result of piuparts running, thats why we store them in
> /var/lib.
>
> I also wonder whether we should store the master+slave logs in
> /var/log/piuparts/...

1515eee - Update server README to explain the logs in /var/lib/piuparts

As you point out, the logs don't really belong there. There are a
number of topics to be addressed for maturing the packaging which
conflict with 'treading lightly' on this branch. I recommend putting
them off to a future release.

>
> - I don't like (/usr)/sbin/slave_run - IMO that should either end up in
> /usr/share/piuparts/master/ or rename it to piuparts_slave_run and put it in
> /usr/sbin/. I think I prefer the later.
>

valid. changed in 43ddbf7 - Rename the public links to slave_run/join
to piuparts_slave_*
      and place in /usr/sbin.

>
> - piuparts-server.postinst: thats totally wrong, like this it would be
> executed on any upgrade, removal, purge, etc. You need to interpret $1 and act
> accordingly... see
> http://www.debian.org/doc/debian-policy/ch-maintainerscripts.html and postinst
> scripts of your choice :-)
>

I read the policy before writing the script, and again now. I
understand that what I submitted doesn't match the usual convention
for postinst scripts. But, honestly, other than an unchecked reference
to ssh-keygen, I don't see a policy violation in it. In any case, as
best I can tell from the reference, the script isn't called for
removal, purge, etc.

77f08f0 - Add argument support to piuparts-server.postinstall

I haven't done a fresh install with this.

> - why oh why do you reintroduce piuparts-server.preinst which you removed
> (albeit named preinst) in 89926a72e5675218abfbbd6dce50dba0292c43ad? I'm not
> impressed :/
>

It was code that managed "/etc/piuparts/piuparts.conf", moved to the
package that owns that file.

b94d703 - Remove obsolete piuparts-server.preinst

> - debian/rules: please explain:
>
> -       dh_installman
> -       $(MAKE) prefix=$(CURDIR)/debian/piuparts/usr
> etcdir=$(CURDIR)/debian/piuparts/etc all
> +       $(MAKE) prefix=$(CURDIR)/debian/tmp/usr
> etcdir=$(CURDIR)/debian/tmp/etc all
> +       dh_install
> +       dh_installman --sourcedir=debian/piuparts
>

dh_install supports the creation of multiple packages. It expects the
files to be in debian/tmp by default.


I've also added a few patches resulting from issues I ran across
setting up a new server.

b4849f3 - sudoer files with a '.' are invalid. Change file to 'piuparts'
0ce198d - Add server Depends on 'screen'

and this one from 'multiple-slaves':

c43c9eb - Slave working directory owned by piupartss

>
> And then finally, I must say I'm disappointed by debian/control, just
> introducing a piuparts-server package. I thought we would get piuparts-master
> and a piuparts-slave packages instead 8-)
>

We've looked that this issue and have come to different conclusions on
how packaging should be accomplished. You haven't said why you have a
strong preference for one strategy over the other. I'd like to
understand what your drivers are for wanting the master and slave to
be separate.