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

Re: Reworking the Apache package (with patches)

Hash: SHA1


I'm sorry for my delayed reply. I was busy over the last few days. I
just committed my changes to the repository. I hope I didn't break
anything as I was using git-svn to have local commits and branches for
my experimental work.

> I have no problems in principle. But are you sure that logrotate
> won't get confused about a sub-directory in /etc/logrotate.d/? It
> interprets all files in that dir as config files and in my
> experience, you can't assume that logrotate handles its config in a
> sane way. We could use a subdir in /etc/apache2 instead.

Yes, I verified that. Please see the following proof of concept:

# echo -e "#! /bin/sh\necho 'hello world'" >>
# chmod a+x /etc/logrotate.d/httpd-prerotate/testme
# run-parts /etc/logrotate.d/httpd-prerotate;
hello world
# /etc/init.d/apache2 start
Starting web server: apache2.
# /usr/sbin/logrotate -f -d /etc/logrotate.conf
reading config file /etc/logrotate.conf
including /etc/logrotate.d
reading config file apache2
reading config file apt
reading config file aptitude
reading config file dpkg
Ignoring httpd-prerotate because it's not a regular file.

Handling 8 logs

rotating pattern: /var/log/apache2/*.log  forced from command line (52
empty log files are not rotated, old logs are removed
considering log /var/log/apache2/access.log
  log needs rotating
considering log /var/log/apache2/error.log
  log needs rotating
considering log /var/log/apache2/other_vhosts_access.log
  log does not need rotating
rotating log /var/log/apache2/access.log, log->rotateCount is 52
dateext suffix '-20111213'
glob pattern '-[0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]'
previous log /var/log/apache2/access.log.1 does not exist
running prerotate script
running script (multiple) with arg /var/log/apache2/*.log : "
                if [ -d /etc/logrotate.d/httpd-prerotate ]; then \
                        run-parts /etc/logrotate.d/httpd-prerotate; \
                fi; \
renaming /var/log/apache2/access.log to /var/log/apache2/access.log.1

You can see, logroate recognizes the file as directory and skips it
entirely. The files in it are then executed by run-parts as desired.

>> * I fixed #440058 by changing the DefaultType. That change seems 
>> feasible.
> Not sure. Upstream leaves this untouched in 2.2.x but changes it in 
> 2.4. Maybe we should do the same.

I committed it as is now to ease the import. I'll revert that change if
you don't want to have DefaultType set to none.

>> #557612 During package upgrade, apache2 is stopped, but not
>> started * dh_installinit already has --no-restart-on-upgrade (I
>> made that more clear) this avoids the problem entirely * fixed
>> already, thus close
> I don't think that it is fixed. IIRC the --no-restart-on-upgrade is 
> because the restarting is done in a different package than the one 
> that contains the init script. Therefore debhelper's auto-generated 
> scriptlets wouldn't work.

That's one rationale, but this also causes dh_installinit not so install
any debhelper hook to stop or start the script upon upgrade. As you
said, we couldn't do so anyway, as we have no guarantee the -bin package
is already installed and thus the init script itself would fail. The
scriptlets itself would fairly work fine.

However, there is also a substantial difference. Typically
dh_installinit stops the daemon in prerm and starts it again after
configuring. The -r switch now does not call the init script at all. I
am pretty sure this works around the problem (albeit it does not really
fixes it). These are our scripts:

(no prerm script)

# Automatically added by dh_installinit
if [ -x "/etc/init.d/apache2" ]; then
        update-rc.d apache2 defaults 91 09 >/dev/null || exit $?
# End automatically added section

Back in Lenny, Apache used the following code in prerm:

# This is an evil hack around the fact that dpkg is unpacking in the
# wrong order causing "stop" to fail.

if [ -z "$2" ] && [ -e /usr/sbin/apache2 ]; then
    if [ -x "/etc/init.d/apache2" ]; then
        if [ -x /usr/sbin/invoke-rc.d ]; then
                invoke-rc.d apache2 stop || true
                /etc/init.d/apache2 stop || true
    chmod -x /usr/sbin/apache2

and this in postinst:

# Automatically added by dh_installinit
if [ -x "/etc/init.d/apache2" ]; then
        update-rc.d apache2 defaults 91 09 >/dev/null || exit $?
# End automatically added section

You see, it does indeed stop the server upon upgrade, but does not start
it again. Whoever removed that prerm script back then also fixed #557612

> But I am not sure that we should fix it, either. I would prefer a
> Debian global solution or at least policy. Do you know if there are
> any other daemons that do this?

Do what exactly? Restart itself on upgrade? More or less much every
daemon does that (since this is dh_installinits default).

> Not yet. All reverse deps will need to be changed with 2.4, anyway, 
> and we should try that they only need to be changed once. Besides, 
> with 2.2 and non-trivial auth configuration, having access denied on
> / by default either does not do much good or is a PITA. 2.4's
> auth/authz handling is way more flexible.

Fair enough, will leave that as is for now then.

> BTW, I had TODO lists in the wiki for Lenny and Squeeze [1]. Do you
> think it would make sense to have that for Wheezy again?

It wouldn't hurt at least. However, I am not sure what the target
audience is? Isn't everyone contributing to Apache supposed to comment
on threads like this anyway?

> An alternative may be to make override_dh_auto_configure, 
> override_dh_auto_build, and override_dh_auto_install do basically 
> nothing, put all the logic in a single "build-and-install-%" pattern 
> rule and then build the four build-and-install-* targets with another 
> make invocation that enables parallel build. I have attached a proof-
> of-concept diff to illustrate the idea. This is not really beautiful 
> but reduces the build time to 1:55. A bit more reduction should be 
> possible by not copying the source tree for the non-itk MPMs.

I applied and committed your patch. It does indeed parallelize the whole
build process just fine, whereas mine only parallelized the build
itself, not configure.

I don't think your patch is that ugly anyway. It is a bit uncommon, but
oh well, that holds for the entire rules file. That said I truly believe
it is more readable now. The install target is still a major
construction ground though. I consider using the new debhelper feature
to have executable install files to solve that problem [1].

If you want, I can spend more work in the patch. Basically I'd see
whether I can achieve parallelization by having the build target a bit
more readable.

> The "@set -ex" calls are mostly useless, because they only affect the 
> current shell. You would have to add a line continuation backslash to 
> make them affect whatever script comes after.

I think I removed them all, or put them into a shell sequence.

> The first line of the auto-install-% rules begins with spaces while it 
> should be a tab.

I think you fixed that in your patch.

> The "cp debian/default-index.html ..." line is duplicate.

Fixed that.

> We don't need DH_VERBOSE=1 by default.

Whoops! You're right, I tend to forget that often :)

> We can probably get rid of the config.sub/config.guess patch by using 
> "dh --with autotools_dev". I just saw that in the man page, but 
> haven't tried it. Would be nice, though.

Yes, I think so too. I'll have a look on that soonish.

> If you want to have a verbose description in the rules file, you 
> should probably mention in paragraph 2 that some develpment files are 
> also taken from prefork mpm.

I did. I find it good practice to document non-trivial rules files.
However, if you want I can also remove the comments entirely.

> For the icons dir, "Options -Indexes -MultiViews" means "take whatever 
> the Options were, but disable Indexes MultiViews". I think "Options 
> FollowSymlinks", meaning "only enable FollowSymlinks", is more 
> appropriate/predictable.

Fixed that.

> So, feel free to commit to svn. I don't mind if you fix the issues 
> first, or commit the current state right away. You should also add 
> yourself as Uploader.

Did that too. Without introducing new regressions I hope.

[1] http://kitenet.net/~joey/code/debhelper/news/version_8.9.12/

- -- 
with kind regards,
Arno Töll
IRC: daemonkeeper on Freenode/OFTC
GnuPG Key-ID: 0x9D80F36D
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/


Reply to: