May 2008 - Pacman-dev - lists.archlinux.org

[pacman-dev] memleak around -Qu
by Nagy Gabor 26 May '08

26 May '08

Hi! This is not a big issue, but memleaks are just ugly. I mentioned this in the 2. point of my http://bugs.archlinux.org/task/7884#comment25485 comment. See pacman/query.c/query_upgrades(): The result of alpm_sync_sysupgrade just printed but not freed (as I see, the front-end couldn't do it anyway) Bye ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/

2 1

[pacman-dev] [PATCH] Stop reinstalling message when using print-uris
by Allan McRae 26 May '08

26 May '08

The stops the "pkg is up to date -- reinstalling" message when only asking for uris to be printed. Signed-off-by: Allan McRae <mcrae_allan(a)hotmail.com> --- lib/libalpm/sync.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 0d6a6ee..02d64ea 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -339,7 +339,7 @@ int _alpm_sync_addtarget(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sy alpm_pkg_get_name(local), alpm_pkg_get_version(local)); return(0); } else { - if(!(trans->flags & PM_TRANS_FLAG_DOWNLOADONLY)) { + if(!(trans->flags & (PM_TRANS_FLAG_DOWNLOADONLY | PM_TRANS_FLAG_PRINTURIS))) { _alpm_log(PM_LOG_WARNING, _("%s-%s is up to date -- reinstalling\n"), alpm_pkg_get_name(local), alpm_pkg_get_version(local)); } -- 1.5.5.1

2 1

[pacman-dev] PM_TRANS_EVT_EXTRACT_START missing?
by Sebastian Nowicki 24 May '08

24 May '08

It seems every action has a START and DONE event, apart from package extraction. I was wondering if the extract action was supposed to have a PM_TRANS_EVT_EXTRACT_START event as well? -- Sebastian Nowicki

2 2

[pacman-dev] [PATCH] be_files : stores REPLACES and FORCE in desc.
by Xavier Chantry 23 May '08

23 May '08

repo-add and db_read both assume that REPLACES and FORCE fields are in the desc file, so do that for db_write as well (instead of depends file). Note that db_write is currently only used on the local database. And the only purpose of replaces and force in local database is for information purpose (available on -Qi operations). So this is not a big problem. Ref: http://www.archlinux.org/pipermail/pacman-dev/2008-May/011859.html Signed-off-by: Xavier Chantry <shiningxc(a)gmail.com> --- lib/libalpm/be_files.c | 21 ++++++++++----------- 1 files changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/libalpm/be_files.c b/lib/libalpm/be_files.c index 7b3d824..2302374 100644 --- a/lib/libalpm/be_files.c +++ b/lib/libalpm/be_files.c @@ -672,6 +672,16 @@ int _alpm_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) } fprintf(fp, "\n"); } + if(info->replaces) { + fputs("%REPLACES%\n", fp); + for(lp = info->replaces; lp; lp = lp->next) { + fprintf(fp, "%s\n", (char *)lp->data); + } + fprintf(fp, "\n"); + } + if(info->force) { + fprintf(fp, "%%FORCE%%\n\n"); + } if(local) { if(info->url) { fprintf(fp, "%%URL%%\n" @@ -795,17 +805,6 @@ int _alpm_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) } fprintf(fp, "\n"); } - if(info->replaces) { - fputs("%REPLACES%\n", fp); - for(lp = info->replaces; lp; lp = lp->next) { - fprintf(fp, "%s\n", (char *)lp->data); - } - fprintf(fp, "\n"); - } - if(info->force) { - /* note the extra newline character, which is necessary! */ - fprintf(fp, "%%FORCE%%\n\n"); - } fclose(fp); fp = NULL; } -- 1.5.5.1

2 1

[pacman-dev] REPLACES and FORCE in depends or desc files
by Xavier 22 May '08

22 May '08

While inspecting the freshly rebuilt databases, I noticed the following problem : * repo-add adds these two entries in desc file * pacman reads them from desc file (for both local or sync db) * pacman writes them to depends file (only possible with local db) after the following commit : http://projects.archlinux.org/?p=pacman.git;a=commitdiff;h=26f4993e1d67a7da… By the way, the above commit actually enabled previously unused code, as Nagy noticed : http://www.archlinux.org/pipermail/pacman-dev/2008-January/010662.html This code was only run when local == 0. But when we use db_write, it is always on local db, so we always have local == 1. The fact that this code was previously unused probably explains why this problem hasn't been detected until now. But actually I just found the real reason by going deeper in the history. The replaces and force fields were originally moved from desc to depends during pacman 3 development, but then it was reverted before the release : http://projects.archlinux.org/?p=pacman.git;a=commitdiff;h=13e21110459aaf99… But only db_read was reverted, not db_write. This was impossible to detect back then because this code was unused. This code was later enabled, leading to the inconsistency. I guess the quickest / easiest fix is to fix db_write to also put force / replaces in the desc file, and then have a new 3.1.x release. The local database will be slightly messed up but it will go away eventually, as upgrade goes. Note that this is totally harmless, because the force / replaces in local db are totally unused. They were just added for consistency / completeness, and to have all informations on -Qi operations. So harmless but still worth fixing imo.

3 3

[pacman-dev] Kernel Provision
by Ray Rashif 21 May '08

21 May '08

Would it be alright for kernel packages to provide for others that depend on in-kernel features/functionalities, say, those from patches? Right now kernel26 has the linux-phc patch, and the following is a GUI tool to do what can be done, well, graphically: http://aur.archlinux.org/packages.php?ID=17059 As you can see, it could either skip the kernel26 dependency and tell the user about the patch in install script - or - include the dependency like the above case and expect the user to adjust PKGBUILD for other kernels with the same patch.

3 3

[pacman-dev] [PATCH] Pactests for sync operations with NoUpgrade and NoExtract
by Allan McRae 20 May '08

20 May '08

This adds the sync equivalents of upgrade010.py and upgrade070.py. Both additional test pass. Signed-off-by: Allan McRae <mcrae_allan(a)hotmail.com> --- pactest/tests/sync500.py | 15 +++++++++++++++ pactest/tests/sync501.py | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+), 0 deletions(-) create mode 100644 pactest/tests/sync500.py create mode 100644 pactest/tests/sync501.py diff --git a/pactest/tests/sync500.py b/pactest/tests/sync500.py new file mode 100644 index 0000000..ccfffa7 --- /dev/null +++ b/pactest/tests/sync500.py @@ -0,0 +1,15 @@ +self.description = "Install a package from a sync db with NoExtract" + +sp = pmpkg("dummy") +sp.files = ["bin/dummy", + "usr/man/man1/dummy.1"] +self.addpkg2db("sync", sp) + +self.option["NoExtract"] = ["usr/man/man1/dummy.1"] + +self.args = "-S %s" % sp.name + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PKG_EXIST=dummy") +self.addrule("FILE_EXIST=bin/dummy") +self.addrule("!FILE_EXIST=usr/man/man1/dummy.1") diff --git a/pactest/tests/sync501.py b/pactest/tests/sync501.py new file mode 100644 index 0000000..eb826e2 --- /dev/null +++ b/pactest/tests/sync501.py @@ -0,0 +1,18 @@ +self.description = "Upgrade a package, with a file in NoUpgrade" + +sp = pmpkg("dummy", "1.0-2") +sp.files = ["etc/dummy.conf"] +self.addpkg2db("sync", sp) + +lp = pmpkg("dummy") +lp.files = ["etc/dummy.conf"] +self.addpkg2db("local", lp) + +self.option["NoUpgrade"] = ["etc/dummy.conf"] + +self.args = "-S %s" % sp.name + +self.addrule("PKG_VERSION=dummy|1.0-2") +self.addrule("!FILE_MODIFIED=etc/dummy.conf") +self.addrule("FILE_PACNEW=etc/dummy.conf") +self.addrule("!FILE_PACSAVE=etc/dummy.conf") -- 1.5.5.1

3 2

[pacman-dev] [PATCH] makepkg - add check for valid options in PKGBUILD
by Allan McRae 20 May '08

20 May '08

This patch removes the code block in makepkg that checked for depreciated options in a PKGBUILD provided a workaround. Unknown and depreciated options are upgraded to error conditions. Also, removed TODO regarding including install script if exists and $install is unset. That should never happen. Signed-off-by: Allan McRae <mcrae_allan(a)hotmail.com> --- scripts/makepkg.sh.in | 38 ++++++++++++++++++-------------------- 1 files changed, 18 insertions(+), 20 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 53d7f98..690ab9c 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -182,25 +182,6 @@ check_option() { return fi - # BEGIN DEPRECATED - # TODO: This code should be removed in the next release of makepkg. - local needle=$(echo $1 | tr [:upper:] [:lower:]) - local opt - for opt in ${options[@]}; do - opt=$(echo $opt | tr [:upper:] [:lower:]) - if [ "$opt" = "no$needle" ]; then - warning "$(gettext "Options beginning with 'no' will be deprecated in the next version of makepkg!")" - plain "$(gettext "Please replace 'no' with '!': %s -> %s.")" "no$needle" "!$needle" - echo 'n' # Disabled - return - elif [ "$opt" = "keepdocs" -a "$needle" = "docs" ]; then - warning "$(gettext "Option 'keepdocs' may not work as intended. Please replace with 'docs'.")" - echo 'y' # Enabled - return - fi - done - # END DEPRECATED - # fall back to makepkg.conf options ret=$(in_opt_array "$1" ${OPTIONS[@]}) if [ "$ret" != '?' ]; then @@ -870,7 +851,6 @@ create_package() { local comp_files=".PKGINFO" # check for an install script - # TODO: should we include ${pkgname}.install if it exists and $install is unset? if [ "$install" != "" ]; then msg2 "$(gettext "Adding install script...")" cp "$startdir/$install" .INSTALL @@ -1369,6 +1349,24 @@ if [ "$install" -a ! -f "$install" ]; then exit 1 fi +known_options=('strip' 'docs' 'libtool' 'emptydirs' 'ccache' 'distcc' 'makeflags' 'force') +valid_options=0 +for opt in ${options[@]}; do + known=1 + for kopt in ${known_options[@]}; do + if [ "${opt}" = "${kopt}" -o "${opt}" = "!${kopt}" ]; then + known=0 + fi + done + if [ $known -eq 1 ]; then + error "$(gettext "Unknown option '%s'")" "$opt" + valid_options=1 + fi +done +if [ $valid_options -eq 1 ]; then + exit 1 +fi + # We need to run devel_update regardless of whether we are in the fakeroot # build process so that if the user runs makepkg --forcever manually, we # 1) output the correct pkgver, and 2) use the correct filename when -- 1.5.5.1

3 7

[pacman-dev] [PATCH] New _alpm_find_first_satisfier function.
by Xavier Chantry 20 May '08

20 May '08

Address two concerns brought up by Nagy about the _alpm_find_dep_satisfier function by adding the following new function : pmpkg_t *_alpm_find_first_satisfier(alpm_list_t *pkgs, pmdepend_t *dep) This function just returns the first satisfier, so this takes care of the performance problem by restoring the old behavior. The memleak was also fixed. Ref: http://www.archlinux.org/pipermail/pacman-dev/2008-May/011779.html Signed-off-by: Xavier Chantry <shiningxc(a)gmail.com> --- lib/libalpm/deps.c | 37 ++++++++++++++++++++++++++++++------- lib/libalpm/deps.h | 1 + 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index 37e9916..383f02d 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -194,7 +194,10 @@ alpm_list_t *_alpm_sortbydeps(alpm_list_t *targets, int reverse) return(newtargs); } -alpm_list_t *_alpm_find_dep_satisfiers(alpm_list_t *pkgs, pmdepend_t *dep) +/* if full > 0, return the full list of satisfiers + * if full == 0, return the first satisfier + */ +static alpm_list_t *find_dep_satisfiers(alpm_list_t *pkgs, pmdepend_t *dep, int full) { alpm_list_t *i, *ret = NULL; @@ -202,11 +205,31 @@ alpm_list_t *_alpm_find_dep_satisfiers(alpm_list_t *pkgs, pmdepend_t *dep) pmpkg_t *pkg = i->data; if(alpm_depcmp(pkg, dep)) { ret = alpm_list_add(ret, pkg); + if(!full) { + break; + } } } return(ret); } +alpm_list_t *_alpm_find_dep_satisfiers(alpm_list_t *pkgs, pmdepend_t *dep) +{ + return(find_dep_satisfiers(pkgs, dep, 1)); +} + +pmpkg_t *_alpm_find_first_satisfier(alpm_list_t *pkgs, pmdepend_t *dep) +{ + pmpkg_t *pkg = NULL; + alpm_list_t *list = find_dep_satisfiers(pkgs, dep, 0); + if(list) { + pkg = list->data; + alpm_list_free(list); + } + return(pkg); +} + + /** Find packages in a list that provide a given package. * @param pkgs an alpm_list_t* of package to search * @param pkgname the name of the package @@ -237,7 +260,7 @@ alpm_list_t SYMEXPORT *alpm_deptest(pmdb_t *db, alpm_list_t *targets) target = alpm_list_getdata(i); dep = _alpm_splitdep(target); - if(!_alpm_find_dep_satisfiers(_alpm_db_get_pkgcache(db), dep)) { + if(!_alpm_find_first_satisfier(_alpm_db_get_pkgcache(db), dep)) { ret = alpm_list_add(ret, target); } _alpm_dep_free(dep); @@ -288,8 +311,8 @@ alpm_list_t SYMEXPORT *alpm_checkdeps(pmdb_t *db, int reversedeps, pmdepend_t *depend = j->data; /* 1. we check the upgrade list */ /* 2. we check database for untouched satisfying packages */ - if(!_alpm_find_dep_satisfiers(upgrade, depend) && - !_alpm_find_dep_satisfiers(dblist, depend)) { + if(!_alpm_find_first_satisfier(upgrade, depend) && + !_alpm_find_first_satisfier(dblist, depend)) { /* Unsatisfied dependency in the upgrade list */ char *missdepstring = alpm_dep_get_string(depend); _alpm_log(PM_LOG_DEBUG, "checkdeps: missing dependency '%s' for package '%s'\n", @@ -308,13 +331,13 @@ alpm_list_t SYMEXPORT *alpm_checkdeps(pmdb_t *db, int reversedeps, pmpkg_t *lp = i->data; for(j = alpm_pkg_get_depends(lp); j; j = j->next) { pmdepend_t *depend = j->data; - pmpkg_t *causingpkg = alpm_list_getdata(_alpm_find_dep_satisfiers(modified, depend)); + pmpkg_t *causingpkg = _alpm_find_first_satisfier(modified, depend); /* we won't break this depend, if it is already broken, we ignore it */ /* 1. check upgrade list for satisfiers */ /* 2. check dblist for satisfiers */ if(causingpkg && - !_alpm_find_dep_satisfiers(upgrade, depend) && - !_alpm_find_dep_satisfiers(dblist, depend)) { + !_alpm_find_first_satisfier(upgrade, depend) && + !_alpm_find_first_satisfier(dblist, depend)) { char *missdepstring = alpm_dep_get_string(depend); _alpm_log(PM_LOG_DEBUG, "checkdeps: transaction would break '%s' dependency of '%s'\n", missdepstring, alpm_pkg_get_name(lp)); diff --git a/lib/libalpm/deps.h b/lib/libalpm/deps.h index 70badfd..5c7da87 100644 --- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -52,6 +52,7 @@ int _alpm_resolvedeps(pmdb_t *local, alpm_list_t *dbs_sync, pmpkg_t *syncpkg, **data); pmdepend_t *_alpm_splitdep(const char *depstring); alpm_list_t *_alpm_find_dep_satisfiers(alpm_list_t *pkgs, pmdepend_t *dep); +pmpkg_t *_alpm_find_first_satisfier(alpm_list_t *pkgs, pmdepend_t *dep); #endif /* _ALPM_DEPS_H */ -- 1.5.5.1

2 1

Re: [pacman-dev] [PATCHES] Refreshed PKGBUILDs
by Geoffroy Carrier 20 May '08

20 May '08

Excerpts from Allan McRae's message of Sat May 17 16:40:41 +0200 2008: > It is much better to put patches inline so we can comment on them directly. Noticed. I'll have to read my mailer's doc deeper. > I must be missing something here... where exactly are you changing > $srcdir or $pkgdir not to point at $startdir/src and $startdir/pkg? You > can have these directories as tmpfs if you really want but how does that > need this change? I also like the use of $startdir/* because it is > quite obvious what the startdir is. I don't. The whole list I wrote in this mail are rules I think should be followed. Not using $srcdir anymore would permit such changes in the future. > Quoting paths with variable names seems a good idea. Thank you. Except for variables like cvs/svn/etc. paths which are not likely to include any spaces or horrible chars. URLs in general are. > > - Still, lightweight means smater > > => Don't use > > "$_svntrunk" ${something} > > instead of > > $_svntrunk $something > > > Where exactly is this change? This was an example of the rule I wanted to follow... ie. i did not introduced quotes of $_svntrunk which obviously won't include spaces. (or it's REAAALLY dumb). > It would be a poorly formed makefile if it did this, but I have struck > this once problem before. It is also readily noticeable when building > the package so I'm not sure if this is really needed. So did I. I think new packagers which are discovering makepkg are more likely to think it's a makepkg problem instead of a wrong Makefile, and might not even think about adding the trailing slash. Anyway, It's not something important IMHO. IMHO getting rid of $startdir is. -- Geoffroy Carrier http://gcarrier.koon.fr/

9 25