Age | Commit message (Collapse) | Author |
|
Add better acquire debugging support
See merge request apt-team/apt!130
|
|
The old code was fairly confusing, and contradictory. Notably, the
second `if` also only applied to the Data state, whereas we already
terminated the Data state earlier. This was bad.
The else fallback applied in three cases:
(1) We reached our limit
(2) We are Persistent
(3) We are headers
Now, it always failed as a transient error if it had
nothing left in the buffer. BUT: Nothing left in the buffer
is the correct thing to happen if we were fetching content.
Checking all combinations for the flags, we can compare the results
of Die() between 2.1.7 - the last "known-acceptable-ish" version
and this version:
2.1.7 this
Data !Persist !Space !Limit OK (A) OK
Data !Persist !Space Limit OK (A) OK
Data !Persist Space !Limit OK (C) OK
Data !Persist Space Limit OK OK
Data Persist !Space !Limit ERR ERR *
Data Persist !Space Limit OK (B) OK
Data Persist Space !Limit ERR ERR
Data Persist Space Limit OK OK
=> Data connections are OK if they have not reached their limit,
or are persistent (in which case they'll probably be chunked)
Header !Persist !Space !Limit ERR ERR
Header !Persist !Space Limit ERR ERR
Header !Persist Space !Limit OK OK
Header !Persist Space Limit OK OK
Header Persist !Space !Limit ERR ERR
Header Persist !Space Limit ERR ERR
Header Persist Space !Limit OK OK
Header Persist Space Limit OK OK
=> Common scheme here is that header connections are fine if they have
read something into the input buffer (Space). The rest does not matter.
(A) Non-persistent connections with !space always enter the else clause, hence success
(B) no Space means we enter the if/else, we go with else because IsLimit(), and we succeed because we don't have space
(C) Having space we do enter the while (WriteSpace()) loop, but we never reach IsLimit(),
hence we fall through. Given that our connection is not persistent, we fall through to the
else case, and there we win because we have data left to write.
|
|
We do not want to end up in a code path while reading content
from the server where we have local data left to write, which
can happen if a previous read included both headers and content.
Restructure Flush() to accept a new argument to allow incomplete
flushs (which do not match our limit), so that it can flush as
far as possible, and modify Go() and use that before and after
reading from the server.
|
|
This causes some more issues, really.
|
|
We have successfully finished reading data if our buffer is empty,
so we don't need to do any further checks.
|
|
If we have errors pending, always log them with our failure
message to provide more context.
|
|
Default Acquire::AllowReleaseInfoChange::Suite to "true"
See merge request apt-team/apt!128
|
|
Closes: #931566
|
|
|
|
http: Fix infinite loop on read errors
See merge request apt-team/apt!126
|
|
See merge request !127 for more information.
|
|
While we fixed the infinite retrying earlier, we still have
problems if we retry in the middle of a transfer, we might
end up resuming downloads that are already done and read
more than we should (removing the IsOpen() check so that
it always retries makes test-ubuntu-bug-1098738-apt-get-source-md5sum
fail with wrong file sizes).
I think the retrying was added to fixup pipelining messups,
but we have better solutions now, so let's get rid of it,
until we have implemented this properly.
|
|
When we failed after a retry, we only communicated failure as
transient, but this seems wrong, especially given that the code
now always triggers a retry when Die() is called, as Die() closes
the server fd.
Instead, remove the error handling in that code path, and reuse
the existing fatal-ish error code handling path.
|
|
If there was a transient error and the server fd was closed, the
code would infinitely retry - it never reached FailCounter >= 2
because it falls through to the end of the loop, which sets
FailCounter = 0.
Add a continue just like the DNS rotation code has, so that the
retry actually fails after 2 attempts.
Also rework the error logic to forward the actual error message.
|
|
|
|
Pu/http fixes 2
See merge request apt-team/apt!125
|
|
Support marking all newly installed packages as automatically installed
See merge request apt-team/apt!110
|
|
Remove master/slave terminology
See merge request apt-team/apt!124
|
|
|
|
|
|
|
|
Apologies.
|
|
Fully deprecate apt-key, schedule removal for Q2/2022
See merge request apt-team/apt!119
|
|
We only add the file to the select() call if we have data to
write to it prior to the select() call. This is problematic:
Assuming we enter Go() with no data to write to the file,
but we read some from the server as well as an EOF, we end
up not writing it to the file because we did not add the file
to the select.
We can't always add the file to the select(), because it's
basically always ready and we don't want to wake up if we
don't have anything to read or write.
So for a solution, let's just always write data to the file
if there's data to write to it. If some gets leftover, or if
some was already present when we started Go(), it will still
be added to the select() call and unblock it.
Closes: #959518
|
|
Pu/http fixes
See merge request apt-team/apt!122
|
|
Instead of reading the data early, disable the timeout for the
select() call and read the data later. Also, change Read() to
call only once to drain the buffer in such instances.
We could optimize this to call read() multiple times if there
is also pending stuff on the socket, but that it slightly more
complex and should not provide any benefits.
|
|
The error handling in Die() that's supposed to add useful error
messages is not super useful here.
|
|
This avoids a case where we read data, then write to the server
and only then realize the connection was closed. It is somewhat
slower, though.
|
|
By changing the buffer implementation to return true if it
read or wrote something, even on EOF, we should not have a
need to flush the buffer in Die() anymore - we should only
be calling Die() if the buffer is empty now.
|
|
This should avoid the need to Flush the buffer in Die(), because
if we read anything, we are returning true, and not entering Die()
at that point.
Also Write() does not have a concept of EOF, so get rid of code
handling that there. Was that copied from Read()?
|
|
Die() needs its own Copy() of Flush() because it needs to return
success or failure based on some states, but those are not precisely
the same as Flush(), as Flush() will always return false at the end,
for example, but we want to fall through to our error handling.
|
|
If we reached Die() there was an issue with the server connection,
so we should always explicitly close it.
|
|
Sorry!
|
|
|
|
dpkg (>= 1.20.3) has better support for its own DPKG_ROOT resulting in
architectures for the root being reported rather than the host system.
Sadly the hookscript from pkg-config is not prepared for this resulting
in our `dpkg --add-architecture` calls failing in the hook after dpkg
has successfully added the architecture internally. The failure
triggered fallback handling in the tests to work with an older version
of dpkg with a different multi-arch implementation.
So instead of doing the fallback, we ignore the failure if it seems like
pkg-config-dpkghook is involved only producing a bunch of warnings
to hint at this problem, but otherwise make the tests work again as it
is a post-invoke script.
References: #824774
|
|
The test runs ls on the opened fds and greps the result for 'root root'
which is how ls (<= 8.30) used to report user and group for these. Now
that Debian contains 8.32 it reports user and group of the process
owning them (supposedly). grepping for both unbreaks the test.
lr-x------ 1 root root 64 Jul 7 19:07 0 -> 'pipe:[10458045]'
lrwx------ 1 root root 64 Jul 7 19:07 1 -> /dev/pts/12
lrwx------ 1 root root 64 Jul 7 19:07 2 -> /dev/pts/12
lr-x------ 1 root root 64 Jul 7 19:07 3 -> /proc/1266484/fd
vs (assuming user:group is david:david)
lr-x------ 1 david david 64 Jul 7 19:07 0 -> 'pipe:[10458045]'
lrwx------ 1 david david 64 Jul 7 19:07 1 -> /dev/pts/12
lrwx------ 1 david david 64 Jul 7 19:07 2 -> /dev/pts/12
lr-x------ 1 david david 64 Jul 7 19:07 3 -> /proc/1266484/fd
|
|
It isn't needed to iterate over all results if we will be doing nothing
anyhow as it isn't that common to have that debug option enabled.
|
|
If this option is disabled (which it is by default in Debian), we don't
have to make the call and the checks around it. Not that it really
matters that much as if it would we would be better checking only once.
|
|
Closes: #963008
|
|
We were traditionally adding points for some dependency types to the
real package, but we should also do it for providers of that package to
help the resolver especially if the real package is for some reason not
tagged for removal yet/anymore.
While at it we ensure that the points are only attributed once for each
package as especially with versioned provides a package can nowadays
provide another many times and would hence acquire a lot of points.
|
|
If the package providing the given solution is tagged already for
removal (or at least for "not installing") we can ignore this solution
as a possibility as it is not one, which means we can avoid exploring
the option and potentially forward the protected flag further if that
helps in reducing the possibilities to a single one.
|
|
Marking a package for removal is fine if we know that we have to remove
that package, but if we are in an alternative branch we might not go
this route in the end and hence have a package pointlessly marked for
removal which isn't questioned later on.
We check if we are allowed to remove that package to avoid working on
the positive dependencies if not, but we mark them for removal only
after all the other dependencies are successfully resolved.
In an ideal world we would let the problemResolver do its job on them,
but the resolver might decide against doing the removal exploring
another option like the next alternative, which might be a good idea,
but is not the behaviour we had before, so that is the best we can do
for now without changing the resolver drastically.
|
|
Minor Catalan grammar typo
See merge request apt-team/apt!123
|
|
|
|
This will be mapped to Important for the time being.
|
|
|
|
M-A:foreign causes Provides to apply to all architectures and as we
wanted to avoid resolver changes for M-A those are done by explicitly
creating these provides instead of forcing the resolvers to learn about
this. The EDSP is a different beast though & we don't need this trick
here especially as it leads to needless (but harmless) duplication.
No sort+unique is done to avoid changing order (not that it should
matter, but just to be sure), but the sets should be small enough to not
make a huge difference either way.
|
|
We usually tell EDSP solvers only about architectures we are configured
to treat as native/foreign, but the system could have packages from
other architectures installed (even if very unlikely) which could
influence the solution (e.g. requiring a removal) so we make sure to
tell them.
|
|
If package is installed via an explicitly given deb file we store the
filename as a provides, so that the frontend can request the filename
and get the usual "Selected foo instead of foo.deb" message.
We do not need to trouble the EDSP solvers with that though as these
provides are not valid in various ways and we have already solved the
link between commandline and package (and version) for them.
Closes: #962741
|
|
Closes: #962483
|