[PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

morganamilo
Commit 106d0fc54 Added the usage option for databases and
alpm_sync_newversion was restricted by USAGE_SEARCH instead of
USAGE_UPGRADE.

USAGE_UPGRADE should be used instead. This means packages only show up
in commands such as `pacman -Qu` if the database Has the Upgrade option.

Signed-off-by: morganamilo <[hidden email]>

diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index 696a5131..23b0ccfa 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -61,7 +61,7 @@ alpm_pkg_t SYMEXPORT *alpm_sync_newversion(alpm_pkg_t *pkg, alpm_list_t *dbs_syn
 
  for(i = dbs_sync; !spkg && i; i = i->next) {
  alpm_db_t *db = i->data;
- if(!(db->usage & ALPM_DB_USAGE_SEARCH)) {
+ if(!(db->usage & ALPM_DB_USAGE_UPGRADE)) {
  continue;
  }
 
--
2.18.0
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

dave reisner
On Tue, Jul 24, 2018 at 05:53:00PM +0100, morganamilo wrote:
> Commit 106d0fc54 Added the usage option for databases and
> alpm_sync_newversion was restricted by USAGE_SEARCH instead of
> USAGE_UPGRADE.

I don't recall exactly what my thinking was when I wrote this patch, but
looking at 'pacman -Qu' output right now, I think I actually I like
seeing potential upgrades, not just "actual" upgrades.

Anyone else have an opinion?

> USAGE_UPGRADE should be used instead. This means packages only show up
> in commands such as `pacman -Qu` if the database Has the Upgrade option.
>
> Signed-off-by: morganamilo <[hidden email]>
>
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index 696a5131..23b0ccfa 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -61,7 +61,7 @@ alpm_pkg_t SYMEXPORT *alpm_sync_newversion(alpm_pkg_t *pkg, alpm_list_t *dbs_syn
>  
>   for(i = dbs_sync; !spkg && i; i = i->next) {
>   alpm_db_t *db = i->data;
> - if(!(db->usage & ALPM_DB_USAGE_SEARCH)) {
> + if(!(db->usage & ALPM_DB_USAGE_UPGRADE)) {
>   continue;
>   }
>  
> --
> 2.18.0
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

Erich Eckner
On 24.07.2018 20:48, Dave Reisner wrote:

> On Tue, Jul 24, 2018 at 05:53:00PM +0100, morganamilo wrote:
>> Commit 106d0fc54 Added the usage option for databases and
>> alpm_sync_newversion was restricted by USAGE_SEARCH instead of
>> USAGE_UPGRADE.
>
> I don't recall exactly what my thinking was when I wrote this patch, but
> looking at 'pacman -Qu' output right now, I think I actually I like
> seeing potential upgrades, not just "actual" upgrades.
>
> Anyone else have an opinion?
Frankly, I don't really understand what would change - what does "the
database Has the Upgrade option" mean? What is the difference between an
"actual" and a "potential" upgrade?

Please exuse if these are stupid questions - I'm just a user of "-Qu".

regards,
Erich

>
>> USAGE_UPGRADE should be used instead. This means packages only show up
>> in commands such as `pacman -Qu` if the database Has the Upgrade option.
>>
>> Signed-off-by: morganamilo <[hidden email]>
>>
>> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
>> index 696a5131..23b0ccfa 100644
>> --- a/lib/libalpm/sync.c
>> +++ b/lib/libalpm/sync.c
>> @@ -61,7 +61,7 @@ alpm_pkg_t SYMEXPORT *alpm_sync_newversion(alpm_pkg_t *pkg, alpm_list_t *dbs_syn
>>  
>>   for(i = dbs_sync; !spkg && i; i = i->next) {
>>   alpm_db_t *db = i->data;
>> - if(!(db->usage & ALPM_DB_USAGE_SEARCH)) {
>> + if(!(db->usage & ALPM_DB_USAGE_UPGRADE)) {
>>   continue;
>>   }
>>  
>> --
>> 2.18.0


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

morganamilo
On Tue, 24 Jul 2018 at 20:24, Erich Eckner <[hidden email]> wrote:

>
> On 24.07.2018 20:48, Dave Reisner wrote:
> > On Tue, Jul 24, 2018 at 05:53:00PM +0100, morganamilo wrote:
> >> Commit 106d0fc54 Added the usage option for databases and
> >> alpm_sync_newversion was restricted by USAGE_SEARCH instead of
> >> USAGE_UPGRADE.
> >
> > I don't recall exactly what my thinking was when I wrote this patch, but
> > looking at 'pacman -Qu' output right now, I think I actually I like
> > seeing potential upgrades, not just "actual" upgrades.
> >
> > Anyone else have an opinion?
>
> Frankly, I don't really understand what would change - what does "the
> database Has the Upgrade option" mean? What is the difference between an
> "actual" and a "potential" upgrade?
>
> Please exuse if these are stupid questions - I'm just a user of "-Qu".
>
> regards,
> Erich
>
> >
> >> USAGE_UPGRADE should be used instead. This means packages only show up
> >> in commands such as `pacman -Qu` if the database Has the Upgrade option.
> >>
> >> Signed-off-by: morganamilo <[hidden email]>
> >>
> >> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> >> index 696a5131..23b0ccfa 100644
> >> --- a/lib/libalpm/sync.c
> >> +++ b/lib/libalpm/sync.c
> >> @@ -61,7 +61,7 @@ alpm_pkg_t SYMEXPORT *alpm_sync_newversion(alpm_pkg_t *pkg, alpm_list_t *dbs_syn
> >>
> >>      for(i = dbs_sync; !spkg && i; i = i->next) {
> >>              alpm_db_t *db = i->data;
> >> -            if(!(db->usage & ALPM_DB_USAGE_SEARCH)) {
> >> +            if(!(db->usage & ALPM_DB_USAGE_UPGRADE)) {
> >>                      continue;
> >>              }
> >>
> >> --
> >> 2.18.0
>

See the Usage option in pacman.conf (5)

Personally I would like -Qu to show actual upgrades. Mainly for the
checkupdates script. Which many people use create update notifiers.
It's quite annoying to have a notification always saying there's an
update available when in reality it's just a package in a database
that you only have Usage = Sync Search.

On a lower level I would also like to use alpm_sync_newversion so get
pending upgrades. The alternative right now Is starting a transaction
and calling alpm_sync_sysupgrade.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

Andrew Gregory
In reply to this post by dave reisner
On 07/24/18 at 02:48pm, Dave Reisner wrote:

> On Tue, Jul 24, 2018 at 05:53:00PM +0100, morganamilo wrote:
> > Commit 106d0fc54 Added the usage option for databases and
> > alpm_sync_newversion was restricted by USAGE_SEARCH instead of
> > USAGE_UPGRADE.
>
> I don't recall exactly what my thinking was when I wrote this patch, but
> looking at 'pacman -Qu' output right now, I think I actually I like
> seeing potential upgrades, not just "actual" upgrades.
>
> Anyone else have an opinion?

Based on how it's used, I'd say it should be SEARCH; it's being used
as a filter for -Q and no upgrade transaction is being performed, or
even prepared.

Really, though, I'd say this is a great example as to why usages
should have been implemented in the front-end or limited to only the
highest-level library functions.  Usage is contextual, how is libalpm
supposed to know how such a basic function is being used?  pacman may
only use it as a filter for -Q, but some other front-end could use it
to actually prepare an upgrade.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

morganamilo
On Tue, 24 Jul 2018 at 20:40, Andrew Gregory <[hidden email]> wrote:

> Based on how it's used, I'd say it should be SEARCH; it's being used
> as a filter for -Q and no upgrade transaction is being performed, or
> even prepared.
>
> Really, though, I'd say this is a great example as to why usages
> should have been implemented in the front-end or limited to only the
> highest-level library functions.  Usage is contextual, how is libalpm
> supposed to know how such a basic function is being used?  pacman may
> only use it as a filter for -Q, but some other front-end could use it
> to actually prepare an upgrade.

The thing is pacman with not let you use -s with -u: error: invalid
option: '--search' and '--upgrade' may not be used together. By that
logic you could argue it is not a search at all.
Front ends aside the function is called alpm_sync_newversion, it makes
no mention to searching.

Slightly off topic of the original patch. Playing around more I've
come to find that Upgrade implies Install. Is that an oversight or is
it intended?
As for use case I would like tp pacman -S db/pkg explicitly and have
them upgrade but never touch the repo during a normal pacman -S pkg.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

Andrew Gregory
On 07/25/18 at 02:23am, Morgan Adamiec wrote:

> On Tue, 24 Jul 2018 at 20:40, Andrew Gregory <[hidden email]> wrote:
> > Based on how it's used, I'd say it should be SEARCH; it's being used
> > as a filter for -Q and no upgrade transaction is being performed, or
> > even prepared.
> >
> > Really, though, I'd say this is a great example as to why usages
> > should have been implemented in the front-end or limited to only the
> > highest-level library functions.  Usage is contextual, how is libalpm
> > supposed to know how such a basic function is being used?  pacman may
> > only use it as a filter for -Q, but some other front-end could use it
> > to actually prepare an upgrade.
>
> The thing is pacman with not let you use -s with -u: error: invalid
> option: '--search' and '--upgrade' may not be used together. By that
> logic you could argue it is not a search at all.
> Front ends aside the function is called alpm_sync_newversion, it makes
> no mention to searching.

It makes no mention of upgrading either.

> Slightly off topic of the original patch. Playing around more I've
> come to find that Upgrade implies Install. Is that an oversight or is
> it intended?
> As for use case I would like tp pacman -S db/pkg explicitly and have
> them upgrade but never touch the repo during a normal pacman -S pkg.

How is that different from just putting that repo below the one you
want it to use normally?
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

morganamilo
On Wed, 25 Jul 2018 at 02:28, Andrew Gregory <[hidden email]> wrote:
> It makes no mention of upgrading either

It is called alpm_sync_*newversion*

>How is that different from just putting that repo below the one you
want it to use normally?

pacman -S pkg should return an error. These packages are not in any
other of my repos, so there is nothing to move it under really.

But bedsides that It would just make more sense for Upgrade to not
imply install. Or at least document that it does.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

Eli Schwartz-2
In reply to this post by dave reisner
On 07/24/2018 02:48 PM, Dave Reisner wrote:

> On Tue, Jul 24, 2018 at 05:53:00PM +0100, morganamilo wrote:
>> Commit 106d0fc54 Added the usage option for databases and
>> alpm_sync_newversion was restricted by USAGE_SEARCH instead of
>> USAGE_UPGRADE.
>
> I don't recall exactly what my thinking was when I wrote this patch, but
> looking at 'pacman -Qu' output right now, I think I actually I like
> seeing potential upgrades, not just "actual" upgrades.
>
> Anyone else have an opinion?
I don't really see the utility... a search-only database will not show
the upgrades in pacman -Su (even ignored packages get shown, just as a
warning instead of an active transaction member).

pacman -Qu should therefore (IMHO) only show those Sync packages (with
[ignored] in cases, yes).

When people hear about -Qu, they generally hear about it as "oh, the
thing which is like -Su except it doesn't actually do the thing".

Admittedly the actual thing which doesn't actually do the thing would be
pacman -Sup --print-format '%r/%n %v'

But then we're faced with the classic problem "print-format? no such
thing, we don't support that and you should use expac, HAHAHAHA, oh wait
print-format is actually useful here (surprise!)"

So actually I guess in theory it's completely different, but now I'm not
sure what I would specifically use this behavior for.

--
Eli Schwartz
Bug Wrangler and Trusted User


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

Eli Schwartz-2
In reply to this post by morganamilo
On 07/24/2018 09:23 PM, Morgan Adamiec wrote:

> On Tue, 24 Jul 2018 at 20:40, Andrew Gregory <[hidden email]> wrote:
>> Based on how it's used, I'd say it should be SEARCH; it's being used
>> as a filter for -Q and no upgrade transaction is being performed, or
>> even prepared.
>>
>> Really, though, I'd say this is a great example as to why usages
>> should have been implemented in the front-end or limited to only the
>> highest-level library functions.  Usage is contextual, how is libalpm
>> supposed to know how such a basic function is being used?  pacman may
>> only use it as a filter for -Q, but some other front-end could use it
>> to actually prepare an upgrade.
>
> The thing is pacman with not let you use -s with -u: error: invalid
> option: '--search' and '--upgrade' may not be used together. By that
> logic you could argue it is not a search at all.
It has nothing to do with "upgrades", -S means different things
depending on whether you just give it package names, a bunch of inferred
package names via -u, or whatever.

-Us doesn't work either. Nor does -Ssw.

Likewise this low-level function I guess is not called just by pacman
-Su ...

> Front ends aside the function is called alpm_sync_newversion, it makes
> no mention to searching.

It is a function to find packages from sync repositories (in contrast to
the "local" repository) that have new versions. I think it's obvious
this function does not handle the actual syncing...

Anyway. Seems to me the name is a reference to the local/remote nature
of the repo, not its Usage field.

> Slightly off topic of the original patch. Playing around more I've
> come to find that Upgrade implies Install. Is that an oversight or is
> it intended?

It implies this where? Shouldn't anything using it be
checking/specifying both bitmasks?

--
Eli Schwartz
Bug Wrangler and Trusted User


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

Allan McRae
In reply to this post by dave reisner
On 25/07/18 04:48, Dave Reisner wrote:

> On Tue, Jul 24, 2018 at 05:53:00PM +0100, morganamilo wrote:
>> Commit 106d0fc54 Added the usage option for databases and
>> alpm_sync_newversion was restricted by USAGE_SEARCH instead of
>> USAGE_UPGRADE.
> I don't recall exactly what my thinking was when I wrote this patch, but
> looking at 'pacman -Qu' output right now, I think I actually I like
> seeing potential upgrades, not just "actual" upgrades.
>
> Anyone else have an opinion?
>

I just spent some time looking into this...  only a month after submission!

My thoughts:

1) Implementing --print-format usage for -Su is the correct way to
determine updates.  (I guess in combination with --no-confirm?)

2) -Qu should label those package updates that are available but won't
be updated by -Su due to database restriction.  We currently do this for
packages in IgnorePkg, so should be an "easy" extension.

Would that be suitable for everyone?

Allan
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

Eli Schwartz-2
On 8/29/18 12:30 AM, Allan McRae wrote:

> On 25/07/18 04:48, Dave Reisner wrote:
>> On Tue, Jul 24, 2018 at 05:53:00PM +0100, morganamilo wrote:
>>> Commit 106d0fc54 Added the usage option for databases and
>>> alpm_sync_newversion was restricted by USAGE_SEARCH instead of
>>> USAGE_UPGRADE.
>> I don't recall exactly what my thinking was when I wrote this patch, but
>> looking at 'pacman -Qu' output right now, I think I actually I like
>> seeing potential upgrades, not just "actual" upgrades.
>>
>> Anyone else have an opinion?
>>
>
> I just spent some time looking into this...  only a month after submission!
>
> My thoughts:
>
> 1) Implementing --print-format usage for -Su is the correct way to
> determine updates.  (I guess in combination with --no-confirm?)
It works today, though, as I mentioned above. The only "problem" is that
people never remember --print-format exists, mostly because it's not
implemented in all the *other* cases where we'd want to have it.

> 2) -Qu should label those package updates that are available but won't
> be updated by -Su due to database restriction.  We currently do this for
> packages in IgnorePkg, so should be an "easy" extension.
>
> Would that be suitable for everyone?

That sounds like something very nice to have, yes.

--
Eli Schwartz
Bug Wrangler and Trusted User


signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

Allan McRae
On 29/08/18 14:59, Eli Schwartz wrote:
>> My thoughts:
>>
>> 1) Implementing --print-format usage for -Su is the correct way to
>> determine updates.  (I guess in combination with --no-confirm?)
>
> It works today, though, as I mentioned above. The only "problem" is that
> people never remember --print-format exists, mostly because it's not
> implemented in all the *other* cases where we'd want to have it.
>

Well...   that is embarrassing!  Seems I forgot it works for -Su too!

Someone should send the pacman-contrib maintainers a patch to use that
instead of -Qu.

A
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libalpm/sync.c: restrict alpm_sync_newversion by USAGE_UPGRADE

morganamilo
On Wed, 29 Aug 2018 at 06:03, Allan McRae <[hidden email]> wrote:
> Someone should send the pacman-contrib maintainers a patch to use that
> instead of -Qu.

Sadly you would need some sort of 'currentpkgver' formatter
`--print-format "%n %currentpkgver -> %v"`.