etckeeper hook breaks upgrade with "Argument list too long"

[Mekk]

Binary package hint: etckeeper

I just executed distribution upgrade from 9.04 to 9.10.

The upgrade failed nearby the end because of etckeeper failure. I don't remember exact message but it was about command line being too long (I suspect bzr command was executed with all changed etc files, which are numerous)

Definitely deserves some thought.

ProblemType: Bug
DistroRelease: Ubuntu 10.04
Package: etckeeper 0.41ubuntu3
ProcVersionSignature: Ubuntu 2.6.31-21.59-generic
Uname: Linux 2.6.31-21-generic x86_64
NonfreeKernelModules: nvidia
Architecture: amd64
Date: Mon May 3 09:21:23 2010
InstallationMedia: Ubuntu 9.10 "Karmic Koala" - Release amd64 (20091027)
PackageArchitecture: all
ProcEnviron:
 PATH=(custom, no user)
 LANG=en_US.UTF-8
 SHELL=/bin/bash
SourcePackage: etckeeper

== SRU Report ==
Impact:
If the user has thousands of packages installed and upgrades to a new release, etckeeper tries to commit after the apt run with a commit message that is too long, resulting in an upgrade failure.

Development branch fix:
Version 0.46 fixed the issue by adding support for --stdin and making etckeeper use it for post-install.d commit messages, so it is no longer subject to the issue.

Minimal patch:
Backporting the fix from 0.46 would result IMHO in a significant patch that also introduces features (--stdin), so this SRU proposes to truncate the commit message. See attached in comment 10.

TEST CASE:
This is relatively tricky to reproduce, since this requires to have a very large number of packages installed. Here is a way to trigger the bug:
- Install etckeeper
- Prepare a /tmp/testfile with more than 128K of data (copy a log file) (for example cp /var/log/udev /tmp/testfile)
- Start upgrading packages
- In parallel, once the packages are downloaded and the /var/cache/etckeeper/packagelist.pre-install is generated, replace it by your testfile (cp /tmp/testfile /var/cache/etckeeper/packagelist.pre-install)
- Wait for the upgrades to complete
This generates an artificially long set of differences in packages installed and will trigger the bug.
Without fix: upgrade fails at the end with "Argument list too long"
With fix: upgrade does not fail at the end, etckeeper commit message is truncated.

Regression potential:
The patch truncates the commit message only in the cases where it would trigger an "argument too long" error, so it should not create a regression.

[pklaus (pklaus)]

I can confirm this bug. I attached a screenshot of the failed upgrade.

I resolved it by manually commiting the configuration to etckeeper and then doing a partial upgrade (that worked well then).

The title of the bug is misleading. It should be 9.10 → 10.04, shouldn't it?

[Alex Muntada (alex.muntada)]

The last command in "/etc/etckeeper/post-install.d/50vcs-commit" does:

etckeeper commit "$(printf "$message")"

and $message is a very large string, larger than the shell permits. Either the commit message is shortened or etckeeper is modified to use an option to specify a log file.

[Daniel Hahler (blueyed)]

Thanks for your investigation, I've just forwarded it to Debian and will link it later.

[Thierry Carrez (ttx)]

This was fixed in 0.46 by adding support for --stdin and making etckeeper use it for post-install.d commit messages. Maybe that's slightly too featureful for a lucid SRU though, and truncating the existing message is a smaller fix (like keeping the 2000 first lines of the list-installed | diff output).

Reducing impact since this won't affect people with < 3000 packages installed, which should be the majority.

[Thierry Carrez (ttx)]

Proposed patch

[Thierry Carrez (ttx)]

Fix uploaded to lucid-proposed for your consideration.

[Daniel Hahler (blueyed)]

Well, shouldn't this rather count the number of bytes when trimming?

Currently it assumes some average package name length, and might still fail.
Also, it might crop although cropping isn't necessary.

To reproduce this easier, some fake packages with very long names might get used.

[Thierry Carrez (ttx)]

Yep, I realized this while working on the SRU report... The average package name length is very conservative so it probably wouldn't still fail, but it definitely would crop although cropping is not necessary. I'll upload something that uses head -c instead.

Daniel: could you setup a system that allows to validate the fix ? That could come in handy when it comes to SRU validation.

[Thierry Carrez (ttx)]

New proposed patch, to avoid abusive truncation.

[Thierry Carrez (ttx)]

Better fix uploaded to lucid-proposed for your consideration.

[Arvid Norlander (vorpalblade)]

How does one fix a system affected by this issue? I have here a presumably broken upgrade due to hitting this bug. A "partial upgrade" was mentioned in one comment, but how does one perform that?

[Michael Nagel (nailor)]

it should be sufficient to
cd /etc
and do a manual commit like
sudo etckeeper commit "some reasonable message"

[Arvid Norlander (vorpalblade)]

Does that mean that there will be no issues with the package manager that needs to be resolved after the crash? Like cleaning up phase (if I understood the upgrade screen correctly it crashed at the end of the stage before "clean up" [note this is backtranslated from Swedish l10n, might be called something else in the English version]).

[Michael Nagel (nailor)]

even though i cannot guarantee, i do believe so. for me it crashed as you described, and i did what posted and things seem to be allright afterwards.

[Thierry Carrez (ttx)]

The etckeeper post-install command that fails is run at the very end of the apt run (here, at the very end of the update) so that should not leave things in a broken state. It just fails to commit the /etc changes in the etckeeper bzr repository. As the previous commenter said, doing "sudo etckeeper commit 'I just upgraded'" should be enough. Subsequent updates should be just alright.

[pklaus (pklaus)]

@Arvid: my system was not in a broken state although it reported a "partial upgrade" some boot operations later. So I followed the proposed "upgrade" and the system was upgraded once more. I don't know what it did there exactly but it was similar to the normal upgrade process. It just didn't take a long time because all the packages were upgraded already.

The "cleanup" can mostly be done manually using "System → Administration → Computer Janitor" (don't know the program name in Swedish though).

[Martin Pitt (pitti)]

Accepted etckeeper into lucid-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[Thierry Carrez (ttx)]

Verified fixed using test case in description.

[Launchpad Janitor (janitor)]

This bug was fixed in the package etckeeper - 0.41ubuntu3.1

---------------
etckeeper (0.41ubuntu3.1) lucid-proposed; urgency=low

  * post-install.d/50vcs-commit: Truncate commit message to avoid breaking
    when upgrading very large numbers of packages (LP: #574244)
 -- Thierry Carrez <email address hidden> Fri, 11 Jun 2010 09:26:02 +0200