Re: [pacman-dev] [PATCH 1/4] util/pkgbuild: guard against unset variable

[Eli Schwartz]

On 3/2/17 00:11 AM, Andrew Gregory wrote:
> On 03/02/17 at 02:59pm, Allan McRae wrote:
>> On 02/03/17 14:46, Andrew Gregory wrote:
>> > On 03/02/17 at 02:37pm, Allan McRae wrote:
>> >> On 26/02/17 03:21, Andrew Gregory wrote:
>> >>> Allows use under 'set -u'.
>> >>> ---
>> >>>  scripts/libmakepkg/util/pkgbuild.sh.in | 2 +-
>> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>>
>> >>> diff --git a/scripts/libmakepkg/util/pkgbuild.sh.in 
>> >>> b/scripts/libmakepkg/util/pkgbuild.sh.in
>> >>> index 2a4bd3af..08b35f53 100644
>> >>> --- a/scripts/libmakepkg/util/pkgbuild.sh.in
>> >>> +++ b/scripts/libmakepkg/util/pkgbuild.sh.in
>> >>> @@ -18,7 +18,7 @@
>> >>>  #   along with this program.  If not, see 
>> >>> .
>> >>>  #
>> >>>  
>> >>> -[[ -n "$LIBMAKEPKG_UTIL_PKGBUILD_SH" ]] && return
>> >>> +[[ -n "${LIBMAKEPKG_UTIL_PKGBUILD_SH:-}" ]] && return
>> >>>  LIBMAKEPKG_UTIL_PKGBUILD_SH=1
>> >>>  
>> >>>
>> >>
>> >> Is this going to need done for every file that you test?  I'd prefer one
>> >> big patch if so.
>> >>
>> >> A
>> > 
>> > If we want to run the tests with 'set -u', this, or something similar
>> > will have to be done for all libmakepkg files.  I actually think
>> > I like `[[ -v LIBMAKEPKG_UTIL_PKGBUILD_SH ]]` better though.
>> > 
>> 
>> How long has bash 4.2 been around?   Our minimum is currently 4.1.
>
> 4.2 appears to have been tagged at the end of 2011.

We now require 4.4 anyway...

-- 
Eli Schwartz
Bug Wrangler and Trusted User



signature.asc
Description: OpenPGP digital signature

[pacman-dev] [GIT] The official pacman repository branch, master, updated. v5.1.1-35-g7afe5117

[Allan McRae]

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  7afe51171fe063bf3031cc68fc8c7ac914a01de2 (commit)
   via  bae74c8e9e69b4f4e1a235eef21b9b27fb14aff0 (commit)
   via  cd7b2d6e07bdbc11f8973bedef0cb0ef02f81563 (commit)
   via  192d6166e9cb2a8f26d7256690e0158bd5a5d226 (commit)
   via  b54b33d816cdc3d1aab3b80f4eb94c5bad56c889 (commit)
   via  3d5a056452fe897e533edfac2ddbe2d1ca702a6e (commit)
   via  961ef1a4c8cfc0fa7b8da4e6cb77d8327934e32f (commit)
   via  2bec380e108536f5e5f728ef66223ed3fabf5ab1 (commit)
   via  3318039e3b1530396b0e3ced49ea6fe5b6ea00c5 (commit)
   via  ba2984db3e83e0627c29897e28982e9f4cf24a5d (commit)
   via  58c76daf5e9116dd044f73958b67163549d0e795 (commit)
   via  d03409ccde5a995f19e40a12d61b22be9d4c3af7 (commit)
   via  62eef5bbdb025d9557a1609760b42d7fbac16ad2 (commit)
   via  5b2ff51c399a906fd70df62ca179e2a696ea3860 (commit)
   via  16f6aae33087ec04c1dc90aa84f7d3dde4333046 (commit)
   via  48c8f9f2a24632dabe5b1c74a474e3940ddd8748 (commit)
   via  0696307a3b2e0cbde7d208eb78bbad6a9c8b336f (commit)
   via  3370c08a29a60e1cd1227d43652c22738c8e4f6e (commit)
   via  3e9a62e72139a71cb7c41a5e4bd896d22943cd7b (commit)
   via  9fde55c0c71f473d32fb638eb4f251041e3b3ffa (commit)
   via  8b2f3323b84a34d1d104136e455302dc32892b9a (commit)
   via  2d8d8af915d352b61178a981603360c27a3899f2 (commit)
  from  7d05ffceaf9161a6572505d25b5017e1eb33bf0e (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 7afe51171fe063bf3031cc68fc8c7ac914a01de2
Author: Eli Schwartz 
Date:   Mon Oct 30 14:15:19 2017 -0400

repo-add: add support for the zst format

Signed-off-by: Eli Schwartz 
Signed-off-by: Allan McRae 

commit bae74c8e9e69b4f4e1a235eef21b9b27fb14aff0
Author: Eli Schwartz 
Date:   Mon Oct 30 14:15:18 2017 -0400

makepkg: add support for the zst format

Signed-off-by: Eli Schwartz 
Signed-off-by: Allan McRae 

commit cd7b2d6e07bdbc11f8973bedef0cb0ef02f81563
Author: Luke Shumaker 
Date:   Thu Aug 9 13:42:37 2018 -0400

makepkg: lint_pkgver: Run even if PKGVERFUNC

lint_pkgver returns 0 if PKGVERFUNC, since it's likely that update_pkgver()
will change the value of pkgver anyway, and there's no point in linting the
old value.  update_pkgver() will call check_pkgver() itself to validate the
new value.

However, that "optimization" only holds if we're definitely going to call
update_pkgver() later; and that's way more complicated than

if (( PKGVERFUNC )); then

it's more like:

if (( !GENINTEG && !PACKAGELIST && !PRINTSRCINFO && !SOURCEONLY && 
!REPKG && PKGVERFUNC )); then

Which is to say: If I have a PKGBUILD with pkgver():

 * if I run `makepkg -g` I expect it to lint pkgver, but it won't
 * if I run `makepkg -R` I expect it to lint pkgver, but it won't
 * ...

So let's fix that.

Rather than try to keep a huge list of conditions in sync with the flow of
makepkg.sh.in, let's just drop it.  As far as I can tell, the only thing
that skipping lint_pkgver() really enables is letting the PKGBUILD author
write `pkgver=` in the initial version, and letting pkgver() fill it in.
They can just start writing `pkgver=0` for that workflow.

Signed-off-by: Allan McRae 

commit 192d6166e9cb2a8f26d7256690e0158bd5a5d226
Author: David Phillips 
Date:   Wed Sep 19 14:30:37 2018 +1200

User-visible log when validity check fails due to access

Currently, if checking the validity of packages fails due to an access
error on one or more packages, the user must sift through debug output
in order to find the culprit package(s). This patch adds a call to
_alpm_log in such a case to make the culprits more easily visible.

Signed-off-by: Allan McRae 

commit b54b33d816cdc3d1aab3b80f4eb94c5bad56c889
Author: David Phillips 
Date:   Wed Sep 19 14:28:35 2018 +1200

Change if-else chain to switch

Signed-off-by: Allan McRae 

commit 3d5a056452fe897e533edfac2ddbe2d1ca702a6e
Author: Eli Schwartz 
Date:   Thu Jun 21 12:39:21 2018 -0400

makepkg: reject PKGBUILDs with both split and non-split package functions

We accept package_foo() in non-split packages, because it's easier to
switch to/from a split package just by removing a pkgname element. But
it makes no sense to have both in one PKGBUILD.

Signed-off-by: Eli Schwartz 
Signed-off-by: Allan McRae 

commit 961ef1a4c8cfc0fa7b8da4e6cb77d8327934e32f
Author: morganamilo 
Date:   Tue Sep 4 14:47:44 2018 

Re: [pacman-dev] [PATCH] during -Qu add [ignored] for repos without Usage = Upgrade

[Morgan Adamiec]

On Wed, 19 Sep 2018 at 22:28, Allan McRae  wrote:
> My concern is of users of that function other than pacman (if there are
> any...).  Adding a parameter at least flags the change in behaviour an
> allows other users to keep it the way it was.

Ah yeah good point, forgot that other frontends are a thing. Your idea
is probably
best then. Thanks for all the feedback.

Re: [pacman-dev] [PATCH] during -Qu add [ignored] for repos without Usage = Upgrade

[Allan McRae]

On 20/9/18 3:18 am, Morgan Adamiec wrote:
> Yeah -Qu is still a little funky.
> 
>> It gets better...   packages in databases with Usage = Upgrade do not
>> show up in -Qu.  That is clearly wrong!
> 
> I left the requirement on Usage = Search after every one disagreed with
> my patch [1] to change it. Although I would prefer it if Search had no
> bearing on this.
> 
> So with this patch, Search is needed for a package to be listed and
> Upgrade is needed for [ignore] to not be added.
> 
>> My plan is to change alpm_sync_newversion to take an additional
>> parameter to flag whether it should include all databases or just those
>> available for upgrades.  It could even be make more flexible and just
>> take the bitmask as a parameter for filtering.
> 
> Why not just remove the Usage check all together? You need
> alpm_sync_newversion to find the packages then later we do the Usage =
> Upgrade check to add [ignored] to the end of the line.

My concern is of users of that function other than pacman (if there are
any...).  Adding a parameter at least flags the change in behaviour an
allows other users to keep it the way it was.

> Unless you want the packages to not show up at all? That's exactly what
> the previous patch [1] did.
> 
> [1] https://lists.archlinux.org/pipermail/pacman-dev/2018-July/022723.html
> .
> 

Re: [pacman-dev] [PATCH] during -Qu add [ignored] for repos without Usage = Upgrade

[Morgan Adamiec]

Yeah -Qu is still a little funky.

> It gets better...   packages in databases with Usage = Upgrade do not
> show up in -Qu.  That is clearly wrong!

I left the requirement on Usage = Search after every one disagreed with
my patch [1] to change it. Although I would prefer it if Search had no
bearing on this.

So with this patch, Search is needed for a package to be listed and
Upgrade is needed for [ignore] to not be added.

> My plan is to change alpm_sync_newversion to take an additional
> parameter to flag whether it should include all databases or just those
> available for upgrades.  It could even be make more flexible and just
> take the bitmask as a parameter for filtering.

Why not just remove the Usage check all together? You need
alpm_sync_newversion to find the packages then later we do the Usage =
Upgrade check to add [ignored] to the end of the line.

Unless you want the packages to not show up at all? That's exactly what
the previous patch [1] did.

[1] https://lists.archlinux.org/pipermail/pacman-dev/2018-July/022723.html

Re: [pacman-dev] [PATCH] during -Qu add [ignored] for repos without Usage = Upgrade

[Allan McRae]

On 1/9/18 9:35 am, morganamilo wrote:
> Fixes FS#59854
> 
> Signed-off-by: morganamilo 
> ---

You patch is fine...  But in testing it, I once again come to see how
screwy -Qu is!  We need to go back to your original issue.


-Qu ignores databases with Usage = Sync or Install when calculating the
list of possible package updates, but not databases with Usage = Search.

It gets better...   packages in databases with Usage = Upgrade do not
show up in -Qu.  That is clearly wrong!


What is special about Usage = Search?  I can't see why these packages
should be included in a -Qu output and not those from a database with
Usage = Sync/Install/Upgrade.


Lets look at the relevant function:

/** Check for new version of pkg in sync repos
 * (only the first occurrence is considered in sync)
 */
alpm_pkg_t SYMEXPORT *alpm_sync_newversion(alpm_pkg_t *pkg, alpm_list_t
*dbs_sync)
{

We need to define the sync repos that are searched.  We either look at
all sync repos, or those with Usage = Upgrade/All.  I can't see a
justification for anything else.


Going to "man pacman" for -Qu

-u, --upgrades
   Restrict or filter output to packages that are out-of-date on the
   local system. Only package versions are used to find outdated
   packages; replacements are not checked here. This option works best
   if the sync database is refreshed using -Sy.

The option name implies we should only print those with Usage =
Upgrade/All, but the description suggests we can print all of them.  I'm
inclined to go with the option name, as that is less mutable than the
description.


My plan is to change alpm_sync_newversion to take an additional
parameter to flag whether it should include all databases or just those
available for upgrades.  It could even be make more flexible and just
take the bitmask as a parameter for filtering.

Allan

Re: [pacman-dev] [PATCH v3 3/4] makepkg: add support for the zst format

[Allan McRae]

On 4/9/18 6:14 pm, Eli Schwartz wrote:
> On 12/7/17 12:51 AM, Allan McRae wrote:
>> On 31/10/17 04:15, Eli Schwartz wrote:
>>> Signed-off-by: Eli Schwartz 
>>> ---
>>
>> This and the repo-add patch look fine, but will not be pulled until
>> libarchive supporting these formats is released.
> 
> libarchive 3.3.3 is now released and in [testing] as of 45 minutes ago
> so this is good to go!
> 

rebased and applied.

A

Re: [pacman-dev] [PATCH 2/4] Create a convenience library for reused functionality

[Allan McRae]

On 6/7/18 12:42 am, Dave Reisner wrote:
> This is shared between pacman and pacman-conf (and might be used by
> other binaries in the future) -- no need to compile it once for each
> consumer.
> ---

I don't understand this patch...

(before)
Making all in src/pacman
  CC   check.o
  CC   conf.o
  CC   database.o
  CC   deptest.o
  CC   files.o
  CC   ini.o
  CC   package.o
  CC   pacman.o
  CC   query.o
  CC   remove.o
  CC   sighandler.o
  CC   sync.o
  CC   callback.o
  CC   upgrade.o
  CC   util.o
  CC   util-common.o
  CCLD pacman
  CC   pacman-conf.o
  CCLD pacman-conf

(after)
Making all in src/pacman
  CC   conf.lo
  CC   ini.lo
  CC   callback.lo
  CC   util.lo
  CC   util-common.lo
  CCLD libbasic.la
  CC   check.o
  CC   database.o
  CC   deptest.o
  CC   files.o
  CC   package.o
  CC   pacman.o
  CC   query.o
  CC   remove.o
  CC   sighandler.o
  CC   sync.o
  CC   upgrade.o
  CCLD pacman
  CC   pacman-conf.o
  CCLD pacman-conf

Nothing is compiled twice there.  ini.c and util-common.c are still
compiled twice - once in lib/libalpm and once in src/pacman.

How is the library helping?  Some gain in linking time?

A

Re: [pacman-dev] [PATCH 1/2] Change if-else chain to switch

[Allan McRae]

On 19/9/18 12:28 pm, David Phillips wrote:
> ---
>  lib/libalpm/sync.c | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 

Both patches are fine.   Note that case statements should be indented
one further than the switch statement.  I have handed the adjustment.

A



> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index 696a5131..699fb2fd 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -1176,17 +1176,23 @@ static int check_validity(alpm_handle_t *handle,
>   if(errors) {
>   for(i = errors; i; i = i->next) {
>   struct validity *v = i->data;
> - if(v->error == ALPM_ERR_PKG_MISSING_SIG) {
> + switch(v->error) {
> + case ALPM_ERR_PKG_MISSING_SIG:
>   _alpm_log(handle, ALPM_LOG_ERROR,
>   _("%s: missing required 
> signature\n"), v->pkg->name);
> - } else if(v->error == ALPM_ERR_PKG_INVALID_SIG) {
> + break;
> + case ALPM_ERR_PKG_INVALID_SIG:
>   _alpm_process_siglist(handle, v->pkg->name, 
> v->siglist,
>   v->siglevel & 
> ALPM_SIG_PACKAGE_OPTIONAL,
>   v->siglevel & 
> ALPM_SIG_PACKAGE_MARGINAL_OK,
>   v->siglevel & 
> ALPM_SIG_PACKAGE_UNKNOWN_OK);
> + /* fallthrough */
> + case ALPM_ERR_PKG_INVALID_CHECKSUM:
>   prompt_to_delete(handle, v->path, v->error);
> - } else if(v->error == ALPM_ERR_PKG_INVALID_CHECKSUM) {
> - prompt_to_delete(handle, v->path, v->error);
> + break;
> + default:
> + /* ignore */
> + break;
>   }
>   alpm_siglist_cleanup(v->siglist);
>   free(v->siglist);
> 

Re: [pacman-dev] [PATCH] makepkg: lint_pkgver: Run even if PKGVERFUNC

[Allan McRae]

On 10/8/18 3:42 am, Luke Shumaker wrote:
> From: Luke Shumaker 
> 
> lint_pkgver returns 0 if PKGVERFUNC, since it's likely that update_pkgver()
> will change the value of pkgver anyway, and there's no point in linting the
> old value.  update_pkgver() will call check_pkgver() itself to validate the
> new value.
> 
> However, that "optimization" only holds if we're definitely going to call
> update_pkgver() later; and that's way more complicated than
> 
> if (( PKGVERFUNC )); then
> 
> it's more like:
> 
> if (( !GENINTEG && !PACKAGELIST && !PRINTSRCINFO && !SOURCEONLY && !REPKG 
> && PKGVERFUNC )); then
> 
> Which is to say: If I have a PKGBUILD with pkgver():
> 
>  * if I run `makepkg -g` I expect it to lint pkgver, but it won't
>  * if I run `makepkg -R` I expect it to lint pkgver, but it won't
>  * ...
> 
> So let's fix that.
> 
> Rather than try to keep a huge list of conditions in sync with the flow of
> makepkg.sh.in, let's just drop it.  As far as I can tell, the only thing
> that skipping lint_pkgver() really enables is letting the PKGBUILD author
> write `pkgver=` in the initial version, and letting pkgver() fill it in.
> They can just start writing `pkgver=0` for that workflow.
> ---

Massive commit message for a -5 patch!  You have me convinced.

Also, it is great to see an @parabola email address contributing.


>  scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in 
> b/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
> index 65216b64..e9b1566f 100644
> --- a/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
> +++ b/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
> @@ -44,10 +44,5 @@ check_pkgver() {
>  }
>  
>  lint_pkgver() {
> - if (( PKGVERFUNC )); then
> - # defer check to after getting version from pkgver function
> - return 0
> - fi
> -
>   check_pkgver "$pkgver"
>  }
> 

Re: [pacman-dev] [PATCH] during -Qu add [ignored] for repos without Usage = Upgrade

[Allan McRae]

On 1/9/18 9:35 am, morganamilo wrote:
> Fixes FS#59854
> 
> Signed-off-by: morganamilo 
> ---

Looks good.

A

>  src/pacman/query.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/pacman/query.c b/src/pacman/query.c
> index 00c39638..faa06e60 100644
> --- a/src/pacman/query.c
> +++ b/src/pacman/query.c
> @@ -325,10 +325,14 @@ static int display(alpm_pkg_t *pkg)
>   colstr->version, 
> alpm_pkg_get_version(pkg), colstr->nocolor);
>  
>   if(config->op_q_upgrade) {
> + int usage;
>   alpm_pkg_t *newpkg = alpm_sync_newversion(pkg, 
> alpm_get_syncdbs(config->handle));
> + alpm_db_t *db = alpm_pkg_get_db(newpkg);
> + alpm_db_get_usage(db, );
> +
>   printf(" -> %s%s%s", colstr->version, 
> alpm_pkg_get_version(newpkg), colstr->nocolor);
>  
> - if(alpm_pkg_should_ignore(config->handle, pkg)) 
> {
> + if(alpm_pkg_should_ignore(config->handle, pkg) 
> || !(usage & ALPM_DB_USAGE_UPGRADE)) {
>   printf(" %s", _("[ignored]"));
>   }
>   }
>