Debians commandline package manager https://git.kalnischkies.de/apt/atom?h=2.1.5 2020-05-23T15:59:19Z 2020-05-23T15:59:19Z David Kalnischkies david@kalnischkies.de 2020-05-23T13:53:06Z urn:sha1:4f71dc657c34915508a9e34b000e1b577931655a It looks like hack and therefore I wanted this to be a very isolated commit so we can find it & revert it easily if need be, but for now it seems to work. The idea is that Status is telling us how the candidate is in relation to the current installed version which is used to figure out if a package is "kept back" by the algorithm or not, but by discarding the candidate version we loose this information. Ideally we would keep better tabs on what we do to a package and why, but for now that seems okayish. It will cause the wrong version to be displayed though as if the package is installed the installed version becomes the candidate and hence (installed => installed) is displayed. 2020-05-23T15:58:42Z David Kalnischkies david@kalnischkies.de 2020-05-22T16:56:40Z urn:sha1:04a020d7a217d6b5af86c048c2974760053b8079 For a (partially) installed package like the one MarkInstall operates on at the moment we want to discard the candidate from, we have to first remove the package from the internal state keeping to have proper broken counts and such and only then reset the candidate version which is a trivial operation in comparison. Take a look at the testcase: Now, what is the problem? Correct, git:i386. Didn't see that coming, right? It is M-A:foreign so apt tries to switch the architecture of git here (which is pointless, it knows that this won't work, but lets fix that in another commit) will eventually realize that it can't install it and wants to discard the candidate of git:i386 first removing the broken indication like it should, removing the install flag and then reapplies the broken indication: Expect it doesn't as it wants to do that over the candidate version which the package no longer had so seemingly nothing is broken. It is a bit of a hairball to figure out which commit it is exactly that is wrong here as they are all influencing each other a bit, but >= 2.1 is an acceptable ballpark. Bisect says 57df273 but that is mostly a lie. Closes: #961266 2020-05-19T09:22:19Z David Kalnischkies david@kalnischkies.de 2020-05-19T09:20:28Z urn:sha1:dcd920e99df964d320e18ac133d575d4151deb85 References: dcdfb4723a9969b443d1c823d735e192c731df69 2020-05-18T13:55:36Z David Kalnischkies david@kalnischkies.de 2020-05-17T23:03:52Z urn:sha1:1f641cf70e9cd52c093b4b62dc392f29cf34f03e Turns out that pkgDepCache and pkgProblemResolver maintain two (semi) independent sets of protected flags – except that a package if marked protected in the pkgProblemResolver is automatically also marked in the pkgDepCache as protected. This way the pkgProblemResolver will have as protected only the direct user requests while pkgDepCache will (hopefully) propagate the flag to unavoidable dependencies of these requests nowadays. The pkgProblemResolver was only checking his own protected flag though and based on that calls our Mark* methods usually without checking return, leading to it believing it could e.g. remove packages it actually can't remove as pkgDepCache will not allow it as it is marked as protected there. Teaching it to check for the flag in the pkgDepCache instead avoids it believing in the wrong things eventually giving up. The scoring is keeping the behaviour of adding the large score boost only for the direct user requests though as there is no telling which other sideeffects this might have if too many packages get too many points from the get-go. Second part of fixing #960705, now with pkgProblemResolver output which looks more like the whole class of problem is resolved rather than a teeny tiny edgecase it was before. 2020-05-18T13:55:36Z David Kalnischkies david@kalnischkies.de 2020-05-16T12:46:05Z urn:sha1:dbed89f296106f82e9fe8f866fa87a4c14b44584 The previous commit deals with negative, now we add the positive side of things as well which makes this a recursive endevour. As we can push the protected flag forward only if a single solution for a dependency exists it is easy for trees to not get it, so if resolving becomes difficult it won't help at all. 2020-05-18T13:55:36Z David Kalnischkies david@kalnischkies.de 2020-05-16T09:17:21Z urn:sha1:57df27397b1a10e50d5876482a30b9dedb2ad219 If we propagate protected e.g. due to a user request we should also act upon (at the moment) satisfied negative dependencies so that the resolver knows that installing this package later is not an option. That the problem resolver is trying bad solutions is a bug by itself which existed before and after and should be worked on. Closes: #960705 2020-05-18T13:55:36Z David Kalnischkies david@kalnischkies.de 2020-05-15T14:44:49Z urn:sha1:65ce0eb3c0d71031c59c14c7e433b0b969548978 We exit early from installing dependencies of a package only if it is not a user request to avoid polluting the state with installs which might not be needed (or detrimental even) for alternative choices. We do continue with installing dependencies though if it is a user request as it will improve error reporting for apt and can even help aptitude not hang itself so much as we trim the problem space down for its resolver dealing with all the easy things. Similar things can be said about the testcase I have short-circuit previously… keep going test, do what you should do to report errors! 2020-05-18T13:55:36Z David Kalnischkies david@kalnischkies.de 2020-05-18T09:55:54Z urn:sha1:c470d92366d7c3c239a689f0a10d6d0d9daafbff Our testcases had their own implementation of GetTempFile with the feature of a temporary file with a choosen suffix. Merging this into GetTempFile lets us drop this duplicate and hence test more our code rather than testing our helpers for test implementation. And then hashsums_test had another implementation… and extracttar wasn't even trying to use a real tempfile… one GetTempFile to rule them all! That also ensures that these tempfiles are created in a temporary directory rather than the current directory which is a nice touch and tries a little harder to clean up those tempfiles. 2020-05-13T20:04:13Z Julian Andres Klode julian.klode@canonical.com 2020-05-13T08:51:10Z urn:sha1:3368ae121112405259288c9139a300dc0cac31fe 2020-05-12T16:55:55Z Julian Andres Klode julian.klode@canonical.com 2020-05-12T09:49:09Z urn:sha1:dceb1e49e4b8e4dadaf056be34088b415939cda6 When normalizing ar member names by removing trailing whitespace and slashes, an out-out-bound read can be caused if the ar member name consists only of such characters, because the code did not stop at 0, but would wrap around and continue reading from the stack, without any limit. Add a check to abort if we reached the first character in the name, effectively rejecting the use of names consisting just of slashes and spaces. Furthermore, certain error cases in arfile.cc and extracttar.cc have included member names in the output that were not checked at all and might hence not be nul terminated, leading to further out of bound reads. Fixes Debian/apt#111 LP: #1878177