Re: [pacman-dev] [PATCH v2] lock file may be left behind when a signal arrives while in cleanup

[Andrew Gregory]

On 12/02/20 at 08:02pm, lilydjwg wrote:
> Hi there,
> 
> Is there anything remaining I could do to get this merged? Yesterday we
> had a dangling db.lck again when a member lost his connection to the
> server while running devtools.
> 
> On Wed, Aug 12, 2020 at 11:17:49PM +0800, lilydjwg wrote:
> > When a SIGINT or SIGHUP arrives before alpm_unlock (in alpm_release) but
> > after remove_soft_interrupt_handler, the lock file will be left behind.
> > 
> > Signed-off-by: lilydjwg 
> > ---
> > 
> > Oops, here is an updated version that calls _alpm_handle_unlock
> > before remove_soft_interrupt_handler. It should be fine to call
> > _alpm_handle_unlock twice.
> > 
> >  src/pacman/pacman.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
> > index fefd3fa4..8928ccc4 100644
> > --- a/src/pacman/pacman.c
> > +++ b/src/pacman/pacman.c
> > @@ -287,6 +287,9 @@ static void setuseragent(void)
> >   */
> >  static void cleanup(int ret)
> >  {
> > +   if(config) {
> > +   _alpm_handle_unlock(config->handle);
> > +   }
> > remove_soft_interrupt_handler();
> > if(config) {
> > /* free alpm library resources */

_alpm_handle_unlock is a private function not exposed to front-ends.

Re: [pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values

[Andrew Gregory]

On 10/28/20 at 01:17am, Morgan Adamiec wrote:
> 
> 
> On 28/10/2020 01:13, Eli Schwartz wrote:
> > On 10/27/20 8:59 PM, Morgan Adamiec wrote:
> >> On 28/10/2020 00:46, Eli Schwartz wrote:
> >>> It remains unclear to me, why the ambiguity matters.
> >>>
> >>
> >> I guess if you were to officially declare `Key =` means to set the key
> >> to none then that would solve the issue of it being ambiguous
> >>
> >> Then again that would make `license=('a' '' 'b')` parse as only as
> >> licence=(b) seems as it was cleared before.
> >>
> >> But funnily enough, if you build the package then pacman -Qi reports
> >> only license a. Fun!
> >
> > I just checked this, but -Qip reported all the licenses, both before and
> > after the '' license. Is this an observation about the current state of
> > pacman, or about what the proposed semantics would be?
> >
> 
> So as it turns out -Qip shows the licenses. Installing the package and
> doing -Qi only shows a.

The joys of having two different package info file formats.  Whatever
it does with .SRCINFO, an empty string is not a valid value in the db
package info file, so makepkg should not be writing .PKGINFO files
with them.

Re: [pacman-dev] [PATCH] repo-add: add --include-sigs option

[Andrew Gregory]

On 09/21/20 at 03:19pm, Allan McRae wrote:
> On 4/9/20 12:55 pm, Allan McRae wrote:
> > On 4/9/20 12:40 pm, Eli Schwartz wrote:
> >> On 9/2/20 11:02 PM, Allan McRae wrote:
> >>> Pacman now downloads the signature files for all packages when present in 
> >>> a
> >>> repository.  That makes distributing signatures within repository 
> >>> databases
> >>> redundant and costly.
> >>>
> >>> Do not distribute the package signature files within the repo databases by
> >>> default and add an --include-sigs to revert to the old behaviour.
> >>
> >> As I've mentioned on the list before, I would like an --ignore-sigs
> >> option and continue to distribute sigs by default for pacman 6.0
> >>
> >> In pacman 6.1 we'll switch by default to ignoring them, and let people
> >> use --include-sigs to revert to the old behavior.
> >>
> >> Ignoring sigs right out of the gate means the default behavior of
> >> repo-add is to be unusable for people upgrading from pacman N-1. For
> >> example, Arch Linux would most certainly need to use the option to
> >> provide backwards compat while upgrading. So do third-party repositories.
> >>
> >> Also: this option cannot be added to scripts ahead of time, since
> >> repo-add will error on an unknown option, and it cannot be added after
> >> the fact, since some packages will be broken in the meantime.
> >>
> >> I don't see what the rush is here to add behavior that no one will want
> >> to use.
> >> - It makes sense to make this configurable now that it's useful to be
> >>   able to ignore them.
> >> - At the same time, defaults should be based on what is more likely for
> >>   people to want.
> >>
> > 
> > I really do not like the idea of adding an option, just to remove it in
> > the next release.   But we won't actually be able to remove it for at
> > least two releases, as you have just made the case that people won't be
> > able to change their scripts on release.
> > 
> > Given pacman-6.0 is likely a few months out,  can we do a 5.2.3 release
> > including something like:
> > 
> 
> Any feedback on this option?

I would suggest just allowing the user to specify either way
(--include-sigs/--no-include-sigs, --include-sigs={yes,no}, etc).
Then uses can specify whatever they want without having to worry about
what we set as a default.

> > 
> > diff --git a/doc/repo-add.8.asciidoc b/doc/repo-add.8.asciidoc
> > index 8de4485b..19e2336a 100644
> > --- a/doc/repo-add.8.asciidoc
> > +++ b/doc/repo-add.8.asciidoc
> > @@ -70,6 +70,10 @@ repo-add Options
> > Remove old package files from the disk when updating their entry in the
> > database.
> > 
> > +*\--include-sigs*::
> > +   Dummy option for forward compatibility with pacman-6.0.
> > +   Include package PGP signatures in the repository database (if available)
> > +
> > 
> >  Example
> >  ---
> > diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in
> > index b0b3505d..ee010dba 100644
> > --- a/scripts/repo-add.sh.in
> > +++ b/scripts/repo-add.sh.in
> > @@ -43,6 +43,7 @@ LOCKFILE=
> >  CLEAN_LOCK=0
> >  USE_COLOR='y'
> >  PREVENT_DOWNGRADE=0
> > +INCLUDE_SIGS=0
> > 
> >  # Import libmakepkg
> >  source "$LIBRARY"/util/message.sh
> > @@ -631,6 +632,9 @@ while (( $# )); do
> > -p|--prevent-downgrade)
> > PREVENT_DOWNGRADE=1
> > ;;
> > +   --include-sigs)
> > +   INCLUDE_SIGS=1
> > +   ;;
> > *)
> > args+=("$1")
> > ;;
> > .
> > 

Re: [pacman-dev] [PATCH] util.c: table_print_line: properly align texts involving CJK

[Andrew Gregory]

On 09/12/20 at 04:25pm, Chih-Hsuan Yen wrote:
> Fixes FS#59229
> 
> Signed-off-by: Chih-Hsuan Yen 
> ---
>  src/pacman/util.c | 20 +++-
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index e9187529..b45ca22d 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -516,22 +516,32 @@ static void table_print_line(const alpm_list_t *line, 
> short col_padding,
>   i++, curcell = alpm_list_next(curcell)) {
>   const struct table_cell_t *cell = curcell->data;
>   const char *str = (cell->label ? cell->label : "");
> - int cell_width;
> + int padding_width;
>  
>   if(!has_data[i]) {
>   continue;
>   }
>  
> - cell_width = (cell->mode & CELL_RIGHT_ALIGN ? (int)widths[i] : 
> -(int)widths[i]);
> -
>   if(need_padding) {
>   printf("%*s", col_padding, "");
>   }
>  
>   if(cell->mode & CELL_TITLE) {
> - printf("%s%*s%s", config->colstr.title, cell_width, 
> str, config->colstr.nocolor);
> + printf("%s", config->colstr.title);
> + }
> + /* Handle alignment manually here - for printf in C, "If the
> +  * precision is specified, no more than that many bytes are
> +  * written." [1]
> +  * [1] Section 7.21.6, N2176, final draft for ISO/IEC 9899:2017 
> (C18)
> +  * Fixes FS#59229. */

I appreciate a reference to a spec, but this is a bit verbose for
a source comment.  This information is better suited to the commit
message with a shorter comment in the source about printf width being
in bytes instead of characters.

I think this solution is more complex than necessary though.  It
should be sufficient to just adjust the above cell_width to be in
bytes rather than characters.  Something like:

/* calculate cell width adjusting for multi-byte character strings */
cell_width = (int)widths[i] + strlen(str) - string_length(str);
cell_width = cell->mode & CELL_RIGHT_ALIGN ? cell_width : 
-cell_width;

> + padding_width = (int)(widths[i] - string_length(str));
> + if (cell->mode & CELL_RIGHT_ALIGN) {
> + printf("%*s%s", padding_width, "", str);
>   } else {
> - printf("%*s", cell_width, str);
> + printf("%s%*s", str, padding_width, "");
> + }
> + if(cell->mode & CELL_TITLE) {
> + printf("%s", config->colstr.nocolor);
>   }
>   need_padding = 1;
>   }
> -- 
> 2.28.0

Re: [pacman-dev] [PATCH v4] libalpm: Add support for trigger dropins

[Andrew Gregory]

On 09/04/20 at 12:22pm, Allan McRae wrote:
...
> I'm trying to figure out how includes would address this problem.
> 
> We have a hook that runs when its triggers are met.  We want to have a
> package add additional triggers, by dropping in a file into our config,
> but do not want to duplicate the function of the hook, or to directly
> edit the hook.  An Include would provide a place where we can drop more
> config files with (e.g.) more triggers, but that requires the original
> hook to be set up such that it is extendable.  Just adding additional
> triggers does not require the original hook to do anything.

Requiring the hook to opt-in to this behavior is the point.  Once you
make a hook extensible, making any changes to that hook become much
more difficult because those changes could render any extensions
useless, and the hook author doesn't have any way to know if anything
else is depending on the current implementation.  Requiring the hook
to opt-in gives the author a way to indicate that the hook is suitable
for extension and won't change in a way that breaks those extensions
in the future.  Essentially we're allowing a hook to choose to provide
a public interface instead of automatically doing it for all hooks.
And, of course, the fact that we already have code to deal with
Includes is a nice plus.

Re: [pacman-dev] [PATCH] Add pacman-conf zsh completions

[Andrew Gregory]

On 08/21/20 at 08:20pm, Ronan Pigott wrote:
> From: Ronan Pigott 
> 
> Signed-off-by: Ronan Pigott 

Sure, I guess.  pacman-conf is meant for use in scripts; who on Earth
is running it in a terminal?

Re: [pacman-dev] [PATCH 0/1] Introduce "AssumeInstalled" option in the config file

[Andrew Gregory]

On 09/03/20 at 12:17pm, Damjan Georgievski wrote:
> On Sun, 30 Aug 2020 at 19:13, Eli Schwartz  wrote:
> >
> > On 8/30/20 1:07 PM, Дамјан Георгиевски wrote:
> > > I have had at least two or three use cases for this option,
> > > * one has been since I always have to type
> > >   pacman -Syu --assume-installed noto-fonts when plasma-integration is 
> > > updated
> > > * `pacman -Syu --assume-installed perl` since it's needlesly pulled by 
> > > openssl
> > >   and I didn't need it for containers and similar
> > >
> > > Having this in the config file allows to use the option together with 
> > > pacstrap too.
> > >
> > > Please review and if the change is acceptable suggest what else needs to 
> > > be added
> >
> > I'm not sure it's a good idea to encourage this in the general case.
> > Supporting it only as a command-line option is still useful since it can
> > help resolve certain types of manual itervention.
> 
> maybe I should remove it from etc/pacman.conf.in so in essence it can
> be a hidden option?
> 
> I still think that CLI options should be able to be specified in a
> config file (or possibly env vars).

Without a better use case, I'm -1 on this.  --assume-installed was
intended for ignoring individual dependencies in exceptional
circumstances, not permanently ignore a dependency you don't like.
The preferred way of dealing with that, should you feel the need, is
with a meta package.

Re: [pacman-dev] [PATCH] NO_MSGSIGNAL and SIGPOLL don't exist on darwin

[Andrew Gregory]

On 09/01/20 at 03:50pm, Cameron Katri wrote:
> I believe I found a more portable solution. We use signal(SIGPIPE,
> SIG_IGN); which will block SIGPIPE, and if MSG_NOSIGNAL is not
> supported we set it to 0x0 so that it just gets ignored. Hopefully
> this is a more comprehensive solution.  
...
>  
> + signal(SIGPIPE, SIG_IGN);

NACK. Signal handlers are process-wide and not something libraries
should be modifying without very good reason, especially not
conditionally and mid-operation.  The fact that gpgme ignores SIGPIPE
is what broke scripts and required us to start resetting signals.  The
whole point of using sockets was to selectively ignore SIGPIPE without
modifying the signal handler.

>   nwrite = send(fd, buf, *buf_size, MSG_NOSIGNAL);
>  
>   if(nwrite != -1) {
> @@ -557,8 +564,11 @@ static void _alpm_reset_signals(void)
>   int *i, signals[] = {
>   SIGABRT, SIGALRM, SIGBUS, SIGCHLD, SIGCONT, SIGFPE, SIGHUP, 
> SIGILL,
>   SIGINT, SIGKILL, SIGPIPE, SIGQUIT, SIGSEGV, SIGSTOP, SIGTERM, 
> SIGTSTP,
> - SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPOLL, SIGPROF, SIGSYS, 
> SIGTRAP,
> + SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPROF, SIGSYS, SIGTRAP,
>   SIGURG, SIGVTALRM, SIGXCPU, SIGXFSZ,
> +#ifndef __APPLE__

Signal names are macros, test for SIGPOLL directly.

> + SIGPOLL,
> +#endif

Re: [pacman-dev] [PATCH v4] libalpm: Add support for trigger dropins

[Andrew Gregory]

On 08/24/20 at 10:34pm, Daan De Meyer wrote:
> > What about adding Include support to hooks?  Then hooks that need this
> > type of functionality could explicitly include trigger files from
> > a particular directory, insulating the process from simple hook
> > renaming and hopefully making it more obvious when changes to the hook
> > will require changes to any package that modifies it.
> 
> Just to make sure I understand correctly, does Include support imply
> that hooks opt in to reading triggers from a specific directory and
> that we can rename the hook itself without changing the drop-in
> directory for that hook to avoid breakage? If so, that sounds totally
> reasonable and even preferable since it also avoids packages adding
> triggers to hooks that can't deal with extra targets when NeedsTargets
> is used. I can adapt the patch to use this approach.
> 
> Do we pass a full path to Include or just a directory name? The
> advantage of just a directory name is that it could be created in any
> of the hook directories (hook installed in /usr/share/libalpm/hooks
> and the added triggers in /etc/pacman.d/hooks/ for
> example). It doesn't necessarily have to be just a directory name, we
> could interpret it as a relative path and you could still put it in
> each of the hook directories but that might just be adding too much
> complexity.

Include should take a glob, just like pacman.conf.

Re: [pacman-dev] [PATCH] Disable embedded signatures by default

[Andrew Gregory]

On 08/28/20 at 02:37pm, Allan McRae wrote:
> On 27/8/20 10:26 am, Anatol Pomozov wrote:
> > Hi
> > 
> > On Mon, Aug 10, 2020 at 2:45 PM Eli Schwartz  
> > wrote:
> >> This is the right approach, yeah. I was thinking we'd wait until pacman
> >> 6.1 before stopping the signature embedding, to provide a transition
> >> period for people depending on SigLevel = Required (which should be
> >> everyone, and certainly includes Arch!) to upgrade to 6.x before
> >> repo-add starts generating databases useless to pacman 5.x
> > 
> > There are 2 sets of changes that need to be done:
> > 1) make pacman to use detached signatures instead of embedded ones
> > 2) change "repo-add" to avoid adding embedded signatures
> > 
> > We should release changes for #1 first, test it, make sure that
> > detached signatures fully work (while dbs still have pacman
> > 5.x-compatible embedded sigs). And only then release #2 to get smaller
> > databases compatible with pacman version >= 6.0.
> > 
> > I was thinking #1 can be released with 6.0 and #2 with 6.1.
> 
> I was thinking #2 would be an option to repo-add.  I'm looking at making
> signature embedding only occur with the "--add-signatures" option (or
> whatever I decide to call it).  Arch would need to patch devtools to use
> this option.  They would then make a News announcement about the need to
> have pacman-6.0 installed after 3-6 months and stop repo-add including
> signatures.
> 
> However, I think pacman should always use the signatures in the database
> if they are present.  Particularly if they are not embedded by default.
> 
> So to actually test the detached signature path, I am thinking it best
> to tag 6.0.0beta1, make a package from that tag with a patch to enable
> using detached signatures as a priority.  While that is not an ideal
> approach to testing, I think the current code path is well tested, and
> this should be a reasonably trivial patch.

We should implement FS#33091.  Instead of adding an option to disable
detached signatures, add one to disable embedded signatures.  This
gives anybody that wants to help test a way to do so without forcing
it on people and provides a useful feature for any repos that continue
providing embedded signatures.  I don't even know that we'd need
a beta release because the new behavior would be opt-in and could be
disabled at any time.

Re: [pacman-dev] [PATCH v4] libalpm: Add support for trigger dropins

[Andrew Gregory]

On 08/24/20 at 11:22am, Allan McRae wrote:
> Lets take a step back here...
> 
> I don't really care about the kernel use case, but more whether this
> could be more generally used.   Here are other examples I came up with.
> 
> Font caches:
> A package could drop a config file in /etc/font/conf.d/ to add another
> directory that is to be used when building font caches.  They would also
> want to add a .trigger file for the hook.
> 
> OK...  I just came up with one other example!  But I did not look too
> hard at what hooks on my system actually do.
> 
> 
> Back to the question of whether we support this.  A quick skim of the
> patch shows me it is quite long, but relatively simple.  (it seems to be
> doing more than just adding .trigger support - also adding drop in
> directories - and this seems two patches to me...)
> 
> So, I am leaning in the direction of this being a useful addition.
> 
> Further discussion of the usefulness should not focus around the kernel
> approach, but rather the general idea of the feature.

I'm still a bit skeptical.  I can't say I'm particularly familiar with
fonts and their configuration, but is there really a valid use case
for that kind of per-package configuration that we need to support?

More abstractly, I'm not fond of how much this ties packages together.
One of the ideas of hooks was for the package that does the work to
handle all of the necessary details so the triggering packages
wouldn't have to worry about it.  With this approach, they not only
have to know to adjust the hook, they also have to know the name of
the hook and how the hook works.  Changes to the hook then require an
adjustment and rebuild of any package modifying it.  This is not an
entirely new problem, user overrides are also dependant on hook
naming, but this does exacerbate it in a way that I think will be easy
for packagers to miss and cause breakage.

What about adding Include support to hooks?  Then hooks that need this
type of functionality could explicitly include trigger files from
a particular directory, insulating the process from simple hook
renaming and hopefully making it more obvious when changes to the hook
will require changes to any package that modifies it.

Re: [pacman-dev] [PATCH] lock file may be left behind when a signal arrives while in cleanup

[Andrew Gregory]

On 08/12/20 at 01:36pm, lilydjwg wrote:
> When a SIGINT or SIGHUP arrives before alpm_unlock (in alpm_release) but
> after remove_soft_interrupt_handler, the lock file will be left behind.
> 
> Signed-off-by: lilydjwg 
> ---
>  src/pacman/pacman.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
> index fefd3fa4..2d2e41d2 100644
> --- a/src/pacman/pacman.c
> +++ b/src/pacman/pacman.c
> @@ -287,7 +287,6 @@ static void setuseragent(void)
>   */
>  static void cleanup(int ret)
>  {
> - remove_soft_interrupt_handler();
>   if(config) {
>   /* free alpm library resources */
>   if(config->handle && alpm_release(config->handle) == -1) {
> @@ -297,6 +296,7 @@ static void cleanup(int ret)
>   config_free(config);
>   config = NULL;
>   }

The signal handler uses config->handle; this will segfault if a signal
arrives between alpm_release and here.

> + remove_soft_interrupt_handler();
>  
>   /* free memory */
>   FREELIST(pm_targets);
> -- 
> 2.28.0

[pacman-dev] [GIT] The official pacman repository annotated tag, v5.2.2, created. v5.2.2

[Andrew Gregory]

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 annotated tag, v5.2.2 has been created
at  c95ee2ab01a828cb55232ddffd25570610c1af39 (tag)
   tagging  5537881b2525a4f114fcf10b00413b4575a74968 (commit)
  replaces  v5.2.1
 tagged by  Andrew Gregory
on  Thu Jun 25 23:46:11 2020 -0700

- Log -
v5.2.2
-BEGIN PGP SIGNATURE-

iQEzBAABCAAdFiEEuBUbEXA3eBCVUUynu9/8kjBrESEFAl71mbMACgkQu9/8kjBr
ESFuEAgAo7SFqphsUbw0KABZb/ofD8n8qZtzqjP5WdbJmqz60HgiCHtHtnhWEj/p
2mBxOuoJexfZadi4DUBUmMmy8Hk+k2/RFneD182ARrGqvOz50TDgh+NGGvG29ZWU
+0CiuT+/gbHQhiq4+wIFj7fQR2l/tXC0fDULat2cVRceNHqWoCDhiF4RdGIl2aF8
eLrrD8YuzgpX1aLXo+qt0YYwSipDK5qVmDejo4s89eCBlG3Grqcre9s0SS43rTwu
djRHmy3FjnXgAx6sRDwftw+9DThst0JcnNo1xzYifWfBoAzhxfMYgnpUDPfBlxzv
2nY8SJpCDh7GfBTcNrFzcDVPTgczDw==
=is9q
-END PGP SIGNATURE-

Allan McRae (11):
  pactest: set package tar format to GNU_FORMAT
  libalpm/sync.c: Do not download missing keys multiple times
  Handle .part files that are the size of the correct package
  Remove unneeded ltmain patch
  Increase maximum database size
  Fix "pacman -U " operations
  build-aux/update-copyright 2019 2020
  pacman.8: Fix typo
  libalpm/signing.c: Fix calculation of packet size in parse_subpacket
  Pull translation updates
  pacman-key: change signing key to RSA4096

Andrew Gregory (2):
  update NEWS for 5.2.2
  Release 5.2.2

Daniel T. Borelli (1):
  Dereference double pointer before assigning NULL

Earnestly (1):
  Use noextract with pacman-conf NoExtract

Eli Schwartz (13):
  makepkg: fix one more file-seccomp issue
  makepkg: fix regression that broke extraction of file:// sources
  makepkg: make per-package files containing '$pkgname' consistently work
  pacman-conf: fix incomplete support for ILoveCandy
  Log invalid conf settings as an error
  libmakepkg/strip: don't re-add the same debug source multiple times
  makepkg: guard against undefined git pinned sources
  makepkg: correctly handle missing download clients
  libmakepkg: fix regression in sending plain() output to stderr
  makepkg/repo-add: handle GPGKEY with spaces
  makepkg/repo-add: do not accept public-only keys for signing
  doc/pacman.8: fix typo
  autotools: emit error message when autoconf-archive is missing

Levente Polyak (1):
  makepkg: deterministic PKGINFO libprovides for multiple library versions

morganamilo (1):
  libalpm: fix alpm_option_set_assumeinstalled

---


hooks/post-receive
-- 
The official pacman repository

[pacman-dev] [GIT] The official pacman repository branch, release/5.2.x, updated. v5.2.2

[Andrew Gregory]

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, release/5.2.x has been updated
   via  5537881b2525a4f114fcf10b00413b4575a74968 (commit)
   via  027c87ae3f7f4728b4058adb23baf7612cc5ea77 (commit)
   via  b2c97ad76290eab37afd3873d011745d43059585 (commit)
   via  c834a75718949103a0d3b71f6de8bb6dbabfd74a (commit)
   via  82d4b98ddce0165a0d1ec5474d448b2a93167b4e (commit)
  from  50f5e484f278d3550da71246bfb51d3d8053bae8 (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 5537881b2525a4f114fcf10b00413b4575a74968
Author: Andrew Gregory 
Date:   Mon Jun 22 02:15:23 2020 -0700

Release 5.2.2

Signed-off-by: Andrew Gregory 

commit 027c87ae3f7f4728b4058adb23baf7612cc5ea77
Author: Andrew Gregory 
Date:   Mon Jun 22 02:12:41 2020 -0700

update NEWS for 5.2.2

Signed-off-by: Andrew Gregory 

commit b2c97ad76290eab37afd3873d011745d43059585
Author: Eli Schwartz 
Date:   Sun Jan 5 16:04:21 2020 -0500

autotools: emit error message when autoconf-archive is missing

Forbid the AX_COMPARE_VERSION macro from being found in the output
configure script. If autoconf-archive is not installed when autoreconf
is run, the following error message is emitted:

configure.ac:231: error: possibly undefined macro: AX_COMPARE_VERSION
  If this token and others are legitimate, please use m4_pattern_allow.
  See the Autoconf documentation.
autoreconf: /usr/bin/autoconf failed with exit status: 1

Signed-off-by: Eli Schwartz 
Signed-off-by: Allan McRae 
(cherry picked from commit 435f5fc20484d3728d8877eea15c7a8653da6d10)

commit c834a75718949103a0d3b71f6de8bb6dbabfd74a
Author: Allan McRae 
Date:   Mon Jun 15 19:17:41 2020 +1000

pacman-key: change signing key to RSA4096

RSA2048 may have been fine when this was written many moons ago, but time
this has a bump.

Signed-off-by: Allan McRae 
(cherry picked from commit 7ba8e5f376528fcef9f7613304d8f18b2f664ee8)

commit 82d4b98ddce0165a0d1ec5474d448b2a93167b4e
Author: Eli Schwartz 
Date:   Sun Jun 14 14:52:02 2020 -0400

doc/pacman.8: fix typo

Fixes FS#67000

Signed-off-by: Eli Schwartz 
Signed-off-by: Allan McRae 
(cherry picked from commit 59e751f72d09390067045168ac45a09a89419389)

---

Summary of changes:
 NEWS | 30 ++
 configure.ac |  5 +++--
 doc/index.asciidoc   |  1 +
 doc/pacman.8.asciidoc|  2 +-
 meson.build  |  4 ++--
 scripts/pacman-key.sh.in |  2 +-
 6 files changed, 38 insertions(+), 6 deletions(-)


hooks/post-receive
-- 
The official pacman repository

[pacman-dev] [GIT] The official pacman repository branch, release/5.2.x, updated. v5.2.1-24-g1e9cd30e

[Andrew Gregory]

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, release/5.2.x has been updated
   via  1e9cd30e488cd133d24eac7ed9cac7806db2c406 (commit)
   via  2a345604cd8bd6a8fe5041ea7ec046c8d20aa30b (commit)
   via  f4da297de252dcb00e6c83df00fea11c86606348 (commit)
   via  22e6daa79455b1c59ab73af9438ca0c1d2a884b9 (commit)
   via  d69da08abef42a52d2fa81daf4c045c3305598cf (commit)
   via  12503767c0be8f3eb28433fd58bcab0ec6f44a26 (commit)
   via  03cfe9e21c6aaecaac21e932229a55f5f1041a3c (commit)
   via  b3be0ce99b29e5384969c7b3e5ff6f4b0f581c4b (commit)
   via  68418c54427b2ecbd5f882ffad261e0ba6e67c42 (commit)
   via  bb50e8d73afda7f1c464bd7bfa127a749b8d (commit)
   via  b9d397c7317b9b495da73051fc670b0e9f472a06 (commit)
   via  39ce2b766303f046bb75ad900cc8a2695e947f83 (commit)
   via  01d5a68c1ae07ad43b5255721d2429710794ea04 (commit)
   via  08aa3d97ea3823f2919e7d66041438db9e7488a0 (commit)
   via  0bf4779cda35cfb8dc049ec5f27017c65c8ce716 (commit)
   via  d61c398b2cb3ec41e62a6e258b5791d0ce9de720 (commit)
   via  7faa79526874c09fc89fb769744d103bc4918e17 (commit)
   via  0d0a4bd68099596ba17b8c314664484c3819fbf3 (commit)
   via  76c50e34393d68a740f825a6af9df8b7ac677303 (commit)
   via  c2fa9f854b576816330762fb55167a1ba78c (commit)
   via  6f1a9e6ea85a49b8aeb3b7bd2454215e9c34f565 (commit)
   via  41c3b1d78c3807beb4246624b20a31e374fb5af1 (commit)
   via  d6dcc936457815cdda87a8507cf82169b437e4d2 (commit)
   via  019f9386ef2c85b4c3c9f9293f462cfb5ddcaa32 (commit)
  from  f6564377a2b0a0dd6294fb366a3f91a31142e124 (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 1e9cd30e488cd133d24eac7ed9cac7806db2c406
Author: Eli Schwartz 
Date:   Mon Jun 8 22:03:18 2020 -0400

makepkg/repo-add: do not accept public-only keys for signing

If it's not listed by --list-secret-key we don't care if it has been
imported into your keyring, it's unusable. And you might not have a
private key at all in the no-keyid-specified case.

Signed-off-by: Eli Schwartz 
Signed-off-by: Allan McRae 
(cherry picked from commit 02ae97b0da220d9079c6c2c1ac3e3ab0d12c1ac7)

commit 2a345604cd8bd6a8fe5041ea7ec046c8d20aa30b
Author: Eli Schwartz 
Date:   Mon Jun 8 21:59:18 2020 -0400

makepkg/repo-add: handle GPGKEY with spaces

We pass this to gpg -u and this gpg option can accept a number of
different formats, not just the historical hexadecimal fingerprint we
assumed. We should not barf hard if a format is used which happens to
contain spaces.

This also fixes a validation bug. When we initially check if the desired
key is available, we don't quote spaces, so gpg goes ahead and treats
each space-separated string as a *different key* to search for,
returning partial matches, and returning success if at least one key is
found. But gpg --detach-sign -u will certainly not accept multiple keys!

Fixes FS#66949

Signed-off-by: Eli Schwartz 
Signed-off-by: Allan McRae 
(cherry picked from commit 899d39b635d46f9e2daff1aada75ea07f08fef64)

commit f4da297de252dcb00e6c83df00fea11c86606348
Author: Eli Schwartz 
Date:   Tue Jun 2 17:50:24 2020 -0400

libmakepkg: fix regression in sending plain() output to stderr

In commit 882e707e40bbade0111cf3bdedbdac4d4b70453b we changed message
output to go to stdout by default, unless it was an error. The plain()
function doesn't *look* like an error function, but in practice it was
-- it's used to continue multiline messages, and all in-tree uses were
for warning/error.

This is a problem both because we're sending output to the wrong place,
and because in some cases, we were performing error logging from a
function which would otherwise return a value to be captured in a
variable using command substution.

Fix this and straighten out the API by providing two functions: one for
continuing msg output, and one which wraps this by sending output to
stderr, for continuing error output.

Change all callers to use the second function.

(cherry picked from commit bf458cced7c0845f7b6fabb887d3878ae4cd51b2)

commit 22e6daa79455b1c59ab73af9438ca0c1d2a884b9
Author: Eli Schwartz 
Date:   Tue Jun 2 18:16:48 2020 -0400

makepkg: correctly handle missing download clients

This was broken in commit 882e707e40bbade0111cf3bdedbdac4d4b70453b,
which changed 'plain()' messages to go to stdout, which was then
captured as the download client in question: cmdline=("Aborting...").

The result was a very 

Re: [pacman-dev] [PATCH v3] Fallback to detached signatures during keyring check

[Andrew Gregory]

On 06/01/20 at 11:45pm, Anatol Pomozov wrote:
> Pacman has a 'key in keyring' verification step that makes sure the signatures
> have a valid keyid. Currently pacman parses embedded package signatures only.
> 
> Add a fallback to detached signatures. If embedded signature is missing then 
> it
> tries to read corresponding *.sig file and get keyid from there.
> 
> Verification:
>   debug: found cached pkg: 
> /var/cache/pacman/pkg/glib-networking-2.64.3-1-x86_64.pkg.tar.zst
>   debug: found detached signature 
> /var/cache/pacman/pkg/glib-networking-2.64.3-1-x86_64.pkg.tar.zst.sig with 
> size 310
>   debug: found signature key: A5E9288C4FA415FA
>   debug: looking up key A5E9288C4FA415FA locally
>   debug: key lookup success, key exists
> 
> Signed-off-by: Anatol Pomozov 
> ---
>  lib/libalpm/alpm.h| 11 +++
>  lib/libalpm/package.c | 34 ++
>  lib/libalpm/sync.c| 18 +-
>  lib/libalpm/util.c| 37 +
>  lib/libalpm/util.h|  1 +
>  5 files changed, 92 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index c6a13273..0ba57109 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -1403,6 +1403,17 @@ alpm_db_t *alpm_pkg_get_db(alpm_pkg_t *pkg);
>   */
>  const char *alpm_pkg_get_base64_sig(alpm_pkg_t *pkg);
>  
> +/** Extracts package signature either from embedded package signature
> + * or if it is absent then reads data from detached signature file.
> + * @param pkg a pointer to package.
> + * @param sig output parameter for signature data. Callee function allocates
> + * a buffer needed for the signature data. Caller is responsible for
> + * freeing this buffer.
> + * @param sig_len output parameter for the signature data length.
> + * @return 0 on success, negative number on error.

alpm_errno_t values are positive.

> + */
> +int alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig, size_t *sig_len);
> +
>  /** Returns the method used to validate a package during install.
>   * @param pkg a pointer to package
>   * @return an enum member giving the validation method
> diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
> index 5c5fa073..1335b70e 100644
> --- a/lib/libalpm/package.c
> +++ b/lib/libalpm/package.c
> @@ -268,6 +268,40 @@ const char SYMEXPORT *alpm_pkg_get_base64_sig(alpm_pkg_t 
> *pkg)
>   return pkg->base64_sig;
>  }
>  
> +int SYMEXPORT alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig, size_t 
> *sig_len)
> +{
> + ASSERT(pkg != NULL, return -1);
> +
> + if(pkg->base64_sig) {
> + return alpm_decode_signature(pkg->base64_sig, sig, sig_len);

You're returning an error without setting pm_errno.

> + } else {
> + char *pkgpath, *sigpath;
> + alpm_errno_t err;
> + int ret = -1;
> +
> + pkgpath = _alpm_filecache_find(pkg->handle, pkg->filename);
> + if(!pkgpath) {
> + RET_ERR(pkg->handle, ALPM_ERR_PKG_NOT_FOUND, -1);
> + }
> + sigpath = _alpm_sigpath(pkg->handle, pkgpath);
> + if(!sigpath || _alpm_access(pkg->handle, NULL, sigpath, R_OK)) {
> + GOTO_ERR(pkg->handle, ALPM_ERR_SIG_MISSING, exit);
> + }
> + err = _alpm_read_file(sigpath, sig, sig_len);
> + if(err == ALPM_ERR_OK) {
> + _alpm_log(pkg->handle, ALPM_LOG_DEBUG, "found detached 
> signature %s with size %ld\n",
> + sigpath, *sig_len);
> + } else {
> + GOTO_ERR(pkg->handle, err, exit);
> + }
> + ret = 0;
> +exit:

Change exit to cleanup; exit makes it look like you are terminating
the program.

> + FREE(pkgpath);
> + FREE(sigpath);
> + return ret;
> + }
> +}
> +
>  const char SYMEXPORT *alpm_pkg_get_arch(alpm_pkg_t *pkg)
>  {
>   ASSERT(pkg != NULL, return NULL);
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index 8c01ad95..9350793a 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -880,18 +880,18 @@ static int check_keyring(alpm_handle_t *handle)
>   }
>  
>   level = alpm_db_get_siglevel(alpm_pkg_get_db(pkg));
> - if((level & ALPM_SIG_PACKAGE) && pkg->base64_sig) {
> - unsigned char *decoded_sigdata = NULL;
> - size_t data_len;
> - int decode_ret = alpm_decode_signature(pkg->base64_sig,
> - _sigdata, _len);
> - if(decode_ret == 0) {
> + if((level & ALPM_SIG_PACKAGE)) {
> + unsigned char *sig = NULL;
> + size_t sig_len;
> + int ret = alpm_pkg_get_sig(pkg, , _len);
> + if(ret == 0) {
>   alpm_list_t *keys = NULL;
> -

Re: [pacman-dev] [PATCH] libalpm/signing.c: Fix calculation of packet size in parse_subpacket

[Andrew Gregory]

On 05/20/20 at 02:22pm, Allan McRae wrote:
> Given RFC 4880 provides the code to do this calculation, I am not sure
> how I managed to stuff that up!  This bug was only exposed when a
> signature made with "include-key-block" was added to the Arch repos,
> which provided a subpacket with the required size to hit this issue.

LGTM. Though, it might be worth it to use + instead of | just so we
match 4880 and extract_keyid exactly.

> Signed-off-by: Allan McRae 
> ---
> 
> Also appropriate for 5.2.2
> 
>  lib/libalpm/signing.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/libalpm/signing.c b/lib/libalpm/signing.c
> index c8eaaca2..422523b6 100644
> --- a/lib/libalpm/signing.c
> +++ b/lib/libalpm/signing.c
> @@ -1058,7 +1058,7 @@ static int parse_subpacket(alpm_handle_t *handle, const 
> char *identifier,
>   if(length_check(len, spos, 2, handle, 
> identifier) != 0){
>   return -1;
>   }
> - slen = (sig[spos] << 8) | sig[spos + 1];
> + slen = (((sig[spos] - 192) << 8) | sig[spos + 
> 1]) + 192;
>   spos = spos + 2;
>   } else {
>   if(length_check(len, spos, 5, handle, 
> identifier) != 0) {
> -- 
> 2.26.2

Re: [pacman-dev] [PATCH v2] Fallback to detached signatures during keyring check

[Andrew Gregory]

On 05/31/20 at 01:37am, Anatol Pomozov wrote:
> Hi Andrew, thank you for the quick response
> 
> On Sat, May 30, 2020 at 9:31 PM Andrew Gregory
>  wrote:
> >
> > On 05/30/20 at 07:51pm, Anatol Pomozov wrote:
> > > Pacman has a 'key in keyring' verification step that makes sure the 
> > > signatures
> > > have a valid keyid. Currently pacman parses embedded package signatures 
> > > only.
> > >
> > > Add a fallback to detached signatures. If embedded signature is missing 
> > > then it
> > > tries to read corresponding *.sig file and get keyid from there.
> > >
> > > Verification:
> > >   debug: found cached pkg: 
> > > /var/cache/pacman/pkg/glib-networking-2.64.3-1-x86_64.pkg.tar.zst
> > >   debug: found detached signature 
> > > /var/cache/pacman/pkg/glib-networking-2.64.3-1-x86_64.pkg.tar.zst.sig 
> > > with size 310
> > >   debug: found signature key: A5E9288C4FA415FA
> > >   debug: looking up key A5E9288C4FA415FA locally
> > >   debug: key lookup success, key exists
> > >
> > > Signed-off-by: Anatol Pomozov 
> > > ---
> > >  lib/libalpm/alpm.h| 10 ++
> > >  lib/libalpm/package.c | 31 +++
> > >  lib/libalpm/sync.c| 17 -
> > >  lib/libalpm/util.c| 35 +++
> > >  lib/libalpm/util.h|  1 +
> > >  5 files changed, 85 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> > > index c6a13273..9c5fff73 100644
> > > --- a/lib/libalpm/alpm.h
> > > +++ b/lib/libalpm/alpm.h
> > > @@ -1403,6 +1403,16 @@ alpm_db_t *alpm_pkg_get_db(alpm_pkg_t *pkg);
> > >   */
> > >  const char *alpm_pkg_get_base64_sig(alpm_pkg_t *pkg);
> > >
> > > +/** Extracts package signature either from embedded package signature
> > > + * or if it is absent then reads data from detached signature file.
> > > + * @param pkg a pointer to package
> > > + * @param sig output parameter for signature data. Callee function 
> > > allocates
> > > + * buffer needed for the signature data. Caller is responsible for
> > > + * freeing this buffer.
> > > + * @return size of a signature or negative number if error.
> > > + */
> > > +int alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig);
> > > +
> > >  /** Returns the method used to validate a package during install.
> > >   * @param pkg a pointer to package
> > >   * @return an enum member giving the validation method
> > > diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
> > > index 5c5fa073..e0e4d987 100644
> > > --- a/lib/libalpm/package.c
> > > +++ b/lib/libalpm/package.c
> > > @@ -268,6 +268,37 @@ const char SYMEXPORT 
> > > *alpm_pkg_get_base64_sig(alpm_pkg_t *pkg)
> > >   return pkg->base64_sig;
> > >  }
> > >
> > > +int SYMEXPORT alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig)
> >
> > This API is weird, why are you returning an int when neither of the
> > successful values are int and can potentially overflow an int?  You're
> > returning the length of an allocated string, size_t is the appropriate
> > type.
> 
> size_t is unsigned int. But this function returns positive length in
> case of successful signature read or negative value in case of error.
> Thus return value cannot be size_t.

I get that, but you chose that interface.  Why?  Surely it makes more
sense to either return 0 on error, since you treat a missing sig as an
error anyway, or take a size_t* arg like decode_signature.

> >
> > > +{
> > > + ASSERT(pkg != NULL, return -1);
> > > + pkg->handle->pm_errno = ALPM_ERR_OK;
> >
> > I don't like clearing pm_errno without a good reason.  It generally
> > doesn't serve any purpose and can get us into trouble if a function
> > gets called during cleanup somewhere.
> 
> I personally do not feel that "pm_errno = ALPM_ERR_OK" is useful here
> and I am more than happy to remove it.
> 
> But I also I see this pattern is used a lot. Only this file
> (lib/libalpm/package.c) has like 20 use-cases of it. e.g.
> 
> off_t SYMEXPORT alpm_pkg_get_isize(alpm_pkg_t *pkg)
> {
> ASSERT(pkg != NULL, return -1);
> pkg->handle->pm_errno = ALPM_ERR_OK;
> return pkg->ops->get_isize(pkg);
> }
> 
> So I was trying to follow a standard practice here. Or are you trying
> to say that li

Re: [pacman-dev] [PATCH v2] Fallback to detached signatures during keyring check

[Andrew Gregory]

On 05/30/20 at 07:51pm, Anatol Pomozov wrote:
> Pacman has a 'key in keyring' verification step that makes sure the signatures
> have a valid keyid. Currently pacman parses embedded package signatures only.
> 
> Add a fallback to detached signatures. If embedded signature is missing then 
> it
> tries to read corresponding *.sig file and get keyid from there.
> 
> Verification:
>   debug: found cached pkg: 
> /var/cache/pacman/pkg/glib-networking-2.64.3-1-x86_64.pkg.tar.zst
>   debug: found detached signature 
> /var/cache/pacman/pkg/glib-networking-2.64.3-1-x86_64.pkg.tar.zst.sig with 
> size 310
>   debug: found signature key: A5E9288C4FA415FA
>   debug: looking up key A5E9288C4FA415FA locally
>   debug: key lookup success, key exists
> 
> Signed-off-by: Anatol Pomozov 
> ---
>  lib/libalpm/alpm.h| 10 ++
>  lib/libalpm/package.c | 31 +++
>  lib/libalpm/sync.c| 17 -
>  lib/libalpm/util.c| 35 +++
>  lib/libalpm/util.h|  1 +
>  5 files changed, 85 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index c6a13273..9c5fff73 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -1403,6 +1403,16 @@ alpm_db_t *alpm_pkg_get_db(alpm_pkg_t *pkg);
>   */
>  const char *alpm_pkg_get_base64_sig(alpm_pkg_t *pkg);
>  
> +/** Extracts package signature either from embedded package signature
> + * or if it is absent then reads data from detached signature file.
> + * @param pkg a pointer to package
> + * @param sig output parameter for signature data. Callee function allocates
> + * buffer needed for the signature data. Caller is responsible for
> + * freeing this buffer.
> + * @return size of a signature or negative number if error.
> + */
> +int alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig);
> +
>  /** Returns the method used to validate a package during install.
>   * @param pkg a pointer to package
>   * @return an enum member giving the validation method
> diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
> index 5c5fa073..e0e4d987 100644
> --- a/lib/libalpm/package.c
> +++ b/lib/libalpm/package.c
> @@ -268,6 +268,37 @@ const char SYMEXPORT *alpm_pkg_get_base64_sig(alpm_pkg_t 
> *pkg)
>   return pkg->base64_sig;
>  }
>  
> +int SYMEXPORT alpm_pkg_get_sig(alpm_pkg_t *pkg, unsigned char **sig)

This API is weird, why are you returning an int when neither of the
successful values are int and can potentially overflow an int?  You're
returning the length of an allocated string, size_t is the appropriate
type.

> +{
> + ASSERT(pkg != NULL, return -1);
> + pkg->handle->pm_errno = ALPM_ERR_OK;

I don't like clearing pm_errno without a good reason.  It generally
doesn't serve any purpose and can get us into trouble if a function
gets called during cleanup somewhere.

> + if(pkg->base64_sig) {
> + size_t sig_len;
> + int ret = alpm_decode_signature(pkg->base64_sig, sig, _len);
> + return ret == 0 ? (int)sig_len : -1;
> + } else {
> + char *pkgpath, *sigpath;
> + int len;
> +
> + pkgpath = _alpm_filecache_find(pkg->handle, pkg->filename);
> + if(!pkgpath) {
> + RET_ERR(pkg->handle, ALPM_ERR_PKG_NOT_FOUND, -1);
> + }
> + sigpath = _alpm_sigpath(pkg->handle, pkgpath);
> + if(!sigpath || _alpm_access(pkg->handle, NULL, sigpath, R_OK)) {
> + FREE(pkgpath);
> + FREE(sigpath);
> + RET_ERR(pkg->handle, ALPM_ERR_SIG_MISSING, -1);
> + }
> + len = _alpm_read_file(sigpath, sig);

You need to check for and handle failure.

> + _alpm_log(pkg->handle, ALPM_LOG_DEBUG, "found detached 
> signature %s with size %d\n", sigpath, len);
> + FREE(pkgpath);
> + FREE(sigpath);
> + return len;
> + }
> +}
> +
>  const char SYMEXPORT *alpm_pkg_get_arch(alpm_pkg_t *pkg)
>  {
>   ASSERT(pkg != NULL, return NULL);
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index 8c01ad95..0ab0fe26 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -880,18 +880,17 @@ static int check_keyring(alpm_handle_t *handle)
>   }
>  
>   level = alpm_db_get_siglevel(alpm_pkg_get_db(pkg));
> - if((level & ALPM_SIG_PACKAGE) && pkg->base64_sig) {
> - unsigned char *decoded_sigdata = NULL;
> - size_t data_len;
> - int decode_ret = alpm_decode_signature(pkg->base64_sig,
> - _sigdata, _len);
> - if(decode_ret == 0) {
> + if((level & ALPM_SIG_PACKAGE)) {
> + unsigned char *signature = NULL;
> + int sig_len = alpm_pkg_get_sig(pkg, );
> + if(sig_len > 0) {
>

Re: [pacman-dev] [PATCH v2] alpm: Add better logging when a command fails to execute correctly

[Andrew Gregory]

On 05/23/20 at 02:04pm, Daan De Meyer wrote:
> This makes debugging hook failures when running pacman less of a
> hassle. Instead of a generic "command failed to execute correctly",
> we now get the exact command and its arguments, as well as the exit
> code of the command logged.
> 
> Signed-off-by: Daan De Meyer 
> ---
> 
> Updated strv_join failure handling. I'm also not entirely sure what to
> do with the translations. Do I update all the separate files with the
> updated message identifier? I can fix some of the translations but
> the Arabic ones are impossible for me to fix.

No, translations are done separately through transifex.

>  lib/libalpm/util.c | 45 -
>  1 file changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> index ead03004..22ae48f6 100644
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -568,6 +568,42 @@ static void _alpm_reset_signals(void)
>   }
>  }
>  
> +static char *strv_join(char *const *strv, const char *separator)
> +{
> + char *const *i;
> + char *r, *p;
> + size_t size = 0;
> +
> + if (!separator) {
> + separator = " ";
> + }
> +
> + for (i = strv; *i; i++) {
> + if (i != strv) {
> + size += strlen(separator);
> + }
> +
> + size += strlen(*i);
> + }
> +
> + r = malloc(size + 1);
> + if (r == NULL) {
> + return NULL;
> + }
> +
> + p = r;
> +
> + for (i = strv; *i; i++) {
> + if (i != strv) {
> + p = stpcpy(p, separator);
> + }
> +
> + p = stpcpy(p, *i);
> + }
> +
> + return r;
> +}

We already have an almost identical function in pacman/util.c.

>  /** Execute a command with arguments in a chroot.
>   * @param handle the context handle
>   * @param cmd command to execute
> @@ -745,7 +781,14 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char 
> *cmd, char *const argv[],
>   if(WIFEXITED(status)) {
>   _alpm_log(handle, ALPM_LOG_DEBUG, "call to waitpid 
> succeeded\n");
>   if(WEXITSTATUS(status) != 0) {
> - _alpm_log(handle, ALPM_LOG_ERROR, _("command 
> failed to execute correctly\n"));
> + char *argv_str = strv_join(argv, NULL);
> + if (argv_str) {
> + _alpm_log(handle, ALPM_LOG_ERROR,
> + _("command \"%s\" 
> failed to execute correctly: %i\n"),
> + argv_str, 
> WEXITSTATUS(status));

I doubt that the exit status is generally going to be useful to the
user, nor is it clear from the error message that that's even what is
being printed.

> + free(argv_str);
> + }

I don't know that this is the right approach.  This is going to give
pretty useless, and probably confusing, output for scriptlets.  Hooks
and scripts should really be printing appropriate error messages when
they fail, so that our error message should really only matter in the
rare occasions where the command fails to execute at all.  I would be
more inclined to point the user at the actual hook or script that
failed.

> +
>   retval = 1;
>   }
>   } else if(WIFSIGNALED(status) != 0) {
> -- 
> 2.26.2

Re: [pacman-dev] [PATCH] Avoid depending on side effects in assert(...) expressions

[Andrew Gregory]

On 05/14/20 at 11:30pm, Anatol Pomozov wrote:
> Hi
> 
> On Wed, May 13, 2020 at 7:54 PM Andrew Gregory
>  wrote:
> >
> > You say
> > that the only way there would not be a corresponding progress bar is
> > if the progress event is called without an init event; that's not
> > true.  If there's a memory allocation failure in the init event there
> > won't be a progress object.
> 
> If there is a malloc failure in the init callback then application
> will crash with SIGSEGV while
> trying to access the NULL pointer
> 
> https://github.com/anatol/pacman/blob/b96e0df4dceaa2677baa1a3563211950708d3e63/src/pacman/callback.c#L850

You are looking at the wrong line, move down 3.

Also, that function needs to be fixed to meet our style guidelines.

> > At that point bar and index are
> > indeterminate and the best we can hope for is a quick segfault.
> 
> an application crash is pretty much what assert() does here. It
> basically checks the pointer before we start using it.
> 
> In some sense assert() is a form of a self-documenting check
> "this situation should never happen, but if it does then it is a bug
> in the app logic. so let's crash the app to bring
> maximum attention to it".
> 
> These asserts() are extra-checks but they are not required. It can be
> disabled or even removed without
> loosing functionality.

I know how assert works.  Like I said, if that assert gets compiled
out and we hit a malloc failure at the right time the results are
undefined and anything can happen.

> > The
> > callback api should be fixed so that callbacks can indicate an error
> > that should cancel the download.
> 
> It sounds like a good idea to return an error code from the callback.
> In this case we can
> handle malloc() failure in init() without crashing the application.
> 
> But I do not think it should block the multi-download testing. It
> would be great to get people testing the new functionality
> to discover issues as soon as possible.

Why the rush?  If we take a second and settle on a decent API first,
things that link to alpm can update and we can get that much more
testing and have a smoother update when we're ready for final release.
Otherwise, I'm not even going to be able to install a beta release
myself because I have no intention of making multiple updates to the
rest of my software while we figure out what the API should be.

> > I'd also say we should be passing the full list of download items to
> > the callback so that it's possible to write at least a very simple
> > multi-download callback without having to maintain a bunch of
> > complicated state information.
> 
> I am not sure I fully understand this idea. Could you please expand more on 
> it?

You're only passing data for whatever particular download item saw an
event, so the callback has to maintain its own list of the full set of
download objects.  We should give it the complete list each time.

Re: [pacman-dev] [PATCH] Avoid depending on side effects in assert(...) expressions

[Andrew Gregory]

On 05/13/20 at 12:08pm, Anatol Pomozov wrote:
> > ---
> > It's perhaps worth mentioning that nowhere else in the ALPM codebase
> > do we use assert().
> 
> I quite like the idea of defensive programming. This is something that
> I learnt the hard way when I was working with chips firmware.
> So I often add additional checks across the codebase and it saves me
> time during active phase of development/refactoring.
...
> A bit of context for this assert(). Both "progress" and "complete"
> events should always have a corresponding "init" event where
> progressbar structure is initialized.
> 
> If callback.c received a "progress" event for a non-existent
> progressbar then it is a programming error.

The problem is not defensive programming, it's that assert gets
compiled out under -DNDEBUG, so all your defenses disappear.  You say
that the only way there would not be a corresponding progress bar is
if the progress event is called without an init event; that's not
true.  If there's a memory allocation failure in the init event there
won't be a progress object.  At that point bar and index are
indeterminate and the best we can hope for is a quick segfault.  The
callback api should be fixed so that callbacks can indicate an error
that should cancel the download.

I'd also say we should be passing the full list of download items to
the callback so that it's possible to write at least a very simple
multi-download callback without having to maintain a bunch of
complicated state information.

Re: [pacman-dev] [PATCH] Split target output

[Andrew Gregory]

On 04/17/20 at 06:57pm, Carson Black wrote:
> This patch splits the monolithic 'Packages (count):' output on transactions 
> into
> multiple package outputs per category of action: 'Installing (count):', 
> 'Upgrading
> (count):', and 'Removing (count):'.
> 
> Signed-off-by: Carson Black 
> ---
>  src/pacman/util.c | 79 ++-
>  1 file changed, 65 insertions(+), 14 deletions(-)
>
> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index 6693ec75..3bec6774 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -898,7 +898,10 @@ static void _display_targets(alpm_list_t *targets, int 
> verbose)
>   char *str;
>   off_t isize = 0, rsize = 0, dlsize = 0;
>   unsigned short cols;
> - alpm_list_t *i, *names = NULL, *header = NULL, *rows = NULL;
> + char *install_header;
> + char *upgrade_header;
> + char *remove_header;
> + alpm_list_t *i, *install = NULL, *upgrade = NULL, *remove = NULL, 
> *header = NULL, *rows = NULL;
>  
>   if(!targets) {
>   return;
> @@ -918,6 +921,10 @@ static void _display_targets(alpm_list_t *targets, int 
> verbose)
>   }
>   }
>  
> + uint install_targets = 0;
> + uint upgrade_targets = 0;
> + uint remove_targets = 0;

No need for these, just use alpm_list_count as needed.

>   /* form data for both verbose and non-verbose display */
>   for(i = targets; i; i = alpm_list_next(i)) {
>   pm_target_t *target = i->data;
> @@ -926,38 +933,82 @@ static void _display_targets(alpm_list_t *targets, int 
> verbose)
>   rows = alpm_list_add(rows, create_verbose_row(target));
>   }
>  
> - if(target->install) {
> + if(target->install && target->remove) {
>   pm_asprintf(, "%s%s-%s%s", 
> alpm_pkg_get_name(target->install), config->colstr.faint,
> - alpm_pkg_get_version(target->install), 
> config->colstr.nocolor);
> - } else if(isize == 0) {
> - pm_asprintf(, "%s%s-%s%s", 
> alpm_pkg_get_name(target->remove), config->colstr.faint,
> - alpm_pkg_get_version(target->remove), 
> config->colstr.nocolor);
> + 
> alpm_pkg_get_version(target->install), config->colstr.nocolor);

Remove the extra indentation.

> + } else if(target->install) {
> + pm_asprintf(, "%s%s-%s%s", 
> alpm_pkg_get_name(target->install), config->colstr.faint,
> + 
> alpm_pkg_get_version(target->install), config->colstr.nocolor);
>   } else {
> - pm_asprintf(, "%s%s-%s %s[%s]%s", 
> alpm_pkg_get_name(target->remove), config->colstr.faint,
> - alpm_pkg_get_version(target->remove), 
> config->colstr.nocolor, _("removal"), config->colstr.nocolor);
> + pm_asprintf(, "%s%s-%s%s", 
> alpm_pkg_get_name(target->remove), config->colstr.faint,
> + 
> alpm_pkg_get_version(target->remove), config->colstr.nocolor);
> + }
> +
> + if (target->install && target->remove) {
> + upgrade = alpm_list_add(upgrade, str);
> + upgrade_targets++;
> + } else if (target->install) {
> + install = alpm_list_add(install, str);
> + install_targets++;
> + } else if (target->remove) {
> + remove = alpm_list_add(remove, str);
> + remove_targets++;

This just duplicates the conditional above, combine the two.

>   }
> - names = alpm_list_add(names, str);
>   }
>  
>   /* print to screen */
> - pm_asprintf(, "%s (%zu)", _("Packages"), alpm_list_count(targets));
> - printf("\n");
> + if (install_targets) {
> + pm_asprintf(_header, _("Installing (%u):"), 
> install_targets);
> + }
> + if (remove_targets) {
> + pm_asprintf(_header, _("Removing (%u):"), 
> remove_targets);
> + }
> + if (upgrade_targets) {
> + pm_asprintf(_header, _("Upgrading (%u):"), 
> upgrade_targets);
> + }

I don't see any point in pre-generating all three headers, just use
str and generate and free them as they're used.

>   cols = getcols();
>   if(verbose) {
>   header = create_verbose_header(alpm_list_count(targets));
>   if(table_display(header, rows, cols) != 0) {
>   /* fallback to list display if table wouldn't fit */
> - list_display(str, names, cols);
> + if (install_targets) {
> + list_display(install_header, install, cols);
> + }
> + if 

Re: [pacman-dev] [PATCH] Use noextract with pacman-conf NoExtract

[Andrew Gregory]

On 04/01/20 at 11:29am, Eli Schwartz wrote:
> From: Earnestly 
> 
> Current code accidently uses noupgrade for the NoExtract directive.
> 
> https://medium.com/@Code_Analysis/the-last-line-effect-7b1cb7f0d2a1
> 
> Signed-off-by: Eli Schwartz 
> ---

ACK

Re: [pacman-dev] [PATCH] Increase maximum database size

[Andrew Gregory]

On 01/19/20 at 09:42am, Allan McRae wrote:
> We previously has the maximum database size as 25MB.  This was set in the days
> before repos had as many packages as they do now, and before we started
> distributing files databases.  Increase this limit to 128MB.
> 
> Signed-off-by: Allan McRae 
> ---
> 
> So this has been hit in the wild.   Manjaro patches their pacman package to
> allow databases of 32MB, because their [community] repo files database
> breaks the 25MB limit.   But being Manjaro, the patch was never forwarded
> upstream, just like everything they have ever done.
> 
> People in Arch are no better.  A bug was reported, but some idiot (named
> Antonio Rojas) closed the bug as "not a bug", because it was not an
> Arch repo running into the issue.
> 
> So I only discovered this by seeing a closed bug report.
> 
> 
> Now, onto the change...  this is ~4x bigger than anything seen in the
> wild currently.  Is that enough of an increasse.

ACK.  Just update the comment.
 
>  lib/libalpm/be_sync.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index 07d2b4ae..a7050290 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -224,7 +224,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
>   int sig_ret = 0;
>  
>   /* set hard upper limit of 25MiB */
> - payload.max_size = 25 * 1024 * 1024;
> + payload.max_size = 128 * 1024 * 1024;
>  
>   /* print server + filename into a buffer */
>   len = strlen(server) + strlen(db->treename) + strlen(dbext) + 2;
> -- 
> 2.25.0

Re: [pacman-dev] [PATCH 1/2] Attempted to free lock on failure in alpm_db_update()

[Andrew Gregory]

On 12/19/19 at 11:17am, Allan McRae wrote:
> On 17/12/19 5:13 pm, Andrew Gregory wrote:
> > s/Attempted to//?
> > 
> > On 12/17/19 at 01:04am, Allan McRae wrote:
> >> Also fixes a memory leak under an error condition.
> >>
> >> Signed-off-by: Allan McRae 
> >> ---
> >>  lib/libalpm/be_sync.c | 18 ++
> >>  1 file changed, 10 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> >> index 041b2266..5502d92d 100644
> >> --- a/lib/libalpm/be_sync.c
> >> +++ b/lib/libalpm/be_sync.c
> > ...
> >> @@ -324,6 +325,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
> >>}
> >>}
> >>  
> >> +cleanup:
> > 
> > Shouldn't this be before the previous if(updated){...} block?
> 
> It could be...   No need to drop the cache and reset validation if we
> are restoring the old database, but it does not hurt.

That doesn't happen until patch 2 though.  If they're going to be
split into separate patches, this patch should be able to stand on its
own.  It's unlikely, but the restoration could also fail, causing
a package list to be loaded from an invalid database.

Re: [pacman-dev] [PATCH 2/2] Backup old database before updating and restore on failure

[Andrew Gregory]

On 12/17/19 at 01:05am, Allan McRae wrote:
> If alpm_db_update() fails due to an invalid signature, then the system
> is left with an unusable repo database.  Instead, backup the currently
> working database before performing the update, and restore on error.
> 
> Note that the calls rename and unlink are not checked for errors. If these
> fail, there is nothing to be done anyway.  It also allows for less complex
> flow, as these function fail gracefully when passed NULL arguments.
> 
> Signed-off-by: Allan McRae 
> ---

I really don't like the backup/restore method and would much prefer to
download to a new file and rotate it in once verified, but this is at
least a step in the right direction.

> That is a lot of teduim for adding ".bak" to the end of a string...

Sounds like a great excuse to start using asprintf.

> 
> I'd appreciate more eyes on these changes.
> 
>  lib/libalpm/be_sync.c | 58 ---
>  1 file changed, 44 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index 5502d92d..ae46c120 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -136,6 +136,7 @@ valid:
>   return 0;
>  }
>  
> +

Extra whitespace change.

>  /** Update a package database
>   *
>   * An update of the package database \a db will be attempted. Unless
> @@ -173,14 +174,15 @@ valid:
>   */
>  int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
>  {
> - char *syncpath;
> - const char *dbext;
> + char *syncpath, *dbsig = NULL, *dbfilebak = NULL, *dbsigbak = NULL;
> + const char *dbext, *dbfile;
>   alpm_list_t *i;
>   int updated = 0;
>   int ret = -1;
>   mode_t oldmask;
>   alpm_handle_t *handle;
>   int siglevel;
> + size_t len;
>  
>   /* Sanity checks */
>   ASSERT(db != NULL, return -1);
> @@ -217,10 +219,39 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
>  
>   dbext = handle->dbext;
>  
> + /* create backup of current database version */
> + dbfile = _alpm_db_path(db);
> + len = strlen(dbfile) + 5;
> + MALLOC(dbfilebak, len,
> + {
> + handle->pm_errno = ALPM_ERR_MEMORY;
> + ret = -1;
> + goto cleanup;
> + }
> + );
> + snprintf(dbfilebak, len, "%s.bak", dbfile);
> +
> +
> + dbsig = _alpm_sigpath(db->handle, dbfile);
> + len = strlen(dbsig) + 5;
> + MALLOC(dbsigbak, len,
> + {
> + handle->pm_errno = ALPM_ERR_MEMORY;
> + ret = -1;
> + goto cleanup;
> + }
> + );
> + snprintf(dbsigbak, len, "%s.bak", dbsig);
> +
> + /* remove any old backup files to prevent potential sig mismatch */
> + unlink(dbfilebak);
> + unlink(dbsigbak);
> + _alpm_copyfile(dbfile, dbfilebak);
> + _alpm_copyfile(dbsig, dbsigbak);

_alpm_copyfile doesn't currently use either O_EXCL or O_TRUNC when it
opens the destination, so if the unlink fails, this could result in
thoroughly broken backup files.

>   for(i = db->servers; i; i = i->next) {
>   const char *server = i->data, *final_db_url = NULL;
>   struct dload_payload payload;
> - size_t len;
>   int sig_ret = 0;
>  
>   memset(, 0, sizeof(struct dload_payload));
> @@ -247,17 +278,6 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
>   updated = (updated || ret == 0);
>  
>   if(ret != -1 && updated && (siglevel & ALPM_SIG_DATABASE)) {
> - /* an existing sig file is no good at this point */
> - char *sigpath = _alpm_sigpath(handle, 
> _alpm_db_path(db));
> - if(!sigpath) {
> - /* pm_errno should be set */
> - ret = -1;
> - goto cleanup;
> - }
> - unlink(sigpath);
> - free(sigpath);
> -
> -
>   /* check if the final URL from internal downloader 
> looks reasonable */
>   if(final_db_url != NULL) {
>   if(strlen(final_db_url) < 3
> @@ -327,13 +347,23 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
>  
>  cleanup:
>   if(ret == -1) {
> + rename(dbfilebak, dbfile);
> + rename(dbsigbak, dbsig);

This obliterates the invalid file, making troubleshooting impossible.

We really need to be sure that either this restoration succeeds or the
validity and cache were reset.

> +
>   /* pm_errno was set by the download code */
>   _alpm_log(handle, ALPM_LOG_DEBUG, "failed to sync db: %s\n",
>   alpm_strerror(handle->pm_errno));
>   } else {
> + unlink(dbfilebak);
> + unlink(dbsigbak);
> +
>   handle->pm_errno = 

Re: [pacman-dev] [PATCH 1/2] Attempted to free lock on failure in alpm_db_update()

[Andrew Gregory]

s/Attempted to//?

On 12/17/19 at 01:04am, Allan McRae wrote:
> Also fixes a memory leak under an error condition.
> 
> Signed-off-by: Allan McRae 
> ---
>  lib/libalpm/be_sync.c | 18 ++
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index 041b2266..5502d92d 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
...
> @@ -324,6 +325,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
>   }
>   }
>  
> +cleanup:

Shouldn't this be before the previous if(updated){...} block?

>   if(ret == -1) {
>   /* pm_errno was set by the download code */
>   _alpm_log(handle, ALPM_LOG_DEBUG, "failed to sync db: %s\n",

Re: [pacman-dev] [PATCH] Handle .part files that are the size of the correct package

[Andrew Gregory]

On 11/15/19 at 11:46pm, Allan McRae wrote:
> In rare cases, likely due to a well timed Ctrl+C, but possibly due to a
> broken mirror, a ".part" file may have size at least that of the correct
> package size.
> 
> When encountering this issue, currently pacman fails in different ways
> depending on where the package falls in the list to download.  If last,
> "wrong or NULL argument passed" error is reported, or a "invalid or
> corrupt package" issue if not.
> 
> Capture these .part files, and remove the extension. This lets pacman
> either use the package if valid, or offer to remove it if it fails checksum
> or signature verification.
> 
> Signed-off-by: Allan McRae 
> ---
> 
> To test this patch do:
> 
> mv /var/cache/pacman/pkg/xz-5.2.4-2-x86_64.pkg.tar.xz{,.part}
> pacman -S xz
> 
> 
> Having to run _alpm_filecache_find() in find_dl_candidates() on all packages 
> that
> have download size of 0 is not given we run already it in 
> compute_download_size().
> However, we would require storing it in the alpm_pkg_t struct and I don't 
> think
> that overhead was worth it.  However, we then call _alpm_filecache_find() in 
> check_validity() and load_package()...  Still probably not worth it!
> 
>  lib/libalpm/dload.c |  6 ++
>  lib/libalpm/sync.c  | 14 --
>  2 files changed, 18 insertions(+), 2 deletions(-)

ACK.  This does cause one minor oddity: if the .part is the only
package being installed, pacman prints "::Retrieving packages..." but
then doesn't actually download anything.

Re: [pacman-dev] [PATCH] Add Eli to current maintainers

[Andrew Gregory]

On 11/16/19 at 12:15am, Allan McRae wrote:
> Also retire Dan into past major contributors.

ACK.

Re: [pacman-dev] Alternatives system brainstorm

[Andrew Gregory]

On 10/19/19 at 10:43pm, Allan McRae wrote:
> On 19/10/19 10:41 pm, Allan McRae wrote:
> > On 19/10/19 10:38 pm, Allan McRae wrote:
> >> On 19/10/19 10:24 pm, Andrew Gregory wrote:
> >>> No rebuilding necessary if the conflicts are changed to just 'sh'.
> >>
> >> What will happen with "pacman -S dash"?  My initial thought is dash will
> >> pull in the first provider of sh - sh-bash - which will then pull in bash.
> > 
> > I forgot - pacman queries the user when multiple providers.
> 
> $ pacman -S b
> resolving dependencies...
> :: There are 2 providers available for foo:
> :: Repository test
>1) a-foo  2) b-foo
> 
> Enter a number (default=1):
> 
> It just picks the wrong one by default...

Implementing something like https://bugs.archlinux.org/task/62480
would take care of that.

In order for an alternatives system to be worth it, I think it needs
to be able to provide better flexibility than these link packages.
For example, you mentioned using it for something like python.
Imagine python4 ships IDLE separately; in order to use link packages
you'd need something like python4-links-interpreter and
python4-links-idle, and you'd probably want an all-encompassing
python4-links for convenience too.  That would get unwieldy pretty
quickly.  An alternatives system could allow the user to use python4
for everything else but leave idle pointing to an older version until
idle4 is installed.

So the features I think would be useful are:
1) the ability to select alternatives per-file
2) the ability to select alternatives per-package
3) the ability to group alternatives, potentially spanning multiple
   packages, and select them per-group

As for conflicts, I think go with first-installed-wins and print
a warning when a package gets a new alternative entry and the link is
already set to something else.

Re: [pacman-dev] Alternatives system brainstorm

[Andrew Gregory]

On 10/19/19 at 10:15pm, Allan McRae wrote:
> On 19/10/19 9:57 pm, Daan van Rossum wrote:
> > * on Saturday, 2019-10-19 18:15 +1000, Allan McRae  
> > wrote:
> > 
> >> /bin/sh -> bash/dash
> >> /usr/bin/awk -> gawk/nawk
> >> /usr/bin/cc -> gcc/clang
> > 
> > Can we provide alternatives by means of sets of mutually exclusive link 
> > packages, using package properties that are already supported in pacman?
> > 
> > sh-bash
> > provides=('sh')
> > depends=('bash')
> > conflicts=('sh-dash')
> > sh-dash
> > provides=('sh')
> > depends=('dash')
> > conflicts=('sh-bash')
> > bash
> > depends('sh')
> > dash
> > depends('sh')
> > 
> 
> Sure - or we could have a system to handle it in a less obtuse manner...
> 
> What happens when a new package wants to provide "sh"?   We need to
> rebuild sh-bash and sh-dash?  Easy enough, except that assumes that all
> three packages are packaged by the same group.  If bash and dash are
> packaged by a distro and the new package by a third party, then the
> conflicts provided by the distro are not enough.  But an alternatives
> system would work.

No rebuilding necessary if the conflicts are changed to just 'sh'.

Re: [pacman-dev] configure syntax error even after issuing autogen.sh

[Andrew Gregory]

On 10/18/19 at 06:38pm, Saul Tigh wrote:
> Dear pacman developers,
> 
> First time poster here, so please be gentle. I've been trying to compile
> pacman from git and I constantly run into an issue. First I issue
> ./autogen.sh and then ./configure but this error always pops up:
> 
> ./configure: line 14968: syntax error near unexpected token `$file_version,'
> ./configure: line 14968: ` AX_COMPARE_VERSION($file_version, ge, 5.38,
> with_file_seccomp=yes)'
> 
> What am I doing wrong?

Install autoconf-archive

[pacman-dev] [PATCH] remove: improve broken dependency error message

[Andrew Gregory]

This message was clarified for sync operations in
2b1b7b70753eb56bee08cd270efc7cfa342bc0ec.

Signed-off-by: Andrew Gregory 
---
 src/pacman/remove.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/pacman/remove.c b/src/pacman/remove.c
index 9d44cf53..0eeb19d3 100644
--- a/src/pacman/remove.c
+++ b/src/pacman/remove.c
@@ -116,8 +116,8 @@ int pacman_remove(alpm_list_t *targets)
for(i = data; i; i = alpm_list_next(i)) {
alpm_depmissing_t *miss = i->data;
char *depstring = 
alpm_dep_compute_string(miss->depend);
-   colon_printf(_("%s: removing %s breaks 
dependency '%s'\n"),
-   miss->target, 
miss->causingpkg, depstring);
+   colon_printf(_("removing %s breaks 
dependency '%s' required by %s\n"),
+   miss->causingpkg, 
depstring, miss->target);
free(depstring);
alpm_depmissing_free(miss);
}
-- 
2.23.0

Re: [pacman-dev] [PATCH v2 3/3] run XferCommand via exec

[Andrew Gregory]

On 10/12/19 at 09:11pm, Allan McRae wrote:
> On 12/10/19 1:45 pm, Andrew Gregory wrote:
> > system() runs the provided command via a shell, which is subject to
> > command injection.  Even though pacman already provides a mechanism to
> > sign and verify the databases containing the urls, certain distributions
> > have yet to get their act together and start signing databases, leaving
> > them vulnerable to MITM attacks.  Replacing the system call with an
> > almost equivalent exec call removes the possibility of a shell-injection
> > attack for those users.
> > 
> > Signed-off-by: Andrew Gregory 
> 
> 
> > @@ -230,17 +300,26 @@ static int download_with_xfercommand(const char *url, 
> > const char *localpath,
> > unlink(destfile);
> > }
> >  
> > -   tempcmd = strdup(config->xfercommand);
> > -   /* replace all occurrences of %o with fn.part */
> > -   if(strstr(tempcmd, "%o")) {
> > -   usepart = 1;
> > -   parsedcmd = strreplace(tempcmd, "%o", tempfile);
> > -   free(tempcmd);
> > -   tempcmd = parsedcmd;
> > +   if((argv = calloc(config->xfercommand_argc + 1, sizeof(char*))) == 
> > NULL) {
> 
> need to free this at the end.

Updated patch pushed to my repo that fixes this and the misplaced free
and also corrects the indenting in systemvp to use tabs.

[pacman-dev] [PATCH v2 2/3] add arg_to_string helper

[Andrew Gregory]

Converts an argc/argv pair to a string for presentation to the user.

Signed-off-by: Andrew Gregory 
---
 src/pacman/pacman.c | 26 +-
 src/pacman/util.c   | 23 +++
 src/pacman/util.h   |  1 +
 3 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
index b7406cea..c8e86b9d 100644
--- a/src/pacman/pacman.c
+++ b/src/pacman/pacman.c
@@ -1066,28 +1066,12 @@ static int parseargs(int argc, char *argv[])
  */
 static void cl_to_log(int argc, char *argv[])
 {
-   size_t size = 0;
-   int i;
-   for(i = 0; i < argc; i++) {
-   size += strlen(argv[i]) + 1;
+   char *cl_text = arg_to_string(argc, argv);
+   if(cl_text) {
+   alpm_logaction(config->handle, PACMAN_CALLER_PREFIX,
+   "Running '%s'\n", cl_text);
+   free(cl_text);
}
-   if(!size) {
-   return;
-   }
-   char *cl_text = malloc(size);
-   if(!cl_text) {
-   return;
-   }
-   char *p = cl_text;
-   for(i = 0; i + 1 < argc; i++) {
-   strcpy(p, argv[i]);
-   p += strlen(argv[i]);
-   *p++ = ' ';
-   }
-   strcpy(p, argv[i]);
-   alpm_logaction(config->handle, PACMAN_CALLER_PREFIX,
-   "Running '%s'\n", cl_text);
-   free(cl_text);
 }
 
 /** Main function.
diff --git a/src/pacman/util.c b/src/pacman/util.c
index 8f6290db..68cdb2e9 100644
--- a/src/pacman/util.c
+++ b/src/pacman/util.c
@@ -1771,3 +1771,26 @@ int pm_vfprintf(FILE *stream, alpm_loglevel_t level, 
const char *format, va_list
ret = vfprintf(stream, format, args);
return ret;
 }
+
+char *arg_to_string(int argc, char *argv[])
+{
+   char *cl_text, *p;
+   size_t size = 0;
+   int i;
+   for(i = 0; i < argc; i++) {
+   size += strlen(argv[i]) + 1;
+   }
+   if(!size) {
+   return NULL;
+   }
+   if(!(cl_text = malloc(size))) {
+   return NULL;
+   }
+   for(p = cl_text, i = 0; i + 1 < argc; i++) {
+   strcpy(p, argv[i]);
+   p += strlen(argv[i]);
+   *p++ = ' ';
+   }
+   strcpy(p, argv[i]);
+   return cl_text;
+}
diff --git a/src/pacman/util.h b/src/pacman/util.h
index a1fbef46..4b049849 100644
--- a/src/pacman/util.h
+++ b/src/pacman/util.h
@@ -76,6 +76,7 @@ int multiselect_question(char *array, int count);
 int colon_printf(const char *format, ...) __attribute__((format(printf, 1, 
2)));
 int yesno(const char *format, ...) __attribute__((format(printf, 1, 2)));
 int noyes(const char *format, ...) __attribute__((format(printf, 1, 2)));
+char *arg_to_string(int argc, char *argv[]);
 
 int pm_printf(alpm_loglevel_t level, const char *format, ...) 
__attribute__((format(printf,2,3)));
 int pm_asprintf(char **string, const char *format, ...) 
__attribute__((format(printf,2,3)));
-- 
2.23.0

[pacman-dev] [PATCH v2 3/3] run XferCommand via exec

[Andrew Gregory]

system() runs the provided command via a shell, which is subject to
command injection.  Even though pacman already provides a mechanism to
sign and verify the databases containing the urls, certain distributions
have yet to get their act together and start signing databases, leaving
them vulnerable to MITM attacks.  Replacing the system call with an
almost equivalent exec call removes the possibility of a shell-injection
attack for those users.

Signed-off-by: Andrew Gregory 
---
v2:
* properly deal with signals
* pass errno via pipe instead of mmap
* fix debug logging

 src/pacman/conf.c   | 129 
 src/pacman/conf.h   |   2 +
 test/pacman/tests/sync200.py|   2 +-
 test/pacman/tests/xfercommand001.py |   2 +-
 4 files changed, 116 insertions(+), 19 deletions(-)

diff --git a/src/pacman/conf.c b/src/pacman/conf.c
index 2d8518c4..9a39bba9 100644
--- a/src/pacman/conf.c
+++ b/src/pacman/conf.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include  /* uname */
+#include 
 #include 
 
 /* pacman */
@@ -153,6 +154,7 @@ int config_free(config_t *oldconfig)
free(oldconfig->print_format);
free(oldconfig->arch);
free(oldconfig);
+   wordsplit_free(oldconfig->xfercommand_argv);
 
return 0;
 }
@@ -201,18 +203,86 @@ static char *get_tempfile(const char *path, const char 
*filename)
return tempfile;
 }
 
+/* system()/exec() hybrid function allowing exec()-style direct execution
+ * of a command with the simplicity of system()
+ * - not thread-safe
+ * - errno may be set by fork(), pipe(), or execvp()
+ */
+static int systemvp(const char *file, char *const argv[])
+{
+int pid, err = 0, ret = -1, err_fd[2];
+sigset_t oldblock;
+struct sigaction sa_ign = { .sa_handler = SIG_IGN }, oldint, oldquit;
+
+if(pipe(err_fd) != 0) {
+return -1;
+}
+
+sigaction(SIGINT, _ign, );
+sigaction(SIGQUIT, _ign, );
+sigaddset(_ign.sa_mask, SIGCHLD);
+sigprocmask(SIG_BLOCK, _ign.sa_mask, );
+
+pid = fork();
+
+/* child */
+if(pid == 0) {
+close(err_fd[0]);
+fcntl(err_fd[1], F_SETFD, FD_CLOEXEC);
+
+/* restore signal handling for the child to inherit */
+sigaction(SIGINT, , NULL);
+sigaction(SIGQUIT, , NULL);
+sigprocmask(SIG_SETMASK, , NULL);
+
+execvp(file, argv);
+
+/* execvp failed, pass the error back to the parent */
+while(write(err_fd[1], , sizeof(errno)) == -1 && errno == EINTR);
+_Exit(127);
+}
+
+/* parent */
+close(err_fd[1]);
+
+if(pid != -1)  {
+int wret;
+while((wret = waitpid(pid, , 0)) == -1 && errno == EINTR);
+if(wret > 0) {
+while(read(err_fd[0], , sizeof(err)) == -1 && errno == EINTR);
+}
+} else {
+/* fork failed, make sure errno is preserved after cleanup */
+err = errno;
+}
+
+close(err_fd[0]);
+
+sigaction(SIGINT, , NULL);
+sigaction(SIGQUIT, , NULL);
+sigprocmask(SIG_SETMASK, , NULL);
+
+if(err) {
+errno = err;
+ret = -1;
+}
+
+return ret;
+}
+
 /** External fetch callback */
 static int download_with_xfercommand(const char *url, const char *localpath,
int force)
 {
int ret = 0, retval;
int usepart = 0;
-   int cwdfd;
+   int cwdfd = -1;
struct stat st;
-   char *parsedcmd, *tempcmd;
char *destfile, *tempfile, *filename;
+   const char **argv;
+   size_t i;
 
-   if(!config->xfercommand) {
+   if(!config->xfercommand_argv) {
return -1;
}
 
@@ -230,17 +300,26 @@ static int download_with_xfercommand(const char *url, 
const char *localpath,
unlink(destfile);
}
 
-   tempcmd = strdup(config->xfercommand);
-   /* replace all occurrences of %o with fn.part */
-   if(strstr(tempcmd, "%o")) {
-   usepart = 1;
-   parsedcmd = strreplace(tempcmd, "%o", tempfile);
-   free(tempcmd);
-   tempcmd = parsedcmd;
+   if((argv = calloc(config->xfercommand_argc + 1, sizeof(char*))) == 
NULL) {
+   size_t bytes = (config->xfercommand_argc + 1) * sizeof(char*);
+   pm_printf(ALPM_LOG_ERROR,
+   _n("malloc failure: could not allocate %zu 
byte\n",
+  "malloc failure: could not allocate %zu 
bytes\n",
+bytes),
+   bytes);
+   goto cleanup;
+   }
+
+   for(i = 0; i <= config->xfercommand_argc; i++) {
+   const char *val = config->xfercommand_argv[i];
+   if(val && strcmp(val, "%o") == 0) {
+   usepart = 1;
+ 

[pacman-dev] [PATCH v2 1/3] move wordsplit into common for sharing

[Andrew Gregory]

Signed-off-by: Andrew Gregory 
---

v2: removes _alpm_prefix and moves to util-common

 lib/libalpm/hook.c   | 119 +--
 src/common/util-common.c | 112 
 src/common/util-common.h |   3 +
 3 files changed, 118 insertions(+), 116 deletions(-)

diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c
index 6143ea0f..9d1aa215 100644
--- a/lib/libalpm/hook.c
+++ b/lib/libalpm/hook.c
@@ -17,7 +17,6 @@
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include 
 #include 
 #include 
 #include 
@@ -71,23 +70,12 @@ static void _alpm_trigger_free(struct _alpm_trigger_t 
*trigger)
}
 }
 
-static void _alpm_wordsplit_free(char **ws)
-{
-   if(ws) {
-   char **c;
-   for(c = ws; *c; c++) {
-   free(*c);
-   }
-   free(ws);
-   }
-}
-
 static void _alpm_hook_free(struct _alpm_hook_t *hook)
 {
if(hook) {
free(hook->name);
free(hook->desc);
-   _alpm_wordsplit_free(hook->cmd);
+   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);
@@ -158,107 +146,6 @@ static int _alpm_hook_validate(alpm_handle_t *handle,
return ret;
 }
 
-static char **_alpm_wordsplit(char *str)
-{
-   char *c = str, *end;
-   char **out = NULL, **outsave;
-   size_t count = 0;
-
-   if(str == NULL) {
-   errno = EINVAL;
-   return NULL;
-   }
-
-   for(c = str; isspace(*c); c++);
-   while(*c) {
-   size_t wordlen = 0;
-
-   /* extend our array */
-   outsave = out;
-   if((out = realloc(out, (count + 1) * sizeof(char*))) == NULL) {
-   out = outsave;
-   goto error;
-   }
-
-   /* calculate word length and check for unbalanced quotes */
-   for(end = c; *end && !isspace(*end); end++) {
-   if(*end == '\'' || *end == '"') {
-   char quote = *end;
-   while(*(++end) && *end != quote) {
-   if(*end == '\\' && *(end + 1) == quote) 
{
-   end++;
-   }
-   wordlen++;
-   }
-   if(*end != quote) {
-   errno = EINVAL;
-   goto error;
-   }
-   } else {
-   if(*end == '\\' && (end[1] == '\'' || end[1] == 
'"')) {
-   end++; /* skip the '\\' */
-   }
-   wordlen++;
-   }
-   }
-
-   if(wordlen == (size_t) (end - c)) {
-   /* no internal quotes or escapes, copy it the easy way 
*/
-   if((out[count++] = strndup(c, wordlen)) == NULL) {
-   goto error;
-   }
-   } else {
-   /* manually copy to remove quotes and escapes */
-   char *dest = out[count++] = malloc(wordlen + 1);
-   if(dest == NULL) { goto error; }
-   while(c < end) {
-   if(*c == '\'' || *c == '"') {
-   char quote = *c;
-   /* we know there must be a matching end 
quote,
-* no need to check for '\0' */
-   for(c++; *c != quote; c++) {
-   if(*c == '\\' && *(c + 1) == 
quote) {
-   c++;
-   }
-   *(dest++) = *c;
-   }
-   c++;
-   } else {
-   if(*c == '\\' && (c[1] == '\'' || c[1] 
== '"')) {
-   c++; /* skip the '\\' */
-   }
-   *(dest++) = *(c++);
-   }
-   }
-   *dest = '\0';
-   }
-
-   if(*end == '\0') {
-   break;
-   } else {
-

Re: [pacman-dev] [PATCH 2/4] libalpm: resolvedep(): don't compare names twice

[Andrew Gregory]

On 09/08/19 at 10:45pm, morganamilo wrote:
> If we failed to get the pkg from pkgcache then we know no satisfying
> package exists by name. So only compare provides.
> ---
>  lib/libalpm/deps.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c
> index ce7869c3..322c4e7e 100644
> --- a/lib/libalpm/deps.c
> +++ b/lib/libalpm/deps.c
> @@ -698,7 +698,8 @@ static alpm_pkg_t *resolvedep(alpm_handle_t *handle, 
> alpm_depend_t *dep,
>   for(j = _alpm_db_get_pkgcache(db); j; j = j->next) {
>   alpm_pkg_t *pkg = j->data;
>   if((pkg->name_hash != dep->name_hash || 
> strcmp(pkg->name, dep->name) != 0)

Unrelated to this patch, but we probably shouldn't be filtering out
name matches here.  A package could provide a different version of
itself.

> - && _alpm_depcmp(pkg, dep) && 
> !alpm_pkg_find(excluding, pkg->name)) {
> + && _alpm_depcmp_provides(dep, 
> alpm_pkg_get_provides(pkg))
> + && !alpm_pkg_find(excluding, 
> pkg->name)) {
>   if(alpm_pkg_should_ignore(handle, pkg)) {
>   alpm_question_install_ignorepkg_t 
> question = {
>   .type = 
> ALPM_QUESTION_INSTALL_IGNOREPKG,
> -- 
> 2.23.0

Re: [pacman-dev] [PATCH] pacman: better handle -F when file is not found

[Andrew Gregory]

On 09/09/19 at 05:49pm, morganamilo wrote:
> Error messages are now printed.
> Pacman now returns 1 if any of the files queried are not found.
> ---
>  src/pacman/files.c | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/src/pacman/files.c b/src/pacman/files.c
> index 8e518486..c4351114 100644
> --- a/src/pacman/files.c
> +++ b/src/pacman/files.c
> @@ -115,7 +115,6 @@ static int files_search(alpm_list_t *syncs, alpm_list_t 
> *targets, int regex) {
>  
>   if(regex) {
>   if(regcomp(, targ, REG_EXTENDED | REG_NOSUB | 
> REG_ICASE | REG_NEWLINE) != 0) {
> - /* TODO: error message */
>   goto notfound;

...

>  notfound:
> - if(!found) {
> - ret++;
> + ret = 1;
> + if(regex) {
> + pm_printf(ALPM_LOG_ERROR, _("no files match '%s'\n"), 
> targ);

"no files match" isn't really a great error message if the real
problem is that the regex failed to even compile.

Re: [pacman-dev] pacman --print bug

[Andrew Gregory]

On 08/12/19 at 02:15pm, AstroSnail via pacman-dev wrote:
> Hi,
> 
> I think I found a bug in pacman.
> 
> When a package remove operation can't be satisfied, pacman prints an
> error, describes what went wrong, and does nothing:
> $ sudo pacman -R linux-api-headers
> checking dependencies...
> error: failed to prepare transaction (could not satisfy dependencies)
> :: glibc: removing linux-api-headers breaks dependency 
> 'linux-api-headers>=4.10'
> This is obviously correct.
> 
> When I try to --print a package remove operation that can't be
> satisfied, pacman prints an error, describes it, and does
> nothing, without printing the targets:
> $ pacman -Rp linux-api-headers
> error: failed to prepare transaction (could not satisfy dependencies)
> :: glibc: removing linux-api-headers breaks dependency 
> 'linux-api-headers>=4.10'
> I can understand if this is correct behaviour as well, but at a glance it 
> looks strange.
> 
> When a package remove operation *can* be satisfied, but only with
> user confirmation (e.g. a HoldPkg was found in target list), pacman
> will print warnings and ask for user input before deciding whether
> or not to continue:
> $ sudo pacman -Rc linux-api-headers
> checking dependencies...
> 
> warning: pacman is designated as a HoldPkg.
> warning: glibc is designated as a HoldPkg.
> :: HoldPkg was found in target list. Do you want to continue? [y/N]
> This is obviously correct.
> 
> When I try to --print a package remove operation that *can* be
> satisfied, but only with user input, pacman prints a prompt for
> input but doesn't wait, and exits immediately, without even printing
> the warnings:
> $ pacman -Rcp linux-api-headers
> :: HoldPkg was found in target list. Do you want to continue? [y/N]
> This does not sound like correct behaviour to me.

What would you consider correct behavior?

Re: [pacman-dev] [PATCH] pacman/callback: fix buffer over-read

[Andrew Gregory]

On 08/03/19 at 01:27am, László Várady wrote:
> Commit 11ab9aa9f5f0f3873df89c73e8715b82f485bd9b replaced a strcpy() call
> with memcpy(), without copying the terminating null character.
> 
> Since fname is allocated with malloc(), subsequent strstr() calls will
> overrun the buffer's boundary.
> 
> Signed-off-by: László Várady 

ACK.

Re: [pacman-dev] [PATCH] libalpm: fix documentation for alpm_pkg_mtree_next

[Andrew Gregory]

On 06/21/19 at 03:24pm, Morgan Adamiec wrote:
> On Thu, 20 Jun 2019 at 05:46, Allan McRae  wrote:
> >
> > On 14/6/19 11:26 pm, Dave Reisner wrote:
> > > On Fri, Jun 14, 2019 at 02:19:52PM +0100, Morgan Adamiec wrote:
> > >> On Fri, 14 Jun 2019 at 14:09, Dave Reisner  wrote:
> > >>>
> > >>> On Fri, Jun 14, 2019 at 02:51:14AM +0100, morganamilo wrote:
> >  libarchive uses 1 for EOF, not 0. Instead of using the actual ints, use
> >  libarchive's error codes.
> >  ---
> > 
> >  By the way, not familiar with doxygen. Is my wording fine or is there
> >  some built in "see also" functionality?
> > 
> >  diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> >  index ffb2ad96..ece894cf 100644
> >  --- a/lib/libalpm/alpm.h
> >  +++ b/lib/libalpm/alpm.h
> >  @@ -1326,7 +1326,8 @@ struct archive *alpm_pkg_mtree_open(alpm_pkg_t 
> >  *pkg);
> >    * @param pkg the package that the mtree file is being read from
> >    * @param archive the archive structure reading from the mtree file
> >    * @param entry an archive_entry to store the entry header information
> >  - * @return 0 if end of archive is reached, non-zero otherwise.
> >  + * @return ARCHIVE_OK on success, ARCHIVE_EOF if end of archive is 
> >  reached,
> >  + * otherwise an error occured (see archive.h).
> > >>>
> > >>> Please, no. Let's not leak details from libarchive in our own API.
> > >>>
> >    */
> >   int alpm_pkg_mtree_next(const alpm_pkg_t *pkg, struct archive 
> >  *archive,
> >    struct archive_entry **entry);
> >  --
> >  2.21.0
> > >>
> > >> Why not? The return value is exactly that. If libarchive's return
> > >> codes suddenly changed then so would libalpms's. Plus pacman itself
> > >> already uses ARCHIVE_OK to check the return code. And finally if we
> > >> did not depend on magic numbers then the doc wouldn't be wrong in the
> > >> first place.
> > >
> > > Because users of libalpm should only need to understand libalpm and not
> > > concern themselves with details of libarchive. Exposing ARCHIVE_* in
> > > libalpm is a leaky abstraction.
> > >
> > > If the code is broken (and it sounds like it is), then it should be
> > > fixed along with the documentation.
> > >
> >
> > Agreed.   Not this is the only place in pacman we use an ARCHIVE_*
> > value, so this is broken.
> >
> > src/pacman/check.c: while(alpm_pkg_mtree_next(pkg, mtree, ) ==
> > ARCHIVE_OK) {
> 
> Would is then also make sense to do `typedef struct archive
> alpm_mtree_t` or something similar?

I already said this on IRC, but, for the record, I don't see a need for a full
alpm mtree implementation.  I wouldn't reject it if somebody really wanted to
put in that effort, but it seems like a lot of work to reimplement libarchive's
entire archive_entry_* API for little gain.  Personally, I'd just delete this
function and alpm_pkg_mtree_close.

Re: [pacman-dev] [PATCH v2] doc: Add a man page describing PKGINFO

[Andrew Gregory]

On 06/29/19 at 11:18pm, Eli Schwartz wrote:
> On 6/28/19 2:55 PM, Jelle van der Waa wrote:
> > From: Jelle van der Waa 
> > 
> > Describe the PKGINFO format which resides in a package produced makepkg.
> As mentioned on IRC, there's a lot of duplication with the PKGBUILD
> fields, so I'd feel better if we could somehow document just a list of
> fields and say that their contents are derived from the PKGBUILD(5) meaning.
> 
> The PKGINFO manpage should just describe the format of the file (as
> given in the suggested synopsis) and pointers to where the relevant
> metadata comes from.
> 
> Some PKGINFO keys are specific to the built package, e.g. size,
> builddate, or mean something different, like arch/pkgver -- we should
> document why and how they differ.

This is low-level documentation.  If we're going to document the file
format, we should document it as libalpm sees it, not just the flavor
that makepkg generates.  For example, all of the dependency fields,
that includes provides, replaces, and conflicts, can have an optional
description even though makepkg doesn't allow one.  There are only
a handful of fields that actually map directly.

Re: [pacman-dev] [PATCH v2] doc: Add a man page describing PKGINFO

[Andrew Gregory]

On 06/28/19 at 08:55pm, Jelle van der Waa wrote:
> From: Jelle van der Waa 
> 
> Describe the PKGINFO format which resides in a package produced makepkg.
> ---
>  doc/Makefile.am|  4 +-
>  doc/PKGINFO.5.asciidoc | 87 ++
>  doc/meson.build|  1 +
>  3 files changed, 91 insertions(+), 1 deletion(-)
>  create mode 100644 doc/PKGINFO.5.asciidoc
> 
> diff --git a/doc/Makefile.am b/doc/Makefile.am
> index 5c575832..634388e8 100644
> --- a/doc/Makefile.am
> +++ b/doc/Makefile.am
> @@ -16,7 +16,8 @@ MANPAGES = \
>   pacman.conf.5 \
>   libalpm.3 \
>   BUILDINFO.5 \
> - pacman-conf.8
> + pacman-conf.8 \
> + PKGINFO.5
>  
>  DOXYGEN_MANS = $(wildcard man3/*.3)
>  
> @@ -47,6 +48,7 @@ EXTRA_DIST = \
>   makepkg.conf.5.asciidoc \
>   pacman.conf.5.asciidoc \
>   BUILDINFO.5.asciidoc \
> + PKGINFO.5.asciidoc \
>   libalpm.3.asciidoc \
>   footer.asciidoc \
>   index.asciidoc \
> diff --git a/doc/PKGINFO.5.asciidoc b/doc/PKGINFO.5.asciidoc
> new file mode 100644
> index ..101ba34c
> --- /dev/null
> +++ b/doc/PKGINFO.5.asciidoc
> @@ -0,0 +1,87 @@
> +/
> +vim:set ts=4 sw=4 syntax=asciidoc noet spell spelllang=en_us:
> +/

A few have sneaked in since the removal, but we don't include vim
modelines anymore.

> +PKGINFO(5)
> +==
> +
> +Name
> +
> +PKGINFO - package information file
> +
> +
> +Synopsis
> +
> +This manual page describes the format of a PKGINFO file found in the root of
> +a package created by makepkg. The file contains a description of the 
> package's
> +information. The information is formatted in key-value pairs separated by
> +a '=', one value per line. Arrays are represented multiple keys with the same
> +value.

Key-value pairs are separated by " = ", not "=".

Arrays are represented by repeating the same key multiple times, each
line presumably has a different value, not the same one.

Empty lines and lines beginning with "#" are ignored.

> +
> +
> +Description
> +---
> +
> +*pkgname*::
> + The name of the package.
> +
> +*pkgbase*::
> + The base name of a package, usually the same as the pkgname except for
> + split packages.
> +
> +*pkgver*::
> + The version of the package including pkgrel and epoch.

This should include the actual format.

> +*pkgdesc*::
> + A description of the software contained in the package.
> +
> +*url*::
> + The upstream url of the package.

packaged software.

> +*builddate*::
> + The build date of the package in epoch.
> +
> +*packager*::
> + The packager of the package formatted "Name ".

Neither makepkg nor libalpm says anything about the format of this
field, neither should this documentation.

> +*size*::
> + The size of the package in bytes.

"installation size" or "packaged files", it is not the size of the
package itself.

> +*arch*::
> + The architecture of the package.
> +
> +*license*::
> + The license of the package.

License is an array.

> +*replaces (array)*::
> + An array of packages that this package should replace.
> +
> +*group (array)*::
> + An array of names that represent groups of packages.

Groups to which this package belongs.

> +*conflict (array)*::
> + An array of packages that will conflict with this package.
> +
> +*provides (array)*::
> + An array of "virtual provisions" this package provides.
> +
> +*backup (array)*::
> + An array of file names, which should be backed up if the package is 
> removed
> + or upgraded.
> +
> +*depend (array)*::
> + The dependencies of the package.
> +
> +*optdepend (array)*::
> + The optional dependencies of the package.
> +
> +*makedepend (array)*::
> + The make dependencies of the package.
> +
> +*checkdepend (array)*::
> + The check dependencies of the package.

This should document the format for dependency values.

> +See Also
> +
> +linkman:makepkg[8], linkman:pkgbuild[5]
> +
> +include::footer.asciidoc[]
> diff --git a/doc/meson.build b/doc/meson.build
> index a5bcd5b3..2f966616 100644
> --- a/doc/meson.build
> +++ b/doc/meson.build
> @@ -12,6 +12,7 @@ manpages = [
>{ 'name': 'libalpm.3' },
>{ 'name': 'BUILDINFO.5' },
>{ 'name': 'pacman-conf.8' },
> +  { 'name': 'PKGINFO.5' },
>  ]
>  
>  sitepages = [
> -- 
> 2.22.0

Re: [pacman-dev] [PATCH] fixup run XferCommand via exec

[Andrew Gregory]

On 06/11/19 at 11:06am, Allan McRae wrote:
> On 10/6/19 6:50 am, Andrew Gregory wrote:
> > Does anybody know what usepart was for?  It was unset unless %o was
> > used in XferCommand, but I'm not sure what the use case for an
> > XferCommand without %o would be.
> 
> When %o was added, there were cases in the wild where users had a
> downloader that did not support resuming the download.  No sure this
> would be the case now...

I assume nobody minds if that possibility is removed.

[pacman-dev] [PATCH] fixup run XferCommand via exec

[Andrew Gregory]

---

A few changes I omitted from the initial patch.

Does anybody know what usepart was for?  It was unset unless %o was
used in XferCommand, but I'm not sure what the use case for an
XferCommand without %o would be.

 src/pacman/conf.c   | 11 ---
 test/pacman/tests/sync200.py|  2 +-
 test/pacman/tests/xfercommand001.py |  2 +-
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/pacman/conf.c b/src/pacman/conf.c
index faf446dc..c2b7da43 100644
--- a/src/pacman/conf.c
+++ b/src/pacman/conf.c
@@ -249,7 +249,6 @@ static int download_with_xfercommand(const char *url, const 
char *localpath,
int force)
 {
int ret = 0, retval;
-   int usepart = 0;
int cwdfd = -1;
struct stat st;
char *destfile, *tempfile, *filename;
@@ -322,12 +321,10 @@ static int download_with_xfercommand(const char *url, 
const char *localpath,
} else {
/* download was successful */
ret = 0;
-   if(usepart) {
-   if(rename(tempfile, destfile)) {
-   pm_printf(ALPM_LOG_ERROR, _("could not rename 
%s to %s (%s)\n"),
-   tempfile, destfile, 
strerror(errno));
-   ret = -1;
-   }
+   if(rename(tempfile, destfile)) {
+   pm_printf(ALPM_LOG_ERROR, _("could not rename %s to %s 
(%s)\n"),
+   tempfile, destfile, strerror(errno));
+   ret = -1;
}
}
 
diff --git a/test/pacman/tests/sync200.py b/test/pacman/tests/sync200.py
index 2bcdd5d3..18f38b81 100644
--- a/test/pacman/tests/sync200.py
+++ b/test/pacman/tests/sync200.py
@@ -1,6 +1,6 @@
 self.description = "Synchronize the local database"
 
-self.option['XferCommand'] = ['/usr/bin/curl %u > %o']
+self.option['XferCommand'] = ['/usr/bin/curl %u -o %o']
 
 sp1 = pmpkg("spkg1", "1.0-1")
 sp1.depends = ["spkg2"]
diff --git a/test/pacman/tests/xfercommand001.py 
b/test/pacman/tests/xfercommand001.py
index 0d244dc6..0ac99080 100644
--- a/test/pacman/tests/xfercommand001.py
+++ b/test/pacman/tests/xfercommand001.py
@@ -3,7 +3,7 @@
 # this setting forces us to download packages
 self.cachepkgs = False
 #wget doesn't support file:// urls.  curl does
-self.option['XferCommand'] = ['/usr/bin/curl %u > %o']
+self.option['XferCommand'] = ['/usr/bin/curl %u -o %o']
 
 numpkgs = 10
 pkgnames = []
-- 
2.21.0

[pacman-dev] [PATCH 2/2] [WIP] run XferCommand via exec

[Andrew Gregory]

---

systemvp should pretty much be a drop-in replacement for system with
the exception that it takes an argv array and uses exec.  If anybody
wants to play with it to stress test it a little, I have
a self-contained copy and test program at:
https://github.com/andrewgregory/snippets/blob/systemv/c/systemv.c

TODO:
* update docs
* fix debug logging
* should the command be run with PATH lookup (execv vs execvp)?
* Is the use of mmap with MAP_ANONYMOUS okay?  MAP_ANONYMOUS is
  not POSIX but "most systems also support MAP_ANONYMOUS (or its
  synonym MAP_ANON)" (mmap(2)).
* should we reset signals prior to exec'ing like we do with
  hooks/scripts?

 src/pacman/conf.c | 95 ++-
 src/pacman/conf.h |  2 +
 2 files changed, 80 insertions(+), 17 deletions(-)

diff --git a/src/pacman/conf.c b/src/pacman/conf.c
index 2d8518c4..faf446dc 100644
--- a/src/pacman/conf.c
+++ b/src/pacman/conf.c
@@ -26,9 +26,11 @@
 #include 
 #include 
 #include  /* strdup */
+#include 
 #include 
 #include 
 #include  /* uname */
+#include 
 #include 
 
 /* pacman */
@@ -153,6 +155,7 @@ int config_free(config_t *oldconfig)
free(oldconfig->print_format);
free(oldconfig->arch);
free(oldconfig);
+   _alpm_wordsplit_free(oldconfig->xfercommand_argv);
 
return 0;
 }
@@ -201,18 +204,59 @@ static char *get_tempfile(const char *path, const char 
*filename)
return tempfile;
 }
 
+static int systemvp(const char *file, char *const argv[])
+{
+   int pid, *err;
+
+   err = mmap(NULL, sizeof(*err), PROT_READ | PROT_WRITE,
+   MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+   if(err == NULL) {
+   return -1;
+   }
+
+   pid = fork();
+   if(pid == -1) {
+   /* error */
+   munmap(err, sizeof(*err));
+   return -1;
+   } else if(pid == 0) {
+   /* child */
+   *err = 0;
+   execvp(file, argv);
+   *err = errno;
+   munmap(err, sizeof(*err));
+   _Exit(1);
+   } else {
+   /* parent */
+   int status;
+   while(waitpid(pid, , 0) == -1) {
+   if(errno != EINTR) {
+   munmap(err, sizeof(*err));
+   return -1;
+   }
+   }
+   if(*err != 0) {
+   errno = *err;
+   status = -1;
+   }
+   munmap(err, sizeof(*err));
+   return status;
+   }
+}
+
 /** External fetch callback */
 static int download_with_xfercommand(const char *url, const char *localpath,
int force)
 {
int ret = 0, retval;
int usepart = 0;
-   int cwdfd;
+   int cwdfd = -1;
struct stat st;
-   char *parsedcmd, *tempcmd;
char *destfile, *tempfile, *filename;
+   const char **argv;
+   size_t i;
 
-   if(!config->xfercommand) {
+   if(!config->xfercommand_argv) {
return -1;
}
 
@@ -230,17 +274,20 @@ static int download_with_xfercommand(const char *url, 
const char *localpath,
unlink(destfile);
}
 
-   tempcmd = strdup(config->xfercommand);
-   /* replace all occurrences of %o with fn.part */
-   if(strstr(tempcmd, "%o")) {
-   usepart = 1;
-   parsedcmd = strreplace(tempcmd, "%o", tempfile);
-   free(tempcmd);
-   tempcmd = parsedcmd;
+   if((argv = calloc(config->xfercommand_argc + 1, sizeof(char*))) == 
NULL) {
+   pm_printf(ALPM_LOG_ERROR, _("could not get current working 
directory\n"));
+   goto cleanup;
+   }
+
+   for(i = 0; i <= config->xfercommand_argc; i++) {
+   const char *val = config->xfercommand_argv[i];
+   if(val && strcmp(val, "%o") == 0) {
+   val = tempfile;
+   } else if(val && strcmp(val, "%u") == 0) {
+   val = url;
+   }
+   argv[i] = val;
}
-   /* replace all occurrences of %u with the download URL */
-   parsedcmd = strreplace(tempcmd, "%u", url);
-   free(tempcmd);
 
/* save the cwd so we can restore it later */
do {
@@ -256,9 +303,13 @@ static int download_with_xfercommand(const char *url, 
const char *localpath,
ret = -1;
goto cleanup;
}
-   /* execute the parsed command via /bin/sh -c */
-   pm_printf(ALPM_LOG_DEBUG, "running command: %s\n", parsedcmd);
-   retval = system(parsedcmd);
+
+   printf("running command: %s", config->xfercommand_argv[0]);
+   for(i = 1; argv[i]; i++) {
+   printf(" '%s'", argv[i]);
+   }
+   printf("\n");
+   retval = systemvp(argv[0], (char**)argv);
 
if(retval == -1) {

[pacman-dev] [PATCH 1/2] [WIP] move wordsplit into ini for sharing

[Andrew Gregory]

Not technically related to INI parsing, but we use it with INI files.
---
 lib/libalpm/hook.c | 113 -
 src/common/ini.c   | 113 +
 src/common/ini.h   |   3 ++
 3 files changed, 116 insertions(+), 113 deletions(-)

diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c
index 6143ea0f..c385b5d5 100644
--- a/lib/libalpm/hook.c
+++ b/lib/libalpm/hook.c
@@ -17,7 +17,6 @@
  *  along with this program.  If not, see .
  */
 
-#include 
 #include 
 #include 
 #include 
@@ -71,17 +70,6 @@ static void _alpm_trigger_free(struct _alpm_trigger_t 
*trigger)
}
 }
 
-static void _alpm_wordsplit_free(char **ws)
-{
-   if(ws) {
-   char **c;
-   for(c = ws; *c; c++) {
-   free(*c);
-   }
-   free(ws);
-   }
-}
-
 static void _alpm_hook_free(struct _alpm_hook_t *hook)
 {
if(hook) {
@@ -158,107 +146,6 @@ static int _alpm_hook_validate(alpm_handle_t *handle,
return ret;
 }
 
-static char **_alpm_wordsplit(char *str)
-{
-   char *c = str, *end;
-   char **out = NULL, **outsave;
-   size_t count = 0;
-
-   if(str == NULL) {
-   errno = EINVAL;
-   return NULL;
-   }
-
-   for(c = str; isspace(*c); c++);
-   while(*c) {
-   size_t wordlen = 0;
-
-   /* extend our array */
-   outsave = out;
-   if((out = realloc(out, (count + 1) * sizeof(char*))) == NULL) {
-   out = outsave;
-   goto error;
-   }
-
-   /* calculate word length and check for unbalanced quotes */
-   for(end = c; *end && !isspace(*end); end++) {
-   if(*end == '\'' || *end == '"') {
-   char quote = *end;
-   while(*(++end) && *end != quote) {
-   if(*end == '\\' && *(end + 1) == quote) 
{
-   end++;
-   }
-   wordlen++;
-   }
-   if(*end != quote) {
-   errno = EINVAL;
-   goto error;
-   }
-   } else {
-   if(*end == '\\' && (end[1] == '\'' || end[1] == 
'"')) {
-   end++; /* skip the '\\' */
-   }
-   wordlen++;
-   }
-   }
-
-   if(wordlen == (size_t) (end - c)) {
-   /* no internal quotes or escapes, copy it the easy way 
*/
-   if((out[count++] = strndup(c, wordlen)) == NULL) {
-   goto error;
-   }
-   } else {
-   /* manually copy to remove quotes and escapes */
-   char *dest = out[count++] = malloc(wordlen + 1);
-   if(dest == NULL) { goto error; }
-   while(c < end) {
-   if(*c == '\'' || *c == '"') {
-   char quote = *c;
-   /* we know there must be a matching end 
quote,
-* no need to check for '\0' */
-   for(c++; *c != quote; c++) {
-   if(*c == '\\' && *(c + 1) == 
quote) {
-   c++;
-   }
-   *(dest++) = *c;
-   }
-   c++;
-   } else {
-   if(*c == '\\' && (c[1] == '\'' || c[1] 
== '"')) {
-   c++; /* skip the '\\' */
-   }
-   *(dest++) = *(c++);
-   }
-   }
-   *dest = '\0';
-   }
-
-   if(*end == '\0') {
-   break;
-   } else {
-   for(c = end + 1; isspace(*c); c++);
-   }
-   }
-
-   outsave = out;
-   if((out = realloc(out, (count + 1) * sizeof(char*))) == NULL) {
-   out = outsave;
-   goto error;
-   }
-
-   out[count++] = NULL;
-
-   return out;
-
-error:
-   /* can't use wordsplit_free here because NULL has not been appended */
-   while(count) {
-   free(out[--count]);
-   }
-   free(out);
-   

[pacman-dev] [PATCH] use consistent time notation for the log

[Andrew Gregory]

%X is locale-dependent, making it impossible to reliably parse and
potentially overflowing the buffer.  %T is consistent across locales.

Also fixes some adjacent whitespace.

Signed-off-by: Andrew Gregory 
---
 lib/libalpm/log.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/libalpm/log.c b/lib/libalpm/log.c
index d8842a55..2ff5544b 100644
--- a/lib/libalpm/log.c
+++ b/lib/libalpm/log.c
@@ -39,11 +39,11 @@ static int _alpm_log_leader(FILE *f, const char *prefix)
 {
time_t t = time(NULL);
struct tm *tm = localtime();
-int length = 32;
-char timestamp[length];
+   int length = 32;
+   char timestamp[length];
 
/* Use ISO-8601 date format */
-strftime(timestamp,length,"%FT%X%z", tm);
+   strftime(timestamp,length,"%FT%T%z", tm);
return fprintf(f, "[%s] [%s] ", timestamp, prefix);
 }
 
-- 
2.21.0

[pacman-dev] [PATCH 2/2] create coredump on segfault

[Andrew Gregory]

Overriding the segfault handler prevents the creation of core dumps by
the default handler, which makes debugging segfaults difficult.

Signed-off-by: Andrew Gregory 
---
 src/pacman/sighandler.c | 28 ++--
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c
index a4849a0c..082756b5 100644
--- a/src/pacman/sighandler.c
+++ b/src/pacman/sighandler.c
@@ -38,6 +38,15 @@ static ssize_t xwrite(int fd, const void *buf, size_t count)
return ret;
 }
 
+static void _reset_handler(int signum)
+{
+   struct sigaction new_action;
+   sigemptyset(_action.sa_mask);
+   new_action.sa_handler = SIG_DFL;
+   new_action.sa_flags = 0;
+   sigaction(signum, _action, NULL);
+}
+
 /** Catches thrown signals. Performs necessary cleanup to ensure database is
  * in a consistent state.
  * @param signum the thrown signal
@@ -76,19 +85,26 @@ void install_soft_interrupt_handler(void)
 
 void remove_soft_interrupt_handler(void)
 {
-   struct sigaction new_action;
-   sigemptyset(_action.sa_mask);
-   new_action.sa_handler = SIG_DFL;
-   new_action.sa_flags = 0;
-   sigaction(SIGINT, _action, NULL);
-   sigaction(SIGHUP, _action, NULL);
+   _reset_handler(SIGINT);
+   _reset_handler(SIGHUP);
 }
 
 static void segv_handler(int signum)
 {
+   sigset_t segvset;
const char msg[] = "\nerror: segmentation fault\n"
"Please submit a full bug report with --debug if 
appropriate.\n";
xwrite(STDERR_FILENO, msg, sizeof(msg) - 1);
+
+   /* restore the default handler */
+   _reset_handler(signum);
+   /* unblock SIGSEGV */
+   sigaddset(, signum);
+   sigprocmask(SIG_UNBLOCK, , NULL);
+   /* re-raise to trigger a core dump */
+   raise(signum);
+
+   /* raise should immediately abort, but just to make absolutely sure */
_Exit(signum);
 }
 
-- 
2.21.0

[pacman-dev] [PATCH 1/2] sighandler: block signals while handling SIGSEGV

[Andrew Gregory]

If we get SIGSEGV we need to bail out quickly, leaving other signals
unblocked could lead to other signal handlers getting triggered.

Signed-off-by: Andrew Gregory 
---

Signals are hard.  I'd appreciate if somebody could double check my
math on these patches.

 src/pacman/sighandler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c
index ebcdebae..a4849a0c 100644
--- a/src/pacman/sighandler.c
+++ b/src/pacman/sighandler.c
@@ -96,7 +96,7 @@ void install_segv_handler(void)
 {
struct sigaction new_action;
new_action.sa_handler = segv_handler;
-   sigemptyset(_action.sa_mask);
+   sigfillset(_action.sa_mask);
new_action.sa_flags = SA_RESTART;
sigaction(SIGSEGV, _action, NULL);
 }
-- 
2.21.0

[pacman-dev] [PATCH] hooks: rename type File to Path

[Andrew Gregory]

Make it clearer that the targets are matched against both directories
and regular files and free up File to potentially refer specifically to
regular files in the future.  File is retained as a deprecated alias for
Path for the time being to avoid breaking existing hooks and will be
removed in a future release.

See FS#53136.

Signed-off-by: Andrew Gregory 
---
 doc/alpm-hooks.5.asciidoc   | 17 +
 lib/libalpm/hook.c  |  8 ++--
 test/pacman/tests/hook-file-change-packages.py  |  2 +-
 .../tests/hook-file-remove-trigger-match.py |  2 +-
 test/pacman/tests/hook-file-upgrade-nomatch.py  |  2 +-
 test/pacman/tests/hook-target-list.py   |  2 +-
 test/pacman/tests/hook-type-reused.py   |  2 +-
 7 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/doc/alpm-hooks.5.asciidoc b/doc/alpm-hooks.5.asciidoc
index 0f6a1057..916d43bb 100644
--- a/doc/alpm-hooks.5.asciidoc
+++ b/doc/alpm-hooks.5.asciidoc
@@ -12,7 +12,7 @@ SYNOPSIS
 
 [Trigger] (Required, Repeatable)
 Operation = Install|Upgrade|Remove (Required, Repeatable)
-Type = File|Package (Required)
+Type = Path|Package (Required)
 Target =  (Required, Repeatable)
 
 [Action] (Required)
@@ -49,19 +49,20 @@ defined the hook will run if the transaction matches *any* 
of the triggers.
Select the type of operation to match targets against.  May be specified
multiple times.  Installations are considered an upgrade if the package 
or
file is already present on the system regardless of whether the new 
package
-   version is actually greater than the currently installed version.  For 
File
+   version is actually greater than the currently installed version.  For 
Path
triggers, this is true even if the file changes ownership from one 
package
to another.  Required.
 
-*Type =* File|Package::
+*Type =* Path|Package::
Select whether targets are matched against transaction packages or 
files.
-   See CAVEATS for special notes regarding File triggers.  Required.
+   See CAVEATS for special notes regarding Path triggers. 'File' is a 
deprecated
+   alias for 'Path' and will be removed in a future release.  Required.
 
 *Target =* ::
-   The file path or package name to match against the active transaction.
-   File paths refer to the files in the package archive; the installation 
root
+   The path or package name to match against the active transaction.
+   Paths refer to the files in the package archive; the installation root
should *not* be included in the path.  Shell-style glob patterns are
-   allowed. It is possible to invert matches by prepending a file with an
+   allowed. It is possible to invert matches by prepending a target with an
exclamation mark. May be specified multiple times. Required.
 
 ACTIONS
@@ -119,7 +120,7 @@ Exec = /usr/bin/sync
 CAVEATS
 ---
 
-There are situations when file triggers may act in unexpected ways.  Hooks are
+There are situations when path triggers may act in unexpected ways.  Hooks are
 triggered using the file list of the installed, upgraded, or removed package.
 When installing or upgrading a file that is extracted with a '.pacnew'
 extension, the original file name is used in triggering the hook.  When
diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c
index d90ed2da..6143ea0f 100644
--- a/lib/libalpm/hook.c
+++ b/lib/libalpm/hook.c
@@ -38,7 +38,7 @@ enum _alpm_hook_op_t {
 
 enum _alpm_trigger_type_t {
ALPM_HOOK_TYPE_PACKAGE = 1,
-   ALPM_HOOK_TYPE_FILE,
+   ALPM_HOOK_TYPE_PATH,
 };
 
 struct _alpm_trigger_t {
@@ -303,7 +303,11 @@ static int _alpm_hook_parse_cb(const char *file, int line,
if(strcmp(value, "Package") == 0) {
t->type = ALPM_HOOK_TYPE_PACKAGE;
} else if(strcmp(value, "File") == 0) {
-   t->type = ALPM_HOOK_TYPE_FILE;
+   _alpm_log(handle, ALPM_LOG_DEBUG,
+   "File targets are deprecated, 
use Path instead\n");
+   t->type = ALPM_HOOK_TYPE_PATH;
+   } else if(strcmp(value, "Path") == 0) {
+   t->type = ALPM_HOOK_TYPE_PATH;
} else {
error(_("hook %s line %d: invalid value %s\n"), 
file, line, value);
}
diff --git a/test/pacman/tests/hook-file-change-packages.py 
b/test/pacman/tests/hook-file-change-packages.py
index ad96fc14..4671dbe8 100644
--- a/test/pacman/tests/hook-file-change-packages.py
+++ b/test/pacman/tests/hook-file-change-packages.py
@@ -4,7 +4,7 @@
 self.add_hook("hook",
 """
 [Trigger]
-Type = File
+Type = Path
 O

[pacman-dev] [PATCH] makepkg: restrict pkgname and pkgver to ascii

[Andrew Gregory]

pkgname and pkgver are used as directory names within database files.
libarchive does not provide a reliable locale-independent method for
reading archive file names, causing errors when archive paths include
non-ascii characters.

This is a first step toward dealing with FS#49342, by hopefully reducing
the number of packages with non-ascii data in the wild before updating
libalpm to reject them outright.

See https://github.com/libarchive/libarchive/wiki/Filenames
and https://github.com/libarchive/libarchive/issues/587

Signed-off-by: Andrew Gregory 
---
 scripts/libmakepkg/lint_pkgbuild/pkgname.sh.in | 4 
 scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in  | 5 +
 2 files changed, 9 insertions(+)

diff --git a/scripts/libmakepkg/lint_pkgbuild/pkgname.sh.in 
b/scripts/libmakepkg/lint_pkgbuild/pkgname.sh.in
index 84666a29..51a25d2c 100644
--- a/scripts/libmakepkg/lint_pkgbuild/pkgname.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/pkgname.sh.in
@@ -45,6 +45,10 @@ lint_one_pkgname() {
error "$(gettext "%s is not allowed to start with a dot.")" 
"$type"
ret=1
fi
+   if [[ $name = *[![:ascii:]]* ]]; then
+   error "$(gettext "%s may only contain ascii characters.")" 
"$type"
+   return 1
+   fi
if [[ $name = *[^[:alnum:]+_.@-]* ]]; then
error "$(gettext "%s contains invalid characters: '%s'")" \
"$type" "${name//[[:alnum:]+_.@-]}"
diff --git a/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in 
b/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
index 8d5d04f1..b0f45027 100644
--- a/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/pkgver.sh.in
@@ -41,6 +41,11 @@ check_pkgver() {
error "$(gettext "%s is not allowed to contain colons, forward 
slashes, hyphens or whitespace.")" "pkgver${type:+ in $type}"
return 1
fi
+
+   if [[ $ver = *[![:ascii:]]* ]]; then
+   error "$(gettext "%s may only contain ascii characters.")" 
"pkgver${type:+ in $type}"
+   return 1
+   fi
 }
 
 lint_pkgver() {
-- 
2.21.0

Re: [pacman-dev] [PATCH] Add config var to limit repo connection timeout

[Andrew Gregory]

On 04/06/19 at 04:43pm, Nathan Aclander wrote:
> 
> I'm fairly new to using git send-email, so I wasn't sure how to add an
> explanation to the patch while cleanly separating it from the git commit
> message. I can speak more to why I found it useful.

Use --annotate and put comments you don't want to end up in the log between the
diff stats and the first 'diff --git' line:

---
 doc/pacman.conf.5.asciidoc | 3 +++
 8 files changed, 24 insertions(+), 1 deletion(-)

<--- Comments go here.

diff --git a/doc/pacman.conf.5.asciidoc b/doc/pacman.conf.5.asciidoc

> While I was traveling, certain mirrors became unreachable, or extremely
> slow. I had to wait 10 seconds not only for each repo, but then again
> every time pacman would try and download an individual package. Since
> this was set at 10 seconds by default, the wait time was far too long. I
> wanted to change it to 1 second so that pacman would move on to the next
> mirror faster.
> 
> Using this patch seemed more usable for me than to having to comment out
> mirrors manually depending my network conditions. Are you concerned that
> having this parameter be tunable would cause pacman.conf to be too
> complex?

More or less, yes.  We don't want to have to implement every single option that
libcurl provides in pacman, so we try to require a fairly compelling
justification before adding them.  We are not against making network operations
more configurable, though.  In fact, we would like to get rid of the external
downloader, but can't because some people need specific settings that we don't,
and never will, expose directly.  So, if you, or anybody else, can come up with
a more general solution that doesn't require us to add dozens of options to
pacman.conf, we'd love to hear it.

Re: [pacman-dev] [PATCH] Fix clang 8 string-plus-int warnings

[Andrew Gregory]

On 04/05/19 at 12:02am, Rikard Falkeborn wrote:
> Clang 8 warns that adding a string to an integer does not append to
> string. Indeed it doesn't, but that was not the intentetion. Use array
> indexing as suggested by the compiler to silence the warning. There
> should be no functional change.
> 
> Example of warning message:
> 
> alpm.c:71:54: warning: adding 'int' to a string does not append to the 
> string [-Wstring-plus-int]
> sprintf(hookdir, "%s%s", myhandle->root, SYSHOOKDIR + 1);
>  ~~~^~~
> alpm.c:71:54: note: use array indexing to silence this warning
> sprintf(hookdir, "%s%s", myhandle->root, SYSHOOKDIR + 1);
> ^
>  &  [  ]
> 1 warning generated.
> ---
>  lib/libalpm/alpm.c | 2 +-
>  src/pacman/conf.c  | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

I think this warning is dumb, but I guess the fix doesn't hurt anything.  ACK

Re: [pacman-dev] [PATCH] Style: Removing unecessary indentation level.

[Andrew Gregory]

On 04/04/19 at 12:21am, Leonardo Bras wrote:
> Signed-off-by: Leonardo Bras 
> ---
>  src/pacman/sync.c | 84 ---
>  1 file changed, 43 insertions(+), 41 deletions(-)

I appreciate the reduction of indentation, but I think this does more harm than
good.  It breaks our official style rules because err is no longer declared at
the top of its block; it makes sync_prepare_execute inconsistent because the
trans_prepare error is still handled in an if block; finally, it makes the
error branch look like the primary execution branch instead of an exception.
I would move the error printing out of sync_prepare_execute altogether into
print_{prepare,commit}_error functions.  That will also have the benefit of
shortening sync_prepare_execute, which is a bit long.

Re: [pacman-dev] Alternatives system

[Andrew Gregory]

On 04/01/19 at 09:56am, Allan McRae wrote:
> Hi all,
> 
> I plan to finish implementing an alternatives system for pacman post 5.2
> release:
> https://wiki.archlinux.org/index.php/User:Allan/Alternatives
> 
> Any comments or suggestions?

"All alternatives are symlinked in /usr/bin so no need to specify the
full path."

Why?  Arch puts all executables in /usr/bin but other users may not.

"TODO bikeshed the separator"

We're in the rather unfortunate position that neither of our database
formats can handle multiple arbitrary filenames on a single line
without escaping.  We can use -> in PKGBUILDs if you like, but for
.PKGINFO and desc files I would pick a single character to make
escaping easier.

You don't mention anywhere on the wiki what happens when a package is
removed.  I'm guessing alpm would remove the alternative if the link
target is owned by the package and call a callback to prompt the user
to select a new provider?  That means the upgrade logic will need to
track whether the new version still provides the alternative.

How would packages that provide a file as an alternative interact with
those that provide it directly?  What happens if an alternative is
installed and then a user attempts to install a package that provides
that file directly?  Does that just fail as a file conflict?  What if
the package that provides the file directly is uninstalled?  We'd have
to scan the local database for every single file we remove (or at
least those in usr/bin, if that stands) in case any packages provide
that file as an alternative.

Overall, I'm torn between "this seems really neat" and "this seems
like a lot of complexity for something we don't /really/ need".  So,
I'm going to play devil's advocate here: what's the advantage of this
over the existing solutions?  The example you give is bin/sh.  On
Arch, that could be solved by the user with NoExtract.  For a more
general solution, the bin/sh link could be distributed by itself in
sh-bash and sh-dash packages.  It's not very aesthetically pleasing,
but it gets the job done with just what we already have.

Re: [pacman-dev] should repo-add handle *.links.db?

[Andrew Gregory]

On 03/26/19 at 10:42pm, Erich Eckner wrote:
> On Wed, 27 Mar 2019, Allan McRae wrote:
> 
> > On 27/3/19 7:20 am, Erich Eckner wrote:
> >> https://mirror.pkgbuild.com/core/os/x86_64/core.links.tar.gz
> >>
> >> It contains (as far as I can tell) the names and versions of libraries
> >> against which binaries in a package are linked.
> >
> > This is not a repo, so should not be managed by repo-add.
> 
> Thanks for your answer, but:
> 
> Is the *.files.tar.gz a "repo"?
> 
> My interpretaion (so far) was, that the *.files.tar.gz and *.links.tar.gz 
> files are caches for search accellerations inside the repo (the first for 
> "who owns that file" and the second for "who links against that file") - 
> is this not the case?
> 
> Also note, that managing the *.links.tar.gz file with repo-add would have 
> benefits from a cleanness point of view: currently it's created 
> asynchronously but it could be created synchronously (e.g. *.db.tar.gz, 
> *.files.tar.gz and *.links.tar.gz would be always in-sync).
> 
> Furthermore, each downstream distribution wanting to have sogrep 
> needs to implement the createlinks script asynchronously, too. This would 
> become obsolete if it was part of repo-add.

I don't understand these arguments.  sogrep is the only thing
consuming these databases and no other uses have been proposed.  Why
can't sogrep provide a script to create its own databases that
distributions can use in their tools at the same time they call
repo-add?  I'm not very familiar with them, but I know Arch has
various scripts for building packages and managing repos; just have
them call `buildlinksdb`, or whatever, immediately after repo-add.

Re: [pacman-dev] New Optional Dependencies Aren't Logged.

[Andrew Gregory]

On 03/17/19 at 06:02pm, Ralph Corderoy wrote:
> Hi Andrew,
> 
> > I'm not trying to weaken anything; you simply haven't told us why you
> > needed the information beyond "I wanted to".  If there was an actual
> > problem you were trying to solve, describing it might help us come up
> > with a better solution.
> 
> Okay, sorry.
> 
> > it sounds like -Qi/-Sii would have been enough
> 
> Oh, the second -i adds the `Optional For' output.  pacman(1) omits that
> useful fact.  Does this count as a report to get it added?
> 
> -i, --info
> Display information on a given package.  The -p option can be
> used if querying a package file instead of the local database.
> Passing two --info or -i flags will also display the list of
> backup files and their modification states.

You're looking at the -Q options; listing reverse dependencies with
-ii only applies to -S and is documented in that section.

> So I could have done `pacman -Sii gst-plugins-bad' and then looked at
> the `Optional For' to see what of those I had installed and were
> upgraded recently.  Thanks.

Re: [pacman-dev] New Optional Dependencies Aren't Logged.

[Andrew Gregory]

On 03/17/19 at 04:49pm, Ralph Corderoy wrote:
> Hi Andrew,
> 
> > Based on your description, this sounds like a case of "I was mildly
> > curious about something this one time
> 
> No, it would have been useful to know.  There's no need to overly weaken
> it.

I'm not trying to weaken anything; you simply haven't told us why you
needed the information beyond "I wanted to".  If there was an actual
problem you were trying to solve, describing it might help us come up
with a better solution.

> > I understand the desire, but there are already tools to find what a
> > package optionally depends on and what optionally depends on a
> > package.
> 
> Have you one in mind?  It's not pactree(1), I checked that first.

Depends somewhat on exactly what you need.  Based on the information
you have provided, it sounds like -Qi/-Sii would have been enough, but
paccheck would be another potentially useful tool.

> > Before we start bloating every user's log with extraneous information
> 
> Point taken, though looking at pacman.log, it would be very slight
> bloat given all its existing content.

Re: [pacman-dev] New Optional Dependencies Aren't Logged.

[Andrew Gregory]

On 03/17/19 at 03:31pm, Ralph Corderoy wrote:
> `pacman -Su' suggested an optional dependency of gst-plugins-bad.
> I looked to see what that was and didn't want it.  Later, due to
> discussion on the local LUG IRC, I wanted to look up what package had
> the new optional depends but found /var/log/pacman.log didn't record it.
> Please consider logging them.
> 
> Looking at the optional depends of all the upgraded packages, I think it
> was webkit2gtk, but if it were logged then I could check if there were
> any new optional depends in the more general case, e.g. looking back in
> time.

Based on your description, this sounds like a case of "I was mildly
curious about something this one time, so please make a permanent
record of this information on every user's system in case I happen to
be curious about it again in the future."  I understand the desire,
but there are already tools to find what a package optionally depends
on and what optionally depends on a package.  Before we start bloating
every user's log with extraneous information, there needs to be a more
substantial use case than just idle curiosity.

Re: [pacman-dev] [PATCH 0/5][RFC] Die delta, die!

[Andrew Gregory]

On 03/04/19 at 11:23am, Allan McRae wrote:
> On 2/3/19 8:19 pm, Allan McRae wrote:
> > Deltas are broken. So much so that I would strongly recommend never
> > using a delta from a repo that you did not generate yourself. In short,
> > we call "system(command)", with a command that includes the name of
> > a delta file, and the name of the package file before and after applying
> > the delta. The name of the delta and the package files is controlled by
> > the information in the repo, and could contain a malicious command to be
> > run as root.
> > 
> > We could possibly work around this, but it is a very risky piece of code
> > and I believe it would be very hard to fully secure. Instead, I propose
> > to remove delta support completely.
> 
> FYI, I'll retract my statement that it would be hard to fully secure.
> It is entirely possible to avoid spiking in shell code into the file
> names.  But I'd still be happy removing deltas.

I've wanted to remove deltas for some time just due to how poorly
tested and maintained they are, so this still gets a +1 from me.  You
missed references to deltaratio in etc/pacman.conf and README.

Re: [pacman-dev] [PATCH] libalpm: Check cachedir access before re-creating

[Andrew Gregory]

On 01/23/19 at 10:52pm, David Phillips wrote:
> If users have mounted a filesystem such as sshfs or NFS that doesn't
> allow root access by default for security reasons, _alpm_filecache_setup
> will blindly try and re-create the components of the cachedir path,
> eating EACCES (to be liek mkdir -p). Then, it will return this path as
> if it is valid when it may not be readable by root, causing
> alpm_check_downloadspace to trigger a FPE since it assumes the cachedir
> is R/W and uses fsinfo which may be zero.
> 
> This patch adds a check before trying to re-create the cachedir in order
> to determine if creating it will result in EACCES, and outputs a warning
> if so. It also discards the current cachedir from being considered. This
> causes a more meaningful and graceful failure than a FPE.
> ---
>  lib/libalpm/util.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> index d33eef2a..d4375593 100644
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -849,12 +849,16 @@ const char *_alpm_filecache_setup(alpm_handle_t *handle)
>   for(i = handle->cachedirs; i; i = i->next) {
>   cachedir = i->data;
>   if(stat(cachedir, ) != 0) {
> - /* cache directory does not exist try creating it */
> - _alpm_log(handle, ALPM_LOG_WARNING, _("no %s cache 
> exists, creating...\n"),
> - cachedir);
> - if(_alpm_makepath(cachedir) == 0) {
> - _alpm_log(handle, ALPM_LOG_DEBUG, "using 
> cachedir: %s\n", cachedir);
> - return cachedir;
> + if (errno == EACCES) {
> + _alpm_log(handle, ALPM_LOG_WARNING, _("failed 
> to stat %s: %s\n"), cachedir, strerror(errno));
> + } else {
> + /* cache directory does not exist try 
> creating it */
> + _alpm_log(handle, ALPM_LOG_WARNING, _("no %s 
> cache exists, creating...\n"),
> + cachedir);
> + if(_alpm_makepath(cachedir) == 0) {
> + _alpm_log(handle, ALPM_LOG_DEBUG, 
> "using cachedir: %s\n", cachedir);
> + return cachedir;
> + }

It's been quite a while since I used sshfs or NFS, so maybe I'm
missing something, but this doesn't make a lot of sense to me.  Does
makepath really succeed if we don't have access rights?  I would have
thought that would already fail, causing the next directory to be
tried.  Second, why single out EACCESS as the exception?  Of all the
reasons stat can fail, ENOENT is really the only one that makes sense
for us to try to create the directory.  Where is the FPE actually
occurring?  That should be fixed directly, because it could still be
triggered if our access rights change between selecting the cachedir
and actually using it.

apg

Re: [pacman-dev] [PATCH] Check for all return values of _alpm_key_in_keychain

[Andrew Gregory]

On 04/21/17 at 04:07pm, David Phillips wrote:
> This fixes a bug I encountered with a GPG keyring where the
> key id used to locate a key in the keyring was ambiguous within
> my keychain.
> 
> This commit ensures that all valid return values are checked to
> catch this and related error cases rather than incorrectly taking
> an error case to mean the key was found, since this is rarely going
> to be the case.
> ---
>  lib/libalpm/be_package.c | 17 ++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c
> index 7e8b7920..1891fa5a 100644
> --- a/lib/libalpm/be_package.c
> +++ b/lib/libalpm/be_package.c
> @@ -754,10 +754,21 @@ int SYMEXPORT alpm_pkg_load(alpm_handle_t *handle, 
> const char *filename, int ful
>   alpm_list_t *k;
>   for(k = keys; k; k = k->next) {
>   char *key = k->data;
> - if(_alpm_key_in_keychain(handle, key) 
> == 0) {
> - if(_alpm_key_import(handle, 
> key) == -1) {
> + switch(_alpm_key_in_keychain(handle, 
> key)) {
> + case 1:
> + /* key is known; 
> proceed */
> + break;
> + case 0:
> + /* key is unknown; 
> attempt to import */
> + 
> if(_alpm_key_import(handle, key) == -1) {
> + fail = 1;
> + }
> + break;
> + case -1:
> + /* error finding key in 
> keychain */
> + default:
>   fail = 1;

Just below this, an error message is printed if fail is true, saying
that the key is missing.  If we got a generic code, that may not be
the case, and is the exact opposite of the problem that prompted
patch.

I'm wondering if this is necessary at all, though.  This bit of code
is specifically just for importing missing keys.  Any other
significant errors should be caught during the actual validation
attempt, where we already provide the actual gpg error message.  If
the gpgme segmentation fault is no longer an issue, do we gain
anything other than bailing out slightly sooner by adding the extra
check and complexity for printing an appropriate error message here?

> - }
> + break;
>   }
>   }
>   FREELIST(keys);
> -- 
> 2.12.2

Re: [pacman-dev] [PATCH] Add [ignored] to -Qu output for packages in repos that are not Usage = Upgrade

[Andrew Gregory]

On 01/04/19 at 02:21pm, Allan McRae wrote:
> The behaviour of "pacman -Qu" was very strange...  It would only consider
> packages from repos with Usage = Search (or All), and ignore those with
> Usage = Sync, Install or Upgrade.
> 
> This is because alpm_sync_newversion() used ALPM_DB_USAGE_SEARCH for its
> filtering. Given this function is documented (at least in the source) to
> "Check for new version of pkg in sync repos", I would expect that to look at
> all repos. However, just changing this parameter, would result in a fairly
> silent change in behaviour of this function.  To counter that, add a parameter
> to the function that tells it which databases usage levels to consider.
> 
> Finally, list all available updates in -Qu output, but include [ignored] 
> beside
> those that will not be updated in a -Su operation due to thier repo Usage
> value (in addition to those that are Ignored).
> 
> Fixes FS#59854.
> 
> With thanks to the following who provided initial patches to print [ignored] 
> on
> -Qu operations, which highlighted the larger problem:
> morganamilo 
> Michael Straube 
> 
> Signed-off-by: Allan McRae 
> ---
> 
> In comments on earlier patches, there was debate about what the
> alpm_sync_newversion() is doing.   Although I expect users of this function
> to likely always pass ALPM_DB_USAGE_ALL, I think changing the function
> signature is the safest way to handle this change.
> 
> Comments?

I was initially on board with this approach, but the more I think
about it, the more convinced I am that having a function that takes an
explicit list of db's filter them based on usage is a fundamentally
bad design.  As I said in the initial discussion, these are low-level
functions that may be used in different contexts, in which case
different usage filters would be appropriate.  For example, this patch
uses newversion with USAGE_ALL because it's being used for purely
informational purposes.  Manjaro's syncfirst patch, however, uses it
to actually prepare an upgrade, making USAGE_UPGRADE the correct
filter.

But, requiring the caller to provide a usage everywhere would defeat
the purpose of making repo usages a back-end feature.  We might as
well just have the caller do the filtering.  Given that there are only
a handful of usage levels, the filtered lists could even be cached,
completely saving us the hassle of iterating over db's that won't get
used.

I think the best way to fulfill the original goal of adding repo
usages would be to move the usage filtering to higher level functions
that have more narrowly defined purposes and take a handle rather than
a db list.  This would allow callers that just want to "do the right
thing" to not have to worry about usages while allowing callers with
more specific needs to use the low-level functions without having to
worry about what extra filtering alpm might be doing behind the
scenes.  It would also result in a cleaner API that doesn't have
functions ignoring databases they were explicitly told to use.

[pacman-dev] [GIT] The official pacman repository annotated tag, v5.1.2, created. v5.1.2

[Andrew Gregory]

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 annotated tag, v5.1.2 has been created
at  ad1baa4100a996a8c957fb42299ed194385114f7 (tag)
   tagging  0b36d8781783d7f4a0086e12eb7cae542e7f6bd7 (commit)
  replaces  v5.1.1
 tagged by  Andrew Gregory
on  Tue Dec 25 01:57:46 2018 -0800

- Log -
v5.1.2
-BEGIN PGP SIGNATURE-

iQEzBAABCAAdFiEEuBUbEXA3eBCVUUynu9/8kjBrESEFAlwh/xoACgkQu9/8kjBr
ESE+Twf/eRh/JmqTEhHuU296Kc1w9YcwBUgJE1cYVDm3rAaKsCxD8vQzExS6PP6f
MkNWymbx7YjUb8CdHv47W3YONfDmbw55Io46D4jOLA0yrUZhP7HfY+L/jcHCAVi6
vsPG98QHWxWfSJOaXqEZSWw7Kjfgj8+MAyrDlm718mq2TBibcUHz2ElH2i78m/y1
rH1MVL/LAwONtQHYLXHxmepQ92eaoUbdKJ3I5aRP3im+uFFwrkRbHFxYolFFFMKt
gMOdgDoJlu9YrmiQWfy0slfNCT3oGVJdoSULhtHmTq/fKs451xc7KN0egZnBOm5d
WOKTY2fBq9G4IxcqMk/TVqmvYIkSVw==
=IvGY
-END PGP SIGNATURE-

Allan McRae (1):
  Pull updated translations from Transifex

Andrew Gregory (5):
  reset signal handlers before running scripts/hooks
  handle EINTR while polling scripts/hooks
  always allow explicit empty siglevel for sync dbs
  update NEWS for v5.1.2
  Release v5.1.2

Eli Schwartz (2):
  pacman: check versioned optdepends in -Qi operation
  libmakepkg/lint_config: fix lint_variable actually running the PKGBUILD 
lint

Michael Straube (1):
  libalpm/dload.c: add case for CURLE_COULDNT_RESOLVE_HOST

Olivier Brunel (2):
  alpm: Do not raise SIGINT when filesize goes over limit
  alpm: Fix SIGINT handling re: aborting download

morganamilo (1):
  pacman-conf: add missing DisableDownloadTimeout

---


hooks/post-receive
-- 
The official pacman repository

[pacman-dev] [GIT] The official pacman repository branch, release/5.1.x, updated. v5.1.2

[Andrew Gregory]

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, release/5.1.x has been updated
   via  0b36d8781783d7f4a0086e12eb7cae542e7f6bd7 (commit)
   via  f8c73464c9cf86d6b917542585d74514f150c67b (commit)
   via  48a6adee3e63d335ac431ff6a67351988e574b79 (commit)
  from  cfa1e8b5e2b2fdf66fe41c72d04a8bbc23c28027 (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 0b36d8781783d7f4a0086e12eb7cae542e7f6bd7
Author: Andrew Gregory 
Date:   Sun Dec 23 14:33:01 2018 -0800

Release v5.1.2

Signed-off-by: Andrew Gregory 

commit f8c73464c9cf86d6b917542585d74514f150c67b
Author: Andrew Gregory 
Date:   Sun Dec 23 14:30:49 2018 -0800

update NEWS for v5.1.2

Signed-off-by: Andrew Gregory 

commit 48a6adee3e63d335ac431ff6a67351988e574b79
Author: Andrew Gregory 
Date:   Sat Nov 24 15:56:12 2018 -0800

always allow explicit empty siglevel for sync dbs

An empty siglevel does not do any signature verification which is
exactly what we want when compiled without gpg support.  This is already
allowed in other parts of the codebase and required for the test suite
to pass when compiled without gpg support.

Fixes: FS#60880

Signed-off-by: Andrew Gregory 
Signed-off-by: Allan McRae 
(cherry picked from commit 61fe73804305a8bbb434cdc245944df5284f1964)

---

Summary of changes:
 NEWS  | 15 +++
 configure.ac  |  4 ++--
 doc/index.asciidoc|  1 +
 lib/libalpm/be_sync.c |  2 +-
 4 files changed, 19 insertions(+), 3 deletions(-)


hooks/post-receive
-- 
The official pacman repository

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

[Andrew Gregory]

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  6b541404cc691b2aea3d373e663d9628fda15a2b (commit)
   via  281cdb2e1c5b706177beba4a932a75322c1c2351 (commit)
  from  b109d7096b3f3f3244b7bb3796be658393eff98a (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 6b541404cc691b2aea3d373e663d9628fda15a2b
Author: Andrew Gregory 
Date:   Sun Dec 23 14:30:49 2018 -0800

update NEWS for v5.1.2

Signed-off-by: Andrew Gregory 
(cherry picked from commit f8c73464c9cf86d6b917542585d74514f150c67b)

commit 281cdb2e1c5b706177beba4a932a75322c1c2351
Author: Andrew Gregory 
Date:   Sat Jun 3 22:15:22 2017 -0400

update NEWS for v5.0.2

Signed-off-by: Andrew Gregory 
(cherry picked from commit fdf53393bc996a514bbfc9fcd6ea19a8bb2f02ed)

---

Summary of changes:
 NEWS   | 33 +
 doc/index.asciidoc |  2 ++
 2 files changed, 35 insertions(+)


hooks/post-receive
-- 
The official pacman repository

[pacman-dev] [PATCH 3/4] tap.py: add skip_all function

[Andrew Gregory]

Signed-off-by: Andrew Gregory 
---
 test/pacman/tap.py | 5 +
 1 file changed, 5 insertions(+)

diff --git a/test/pacman/tap.py b/test/pacman/tap.py
index 6a3bee0d..eb0747be 100644
--- a/test/pacman/tap.py
+++ b/test/pacman/tap.py
@@ -21,6 +21,11 @@
 def _output(msg):
 print("%s%s" % (""*level, str(msg).replace("\n", "\\n")))
 
+def skip_all(description=""):
+if description:
+description = " # " + description
+_output("1..0%s" % (description))
+
 def ok(ok, description=""):
 global count, failed
 count += 1
-- 
2.19.2

[pacman-dev] [PATCH 2/4] remove unused test summary code

[Andrew Gregory]

Unused since 12e00af5315135a29a66c9aaa01e141a32d4634b

Signed-off-by: Andrew Gregory 
---
 test/pacman/pactest.py | 3 ---
 test/pacman/pmenv.py   | 5 -
 2 files changed, 8 deletions(-)

diff --git a/test/pacman/pactest.py b/test/pacman/pactest.py
index 85cce6a1..a563fa72 100755
--- a/test/pacman/pactest.py
+++ b/test/pacman/pactest.py
@@ -160,6 +160,3 @@ def create_parser():
 shutil.rmtree(root_path)
 else:
 tap.diag("pacman testing root saved: %s" % root_path)
-
-if env.failed > 0:
-sys.exit(1)
diff --git a/test/pacman/pmenv.py b/test/pacman/pmenv.py
index f1d626d3..9e6b5625 100644
--- a/test/pacman/pmenv.py
+++ b/test/pacman/pmenv.py
@@ -24,12 +24,7 @@
 class pmenv(object):
 """Environment object
 """
-
 testcases = []
-passed = 0
-failed = 0
-expectedfail = 0
-unexpectedpass = 0
 
 def __init__(self, root = "root"):
 self.root = os.path.abspath(root)
-- 
2.19.2

[pacman-dev] [PATCH 4/4] allow tests for disabled features to be skipped

[Andrew Gregory]

Previously, pacman's test suite would fail when compiled without
signature support.

Adds a require_capability method to pmtest objects.  Currently
recognized values are 'gpg', 'curl', and 'nls'; although only gpg is
used presently.  Missing features are indicated by running pactest with
one of the --without- options.

This modifies pmenv to run each case as independent tests.  Previously,
a single pmenv could run multiple tests, combining there output into
a single TAP stream but making it impossible to properly skip an entire
test case.  This change does not affect running pactest.py with a single
test (as both autotools and meson do), but will affect anybody manually
running pactest.py with multiple tests at once.

Signed-off-by: Andrew Gregory 
---
 Makefile.am  |  6 ++
 test/pacman/meson.build  | 25 -
 test/pacman/pactest.py   | 12 
 test/pacman/pmenv.py | 26 +-
 test/pacman/pmtest.py|  8 +++-
 test/pacman/tests/sign001.py |  1 +
 6 files changed, 59 insertions(+), 19 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 1e9ee152..98ad8b62 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -51,6 +51,12 @@ AM_PY_LOG_FLAGS = \
--ldconfig $(LDCONFIG) \
--bindir $(top_builddir)/src/pacman \
--bindir $(top_builddir)/scripts
+if !HAVE_LIBGPGME
+AM_PY_LOG_FLAGS += --without-gpg
+endif
+if !HAVE_LIBCURL
+AM_PY_LOG_FLAGS += --without-curl
+endif
 
 # create the pacman DB, cache, makepkg-template and system hook directories 
upon install
 install-data-local:
diff --git a/test/pacman/meson.build b/test/pacman/meson.build
index 3ad1fcd0..5edbbd43 100644
--- a/test/pacman/meson.build
+++ b/test/pacman/meson.build
@@ -342,19 +342,26 @@ foreach testobj : pacman_tests
   input = testobj.get('name')
   test_name = input.split('/')[1]
   should_fail = testobj.get('should_fail', false)
+  args = [
+join_paths(meson.source_root(), 'build-aux/tap-driver.py'),
+join_paths(meson.current_source_dir(), 'pactest.py'),
+'--scriptlet-shell', get_option('scriptlet-shell'),
+'--bindir', meson.build_root(),
+'--ldconfig', LDCONFIG,
+'--verbose',
+join_paths(meson.current_source_dir(), input)
+  ]
+  if not conf.get('HAVE_LIBCURL')
+args += '--without-curl'
+  endif
+  if not conf.get('HAVE_LIBGPGME')
+args += '--without-gpg'
+  endif
 
   test(
 test_name,
 PYTHON,
-args : [
-  join_paths(meson.source_root(), 'build-aux/tap-driver.py'),
-  join_paths(meson.current_source_dir(), 'pactest.py'),
-  '--scriptlet-shell', get_option('scriptlet-shell'),
-  '--bindir', meson.build_root(),
-  '--ldconfig', LDCONFIG,
-  '--verbose',
-  join_paths(meson.current_source_dir(), input)
-],
+args : args,
 depends : [pacman_bin],
 should_fail : should_fail)
 endforeach
diff --git a/test/pacman/pactest.py b/test/pacman/pactest.py
index a563fa72..2d1edf1d 100755
--- a/test/pacman/pactest.py
+++ b/test/pacman/pactest.py
@@ -77,6 +77,15 @@ def create_parser():
 parser.add_option("--bindir", type = "string",
   dest = "bindir", action = "append",
   help = "specify location of binaries")
+parser.add_option("--without-gpg", action = "store_true",
+  dest = "missing_gpg", default = False,
+  help = "skip gpg-related tests")
+parser.add_option("--without-curl", action = "store_true",
+  dest = "missing_curl", default = False,
+  help = "skip downloader-related tests")
+parser.add_option("--without-nls", action = "store_true",
+  dest = "missing_nls", default = False,
+  help = "skip translation-related tests")
 parser.add_option("--keep-root", action = "store_true",
   dest = "keeproot", default = False,
   help = "don't remove the generated pacman root 
filesystem")
@@ -137,6 +146,9 @@ def create_parser():
 env.pacman["manual-confirm"] = opts.manualconfirm
 env.pacman["scriptlet-shell"] = opts.scriptletshell
 env.pacman["ldconfig"] = opts.ldconfig
+env.config["gpg"] = not opts.missing_gpg
+env.config["nls"] = not opts.missing_nls
+env.config["curl"] = not opts.missing_curl
 
 try:
 for i in args:
diff --git a/test/pacman/pmenv.py b/test/pacman/pmenv.py
index 9e6b5625..71001dbf 100644
--- a/test/pacman/pmenv.py
+++ b/test/pacman/pmenv.py
@@ -36,6 +36,11 @@ def __init__(self, root = "root"):
 "valgrind": 0,
 "nolog": 0
 }
+  

[pacman-dev] [PATCH 1/4] silence warning when built without curl

[Andrew Gregory]

Signed-off-by: Andrew Gregory 
---
 lib/libalpm/handle.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
index f912d2f5..c491d87c 100644
--- a/lib/libalpm/handle.c
+++ b/lib/libalpm/handle.c
@@ -876,6 +876,8 @@ int SYMEXPORT 
alpm_option_set_disable_dl_timeout(alpm_handle_t *handle,
CHECK_HANDLE(handle, return -1);
 #ifdef HAVE_LIBCURL
handle->disable_dl_timeout = disable_dl_timeout;
+#else
+   (void)disable_dl_timeout; /* silence unused variable warnings */
 #endif
return 0;
 }
-- 
2.19.2

Re: [pacman-dev] [PATCH] Move skipping of duplicate sync/remove targets into libalpm

[Andrew Gregory]

On 12/13/18 at 11:44am, Michael Straube wrote:
> sync:
> As pointed out by Andrew Gregory there could be an error when adding
> duplicates if they are two separate packages with the same name. Add a
> check in alpm_add_pkg() to test whether the duplicate is actually the
> same package, and if so, log a debug message and return success to skip
> the package. If the duplicate is a different package return
> ALPM_ERR_TRANS_DUP_TARGET and treat that error just like any other error
> in pacman.
> 
> remove:
> Change alpm_remove_pkg() to just log a debug message and return success
> to skip duplicates. Remove the handling of ALPM_ERR_TRANS_DUP_TARGET in
> pacman.
> 
> Also fixes FS#49377.
> 
> Suggested-by: Andrew Gregory 
> Signed-off-by: Michael Straube 
> ---
>  lib/libalpm/add.c|  9 -
>  lib/libalpm/remove.c |  3 ++-
>  src/pacman/remove.c  | 10 ++
>  src/pacman/sync.c| 11 ++-
>  4 files changed, 14 insertions(+), 19 deletions(-)

Looks good, just a couple minor notes.

> diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
> index e415bb17..f357c076 100644
> --- a/lib/libalpm/add.c
> +++ b/lib/libalpm/add.c
> @@ -53,6 +53,7 @@ int SYMEXPORT alpm_add_pkg(alpm_handle_t *handle, 
> alpm_pkg_t *pkg)
>   const char *pkgname, *pkgver;
>   alpm_trans_t *trans;
>   alpm_pkg_t *local;
> + alpm_pkg_t *dup;
>  
>   /* Sanity checks */
>   CHECK_HANDLE(handle, return -1);
> @@ -70,7 +71,13 @@ int SYMEXPORT alpm_add_pkg(alpm_handle_t *handle, 
> alpm_pkg_t *pkg)
>  
>   _alpm_log(handle, ALPM_LOG_DEBUG, "adding package '%s'\n", pkgname);
>  
> - if(alpm_pkg_find(trans->add, pkgname)) {
> + dup = alpm_pkg_find(trans->add, pkgname);
> + if(dup) {

This isn't an official style rule, but for short assignments that
immediately get tested, we typically put them directly in the
conditional.

> + if(dup == pkg) {
> + _alpm_log(handle, ALPM_LOG_DEBUG, _("skipping duplicate 
> target: %s\n"), pkgname);

Debug messages shouldn't be translated.

> + return 0;
> + }
> + /* error for separate packages with the same name */
>   RET_ERR(handle, ALPM_ERR_TRANS_DUP_TARGET, -1);
>   }
>  
> diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c
> index 78ca5be7..ffdc0da5 100644
> --- a/lib/libalpm/remove.c
> +++ b/lib/libalpm/remove.c
> @@ -74,7 +74,8 @@ int SYMEXPORT alpm_remove_pkg(alpm_handle_t *handle, 
> alpm_pkg_t *pkg)
>   pkgname = pkg->name;
>  
>   if(alpm_pkg_find(trans->remove, pkgname)) {
> - RET_ERR(handle, ALPM_ERR_TRANS_DUP_TARGET, -1);
> + _alpm_log(handle, ALPM_LOG_DEBUG, _("skipping duplicate target: 
> %s\n"), pkgname);

Ditto.

> + return 0;
>   }
>  
>   _alpm_log(handle, ALPM_LOG_DEBUG, "adding package %s to the transaction 
> remove list\n",
> diff --git a/src/pacman/remove.c b/src/pacman/remove.c
> index a2269ed8..9d44cf53 100644
> --- a/src/pacman/remove.c
> +++ b/src/pacman/remove.c
> @@ -44,14 +44,8 @@ static int remove_target(const char *target)
>   if((pkg = alpm_db_get_pkg(db_local, target)) != NULL) {
>   if(alpm_remove_pkg(config->handle, pkg) == -1) {
>   alpm_errno_t err = alpm_errno(config->handle);
> - if(err == ALPM_ERR_TRANS_DUP_TARGET) {
> - /* just skip duplicate targets */
> - pm_printf(ALPM_LOG_WARNING, _("skipping target: 
> %s\n"), target);
> - return 0;
> - } else {
> - pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", target, 
> alpm_strerror(err));
> - return -1;
> - }
> + pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", target, 
> alpm_strerror(err));
> + return -1;
>   }
>   config->explicit_removes = 
> alpm_list_add(config->explicit_removes, pkg);
>   return 0;
> diff --git a/src/pacman/sync.c b/src/pacman/sync.c
> index 57677a42..2406fed5 100644
> --- a/src/pacman/sync.c
> +++ b/src/pacman/sync.c
> @@ -521,15 +521,8 @@ static int process_pkg(alpm_pkg_t *pkg)
>  
>   if(ret == -1) {
>   alpm_errno_t err = alpm_errno(config->handle);
> - if(err == ALPM_ERR_TRANS_DUP_TARGET) {
> - /* just skip duplicate targets */
> - pm_printf(ALPM_LOG_WARNING, _("skipping target: %s\n"), 
> alpm_pkg_ge

Re: [pacman-dev] [PATCH] pacman: better warning message when skipping duplicate targets

[Andrew Gregory]

On 12/11/18 at 06:14pm, Michael Straube wrote:
> Am 09.12.18 um 19:47 schrieb Andrew Gregory:
> > On 12/09/18 at 06:31pm, Michael Straube wrote:
> > > Change the warning message to reflect the reason when skipping duplicate
> > > targets. skipping target -> skipping duplicate target
> > > 
> > > FS#49377
> > > 
> > > Signed-off-by: Michael Straube 
> > > ---
> > >   src/pacman/remove.c | 2 +-
> > >   src/pacman/sync.c   | 2 +-
> > >   2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > Should we just remove the error altogether and move the message to
> > DEBUG?  The user doesn't need to do anything in response to it and
> > I can't think of any reason a front-end would want to actually die
> > from it.  It seems to just be useless line noise that requires
> > front-ends to check for it specifically just to ignore it.
> > 
> 
> Sounds reasonable. On the other hand I can imagine that some people
> would complain that too much from what's going on is hidden from the
> user. What do others think?
> 
> 
> P.S.: You mean?
> pm_printf(ALPM_LOG_DEBUG, _("skipping duplicate target: %s\n"), target);

I realized after sending this that adding a duplicate could actually
be an error if they are two separate packages with the same name.  So,
I'm going to say to add a check in add_pkg for whether the duplicate
is actually the same package (simple pointer cmp) and, if they are the
same, log a debug message and return success, if they are different
return the error as we do now.  process_pkg in pacman should then
treat that error just like any other instead of printing a warning and
continuing on.  alpm_remove_pkg should just log the debug message and
return success.

apg

Re: [pacman-dev] [PATCH] pacman: better warning message when skipping duplicate targets

[Andrew Gregory]

On 12/09/18 at 06:31pm, Michael Straube wrote:
> Change the warning message to reflect the reason when skipping duplicate
> targets. skipping target -> skipping duplicate target
> 
> FS#49377
> 
> Signed-off-by: Michael Straube 
> ---
>  src/pacman/remove.c | 2 +-
>  src/pacman/sync.c   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Should we just remove the error altogether and move the message to
DEBUG?  The user doesn't need to do anything in response to it and
I can't think of any reason a front-end would want to actually die
from it.  It seems to just be useless line noise that requires
front-ends to check for it specifically just to ignore it.

Re: [pacman-dev] [PATCH] check localdb before upgrading package

[Andrew Gregory]

On 11/27/18 at 09:24pm, Allan McRae wrote:
> On 17/11/18 1:47 pm, Andrew Gregory wrote:
> > Commit 2ee7a8d89ad693617307260604e1d58757fd2978 replaced a manual check
> > for a local package with a check for the "oldpkg" member, which gets set
> > at the beginning of the transaction.  If the package was also in the
> > remove list, such as when a package gets replaced, it would no longer be
> > in the local db and pacman would try to remove it twice, resulting in
> > superfluous error messages.
> > 
> > Fixes: FS#50875, FS#5554
> > 
> > Signed-off-by: Andrew Gregory 
> > ---
> >  lib/libalpm/add.c |  2 +-
> >  test/pacman/meson.build   |  1 +
> >  test/pacman/tests/TESTS   |  1 +
> >  .../tests/replace-and-upgrade-package.py  | 27 +++
> >  4 files changed, 30 insertions(+), 1 deletion(-)
> >  create mode 100644 test/pacman/tests/replace-and-upgrade-package.py
> > 
> 
> Thanks!  Committed with bug number typo fixed.

Did this patch get lost or am I blind?

[pacman-dev] [PATCH 2/3] always allow explicit empty siglevel for sync dbs

[Andrew Gregory]

An empty siglevel does not do any signature verification which is
exactly what we want when compiled without gpg support.  This is already
allowed in other parts of the codebase and required for the test suite
to pass when compiled without gpg support.

Fixes: FS#60880

Signed-off-by: Andrew Gregory 
---

Signature-related tests still fail without gpg support for obvious
reasons, but the rest should pass after this.

 lib/libalpm/be_sync.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 6adf1cd9..4a4be548 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -787,7 +787,7 @@ alpm_db_t *_alpm_db_register_sync(alpm_handle_t *handle, 
const char *treename,
_alpm_log(handle, ALPM_LOG_DEBUG, "registering sync database '%s'\n", 
treename);
 
 #ifndef HAVE_LIBGPGME
-   if(level != ALPM_SIG_USE_DEFAULT) {
+   if(level != 0 && level != ALPM_SIG_USE_DEFAULT) {
RET_ERR(handle, ALPM_ERR_MISSING_CAPABILITY_SIGNATURES, NULL);
}
 #endif
-- 
2.19.1

[pacman-dev] [PATCH 1/3] add specific error for missing gpg support

[Andrew Gregory]

"wrong or NULL argument passed" is a useless error for end users.

Fixes FS#60880.

Signed-off-by: Andrew Gregory 
---
 lib/libalpm/alpm.h| 4 +++-
 lib/libalpm/be_sync.c | 2 +-
 lib/libalpm/error.c   | 3 +++
 lib/libalpm/handle.c  | 6 +++---
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 2d3d198a..597e11bd 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -118,7 +118,9 @@ typedef enum _alpm_errno_t {
ALPM_ERR_LIBARCHIVE,
ALPM_ERR_LIBCURL,
ALPM_ERR_EXTERNAL_DOWNLOAD,
-   ALPM_ERR_GPGME
+   ALPM_ERR_GPGME,
+   /* Missing compile-time features */
+   ALPM_ERR_MISSING_CAPABILITY_SIGNATURES
 } alpm_errno_t;
 
 /** Returns the current error code from the handle. */
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 5009a7da..6adf1cd9 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -788,7 +788,7 @@ alpm_db_t *_alpm_db_register_sync(alpm_handle_t *handle, 
const char *treename,
 
 #ifndef HAVE_LIBGPGME
if(level != ALPM_SIG_USE_DEFAULT) {
-   RET_ERR(handle, ALPM_ERR_WRONG_ARGS, NULL);
+   RET_ERR(handle, ALPM_ERR_MISSING_CAPABILITY_SIGNATURES, NULL);
}
 #endif
 
diff --git a/lib/libalpm/error.c b/lib/libalpm/error.c
index b4fc99ae..95be9d7b 100644
--- a/lib/libalpm/error.c
+++ b/lib/libalpm/error.c
@@ -159,6 +159,9 @@ const char SYMEXPORT *alpm_strerror(alpm_errno_t err)
return _("gpgme error");
case ALPM_ERR_EXTERNAL_DOWNLOAD:
return _("error invoking external downloader");
+   /* Missing compile-time features */
+   case ALPM_ERR_MISSING_CAPABILITY_SIGNATURES:
+   return _("compiled without signature support");
/* Unknown error! */
default:
return _("unexpected error");
diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
index 2213ce53..be5666dc 100644
--- a/lib/libalpm/handle.c
+++ b/lib/libalpm/handle.c
@@ -807,7 +807,7 @@ int SYMEXPORT 
alpm_option_set_default_siglevel(alpm_handle_t *handle,
handle->siglevel = level;
 #else
if(level != 0 && level != ALPM_SIG_USE_DEFAULT) {
-   RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1);
+   RET_ERR(handle, ALPM_ERR_MISSING_CAPABILITY_SIGNATURES, -1);
}
 #endif
return 0;
@@ -827,7 +827,7 @@ int SYMEXPORT 
alpm_option_set_local_file_siglevel(alpm_handle_t *handle,
handle->localfilesiglevel = level;
 #else
if(level != 0 && level != ALPM_SIG_USE_DEFAULT) {
-   RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1);
+   RET_ERR(handle, ALPM_ERR_MISSING_CAPABILITY_SIGNATURES, -1);
}
 #endif
return 0;
@@ -851,7 +851,7 @@ int SYMEXPORT 
alpm_option_set_remote_file_siglevel(alpm_handle_t *handle,
handle->remotefilesiglevel = level;
 #else
if(level != 0 && level != ALPM_SIG_USE_DEFAULT) {
-   RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1);
+   RET_ERR(handle, ALPM_ERR_MISSING_CAPABILITY_SIGNATURES, -1);
}
 #endif
return 0;
-- 
2.19.1

[pacman-dev] [PATCH 3/3] require actual siglevel for default

[Andrew Gregory]

ALPM_SIG_USE_DEFAULT does not refer to an actual siglevel, rather it
indicates that the global default should be used in place of the
operation-specific one.  Setting this value for the global default
itself makes no sense.

Signed-off-by: Andrew Gregory 
---
 lib/libalpm/handle.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
index be5666dc..f912d2f5 100644
--- a/lib/libalpm/handle.c
+++ b/lib/libalpm/handle.c
@@ -803,10 +803,13 @@ int SYMEXPORT 
alpm_option_set_default_siglevel(alpm_handle_t *handle,
int level)
 {
CHECK_HANDLE(handle, return -1);
+   if(level == ALPM_SIG_USE_DEFAULT) {
+   RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1);
+   }
 #ifdef HAVE_LIBGPGME
handle->siglevel = level;
 #else
-   if(level != 0 && level != ALPM_SIG_USE_DEFAULT) {
+   if(level != 0) {
RET_ERR(handle, ALPM_ERR_MISSING_CAPABILITY_SIGNATURES, -1);
}
 #endif
-- 
2.19.1

[pacman-dev] [GIT] The official pacman repository branch, release/5.1.x, updated. v5.1.1-8-g3a88fcb1

[Andrew Gregory]

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, release/5.1.x has been updated
   via  3a88fcb19199c5b5fa18162f5f9dc8990311dbd2 (commit)
   via  2a7bdd3e3aabbf9fa3cc31379cadad909e6c1da3 (commit)
   via  b39a62f57551a2e23ceaa37b3fbc71a2273545e6 (commit)
   via  cad8fe2fbfc3a63554a2537ea7b5627a71453075 (commit)
   via  0dbb94538770d2ffce3709b5854d1e5e44cfc183 (commit)
   via  519685e4b1cff3df9bc389550fa6e19ee4ba8794 (commit)
   via  4fc7c1d41efcce7f85def270c0ce8edca1062fc4 (commit)
   via  5e81518ecb4ba9c8b7231e9b627f45d3abb19ce3 (commit)
  from  7e081d2adf8321f25165255fd21fab61d4055a53 (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 3a88fcb19199c5b5fa18162f5f9dc8990311dbd2
Author: Andrew Gregory 
Date:   Fri Oct 12 19:16:53 2018 -0700

handle EINTR while polling scripts/hooks

If poll() is interrupted by a signal, alpm was closing the socket it
uses for listening to script/hook output.  This would drop script output
at the least and kill the script at the worst.

Fixes FS#60396

Signed-off-by: Andrew Gregory 
Signed-off-by: Allan McRae 
(cherry picked from commit ac959bb9c6ce549047a954109ae825158855e386)

commit 2a7bdd3e3aabbf9fa3cc31379cadad909e6c1da3
Author: Andrew Gregory 
Date:   Wed Oct 3 00:42:38 2018 -0700

reset signal handlers before running scripts/hooks

Front-ends or libraries may set signals to be ignored, which gets
inherited across fork and exec.  This can cause scripts to malfunction
if they expect the signal.  To make matters worse, scripts written in
bash can't reset signals that were ignored when bash was started.

Fixes FS#56756

Signed-off-by: Andrew Gregory 
Signed-off-by: Allan McRae 
(cherry picked from commit 9886566abb375043740167ce5066f1a186c71176)

commit b39a62f57551a2e23ceaa37b3fbc71a2273545e6
Author: Olivier Brunel 
Date:   Wed Oct 17 17:11:01 2018 +0200

alpm: Fix SIGINT handling re: aborting download

Upon receiving SIGINT a flag is set to abort the (curl) download.
However, since it was never reset/initialized, if a front-end doesn't
actually exit on SIGINT, and later tries any operation that needs to
perform a new download, said download would always get aborted right
away due to the flag not having been reset.

(cherry picked from commit ffde85aadfe0e08fb710102d0a547335e9d1a200)

commit cad8fe2fbfc3a63554a2537ea7b5627a71453075
Author: Olivier Brunel 
Date:   Tue Oct 9 18:29:05 2018 +0200

alpm: Do not raise SIGINT when filesize goes over limit

Variable dload_interrupted is used both to abort a download because
SIGINT was caught, and when a file limit is reached. But raising SIGINT
is only meant to happen in the first case.

Signed-off-by: Olivier Brunel 
(cherry picked from commit d96d0ffe7c88d9521a9e6cdd65939e9a20733cdf)

commit 0dbb94538770d2ffce3709b5854d1e5e44cfc183
Author: Michael Straube 
Date:   Sun Jun 10 18:58:34 2018 +0200

libalpm/dload.c: add case for CURLE_COULDNT_RESOLVE_HOST

Add a case for curl error 'Could not resolve host'.
An attempt to fix FS#48285.

Signed-off-by: Michael Straube 
Signed-off-by: Allan McRae 
(cherry picked from commit 9e960d9d5a735bbc7d418f2ad81d3f3e92d99968)

commit 519685e4b1cff3df9bc389550fa6e19ee4ba8794
Author: Eli Schwartz 
Date:   Tue Sep 4 15:17:54 2018 -0400

libmakepkg/lint_config: fix lint_variable actually running the PKGBUILD lint

Due to a copy-paste error when initially implementing this, it actually
uses a duplicate function name, usually resulting in lint_pkgbuild
overwriting the function definition.

Then the PKGBUILD lint gets run twice, one time before the PKGBUILD is
even sourced -- to potentially surprising results, like erroring out on
a pre-existing shell definition that doesn't match our expectations.

Seen in the wild with lint_config triggering an error for
'declare -x arch="foo"'

Signed-off-by: Eli Schwartz 
Signed-off-by: Allan McRae 
(cherry picked from commit 2bec380e108536f5e5f728ef66223ed3fabf5ab1)

commit 4fc7c1d41efcce7f85def270c0ce8edca1062fc4
Author: Eli Schwartz 
Date:   Tue Sep 18 10:08:37 2018 -0400

pacman: check versioned optdepends in -Qi operation

Fixes FS#60106

Signed-off-by: Eli Schwartz 
Signed-off-by: Allan McRae 
(cherry picked from commit 3318039e3b1530396b0e3ced49ea6fe5b6ea00c5)

commit 5e81518ecb4ba9c8b7231e9b627f45d3abb19ce3
Author: morganamilo 
Date:   Mon Sep 10 22:41:07 2018 +0100

pacm

Re: [pacman-dev] [PATCH] check localdb before upgrading package

[Andrew Gregory]

On 11/17/18 at 08:02pm, Eli Schwartz wrote:
> On 11/16/18 10:47 PM, Andrew Gregory wrote:
> > Commit 2ee7a8d89ad693617307260604e1d58757fd2978 replaced a manual check
> > for a local package with a check for the "oldpkg" member, which gets set
> > at the beginning of the transaction.  If the package was also in the
> > remove list, such as when a package gets replaced, it would no longer be
> > in the local db and pacman would try to remove it twice, resulting in
> > superfluous error messages.
> > 
> > Fixes: FS#50875, FS#5554
> Is this second one, FS#55534?

Yes.

[pacman-dev] [PATCH] add missing tests to meson.build

[Andrew Gregory]

Signed-off-by: Andrew Gregory 
---
 test/pacman/meson.build | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/pacman/meson.build b/test/pacman/meson.build
index 90885015..3ad1fcd0 100644
--- a/test/pacman/meson.build
+++ b/test/pacman/meson.build
@@ -158,6 +158,8 @@ pacman_tests = [
 'should_fail': true },
   { 'name': 'tests/scriptlet001.py' },
   { 'name': 'tests/scriptlet002.py' },
+  { 'name': 'tests/scriptlet-signal-handling.py' },
+  { 'name': 'tests/scriptlet-signal-reset.py' },
   { 'name': 'tests/sign001.py' },
   { 'name': 'tests/sign002.py' },
   { 'name': 'tests/skip-remove-with-glob-chars.py' },
-- 
2.19.1

[pacman-dev] [PATCH] common/ini: remove unnecessary alpm include

[Andrew Gregory]

Signed-off-by: Andrew Gregory 
---
 src/common/ini.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/common/ini.c b/src/common/ini.c
index 8e646528..30f1fac8 100644
--- a/src/common/ini.c
+++ b/src/common/ini.c
@@ -19,10 +19,9 @@
 
 #include 
 #include 
+#include 
 #include  /* strdup */
 
-#include 
-
 #include "ini.h"
 #include "util-common.h"
 
-- 
2.19.1

[pacman-dev] [PATCH] check localdb before upgrading package

[Andrew Gregory]

Commit 2ee7a8d89ad693617307260604e1d58757fd2978 replaced a manual check
for a local package with a check for the "oldpkg" member, which gets set
at the beginning of the transaction.  If the package was also in the
remove list, such as when a package gets replaced, it would no longer be
in the local db and pacman would try to remove it twice, resulting in
superfluous error messages.

Fixes: FS#50875, FS#5554

Signed-off-by: Andrew Gregory 
---
 lib/libalpm/add.c |  2 +-
 test/pacman/meson.build   |  1 +
 test/pacman/tests/TESTS   |  1 +
 .../tests/replace-and-upgrade-package.py  | 27 +++
 4 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100644 test/pacman/tests/replace-and-upgrade-package.py

diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index 5a3e06b8..e415bb17 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -427,7 +427,7 @@ static int commit_single_pkg(alpm_handle_t *handle, 
alpm_pkg_t *newpkg,
ASSERT(trans != NULL, return -1);
 
/* see if this is an upgrade. if so, remove the old package first */
-   if((oldpkg = newpkg->oldpkg)) {
+   if(_alpm_db_get_pkgfromcache(db, newpkg->name) && (oldpkg = 
newpkg->oldpkg)) {
int cmp = _alpm_pkg_compare_versions(newpkg, oldpkg);
if(cmp < 0) {
log_msg = "downgrading";
diff --git a/test/pacman/meson.build b/test/pacman/meson.build
index dbdb429e..90885015 100644
--- a/test/pacman/meson.build
+++ b/test/pacman/meson.build
@@ -148,6 +148,7 @@ pacman_tests = [
   { 'name': 'tests/remove060.py' },
   { 'name': 'tests/remove070.py' },
   { 'name': 'tests/remove071.py' },
+  { 'name': 'tests/replace-and-upgrade-package.py' },
   { 'name': 'tests/replace100.py' },
   { 'name': 'tests/replace101.py' },
   { 'name': 'tests/replace102.py' },
diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS
index dc0b4ec3..74c8f674 100644
--- a/test/pacman/tests/TESTS
+++ b/test/pacman/tests/TESTS
@@ -144,6 +144,7 @@ TESTS += test/pacman/tests/remove052.py
 TESTS += test/pacman/tests/remove060.py
 TESTS += test/pacman/tests/remove070.py
 TESTS += test/pacman/tests/remove071.py
+TESTS += test/pacman/tests/replace-and-upgrade-package.py
 TESTS += test/pacman/tests/replace100.py
 TESTS += test/pacman/tests/replace101.py
 TESTS += test/pacman/tests/replace102.py
diff --git a/test/pacman/tests/replace-and-upgrade-package.py 
b/test/pacman/tests/replace-and-upgrade-package.py
new file mode 100644
index ..ef2152a7
--- /dev/null
+++ b/test/pacman/tests/replace-and-upgrade-package.py
@@ -0,0 +1,27 @@
+self.description = 'Simultaneously replace and upgrade a package'
+
+# replacement package
+sp1 = pmpkg('foo1', '1-2')
+sp1.replaces = ['foo=1-1']
+sp1.conflicts = ['foo=1-1']
+self.addpkg2db('sync', sp1)
+
+# upgrade package
+sp2 = pmpkg('foo', '2-1')
+self.addpkg2db('sync', sp2)
+
+# depend on upgraded package to pull it into transaction
+sp3 = pmpkg('bar', '1-1')
+sp3.depends = [ 'foo=2-1' ]
+self.addpkg2db('sync', sp3)
+
+lp = pmpkg('foo', '1-1')
+self.addpkg2db('local', lp)
+
+self.args = "-Su bar"
+
+self.addrule('PACMAN_RETCODE=0')
+self.addrule('PKG_VERSION=foo1|1-2')
+self.addrule('PKG_VERSION=foo|2-1')
+self.addrule('PKG_VERSION=bar|1-1')
+self.addrule('!PACMAN_OUTPUT=error')
-- 
2.19.1

Re: [pacman-dev] [PATCH] Add meson.build files to build with meson

[Andrew Gregory]

On 11/01/18 at 08:51pm, Dave Reisner wrote:
> On Thu, Nov 01, 2018 at 01:03:27AM -0700, Andrew Gregory wrote:
> > On 10/21/18 at 05:46pm, Dave Reisner wrote:

...

> > > +libcommon = static_library(
> > > +  'common',
> > > +  libcommon_sources,
> > > +  install : false)
> > 
> > It's a mistake, but common/ini.c currently includes alpm.h, which
> > grabs the system alpm.h, or dies if it's not installed, because this
> > doesn't link_with libalpm.  I'll send a patch to fix this particular
> > error, but I can imagine this sort of subtle error creeping in again.
> > Should we proactively link_with libalpm to prevent this from
> > recurring?
> 
> I get what you're saying about ini.c wrongly including alpm.h, but I'm
> not sure I follow about linking with libalpm. Shouldn't the includes be
> fixed such that the inclusion of alpm.h comes from lib/libalpm rather
> than /usr/include? I'm not clear on what linking with the local libalpm
> accomplishes other than being an unnecessary dependency.

I've still not played with meson enough to fully understand exactly
how it works.  The use of link_with was just to get meson to use
lib/libalpm as an include dir.  If there's a better way to do that,
great, I just want to make sure that if a common file includes alpm.h
in the future, it doesn't sneakily use the system copy.

Re: [pacman-dev] [PATCH] Add meson.build files to build with meson

[Andrew Gregory]

On 10/21/18 at 05:46pm, Dave Reisner wrote:

-- >8 -- (lots of words)

> diff --git a/meson.build b/meson.build
> new file mode 100644
> index ..3f9b2ae0
> --- /dev/null
> +++ b/meson.build
> @@ -0,0 +1,487 @@

-- >8 -- (many more words)

> +PYTHON = find_program('python')

This should look for python3, should it not?

-- >8 -- (I really hope this was mostly copy-paste)

> +libcommon = static_library(
> +  'common',
> +  libcommon_sources,
> +  install : false)

It's a mistake, but common/ini.c currently includes alpm.h, which
grabs the system alpm.h, or dies if it's not installed, because this
doesn't link_with libalpm.  I'll send a patch to fix this particular
error, but I can imagine this sort of subtle error creeping in again.
Should we proactively link_with libalpm to prevent this from
recurring?

-- >8 -- (seriously, this patch is huge)

> diff --git a/test/pacman/meson.build b/test/pacman/meson.build
> new file mode 100644
> index ..dbdb429e
> --- /dev/null
> +++ b/test/pacman/meson.build
> @@ -0,0 +1,357 @@
> +pacman_tests = [
> +  { 'name': 'tests/backup001.py' },

Having the test list and expected success/failure duplicated here is
almost certain to lead to meson and autotools getting out of sync.
Can/should we dynamically create this list at least for as long as
we're maintaining both build systems?

[pacman-dev] [GIT] The official pacman repository annotated tag, v3.0.0-rc1, updated. v3.0.0-rc1

[Andrew Gregory]

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 annotated tag, v3.0.0-rc1 has been updated
to  bdb94daf7256d315c39ce73b78ffcf2c8b5a77cf (tag)
  from  59ae306a5453e0113ffba8395db9df1b5507c650 (which is now obsolete)
   tagging  11fbc595d48d55be929e259285bb65e53ef33007 (commit)
 tagged by  Andrew Gregory
on  Sat Oct 20 12:47:37 2018 -0700

- Log -
replaces broken tag 59ae306a5453e0113ffba8395db9df1b5507c650
-BEGIN PGP SIGNATURE-

iQEzBAABCAAdFiEEuBUbEXA3eBCVUUynu9/8kjBrESEFAlvLhlkACgkQu9/8kjBr
ESFf1AgAn3mykXPagGnkXeunD9xjXmzo4Fnx7OLbOCBzBL0nZATLUjAyu0j26HO7
PaScKOuSOc5m9WHYdmbqQy8fIsu1mI9jyNC0q1iTCpX54DCp1RZSmMIHlO7wgHCJ
kMHVHfmnt5Zz7Y9bmYm0MSbLPklbUzcSeOh1DzgIERsJD2aCJ3Dwstfpg9uKTGty
jM2n3ZhB5WSw4Sz6cKt0eH+y64YKkwvQVTNoMS7kgQBXrEHITh4gtmDeEz4FcYAT
OEJc7xXFQFYbblcpAMeYKRfyjN54F5fkXHj7jWijBSa4LBOlwTl6UGDZxKXnjXR1
Z6ZBWoROrhOs3ASDlubfFIGPzC3Bsw==
=z+VF
-END PGP SIGNATURE-

Aaron Griffin (313):
  Merging Frugalware changes - these need to be checked for instances of 
"Frugal"
  Added pactest to repository, from Aurelien Foret:
  Frugalware changes - mainly architecture updates and signed/unsigned 
changes
  Merged frugalware changes (too many to list).  Also added some config file
  Merged frugalware changes.  Added a few other minor things too, but 
there's alot
  Final frugalware changes commit
  include changes so that this compiles
  Fixed java includes - java bindings now compile
  Corrected documentation compilation - succeeds now
  Added alpm function docs along
  *** empty log message ***
  Whoops forgot some .in files
  *** empty log message ***
  Added po files... not sure if this is the right approach
  Added sha1 support (for now?)
  Another forgotten file set
  Removed convertdb
  Whoops, actually remove convertdb this time
  Added po files
  Applied changes from frugalware:
  Frugalware patch for AC_CHECK_PROGS fixups:
  *** empty log message ***
  Applied Frugalware patch from Christian Hamar alias krix 

  configure changes for java detection (require swig)
  Copyright changes
  Adjust progress bar to align with frugalware's progressbar
  Removed extra includes
  Removed three checks for po4a (patching issue)
  Yet another "added three times" issue.
  Fixed doxygen comments
  Reverted debug parameter to -1
  Added copyright holder
  Removed old db files (unused)
  From VMiklos 
  A handful of minor changes:
  Whoops, this file got left out
  Minor changes:
  More extern moving - keep extern decls in the headers makes for 
easier/better
  Fixed this test so that it succeeds - it is probably not 100% 
appropriate, but
  Fixes from frugalware: few 'typos' included while patching
  Reverted the "out of memory" error to 1 to prevent API changes.
  Added re-pacman
  Added PM_DLFNM_LEN define, via VMiklos
  Attempted fix for x86_64 - switched some unsigned char variables to ints, 
and
  Fixed library directory for bindings - we need the craptastic .libs 
libtool dir
  Moved downloaded db unpacking to the backend files, to easier allow 
conversion
  Remove generated man pages from CVS
  Fri Oct 27 21:54:32 CEST 2006  VMiklos 
  Added libfetch linux port + misc changes
  Numerous changes:
  Numerous changes:
  * Makefile changes for libfetch integration
  Forgot to cycle to next server on download error - fixed, as well as the
  Removed user-CFLAGS during a debug build.  Added -Wall to normal build(I 
like -Wall)
  Fixed pacman -U:
  Whoops forgot this
  Added a newline upong progress completion - this was accidentally left 
out causing the second progress bar to overwrite the first
  * Fixed some alpm_get_option calls (long params were used for C99 
compliance,
  * Modified some error output and logging
  Removed antesis mirror, as it no longer works
  Added mcheck support for memory debugging
  * Fixed an error message that should be a debug message
  * Improved mcheck output
  mcheck() seems to cause segfaults.  Annoying.  Switched back to useing 
mtrace() - if anything valgrind is superior to mcheck anyway
  Whoops - I fail at setenv
  Last mtrace/setenv change, I swear
  Skip root check on -Sp
  *** empty log message ***
  * Numerous mini valgrind fixes.
  * autotool fixes
  * has_archname changes
  * has_archname additions
  * Initial changes to gensync - makepkg changes were not checked in from 
another
  * Changes to some of the TODOs and a brief glance at the NEWS file...
  * integrity check loopi

[pacman-dev] [GIT] The official pacman repository annotated tag, v3.0.0-rc2, updated. v3.0.0-rc2

[Andrew Gregory]

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 annotated tag, v3.0.0-rc2 has been updated
to  fc4cd3864aae4edd5835116d9191b48f60ffa4a4 (tag)
  from  b977b9c030861decd22376e4d2a9147e830ebfae (which is now obsolete)
   tagging  9e3a1853451230d887c3c89f49f04e502fb69621 (commit)
  replaces  v3.0.0-rc1
 tagged by  Andrew Gregory
on  Sat Oct 20 12:53:22 2018 -0700

- Log -
replaces broken tag b977b9c030861decd22376e4d2a9147e830ebfae
-BEGIN PGP SIGNATURE-

iQEzBAABCAAdFiEEuBUbEXA3eBCVUUynu9/8kjBrESEFAlvLh7IACgkQu9/8kjBr
ESFJ9QgAgulpF3KQDRNRgGIpoIlblE8U0H6AziEr+lG3RRUGZs4gxJybD2aMkcrB
KSQ0L4dTHfcQ1d//d+9N2TnEf60erz+ryC+lBDzNenqZ4ocLQwiYeRCzyc8DqIoU
7Rl8mFgv0IB+4IOh5r9nPFzsGqV1UJkTjOP5guHVsy7U/1ZbXEEjhBUwkhWPfpke
3QV+7T2I2oq8aFMm6Xx30WuOIUeFpjQ+HzyI3evVWQW/B3sL5CsTm/iyav3MaAZs
rtCrGkNpfH1EI2+N8WW/sA6ZON+LpnEoUnovaYD2wU2KRlnZDc5sZt3jXp9ETBvp
poKY0EgIJulQ5US59AdIWg2zWKDfSw==
=oJsw
-END PGP SIGNATURE-

Aaron Griffin (7):
  * Fixed an issue with globbing the --test argument
  Added this test to check the XferCommand functionality
  * Fix the double package name URL when using XferCommand
  * Bug fix for makepkg dependency testing.  This requires that we
  * Updated -V output to include the 2007 copyright date.
  * Two fixes when running under a new root (-r|--root)
  * Fixed an error with 'cascade' removal due to creation of a new pmpkg_t 
struct

Dan McGee (33):
  * rankmirrors updates from Scott Horowitz .
  * Removed a mirrorlist that codemac says is quite outdated.
  * Failure to #include config.h cost us here, we lost all NLS in alpm.c.
  This commit looks much more monumental than it is. Almost all just 
#include
  * Added missing header include guards in md5.h and sha1.h.
  Trying to fix up this autotools stuff a bit more.
  * Remove sha1 checksums for now from the INTEGRITY_CHECK array.
  * Updated Italian translation
  * Fix FS #6534- unclear IgnorePkg message. Sorry translators, had to 
update
  * Sorry tranlators, another string update. .pacorig was displayed twice in
  * Slight updates of Hungarian translation
  * -Qs was returning an error if no package found, which is not the same 
behavior as -Ss.
  * Oops, that last commit had some debug stuff in it. Removed it and added
  * Updated Italian translation
  * Added a readme file to the contrib/ directory.
  * Updated Brazilian Portuguese translation
  * Updated German translation
  * Updated pot files for hopefully the last time before release.
  * Slight updates to NEWS file.
  * Mark a function as static that is only used in deptest.c.
  * Updated Italian translation
  * Added confirmation step to makepkg -C operation to ensure user is 
deleting
  * Another slight update, getting rid of -rf flags and clarifying a 
message.
  * Updated Italian translation
  * Added Russian language translation. Thanks!
  * Fix group comparison issue and associated compilation warnings by using
  * Updated Hungarian translation, thanks!
  * Updates to Hungarian translation from Nagy, mostly cleanup stuff.
  * Fixed -Qil regression, now both flags are honored. (FS #1355)
  * Removed ${CFLAGS} from Makefile.am(s) as it was causing all CFLAGS to be
  * Fix an issue where the same dependency was recorded multiple times in 
the
  * Fix wrong filesize being recorded to local DB. Reported by Andreas 
Radke.
  * Noted a misspelling for later (after we are not in string freeze).

Jürgen Hötzel (1):
  fixed string comparison callback (patch from Nagy Gabor)

---


hooks/post-receive
-- 
The official pacman repository

[pacman-dev] [GIT] The official pacman repository annotated tag, v3.0.0, updated. v3.0.0

[Andrew Gregory]

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 annotated tag, v3.0.0 has been updated
to  c914ef466fc08048b95f999cb99c4e8431548c18 (tag)
  from  c6b17b224f264973f32291ec0e4101d1a9fd0af1 (which is now obsolete)
   tagging  58fe79eef64fb87b2553e6514a47beaa6d0249c1 (commit)
  replaces  v3.0.0-rc2
 tagged by  Andrew Gregory
on  Sat Oct 20 12:45:51 2018 -0700

- Log -
replaces broken tag c6b17b224f264973f32291ec0e4101d1a9fd0af1
-BEGIN PGP SIGNATURE-

iQEzBAABCAAdFiEEuBUbEXA3eBCVUUynu9/8kjBrESEFAlvLhe8ACgkQu9/8kjBr
ESGH8QgArLmBhjuFwAaOc7Sj1THPF5BkwWqDcTQDSqMjmBdv3LskC1/ija/IsV2J
k6e9FGqHZk2H/EsSuxflpDd0M0XpLAxmcHnNe1qTPoqyCBBmNpwNZDeVE6115FBX
GX3q4mHQLmDJduhQJR0hHBvmUGXfKZoxNznHl/0MYZEosl6lAibwLAxdda1ro6RN
XXbAVc/5b8bIsqL2o1Jj66VJnOCarUQ2KoygQ8kMjVyLcxw9g4qsYsLGKzDXgUUm
5dCIskNPDdJDGCtIPNOyPQemS5BJl3e8QFwo6OP9VgeSPhKnSTqnNlxy2mDtcwOP
gPPWAN4Aya5zZa31NzifbV58GyuDhA==
=gB59
-END PGP SIGNATURE-

Aaron Griffin (14):
  * Missed a path when requiring that all paths end with / - this caused -U 
not to
  * Sebastian Sareyko
  * makepkg.conf.in: Added /usr/share/gtk-doc back into default DOC_DIRS
  James Rosten 
  Nagy Gabor 
  * Fix pacman -Se which installs depends only, as it was broken
  * Fix asking the user to upgrade when using -Sp
  Giovanni Scafora 
  * Quick fixup for the translations due to a typo fix committed earlier.
  * -Qo now properly resolves paths when finding an owner
  * Correct install scriptlet usage (reuse of handle->root when not needed)
  I was mistaken, newpkg->data IS useful here, as the scriptlet isn't in 
the FS at
  Quick pactest changes:
  * Added a mirror list for [testing] (it appears most mirrors mirror 
testing)

Dan McGee (29):
  * Resizing terminal while progress bars were displayed caused some weird
  * Small updates to my TODO file.
  * Fixed up broken translation
  * Updated Russian translation
  * Updated Hungarian translation
  * Missing a 'msg' in makepkg- pointed out by wain on Flyspray. Thanks!
  * Added quoting on several makepkg paths that were lacking it.
  James Rosten 
  * Adding new Turkish mirror as posted on the forums.
  * Updated Italian translation
  * TODO list updates.
  * makepkg: fix installation of dependencies with version comparitors.
  * makepkg: Fix behavior of -Ssr: deps were not being correctly removed 
before.
  * makepkg: a few more changes to get dependencies working correctly. If a
  * NEWS: add quick line about backup files.
  * TODO.dan updates.
  * pacman.c: Add CacheDir to -v --verbose output.
  * Typo fix.
  * Updated Italian translation
  * Fix conflict checking to ignore symlinks that were in previous version 
of
  Lots of translation updates before we release.
  Roman Kyrylych 
  * Updated Brazilian Portuguese translation
  Clarify some English messages as suggested by Nagy Gabor. I even did the 
hard
  Fixed alpm_log call- too many parameters passed. (bardo on 
#archlinux-pacman)
  * Updated German translation
  * Removed three dead mirrors (Dale Blount ).
  Andrew Fyfe 
  * Fix bug where 'makepkg -L' doesn't stop on build failure.

Jürgen Hötzel (1):
  * dont use XferCommand if retrieving local files, fixes wget error:

---


hooks/post-receive
-- 
The official pacman repository

Re: [pacman-dev] [PATCH] pacman: don't error when a group exists but all packages are ignored

[Andrew Gregory]

On 10/18/18 at 08:45pm, morganamilo wrote:
> Currently when attempting to sync a group where all packages are
> ignored (either by ignorepkg, ignoregroup or --needed) pacman
> will error with "target not found".
> 
> Instead, if a group has no packages, check if the group exists
> and only throw an error if it does not.
> 
> Signed-off-by: morganamilo 

Let's put group_exists in the front-end.  We may want to modify it to
take usage or other factors into account and I don't want to have to
figure out how to fit that into a public API, and it's a lot easier to
move it from the front-end to the back-end if we do decide to in the
future than the other way around.

> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index 2d3d198a..316853bb 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -1446,6 +1446,8 @@ int alpm_extract_keyid(alpm_handle_t *handle, const 
> char *identifier,
>  
>  alpm_list_t *alpm_find_group_pkgs(alpm_list_t *dbs, const char *name);
>  
> +int alpm_group_exists(alpm_list_t *dbs, const char *name);
> +
>  /*
>   * Sync
>   */
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index 05f58fad..57058782 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -313,6 +313,26 @@ alpm_list_t SYMEXPORT *alpm_find_group_pkgs(alpm_list_t 
> *dbs,
>   return pkgs;
>  }
>  
> +/** Check if a group exists across a list of databases.
> + * @param dbs the list of alpm_db_t *
> + * @param name the name of the group
> + * @return 1 if the group exists, 0 if it does not
> + */
> +int SYMEXPORT alpm_group_exists(alpm_list_t *dbs,
> + const char *name)

No need to break lines under 80 characters.

> +{
> + alpm_list_t *i;
> + for(i = dbs; i; i = i->next) {
> + alpm_db_t *db = i->data;
> +
> + if (alpm_db_get_group(db, name)) {

No space between if and (.

> + return 1;
> + }
> + }
> +
> + return 0;
> +}
> +
>  /** Compute the size of the files that will be downloaded to install a
>   * package.
>   * @param newpkg the new package to upgrade to
> diff --git a/src/pacman/sync.c b/src/pacman/sync.c
> index ef8faedf..32df6e04 100644
> --- a/src/pacman/sync.c
> +++ b/src/pacman/sync.c
> @@ -543,6 +543,10 @@ static int process_group(alpm_list_t *dbs, const char 
> *group, int error)
>   int count = alpm_list_count(pkgs);
>  
>   if(!count) {
> + if(alpm_group_exists(dbs, group)) {
> + return 0;
> + }
> +
>   pm_printf(ALPM_LOG_ERROR, _("target not found: %s\n"), group);
>   return 1;
>   }
> -- 
> 2.19.1

Re: [pacman-dev] [PATCH v2] libalpm: process needed before group selection

[Andrew Gregory]

On 10/17/18 at 04:40pm, morganamilo wrote:
> When --needed is used, up to date packages are now filtered out
> before showing the group select.
> 
> Fixes FS#22870.
> 
> Signed-off-by: morganamilo 
> ---
> v2: Changed per Andrew's feedback.

I wasn't concerned about the misleading message for this patch, but
the failing test is another thing.  If you're not testing your patches
with `make check`, please do.  We could update the test, but let's
just go ahead and fix the "error" after all.  I recommend, in
a separate patch, adding a group_exists function and using that to
distinguish between a non-existent group and a group with no packages
selected after find_group_packages returns NULL.

> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index b6ae7b72..05f58fad 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -277,10 +277,21 @@ alpm_list_t SYMEXPORT *alpm_find_group_pkgs(alpm_list_t 
> *dbs,
>  
>   for(j = grp->packages; j; j = j->next) {
>   alpm_pkg_t *pkg = j->data;
> + alpm_trans_t *trans = db->handle->trans;
>  
>   if(alpm_pkg_find(ignorelist, pkg->name)) {
>   continue;
>   }
> + if(trans != NULL && trans->flags & 
> ALPM_TRANS_FLAG_NEEDED) {
> + alpm_pkg_t *local = 
> _alpm_db_get_pkgfromcache(db->handle->db_local, pkg->name);
> + if(local && _alpm_pkg_compare_versions(pkg, 
> local) == 0) {
> + /* with the NEEDED flag, packages up to 
> date are not reinstalled */
> + _alpm_log(db->handle, ALPM_LOG_WARNING, 
> _("%s-%s is up to date -- skipping\n"),
> + local->name, 
> local->version);
> + ignorelist = alpm_list_add(ignorelist, 
> pkg);
> + continue;
> + }
> + }
>   if(alpm_pkg_should_ignore(db->handle, pkg)) {
>   alpm_question_install_ignorepkg_t question = {
>   .type = ALPM_QUESTION_INSTALL_IGNOREPKG,
> -- 
> 2.19.1

Re: [pacman-dev] [PATCH] alpm: Do not raise SIGINT when filesize goes over limit

[Andrew Gregory]

On 10/09/18 at 06:29pm, Olivier Brunel wrote:
> Variable dload_interrupted is used both to abort a download because
> SIGINT was caught, and when a file limit is reached. But raising SIGINT
> is only meant to happen in the first case.
> 
> Signed-off-by: Olivier Brunel 
> ---

ACK.

Re: [pacman-dev] [PATCH] alpm: Fix SIGINT handling re: aborting download

[Andrew Gregory]

On 10/09/18 at 06:29pm, Olivier Brunel wrote:
> Upon receiving SIGINT a flag is set to abort the (curl) download.
> However, since it was never reset/initialized, if a front-end doesn't
> actually exit on SIGINT, and later tries any operation that needs to
> perform a new download, said download would always get aborted right
> away due to the flag not having been reset.
> 
> Signed-off-by: Olivier Brunel 
> ---
>  lib/libalpm/dload.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index cca39470..0a3293cf 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -253,6 +253,7 @@ static void curl_set_handle_opts(struct dload_payload 
> *payload,
>   const char *useragent = getenv("HTTP_USER_AGENT");
>   struct stat st;
>  
> + dload_interrupted = 0;

I think set_handle_opts is the wrong place to reset it since it is in
fact not a handle option.  Let's put it right before the signal
handlers are registered to keep signal-related things together.

>   /* the curl_easy handle is initialized with the alpm handle, so we only 
> need
>* to reset the handle's parameters for each time it's used. */
>   curl_easy_reset(curl);
> -- 
> 2.19.0

Re: [pacman-dev] [PATCH] libalpm: process needed before group selection

[Andrew Gregory]

On 09/22/18 at 05:24pm, morganamilo wrote:
> When --needed is used, up to date packages are now filtered out
> before showing the group select.
> 
> Signed-off-by: morganamilo 
> ---
> 
> This patch set is currently incomplete. There is a problem where if every
> package in the group is already installed you end up with the eror.
> "error: target not found: groupname". Instead "there is nothing to do" should
> be produced instead.
> 
> I'm unsure of how to solve this so I am submitting this for discussion.
> Currently my idea is to have alpm_find_group_pkgs gain a new param `int 
> *exists`
> which the front end can then check instead of the length of the return. Or
> instead the needed check could just be moved to the front end. Let me know
> if theres a better way.

I really hate this function.  The logic is way too complex and
specific for it to fit comfortably in the backend.  I'd really prefer
the whole thing move to the frontend.  Regardless, I'm not concerned
about the somewhat erroneous message.  That already happens if the
entire group is ignored, so that's really an existing problem that can
be solved separately.  I'd like to see the patch cleaned up a little,
but I think this solution is fine for now.  If you resubmit, please
include a note in the commit message that this fixes FS#22870.

> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index b6ae7b72..f1c02417 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -270,6 +270,8 @@ alpm_list_t SYMEXPORT *alpm_find_group_pkgs(alpm_list_t 
> *dbs,
>   for(i = dbs; i; i = i->next) {
>   alpm_db_t *db = i->data;
>   alpm_group_t *grp = alpm_db_get_group(db, name);
> + alpm_handle_t *handle = db->handle;
> + alpm_trans_t *trans = handle->trans;

If you're going to save db->handle to a variable, go ahead and replace
the other db->handle references in the function.

>   if(!grp) {
>   continue;
> @@ -277,10 +279,26 @@ alpm_list_t SYMEXPORT *alpm_find_group_pkgs(alpm_list_t 
> *dbs,
>  
>   for(j = grp->packages; j; j = j->next) {
>   alpm_pkg_t *pkg = j->data;
> + alpm_pkg_t *local = 
> _alpm_db_get_pkgfromcache(handle->db_local, pkg->name);
>  
>   if(alpm_pkg_find(ignorelist, pkg->name)) {
>   continue;
>   }
> + if(local) {
> + const char *localpkgname = local->name;
> + const char *localpkgver = local->version;
> + int cmp = _alpm_pkg_compare_versions(pkg, 
> local);

These variables are unnecessary.  They are each used once, don't add
any descriptive detail, and the two strings have names longer than the
struct member they reference.

> +
> + if(cmp == 0) {
> + if(trans != NULL && trans->flags & 
> ALPM_TRANS_FLAG_NEEDED) {

Of the three conditions, this will be the fastest to check, so it
should be the outermost.  The other two can then be combined to
avoid unnecessary indentation:

if(trans != NULL && trans->flags & ALPM_TRANS_FLAG_NEEDED) {
alpm_pkg_t *local = 
_alpm_db_get_pkgfromcache(handle->db_local, pkg->name);
if(local && _alpm_pkg_compare_versions(pkg, local) == 
0) {

> + /* with the NEEDED flag, 
> packages up to date are not reinstalled */
> + _alpm_log(handle, 
> ALPM_LOG_WARNING, _("%s-%s is up to date -- skipping\n"),
> + localpkgname, 
> localpkgver);
> + ignorelist = 
> alpm_list_add(ignorelist, pkg);
> + continue;
> + }
> + }
> + }
>   if(alpm_pkg_should_ignore(db->handle, pkg)) {
>   alpm_question_install_ignorepkg_t question = {
>   .type = ALPM_QUESTION_INSTALL_IGNOREPKG,
> -- 
> 2.19.0

Re: [pacman-dev] [PATCH 1/2] Port pactest to python3

[Andrew Gregory]

On 09/20/18 at 05:43am, Dave Reisner wrote:
> Use BytesIO instead of StringIO, and ensure that we unicode-encode data
> where needed.
> ---

Any particular reason for the bump or just dropping the (allegedly)
dead python2?  I've held off from doing this so far because asciidoc
is python2, hoping they'd update to python3 eventually.  It appears to
be dead now, though, so I guess it's time to move on.

...

> diff --git a/test/pacman/pmdb.py b/test/pacman/pmdb.py
> index f7671987..26a8d9a4 100644
> --- a/test/pacman/pmdb.py
> +++ b/test/pacman/pmdb.py
> @@ -17,9 +17,10 @@
>  
>  import os
>  import shutil
> -from StringIO import StringIO
>  import tarfile
>  
> +from io import BytesIO
> +

Why the extra line breaks around the new imports?  They were grouped
by built-in vs local.

Re: [pacman-dev] [PATCH v2] pacman: fix possible buffer overflow

[Andrew Gregory]

On 09/22/18 at 11:30pm, morganamilo wrote:
> in the function query_fileowner, if the user enters a string longer
> than PATH_MAX then rpath will buffer overflow when lrealpath tries to
> strcat everything together.
> 
> So make sure to bail early if the generated path is going to be bigger
> than PATH_MAX.
> 
> This also fixes the compiler warning:
> query.c: In function ‘query_fileowner’:
> query.c:192:4: warning: ‘strncpy’ specified bound 4096 equals destination 
> size [-Wstringop-truncation]
> strncpy(rpath, filename, PATH_MAX);
> 
> Signed-off-by: morganamilo 
> 
> diff --git a/src/pacman/query.c b/src/pacman/query.c
> index 00c39638..c661fafb 100644
> --- a/src/pacman/query.c
> +++ b/src/pacman/query.c
> @@ -102,7 +102,7 @@ static char *lrealpath(const char *path, char 
> *resolved_path)
>  {
>   const char *bname = mbasename(path);
>   char *rpath = NULL, *dname = NULL;
> - int success = 0;
> + int success = 0, len;
>  
>   if(strcmp(bname, ".") == 0 || strcmp(bname, "..") == 0) {
>   /* the entire path needs to be resolved */
> @@ -115,8 +115,14 @@ static char *lrealpath(const char *path, char 
> *resolved_path)
>   if(!(rpath = realpath(dname, NULL))) {
>   goto cleanup;
>   }
> +
> + len = strlen(rpath) + strlen(bname) + 2;
> + if (len > PATH_MAX) {
> + errno = ENAMETOOLONG;
> + goto cleanup;
> + }
>   if(!resolved_path) {
> - if(!(resolved_path = malloc(strlen(rpath) + strlen(bname) + 
> 2))) {
> + if(!(resolved_path = malloc(len))) {
>   goto cleanup;
>   }
>   }
> @@ -187,11 +193,16 @@ static int query_fileowner(alpm_list_t *targets)
>   }
>   }
>  
> + errno = 0;

No need to explicitly set errno here.  lrealpath should always set it
on failure.

>   if(!lrealpath(filename, rpath)) {
> + if (errno == ENAMETOOLONG) {
> + pm_printf(ALPM_LOG_ERROR, _("path too long: 
> %s/\n"), filename);
> + goto targcleanup;
> + }
>   /* Can't canonicalize path, try to proceed anyway */
> - strncpy(rpath, filename, PATH_MAX);
> + strncpy(rpath, filename, PATH_MAX - 1);
> + rpath[PATH_MAX - 1] = '\0';

This silences the warning, but filename could still be longer than
PATH_MAX, in which case the results will be incorrect.  I think the
best thing to do is treat ENAMETOOLONG the same as any other lrealpath
error, and explicitly check if strlen(filename) >= PATH_MAX instead.

>   }
> -

Please leave this empty line to separate the two blocks.

>   if(strncmp(rpath, root, rootlen) != 0) {
>   /* file is outside root, we know nothing can own it */
>   pm_printf(ALPM_LOG_ERROR, _("No package owns %s\n"), 
> filename);
> -- 
> 2.19.0

[pacman-dev] [PATCH] handle EINTR while polling scripts/hooks

[Andrew Gregory]

If poll() is interrupted by a signal, alpm was closing the socket it
uses for listening to script/hook output.  This would drop script output
at the least and kill the script at the worst.

Fixes FS#60396

Signed-off-by: Andrew Gregory 
---
 lib/libalpm/util.c | 10 +-
 test/pacman/tests/TESTS|  1 +
 test/pacman/tests/scriptlet-signal-handling.py | 15 +++
 3 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 test/pacman/tests/scriptlet-signal-handling.py

diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index eaf85e93..d33eef2a 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -665,6 +665,7 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char 
*cmd, char *const argv[],
ssize_t olen = 0, ilen = 0;
nfds_t nfds = 2;
struct pollfd fds[2], *child2parent = &(fds[0]), *parent2child 
= &(fds[1]);
+   int poll_ret;
 
child2parent->fd = child2parent_pipefd[TAIL];
child2parent->events = POLLIN;
@@ -685,7 +686,14 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char 
*cmd, char *const argv[],
 #define STOP_POLLING(p) do { close(p->fd); p->fd = -1; } while(0)
 
while((child2parent->fd != -1 || parent2child->fd != -1)
-   && poll(fds, nfds, -1) > 0) {
+   && (poll_ret = poll(fds, nfds, -1)) != 0) {
+   if(poll_ret == -1) {
+   if(errno == EINTR) {
+   continue;
+   } else {
+   break;
+   }
+   }
if(child2parent->revents & POLLIN) {
if(_alpm_chroot_read_from_child(handle, 
child2parent->fd,
ibuf, , 
sizeof(ibuf)) != 0) {
diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS
index 5deb93c4..dc0b4ec3 100644
--- a/test/pacman/tests/TESTS
+++ b/test/pacman/tests/TESTS
@@ -150,6 +150,7 @@ TESTS += test/pacman/tests/replace102.py
 TESTS += test/pacman/tests/replace103.py
 TESTS += test/pacman/tests/replace104.py
 TESTS += test/pacman/tests/replace110.py
+TESTS += test/pacman/tests/scriptlet-signal-handling.py
 TESTS += test/pacman/tests/scriptlet-signal-reset.py
 TESTS += test/pacman/tests/scriptlet001.py
 TESTS += test/pacman/tests/scriptlet002.py
diff --git a/test/pacman/tests/scriptlet-signal-handling.py 
b/test/pacman/tests/scriptlet-signal-handling.py
new file mode 100644
index ..e65df55b
--- /dev/null
+++ b/test/pacman/tests/scriptlet-signal-handling.py
@@ -0,0 +1,15 @@
+self.description = "Handle signal interrupts while running scriptlets/hooks"
+
+p1 = pmpkg("dummy")
+p1.install['post_install'] = """
+   kill -INT $PPID  # send an arbitrary signal that pacman catches
+   sleep 1  # give alpm time to close the socket if EINTR was not 
handled
+   echo to-parent   # if the interrupt is not handled this will die with 
SIGPIPE
+   echo success > interrupt_was_handled
+   """
+self.addpkg(p1)
+
+self.args = "-U %s" % p1.filename()
+
+self.addrule("PACMAN_RETCODE=0")
+self.addrule("FILE_EXIST=interrupt_was_handled")
-- 
2.19.0

Re: [pacman-dev] [PATCH v2] libalpm: parse {check, make}depends when reading database

[Andrew Gregory]

On 10/09/18 at 02:20am, Morgan Adamiec wrote:
> On 09/10/2018 2:15 am, Allan McRae wrote:
> > On 9/10/18 11:05 am, morganamilo wrote:
> >>  static struct pkg_operations local_pkg_ops = {
> >> -  .get_base= _cache_get_base,
> >> -  .get_desc= _cache_get_desc,
> >> -  .get_url = _cache_get_url,
> >> -  .get_builddate   = _cache_get_builddate,
> >> -  .get_installdate = _cache_get_installdate,
> >> -  .get_packager= _cache_get_packager,
> > 
> >> +  .get_base = _cache_get_base,
> >> +  .get_desc = _cache_get_desc,
> >> +  .get_url  = _cache_get_url,
> >> +  .get_builddate= _cache_get_builddate,
> >> +  .get_installdate  = _cache_get_installdate,
> > ...
> > 
> > Is there any objection to killing this formatting rather than having a
> > lot of uselessness in the patch.
> > 
> > A
> > 
> 
> I could split adding the fields and the formatting to two patches if you
> prefer? Easier to read diffs and the struct stays pretty.

Kill the formatting.  We've had this discussion before, for this very
feature even:
https://lists.archlinux.org/pipermail/pacman-dev/2017-January/021784.html

[pacman-dev] [PATCH] reset signal handlers before running scripts/hooks

[Andrew Gregory]

Front-ends or libraries may set signals to be ignored, which gets
inherited across fork and exec.  This can cause scripts to malfunction
if they expect the signal.  To make matters worse, scripts written in
bash can't reset signals that were ignored when bash was started.

Fixes FS#56756

Signed-off-by: Andrew Gregory 
---

Hopefully nobody ignores or depends on signals outside of the POSIX
list.  As far as I can tell, the only way to reset all signal handlers
is to iterate over the entire range of positive integers.

 lib/libalpm/util.c  | 20 
 test/pacman/tests/TESTS |  1 +
 test/pacman/tests/scriptlet-signal-reset.py | 11 +++
 3 files changed, 32 insertions(+)
 create mode 100644 test/pacman/tests/scriptlet-signal-reset.py

diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index a06f5bfd..eaf85e93 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -548,6 +548,25 @@ static int _alpm_chroot_read_from_child(alpm_handle_t 
*handle, int fd,
return 0;
 }
 
+static void _alpm_reset_signals(void)
+{
+   /* reset POSIX defined signals (see signal.h) */
+   /* there are likely more but there is no easy way
+* to get the full list of valid signals */
+   int *i, signals[] = {
+   SIGABRT, SIGALRM, SIGBUS, SIGCHLD, SIGCONT, SIGFPE, SIGHUP, 
SIGILL,
+   SIGINT, SIGKILL, SIGPIPE, SIGQUIT, SIGSEGV, SIGSTOP, SIGTERM, 
SIGTSTP,
+   SIGTTIN, SIGTTOU, SIGUSR1, SIGUSR2, SIGPOLL, SIGPROF, SIGSYS, 
SIGTRAP,
+   SIGURG, SIGVTALRM, SIGXCPU, SIGXFSZ,
+   0
+   };
+   struct sigaction def;
+   def.sa_handler = SIG_DFL;
+   for(i = signals; *i; i++) {
+   sigaction(*i, , NULL);
+   }
+}
+
 /** Execute a command with arguments in a chroot.
  * @param handle the context handle
  * @param cmd command to execute
@@ -633,6 +652,7 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char 
*cmd, char *const argv[],
exit(1);
}
umask(0022);
+   _alpm_reset_signals();
execv(cmd, argv);
/* execv only returns if there was an error */
fprintf(stderr, _("call to execv failed (%s)\n"), 
strerror(errno));
diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS
index b11cb511..5deb93c4 100644
--- a/test/pacman/tests/TESTS
+++ b/test/pacman/tests/TESTS
@@ -150,6 +150,7 @@ TESTS += test/pacman/tests/replace102.py
 TESTS += test/pacman/tests/replace103.py
 TESTS += test/pacman/tests/replace104.py
 TESTS += test/pacman/tests/replace110.py
+TESTS += test/pacman/tests/scriptlet-signal-reset.py
 TESTS += test/pacman/tests/scriptlet001.py
 TESTS += test/pacman/tests/scriptlet002.py
 TESTS += test/pacman/tests/sign001.py
diff --git a/test/pacman/tests/scriptlet-signal-reset.py 
b/test/pacman/tests/scriptlet-signal-reset.py
new file mode 100644
index ..27246d12
--- /dev/null
+++ b/test/pacman/tests/scriptlet-signal-reset.py
@@ -0,0 +1,11 @@
+self.description = "Reset signals before running scriptlets/hooks"
+
+p1 = pmpkg("dummy")
+# check if SIGPIPE is ignored, it should be fatal, but GPGME ignores it
+p1.install['post_install'] = "kill -PIPE $$; echo fail > sigpipe_was_ignored"
+self.addpkg(p1)
+
+self.args = "-U %s" % p1.filename()
+
+self.addrule("PACMAN_RETCODE=0")
+self.addrule("!FILE_EXIST=sigpipe_was_ignored")
-- 
2.19.0