We did a good job checking this in add.c, but not necessarily anywhere
else. Fix this up by adding checks into dload.c, remove.c, and conf.c in
the frontend. Also add loggers where appropriate and make the message
syntax more consistent.
Signed-off-by: Dan McGee <dan@archlinux.org>
This accomplishes quite a few things with one rather invasive change.
1. Iteration is much more performant, due to a reduction in pointer
chasing and linear item access.
2. Data structures are smaller- we no longer have the overhead of the
linked list as the file struts are now laid out consecutively in
memory.
3. Memory allocation has been massively reworked. Before, we would
allocate three different pieces of memory per file item- the list
struct, the file struct, and the copied filename. What this resulted
in was massive fragmentation of memory when loading filelists since
the memory allocator had to leave holes all over the place. The new
situation here now removes the need for any list item allocation;
allocates the file structs in contiguous memory (and reallocs as
necessary), leaving only the strings as individually allocated. Tests
using valgrind (massif) show some pretty significant memory
reductions on the worst case `pacman -Ql > /dev/null` (366387 files
on my machine):
Before:
Peak heap: 54,416,024 B
Useful heap: 36,840,692 B
Extra heap: 17,575,332 B
After:
Peak heap: 38,004,352 B
Useful heap: 28,101,347 B
Extra heap: 9,903,005 B
Several small helper methods have been introduced, including a list to
array conversion helper as well as a filelist merge sort that works
directly on arrays.
Signed-off-by: Dan McGee <dan@archlinux.org>
This addresses FS#25141. We shouldn't remove every empty directory we
come across during the removal process unless it is truly not known to
any other package. This will prevent removal of essential directories
such as '/var/lock/'.
This is accomplished by first checking the empty/non-empty status of a
directory, which was previously done implicitly by calling rmdir() and
ignoring errors. We do this to avoid the next (new) check in most cases,
which is to look at all local packages to see if the to-be-removed
directory is present in another packages' filelist. If we do not find it
anywhere, then we remove it, else we keep the file around.
The pactest has been updated to test more cases, as well as finding a
flaw in the original expected to fail case- we need separate DIR and
FILE based EXIST rules.
Signed-off-by: Dan McGee <dan@archlinux.org>
The bulk of this commit is adding new tests to ensure the new behavior
works without disrupting old behavior. This is a relatively sane maneuver
when a package adds a conf file (e.g. '/etc/mercurial/hgrc') that was
not previously in the package, but it is placed in the backup array. In
essence, we can treat the existing file as having always been a part of
the package and do our normal compare/install as pacnew logic checks.
Signed-off-by: Dan McGee <dan@archlinux.org>
This code duplication has always been a rather clumsy casuality of
fixing some past upgrade issues. Unify the removal code across upgrade
and remove operations into a new _alpm_remove_single_package() method
wihch makes it very clear how we handle upgrade and remove differently,
via several conditionals on newpkg.
This commit highlights interesting behavior such as the fact that the
implicit removal in every package upgrade never gets transaction events
or progress callbacks.
Signed-off-by: Dan McGee <dan@archlinux.org>
We can readily detect the first node in a list by checking if
node->prev->next is NULL. So there is no need to pass the head
of the list to this function and its prototype now looks like
all the other item accessors.
Signed-off-by: Allan McRae <allan@archlinux.org>
Signed-off-by: Dan McGee <dan@archlinux.org>
This allows us to capture size and mode data when building filelists
from package files. Future patches will take advantage of this newly
available information, and frontends can use it as well.
Signed-off-by: Dan McGee <dan@archlinux.org>
This saves replicating the potentially large list of files in a package
that is being removed.
Signed-off-by: Allan McRae <allan@archlinux.org>
Signed-off-by: Dan McGee <dan@archlinux.org>
We have just looped through the list of files, so might as well get
the count as we go.
Signed-off-by: Allan McRae <allan@archlinux.org>
Signed-off-by: Dan McGee <dan@archlinux.org>
This allows us to separate the name and hash elements in one place and
not scatter different parsing code all over the place, including both
the frontend and backend.
Signed-off-by: Dan McGee <dan@archlinux.org>
We didn't do due diligence before and ensure prior pm_errno values
weren't influencing what happened in further ALPM calls. I observed one
case of early setup code setting pm_errno to PM_ERR_WRONG_ARGS and that
flag persisting the entire time we were calling library code.
Add a new CHECK_HANDLE() macro that does two things: 1) ensures the
handle variable passed to it is non-NULL and 2) clears any existing
pm_errno flag set on the handle. This macro can replace many places we
used the ASSERT(handle != NULL, ...) pattern before.
Several other other places only need a simple 'set to zero' of the
pm_errno field.
Signed-off-by: Dan McGee <dan@archlinux.org>
* Move several variables into better scope
* const-ify a few variables
* Avoid duplicating filelists if it is unnecessary
* Better handling out out of memory condition when adding file conflicts
to our list
Signed-off-by: Dan McGee <dan@archlinux.org>
This is the last user of our global handle object. Once again the diff
is large but the functional changes are not.
Signed-off-by: Dan McGee <dan@archlinux.org>
This makes these functions consistent with the rest of the transaction
related API calls. We do an additional assert to ensure the handle
attached to the package is the same as the handle passed in.
Signed-off-by: Dan McGee <dan@archlinux.org>
The few remaining instances were utilized for buffers in calls to
snprintf() and realpath(). Both of these functions will always ensure
the returned value is padded with '\0', so there is no need for the
extra byte.
Signed-off-by: Dan McGee <dan@archlinux.org>
This will make the patching process less invasive as we start to remove
this variable from all source files.
Signed-off-by: Dan McGee <dan@archlinux.org>
The usefulness of this is rather limited due to it not being compiled
into production builds. When you do choose to see the output, it is
often overwhelming and not helpful. The best bet is to use a debugger
and/or well-placed fprintf() statements.
Signed-off-by: Dan McGee <dan@archlinux.org>
Issue FS#24230. If a symlink is broken and included in the removal
process of a package, we blew up and segfaulted due to
alpm_compute_md5sum() returning NULL and then performing a strcmp()
operation.
Signed-off-by: Dan McGee <dan@archlinux.org>
This does touch a lot of things, and hopefully doesn't break things on
other platforms, but allows us to also clean up a bunch of crud that no
longer needs to be there.
Signed-off-by: Dan McGee <dan@archlinux.org>
This is the standard, and we have had a few of these introduced lately
that should not be here.
Done with:
find -name '*.c' | xargs sed -i -e 's#if (#if(#g'
find -name '*.c' | xargs sed -i -e 's#while (#while(#g'
Signed-off-by: Dan McGee <dan@archlinux.org>
The current state of the code does not allow to see immediately
that it returns a list of pmdepmissing_t structures.
Signed-off-by: Dan McGee <dan@archlinux.org>
This was discussed and more or less agreed upon on the mailing list. A
huge checkin, but if we just do it and let people adjust the pain will
end soon enough. Rebasing should be relatively straighforward for anyone
that sees conflicts; just be sure you use the new return style if
possible.
The following semantic patch was used to do the change, along with some
hand-massaging in order to preserve parenthesis where appropriate:
The semantic match that finds this problem is as follows, although some
hand-massaging was done in order to keep parenthesis where appropriate:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression a;
@@
- return(a);
+ return a;
// </smpl>
A macros_file was also provided with the following content:
Additional steps taken, mainly for ASSERT() macros:
$ sed -i -e 's#return(NULL)#return NULL#' lib/libalpm/*.c
$ sed -i -e 's#return(-1)#return -1#' lib/libalpm/*.c
Signed-off-by: Dan McGee <dan@archlinux.org>
Read the package information for sync/local databases into a pmpkghash_t
structure.
Provide a alpm_db_get_pkgcache_list() method that returns the list from
the hash object. Most usages of alpm_db_get_pkgcache are converted to
this at this stage for ease of implementation. Review whether these are
better accessing the hash table directly at a later stage.
Signed-off-by: Allan McRae <allan@archlinux.org>
It's likely that these interfaces will break sooner or later, now that
pacman no longer uses them.
So better force the two people who use them to migrate their code to the
new add_pkg/remove_pkg interface, which is very easy anyway.
Signed-off-by: Xavier Chantry <chantry.xavier@gmail.com>
For consistency with alpm_add_pkg.
The new recommended interface is alpm_add_pkg / alpm_remove_pkg, all
others interfaces are deprecated.
Signed-off-by: Xavier Chantry <chantry.xavier@gmail.com>
Due to the way we funk around with package data loading, we had a condition
where the filelist got doubled up because it was loaded twice.
Packages are originally loaded with INFRQ_BASE. In an upgrade/sync, the
package is checked for file conflicts next, leaving us in an "INFRQ_BASE |
INFRQ_FILES" state. Later, when committing a single package, we have an
explicit call to _alpm_local_db_read() with INFRQ_ALL as the level. Because
the package's level did not match this, we skipped over our previous "does
the incoming level match where I'm at" shortcut, and continued to load
things again, because of a lack of fine-grained checking for each of DESC,
FILES, and INSTALL.
The end result is we loaded the filelist twice, causing our remove logic to
iterate twice over the installed files, spewing a bunch of "cannot find file
X" messages.
Fix the problem by doing a bit more bitmasking logic throughout the load
method, and also fix the sanity check at the beginning of the function- this
should *only* be used for local packages as opposed to the "not a package"
check that was there before.
A debug log message was added to upgraderemove as well to match the one
already in the normal remove codepath.
Signed-off-by: Dan McGee <dan@archlinux.org>
