- Pacman-dev - lists.archlinux.org

[pacman-dev] [PATCH] [RFC] makepkg: calculate exact total file size
by Allan McRae 25 Dec '11

25 Dec '11

The current calculation of the total file size for a package using "du" suffers from issues in portability and correctness. Especially on btrfs, this can result in clearly wrong package information such as: Download Size : 14684.29 KiB Installed Size : 7628.00 KiB Use a slower but more accurate method involving "cat" and "wc" to calculate total file size. Signed-off-by: Allan McRae <allan(a)archlinux.org> --- configure.ac | 1 - scripts/Makefile.am | 1 - scripts/makepkg.sh.in | 3 +-- 3 files changed, 1 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index fb1e0b3..6941054 100644 --- a/configure.ac +++ b/configure.ac @@ -245,7 +245,6 @@ esac AM_CONDITIONAL([CYGWIN], test "x$host_os_cygwin" = "xyes") AM_CONDITIONAL([DARWIN], test "x$host_os_darwin" = "xyes") -AC_PATH_PROGS([DUPATH], [du], [du], [/usr/bin$PATH_SEPARATOR/bin] ) AC_SUBST(SIZECMD) AC_SUBST(SEDINPLACE) AC_SUBST(STRIP_BINARIES) diff --git a/scripts/Makefile.am b/scripts/Makefile.am index 328fbff..727de25 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -53,7 +53,6 @@ edit = sed \ -e 's|@BUILDSCRIPT[@]|$(BUILDSCRIPT)|g' \ -e 's|@SIZECMD[@]|$(SIZECMD)|g' \ -e 's|@SEDINPLACE[@]|$(SEDINPLACE)|g' \ - -e 's|@DUPATH[@]|$(DUPATH)|g' \ -e 's|@SCRIPTNAME[@]|$@|g' \ -e 's|@configure_input[@]|Generated from $@.sh.in; do not edit by hand.|g' diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 8c6984d..c78db86 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1132,8 +1132,7 @@ write_pkginfo() { else local packager="Unknown Packager" fi - local size="$(@DUPATH@ -sk)" - size="$(( ${size%%[^0-9]*} * 1024 ))" + local size="$(find . | xargs cat 2>/dev/null | wc -c)" msg2 "$(gettext "Generating %s file...")" ".PKGINFO" echo "# Generated by makepkg $myver" -- 1.7.8.1

3 6

[pacman-dev] [PATCH] makepkg: rework libprovides
by Allan McRae 25 Dec '11

25 Dec '11

This reworks the implmentation of libprovides for the following benefits: - Moves functionality from write_pkginfo() to find_libprovides() - Only calculates the version for libraries specifically requested and not all libraries. This has the disadvantage of running find over the $pkgdir for as many libraries as needed, but is unlikely to be an issue due to caching. - The order of the provides array in the PKGBUILD is kept in the package - There are more warning messages when things fail and those that were there are no longer errors (as I do not think failure of libprovides should result in complete packaging failure) - It is now modular so can be easy extended to other library types other than ELF *.so. Signed-off-by: Allan McRae <allan(a)archlinux.org> --- Fixed version. scripts/makepkg.sh.in | 89 +++++++++++++++++++++++++++--------------------- 1 files changed, 50 insertions(+), 39 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 64b33c6..8c6984d 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1076,30 +1076,53 @@ find_libdepends() { } find_libprovides() { - local libprovides - find "$pkgdir" -type f -name \*.so\* | while read filename - do - # check if we really have a shared object - if LC_ALL=C readelf -h "$filename" 2>/dev/null | grep -q '.*Type:.*DYN (Shared object file).*'; then - # 64 - soarch=$(LC_ALL=C readelf -h "$filename" | sed -n 's/.*Class.*ELF$32\|64$/\1/p') - # get the string binaries link to: libfoo.so.1.2 -> libfoo.so.1 - sofile=$(LC_ALL=C readelf -d "$filename" 2>/dev/null | sed -n 's/.*Library soname: \[$.*$\].*/\1/p') - [ -z "$sofile" ] && sofile="${filename##*/}" - - # extract the library name: libfoo.so - soname="${sofile%%\.so\.*}.so" - # extract the major version: 1 - soversion="${sofile##*\.so\.}" - if in_array "${soname}" ${provides[@]}; then - if ! in_array "${soname}=${soversion}-${soarch}" ${libprovides[@]}; then - # libfoo.so=1-64 - echo "${soname}=${soversion}-${soarch}" - libprovides=(${libprovides[@]} "${soname}=${soversion}-${soarch}") + local libprovides missing + for p in ${provides[@]}; do + missing=0 + case "$p" in + *.so) + local filename=$(find "$pkgdir" -type f -name $p\*) + if [[ $filename ]]; then + # packages may provide multiple versions of the same library + for fn in ${filename[@]}; do + # check if we really have a shared object + if LC_ALL=C readelf -h "$fn" 2>/dev/null | grep -q '.*Type:.*DYN (Shared object file).*'; then + # get the string binaries link to (e.g. libfoo.so.1.2 -> libfoo.so.1) + local sofile=$(LC_ALL=C readelf -d "$fn" 2>/dev/null | sed -n 's/.*Library soname: \[$.*$\].*/\1/p') + if [[ -z "$sofile" ]]; then + warning "$(gettext "Library listed in %s is not versioned: %s")" "'provides'" "$p" + libprovides=(${libprovides[@]} "$p") + continue + fi + + # get the library architecture (32 or 64 bit) + local soarch=$(LC_ALL=C readelf -h "$fn" | sed -n 's/.*Class.*ELF$32\|64$/\1/p') + + # extract the library major version + local soversion="${sofile##*\.so\.}" + + libprovides=(${libprovides[@]} "${p}=${soversion}-${soarch}") + else + warning "$(gettext "Library listed in %s is not a shared object: %s")" "'provides'" "$p" + libprovides=(${libprovides[@]} "$p") + fi + done + else + libprovides=(${libprovides[@]} "$p") + missing=1 fi - fi - fi + ;; + *) + libprovides=(${libprovides[@]} "$p") + ;; + esac + + if (( missing )); then + warning "$(gettext "Can not find library listed in %s: %s")" "'provides'" "$p" + fi done + + echo ${libprovides[@]} } write_pkginfo() { @@ -1133,13 +1156,15 @@ write_pkginfo() { [[ $groups ]] && printf "group = %s\n" "${groups[@]}" [[ $optdepends ]] && printf "optdepend = %s\n" "${optdepends[@]//+([[:space:]])/ }" [[ $conflicts ]] && printf "conflict = %s\n" "${conflicts[@]}" + + provides=($(find_libprovides)) + [[ $provides ]] && printf "provides = %s\n" "${provides[@]}" + [[ $backup ]] && printf "backup = %s\n" "${backup[@]}" - local it - libprovides=$(find_libprovides) + local it libdepends=$(find_libdepends) - provides=("${provides[@]}" ${libprovides}) depends=("${depends[@]}" ${libdepends}) for it in "${depends[@]}"; do @@ -1155,20 +1180,6 @@ write_pkginfo() { fi done - for it in "${provides[@]}"; do - # ignore versionless entires (those come from the PKGBUILD) - if [[ $it = *.so ]]; then - # check if the entry has been found by find_libprovides - # if not, it's unneeded; tell the user so he can remove it - if [[ ! $libprovides =~ (^|\s)${it}=.* ]]; then - error "$(gettext "Cannot find library listed in %s: %s")" "'provides'" "$it" - return 1 - fi - else - echo "provides = $it" - fi - done - for it in "${packaging_options[@]}"; do local ret="$(check_option $it)" if [[ $ret != "?" ]]; then -- 1.7.8.1

2 1

[pacman-dev] [PATCH] makepkg: rework libprovides
by Allan McRae 25 Dec '11

25 Dec '11

This reworks the implmentation of libprovides for the following benefits: - Moves functionality from write_pkginfo() to find_libprovides() - Only calculates the version for libraries specifically requested and not all libraries. This has the disadvantage of running find over the $pkgdir for as many libraries as needed, but is unlikely to be an issue due to caching. - The order of the provides array in the PKGBUILD is kept in the package - There are more warning messages when things fail and those that were there are no longer errors (as I do not think failure of libprovides should result in complete packaging failure) - It is now modular so can be easy extended to other library types other than ELF *.so. Signed-off-by: Allan McRae <allan(a)archlinux.org> --- scripts/makepkg.sh.in | 85 +++++++++++++++++++++++++++---------------------- 1 files changed, 47 insertions(+), 38 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 64b33c6..08c4384 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1076,30 +1076,51 @@ find_libdepends() { } find_libprovides() { - local libprovides - find "$pkgdir" -type f -name \*.so\* | while read filename - do - # check if we really have a shared object - if LC_ALL=C readelf -h "$filename" 2>/dev/null | grep -q '.*Type:.*DYN (Shared object file).*'; then - # 64 - soarch=$(LC_ALL=C readelf -h "$filename" | sed -n 's/.*Class.*ELF$32\|64$/\1/p') - # get the string binaries link to: libfoo.so.1.2 -> libfoo.so.1 - sofile=$(LC_ALL=C readelf -d "$filename" 2>/dev/null | sed -n 's/.*Library soname: \[$.*$\].*/\1/p') - [ -z "$sofile" ] && sofile="${filename##*/}" + local libprovides missing + for p in ${provides[@]}; do + missing=0 + case "$p" in + *.so) + local filename=$(find "$pkgdir" -type f -name $p\*) + # packages may provide multiple versions of the same library + for fn in ${filename[@]}; do + # check if we really have a shared object + if LC_ALL=C readelf -h "$fn" 2>/dev/null | grep -q '.*Type:.*DYN (Shared object file).*'; then + # get the string binaries link to (e.g. libfoo.so.1.2 -> libfoo.so.1) + local sofile=$(LC_ALL=C readelf -d "$fn" 2>/dev/null | sed -n 's/.*Library soname: \[$.*$\].*/\1/p') + if [[ -z "$sofile" ]]; then + warning "$(gettext "Library listed in %s is not versioned: %s")" "'provides'" "$p" + libprovides=(${libprovides[@]} "$p") + continue + fi - # extract the library name: libfoo.so - soname="${sofile%%\.so\.*}.so" - # extract the major version: 1 - soversion="${sofile##*\.so\.}" - if in_array "${soname}" ${provides[@]}; then - if ! in_array "${soname}=${soversion}-${soarch}" ${libprovides[@]}; then - # libfoo.so=1-64 - echo "${soname}=${soversion}-${soarch}" - libprovides=(${libprovides[@]} "${soname}=${soversion}-${soarch}") + # get the library architecture (32 or 64 bit) + local soarch=$(LC_ALL=C readelf -h "$fn" | sed -n 's/.*Class.*ELF$32\|64$/\1/p') + + # extract the library major version + local soversion="${sofile##*\.so\.}" + + libprovides=(${libprovides[@]} "${p}=${soversion}-${soarch}") + else + warning "$(gettext "Library listed in %s is not a shared object: %s")" "'provides'" "$p" + libprovides=(${libprovides[@]} "$p") + fi + else + libprovides=(${libprovides[@]} "$p") + missing=1 fi - fi - fi + ;; + *) + libprovides=(${libprovides[@]} "$p") + ;; + esac + + if (( missing )); then + warning "$(gettext "Can not find library listed in %s: %s")" "'provides'" "$p" + fi done + + echo ${libprovides[@]} } write_pkginfo() { @@ -1133,13 +1154,15 @@ write_pkginfo() { [[ $groups ]] && printf "group = %s\n" "${groups[@]}" [[ $optdepends ]] && printf "optdepend = %s\n" "${optdepends[@]//+([[:space:]])/ }" [[ $conflicts ]] && printf "conflict = %s\n" "${conflicts[@]}" + + provides=("$(find_libprovides)") + [[ $provides ]] && printf "provides = %s\n" "${provides[@]}" + [[ $backup ]] && printf "backup = %s\n" "${backup[@]}" - local it - libprovides=$(find_libprovides) + local it libdepends=$(find_libdepends) - provides=("${provides[@]}" ${libprovides}) depends=("${depends[@]}" ${libdepends}) for it in "${depends[@]}"; do @@ -1155,20 +1178,6 @@ write_pkginfo() { fi done - for it in "${provides[@]}"; do - # ignore versionless entires (those come from the PKGBUILD) - if [[ $it = *.so ]]; then - # check if the entry has been found by find_libprovides - # if not, it's unneeded; tell the user so he can remove it - if [[ ! $libprovides =~ (^|\s)${it}=.* ]]; then - error "$(gettext "Cannot find library listed in %s: %s")" "'provides'" "$it" - return 1 - fi - else - echo "provides = $it" - fi - done - for it in "${packaging_options[@]}"; do local ret="$(check_option $it)" if [[ $ret != "?" ]]; then -- 1.7.8.1

2 2

[pacman-dev] [PATCH] Prevent rebuild of man pages when using release tarballs
by Allan McRae 24 Dec '11

24 Dec '11

Commit 43cad9c8 made the building of all docs depend on the Makefile. However, the Makefile is generated after running ./configure so is always newer than any pregenerated docs. This means that people building from released pacman tarballs are forced to rebuild the docs (and thus have asciidoc installed). That defeats the purpose of prebuilding the documentation. Have the documentatin depends on Makefile.am instead as this is probably what was intended. Signed-off-by: Allan McRae <allan(a)archlinux.org> --- Two things: 1) That was a massive commit message for the addition of 9 characters! 2) Somebody did not build the Arch Linux pacman package in a clean chroot or else failures due to missing asciidoc would have occurred... doc/Makefile.am | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/Makefile.am b/doc/Makefile.am index 2c6cc50..a2018c7 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -120,7 +120,7 @@ A2X_OPTS = \ --destination-dir='./' # These rules are due to the includes and files of the asciidoc text -$(ASCIIDOC_MANS): asciidoc.conf footer.txt Makefile +$(ASCIIDOC_MANS): asciidoc.conf footer.txt Makefile.am $(AM_V_GEN)a2x $(A2X_OPTS) --asciidoc-opts="$(ASCIIDOC_OPTS) --out-file=./$@.xml" $(srcdir)/$@.txt %.html: %.txt @@ -132,8 +132,8 @@ HACKING.html: ../HACKING sed -e 's/\r$$//' > $@ # Customizations for certain HTML docs -$(HTML_MANPAGES): asciidoc.conf footer.txt Makefile -$(HTML_OTHER): asciidoc.conf Makefile +$(HTML_MANPAGES): asciidoc.conf footer.txt Makefile.am +$(HTML_OTHER): asciidoc.conf Makefile.am %.html: ASCIIDOC_OPTS += -a linkcss -a toc -a icons -a max-width=960px -a stylesheet=asciidoc-override.css %.8.html: ASCIIDOC_OPTS += -d manpage %.5.html: ASCIIDOC_OPTS += -d manpage -- 1.7.8.1

1 0

[pacman-dev] [PATCH 1/2] be_package: be more explicit parsing key/value pairs
by Dan McGee 24 Dec '11

24 Dec '11

This eliminates the need for strtrim() usage completely, instead relying on the fact that the only allowed delimiter between key and value is the " = " string. Signed-off-by: Dan McGee <dan(a)archlinux.org> --- lib/libalpm/be_package.c | 21 ++++++++++++--------- 1 files changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c index 41ecc75..c602996 100644 --- a/lib/libalpm/be_package.c +++ b/lib/libalpm/be_package.c @@ -171,18 +171,21 @@ static int parse_descfile(alpm_handle_t *handle, struct archive *a, alpm_pkg_t * size_t len = _alpm_strip_newline(buf.line); linenum++; - if(len == 0 || buf.line[0] == '#') { + key = buf.line; + if(len == 0 || key[0] == '#') { continue; } - ptr = buf.line; - key = strsep(&ptr, "="); - if(key == NULL || ptr == NULL) { - _alpm_log(handle, ALPM_LOG_DEBUG, "%s: syntax error in description file line %d\n", - newpkg->name ? newpkg->name : "error", linenum); + /* line is always in this format: "key = value" + * we can be sure the " = " exists, so look for that */ + ptr = memchr(key, ' ', len); + if(!ptr || ptr - key + 2 > len || memcmp(ptr, " = ", 3) != 0) { + _alpm_log(handle, ALPM_LOG_DEBUG, + "%s: syntax error in description file line %d\n", + newpkg->name ? newpkg->name : "error", linenum); } else { - key = _alpm_strtrim(key); - while(*ptr == ' ') ptr++; - ptr = _alpm_strtrim(ptr); + /* NULL the end of the key portion, move ptr to start of value */ + *ptr = '\0'; + ptr += 3; if(strcmp(key, "pkgname") == 0) { STRDUP(newpkg->name, ptr, return -1); newpkg->name_hash = _alpm_hash_sdbm(newpkg->name); -- 1.7.8.1

2 2

[pacman-dev] [PATCH 1/3] pacman/util: return size_t from strtrim
by Dave Reisner 23 Dec '11

23 Dec '11

Instead of returning the same value as the parameter to this function, return the length of the string, which can be useful to the caller when its non-zero (e.g. to find the end of the string). Signed-off-by: Dave Reisner <dreisner(a)archlinux.org> --- This should make an earlier email from dan make a bit more sense when he referred to "the new return value" src/pacman/util.c | 18 +++++++++--------- src/pacman/util.h | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 7846291..4160c44 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -325,13 +325,13 @@ char *strtoupper(char *str) /* Trim whitespace and newlines from a string */ -char *strtrim(char *str) +size_t strtrim(char *str) { - char *pch = str; + char *end, *pch = str; if(str == NULL || *str == '\0') { /* string is empty, so we're done. */ - return str; + return 0; } while(isspace((unsigned char)*pch)) { @@ -348,16 +348,16 @@ char *strtrim(char *str) /* check if there wasn't anything but whitespace in the string. */ if(*str == '\0') { - return str; + return 0; } - pch = (str + (strlen(str) - 1)); - while(isspace((unsigned char)*pch)) { - pch--; + end = (str + strlen(str) - 1); + while(isspace((unsigned char)*end)) { + end--; } - *++pch = '\0'; + *++end = '\0'; - return str; + return end - pch; } /* Replace all occurances of 'needle' with 'replace' in 'str', returning diff --git a/src/pacman/util.h b/src/pacman/util.h index 6ec962f..6291939 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -56,7 +56,7 @@ const char *mbasename(const char *path); char *mdirname(const char *path); void indentprint(const char *str, size_t indent); char *strtoupper(char *str); -char *strtrim(char *str); +size_t strtrim(char *str); char *strreplace(const char *str, const char *needle, const char *replace); alpm_list_t *strsplit(const char *str, const char splitchar); void string_display(const char *title, const char *string); -- 1.7.8.1

1 2

[pacman-dev] [PATCH] Convert strtrim/strlen paired calls to only strtrim
by Dan McGee 23 Dec '11

23 Dec '11

This utilizes the new return value so we don't have to find the length of the string again. Signed-off-by: Dan McGee <dan(a)archlinux.org> --- src/pacman/conf.c | 3 +-- src/pacman/util.c | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 117aecd4..7ba2791 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -726,8 +726,7 @@ static int _parseconfig(const char *file, struct section_t *section, *ptr = '\0'; } - strtrim(line); - line_len = strlen(line); + line_len = strtrim(line); if(line_len == 0) { continue; diff --git a/src/pacman/util.c b/src/pacman/util.c index 4160c44..467bedf 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1228,14 +1228,14 @@ static int multiselect_parse(char *array, int count, char *response) for(str = response; ; str = NULL) { int include = 1; int start, end; + size_t len; char *ends = NULL; char *starts = strtok_r(str, " ", &saveptr); if(starts == NULL) { break; } - strtrim(starts); - int len = strlen(starts); + len = strtrim(starts); if(len == 0) continue; @@ -1314,6 +1314,7 @@ int multiselect_question(char *array, int count) if(fgets(response, response_len, stdin)) { const size_t response_incr = 64; + size_t len; /* handle buffer not being large enough to read full line case */ while(*lastchar == '\0' && lastchar[-1] != '\n') { response_len += response_incr; @@ -1330,8 +1331,9 @@ int multiselect_question(char *array, int count) return -1; } } - strtrim(response); - if(strlen(response) > 0) { + + len = strtrim(response); + if(len > 0) { if(multiselect_parse(array, count, response) == -1) { /* only loop if user gave an invalid answer */ continue; @@ -1374,8 +1376,8 @@ int select_question(int count) flush_term_input(); if(fgets(response, sizeof(response), stdin)) { - strtrim(response); - if(strlen(response) > 0) { + size_t len = strtrim(response); + if(len > 0) { int n; if(parseindex(response, &n, 1, count) != 0) continue; @@ -1423,8 +1425,8 @@ static int question(short preset, char *fmt, va_list args) flush_term_input(); if(fgets(response, sizeof(response), stdin)) { - strtrim(response); - if(strlen(response) == 0) { + size_t len = strtrim(response); + if(len == 0) { return preset; } -- 1.7.8.1

1 0

[pacman-dev] [PATCH] pacman-key: Add some variable quotes and fix vim modeline for securemodelines
by Timothy Redaelli 23 Dec '11

23 Dec '11

From: Timothy Redaelli <timothy.redaelli(a)gmail.com> Signed-off-by: Timothy Redaelli <timothy.redaelli(a)gmail.com> --- scripts/pacman-key.sh.in | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index e0f48f5..c033da4 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -450,9 +450,9 @@ fi # file, falling back on a hard default PACMAN_KEYRING_DIR=${PACMAN_KEYRING_DIR:-$(get_from "$CONFIG" "GPGDir" "@sysconfdir@/pacman.d/gnupg")} -GPG_PACMAN=(gpg --homedir ${PACMAN_KEYRING_DIR} --no-permission-warning) +GPG_PACMAN=(gpg --homedir "${PACMAN_KEYRING_DIR}" --no-permission-warning) if [[ -n ${KEYSERVER} ]]; then - GPG_PACMAN+=(--keyserver ${KEYSERVER}) + GPG_PACMAN+=(--keyserver "${KEYSERVER}") fi # check only a single operation has been given @@ -492,11 +492,11 @@ fi (( POPULATE )) && populate_keyring (( RECEIVE )) && "${GPG_PACMAN[@]}" --recv-keys "${KEYIDS[@]}" (( REFRESH )) && "${GPG_PACMAN[@]}" --refresh-keys "${KEYIDS[@]}" -(( VERIFY )) && "${GPG_PACMAN[@]}" --verify $SIGNATURE +(( VERIFY )) && "${GPG_PACMAN[@]}" --verify "$SIGNATURE" if (( UPDATEDB )); then msg "$(gettext "Updating trust database...")" "${GPG_PACMAN[@]}" --batch --check-trustdb fi -# vim: set ts=2 sw=2 noet: +# vim: set ts=2 sw=2 noet -- 1.7.8.1

3 3

[pacman-dev] [PATCH] Rework package removal code
by Dan McGee 23 Dec '11

23 Dec '11

Extract the actual unlinking of files into a new method, which eliminates a goto used for flow control. Also fix up a few small issues in the code: * Unnecessary (unsigned long) cast, use '%zd' instead * Total up errors returned from unlink_file calls and return to caller * Be consistent with scriptlets- we run pre_remove on dbonly, so we should also run post_remove. Both can be disabled by way of the --noscriptlet argument. * Don't pass an invalid pointer to oldpkg to the event callbacks; instead call the callback before we free the object. Signed-off-by: Dan McGee <dan(a)archlinux.org> --- lib/libalpm/remove.c | 99 +++++++++++++++++++++++++++----------------------- 1 files changed, 54 insertions(+), 45 deletions(-) diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 8967eff..b43d1a6 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -354,38 +354,15 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg, return 0; } -int _alpm_remove_single_package(alpm_handle_t *handle, +static int remove_package_files(alpm_handle_t *handle, alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg, size_t targ_count, size_t pkg_count) { alpm_list_t *skip_remove; - size_t filenum = 0, position = 0; - const char *pkgname = oldpkg->name; - const char *pkgver = oldpkg->version; alpm_filelist_t *filelist; - size_t i; - - if(newpkg) { - _alpm_log(handle, ALPM_LOG_DEBUG, "removing old package first (%s-%s)\n", - pkgname, pkgver); - } else { - EVENT(handle, ALPM_EVENT_REMOVE_START, oldpkg, NULL); - _alpm_log(handle, ALPM_LOG_DEBUG, "removing package %s-%s\n", - pkgname, pkgver); - - /* run the pre-remove scriptlet if it exists */ - if(alpm_pkg_has_scriptlet(oldpkg) && - !(handle->trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { - char *scriptlet = _alpm_local_db_pkgpath(handle->db_local, - oldpkg, "install"); - _alpm_runscriptlet(handle, scriptlet, "pre_remove", pkgver, NULL, 0); - free(scriptlet); - } - } - - if(handle->trans->flags & ALPM_TRANS_FLAG_DBONLY) { - goto db; - } + size_t i, filenum = 0, position = 0; + int err = 0; + int nosave = handle->trans->flags & ALPM_TRANS_FLAG_NOSAVE; if(newpkg) { alpm_filelist_t *newfiles; @@ -416,32 +393,33 @@ int _alpm_remove_single_package(alpm_handle_t *handle, alpm_file_t *file = filelist->files + i; if(!can_remove_file(handle, file, skip_remove)) { _alpm_log(handle, ALPM_LOG_DEBUG, - "not removing package '%s', can't remove all files\n", pkgname); + "not removing package '%s', can't remove all files\n", + oldpkg->name); + FREELIST(skip_remove); RET_ERR(handle, ALPM_ERR_PKG_CANT_REMOVE, -1); } filenum++; } - _alpm_log(handle, ALPM_LOG_DEBUG, "removing %ld files\n", (unsigned long)filenum); + _alpm_log(handle, ALPM_LOG_DEBUG, "removing %zd files\n", filenum); if(!newpkg) { /* init progress bar, but only on true remove transactions */ - PROGRESS(handle, ALPM_PROGRESS_REMOVE_START, pkgname, 0, + PROGRESS(handle, ALPM_PROGRESS_REMOVE_START, oldpkg->name, 0, pkg_count, targ_count); } /* iterate through the list backwards, unlinking files */ for(i = filelist->count; i > 0; i--) { alpm_file_t *file = filelist->files + i - 1; - int percent; - /* TODO: check return code and handle accordingly */ - unlink_file(handle, oldpkg, newpkg, file, skip_remove, - handle->trans->flags & ALPM_TRANS_FLAG_NOSAVE); + if(unlink_file(handle, oldpkg, newpkg, file, skip_remove, nosave) < 0) { + err++; + } if(!newpkg) { /* update progress bar after each file */ - percent = (position * 100) / filenum; - PROGRESS(handle, ALPM_PROGRESS_REMOVE_START, pkgname, + int percent = (position * 100) / filenum; + PROGRESS(handle, ALPM_PROGRESS_REMOVE_START, oldpkg->name, percent, pkg_count, targ_count); } position++; @@ -450,20 +428,56 @@ int _alpm_remove_single_package(alpm_handle_t *handle, if(!newpkg) { /* set progress to 100% after we finish unlinking files */ - PROGRESS(handle, ALPM_PROGRESS_REMOVE_START, pkgname, 100, + PROGRESS(handle, ALPM_PROGRESS_REMOVE_START, oldpkg->name, 100, pkg_count, targ_count); + } + + return err; +} + +int _alpm_remove_single_package(alpm_handle_t *handle, + alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg, + size_t targ_count, size_t pkg_count) +{ + const char *pkgname = oldpkg->name; + const char *pkgver = oldpkg->version; + + if(newpkg) { + _alpm_log(handle, ALPM_LOG_DEBUG, "removing old package first (%s-%s)\n", + pkgname, pkgver); + } else { + EVENT(handle, ALPM_EVENT_REMOVE_START, oldpkg, NULL); + _alpm_log(handle, ALPM_LOG_DEBUG, "removing package %s-%s\n", + pkgname, pkgver); - /* run the post-remove script if it exists */ + /* run the pre-remove scriptlet if it exists */ if(alpm_pkg_has_scriptlet(oldpkg) && !(handle->trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { char *scriptlet = _alpm_local_db_pkgpath(handle->db_local, oldpkg, "install"); - _alpm_runscriptlet(handle, scriptlet, "post_remove", pkgver, NULL, 0); + _alpm_runscriptlet(handle, scriptlet, "pre_remove", pkgver, NULL, 0); free(scriptlet); } } -db: + if(!(handle->trans->flags & ALPM_TRANS_FLAG_DBONLY)) { + /* TODO check returned errors if any */ + remove_package_files(handle, oldpkg, newpkg, targ_count, pkg_count); + } + + /* run the post-remove script if it exists */ + if(!newpkg && alpm_pkg_has_scriptlet(oldpkg) && + !(handle->trans->flags & ALPM_TRANS_FLAG_NOSCRIPTLET)) { + char *scriptlet = _alpm_local_db_pkgpath(handle->db_local, + oldpkg, "install"); + _alpm_runscriptlet(handle, scriptlet, "post_remove", pkgver, NULL, 0); + free(scriptlet); + } + + if(!newpkg) { + EVENT(handle, ALPM_EVENT_REMOVE_DONE, oldpkg, NULL); + } + /* remove the package from the database */ _alpm_log(handle, ALPM_LOG_DEBUG, "updating database\n"); _alpm_log(handle, ALPM_LOG_DEBUG, "removing database entry '%s'\n", pkgname); @@ -477,11 +491,6 @@ db: pkgname); } - if(!newpkg) { - /* TODO: awesome! we're passing invalid pointers. */ - EVENT(handle, ALPM_EVENT_REMOVE_DONE, oldpkg, NULL); - } - return 0; } -- 1.7.8.1

1 0

[pacman-dev] [PATCH] allow querying fileowner for directories
by andrew.gregory.8＠gmail.com 23 Dec '11

23 Dec '11

From: Andrew Gregory <andrew.gregory.8(a)gmail.com> Not allowing fileowner queries for directories was an unnecessary limitation. Queries for directories have poor performance due to having to call real_path on every directory listed in every package's file list. Because more than one package will likely own a given directory, all packages are now checked instead of bailing out after the first owner is found. Signed-off-by: Andrew Gregory <andrew.gregory.8(a)gmail.com> --- src/pacman/query.c | 17 +++++++++-------- 1 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/pacman/query.c b/src/pacman/query.c index 4c2ea81..8162662 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -127,7 +127,7 @@ static int query_fileowner(alpm_list_t *targets) * iteration. */ root = alpm_option_get_root(config->handle); rootlen = strlen(root); - if(rootlen + 1 > PATH_MAX) { + if(rootlen >= PATH_MAX) { /* we are in trouble here */ pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), root, ""); return 1; @@ -164,12 +164,13 @@ static int query_fileowner(alpm_list_t *targets) } } + /* make sure directories end with '/' */ if(S_ISDIR(buf.st_mode)) { - pm_printf(ALPM_LOG_ERROR, - _("cannot determine ownership of directory '%s'\n"), filename); - ret++; - free(filename); - continue; + int fnlen = strlen(filename); + if(filename[fnlen-1] != '/'){ + filename = realloc(filename, sizeof(*filename) * (fnlen+2)); + strcat(filename, "/"); + } } bname = mbasename(filename); @@ -192,7 +193,7 @@ static int query_fileowner(alpm_list_t *targets) } free(dname); - for(i = alpm_db_get_pkgcache(db_local); i && !found; i = alpm_list_next(i)) { + for(i = alpm_db_get_pkgcache(db_local); i ; i = alpm_list_next(i)) { alpm_pkg_t *info = i->data; alpm_filelist_t *filelist = alpm_pkg_get_files(info); size_t j; @@ -214,7 +215,7 @@ static int query_fileowner(alpm_list_t *targets) continue; } - if(rootlen + 1 + strlen(pkgfile) > PATH_MAX) { + if(rootlen + strlen(pkgfile) >= PATH_MAX) { pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), root, pkgfile); } /* concatenate our file and the root path */ -- 1.7.7.3

4 9