| Age | Commit message (Collapse) | Author |
|---|
|
While running our CI we noticed that sometimes we see an error
from the new json hooks code. The error message is:
```
E: Could not read response to hello message from hook [ ! -f /usr/bin/snap ] || /usr/bin/snap advise-snap --from-apt 2>/dev/null || true: Broken pipe
```
when purging the snapd package which provides the hook. This indicates
that we should probably also consider EPIPE not an error (just like
we do for ECONNRESET). This PR does exactly this.
(cherry picked from commit 6af48a7f83540c807be1d2777470d23e6b260eb8)
LP: #1814543
(cherry picked from commit 97412ac47a974b1eb0e8280cf11b3b80ff1ba17f)
|
|
The dpkg frontend lock is a lock dpkg tries to acquire
except if the frontend already acquires it.
This fixes a race condition in the install command where the
dpkg lock is not held for a short period of time between
different dpkg invocations.
For this reason we also define an environment variable
DPKG_FRONTEND_LOCKED for dpkg invocations so dpkg knows
not to try to acquire the frontend lock because it's held
by a parent process.
We can set DPKG_FRONTEND_LOCKED only if the frontend lock
really is held; that is, if our lock count is greater than 0
- otherwise an apt client not using the LockInner family of
functions would run dpkg without the frontend lock set, but
with DPKG_FRONTEND_LOCKED set. Such a process has a weaker
guarantee: Because dpkg would not lock the frontend lock
either, the process is prone to the existing races, and,
more importantly, so is a new style process.
Closes: #869546
[fixups: fix error messages, add public IsLocked() method, and
make {Un,}LockInner return an error on !debSystem]
(cherry picked from commit c2c8b4787b0882234ba2772ec7513afbf97b563a)
LP: #1781169
|
|
The default buffer size for pkgTagFile is 32kb which should be big
enough for everything… expect for enormous lists of provides,
resulting in:
$ apt show librust-winapi-dev
E: Unable to parse package file /var/lib/apt/lists/ftp.br.debian.org_debian_dists_unstable_main_binary-amd64_Packages (2)
E: Internal Error, Unable to parse a package record
The "apt-cache show" codepath uses instead a max size for all files,
which seems a bit excessive, but works – using the max size for the file
in question seems most appropriate.
The patch is written for the 1.6.y series as a rewrite of the related
code in the 1.7.y series (commit bf53f39c9a0221b670ffff74053ed36fc502d5a0)
removed this problem before it was reported.
Closes: #905527
LP: #1787120
|
|
JSON hooks might disappear and the common idiom to work around hooks
disappearing is to check for the hook in the shell snippet that is
in the apt.conf file and if it does not exist, do nothing. This caused
APT to fail however, expecting it to acknowledge the handshake.
Ignoring ECONNRESET on handshakes solves the problem.
The error case, and the other error cases also did not stop execution
of the hook, causing more errors to pile up. Fix this by directly going
to the closing part of the code.
LP: #1776218
(cherry picked from commit 1d53cffad22c92645090e0e6ddde31fe4f7c3b05)
|
|
This allows third-party package managers like snap or flatpak
to hook in and suggest alternatives if packages could not be
found, for example.
This is still highly experimental and the protocol might change
in future versions.
|
|
Collecting the packages we could not find allows us to pass them
to other places.
|
|
This setting was lost in the transition to cmake.
The private library has no public users and hence the default visibility
of symbols changed early to hidden – something which should eventually
be done for the public libraries as well, but one step at the time.
|
|
If a method needs a file to operate like e.g. mirror needs to get a list
of mirrors before it can redirect the the actual requests to them. That
could easily be solved by moving the logic into libapt directly, but by
allowing a method to request other methods to do something we can keep
this logic contained in the method and allow e.g. also methods which
perform binary patching or similar things.
Previously they would need to implement their own acquire system inside
the existing one which in all likelyhood will not support the same
features and methods nor operate with similar security compared to what
we have already running 'above' the requesting method. That said, to
avoid methods producing conflicts with "proper" files we are downloading
a new directory is introduced to keep the auxiliary files in.
[The message magic number 351 is a tribute to the german Grundgesetz
article 35 paragraph 1 which defines that all authorities of the
state(s) help each other on request.]
|
|
The casts are useless, but the reports show some where we can actually
improve the code by replacing them with better alternatives like
converting whatever int type into a string instead of casting to a
specific one which might in the future be too small.
Reported-By: gcc -Wuseless-cast
|
|
gcc was warning about ignored type qualifiers for all of them due to the
last 'const', so dropping that and converting to static_cast in the
process removes the here harmless warning to avoid hidden real issues in
them later on.
Reported-By: gcc
Gbp-Dch: Ignore
|
|
Reported-By: gcc -Wclass-memaccess
Gbp-Dch: Ignore
|
|
apt usually gets the width of the window from the terminal or failing
that has a default value, but especially for testing it can be handy
to control the size as you can't be sure that variable sized content
will always be linebreaked as expected in the testcases.
|
|
It used FindI() > 0, but if it is too big, FindI() would
cause an error "Cannot convert %s to integer: out of range",
so let's also use FindULL() here.
Gbp-Dch: ignore
|
|
Installed-Size for linux-image-4.13.0-1-amd64-dbg and friends
are larger than 4 GB, but read as a signed integer - that's
fine so far, as the value is in KB, but it's multiplied with
1024 which overflows. So let's read it as unsigned long long
instead.
While we're at it, also use unsigned long long for Size, in
case that is bigger than 2 GB.
|
|
cppcheck reports:
(portability) Passing NULL after the last typed argument to a variadic
function leads to undefined behaviour.
We don't ship on any platform which has this as undefined behaviour
through – or it would be pretty well defined "bad" behaviour which
always works, so even through UB is a trigger word, its hardly
noteworthy as a change (and as a bonus the scanners of gcc/clang
don't consider it UB).
The commonly accepted method of fixing that seems to be (const
char*)NULL, but it is in fact much simpler to just switch to the varadic
functions C++ provides resolving the warning and reducing code.
Reported-By: cppcheck
Gbp-Dch: Ignore
|
|
The code only used to warn when it came into a situation where
something actually had to be forced. Warn directly after parsing
the command-line instead, that's more accurate.
|
|
The feature exists for a long while even if we get around to document
it properly only now, so we should push for its adoption a bit to avoid
the problems its supposed to solve like avoiding usage of non-world
readable configuration files as they can cause strange behaviour for the
unsuspecting user (like different solutions as root and non-root).
|
|
We detect the effected sources by matching Release info – that has
potential by-catch of repositories which have incorrect field values,
but those are better fixed now anyhow. The bigger incorrectness is that
this message will not only be printed for the Debian services itself but
also for all mirrors not under Debian control but serving Debian like more
local/private mirrors which will not (directly) shutdown. It is likely
through that many of them will follow suite with less visible
announcements or break downright if their upstream source disappears, so
having false-positives here seems benefitial for the user in the end.
|
|
This makes it easier to see which headers includes what.
The changes were done by running
git grep -l '#\s*include' \
| grep -E '.(cc|h)$' \
| xargs sed -i -E 's/(^\s*)#(\s*)include/\1#\2 include/'
To modify all include lines by adding a space, and then running
./git-clang-format.sh.
|
|
Including cacheiterators.h before pkgcache.h fails because
pkgcache.h depends on cacheiterators.h.
|
|
If we have a user sitting around we can let 'apt' ask the user for a
confirmation rather than print errors at the end and require the user to
figure out which commandline flags are needed to confirm the changes
non-interactively.
|
|
The value of Origin, Label, Codename and co can be used in user
configuration from apts own pinning to unattended upgrades.
A repository changing this values can therefore have serious effects on
the behaviour of apt and other tools using these values.
In a first step we will generate error messages for these changes now
explaining the need for explicit confirmation and provide config options
and commandline flags to accept them.
|
|
The exception was made to give (script) users a one-release grace period
to adapt their setup to deal with apt enforcing signing of repositories.
As we are now at the start of a new release cycle its as good a time as
any to lift it now.
Removes-Exception: 952ee63b0af14a534c0aca00c11d1a99be6b22b2
|
|
Adopting this change in other frontends will require source changes as
well similar to our own changes in apt-private/.
|
|
Showing messages related to downloading in a mode which can't download
is pretty pointless, so instead of trying harder to make it so that
these messages do not trigger just skip them entirely.
That the message triggered here is an artifact of the implementation in
which the download items are finished, while the code expects them to be
still pending – even the in a previous run completely downloaded files.
Closes: 863635
|
|
We are in a dilemma here: The regression of sorts was introduced in 2013
with commit d8a8f9d7f0 allowing pkg modifiers for the upgrade commands.
That calls the autoremover as a sideeffect through and with it comes the
option to remove the garbage packages in these commands (similar to aptitude).
Having the option on the commandline is no problem – people aren't going
to request what they don't want (or so I hope), but the documentation
explicitly states that this option only effects install/remove and
mentions a config knob users might use and expect to not suddenly apply
(especially without documentation) to more commands.
Just reverting the commit is out of question, completely ignoring the
option breaks the workflow of every user who happened to use
--autoremove on the commandline for upgrade and expects that to work
given that it was accepted and worked in a stable release. Changing the
documentation to reflect reality while perhaps the simplest and cleanest
option contradicts freeze and is a surprising change we tend to avoid
like the plague while just leaving it be confuses all users who end up
believing the documentation even if was different in the last 3 years.
So what we do is a tricky compromise: The configuration option if read
from a file does apply only for install/remove as documented, while if
the option is encountered on the commandline it is accepted and applies
to the upgrade which should make 99% of the users happy. The rest has to
wait for us to figure out for buster how to get that documented and
implemented in a saner way.
Closes: #855891
|
|
Most of them in (old) code comments. The two instances of user visible
string changes the po files of the manpages are fixed up as well.
Gbp-Dch: Ignore
Reported-By: spellintian
|
|
Normal cows moo every time they feel like it and it might be a "moo",
"moo!" or "moo?". This is completely unacceptable behaviour in our super
cow through as as a superior being it has to show its superiority over
the common cows and the meager easter eggs by being fully reproducible!
The second version of Chris' patch is modified to include an array of
tests for this feature – which doubles as explanation for some of the
moo lines by giving more exact dates – and falling back to current time
if the environment is invalid + passing time around instead of having an
invalid environment be an unrecoverable error (aka: Guru Meditation) as
that is more inline with how apt usually behaves: The wisdom of super cow
should be available to everyone, even to the most misfortune users not
capable of having a valid environment variable.
That makes the code slightly more ugly, so instead of requiring a young
follower to produce a third version a high priest of the cult applied the
finishing touches as he is used to the pain by now – and another round
with the slowpoke high priest would have been a serious threat to the
Debian release schedule which the cow would not approve.
Closes: #848721
Signed-off-by: Super Cow
Thanks: Chris Lamb for initial patch and guru meditation
|
|
The mode wasn't working at all if not used together with --fix-missing
which while likely to come in pairs its legal to use standalone.
Regression-in: eb1f04dda07c2b69549ad9fd793cca0e91841b3e
|
|
The update command acquires a lock on lists/, but at the end it will
also require the dpkg/lock while building the binary caches. That seems
rather pointless as we are only reading those files, not causing writing
in them. This can also cause problems if a package installation is
running and a background process (like cron) starts an update: If you
are "lucky" enough the update process will pick the dpkg lock in between
apt calls causing the installation process to fail.
|
|
We get the archives/lock for clean – that is enough to ensure that other
apt instances aren't interfering (or are being interfered with). We
don't need to block actions involving dpkg.
|
|
Unlikely that anyone is actually running into this, but if we asked to
not generate a cache and avoid it in the first step we shouldn't create
one implicitly anyway by displaying the statistics.
|
|
This will avoid people from thinking that they have to do nothing
when they change the set of files.
Gbp-Dch: ignore
|
|
The idea is simple: Each¹ Find*( call starts with a call check if the
given option (with the requested type) exists in the whitelist. The
whitelist is specified via our configure-index file so that we have
a better chance at keeping it current. the whitelist is loaded via a
special (undocumented for now) configuration stanza and if none is
loaded the empty whitelist will make it so that no warnings are shown.
Much needs to be done still, but that is as good a time as any to take a
snapshot of the current state and release it into the wild given that it
found some bugs already and has no practical effect on users.
¹ not all in this iteration, but many
|
|
Again no practical difference, but for consistency a boolean option
should really be accessed via a boolean method rather than an int
especially if you happen to try setting the option to "true" …
Gbp-Dch: Ignore
|
|
Users end up believing that this is a --force mode as -f is common for
that, but apt doesn't have such a mode and --fix-broken is really not
about forcing something but actually trying to fix the breakage which
tends to be the result of a user forcing something on its system via
low-level forced dpkg calls.
Example: The "common" pattern of "dpkg -i ./foo.deb; apt install -f" is
nowadays far better dealt with via "apt install ./foo.deb".
And while at it the two places handing out this suggestion are changed
to use the same strings to avoid needless translation work in the future
and the suggestion uses 'apt' instead of 'apt-get' as this will be run
interactively by a user, so its a good opportunity to showcase what we
can do and will allow us to be more helpful to the user.
Closes: #709092
Thanks: Kristian Glass for initial patch!
|
|
The config list APT::Build-Essential gets a similar treatment to other
lists now by having the value of the option itself be an override for
the list allowing to disable build-essentials entirely as well as
adding/overriding as usual by now in other lists.
Reported-By: Johannes 'josch' Schauer on IRC
|
|
The implementation is quite different compared to --arch-only due to ABI
reasons but functionality wise they are similar and usually both
available for symmetry at least.
Closes: #845775
|
|
In bug #757534 the opposite direction was initially requested, but what
we did end up with was having a possibility to configure the options
passed to dpkg. The reasoning given their and in #724744 is specific why
apt doesn't need the checks to be performed by dpkg. In fact, what these
two reports show is that if those checks are run people end up being
confused about the requirement of them being run, so given the best case
those checks can do is do nothing (visibly) while the worst cases are
warnings and errors which are neither we are from a security point
better of with disabling them – as (as mentioned in the bugreports)
false positives for issues are really really bad in a security context.
Closes: 724744
|
|
We are calling system() in this code paths, so all we do here is having
a single child performing the action while the parent waits for it to
finish… with the added strangeness of not having our usual error message
collection and giving up after first failure even if told to act on
multiple packages.
|
|
That was the case already for tar-only and diff-only, but in a more
confusing way and without a message while dsc "worked" before resulting
in a dpkg-source error shortly after as tar/diff files aren't available…
|
|
These new enum values might cause "interesting" behaviour in tools not
expecting them – like an old apt would think a Build-Conflicts-Arch is
some sort of Build-Depends – but that can't reasonably be avoided and
effects only packages using B-D/C-A so if there is any breakage the
tools can easily be adapted.
The APT_PKG_RELEASE number is increased so that libapt users can detect
the availability of these new enum fields via:
#if APT_PKG_ABI > 500 || (APT_PKG_ABI == 500 && APT_PKG_RELEASE >= 1)
Closes: #837395
|
|
In effect this is an extension of the 6 years old commit
a8dfff90aa740889eb99d00fde5d70908d9fd88a which uses the autoremover to
remove packages again from the solution which are no longer needed to be
there. Commonly these are dependencies of packages we end up not
installed due to problem resolver decisions. Slightly less common is
the situation we deal with here: a package which we wanted to upgrade
sporting a new dependency, but ended up holding back.
The problem is that all versions of an installed reverse dependencies can
bring back a "garbage" package – we need to do this as there is
nothing inherently wrong in having garbage packages installed or upgrade
them, which itself would have garbage dependencies, so just blindly
killing all new garbage packages would prevent the upgrade (and actually
generate errors). What we should be doing is looking only at the version
we will have on the system, disregarding all old/new reverse dependencies.
Reported-By: Stuart Prescott (themill) on IRC
|
|
Commit b60c8a89c281f2bb945d426d2215cbf8f5760738 improved the situation,
but due to inconsistency mostly for planners, not for solvers. As the
idea of hiding errors if we show another error is a bit scary (as the
extern error might be a followup of our intern error, rather than the
reason for our intern error as it is at the moment) we don't discard the
errors, but if we got an extern error we show them directly removing
them from the error list at the end of the run – that list will contain
the extern error which hopefully gives us the best of both worlds.
The problem itself is the same as before: The externals exiting before
apt is done talking to them.
Reported-By: Johannes 'josch' Schauer on IRC
|
|
|
|
I probably missed that when I did the usability work. But better
late than never.
|
|
On FreeBSD, we have to include sys/params.h and sys/mount.h,
not sys/vfs.h.
Gbp-Dch: ignore
|
|
Several modules use std::array without including the
array header. Bad modules.
Some modules use STDOUT_FILENO and friends, or close()
without including unistd.h, where they are defined.
One module also uses WIFEXITED() without including
sys/wait.h.
Finally, environ is not specified to be defined in unistd.h. We
are required to define it ourselves according to POSIX, so let's
do that.
|
|
We support "./foobar.deb" as a way to install a deb file directly.
Recently .changes files were added. This highlights a problem as you
can't add the changes file without also trying to install all of them.
Now, it could also be handy to add entire Packages/Sources files to
perhaps get a bunch of packages in without installing them all
implicitly.
This commit introduces --with-source which allows to add *.deb, *.changes,
*.dsc, source-dirs, Packages & Sources files (the later can also be
compressed) without also installing them.
|
|
Bye, bye, old friend.
|
