Age | Commit message (Collapse) | Author |
|
If we have files in partial/ from a previous invocation or similar such
those could be symlinks created by file:// sources. The code is
expecting only real files through and happily changes owner,
modification times and permission on the file the symlink points to
which tend to be files we have no business in touching in this way.
Permissions of symlinks shouldn't be changed, changing owner is usually
pointless to, but just to be sure we pick the easy way out and use
lchown, check for symlinks before chmod/utimes.
Reported-By: Mattia Rizzolo on IRC
|
|
If libapt has builtin support for a compression type it will create a
dummy compressor struct with the Binary set to 'false' as it will catch
these before using the generic pipe implementation which uses the
Binary. The catching happens based on configured Names through, so you
can actually force apt to use the external binaries even if it would
usually use the builtin support. That logic fails through if you don't
happen to have these external binaries installed as it will fallback to
calling 'false', which will end in confusing 'Write error's.
So, this is again something you only encounter in constructed testing.
Gbp-Dch: Ignore
|
|
We deploy atomic renames for some files, but these renames also happen
if something about the file failed which isn't really the point of the
exercise…
Closes: 828908
|
|
Seen first in #826783, but as this buglog also shows leaked uncompressed
files as well we don't close it just yet.
|
|
This effects only compressors configured on the fly (rather then the
inbuilt ones as they use a library).
|
|
If the file is in a failed state there is no point in trying to flush
out the buffers as the file is to be discarded anyhow & its likely all
this flushing is producing is additional error messages.
Git-Dch: Ignore
|
|
Some methods had it missing, some used the keyword directly, which isn't
a problem as it is a cc file, but for consistency lets stick to our
macro for now.
Git-Dch: Ignore
|
|
The liblzma-based write code needs the same tweaks that the read code
already has to cope with the situation where lzma_code returns zero the
first time through because avail_out is zero, but will do more work if
called again.
This ports the read tweaks to the write code as closely as possible
(including matching comments etc.).
Closes: #751688
|
|
If we just reopened the file, we also need to reset the current
seek position when we reset the buffer, otherwise the code will
not try to seek to the position given to Skip (from 0), but will
try to seek to old offset + the position given to skip.
Closes: #812994, #813000
|
|
When writing into the buffer write to free() bytes starting
at getend(), instead of buffersize_max bytes at get()
-> get() is a read pointer.
This makes no difference in practice though, as we reset
the buffer before the call, so start = end = 0.
Gbp-Dch: ignore
|
|
We cannot just return false without setting an error,
as InternalWrite does not set one itself.
|
|
It makes no sense to split a large block into multiple small
blocks, so when we have the chance to write them unbuffered,
do so.
|
|
We do not need the loop, FileFd::Private() handles this for us.
Gbp-Dch: ignore
|
|
We want to check whether the amount of free space is smaller
than the requested write size. Checking maxsize - size() is
incorrect for bufferstart >= 0, as size() = end - start.
Gbp-Dch: ignore
|
|
|
|
This is a multiple of the page size and thus results in less
page faults, speeding up copying.
Also, while we're at at, unify all uses of that size in a
constant variable APT_BUFFER_SIZE.
|
|
Implement native support for LZ4 compression, using the official
lz4 library.
|
|
This makes code easier to read, and somewhat more correct.
Gbp-Dch: ignore
|
|
Gbp-Dch: ignore
|
|
Previously, if flush errored inside the loop, data could have
already been written to the wrapped descriptor without having
been removed from the buffer.
Also try to work around EINTR here. A better solution might be
to have the individual privates detect an interrupt and return
0 in such a case, instead of relying on errno being untouched
in between the syscall and the return from InternalWrite.
|
|
This avoids some issues with InternalWrite returning 0 because
it just cannot write stuff at the moment.
|
|
This is somewhat experimental right now, and might not work
for everyone, so it is on an opt-in basis.
|
|
The flush function can be used for buffered writers.
|
|
We will soon implement a buffered writing decorator and we will
need to forward attribute changes to those.
|
|
These can be used to implement write buffering
Gbp-Dch: ignore
|
|
Suggested by David.
Gbp-Dch: ignore
|
|
Gbp-Dch: ignore
|
|
There is not much point and this is more readable.
Gbp-Dch: ignore
|
|
This is mostly a documentation issue, as the size we want to
read is always less than or equal to the size of the buffer,
so the return value will be the same as the size argument.
Nonetheless, people wondered about it, and it seems clearer
to just always use the return value.
|
|
This further improves our performance, and rred on uncompressed
files now spents 78% of its time in writing. Which means that
we should really look at buffering those.
|
|
The code uses memmove() to move parts of the buffer to the
front when the buffer is only partially read. By simply
reading one page at a time, the maximum size of bytes that
must be moved has a hard limit, and performance improves:
In one test case, consisting of a 430 MB Contents file,
and a 75K PDiff, applying the PDiff previously took about
48 seconds and now completes in 2 seconds.
Further speed up can be achieved by buffering writes, they
account for about 60% of the run-time now.
|
|
Gbp-Dch: ignore
|
|
And as we are at it lets fix the 'style' issue I introduced with the
filefd changes as well.
Reported-By: gcc -fsanitize's & cppcheck
Git-Dch: Ignore
|
|
We don't need the buffer that often - only for ReadLine - as it is only
occasionally used, so it is actually more efficient to allocate it if
needed instead of statically by default. It also allows the caller to
influence the buffer size instead of hardcoding it.
Git-Dch: Ignore
|
|
The default implementation of ReadLine was very naive by just reading
each character one-by-one. That is kinda okay for libraries implementing
compression as they have internal buffers (but still not great), but
while working with files directly or via a pipe as there is no buffer
there so all those reads are in fact system calls.
This commit introduces an internal buffer in the FileFd implementation
which is only used by ReadLine. The more low-level Read and all other
actions remain unbuffered – they just changed to deal with potential
"left-overs" in the buffer correctly.
Closes: 808579
|
|
If we use the library to compress xz, still try to understand and pick
up the arguments we would have used to call xz to figure out which level
the user wants us to use instead of defaulting to level 6 (which is the
default level of xz).
|
|
dpkg switched from CRC32 to CRC64 in
777915108d9d36d022dc4fc4151a615fc95e5032 with the message:
| This is the default CRC used by the xz command-line tool, align with
| it and switch from CRC32 to CRC64. It should provide slightly better
| detection against damaged data, at a negligible speed difference.
|
|
This isn't implementing any new features, it is "just" moving code
around from FileFd methods which decided on each call how to handle the
request by including all logic for all possible compressor backends in
the method body to a model in which backend-specifics are implemented in
a FileFdPrivate subclass. This avoids a big chunk of #ifdef's and should
make it a tiny bit more obvious which backend uses which code.
The execution of the idea is slightly uglified by the need to preserve
ABI and API which causes liberal befriending.
Git-Dch: Ignore
|
|
There's no point trying to read 0 bytes, so let's just not
do this and switch to a while loop like in Write().
Gbp-Dch: ignore
|
|
Turn the do-while loop into while loops, so it simply does nothing
if the Size is already 0.
This reverts commit c0b271edc2f6d9e5dea5ac82fbc911f0e3adfa7a which
introduced a fix for a specific instance of the issue in the
CopyFile() function.
Closes: #808381
|
|
On EOF, ToRead will be 0, which might trigger on some systems (e.g.
on the Hurd) an error due to the invalid byte count passed to write().
The whole loop already checks for ToRead != 0, so perform the writing
step only when there was actual data read.
Closes: #808381
|
|
Commit e977b8b9234ac5db32f2f0ad7e183139b988340d tries to make BufSize
calculated based on the size of the buffer; the problem is that
std::unique_ptr::size() returns a pointer to the data, so sizeof()
equals to the size of a pointer (later divided by sizeof(char), which
is 1). The result is that the CopyFile copies in chunks of 8 bytes,
which is not exactly ideal...
As solution, declare BufSize in advance, and use its value to allocate
the Buf array.
Closes: #808381
|
|
Dropping privileges is an involved process for code and system alike so
ideally we want to verify that all the work wasn't in vain. Stuff
designed to sidestep the usual privilege checks like fakeroot (and its
many alternatives) have their problem with this through, partly through
missing wrapping (#806521), partly as e.g. regaining root from an
unprivileged user is in their design. This commit therefore disables
most of these checks by default so that apt runs fine again in a
fakeroot environment.
Closes: 806475
|
|
This also deals with the unlikely case of groups being mentioned
multiple times or if the effective group isn't mentioned at all.
In practice, it is a debugging aid through like for #806475.
Git-Dch: Ignore
|
|
Git-Dch: Ignore
|
|
In 0940230d we started dropping privileges for file (and a bit later for
copy, too) with the intend of uniforming this for all methods. The
commit message says that the source will likely fail based on the
compressors already – and there isn't much secret in the repository
content. After all, after apt has run the update everyone can access the
content via apt anyway…
There are sources through which worked before which are mostly
single-deb (and those with the uncompressed files available).
The first one being especially surprising for users maybe, so instead of
failing, we make it so that apt detects that it can't access a source as
_apt and if so doesn't drop (for all sources!) privileges – but we limit
this to file/copy, so the uncompress which might be needed will still
fail – but that failed before this regression.
We display a notice about this, mostly so that if it still fails (e.g.
compressed) the user has some idea what is wrong.
Closes: 805069
|
|
Unlinking /dev/null is bad, we shouldn't do that. Also, we should print
at least a warning if we tried to unlink a file but didn't manage to
pull it of (ignoring the case were the file is /dev/null or doesn't
exist in the first place).
This got triggered by a relatively unlikely to cause problem in
pkgAcquire::Worker::PrepareFiles which would while temporary
uncompressed files (which are set to keep compressed) figure out that to
files are the same and prepare for sharing by deleting them. Bad move.
That also shows why not printing a warning is a bad idea as this hide
the error for in non-root test runs.
Git-Dch: Ignore
|
|
The wrapping will fail in the best case and actually end up deleting
/dev/null in the worst case. Given that there is no point in trying to
write atomically to /dev/null as you can't read from it again just
ignore these flags if higher level code ends up trying to use them on
/dev/null.
Git-Dch: Ignore
|
|
Our error reporting is historically grown into some kind of mess.
A while ago I implemented stacking for the global error which is used in
this commit now to wrap calls to functions which do not report (all)
errors via return, so that only failures in those calls cause a failure
to propergate down the chain rather than failing if anything
(potentially totally unrelated) has failed at some point in the past.
This way we can avoid stopping the entire acquire process just because a
single source produced an error for example. It also means that after
the acquire process the cache is generated – even if the acquire
process had failures – as we still have the old good data around we can and
should generate a cache for (again).
There are probably more instances of this hiding, but all these looked
like the easiest to work with and fix with reasonable (aka net-positive)
effects.
|
|
clang detects that fd isn't set in the ReadWrite case – just that this
is supposed to be catched earlier in this method already, but it doesn't
hurt to make it explicit here as well and clang is happy, too.
Git-Dch: Ignore
|