Dear maintainer,

In bug 1026395, I identified behavior where apt-cacher-ng was tagging many
valid, referenced files for deletion.  One cause is mentioned there.  However,
I failed to notice another source of erroneous tagging: SHA256 sums in
the Packages/Sources/etc. files are not being detected.

For examples, debrep/dists/bullseye-backports/main/binary-amd64/Packages,
contains "^SHA256: " lines that are not being used.

The first of the two patches fixes that behavior for Packages.

During this process, a third source showed up: the lists of files in Sources
was getting clobbered because of the behavior of cacheman.cc:ParseGenericRfc822File
("we don't merge").

The second patch implements streaming handling of Sources a la Packages.  That
patch parses possibly untrusted data, so please give it a close read (also I
haven't done a lot of C++ coding recently, so I apologize in advance).

I'm tagging this important because most files for bookworm (and later) will
be automatically purged after a few days, since they are found to never be
referenced.  This has significant impact on many use cases for this package.

Best,
Antonio Russo

Hallo,
* Antonio Russo [Thu, Dec 22 2022, 07:15:12AM]:

Thanks. Regarding the second patch, I am not sure. Looks like my C++
also has become rusty in the last months. It would be good to have an
explicit description of the problem cases, since I don't know exactly
what you mean with "streaming". I.e. it seems like you want the generic
parser implementation to be changed to a specialized local one but how
am I supposed to write a unit test for this?

To be checked in the next days, stay tuned.

Best regards,
Eduard.

Sorry, I was in a bit of a hurry when I wrote that email.

The purpose of the second patch is to properly parse the list of packages
sources, e.g. [1].  Those files are concatenations of RFC822 blocks,
separated by blank lines.  The previous implementation did not work,
because it did not account for the fact that all of these separate blocks
are of importance to us.

In particular, ParseDebianRfc822Index, line 1995 of src/cacheman.cc:

    // we don't merge

justifies the next statement:

    pLastVal->clear();

That serves to remove the last block's information (rather than merge it
with the last package).  The symptom of this error in parsing is that
all Sources blocks (except the last one, which is not clobbered by any
subsequent blocks) are not identified as referencing data in the cache.
Therefore most of the, e.g., .tar{,.gz,.xz,.bzip2} and .dsc files are
marked for expiration from the cache.

I have added a new implementation in the same spirit as the one used for
Packages parsing, (see src/cacheman.cc:1707).  In that branch of the
switch statement (EIDX_PACKAGES), each RFC822 block is parsed.  I used
the word "streaming" to indicate that the whole index does not need to
be loaded into memory before the first call to ret(info) begins returning
data to the rest of the service.

While good for the overall performance of apt-cacher-ng, this streaming
behavior is not the primary purpose of the patch (and I apologize for
overemphasizing that in the title).  Indeed, it is the correctness of
the result that is my main objective here.

As for writing a unit test, I would suggest grabbing some subset of [1],
and ensuring that all of the entries are properly accounted for. Without
this patch, such a unit test would fail.

Best,
Antonio

[1] http://ftp.us.debian.org/debian/dists/bookworm/main/source/Sources.gz

Dear Maintainer,

This issue affects me as well.

For example, just after installing bookworm using apt-cacher-ng, the
maintenance web page wants to remove currently up-to-date packages.

Hopefully, exStartTradeOff parameter prevents this, but it won't last forever.

Cheers,
Christophe

Hello,

Thanks for merging the first of the two patches.

As it stands, only one package per distribution will be preserved by apt-cacher-ng.

The fix for this is to parse each empty-line-delineated block of the downloaded
Sources file as an RFC822 entry, rather than treating the entire file as a single
RFC822 entry.

The previously attached patch does this.  Is there anything I can do to help get
this merged for trixie?  It would be a shame for this package to essentially cache
nothing for more than a few days (the symptom of this bug) for two full release
cycles.

I'll add that I have been using this patch since submitting it (over two years
now).

Best,
Antonio Russo