- Pacman-dev - lists.archlinux.org

[pacman-dev] [PATCH] Document --debug
by Nagy Gabor 22 Mar '09

22 Mar '09

>From c121fa3c9ad702f4daa0f1156869573673d749c7 Mon Sep 17 00:00:00 2001 From: Nagy Gabor <ngaba(a)bibl.u-szeged.hu> Date: Sun, 22 Mar 2009 17:13:29 +0100 Subject: [PATCH] Document --debug After some irc/forum experiences, I decided to document this option. However, I left the debug-level undocumented (--debug=2). Signed-off-by: Nagy Gabor <ngaba(a)bibl.u-szeged.hu> --- doc/pacman.8.txt | 6 +++++- src/pacman/pacman.c | 1 + 2 files changed, 6 insertions(+), 1 deletions(-) diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index c574872..95880a2 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -124,9 +124,13 @@ Options command line or in linkman:pacman.conf[5], their default location will be inside this root path. -*-v, --verbose*:: +*-v, \--verbose*:: Output paths such as as the Root, Conf File, DB Path, Cache Dirs, etc. +*\--debug*:: + Display debug messages. When reporting bugs, this option is recommended + to be used. + *\--cachedir* <'dir'>:: Specify an alternative package cache location (a typical default is ``/var/cache/pacman/pkg''). Multiple cache directories can be specified, diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 59916d6..14cfdfa 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -143,6 +143,7 @@ static void usage(int op, const char * const myname) printf(_(" --noprogressbar do not show a progress bar when downloading files\n")); printf(_(" --noscriptlet do not execute the install scriptlet if one exists\n")); printf(_(" -v, --verbose be verbose\n")); + printf(_(" --debug display debug messages\n")); printf(_(" -r, --root <path> set an alternate installation root\n")); printf(_(" -b, --dbpath <path> set an alternate database location\n")); printf(_(" --cachedir <dir> set an alternate package cache location\n")); -- 1.6.0.3

1 0

[pacman-dev] patch submission help
by Jeff 18 Mar '09

18 Mar '09

In the last month I have created and tested [1] a patch for pacman-3.2.2 that allows for 3 different PKGBUILD functions: build() check() inst(). This allows for a new option in makepkg.conf called check which is expected to default to !check (and overridden for toolchain packages). build() and check() are never run in fakeroot, only inst() (with exception for !fakeroot or --asroot, of course) which conforms to the fakeroot recommendation. My attempt was to make the patch backwards compatible so that if any of the functions weren't present, it was not an error. However, I realized while writing this email that this means that a PKGBUILD with just build() would run completely in a non-fakeroot environment which needs re-thinking. Anyway, it's a rather small patch as seen below: $ grep ^- makepkg.patch | grep -v ^--- |wc -l 16 $ grep ^+ makepkg.patch | grep -v ^+++ |wc -l 33 The reason for this email, though, is that I subbed to this list to watch how things are done, learn protocol, etc. So I grabbed the git repo and started looking at how I was going to apply this patch since enough has changed to cause all but 1 hunk to fail. I am not familiar with git, preferring svn, but can read docs. Where I'm tripped up is realizing that head doesn't reflect what private branches might hold. One person (sorry, I forget who) mentioned having a branch with many makepkg changes in it which would cause me to have to hand apply these changes again. I have no interest in doing that, so I'm asking: does this feature seem like it would be worthwhile (read; would the patch be accepted)? If so, how do I access private branches so my branch doesn't conflict with them? [1] I am building a base system with the new PKGBUILD format and am about 3/4 of the way done with no known makepkg errors (running makepkg with sh -x for extra detail) -- Jeff My other computer is an abacus.

7 8

Re: [pacman-dev] patch submission help
by Jeff 18 Mar '09

18 Mar '09

On Wed, Mar 18, 2009 at 10:03:26AM +1000, Allan McRae wrote: > > What exactly is dangerous about this? There are two options > 1) build() and package() functions - here only package() is run in fakeroot > 2) build() function only - build() is run in fakeroot. I apologize. I misread it as build() is never run in fakeroot. > All my changes are now on the pacman master branch so you can just check > that out. See here for further info: > http://www.archlinux.org/pacman/#_development Sweet. > The check() function should serve as a major hint that it should be > run. Perhaps, if it is present and not invoked using --check then a > warning could be printed. A warning does sound nice, but I was referring to a hint that a testsuite *should* be run. Obviously the toolchain would be in that category whereas some would argue things like sed's testsuite is minor and generally not worth the time or trouble to run. Personally this wouldn't affect me as I run every testsuite. -- Jeff My other computer is an abacus.

1 0

Re: [pacman-dev] patch submission help
by Jeff 18 Mar '09

18 Mar '09

On Tue, Mar 17, 2009 at 11:17:20AM +1000, Allan McRae wrote: > > Note that with the changes that are now in the pacmans git repo, there > is support for build() and package() functions in PKGBUILDs. When the > package() function is present, it is run under fakeroot but the build() > function is not. Otherwise it is backwards compatible. This can be dangerous, unless the pacman devs personally undertake the migration of all extant PKGBUILDs. Since I just realized the problem yesterday, I have not had time to think about the best fix, but I think the fix should be rather trivial at least in reference to my patch. I'd really like to see your branch and I could send you my patch (onlist/offlist your choice) as a matter of reference if you would like my input. > I like the idea of an additional check() function. However, I could > implement it slightly differently - instead of a makepkg.conf flag to > run the check() function or not, I think it may be better to add a > --check flag to makepkg so people specify when to run it. >From a technical standpoint, I prefer the --check arg better than putting an option override in a PKGBUILD, but then I also like the option override as a "hint" to the individual who is building packageX that the testsuite _should_ be run. I'll keep using my modified pacman-3.2.2 until I finish the current rebuild, but after that I'd be happy to test the changes. I have no interest in maintaining my own set of patches. :) As a side note, I've modified dozens of PKGBUILDs to implement this change, and I'm rather clueless as to why all the || return 1 statements exist since set -e is in use. Are they cruft left behind from an earlier version of makepkg that didn't bail on the first error? Also, my patch actually just runs run_build() with an argument (the arg being the name of the PKGBUILD function to run) so there was no code duplication. In run_build() I put in a simple: cd "${srcdir}/${pkgname}-${pkgver}" || cd "${srcdir}" as it seemed silly to have a cd command in every single PKGBUILD when a good number of tarballs follow the ${pkgname}-${pkgver} naming scheme. I don't know if you have added that feature or not. Especially with more than one function in PKGBUILD, cd would have to be duplicated (and still will for non-standard source tarballs). A function for determining that name might be useful, though, and would help in all cases except where the build dir != source dir (like glibc). Anyway, just food for thought! -- Jeff My other computer is an abacus.

2 1

[pacman-dev] [PATCH] Refactor testing for color message output
by Allan McRae 15 Mar '09

15 Mar '09

Signed-off-by: Allan McRae <allan(a)archlinux.org> --- scripts/makepkg.sh.in | 16 +++++++++++----- 1 files changed, 11 insertions(+), 5 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 3ff256f..d44fe0d 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -69,6 +69,7 @@ IGNOREARCH=0 HOLDVER=0 PKGFUNC=0 SPLITPKG=0 +COLORMSG=0 # Forces the pkgver of the current PKGBUILD. Used by the fakeroot call # when dealing with svn/cvs/etc PKGBUILDs. @@ -80,7 +81,7 @@ PACMAN_OPTS= plain() { local mesg=$1; shift - if [ -t 2 -a ! "$USE_COLOR" = "n" -a "$(check_buildenv color)" = "y" ]; then + if [ COLORMSG -eq 1 ]; then printf "\033[1;1m ${mesg}\033[1;0m\n" "$@" >&2 else printf " ${mesg}\n" "$@" >&2 @@ -89,7 +90,7 @@ plain() { msg() { local mesg=$1; shift - if [ -t 2 -a ! "$USE_COLOR" = "n" -a "$(check_buildenv color)" = "y" ]; then + if [ COLORMSG -eq 1 ]; then printf "\033[1;32m==>\033[1;0m\033[1;1m ${mesg}\033[1;0m\n" "$@" >&2 else printf "==> ${mesg}\n" "$@" >&2 @@ -98,7 +99,7 @@ msg() { msg2() { local mesg=$1; shift - if [ -t 2 -a ! "$USE_COLOR" = "n" -a "$(check_buildenv color)" = "y" ]; then + if [ COLORMSG -eq 1 ]; then printf "\033[1;34m ->\033[1;0m\033[1;1m ${mesg}\033[1;0m\n" "$@" >&2 else printf " -> ${mesg}\n" "$@" >&2 @@ -107,7 +108,7 @@ msg2() { warning() { local mesg=$1; shift - if [ -t 2 -a ! "$USE_COLOR" = "n" -a "$(check_buildenv color)" = "y" ]; then + if [ COLORMSG -eq 1 ]; then printf "\033[1;33m==> $(gettext "WARNING:")\033[1;0m\033[1;1m ${mesg}\033[1;0m\n" "$@" >&2 else printf "==> $(gettext "WARNING:") ${mesg}\n" "$@" >&2 @@ -116,7 +117,7 @@ warning() { error() { local mesg=$1; shift - if [ -t 2 -a ! "$USE_COLOR" = "n" -a "$(check_buildenv color)" = "y" ]; then + if [ COLORMSG -eq 1 ]; then printf "\033[1;31m==> $(gettext "ERROR:")\033[1;0m\033[1;1m ${mesg}\033[1;0m\n" "$@" >&2 else printf "==> $(gettext "ERROR:") ${mesg}\n" "$@" >&2 @@ -1303,6 +1304,11 @@ if [ ! $(type -t gettext) ]; then } fi +# check if messages are to be printed using color +if [ -t 2 -a ! "$USE_COLOR" = "n" -a "$(check_buildenv color)" = "y" ]; then + COLORMSG=1 +fi + ARGLIST=$@ # Parse Command Line Options. -- 1.6.2

2 3

[pacman-dev] [GIT] The official pacman repository branch, master, updated. v3.2.2-104-g9519d22
by dan＠archlinux.org 15 Mar '09

15 Mar '09

This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "The official pacman repository". The branch, master has been updated via 9519d22df7cef5b5a48a7d1ebee44a9e935e02b7 (commit) via de97282fbd78755cde71c1d064ec00b806efcca5 (commit) via 36863b968f562840461e60be4ed23abf635d6008 (commit) via 687f04ab4699440e3d8b371454b26039b6eb2772 (commit) via a864a50bc6a65a5c39a4727a07a3b269049a808e (commit) from d8d9ab8c870eb9f2df5e6e607b1da34e6e40e29e (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- commit 9519d22df7cef5b5a48a7d1ebee44a9e935e02b7 Author: Xavier Chantry <shiningxc(a)gmail.com> Date: Sun Mar 1 01:40:24 2009 +0100 be_files.c : use %jd instead of %ju for intmax_t Signed-off-by: Xavier Chantry <shiningxc(a)gmail.com> Signed-off-by: Dan McGee <dan(a)archlinux.org> commit de97282fbd78755cde71c1d064ec00b806efcca5 Author: Xavier Chantry <shiningxc(a)gmail.com> Date: Sun Mar 15 12:43:39 2009 +0100 Revert "popen does NOT require /bin/sh in a subchroot" This reverts commit 9558639d8009483fbf422b138d020745986f82f1. This change was wrong, popen does require /bin/sh in a subchroot. 1) pacman -S lilo -r root Notice no error 2) rm root/bin/sh ; pacman -S lilo -r root Notice an error : error: scriptlet failed to execute correctly Actually, we already get an explicit error here, when popen is run, so there is no need to check for bin/sh explicitely. Besides this check was problematic in some cases. For example, bash itself has a scriptlet, but only post_install and post_upgrade, no pre_install and pre_upgrade. However, since bash has a scriptlet, runscriptlet will also be called before bash is installed. It won't do anything since the scriptlet has no pre_install function. But if we keep the check, we will still get "error : no /bin/sh". Conflicts: lib/libalpm/trans.c Signed-off-by: Xavier Chantry <shiningxc(a)gmail.com> Signed-off-by: Dan McGee <dan(a)archlinux.org> commit 36863b968f562840461e60be4ed23abf635d6008 Author: Xavier Chantry <shiningxc(a)gmail.com> Date: Sun Mar 15 15:42:35 2009 +0100 runscriptlet : Flush open fds before fork() to avoid cloning buffers This is a bug I noticed 2 years ago : http://www.nabble.com/Re%3A-logging-output-crazy-to11437357.html#a11479679 I thought I fixed it with 57d77eab32c01cd7572a11f76480a3724d09c029 But the bug was still here. Reading man fork, this part caught my attention: * The child inherits copies of the parent's set of open file descriptors. Each file descriptor in the child refers to the same open file description (see open(2)) as the corresponding file descriptor in the parent. This means that the two descriptors share open file status flags, current file offset, and signal-driven I/O attributes (see the description of F_SETOWN and F_SETSIG in fcntl(2)). Since the open file descriptors are inherited, it is probably a good idea to flush them before forking. Signed-off-by: Xavier Chantry <shiningxc(a)gmail.com> Signed-off-by: Dan McGee <dan(a)archlinux.org> commit 687f04ab4699440e3d8b371454b26039b6eb2772 Author: Allan McRae <allan(a)archlinux.org> Date: Sat Mar 14 23:43:05 2009 +1000 Only check checksums if there is a source file Signed-off-by: Allan McRae <allan(a)archlinux.org> Signed-off-by: Dan McGee <dan(a)archlinux.org> commit a864a50bc6a65a5c39a4727a07a3b269049a808e Author: Allan McRae <allan(a)archlinux.org> Date: Tue Mar 3 04:13:08 2009 +1000 contrib/pacscripts - print install scripts from a package Prints the install script from a given package file or from a package in the pacman repo. Original-work-by: Giulio "giulivo" Fidente <giulivo.navigante(a)gmail.com> Improvements-by: Xavier Chantry <shiningxc(a)gmail.com> Signed-off-by: Allan McRae <allan(a)archlinux.org> Signed-off-by: Dan McGee <dan(a)archlinux.org> ----------------------------------------------------------------------- Summary of changes: contrib/Makefile.am | 1 + contrib/README | 3 + contrib/pacscripts | 132 ++++++++++++++++++++++++++++++++++++++++++++++++ lib/libalpm/be_files.c | 6 +- lib/libalpm/trans.c | 10 +--- scripts/makepkg.sh.in | 2 + 6 files changed, 144 insertions(+), 10 deletions(-) create mode 100755 contrib/pacscripts hooks/post-receive -- The official pacman repository

1 0

[pacman-dev] [PATCH 1/2] libalpm: sync changes to disk when appropriate
by Dan McGee 15 Mar '09

15 Mar '09

We don't make calls to fsync() or fdatasync() when we are attempting to do something transactionally, such as writing out one of our DB entries. Add a call to fdatasync() when we write DB entries, and also ensure we sync our log changes to disk whenever we close it. Another important thing to ensure is that we commit our database additions and removals as well by syncing the DB directory itself. Signed-off-by: Dan McGee <dan(a)archlinux.org> --- Just a slight refactor- moving the sync code to it's own function so we can call it on both an add and remove operation. -Dan lib/libalpm/be_files.c | 30 +++++++++++++++++++++++------- lib/libalpm/handle.c | 6 +++++- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/lib/libalpm/be_files.c b/lib/libalpm/be_files.c index 0765b8d..3da58a3 100644 --- a/lib/libalpm/be_files.c +++ b/lib/libalpm/be_files.c @@ -27,11 +27,13 @@ #include <string.h> #include <stdint.h> /* uintmax_t, intmax_t */ #include <sys/stat.h> +#include <sys/types.h> #include <dirent.h> #include <ctype.h> #include <time.h> #include <limits.h> /* PATH_MAX */ #include <locale.h> /* setlocale */ +#include <fcntl.h> /* open, close */ /* libalpm */ #include "db.h" @@ -343,6 +345,21 @@ static char *get_pkgpath(pmdb_t *db, pmpkg_t *info) return(pkgpath); } +static int sync_db(pmdb_t *db) +{ + int fd; + /* by syncing the parent directory, we can be sure any database + * changes (adding or removing a package) are committed to disk. */ + fd = open(db->path, 0); + if(fd != -1) { + fsync(fd); + close(fd); + return 0; + } else { + return -1; + } +} + int _alpm_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) { FILE *fp = NULL; @@ -776,8 +793,6 @@ int _alpm_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) "%s\n\n", info->md5sum); } } - fclose(fp); - fp = NULL; } /* FILES */ @@ -804,8 +819,6 @@ int _alpm_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) } fprintf(fp, "\n"); } - fclose(fp); - fp = NULL; } /* DEPENDS */ @@ -848,8 +861,6 @@ int _alpm_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) } fprintf(fp, "\n"); } - fclose(fp); - fp = NULL; } /* INSTALL */ @@ -860,7 +871,11 @@ cleanup: free(pkgpath); if(fp) { + fflush(fp); + fdatasync(fileno(fp)); fclose(fp); + fp = NULL; + sync_db(db); } return(retval); @@ -868,7 +883,7 @@ cleanup: int _alpm_db_remove(pmdb_t *db, pmpkg_t *info) { - int ret = 0; + int ret; char *pkgpath = NULL; ALPM_LOG_FUNC; @@ -881,6 +896,7 @@ int _alpm_db_remove(pmdb_t *db, pmpkg_t *info) ret = _alpm_rmrf(pkgpath); free(pkgpath); + sync_db(db); if(ret != 0) { ret = -1; } diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index 813f439..488459e 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -63,8 +63,10 @@ void _alpm_handle_free(pmhandle_t *handle) /* close logfile */ if(handle->logstream) { + fflush(handle->logstream); + fdatasync(fileno(handle->logstream)); fclose(handle->logstream); - handle->logstream= NULL; + handle->logstream = NULL; } if(handle->usesyslog) { handle->usesyslog = 0; @@ -419,6 +421,8 @@ int SYMEXPORT alpm_option_set_logfile(const char *logfile) FREE(oldlogfile); } if(handle->logstream) { + fflush(handle->logstream); + fdatasync(fileno(handle->logstream)); fclose(handle->logstream); handle->logstream = NULL; } -- 1.6.2

1 1

[pacman-dev] [PATCH] Revert "popen does NOT require /bin/sh in a subchroot"
by Xavier Chantry 15 Mar '09

15 Mar '09

This reverts commit 9558639d8009483fbf422b138d020745986f82f1. This change was wrong, popen does require /bin/sh in a subchroot. 1) pacman -S lilo -r root Notice no error 2) rm root/bin/sh ; pacman -S lilo -r root Notice an error : error: scriptlet failed to execute correctly Actually, we already get an explicit error here, when popen is run, so there is no need to check for bin/sh explicitely. Besides this check was problematic in some cases. For example, bash itself has a scriptlet, but only post_install and post_upgrade, no pre_install and pre_upgrade. However, since bash has a scriptlet, runscriptlet will also be called before bash is installed. It won't do anything since the scriptlet has no pre_install function. But if we keep the check, we will still get "error : no /bin/sh". Conflicts: lib/libalpm/trans.c Signed-off-by: Xavier Chantry <shiningxc(a)gmail.com> --- lib/libalpm/trans.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-) diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index 4b83119..32c6d90 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -443,13 +443,6 @@ int _alpm_runscriptlet(const char *root, const char *installfn, return(0); } - /* NOTE: popen will use the PARENT's /bin/sh, not the chroot's */ - if(access("/bin/sh", X_OK)) { - /* not found */ - _alpm_log(PM_LOG_ERROR, _("No /bin/sh in parent environment, aborting scriptlet\n")); - return(0); - } - /* creates a directory in $root/tmp/ for copying/extracting the scriptlet */ snprintf(tmpdir, PATH_MAX, "%stmp/", root); if(access(tmpdir, F_OK) != 0) { -- 1.6.2

3 2

[pacman-dev] [PATCH] Only check checksums if there is a source file
by Allan McRae 14 Mar '09

14 Mar '09

Signed-off-by: Allan McRae <allan(a)archlinux.org> --- scripts/makepkg.sh.in | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 2ff98f1..3ff256f 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -547,6 +547,8 @@ generate_checksums() { } check_checksums() { + [ ${#source[@]} -eq 0 ] && return 0 + if [ ! $(type -p openssl) ]; then error "$(gettext "Cannot find openssl.")" exit 1 # $E_MISSING_PROGRAM -- 1.6.2

1 0

[pacman-dev] delay writes a fsync
by Allan McRae 14 Mar '09

14 Mar '09

Hi, You have probably heard about this: http://linux.slashdot.org/article.pl?sid=09/03/11/2031231 Here is the link to the bug report that describes the cause of the problem: https://bugs.edge.launchpad.net/ubuntu/+source/linux/+bug/317781/comments/54 A key part of the bug report comment... Applications are expected to use fsync() or fdatasync(), and if that impacts their performance too much, to use a single berkdb or other binary database file, and not do something stupid with hundreds of tiny text files that only hold a few bytes of data in each text file. Hmmm... something sound familiar there? So what should we do? Maybe, add some sort of syncing after a db write. But then after each file or just at the end of the transaction? Allan

8 19