Re: [pacman-dev] Versioned packages

2016-09-12 Thread Sergey Petrenko via pacman-dev
Well, of course! I can also build kernel outside package management, or write a 
hook to backup kernels, but I'd like to see solution that would not require 
such dire and time consuming measures, and, ideally, would not require actions 
from me at all.


>Понедельник, 12 сентября 2016, 10:22 +03:00 от Jelle van der Waa 
><je...@vdwaa.nl>:
>
>On 09/12/16 at 09:47am, Sergey Petrenko via pacman-dev wrote:
>> Should I spam kernel package maintainers then, or maybe someone will resolve 
>> bug as wontfix?
>
>Not sure why they need to be spammed, you can easily build linux47 as a
>package and install it separate from the normal linux package. But I
>guess you want to automatically retain your current installed linux pkg
>when you upgrade to a newer version?
>
>> 
>> 
>> >Суббота, 10 сентября 2016, 0:58 +03:00 от Allan McRae < al...@archlinux.org 
>> >>:
>> >
>> >On 10/09/16 08:41, Sergey Petrenko via pacman-dev wrote:
>> >> Here is my attempt to solve seven years old infamous problem:
>> >>  https://bugs.archlinux.org/task/16702
>> >> 
>> >> Patch won't solve problem out of the box, a small changes in kernel 
>> >> PKGBUILD
>> >> will be required, but only concerning install part.
>> >> 
>> >> Idea behind patch is pretty simple:
>> >> 1) Configure list of packages and number of old versions pacman should 
>> >> try 
>> >> to preserve.
>> >> 2) When upgading to new version, keep old in place, if it has no file 
>> >> conflicts with new one, and mark it as `archived`, remove oldest 
>> >> `archived` 
>> >> version instead.
>> >> 
>> >> Most of time pacman treats `archived` packages as if they aren't 
>> >> installed.
>> >> For now it won't check package conflicts and dependencies, only file 
>> >> conflicts 
>> >> with newer versions. It's only an outline of full solution, proof of 
>> >> concept 
>> >> to illustrate the idea.
>> >> 
>> >> I'd like to hear opinion of community whether this problem should be 
>> >> solved 
>> >> at all, or is it more like a feature of ArchLinux, and if it should, 
>> >> whether
>> >> such approach suits ArchLinux's philosophy.
>> >> 
>> >
>> >How is this better than having a package file sitting in the cache?
>> >
>> >The "kernel problem" in Arch is not because it is not possible to have
>> >multiple kernel packages available.  Other distributions provide endless
>> >amounts of kernels (e.g. Manjaro).
>> >
>> >I don't see anything that needs done on the package manager end for this.
>> >
>> >Allan
>> 
>> 
>> -- 
>> With wish of constant improvement
>> and unstoppable creativity.
>> Sergey Petrenko
>
>-- 
>Jelle van der Waa


-- 
With wish of constant improvement
and unstoppable creativity.
Sergey Petrenko


Re: [pacman-dev] Versioned packages

2016-09-12 Thread Sergey Petrenko via pacman-dev
Should I spam kernel package maintainers then, or maybe someone will resolve 
bug as wontfix?


>Суббота, 10 сентября 2016, 0:58 +03:00 от Allan McRae <al...@archlinux.org>:
>
>On 10/09/16 08:41, Sergey Petrenko via pacman-dev wrote:
>> Here is my attempt to solve seven years old infamous problem:
>>  https://bugs.archlinux.org/task/16702
>> 
>> Patch won't solve problem out of the box, a small changes in kernel PKGBUILD
>> will be required, but only concerning install part.
>> 
>> Idea behind patch is pretty simple:
>> 1) Configure list of packages and number of old versions pacman should try 
>> to preserve.
>> 2) When upgading to new version, keep old in place, if it has no file 
>> conflicts with new one, and mark it as `archived`, remove oldest `archived` 
>> version instead.
>> 
>> Most of time pacman treats `archived` packages as if they aren't installed.
>> For now it won't check package conflicts and dependencies, only file 
>> conflicts 
>> with newer versions. It's only an outline of full solution, proof of concept 
>> to illustrate the idea.
>> 
>> I'd like to hear opinion of community whether this problem should be solved 
>> at all, or is it more like a feature of ArchLinux, and if it should, whether
>> such approach suits ArchLinux's philosophy.
>> 
>
>How is this better than having a package file sitting in the cache?
>
>The "kernel problem" in Arch is not because it is not possible to have
>multiple kernel packages available.  Other distributions provide endless
>amounts of kernels (e.g. Manjaro).
>
>I don't see anything that needs done on the package manager end for this.
>
>Allan


-- 
With wish of constant improvement
and unstoppable creativity.
Sergey Petrenko


[pacman-dev] [PATCH 2/2] Ability to keep several old versions of package when they have no file conflicts with new version.

2016-09-09 Thread Sergey Petrenko via pacman-dev
I had gave a big thought to the question, who should decide whether to try to
archive package. 

At first glance it looks like maintainer's responsibility. She/he/it creates a 
package without file conflicts intending possibility for multiple versions to 
be installed on a system. Yet this means boolean flag on packages in sync 
repository. Such flag will look really really ugly (at least to me).

Other option is no option at all. Pacman always checks file conflicts between
old and new versions and can try to archive any packages that has none. I guess
such approach wouldn't change a bit neither in speed of pacman operations, nor 
in
file system of users with current state of sync databases. It looks like every
package has at least one common file between versions. Yet at some point such
packages may appear and why would user want to keep multiple versions of package
neither she/he/it, nor maintainer intended to be `archived`?

And thus solution I have ended with - new config option. `MaxArchived` specifies
window of versions to `archive`, and `ArchivePkg` - patterns of packages' names.

In this patch `archived` packages are handled only in update and remove 
operations.
Of course additional changes are required to display `archived` packages in 
queries,
possibly package-level conflicts checking for `archived` packages is also 
required.

---
 lib/libalpm/add.c  | 177 +++--
 lib/libalpm/alpm.h |  28 +++-
 lib/libalpm/be_local.c |  62 ++---
 lib/libalpm/conflict.c |  25 ++-
 lib/libalpm/db.c   |  22 ++
 lib/libalpm/db.h   |   3 +
 lib/libalpm/filelist.c |  54 +++
 lib/libalpm/filelist.h |   5 ++
 lib/libalpm/handle.c   |  38 +++
 lib/libalpm/handle.h   |   2 +
 lib/libalpm/package.c  |  13 
 lib/libalpm/package.h  |   2 +
 lib/libalpm/pkghash.c  |  48 ++
 lib/libalpm/pkghash.h  |   8 +++
 lib/libalpm/remove.c   |  16 +
 src/pacman/callback.c  |  21 ++
 src/pacman/conf.c  |  20 ++
 src/pacman/conf.h  |   2 +
 18 files changed, 529 insertions(+), 17 deletions(-)

diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index d132e52..eb035d7 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -46,6 +46,7 @@
 #include "db.h"
 #include "remove.h"
 #include "handle.h"
+#include "filelist.h"
 
 /** Add a package to the transaction. */
 int SYMEXPORT alpm_add_pkg(alpm_handle_t *handle, alpm_pkg_t *pkg)
@@ -393,6 +394,173 @@ static int extract_single_file(alpm_handle_t *handle, 
struct archive *archive,
return errors;
 }
 
+static int should_archive_oldpkg(alpm_pkg_t *oldpkg, alpm_package_operation_t 
operation)
+{
+   return oldpkg->reason == ALPM_PKG_REASON_EXPLICIT &&
+   oldpkg->handle->max_archived > 0 &&
+   operation != ALPM_PACKAGE_REINSTALL &&
+   alpm_list_find(oldpkg->handle->archivepkg, 
oldpkg->name, _alpm_fnmatch) != NULL;
+}
+
+static int should_remove_archivable_oldpkg(alpm_pkg_t *oldpkg, alpm_pkg_t 
*newpkg,
+   alpm_package_operation_t operation, int *files_overlap)
+{
+   if (operation == ALPM_PACKAGE_DOWNGRADE) {
+   return 1;
+   }
+   *files_overlap = _alpm_filelist_overlap(>files, >files);
+   return *files_overlap;
+}
+
+static alpm_errno_t ask_to_remove_archived_pkgs(alpm_handle_t *handle,
+   alpm_pkg_t *newpkg, alpm_list_t *archived)
+{
+   alpm_errno_t ret = (alpm_errno_t)0;
+
+   if (archived) {
+   alpm_question_remove_from_archive_t question = {
+   .type = ALPM_QUESTION_REMOVE_FROM_ARCHIVE,
+   .remove = 0,
+   .pkgs = archived
+   };
+
+   QUESTION(handle, );
+   if(question.remove) {
+   alpm_list_t *itr;
+   for (itr = archived; itr; itr = 
alpm_list_next(archived)) {
+   alpm_pkg_t *archived_pkg = itr->data;
+
+   _alpm_log(handle, ALPM_LOG_DEBUG, _("removing 
archived %s-%s\n"),
+   archived_pkg->name, 
archived_pkg->version);
+
+   if(_alpm_remove_single_package(handle, 
archived_pkg, newpkg, 0, 0) == -1) {
+   ret = ALPM_ERR_TRANS_ABORT;
+   goto cleanup;
+   }
+   }
+   } else {
+   ret = ALPM_ERR_TRANS_ABORT;
+   goto cleanup;
+   }
+   }
+
+cleanup:
+   alpm_list_free(archived);
+   return ret;
+}
+
+static alpm_list_t *get_old_and_conficting_archived_pkgs(alpm_handle_t *handle,
+   alpm_pkg_t *newpkg, int oldpkg_files_overlap)
+{
+   alpm_list_t *archived = _alpm_db_get_archived_pkgs(handle->db_local, 
newpkg->name);
+   

[pacman-dev] [PATCH 1/2] Tests for expected behavior of archiving mechanism. Edits to testing framework required for handling multiple versions of package.

2016-09-09 Thread Sergey Petrenko via pacman-dev
Unfortunately, some changes are required to testing framework to handle
multiple versions of package.
New tests should make expected behavior of `archived` packages pretty clear.

---
 test/pacman/pmdb.py  | 51 +++-
 test/pacman/pmpkg.py |  2 ++
 test/pacman/pmrule.py| 17 ++
 test/pacman/tests/TESTS  |  5 
 test/pacman/tests/archived001.py | 22 +
 test/pacman/tests/archived002.py | 31 
 test/pacman/tests/archived003.py | 37 +
 test/pacman/tests/archived004.py | 31 
 test/pacman/tests/archived005.py | 29 +++
 test/pacman/tests/archived006.py | 39 ++
 10 files changed, 253 insertions(+), 11 deletions(-)
 create mode 100644 test/pacman/tests/archived001.py
 create mode 100644 test/pacman/tests/archived002.py
 create mode 100644 test/pacman/tests/archived003.py
 create mode 100644 test/pacman/tests/archived004.py
 create mode 100644 test/pacman/tests/archived005.py
 create mode 100644 test/pacman/tests/archived006.py

diff --git a/test/pacman/pmdb.py b/test/pacman/pmdb.py
index 1f20506..db2f270 100644
--- a/test/pacman/pmdb.py
+++ b/test/pacman/pmdb.py
@@ -60,6 +60,7 @@ def __init__(self, treename, root):
 self.is_local = True
 self.read_dircache = None
 self.read_pkgcache = {}
+self.archived = {}
 else:
 self.dbdir = None
 self.dbfile = os.path.join(root, util.PM_SYNCDBPATH, treename + 
".db")
@@ -79,35 +80,53 @@ def getpkg(self, name):
 if name == pkg.name:
 return pkg
 
-def db_read(self, name):
+def db_read(self, name, archived=False):
 if not self.dbdir or not os.path.isdir(self.dbdir):
 return None
 
-dbentry = None
+if not archived and name in self.read_pkgcache:
+return self.read_pkgcache[name]
+if archived and name in self.archived:
+return self.archived[name]
+
 if self.read_dircache is None:
 self.read_dircache = os.listdir(self.dbdir)
+
+candidates = []
 for entry in self.read_dircache:
 if entry == "ALPM_DB_VERSION":
 continue
 [pkgname, pkgver, pkgrel] = entry.rsplit("-", 2)
 if pkgname == name:
-dbentry = entry
-break
-if dbentry is None:
+candidates.append(entry)
+if len(candidates) == 0:
 return None
 
-if pkgname in self.read_pkgcache:
-return self.read_pkgcache[pkgname]
+for candidate in candidates:
+self.pkg_load(candidate)
+
+if name not in self.archived:
+self.archived[name] = []
+
+if not archived:
+if name in self.read_pkgcache:
+return self.read_pkgcache[name]
+else:
+return self.archived[name]
+
+return None
 
+def pkg_load(self, dbentry):
+
+[pkgname, pkgver, pkgrel] = dbentry.rsplit("-", 2)
 pkg = pmpkg.pmpkg(pkgname, pkgver + "-" + pkgrel)
-self.read_pkgcache[pkgname] = pkg
 
 path = os.path.join(self.dbdir, dbentry)
 # desc
 filename = os.path.join(path, "desc")
 if not os.path.isfile(filename):
 tap.bail("invalid db entry found (desc missing) for pkg " + 
pkgname)
-return None
+return
 fd = open(filename, "r")
 while 1:
 line = fd.readline()
@@ -136,6 +155,12 @@ def db_read(self, name):
 except ValueError:
 pkg.reason = -1
 raise
+elif line == "%ARCHIVED%":
+try:
+pkg.archived = int(fd.readline().strip("\n"))
+except ValueError:
+pkg.archived = -1
+raise
 elif line == "%SIZE%" or line == "%CSIZE%":
 try:
 pkg.size = int(fd.readline().strip("\n"))
@@ -162,7 +187,7 @@ def db_read(self, name):
 filename = os.path.join(path, "files")
 if not os.path.isfile(filename):
 tap.bail("invalid db entry found (files missing) for pkg " + 
pkgname)
-return None
+return
 fd = open(filename, "r")
 while 1:
 line = fd.readline()
@@ -181,7 +206,10 @@ def db_read(self, name):
 # install
 filename = os.path.join(path, "install")
 
-return pkg
+if pkg.archived:
+self.archived.setdefault(pkgname, []).append(pkg)
+else:
+self.read_pkgcache[pkgname] = pkg
 
 #
 # db_write is used to add both 'local' and 'sync' db entries
@@ -207,6 +235,7 @@ def db_write(self, pkg):
 make_section(data, "INSTALLDATE", pkg.installdate)
   

[pacman-dev] Versioned packages

2016-09-09 Thread Sergey Petrenko via pacman-dev
Here is my attempt to solve seven years old infamous problem:
https://bugs.archlinux.org/task/16702

Patch won't solve problem out of the box, a small changes in kernel PKGBUILD
will be required, but only concerning install part.

Idea behind patch is pretty simple:
1) Configure list of packages and number of old versions pacman should try 
to preserve.
2) When upgading to new version, keep old in place, if it has no file 
conflicts with new one, and mark it as `archived`, remove oldest `archived` 
version instead.

Most of time pacman treats `archived` packages as if they aren't installed.
For now it won't check package conflicts and dependencies, only file conflicts 
with newer versions. It's only an outline of full solution, proof of concept 
to illustrate the idea.

I'd like to hear opinion of community whether this problem should be solved 
at all, or is it more like a feature of ArchLinux, and if it should, whether
such approach suits ArchLinux's philosophy.


Re: [pacman-dev] [PATCH] Option to pass packages' versions to hook. Linearize code in _alpm_hook_trigger_match_pkg.

2016-09-05 Thread Sergey Petrenko via pacman-dev
Sure:
https://github.com/versusvoid/archlinux-kernels-backup/blob/master/usr/bin/restore-kernel#L98

If I can get version of removed kernel, then I don't need to search for new 
backup through 
all the directories, or:

https://github.com/versusvoid/archlinux-kernels-backup/blob/master/usr/bin/backup-kernel#L11

manually parse package version.

>Понедельник,  5 сентября 2016, 10:45 +03:00 от Allan McRae 
><al...@archlinux.org>:
>
>On 05/09/16 03:44, Sergey Petrenko via pacman-dev wrote:
>> I'm writing hooks to solve infamous kernel problem. Having version in hook 
>> isn't necessary,
>> but will save me lot of work (and lot of time at each hook run).
>> 
>
>Can you post an example hook with what your are trying to achieve.
>
>Allan


-- 
With wish of constant improvement
and unstoppable creativity.
Sergey Petrenko


Re: [pacman-dev] [PATCH v2] Option to pass packages' versions to hook. Linearize code in _alpm_hook_trigger_match_pkg.

2016-09-04 Thread Sergey Petrenko via pacman-dev

Version can contain spaces??? O_o

I definitely agree with your reasoning. For now I can imagine two concise 
solutions:
1) Environment variables (mercurial style hooks)
2) Binary hooks (aka plugins)

>Воскресенье,  4 сентября 2016, 18:56 UTC от Andrew Gregory < 
>andrew.gregor...@gmail.com >:
>
>On 09/04/16 at 08:56pm, Sergey Petrenko via pacman-dev wrote:
>> Variables are now at beginning of block.
>> 
>> ---
>
>I would love to provide more information to hooks about the
>transaction, but, without a compelling justification, I'm -1 on this
>patch for two reasons.  First, I don't like the piecemeal approach.
>When the next person decides they need different information, do we
>just keep adding Needs options?  Second, the " "
>format can't be reliably parsed.  Both of those fields can contain
>spaces.
>
>>  lib/libalpm/hook.c| 104 
>> +-
>>  test/pacman/tests/TESTS   |   1 +
>>  test/pacman/tests/hook-target-versions.py |  47 ++
>>  3 files changed, 120 insertions(+), 32 deletions(-)
>>  create mode 100644 test/pacman/tests/hook-target-versions.py
>> 
>> diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c
>> index ccde225..d37924f 100644
>> --- a/lib/libalpm/hook.c
>> +++ b/lib/libalpm/hook.c
>> @@ -55,7 +55,7 @@ struct _alpm_hook_t {
>>  char **cmd;
>>  alpm_list_t *matches;
>>  alpm_hook_when_t when;
>> -int abort_on_fail, needs_targets;
>> +int abort_on_fail, needs_targets, needs_versions;
>>  };
>> 
>>  struct _alpm_hook_cb_ctx {
>> @@ -90,7 +90,7 @@ static void _alpm_hook_free(struct _alpm_hook_t *hook)
>>  _alpm_wordsplit_free(hook->cmd);
>>  alpm_list_free_inner(hook->triggers, (alpm_list_fn_free) 
>> _alpm_trigger_free);
>>  alpm_list_free(hook->triggers);
>> -alpm_list_free(hook->matches);
>> +alpm_list_free_inner(hook->matches, free);
>>  FREELIST(hook->depends);
>>  free(hook);
>>  }
>> @@ -329,8 +329,11 @@ static int _alpm_hook_parse_cb(const char *file, int 
>> line,
>>  hook->abort_on_fail = 1;
>>  } else if(strcmp(key, "NeedsTargets") == 0) {
>>  hook->needs_targets = 1;
>> +} else if(strcmp(key, "NeedsVersions") == 0) {
>> +hook->needs_versions = 1;
>>  } else if(strcmp(key, "Exec") == 0) {
>> -if((hook->cmd = _alpm_wordsplit(value)) == NULL) {
>> +if((hook->
>> +cmd = _alpm_wordsplit(value)) == NULL) {
>>  if(errno == EINVAL) {
>>  error(_("hook %s line %d: invalid value 
>> %s\n"), file, line, value);
>>  } else {
>> @@ -456,31 +459,54 @@ static int _alpm_hook_trigger_match_file(alpm_handle_t 
>> *handle,
>>  static int _alpm_hook_trigger_match_pkg(alpm_handle_t *handle,
>>  struct _alpm_hook_t *hook, struct _alpm_trigger_t *t)
>>  {
>> -alpm_list_t *install = NULL, *upgrade = NULL, *remove = NULL;
>> +alpm_list_t *add = NULL, *remove = NULL;
>> 
>>  if(t->op & ALPM_HOOK_OP_INSTALL || t->op & ALPM_HOOK_OP_UPGRADE) {
>>  alpm_list_t *i;
>>  for(i = handle->trans->add; i; i = i->next) {
>>  alpm_pkg_t *pkg = i->data;
>> -if(_alpm_fnmatch_patterns(t->targets, pkg->name) == 0) {
>> -if(pkg->oldpkg) {
>> -if(t->op & ALPM_HOOK_OP_UPGRADE) {
>> -if(hook->needs_targets) {
>> -upgrade = 
>> alpm_list_add(upgrade, pkg->name);
>> -} else {
>> -return 1;
>> -}
>> -}
>> -} else {
>> -if(t->op & ALPM_HOOK_OP_INSTALL) {
>> -if(hook->needs_targets) {
>> -install = 
>> alpm_list_add(install, pkg->name);
>> - 

[pacman-dev] [PATCH v2] Option to pass packages' versions to hook. Linearize code in _alpm_hook_trigger_match_pkg.

2016-09-04 Thread Sergey Petrenko via pacman-dev
Variables are now at beginning of block.

---
 lib/libalpm/hook.c| 104 +-
 test/pacman/tests/TESTS   |   1 +
 test/pacman/tests/hook-target-versions.py |  47 ++
 3 files changed, 120 insertions(+), 32 deletions(-)
 create mode 100644 test/pacman/tests/hook-target-versions.py

diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c
index ccde225..d37924f 100644
--- a/lib/libalpm/hook.c
+++ b/lib/libalpm/hook.c
@@ -55,7 +55,7 @@ struct _alpm_hook_t {
char **cmd;
alpm_list_t *matches;
alpm_hook_when_t when;
-   int abort_on_fail, needs_targets;
+   int abort_on_fail, needs_targets, needs_versions;
 };
 
 struct _alpm_hook_cb_ctx {
@@ -90,7 +90,7 @@ static void _alpm_hook_free(struct _alpm_hook_t *hook)
_alpm_wordsplit_free(hook->cmd);
alpm_list_free_inner(hook->triggers, (alpm_list_fn_free) 
_alpm_trigger_free);
alpm_list_free(hook->triggers);
-   alpm_list_free(hook->matches);
+   alpm_list_free_inner(hook->matches, free);
FREELIST(hook->depends);
free(hook);
}
@@ -329,8 +329,11 @@ static int _alpm_hook_parse_cb(const char *file, int line,
hook->abort_on_fail = 1;
} else if(strcmp(key, "NeedsTargets") == 0) {
hook->needs_targets = 1;
+   } else if(strcmp(key, "NeedsVersions") == 0) {
+   hook->needs_versions = 1;
} else if(strcmp(key, "Exec") == 0) {
-   if((hook->cmd = _alpm_wordsplit(value)) == NULL) {
+   if((hook->
+   cmd = _alpm_wordsplit(value)) == NULL) {
if(errno == EINVAL) {
error(_("hook %s line %d: invalid value 
%s\n"), file, line, value);
} else {
@@ -456,31 +459,54 @@ static int _alpm_hook_trigger_match_file(alpm_handle_t 
*handle,
 static int _alpm_hook_trigger_match_pkg(alpm_handle_t *handle,
struct _alpm_hook_t *hook, struct _alpm_trigger_t *t)
 {
-   alpm_list_t *install = NULL, *upgrade = NULL, *remove = NULL;
+   alpm_list_t *add = NULL, *remove = NULL;
 
if(t->op & ALPM_HOOK_OP_INSTALL || t->op & ALPM_HOOK_OP_UPGRADE) {
alpm_list_t *i;
for(i = handle->trans->add; i; i = i->next) {
alpm_pkg_t *pkg = i->data;
-   if(_alpm_fnmatch_patterns(t->targets, pkg->name) == 0) {
-   if(pkg->oldpkg) {
-   if(t->op & ALPM_HOOK_OP_UPGRADE) {
-   if(hook->needs_targets) {
-   upgrade = 
alpm_list_add(upgrade, pkg->name);
-   } else {
-   return 1;
-   }
-   }
-   } else {
-   if(t->op & ALPM_HOOK_OP_INSTALL) {
-   if(hook->needs_targets) {
-   install = 
alpm_list_add(install, pkg->name);
-   } else {
-   return 1;
-   }
-   }
+   size_t len;
+   char* output = NULL;
+   int upgrade_match = 0, install_match = 0;
+
+   if(_alpm_fnmatch_patterns(t->targets, pkg->name) != 0) {
+   continue;
+   }
+
+   if (pkg->oldpkg) {
+   upgrade_match = (t->op & ALPM_HOOK_OP_UPGRADE);
+   } else {
+   install_match = (t->op & ALPM_HOOK_OP_INSTALL);
+   }
+
+   if (!upgrade_match && !install_match) {
+   continue;
+   }
+
+   if (!hook->needs_targets) {
+   return 1;
+   }
+
+   len = strlen(pkg->name) + 1;
+   if (hook->needs_versions) {
+   if (upgrade_match) {
+   len += strlen(pkg->oldpkg->version) + 1;
}
+   len += strlen(pkg->version) + 1;
}
+   CALLOC(output, len, sizeof(char), continue);
+
+   strcat(output, pkg->name);
+   if 

Re: [pacman-dev] [PATCH] Option to pass packages' versions to hook. Linearize code in _alpm_hook_trigger_match_pkg.

2016-09-04 Thread Sergey Petrenko via pacman-dev
I'm writing hooks to solve infamous kernel problem. Having version in hook 
isn't necessary,
but will save me lot of work (and lot of time at each hook run).


>Воскресенье,  4 сентября 2016, 17:40 UTC от Dave Reisner <d...@falconindy.com>:
>
>On Sun, Sep 04, 2016 at 08:36:39PM +, Sergey Petrenko via pacman-dev wrote:
>> ---
>
>Missing seems to be some rationale for this patch. What's the problem
>you're interested in solving with this feature?
>
>>  lib/libalpm/hook.c| 102 
>> --
>>  test/pacman/tests/TESTS   |   1 +
>>  test/pacman/tests/hook-target-versions.py |  47 ++
>>  3 files changed, 118 insertions(+), 32 deletions(-)
>>  create mode 100644 test/pacman/tests/hook-target-versions.py
>> 
>> diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c
>> index ccde225..cc6c8bd 100644
>> --- a/lib/libalpm/hook.c
>> +++ b/lib/libalpm/hook.c
>> @@ -55,7 +55,7 @@ struct _alpm_hook_t {
>>  char **cmd;
>>  alpm_list_t *matches;
>>  alpm_hook_when_t when;
>> -int abort_on_fail, needs_targets;
>> +int abort_on_fail, needs_targets, needs_versions;
>>  };
>> 
>>  struct _alpm_hook_cb_ctx {
>> @@ -90,7 +90,7 @@ static void _alpm_hook_free(struct _alpm_hook_t *hook)
>>  _alpm_wordsplit_free(hook->cmd);
>>  alpm_list_free_inner(hook->triggers, (alpm_list_fn_free) 
>> _alpm_trigger_free);
>>  alpm_list_free(hook->triggers);
>> -alpm_list_free(hook->matches);
>> +alpm_list_free_inner(hook->matches, free);
>>  FREELIST(hook->depends);
>>  free(hook);
>>  }
>> @@ -329,8 +329,11 @@ static int _alpm_hook_parse_cb(const char *file, int 
>> line,
>>  hook->abort_on_fail = 1;
>>  } else if(strcmp(key, "NeedsTargets") == 0) {
>>  hook->needs_targets = 1;
>> +} else if(strcmp(key, "NeedsVersions") == 0) {
>> +hook->needs_versions = 1;
>>  } else if(strcmp(key, "Exec") == 0) {
>> -if((hook->cmd = _alpm_wordsplit(value)) == NULL) {
>> +if((hook->
>> +cmd = _alpm_wordsplit(value)) == NULL) {
>>  if(errno == EINVAL) {
>>  error(_("hook %s line %d: invalid value 
>> %s\n"), file, line, value);
>>  } else {
>> @@ -456,31 +459,53 @@ static int _alpm_hook_trigger_match_file(alpm_handle_t 
>> *handle,
>>  static int _alpm_hook_trigger_match_pkg(alpm_handle_t *handle,
>>  struct _alpm_hook_t *hook, struct _alpm_trigger_t *t)
>>  {
>> -alpm_list_t *install = NULL, *upgrade = NULL, *remove = NULL;
>> +alpm_list_t *add = NULL, *remove = NULL;
>> 
>>  if(t->op & ALPM_HOOK_OP_INSTALL || t->op & ALPM_HOOK_OP_UPGRADE) {
>>  alpm_list_t *i;
>>  for(i = handle->trans->add; i; i = i->next) {
>>  alpm_pkg_t *pkg = i->data;
>> -if(_alpm_fnmatch_patterns(t->targets, pkg->name) == 0) {
>> -if(pkg->oldpkg) {
>> -if(t->op & ALPM_HOOK_OP_UPGRADE) {
>> -if(hook->needs_targets) {
>> -upgrade = 
>> alpm_list_add(upgrade, pkg->name);
>> -} else {
>> -return 1;
>> -}
>> -}
>> -} else {
>> -if(t->op & ALPM_HOOK_OP_INSTALL) {
>> -if(hook->needs_targets) {
>> -install = 
>> alpm_list_add(install, pkg->name);
>> -} else {
>> -return 1;
>> -}
>> -}
>> +
>> +if(_alpm_fnmatch_patterns(t->targets, pkg->name) != 0) {
>> +continue;
>> +}
>> +
>> +  

[pacman-dev] [PATCH] Option to pass packages' versions to hook. Linearize code in _alpm_hook_trigger_match_pkg.

2016-09-04 Thread Sergey Petrenko via pacman-dev
---
 lib/libalpm/hook.c| 102 --
 test/pacman/tests/TESTS   |   1 +
 test/pacman/tests/hook-target-versions.py |  47 ++
 3 files changed, 118 insertions(+), 32 deletions(-)
 create mode 100644 test/pacman/tests/hook-target-versions.py

diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c
index ccde225..cc6c8bd 100644
--- a/lib/libalpm/hook.c
+++ b/lib/libalpm/hook.c
@@ -55,7 +55,7 @@ struct _alpm_hook_t {
char **cmd;
alpm_list_t *matches;
alpm_hook_when_t when;
-   int abort_on_fail, needs_targets;
+   int abort_on_fail, needs_targets, needs_versions;
 };
 
 struct _alpm_hook_cb_ctx {
@@ -90,7 +90,7 @@ static void _alpm_hook_free(struct _alpm_hook_t *hook)
_alpm_wordsplit_free(hook->cmd);
alpm_list_free_inner(hook->triggers, (alpm_list_fn_free) 
_alpm_trigger_free);
alpm_list_free(hook->triggers);
-   alpm_list_free(hook->matches);
+   alpm_list_free_inner(hook->matches, free);
FREELIST(hook->depends);
free(hook);
}
@@ -329,8 +329,11 @@ static int _alpm_hook_parse_cb(const char *file, int line,
hook->abort_on_fail = 1;
} else if(strcmp(key, "NeedsTargets") == 0) {
hook->needs_targets = 1;
+   } else if(strcmp(key, "NeedsVersions") == 0) {
+   hook->needs_versions = 1;
} else if(strcmp(key, "Exec") == 0) {
-   if((hook->cmd = _alpm_wordsplit(value)) == NULL) {
+   if((hook->
+   cmd = _alpm_wordsplit(value)) == NULL) {
if(errno == EINVAL) {
error(_("hook %s line %d: invalid value 
%s\n"), file, line, value);
} else {
@@ -456,31 +459,53 @@ static int _alpm_hook_trigger_match_file(alpm_handle_t 
*handle,
 static int _alpm_hook_trigger_match_pkg(alpm_handle_t *handle,
struct _alpm_hook_t *hook, struct _alpm_trigger_t *t)
 {
-   alpm_list_t *install = NULL, *upgrade = NULL, *remove = NULL;
+   alpm_list_t *add = NULL, *remove = NULL;
 
if(t->op & ALPM_HOOK_OP_INSTALL || t->op & ALPM_HOOK_OP_UPGRADE) {
alpm_list_t *i;
for(i = handle->trans->add; i; i = i->next) {
alpm_pkg_t *pkg = i->data;
-   if(_alpm_fnmatch_patterns(t->targets, pkg->name) == 0) {
-   if(pkg->oldpkg) {
-   if(t->op & ALPM_HOOK_OP_UPGRADE) {
-   if(hook->needs_targets) {
-   upgrade = 
alpm_list_add(upgrade, pkg->name);
-   } else {
-   return 1;
-   }
-   }
-   } else {
-   if(t->op & ALPM_HOOK_OP_INSTALL) {
-   if(hook->needs_targets) {
-   install = 
alpm_list_add(install, pkg->name);
-   } else {
-   return 1;
-   }
-   }
+
+   if(_alpm_fnmatch_patterns(t->targets, pkg->name) != 0) {
+   continue;
+   }
+
+   int upgrade_match = 0, install_match = 0;
+   if (pkg->oldpkg) {
+   upgrade_match = (t->op & ALPM_HOOK_OP_UPGRADE);
+   } else {
+   install_match = (t->op & ALPM_HOOK_OP_INSTALL);
+   }
+
+   if (!upgrade_match && !install_match) {
+   continue;
+   }
+
+   if (!hook->needs_targets) {
+   return 1;
+   }
+
+   char* output = NULL;
+   size_t len = strlen(pkg->name) + 1;
+   if (hook->needs_versions) {
+   if (upgrade_match) {
+   len += strlen(pkg->oldpkg->version) + 1;
}
+   len += strlen(pkg->version) + 1;
}
+   CALLOC(output, len, sizeof(char), continue);
+
+   strcat(output, pkg->name);
+   if (hook->needs_versions) {
+   if (upgrade_match) {