Re: Reworking the Apache package (with patches)
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi,
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'" >>
/etc/logrotate.d/httpd-prerotate/testme
# 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
rotations)
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)
postinst:
<snip>
# Automatically added by dh_installinit
if [ -x "/etc/init.d/apache2" ]; then
update-rc.d apache2 defaults 91 09 >/dev/null || exit $?
fi
# End automatically added section
</snip>
Back in Lenny, Apache used the following code in prerm:
<snip>
# 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
else
/etc/init.d/apache2 stop || true
fi
fi
chmod -x /usr/sbin/apache2
fi
</snip>
and this in postinst:
<snip>
# Automatically added by dh_installinit
if [ -x "/etc/init.d/apache2" ]; then
update-rc.d apache2 defaults 91 09 >/dev/null || exit $?
fi
# End automatically added section
</snip>
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
"accidentally".
> 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
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iQIcBAEBAgAGBQJO59fjAAoJEMcrUe6dgPNtADAQAJpMcyHSoGdoYVUbqaegbnH6
Z4GMqiP8qUmM1Sac15W7J2WskB7U1BjKpEn/zd3iJWC1f7IF+1z8RsOyp+XNUxWG
G1dXnO9UynJyFsjcoaHthOB7g5I/NCdzOqEv2opE8/RzmznDUVDc+Zm7hAnVftfB
5T1oFJ62fL71/mRO0jx+LLzpr/nEapjcRKf4QWoI5QV2hn0f5uPocBD0htbxkle0
REVzfub9uEZd6SecwARi9BGUKGIjKyaUYARXl3Ok8uB280JE6NlXhz+apgtpr1OB
7ZqCvWYFT19Nh9G5xEA+pir1WpjEbD2rdJnqyVv8AT4i2QvMBP7CrTSntDXfGsGL
MsUl2j/JqyCHQDBYv6EkreVaPshqOYbeUUEFQKUITrYyQnLBGl10Gj8f5ACQMr4P
NAy8JjhKRJKQijIwIHXIGPRqwGoAKx8vTdXOn+DI/R+NQ7zILY09nX3+/aJXaDfS
6/QLMjZ4BbNQ9oU7cfHfyO8nbdFX44ZygogbMnBMbJOKNYjGYzUmzg+I7AhrCQY8
bIMF647vMqC/A7XCwlCTi5AKNBqC5ka/ZX0qmmVT61Aj6vjfLZGw9ParR2NCx+zD
R29CqCkDkgXdgcP8F9gjP4FdJdzpie1gCi135TRrP2uEe59BFnzWtiQh0tPaXF4+
yOpQ3WswYav0tPSb0hOM
=+wvj
-----END PGP SIGNATURE-----
Reply to: