diff mbox

[pacman-dev] libalpm: ignore .hook suffix when sorting hooks

Message ID 20180609174559.GA1482@Mindship-03
State Accepted, archived
Headers show

Commit Message

Jouke Witteveen June 9, 2018, 5:45 p.m. UTC
It is desirable to have 'a-post.hook' ordered after 'a.hook'. For this,
it is needed to ignore the suffix when sorting.
---

Two years ago, I suggested this patch in https://bugs.archlinux.org/task/49653.
Today, I do so again :-).

Thanks to Jelle van der Waa for pointing me to the correct list.

Regards,
- Jouke

 lib/libalpm/hook.c | 16 ++++++++++++----
 lib/libalpm/hook.h |  2 ++
 2 files changed, 14 insertions(+), 4 deletions(-)

Comments

Allan McRae June 19, 2018, 2:24 p.m. UTC | #1
On 10/06/18 03:45, Jouke Witteveen wrote:
> It is desirable to have 'a-post.hook' ordered after 'a.hook'. For this,
> it is needed to ignore the suffix when sorting.
> ---
> 
> Two years ago, I suggested this patch in https://bugs.archlinux.org/task/49653.
> Today, I do so again :-).
> 
> Thanks to Jelle van der Waa for pointing me to the correct list.
> 
> Regards,
> - Jouke
> 

Thanks for chasing this up.  We tend to forget about patches in the bug
tracker if one of the devs does not submit them to the list.

Patch looks fine to me.

A
Jouke Witteveen July 27, 2018, 7:26 a.m. UTC | #2
On Tue, Jun 19, 2018 at 4:25 PM Allan McRae <allan@archlinux.org> wrote:
>
> On 10/06/18 03:45, Jouke Witteveen wrote:
> > It is desirable to have 'a-post.hook' ordered after 'a.hook'. For this,
> > it is needed to ignore the suffix when sorting.
> > ---
> >
> > Two years ago, I suggested this patch in https://bugs.archlinux.org/task/49653.
> > Today, I do so again :-).
> >
> > Thanks to Jelle van der Waa for pointing me to the correct list.
> >
> > Regards,
> > - Jouke
> >
>
> Thanks for chasing this up.  We tend to forget about patches in the bug
> tracker if one of the devs does not submit them to the list.
>
> Patch looks fine to me.

This one was omitted from 5.1.1, but the documentation was included!

- Jouke
Jouke Witteveen Aug. 3, 2018, 4:16 p.m. UTC | #3
On Fri, Jul 27, 2018 at 9:26 AM Jouke Witteveen <j.witteveen@gmail.com> wrote:
>
> On Tue, Jun 19, 2018 at 4:25 PM Allan McRae <allan@archlinux.org> wrote:
> >
> > On 10/06/18 03:45, Jouke Witteveen wrote:
> > > It is desirable to have 'a-post.hook' ordered after 'a.hook'. For this,
> > > it is needed to ignore the suffix when sorting.
> > > ---
> > >
> > > Two years ago, I suggested this patch in https://bugs.archlinux.org/task/49653.
> > > Today, I do so again :-).
> >
> > Thanks for chasing this up.  We tend to forget about patches in the bug
> > tracker if one of the devs does not submit them to the list.
> >
> > Patch looks fine to me.
>
> This one was omitted from 5.1.1, but the documentation was included!

Shall I wait two more years and try again ;-)?

Regards,
- Jouke
Eli Schwartz Aug. 3, 2018, 4:29 p.m. UTC | #4
On 08/03/2018 12:16 PM, Jouke Witteveen wrote:
>> This one was omitted from 5.1.1, but the documentation was included!
> 
> Shall I wait two more years and try again ;-)?
> 
> Regards,
> - Jouke

It's queued for post-5.1.1:
https://git.archlinux.org/users/allan/pacman.git/commit/?h=post-5.1.1&id=855689788962671d8b75aeafdaefdc1483ed6d15

Marked as accepted: https://patchwork.archlinux.org/patch/599/

I asked allan if he wanted to update the documentation now, because
making documentation better is something easy to do even in bugfix-only
releases. On closer look, neither of us noticed that the documentation
added "where the ordering ignores the suffix"...

For future reference, I advise you in cases like this to submit a
'[PATCH 1/2] include more information on hook files" *without* this
change, then a "[PATCH 2/2]" which changes the behavior of libalpm and
updates the documentation to match. :)
Jouke Witteveen Aug. 3, 2018, 4:54 p.m. UTC | #5
On Fri, Aug 3, 2018 at 6:29 PM Eli Schwartz <eschwartz@archlinux.org> wrote:
>
> On 08/03/2018 12:16 PM, Jouke Witteveen wrote:
> >> This one was omitted from 5.1.1, but the documentation was included!
> >
> > Shall I wait two more years and try again ;-)?
> >
> > Regards,
> > - Jouke
>
> It's queued for post-5.1.1:
> https://git.archlinux.org/users/allan/pacman.git/commit/?h=post-5.1.1&id=855689788962671d8b75aeafdaefdc1483ed6d15
>
> Marked as accepted: https://patchwork.archlinux.org/patch/599/
>
> I asked allan if he wanted to update the documentation now, because
> making documentation better is something easy to do even in bugfix-only
> releases. On closer look, neither of us noticed that the documentation
> added "where the ordering ignores the suffix"...
>
> For future reference, I advise you in cases like this to submit a
> '[PATCH 1/2] include more information on hook files" *without* this
> change, then a "[PATCH 2/2]" which changes the behavior of libalpm and
> updates the documentation to match. :)

Will do, thanks!
- Jouke
Allan McRae Aug. 3, 2018, 10:40 p.m. UTC | #6
On 04/08/18 02:29, Eli Schwartz wrote:
> On 08/03/2018 12:16 PM, Jouke Witteveen wrote:
>>> This one was omitted from 5.1.1, but the documentation was included!
>>
>> Shall I wait two more years and try again ;-)?
>>
>> Regards,
>> - Jouke
> 
> It's queued for post-5.1.1:
> https://git.archlinux.org/users/allan/pacman.git/commit/?h=post-5.1.1&id=855689788962671d8b75aeafdaefdc1483ed6d15
> 
> Marked as accepted: https://patchwork.archlinux.org/patch/599/
> 
> I asked allan if he wanted to update the documentation now, because
> making documentation better is something easy to do even in bugfix-only
> releases. On closer look, neither of us noticed that the documentation
> added "where the ordering ignores the suffix"...
> 
> For future reference, I advise you in cases like this to submit a
> '[PATCH 1/2] include more information on hook files" *without* this
> change, then a "[PATCH 2/2]" which changes the behavior of libalpm and
> updates the documentation to match. :)
> 

I knew I had applied it...   Just not to the branch I thought I had!

I might try and do some post-5.1.1 patch review soon.

A
diff mbox

Patch

diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c
index 0805e661..d90ed2da 100644
--- a/lib/libalpm/hook.c
+++ b/lib/libalpm/hook.c
@@ -551,7 +551,16 @@  static int _alpm_hook_triggered(alpm_handle_t *handle, struct _alpm_hook_t *hook
 
 static int _alpm_hook_cmp(struct _alpm_hook_t *h1, struct _alpm_hook_t *h2)
 {
-	return strcmp(h1->name, h2->name);
+	size_t suflen = strlen(ALPM_HOOK_SUFFIX), l1, l2;
+	int ret;
+	l1 = strlen(h1->name) - suflen;
+	l2 = strlen(h2->name) - suflen;
+	/* exclude the suffixes from comparison */
+	ret = strncmp(h1->name, h2->name, l1 <= l2 ? l1 : l2);
+	if(ret == 0 && l1 != l2) {
+		return l1 < l2 ? -1 : 1;
+	}
+	return ret;
 }
 
 static alpm_list_t *find_hook(alpm_list_t *haystack, const void *needle)
@@ -634,8 +643,7 @@  int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when)
 	alpm_event_hook_t event = { .when = when };
 	alpm_event_hook_run_t hook_event;
 	alpm_list_t *i, *hooks = NULL, *hooks_triggered = NULL;
-	const char *suffix = ".hook";
-	size_t suflen = strlen(suffix), triggered = 0;
+	size_t suflen = strlen(ALPM_HOOK_SUFFIX), triggered = 0;
 	int ret = 0;
 
 	for(i = alpm_list_last(handle->hookdirs); i; i = alpm_list_previous(i)) {
@@ -681,7 +689,7 @@  int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when)
 			memcpy(path + dirlen, entry->d_name, name_len + 1);
 
 			if(name_len < suflen
-					|| strcmp(entry->d_name + name_len - suflen, suffix) != 0) {
+					|| strcmp(entry->d_name + name_len - suflen, ALPM_HOOK_SUFFIX) != 0) {
 				_alpm_log(handle, ALPM_LOG_DEBUG, "skipping non-hook file %s\n", path);
 				continue;
 			}
diff --git a/lib/libalpm/hook.h b/lib/libalpm/hook.h
index 364d22d7..30d565df 100644
--- a/lib/libalpm/hook.h
+++ b/lib/libalpm/hook.h
@@ -22,6 +22,8 @@ 
 
 #include "alpm.h"
 
+#define ALPM_HOOK_SUFFIX ".hook"
+
 int _alpm_hook_run(alpm_handle_t *handle, alpm_hook_when_t when);
 
 #endif /* ALPM_HOOK_H */