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

[Dave Reisner]

When building with -DNDEBUG, assert statements are compiled out to
no-ops. Thus, we can't depend on assignments or other computations
occurring inside the assert().
---
It's perhaps worth mentioning that nowhere else in the ALPM codebase
do we use assert().

 src/pacman/callback.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/pacman/callback.c b/src/pacman/callback.c
index 25909e02..4240a779 100644
--- a/src/pacman/callback.c
+++ b/src/pacman/callback.c
@@ -862,12 +862,14 @@ static void dload_progress_event(const char *filename, 
alpm_download_event_progr
int64_t curr_time = get_time_ms();
double last_chunk_rate;
int64_t timediff;
+   bool ok;
 
if(!dload_progressbar_enabled()) {
return;
}
 
-   assert(find_bar_for_filename(filename, , ));
+   ok = find_bar_for_filename(filename, , );
+   assert(ok);
 
/* compute current average values */
timediff = curr_time - bar->sync_time;
@@ -902,12 +904,14 @@ static void dload_complete_event(const char *filename, 
alpm_download_event_compl
int index;
struct pacman_progress_bar *bar;
int64_t timediff;
+   bool ok;
 
if(!dload_progressbar_enabled()) {
return;
}
 
-   assert(find_bar_for_filename(filename, , ));
+   ok = find_bar_for_filename(filename, , );
+   assert(ok);
bar->completed = true;
 
/* This may not have been initialized if the download finished before
-- 
2.26.2

[pacman-dev] [PATCH] meson: handle XFAIL tests outside of TAP

[Dave Reisner]

This change causes expected fail tests to actually fail by eliding the
'# TODO' from the test plan. In turn, we can now properly use
'should_fail' in the meson test() rule and see these expected fail
tests in the output:

Before:
  ...
  320/332 upgrade077.py   OK 0.12679290771484375 s
  321/332 upgrade078.py   OK 0.12620115280151367 s
  322/332 upgrade080.py   OK 0.1252129077911377 s
  ...

  Ok: 332
  Expected Fail:  0
  Fail:   0
  Unexpected Pass:0
  Skipped:0
  Timeout:0

After:
  ...
  320/332 upgrade077.py   OK 0.12679290771484375 s
  321/332 upgrade078.py   EXPECTEDFAIL0.12620115280151367 s
  322/332 upgrade080.py   OK 0.1252129077911377 s
  ...

  Ok: 326
  Expected Fail:  6
  Fail:   0
  Unexpected Pass:0
  Skipped:0
  Timeout:0
---
 test/pacman/meson.build | 15 ++-
 test/pacman/pmenv.py|  7 ++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/test/pacman/meson.build b/test/pacman/meson.build
index 9de4f5a1..fd0dbf90 100644
--- a/test/pacman/meson.build
+++ b/test/pacman/meson.build
@@ -330,6 +330,17 @@ pacman_tests = [
   'tests/xfercommand001.py',
 ]
 
+xfail_tests = {
+  'tests/deprange001.py': true,
+  # expect failure on 32 bit machines
+  'tests/query006.py': cc.sizeof('ssize_t') < 8,
+  'tests/replace110.py': true,
+  'tests/sync-update-package-removing-required-provides.py': true,
+  'tests/sync403.py': true,
+  'tests/sync406.py': true,
+  'tests/upgrade078.py': true,
+}
+
 foreach input : pacman_tests
   test_name = input.split('/')[1]
   args = [
@@ -351,7 +362,9 @@ foreach input : pacman_tests
 test_name,
 PYTHON,
 protocol : 'tap',
+env : ['RUNNING_UNDER_MESON=1'],
 args : args,
-timeout: 120,
+timeout : 120,
+should_fail : xfail_tests.get(input, false),
 depends : [pacman_bin])
 endforeach
diff --git a/test/pacman/pmenv.py b/test/pacman/pmenv.py
index 24437b61..d87193f2 100644
--- a/test/pacman/pmenv.py
+++ b/test/pacman/pmenv.py
@@ -70,5 +70,10 @@ def run(self):
 t.run(self.pacman)
 
 tap.diag("==> Checking rules")
-tap.todo = t.expectfailure
+# When running under meson, we don't emit 'todo' in the plan 
and instead
+# handle expected failures in the test() objects. This really 
should be
+# fixed in meson:
+# 
https://github.com/mesonbuild/meson/issues/2923#issuecomment-614647076
+tap.todo = (t.expectfailure and
+not 'RUNNING_UNDER_MESON' in os.environ)
 tap.subtest(lambda: t.check(), t.description)
-- 
2.26.2

Re: [pacman-dev] Dropping SIGPIPE workaround from dload.c

[Dave Reisner]

On Mon, Apr 20, 2020, 18:04 Anatol Pomozov  wrote:

> Hi folks
>
> I am looking at the lib/libalpm/dload.c download codepath and found
> that it has a following snippet:
>
>/* Ignore any SIGPIPE signals. With libcurl, these shouldn't be
> happening,
> * but better safe than sorry. Store the old signal handler first.
> */
>

Yup, this is fine to remove. Curl would actually signal DNS timeouts with
SIGALRM not SIGPIPE. This is leftover crap from the libfetch days.

dR

   mask_signal(SIGPIPE, SIG_IGN, _sig_pipe);
>dload_interrupted = 0;
>mask_signal(SIGINT, , _sig_int);
>
> My understanding that this code tries to handle DNS timeouts that are
> signaled as a SIGPIPE. And to avoid killing the whole app this code
> installs this sighandler.
>
> But my experiment (at my dev branch that removes the codesnippet
> above) shows that curl handles DNS timeout correctly:
>
>  sudo pacman -U http://blackhole.webpagetest.org/foo.zst
> :: Retrieving packages...
>  foo.zst failed to download
>  foo.zst.sig failed to download
> error: failed retrieving file 'foo.zst' from blackhole.webpagetest.org
> : Connection timed out after 1 milliseconds
> error: failed retrieving file 'foo.zst.sig' from
> blackhole.webpagetest.org : Connection timed out after 1
> milliseconds
> warning: failed to retrieve some files
>
> There is also a test that checks for SIGPIPE handling
> (test/pacman/tests/scriptlet-signal-reset.py) and it passes as well:
>
> 157/332 scriptlet-signal-reset.py   OK
> 0.11832118034362793 s
>
> Which makes me believe that SIGPIPE handler trick is not needed and
> mCURL handles timeouts correctly.
>
> I've been using a branch without this handler for two months and never
> seen any signal related issues with my parallel-download pacman.
>
> I am thinking of *not* carrying this workaround to the new multiplexed
> function and want to check with you if you are OK with it.
>

[pacman-dev] [PATCH] makepkg: drop duplicate reporting of missing dependencies

[Dave Reisner]

When pacman fails to satisfy deps, we might see output like the
following:

==> Making package: spiderfoot 3.0-1 (Thu 06 Feb 2020 12:45:10 PM CET)
==> Checking runtime dependencies...
==> Installing missing dependencies...
error: target not found: python-pygexf
==> ERROR: 'pacman' failed to install missing dependencies.
==> Missing dependencies:
  -> python-dnspython
  -> python-exifread
  -> python-cherrypy
  -> python-beautifulsoup4
  -> python-netaddr
  -> python-pysocks
  -> python-ipwhois
  -> python-ipaddress
  -> python-phonenumbers
  -> python-pypdf2
  -> python-stem
  -> python-whois
  -> python-future
  -> python-pyopenssl
  -> python-docx
  -> python-pptx
  -> python-networkx
  -> python-cryptography
  -> python-secure
  -> python-pygexf
  -> python-adblockparser
==> Checking buildtime dependencies...
==> ERROR: Could not resolve all dependencies.

This is misleading -- the only truly missing package is python-pygexf,
but we fail to remove sync-able deps from our deplist and report
everything as if it were missing. Simply drop this extra reporting
because pacman already tells us exactly what couldn't be resolved.
---
I thought about trying to make this accurate and diff the lists --
something like:

  mapfile -t deplist < <(printf '%s\n' "${deplist}" | grep -vxFf <(run_pacman 
-Ssq))

but I'm not convinced this is really the right thing to do...

 scripts/makepkg.sh.in | 6 --
 1 file changed, 6 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 7fa791e1..bfbf165b 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -316,12 +316,6 @@ resolve_deps() {
[[ -z $deplist ]] && return $R_DEPS_SATISFIED
fi
 
-   msg "$(gettext "Missing dependencies:")"
-   local dep
-   for dep in ${deplist[@]}; do
-   msg2 "$dep"
-   done
-
return $R_DEPS_MISSING
 }
 
-- 
2.25.0

Re: [pacman-dev] Pacman database size study

[Dave Reisner]

On Wed, Jan 22, 2020 at 11:04 AM Anatol Pomozov 
wrote:

> Hello
>
> On Wed, Jan 22, 2020 at 2:23 AM Allan McRae  wrote:
> >
> > On 22/1/20 6:54 pm, Anatol Pomozov wrote:
> > > The first experiment is to parse db tarfile using the script and then
> > > write it back to a file:
> > >   uncompressed size is 17757184 that is equal to original sample
> > >   'zstd -19' compressed size is 4366994 that is 1.0084540990896713
> > > times better than original sample
> > >
> > > Tar *entries* content is identical to the original file. Uncompressed
> > > size is exactly the same. Compressed (zstd -19) size is 0.8% better.
> > > It comes from the fact that my script does not set entries user/group
> > > value and neither sets tar entries modification time. I am not sure if
> > > this information is actually used by pacman. Modification time
> > > contains a lot of entropy that compressor does not like.
> >
> > tl;dr
> >
> > "original"  4366994
> > no md5  4188019
> > no pgp  1160912
> > np md5+pgp  1021667
> >
> >
> > But do any of these numbers stand if you keep the tar file?
>
> I do not fully understand your question here. plainXXX+uncomressed is
> a TAR file that matches current db format.
>
> original   17757184
> no md5   17536365
> no pgp 14085120
> no md5/pgp 13248000
>
> But compressed size is what really matters for users. Dropping pgp
> signature from db file provides the biggest benefit for compressed
> data (3.8 times smaller files).
>
> >
> > Also, I find downloading signature files causes a big pause in
> > processing the downloads.   Is that just a slow connection to the world
> > at my end?
>
> *.sig files are small so bandwidth should not be a problem.
>
> My guess is that latency to your Arch mirror is too high and setting
> up twice as many ssl connections gives noticeable slowdown. Check if
> you use local Australian mirror - it will help to reduce the
> connection setup time. Using HTTP over HTTPS might help a bit as well.
>

Point of order: If you only use a single mirror, there should only be a
single
connection -- pacman (curl) reuses connections whenever possible and only
gives up if the remote doesn't support keepalives (should be rare) or a
socket
error occurs.


> But the best solution for your problem is to have a proper pacman
> parallel download support. In this case connection setup will run in
> parallel thus sharing its setup latency. It would also require less
> HTTP/HTTPS connections as HTTP2 supports multiplexing - multiple
> downloads from the same server would share single connection.


It's more subtle than this. As I mentioned above, there should only be a
single
(reused) connection. If the problem is actually latency in TTFB, then it
might be a
matter of using a more geographically local mirror. Parallelization could
help mask
some problems here, but it's going to be a LOT of work to change pacman
internals
to accommodate this.

Re: [pacman-dev] [PATCH] Download sync database into temporary directory

[Dave Reisner]

On Wed, Jan 8, 2020, 07:30 Allan McRae  wrote:

> Sync databases (and signatures) are downloaded into a temporary directory.
> On success, they are copied into the sync directory.
>

Why not use a temporary file in the sync dir? That allows you to rename
atomically instead of copy+unlink. My guess is it's less code, too.

Currently, this achieves nothing but adding complexity, but it does open
> up the possibility of validating sync databases on download before
> replacing
> the old version.
>
> Signed-off-by: Allan McRae 
> ---
>
> This patch was long enough, and there is still quite a bit to do to
> validate
> the download sync databases before replacing the old ones, so best to stop
> here.
>
>  lib/libalpm/be_sync.c | 90 +--
>  1 file changed, 69 insertions(+), 21 deletions(-)
>
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index f1caddd8..99bdba54 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -173,8 +173,9 @@ valid:
>   */
>  int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
>  {
> -   char *syncpath;
> -   const char *dbext;
> +   char *syncpath, *sigpath = NULL;
> +   char *tmpdir = NULL, *tmppath = NULL, *tmpsig = NULL;
> +   const char *dbpath, *dbext;
> alpm_list_t *i;
> int updated = 0;
> int ret = -1;
> @@ -193,11 +194,22 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t
> *db)
> return 0;
> }
>
> +   /* attempt to grab a lock */
> +   if(_alpm_handle_lock(handle)) {
> +   RET_ERR(handle, ALPM_ERR_HANDLE_LOCK, -1);
> +   }
> +
> syncpath = get_sync_dir(handle);
> if(!syncpath) {
> return -1;
> }
>
> +   /* create temporary directory to download databases into */
> +   if(_alpm_mkdtemp(handle, ) == 0) {
> +   free(syncpath);
> +   return -1;
> +   }
> +
> /* force update of invalid databases to fix potential mismatched
> database/signature */
> if(db->status & DB_STATUS_INVALID) {
> force = 1;
> @@ -207,15 +219,41 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t
> *db)
> oldmask = umask(0022);
>
> siglevel = alpm_db_get_siglevel(db);
> +   dbext = handle->dbext;
>
> -   /* attempt to grab a lock */
> -   if(_alpm_handle_lock(handle)) {
> -   free(syncpath);
> -   umask(oldmask);
> -   RET_ERR(handle, ALPM_ERR_HANDLE_LOCK, -1);
> +   dbpath = _alpm_db_path(db);
> +   if(dbpath == NULL) {
> +   /* pm_errno set in _alpm_db_path() */
> +   ret = -1;
> +   goto cleanup;
> }
>
> -   dbext = handle->dbext;
> +   if((sigpath = _alpm_sigpath(handle, _alpm_db_path(db))) == NULL) {
> +   /* pm_errno is set by _alpm_sigpath */
> +   ret = -1;
> +   goto cleanup;
> +   }
> +
> +   if(asprintf(, "%s%s%s", tmpdir, db->treename, dbext) ==
> -1) {
> +   handle->pm_errno = ALPM_ERR_MEMORY;
> +   ret = -1;
> +   goto cleanup;
> +   }
> +
> +   if(asprintf(, "%s%s%s.sig", tmpdir, db->treename, dbext) ==
> -1) {
> +   handle->pm_errno = ALPM_ERR_MEMORY;
> +   ret = -1;
> +   goto cleanup;
> +   }
> +
> +   /* copy current db into tempdir to allow up-to-date db handling  */
> +   if(force == 0) {
> +   if(_alpm_copyfile_with_time(dbpath, tmppath) != 0) {
> +   handle->pm_errno = ALPM_ERR_SYSTEM;
> +   ret = -1;
> +   goto cleanup;
> +   };
> +   }
>
> for(i = db->servers; i; i = i->next) {
> const char *server = i->data, *final_db_url = NULL;
> @@ -240,22 +278,11 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t
> *db)
> payload.force = force;
> payload.unlink_on_fail = 1;
>
> -   ret = _alpm_download(, syncpath, NULL,
> _db_url);
> +   ret = _alpm_download(, tmpdir, NULL,
> _db_url);
> _alpm_dload_payload_reset();
> 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 is set by _alpm_sigpath */
> -   ret = -1;
> -   goto cleanup;
> -   }
> -   unlink(sigpath);
> -   free(sigpath);
> -
> -
> /* check if the final URL from internal downloader
> looks reasonable */
> if(final_db_url != NULL) {
>

[pacman-dev] [PATCH 2/2] Use c99 struct initialization to avoid memset calls

[Dave Reisner]

This is guaranteed less error prone than calling memset and hoping the
human gets the argument order correct.
---
 lib/libalpm/be_local.c   |  5 +
 lib/libalpm/be_package.c |  7 ++-
 lib/libalpm/be_sync.c|  7 ++-
 lib/libalpm/dload.c  | 13 ++---
 lib/libalpm/signing.c| 23 +++
 lib/libalpm/util.c   |  2 +-
 src/pacman/conf.c|  3 +--
 7 files changed, 20 insertions(+), 40 deletions(-)

diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
index b89acf05..78e32a24 100644
--- a/lib/libalpm/be_local.c
+++ b/lib/libalpm/be_local.c
@@ -694,7 +694,7 @@ char *_alpm_local_db_pkgpath(alpm_db_t *db, alpm_pkg_t 
*info,
 static int local_db_read(alpm_pkg_t *info, int inforeq)
 {
FILE *fp = NULL;
-   char line[1024];
+   char line[1024] = {0};
alpm_db_t *db = info->origin_data.db;
 
/* bitmask logic here:
@@ -717,9 +717,6 @@ static int local_db_read(alpm_pkg_t *info, int inforeq)
"loading package data for %s : level=0x%x\n",
info->name, inforeq);
 
-   /* clear out 'line', to be certain - and to make valgrind happy */
-   memset(line, 0, sizeof(line));
-
/* DESC */
if(inforeq & INFRQ_DESC && !(info->infolevel & INFRQ_DESC)) {
char *path = _alpm_local_db_pkgpath(db, info, "desc");
diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c
index 5ffea875..9a8b4410 100644
--- a/lib/libalpm/be_package.c
+++ b/lib/libalpm/be_package.c
@@ -164,9 +164,8 @@ static int parse_descfile(alpm_handle_t *handle, struct 
archive *a, alpm_pkg_t *
char *ptr = NULL;
char *key = NULL;
int ret, linenum = 0;
-   struct archive_read_buffer buf;
+   struct archive_read_buffer buf = {0};
 
-   memset(, 0, sizeof(buf));
/* 512K for a line length seems reasonable */
buf.max_line_size = 512 * 1024;
 
@@ -448,13 +447,11 @@ static int build_filelist_from_mtree(alpm_handle_t 
*handle, alpm_pkg_t *pkg, str
char *mtree_data = NULL;
struct archive *mtree;
struct archive_entry *mtree_entry = NULL;
-   alpm_filelist_t filelist;
+   alpm_filelist_t filelist = {0};
 
_alpm_log(handle, ALPM_LOG_DEBUG,
"found mtree for package %s, getting file list\n", 
pkg->filename);
 
-   memset(, 0, sizeof(alpm_filelist_t));
-
/* create a new archive to parse the mtree and load it from archive 
into memory */
/* TODO: split this into a function */
if((mtree = archive_read_new()) == NULL) {
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 2c76fe83..4614610c 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -219,12 +219,10 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
 
for(i = db->servers; i; i = i->next) {
const char *server = i->data, *final_db_url = NULL;
-   struct dload_payload payload;
+   struct dload_payload payload = {};
size_t len;
int sig_ret = 0;
 
-   memset(, 0, sizeof(struct dload_payload));
-
/* set hard upper limit of 25MiB */
payload.max_size = 25 * 1024 * 1024;
 
@@ -601,7 +599,7 @@ static int sync_db_read(alpm_db_t *db, struct archive 
*archive,
 {
const char *entryname, *filename;
alpm_pkg_t *pkg;
-   struct archive_read_buffer buf;
+   struct archive_read_buffer buf = {0};
 
entryname = archive_entry_pathname(entry);
if(entryname == NULL) {
@@ -613,7 +611,6 @@ static int sync_db_read(alpm_db_t *db, struct archive 
*archive,
_alpm_log(db->handle, ALPM_LOG_FUNCTION, "loading package data from 
archive entry %s\n",
entryname);
 
-   memset(, 0, sizeof(buf));
/* 512K for a line length seems reasonable */
buf.max_line_size = 512 * 1024;
 
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 40a1d07d..b3e6a411 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -195,9 +195,10 @@ static int curl_gethost(const char *url, char *buffer, 
size_t buf_len)
 static int utimes_long(const char *path, long seconds)
 {
if(seconds != -1) {
-   struct timeval tv[2];
-   memset(, 0, sizeof(tv));
-   tv[0].tv_sec = tv[1].tv_sec = seconds;
+   struct timeval tv[2] = {
+   { .tv_sec = seconds, },
+   { .tv_sec = seconds, },
+   };
return utimes(path, tv);
}
return 0;
@@ -657,7 +658,7 @@ char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t *handle, 
const char *url)
char *filepath;
const char *cachedir, *final_pkg_url = NULL;
char *final_file = NULL;
-   struct dload_payload payload;
+   struct dload_payload payload = {0};
int ret = 0;
 
CHECK_HANDLE(handle, return NULL);
@@ -666,8 +667,6 

[pacman-dev] [PATCHv2 1/2] Ensure regex object is always initialized

[Dave Reisner]

This avoids a crash in filetarget_free() when regex support isn't
requested in files_search().
---
v2: Use {0} instead of {}. The former is c99, the latter is a GNU
extension.

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

diff --git a/src/pacman/files.c b/src/pacman/files.c
index 7b0c884b..cae7130d 100644
--- a/src/pacman/files.c
+++ b/src/pacman/files.c
@@ -114,7 +114,7 @@ static int files_search(alpm_list_t *syncs, alpm_list_t 
*targets, int regex) {
char *targ = t->data;
size_t len = strlen(targ);
int exact_file = strchr(targ, '/') != NULL;
-   regex_t reg;
+   regex_t reg = {0};
 
if(exact_file) {
while(len > 1 && targ[0] == '/') {
-- 
2.24.1

[pacman-dev] [PATCH] Ensure regex object is always initialized

[Dave Reisner]

This avoids a crash in filetarget_free() when regex support isn't
requested in files_search().
---
 src/pacman/files.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/pacman/files.c b/src/pacman/files.c
index 7b0c884b..29b593be 100644
--- a/src/pacman/files.c
+++ b/src/pacman/files.c
@@ -114,7 +114,7 @@ static int files_search(alpm_list_t *syncs, alpm_list_t 
*targets, int regex) {
char *targ = t->data;
size_t len = strlen(targ);
int exact_file = strchr(targ, '/') != NULL;
-   regex_t reg;
+   regex_t reg = {};
 
if(exact_file) {
while(len > 1 && targ[0] == '/') {
-- 
2.24.1

Re: [pacman-dev] [PATCH v3] libmakepkg: add optional argument support to parseopts

[Dave Reisner]

On Wed, Oct 23, 2019 at 08:38:08PM -0400, Ethan Sommer wrote:
> Adds a "?" suffix that can be used to indicate that an option's argument is
> optional.
> 
> This allows options to have a default behaviour when the user doesn't
> specify one, e.g.: --color=[when] being able to behave like --color=auto
> when only --color is passed
> 
> Options with optional arguments given on the command line will be returned
> in the form "--opt=optarg" and "-o=optarg". Despite that not being the
> syntax for passing an argument with a shortopt (trying to pass -o=foo
> would make -o's argument "=foo"), this is done to allow the caller to split
> the option and its optarg easily
> 
> Signed-off-by: Ethan Sommer 
> ---
>  scripts/libmakepkg/util/parseopts.sh.in | 116 +++-
>  test/scripts/parseopts_test.sh  |  12 ++-
>  2 files changed, 83 insertions(+), 45 deletions(-)
> 
> diff --git a/scripts/libmakepkg/util/parseopts.sh.in 
> b/scripts/libmakepkg/util/parseopts.sh.in
> index c056cb1e..42540438 100644
> --- a/scripts/libmakepkg/util/parseopts.sh.in
> +++ b/scripts/libmakepkg/util/parseopts.sh.in
> @@ -18,16 +18,23 @@
>  #   along with this program.  If not, see .
>  #
>  # A getopt_long-like parser which portably supports longopts and
> -# shortopts with some GNU extensions. It does not allow for options
> -# with optional arguments. For both short and long opts, options
> -# requiring an argument should be suffixed with a colon. After the
> -# first argument containing the short opts, any number of valid long
> -# opts may be be passed. The end of the options delimiter must then be
> -# added, followed by the user arguments to the calling program.
> +# shortopts with some GNU extensions. For both short and long opts,
> +# options requiring an argument should be suffixed with a colon, and
> +# options with optional arguments should be suffixed with a question
> +# mark. After the first argument containing the short opts, any number
> +# of valid long opts may be be passed. The end of the options delimiter
> +# must then be added, followed by the user arguments to the calling
> +# program.
> +#
> +# Options with optional arguments will be returned as "--longopt=optarg"
> +# for longopts, or "-o=optarg" for shortopts. This isn't actually a valid
> +# way to pass an optional argument with a shortopt on the command line,
> +# but is done by parseopts to enable the caller script to split the option
> +# and its optarg easily.
>  #
>  # Recommended Usage:
> -#   OPT_SHORT='fb:z'
> -#   OPT_LONG=('foo' 'bar:' 'baz')
> +#   OPT_SHORT='fb:zq?'
> +#   OPT_LONG=('foo' 'bar:' 'baz' 'qux?')
>  #   if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then
>  # exit 1
>  #   fi
> @@ -49,29 +56,30 @@ parseopts() {
>   longoptmatch() {
>   local o longmatch=()
>   for o in "${longopts[@]}"; do
> - if [[ ${o%:} = "$1" ]]; then
> + if [[ ${o%[:?]} = "$1" ]]; then
>   longmatch=("$o")
>   break
>   fi
> - [[ ${o%:} = "$1"* ]] && longmatch+=("$o")
> + [[ ${o%[:?]} = "$1"* ]] && longmatch+=("$o")
>   done
>  
>   case ${#longmatch[*]} in
>   1)
> - # success, override with opt and return arg req 
> (0 == none, 1 == required)
> - opt=${longmatch%:}
> - if [[ $longmatch = *: ]]; then
> - return 1
> - else
> - return 0
> - fi ;;
> + # success, override with opt and return arg req 
> (0 == none, 1 == required, 2 == optional)
> + opt=${longmatch%[:?]}
> + case $longmatch in
> + *:)  return 1 ;;
> + *\?) return 2 ;;
> + *)   return 0 ;;
> + esac
> + ;;
>   0)
>   # fail, no match found
>   return 255 ;;
>   *)
>   # fail, ambiguous match
>   printf "${0##*/}: $(gettext "option '%s' is 
> ambiguous; possibilities:")" "--$1"
> - printf " '%s'" "${longmatch[@]%:}"
> + printf " '%s'" "${longmatch[@]%[:?]}"
>   printf '\n'
>   return 254 ;;
>   esac >&2
> @@ -87,32 +95,47 @@ parseopts() {
>   for (( i = 1; i < ${#1}; i++ )); do
>   opt=${1:i:1}
>  
> -  

Re: [pacman-dev] [PATCH v2] libmakepkg: add optional argument support to parseopts

[Dave Reisner]

On Wed, Oct 23, 2019 at 06:57:24PM -0400, Ethan Sommer wrote:
> Adds a "?" suffix that can be used to indicate that an option's argument is
> optional.
> 
> This allows options to have a default behaviour when the user doesn't
> specify one, e.g.: --color=[when] being able to behave like --color=auto
> when only --color is passed
> 
> Signed-off-by: Ethan Sommer 
> ---
>  scripts/libmakepkg/util/parseopts.sh.in | 110 +++-
>  test/scripts/parseopts_test.sh  |  12 ++-
>  2 files changed, 77 insertions(+), 45 deletions(-)
> 
> diff --git a/scripts/libmakepkg/util/parseopts.sh.in 
> b/scripts/libmakepkg/util/parseopts.sh.in
> index c056cb1e..9a215648 100644
> --- a/scripts/libmakepkg/util/parseopts.sh.in
> +++ b/scripts/libmakepkg/util/parseopts.sh.in
> @@ -18,16 +18,17 @@
>  #   along with this program.  If not, see .
>  #
>  # A getopt_long-like parser which portably supports longopts and
> -# shortopts with some GNU extensions. It does not allow for options
> -# with optional arguments. For both short and long opts, options
> -# requiring an argument should be suffixed with a colon. After the
> -# first argument containing the short opts, any number of valid long
> -# opts may be be passed. The end of the options delimiter must then be
> -# added, followed by the user arguments to the calling program.
> +# shortopts with some GNU extensions. For both short and long opts,
> +# options requiring an argument should be suffixed with a colon, and
> +# options with optional arguments should be suffixed with a question
> +# mark. After the first argument containing the short opts, any number
> +# of valid long opts may be be passed. The end of the options delimiter
> +# must then be added, followed by the user arguments to the calling
> +# program.
>  #
>  # Recommended Usage:
> -#   OPT_SHORT='fb:z'
> -#   OPT_LONG=('foo' 'bar:' 'baz')
> +#   OPT_SHORT='fb:zq?'
> +#   OPT_LONG=('foo' 'bar:' 'baz' 'qux?')
>  #   if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then
>  # exit 1
>  #   fi
> @@ -49,29 +50,30 @@ parseopts() {
>   longoptmatch() {
>   local o longmatch=()
>   for o in "${longopts[@]}"; do
> - if [[ ${o%:} = "$1" ]]; then
> + if [[ ${o%[:?]} = "$1" ]]; then
>   longmatch=("$o")
>   break
>   fi
> - [[ ${o%:} = "$1"* ]] && longmatch+=("$o")
> + [[ ${o%[:?]} = "$1"* ]] && longmatch+=("$o")
>   done
>  
>   case ${#longmatch[*]} in
>   1)
> - # success, override with opt and return arg req 
> (0 == none, 1 == required)
> - opt=${longmatch%:}
> - if [[ $longmatch = *: ]]; then
> - return 1
> - else
> - return 0
> - fi ;;
> + # success, override with opt and return arg req 
> (0 == none, 1 == required, 2 == optional)
> + opt=${longmatch%[:?]}
> + case $longmatch in
> + *:)  return 1 ;;
> + *\?) return 2 ;;
> + *)   return 0 ;;
> + esac
> + ;;
>   0)
>   # fail, no match found
>   return 255 ;;
>   *)
>   # fail, ambiguous match
>   printf "${0##*/}: $(gettext "option '%s' is 
> ambiguous; possibilities:")" "--$1"
> - printf " '%s'" "${longmatch[@]%:}"
> + printf " '%s'" "${longmatch[@]%[:?]}"
>   printf '\n'
>   return 254 ;;
>   esac >&2
> @@ -87,32 +89,47 @@ parseopts() {
>   for (( i = 1; i < ${#1}; i++ )); do
>   opt=${1:i:1}
>  
> - # option doesn't exist
> - if [[ $shortopts != *$opt* ]]; then
> - printf "${0##*/}: $(gettext 
> "invalid option") -- '%s'\n" "$opt" >&2
> - OPTRET=(--)
> - return 1
> - fi
> -
> - OPTRET+=("-$opt")
> - # option requires optarg
> - if [[ $shortopts = *$opt:* ]]; then
> - # if we're not at the end of 
> the option 

[pacman-dev] [PATCH] dload: never return NULL from get_filename

[Dave Reisner]

Downloads with a Content-Disposition header will typically not include
slashes. When they do, we should most certainly only take the basename,
but when they don't, we should treat the header value as the filename.

Crash introduced in d197d8ab82cf when we started using get_filename
in order to rightfully avoid an arbitrary file overwrite vulnerability.
---
 lib/libalpm/dload.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index e5696bb0..506dcb8e 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -53,9 +53,11 @@ static const char *get_filename(const char *url)
 {
char *filename = strrchr(url, '/');
if(filename != NULL) {
-   filename++;
+   return filename + 1;
}
-   return filename;
+
+   /* no slash found, it's a filename */
+   return url;
 }
 
 static char *get_fullpath(const char *path, const char *filename,
-- 
2.23.0

Re: [pacman-dev] Proposed Changelog Feature

[Dave Reisner]

On Fri, Sep 06, 2019 at 09:51:53PM -0700, Hong Shick Pak wrote:
> Hello!
> 
> I noticed that PKGBUILD supports a 'changelog=' field to specify a file that
> contains release notes for that package and this is query-able via 'pacman -Qc
> '. A lot of software these days maintain a CHANGELOG.md or NEWS file
> that keeps track of notable changes across versions (including pacman).

This isn't how the changelog feature was originally intended to be used.
The intent was for the changelog to be a history of changes made to the
distro packaging, not upstream changes. These days, Arch uses svn/git to
track those changes, so use of changelog has fallen out of favor there.

What you're proposing is actually two-fold:

1) changes in makepkg to allow a changelog that isn't part of the
local packaging files.
2) changes in Arch to generally provide upstream changelogs.

> Would it be possible for the PKGBUILD to allow users to specify a file to set
> for 'changelog=' relative to $srcdir?

This is non-trivial. Consider that something simple in the global scope
such as:

  changelog=$srcdir/$pkgname-$pkgver/NEWS

At the time this var is read, $srcdir isn't yet defined, and while
$pkgver *is* defined, it's subject to change via a pkgver() function.
You can avoid some of this pain if you insist that changelog be partof
the package() function, but that would be highly unusual/annoying.
Alternatives of re-sourcing the PKGBUILD from within user-functions are
equally hard to swallow and probably have strange side effects.

> Of all the packages installed on my system, only 'terraform' and 'powertop'
> have a changelog set. This change would make it extremely trivial for package
> maintainers to specify a changelog file if one exists for a project. I think 
> it
> would be a nice QoL win for Arch users if more packages took advantage of this
> feature.
>
> Let me know if there's an official RFC process for changes! I was not able to
> find any evidence of one.
> 
> Hong

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

[Dave Reisner]

On Sat, Aug 03, 2019 at 01:27:35AM +0200, 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 
> ---
>  src/pacman/callback.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/pacman/callback.c b/src/pacman/callback.c
> index 22865614..a4c637ce 100644
> --- a/src/pacman/callback.c
> +++ b/src/pacman/callback.c
> @@ -765,7 +765,7 @@ void cb_dl_progress(const char *filename, off_t 
> file_xfered, off_t file_total)
>  
>   len = strlen(filename);
>   fname = malloc(len + 1);
> - memcpy(fname, filename, len);
> + memcpy(fname, filename, len + 1);

Ok, but maybe we should remove the now redundant null termination after
the if block.

>   /* strip package or DB extension for cleaner look */
>   if((p = strstr(fname, ".pkg")) || (p = strstr(fname, ".db")) || (p = 
> strstr(fname, ".files"))) {
>   /* tack on a .sig suffix for signatures */
> -- 
> 2.22.0

[pacman-dev] [PATCH 2/4] meson: port over checks for types used from sys/types.h

[Dave Reisner]

These are defined by a POSIX standard, and we should assert that we have
them, or define sane fallbacks (as per sys_types.h(0P)).
---
 meson.build | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/meson.build b/meson.build
index c2ed707a..ec94a44a 100644
--- a/meson.build
+++ b/meson.build
@@ -185,6 +185,21 @@ foreach member : [
   conf.set('HAVE_' + '_'.join([member[0], 
member[1]]).underscorify().to_upper(), have)
 endforeach
 
+foreach type : [
+# type   # program prefix  # fallback
+['mode_t',   '''#include ''', 'unsigned int'],
+['uid_t','''#include ''', 'unsigned int'],
+['off_t','''#include ''', 'signed int'],
+['pid_t','''#include ''', 'signed int'],
+['size_t',   '''#include ''', 'unsigned int'],
+['ssize_t',  '''#include ''', 'signed int'],
+['int64_t',  '''#include ''','signed long int'],
+  ]
+  if not cc.has_type(type[0], prefix: type[1])
+conf.set(type[0], type[2])
+  endif
+endforeach
+
 if conf.has('HAVE_STRUCT_STATVFS_F_FLAG')
   conf.set('FSSTATSTYPE', 'struct statvfs')
 elif conf.has('HAVE_STRUCT_STATFS_F_FLAGS')
-- 
2.22.0

[pacman-dev] [PATCH 3/4] meson: remove tap-driver.py, use meson's TAP protocol

[Dave Reisner]

This includes a patch from Andrew to fix pactest's TAP output for
subtests. Original TAP support in meson was added in 0.50, but 0.51
contains a bugfix that ensures the test still work with the --verbose
flag passed to meson test, so let's depend on that.
---
I'm aware this has a merge conflict with one of Eli's meson patches,
which also bumps the meson version requirement.

 build-aux/tap-driver.py  | 296 -
 meson.build  |   2 +-
 test/pacman/meson.build  | 675 +++
 test/pacman/tap.py   |   3 +-
 test/scripts/meson.build |   3 +-
 5 files changed, 336 insertions(+), 643 deletions(-)
 delete mode 100644 build-aux/tap-driver.py

diff --git a/build-aux/tap-driver.py b/build-aux/tap-driver.py
deleted file mode 100644
index c231caec..
--- a/build-aux/tap-driver.py
+++ /dev/null
@@ -1,296 +0,0 @@
-#!/usr/bin/env python3
-# Adapted from tappy copyright (c) 2016, Matt Layman
-# MIT license
-# https://github.com/python-tap/tappy
-
-import io
-import re
-import subprocess
-import sys
-
-
-class Directive(object):
-"""A representation of a result line directive."""
-
-skip_pattern = re.compile(
-r"""^SKIP\S*
-(?P\s*) # Optional whitespace.
-(?P.*)  # Slurp up the rest.""",
-re.IGNORECASE | re.VERBOSE)
-todo_pattern = re.compile(
-r"""^TODO\b # The directive name
-(?P\s*) # Immediately following must be whitespace.
-(?P.*)  # Slurp up the rest.""",
-re.IGNORECASE | re.VERBOSE)
-
-def __init__(self, text):
-"""Initialize the directive by parsing the text.
-The text is assumed to be everything after a '#\s*' on a result line.
-"""
-self._text = text
-self._skip = False
-self._todo = False
-self._reason = None
-
-match = self.skip_pattern.match(text)
-if match:
-self._skip = True
-self._reason = match.group('reason')
-
-match = self.todo_pattern.match(text)
-if match:
-if match.group('whitespace'):
-self._todo = True
-else:
-# Catch the case where the directive has no descriptive text.
-if match.group('reason') == '':
-self._todo = True
-self._reason = match.group('reason')
-
-@property
-def text(self):
-"""Get the entire text."""
-return self._text
-
-@property
-def skip(self):
-"""Check if the directive is a SKIP type."""
-return self._skip
-
-@property
-def todo(self):
-"""Check if the directive is a TODO type."""
-return self._todo
-
-@property
-def reason(self):
-"""Get the reason for the directive."""
-return self._reason
-
-
-class Parser(object):
-"""A parser for TAP files and lines."""
-
-# ok and not ok share most of the same characteristics.
-result_base = r"""
-\s*# Optional whitespace.
-(?P\d*)# Optional test number.
-\s*# Optional whitespace.
-(?P[^#]*) # Optional description before #.
-\#?# Optional directive marker.
-\s*# Optional whitespace.
-(?P.*)  # Optional directive text.
-"""
-ok = re.compile(r'^ok' + result_base, re.VERBOSE)
-not_ok = re.compile(r'^not\ ok' + result_base, re.VERBOSE)
-plan = re.compile(r"""
-^1..(?P\d+) # Match the plan details.
-[^#]* # Consume any non-hash character to confirm only
-  # directives appear with the plan details.
-\#?   # Optional directive marker.
-\s*   # Optional whitespace.
-(?P.*) # Optional directive text.
-""", re.VERBOSE)
-diagnostic = re.compile(r'^#')
-bail = re.compile(r"""
-^Bail\ out!
-\s*# Optional whitespace.
-(?P.*) # Optional reason.
-""", re.VERBOSE)
-version = re.compile(r'^TAP version (?P\d+)$')
-
-TAP_MINIMUM_DECLARED_VERSION = 13
-
-def parse(self, fh):
-"""Generate tap.line.Line objects, given a file-like object `fh`.
-`fh` may be any object that implements both the iterator and
-context management protocol (i.e. it can be used in both a
-"with" statement and a "for...in" statement.)
-Trailing whitespace and newline characters will be automatically
-stripped from the input lines.
-"""
-with fh:
-for line in fh:
-yield self.parse_line(line.rstrip())
-
-def parse_line(self, text):
-"""Parse a line into whatever TAP category it belongs."""
-match = self.ok.match(text)
-if match:
-return self._parse_result(True, match)
-
-match = self.not_ok.match(text)
-  

[pacman-dev] [PATCH 4/4] build-aux: detect build dir based on build.ninja

[Dave Reisner]

.ninja.log is only present after building (successful or otherwise) the
project, but build.ninja is output as soon as the build dir is setup.
---
 build-aux/update-po | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/build-aux/update-po b/build-aux/update-po
index ce1ad4be..bf8d7cfc 100755
--- a/build-aux/update-po
+++ b/build-aux/update-po
@@ -1,7 +1,7 @@
 #!/bin/bash
 
 find_build_directory() {
-  local build_dirs=(*/.ninja_log)
+  local build_dirs=(*/build.ninja)
 
   if [[ ! -e ${build_dirs[0]} ]]; then
 echo "error: No build directory found. Have you run 'meson build' yet?" >&2
-- 
2.22.0

[pacman-dev] [PATCH 1/4] meson: drop checks for things we don't use

[Dave Reisner]

This was ported over from the AC_CHECK_{FUNCS,HEADERS} lists in
configure.ac, but I never actually checked if the resulting CPP defines
are used. Turns out, lots of symbols, not a lot of define usage.
---
 meson.build | 25 -
 1 file changed, 25 deletions(-)

diff --git a/meson.build b/meson.build
index 91f05031..c2ed707a 100644
--- a/meson.build
+++ b/meson.build
@@ -163,38 +163,13 @@ foreach header : [
 endforeach
 
 foreach sym : [
-'dup2',
-'fork',
-'getcwd',
 'getmntent',
 'getmntinfo',
-'gettimeofday',
-'memmove',
-'memset',
-'mkdir',
-'realpath',
-'regcomp',
-'rmdir',
-'setenv',
-'setlocale',
-'strcasecmp',
-'strchr',
-'strcspn',
-'strdup',
-'strerror',
 'strndup',
 'strnlen',
-'strnlen',
-'strrchr',
-'strsep',
 'strsep',
-'strstr',
-'strtol',
 'swprintf',
 'tcflush',
-'tcflush',
-'uname',
-'wcwidth',
   ]
   have = cc.has_function(sym, args : '-D_GNU_SOURCE')
   conf.set10('HAVE_' + sym.to_upper(), have)
-- 
2.22.0

[pacman-dev] [PATCH] free makedepends/checkdepends when freeing packages

[Dave Reisner]

Credit to Andrew for identifying source of the leak.
---
 lib/libalpm/package.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
index 9471..dde32175 100644
--- a/lib/libalpm/package.c
+++ b/lib/libalpm/package.c
@@ -683,6 +683,8 @@ void _alpm_pkg_free(alpm_pkg_t *pkg)
alpm_list_free(pkg->backup);
free_deplist(pkg->depends);
free_deplist(pkg->optdepends);
+   free_deplist(pkg->checkdepends);
+   free_deplist(pkg->makedepends);
free_deplist(pkg->conflicts);
free_deplist(pkg->provides);
alpm_list_free(pkg->removes);
-- 
2.22.0

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

[Dave Reisner]

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.

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

[Dave Reisner]

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

Re: [pacman-dev] Version comparison algorithm.

[Dave Reisner]

On Sat, Mar 16, 2019, 10:02 Richard Dodd  wrote:

> I'm writing to ask about the version comparison function `rpmvercmp` in
> `libalpm/version.cL83`. The algorithm is complex and I'm trying to
> understand its behavior. I want to hash using the package name and version
> as a key, and so I need a hash function where `i1 == i2 => hash(i1) ==
> hash(i2)` according to the version comparison operation. I'll describe how
> the algorithm behaves and then ask my questions.
>

Could you explain what your actual goal is? Having ordering in a hash
function sounds extremely odd. Generally, you care about associativity *or*
ordering, not both.

The algorithm works on a byte string and uses ascii comparison rules (no
> unicode).
>
>  - First, split the input up into blocks of *alpha*, *digit* or
> *non-alphanum*.
>  - For each pair of blocks
> - If the types are different, then *non-alphanum* is newer than
> *numeric*, which is newer than *alpha*
> - If the types are the same, then the rules are
>   - For *non-alphanum*, compare lengths, longer is newer, equal lengths
> are equal segments (so *--* and *::* are the same)
>   - For *alpha* just do a lexicographic comparison (so *b* is newer
> than *a* etc.)
>   - For *numeric*, do a numeric comparison. (this can be done by
> skipping leading zeros, then comparing length, then comparing
> lexicographically, to avoid overflow on integer conversion)
>   - If one input is longer than the other, and all sections so far have
> been equal, then if the next section of the longer is *alpha*, it is older,
> and if it is *numeric* it is newer. (so "1a" is older than "1", "a1" is
> newer than "a").
>   - If the inputs have the same number of sections that are all equal, then
> they are equal.
>
> Questions:
>
>  1. Is the algorithm correctly described here.
>  2. This should mean that if I hash length for *non-numeric*, the string
> with stripped zeros for *numeric*, and just the string for *alpha*, the has
> law should be upheld. Is this correct?
>
> Thanks
> Rich
>

Re: [pacman-dev] [PATCH] Reformatting log timestamp to include time-zone

[Dave Reisner]

On Thu, Mar 07, 2019 at 10:35:32AM +1000, Allan McRae wrote:
> On 5/3/19 7:46 am, Florian Wehner wrote:
> > The time logged is currently given as localtime without any time-zone
> > information. This is confusing in various scenarios.
> > 
> > Examples:
> > * If one is travelling across time-zones and the timestamps in the log
> > appear out of order.
> 
> Stop updating multiple times per hour!  :D
> 
> > * Comparing dates with `datediff` gives an offset by the time-zone
> > 
> > This patch would reformat the time-stamp to a full ISO-8601 version.
> > It includes the 'T' separating date and time. This could be removed.
> > 
> > Old: [2019-03-04 16:15]
> > New: [2019-03-04T16:15-05:00]
> > 
> 
> Can we switch from localtime() to gmtime() and just use UTC?
> 

I wouldn't do this... it's not normal to have local machine logs in
another timezone. It'll make correlation harder if your natural timezone
isn't already UTC. And, if we magically change the timezone with a new
release of pacman and don't include the TZ explicitly in the timestamp,
it's just going to be a source of confusion for people.

I'm +1 on keeping localtime and explicitly adding the timezone
identifier to the logline.

> 
> > Signed-off-by: Florian Wehner 
> > ---
> >  lib/libalpm/log.c | 10 --
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/libalpm/log.c b/lib/libalpm/log.c
> > index e46ad3c3..cf869a08 100644
> > --- a/lib/libalpm/log.c
> > +++ b/lib/libalpm/log.c
> > @@ -20,6 +20,7 @@
> >  
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  
> > @@ -37,12 +38,17 @@
> >  static int _alpm_log_leader(FILE *f, const char *prefix)
> >  {
> > time_t t = time(NULL);
> > +   int tz_h, tz_m;
> > struct tm *tm = localtime();
> >  
> > +   /* Calculate the timezone offset ±hh:mm */
> > +   tz_h = tm->tm_gmtoff/3600;
> > +   tz_m = abs(tm->tm_gmtoff - (tz_h*3600))/60;
> > +
> > /* Use ISO-8601 date format */
> > -   return fprintf(f, "[%04d-%02d-%02d %02d:%02d] [%s] ",
> > +   return fprintf(f, "[%04d-%02d-%02dT%02d:%02d%+03d:%02d] [%s] ",
> > tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday,
> > -   tm->tm_hour, tm->tm_min, prefix);
> > +   tm->tm_hour, tm->tm_min, tz_h, tz_m, prefix);
> >  }
> >  
> >  /** A printf-like function for logging.
> > 

Re: [pacman-dev] [PATCH] build: link vercmp with a static copy of libalpm

[Dave Reisner]

On Mon, Feb 11, 2019 at 11:19:26AM -0500, Eli Schwartz wrote:
> This has historically been the case in autotools since we want vercmp to
> not break mid-transaction in an install script.
> 
> For convenience, we create libalpm.a and use this to optionally generate
> libalpm.so (when not configured with -Dbuildstatic=true) as well as to
> link any binary which explicitly wishes to be built statically "with
> libalpm", but does not care where a function is defined. meson then
> treats this correctly: it builds the object file only once for both
> libraries, and the compiler strips out unused functionality from the
> final static binary.
> 
> Currently the only binary which requires this is vercmp.
> 
> Fixes FS#61719
> 
> Signed-off-by: Eli Schwartz 
> ---
> 
> This is based on my initial suggestion via IRC for how to solve the
> vercmp build issue using two libalpm libraries.
> 
> The important distinction is that in order to not be an error when
> buildstatic is used, we cannot try to create multiple rules that
> generate libalpm.a -- and also we don't want to install
> /usr/lib/libalpm_a.a

Ack, looks good.

>  meson.build | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 0a710653..4a74786b 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -354,15 +354,24 @@ libcommon = static_library(
>include_directories : includes,
>install : false)
> 
> -libalpm = library(
> +libalpm_a = static_library(
>'alpm',
>libalpm_sources,
> -  version : libalpm_version,
>include_directories : includes,
>dependencies : [crypto_provider, libarchive, libcurl] + gpgme_libs,
>link_with : [libcommon],
>install : true)
> 
> +if not get_option('buildstatic')
> +  libalpm = shared_library(
> +'alpm',
> +version : libalpm_version,
> +link_whole: [libalpm_a],
> +install : true)
> +else
> +  libalpm = libalpm_a
> +endif
> +
>  install_headers(
>'lib/libalpm/alpm.h',
>'lib/libalpm/alpm_list.h')
> @@ -413,7 +422,7 @@ executable(
>'vercmp',
>vercmp_sources,
>include_directories : includes,
> -  link_with : [libalpm],
> +  link_with : [libalpm_a],
>install : true,
>  )
> 
> --
> 2.20.1

Re: [pacman-dev] [PATCH] build: link vercmp staticly with libalpm

[Dave Reisner]

On Mon, Feb 11, 2019 at 08:12:51AM -0500, Dave Reisner wrote:
> This makes the meson-built vercmp equivalent to the autotools build.
> 
> Employ an intermediate archive of libalpm which our generic library rule
> can slurp in. Other alpm-only binaries (specifically we care about
> vercmp) can then link this in for alpm functionality without caring
> exactly where the function is defined. meson passes the right flags to
> ensure that unused data is stripped out of the executable.
> 
> ref: FS#61719
> ---

Sorry, this patch is a bit hasty, but I'm still curious if anyone has
any objections to the approach.

>  meson.build | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 0a710653..5c1efd73 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -354,12 +354,21 @@ libcommon = static_library(
>include_directories : includes,
>install : false)
>  
> +libalpm_a = static_library(
> +  'alpm_a',
> +  libalpm_sources,
> +  version : libalpm_version,
> +  include_directories : includes,
> +  dependencies : [crypto_provider, libarchive, libcurl] + gpgme_libs,
> +  link_with : [libcommon],
> +  install : true)
> +
>  libalpm = library(
>'alpm',
> -  libalpm_sources,
>version : libalpm_version,
>include_directories : includes,
>dependencies : [crypto_provider, libarchive, libcurl] + gpgme_libs,
> +  link_whole : [libalpm_a],
>link_with : [libcommon],
>install : true)
>  
> @@ -413,7 +422,7 @@ executable(
>'vercmp',
>vercmp_sources,
>include_directories : includes,
> -  link_with : [libalpm],
> +  link_with : [libalpm_a],
>install : true,
>  )
>  
> -- 
> 2.20.1

[pacman-dev] [PATCH] build: link vercmp staticly with libalpm

[Dave Reisner]

This makes the meson-built vercmp equivalent to the autotools build.

Employ an intermediate archive of libalpm which our generic library rule
can slurp in. Other alpm-only binaries (specifically we care about
vercmp) can then link this in for alpm functionality without caring
exactly where the function is defined. meson passes the right flags to
ensure that unused data is stripped out of the executable.

ref: FS#61719
---
 meson.build | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 0a710653..5c1efd73 100644
--- a/meson.build
+++ b/meson.build
@@ -354,12 +354,21 @@ libcommon = static_library(
   include_directories : includes,
   install : false)
 
+libalpm_a = static_library(
+  'alpm_a',
+  libalpm_sources,
+  version : libalpm_version,
+  include_directories : includes,
+  dependencies : [crypto_provider, libarchive, libcurl] + gpgme_libs,
+  link_with : [libcommon],
+  install : true)
+
 libalpm = library(
   'alpm',
-  libalpm_sources,
   version : libalpm_version,
   include_directories : includes,
   dependencies : [crypto_provider, libarchive, libcurl] + gpgme_libs,
+  link_whole : [libalpm_a],
   link_with : [libcommon],
   install : true)
 
@@ -413,7 +422,7 @@ executable(
   'vercmp',
   vercmp_sources,
   include_directories : includes,
-  link_with : [libalpm],
+  link_with : [libalpm_a],
   install : true,
 )
 
-- 
2.20.1

Re: [pacman-dev] [PATCH] libalpm: prevent 301 redirect loop from hanging the process

[Dave Reisner]

On Wed, Feb 06, 2019 at 05:22:46AM -0600, Mark Ulrich wrote:
> If a mirror responds with a 301 redirect to itself, it will create an
> infinite redirect loop. This will cause pacman to hang, unresponsive to
> even a SIGINT. The result is pacman being unable to sync or
> download any package from a particular repo if its current mirror
> is stuck in a redirect loop. Setting libcurl's MAXREDIRS option
> effectively prevents a redirect loop from hanging the process.
> 
> Signed-off-by: Mark Ulrich 
> ---
>  lib/libalpm/dload.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index 36ae4ee1..d04a5e46 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -259,6 +259,7 @@ static void curl_set_handle_opts(struct dload_payload 
> *payload,
>   curl_easy_setopt(curl, CURLOPT_URL, payload->fileurl);
>   curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, error_buffer);
>   curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 10L);
> + curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 1L);

But what if you have a mirror which legitimately has 2 hops? I could see
someone trying to run something like:

  pacman -U https://www.archlinux.org/packages/core/x86_64/pacman/download/

This is guaranteed 1 redirect already, what if the mirror that it
redirects to has a legitimate second hop in order to account for some
reorganizing?

I'm fine with the spirit of the patch, but limiting this to a single hop
isn't enough. A larger number like 10 still accomplishes the same goal
while allowing some mirror flexibility/brokenness.

>   curl_easy_setopt(curl, CURLOPT_FILETIME, 1L);
>   curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0L);
>   curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
> -- 
> 2.20.1

Re: [pacman-dev] [PATCH v5 4/4] libmakepkg: lint disallowed architecture specific variables

[Dave Reisner]

On Fri, Jan 25, 2019 at 03:47:14PM +, Morgan Adamiec wrote:
> On Fri, 25 Jan 2019 at 00:52, Allan McRae  wrote:
> >
> > On 24/1/19 11:34 pm, Dave Reisner wrote:
> > > On Thu, Jan 24, 2019 at 09:09:59AM +, Morgan Adamiec wrote:
> > >> On Thu, 24 Jan 2019 at 03:01, Allan McRae  wrote:
> > >>>
> > >>> On 22/1/19 10:05 am, morganamilo wrote:
> > >>>> Variables such as 'pkgdesc_x86_64' are invalid, instead of ignoring 
> > >>>> them
> > >>>> raise an error.
> > >>>>
> > >>>> This also disallows using 'any' as an architecture specific variable
> > >>>>
> > >>>> Signed-off-by: morganamilo 
> > >>>> ---
> > >>>>
> > >>>> v5:
> > >>>>   "libmakepkg: disallow using any as an architecture specific 
> > >>>> variable"
> > >>>>   was squashed into this commit.
> > >>>>
> > >>>>   Move this lint to its own file.
> > >>>
> > >>> Moving this to its own file is fine in principle, but it has duplicated
> > >>> a few arrays of field values.   After this patch there would be:
> > >>>
> > >>> scripts/makepkg.sh.in:
> > >>> splitpkg_overrides=(...
> > >>>
> > >>> scripts/libmakepkg/lint_pkgbuild/variable.sh.in:
> > >>> scripts/libmakepkg/lint_pkgbuild/arch_specific.sh.in:
> > >>> local array=(...
> > >>> local arch_array=(...
> > >>> local string=(...
> > >>>
> > >>> scripts/libmakepkg/lint_pkgbuild/package_function_variable.sh.in:
> > >>> local no_package=(...
> > >>>
> > >>> This will be annoying to update for any new fields or other changes.
> > >>>
> > >>>
> > >>> The properties of each field we are trying to capture are:
> > >>> 1) is an array/string
> > >>> 2) can be architecture specific
> > >>> 3) overridable in package function
> > >>>
> > >>> Can we store this in one file in a readily extendable fashion somewhere?
> > >>>
> > >>> A
> > >>
> > >> Good point. There was already a TODO on the fields, maybe it's finally
> > >> time to act on it.
> > >> I'm not too familiar with bash to really know a better way to structure 
> > >> it.
> > >> Would just moving the arrays to a different file and sourcing be fine?
> > >> util/pkgbuild.sh perhaps?
> > >
> > > Yes, this can be moved to another file and just be declared as arrays. I
> > > would suggest making sure that the names are fairly unique (and
> > > readonly) to avoid accidentally clobbering.
> > >
> > > This feels like semantics that we're overlaying on top of shell, so my 
> > > vote
> > > is for util/schema.sh.
> > >
> >
> > For names, I suggest:
> >
> > pkgbuild_array_variable
> > pkgbuild_string_variable
> > pkgbuild_arch_override_variable
> > pkgbuild_package_override_variable
> >
> > Kind of long, but won't get clobbered.
> >
> > Allan
> 
> Ending a variable name with 'variable' seems weird to me. Also I
> prefer arrays to be plural.
> 
> So I'd prefer 'pkgbuild_array_variables' or even better
> `pkgbuild_arrarys`. But then of course
> the shorter name is more risky.
> 
> Still I'd put my vote on:
> 
> pkgbuild_arrays
> pkgbuild_strings
> pkgbuild_arch_overrides
> pkgbuild_package_overrides
> 
> Also seems as these are pretty much constants should they be in all
> caps? Or is that not
> done in bash?

All caps vars are usually reserved for environment vars (e.g. SHELL,
PAGER, LESS). You might capitalize PKGBUILD (as the prefix), but
capitalizing the whole thing is unnecessary.

Again, I'm going to harp on the fact that this feels like schema, so
paint it blue:

pkgbuild_schema_arrays
pkgbuild_schema_strings
pkgbuild_schema_arch_overrides
pkgbuild_schema_package_overrides

dR

Re: [pacman-dev] [PATCH v5 4/4] libmakepkg: lint disallowed architecture specific variables

[Dave Reisner]

On Thu, Jan 24, 2019 at 09:09:59AM +, Morgan Adamiec wrote:
> On Thu, 24 Jan 2019 at 03:01, Allan McRae  wrote:
> >
> > On 22/1/19 10:05 am, morganamilo wrote:
> > > Variables such as 'pkgdesc_x86_64' are invalid, instead of ignoring them
> > > raise an error.
> > >
> > > This also disallows using 'any' as an architecture specific variable
> > >
> > > Signed-off-by: morganamilo 
> > > ---
> > >
> > > v5:
> > >   "libmakepkg: disallow using any as an architecture specific 
> > > variable"
> > >   was squashed into this commit.
> > >
> > >   Move this lint to its own file.
> >
> > Moving this to its own file is fine in principle, but it has duplicated
> > a few arrays of field values.   After this patch there would be:
> >
> > scripts/makepkg.sh.in:
> > splitpkg_overrides=(...
> >
> > scripts/libmakepkg/lint_pkgbuild/variable.sh.in:
> > scripts/libmakepkg/lint_pkgbuild/arch_specific.sh.in:
> > local array=(...
> > local arch_array=(...
> > local string=(...
> >
> > scripts/libmakepkg/lint_pkgbuild/package_function_variable.sh.in:
> > local no_package=(...
> >
> > This will be annoying to update for any new fields or other changes.
> >
> >
> > The properties of each field we are trying to capture are:
> > 1) is an array/string
> > 2) can be architecture specific
> > 3) overridable in package function
> >
> > Can we store this in one file in a readily extendable fashion somewhere?
> >
> > A
> 
> Good point. There was already a TODO on the fields, maybe it's finally
> time to act on it.
> I'm not too familiar with bash to really know a better way to structure it.
> Would just moving the arrays to a different file and sourcing be fine?
> util/pkgbuild.sh perhaps?

Yes, this can be moved to another file and just be declared as arrays. I
would suggest making sure that the names are fairly unique (and
readonly) to avoid accidentally clobbering.

This feels like semantics that we're overlaying on top of shell, so my vote
is for util/schema.sh.

dR

Re: [pacman-dev] [PATCH v5 2/4] libmakepkg: add exists_function_variable helper

[Dave Reisner]

On Mon, Jan 21, 2019 at 07:40:14PM -0500, Dave Reisner wrote:
> On Mon, Jan 21, 2019 at 11:59:30PM +, morganamilo wrote:
> > This helpers functions allows checking for the existence of a package
> > variable without worrying if it is an array or not.
> 
> Seems reasonable, but where would this be used? Is this meant to
> consolidate existing cases of redundant code? Under what circumstances
> do we care about the variable existing without needing to know if it's
> an array or not?
> 

Ok, I see the followup patch where this is used. IMO, the patch using
this function for the first time ought to be also responsible for
defining it.

> > Signed-off-by: morganamilo 
> > ---
> >  scripts/libmakepkg/util/pkgbuild.sh.in | 9 +
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/scripts/libmakepkg/util/pkgbuild.sh.in 
> > b/scripts/libmakepkg/util/pkgbuild.sh.in
> > index b29229a3..f9fc440b 100644
> > --- a/scripts/libmakepkg/util/pkgbuild.sh.in
> > +++ b/scripts/libmakepkg/util/pkgbuild.sh.in
> > @@ -98,6 +98,15 @@ extract_function_variable() {
> > return $r
> >  }
> >  
> > +exists_function_variable() {
> > +   # $1: function name
> > +   # $2: variable name
> > +
> > +   local funcname=$1 attr=$2 out
> > +   extract_function_variable "$funcname" "$attr" 0 out || \
> 
> The explicit line continuation isn't needed here -- || at the end of the
> line indicates a compound command that needs more tokens to be lexed
> before bash can complete the input.
> 
> > +   extract_function_variable "$funcname" "$attr" 1 out
> > +}
> > +
> >  get_pkgbuild_attribute() {
> > # $1: package name
> > # $2: attribute name
> > -- 
> > 2.20.1

Re: [pacman-dev] [PATCH v5 2/4] libmakepkg: add exists_function_variable helper

[Dave Reisner]

On Mon, Jan 21, 2019 at 11:59:30PM +, morganamilo wrote:
> This helpers functions allows checking for the existence of a package
> variable without worrying if it is an array or not.

Seems reasonable, but where would this be used? Is this meant to
consolidate existing cases of redundant code? Under what circumstances
do we care about the variable existing without needing to know if it's
an array or not?

> Signed-off-by: morganamilo 
> ---
>  scripts/libmakepkg/util/pkgbuild.sh.in | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/scripts/libmakepkg/util/pkgbuild.sh.in 
> b/scripts/libmakepkg/util/pkgbuild.sh.in
> index b29229a3..f9fc440b 100644
> --- a/scripts/libmakepkg/util/pkgbuild.sh.in
> +++ b/scripts/libmakepkg/util/pkgbuild.sh.in
> @@ -98,6 +98,15 @@ extract_function_variable() {
>   return $r
>  }
>  
> +exists_function_variable() {
> + # $1: function name
> + # $2: variable name
> +
> + local funcname=$1 attr=$2 out
> + extract_function_variable "$funcname" "$attr" 0 out || \

The explicit line continuation isn't needed here -- || at the end of the
line indicates a compound command that needs more tokens to be lexed
before bash can complete the input.

> + extract_function_variable "$funcname" "$attr" 1 out
> +}
> +
>  get_pkgbuild_attribute() {
>   # $1: package name
>   # $2: attribute name
> -- 
> 2.20.1

Re: [pacman-dev] [PATCH 2/2] meson: add toplevel target to create the website

[Dave Reisner]

On Sun, Jan 13, 2019 at 10:38:30PM -0500, Eli Schwartz wrote:
> On 1/13/19 10:18 PM, Allan McRae wrote:
> > On 13/1/19 11:17 pm, Dave Reisner wrote:
> >> On Sat, Jan 12, 2019 at 09:15:01PM -0500, Eli Schwartz wrote:
> >>> Signed-off-by: Eli Schwartz 
> >>> ---
> >>
> >> This is really just an alias, right? To match what autotools does? Does
> >> anyone actually care about this? Might it be better to just rename the
> >> existing target to 'website' instead?
> > 
> > That seems the least redundant approach.
> 
> The existing target is specifically a file target, it's a command to
> create output. It only exists as:
> 
> ninja doc/website.tar.gz
> 
> The website target is a toplevel pseudo-target, you can run it directly
> from the build tree and it will depend on the tarball being up to date
> rather than always rerunning bsdtar. I don't like the idea of turning
> the whole thing into one always-rebuilding pseudo-target.

Why bother with this at all, then? Autotools offers website and
website.tar.gz, but both are in doc/. This target also is called
probably once per release. Aliasing it to some shorter (and sub-par)
seems not so useful...

I think we should just skip this...

Re: [pacman-dev] [PATCH 2/2] meson: add toplevel target to create the website

[Dave Reisner]

On Sat, Jan 12, 2019 at 09:15:01PM -0500, Eli Schwartz wrote:
> Signed-off-by: Eli Schwartz 
> ---

This is really just an alias, right? To match what autotools does? Does
anyone actually care about this? Might it be better to just rename the
existing target to 'website' instead?

>  doc/meson.build | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/meson.build b/doc/meson.build
> index 4d796492..5a1a1dc5 100644
> --- a/doc/meson.build
> +++ b/doc/meson.build
> @@ -104,7 +104,7 @@ run_target('html',
> command : ['/bin/true'],
> depends : html_targets)
>  
> -custom_target(
> +website_tar_gz = custom_target(
>'website.tar.gz',
>command : [
>  'bsdtar', 'czf', '@OUTPUT@',
> @@ -124,6 +124,10 @@ custom_target(
>depends : html_targets,
>  )
>  
> +run_target('website',
> +   command : ['/bin/true'],
> +   depends : website_tar_gz)
> +


>  meson.add_install_script(MESON_MAKE_SYMLINK,
>   'repo-add.8',
>   join_paths(MANDIR, 'man8/repo-remove.8'))
> -- 
> 2.20.1

Re: [pacman-dev] [PATCH 1/2] meson: fix website target

[Dave Reisner]

On Sat, Jan 12, 2019 at 09:15:00PM -0500, Eli Schwartz wrote:
> A number of pages don't actually exist as html inside the source tree,
> and need to be generated even though they are manpages.
> 
> This caused the website.tar.gz target to only work inside a dirty tree
> initially created by autotools.
> 
> Signed-off-by: Eli Schwartz 
> ---
>  doc/meson.build | 19 +++
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/meson.build b/doc/meson.build
> index 7c9631cb..4d796492 100644
> --- a/doc/meson.build
> +++ b/doc/meson.build
> @@ -14,6 +14,13 @@ manpages = [
>{ 'name': 'BUILDINFO.5' },
>  ]
>  
> +sitepages = [
> +  { 'name': 'submitting-patches' },
> +  { 'name': 'translation-help' },
> +  { 'name': 'HACKING', 'source': join_paths(meson.current_source_dir(), 
> '../HACKING') },

This would be nicer as join_paths(meson.source_root(), 'HACKING')

> +  { 'name': 'index' },
> +]
> +
>  asciidoc_conf = join_paths(meson.current_source_dir(), 'asciidoc.conf')
>  
>  asciidoc_opts = [
> @@ -58,6 +65,12 @@ foreach page : manpages
>  install : true,
>  install_dir : mandirn,
>)
> +endforeach
> +
> +foreach page: manpages + sitepages
> +  manpage = page['name']
> +  htmlpage = '@0@.html'.format(manpage)
> +  input = page.get('source', '@0@.asciidoc'.format(manpage))
>  
>html = custom_target(
>  htmlpage,
> @@ -85,6 +98,8 @@ foreach page : manpages
>html_files += [htmlpage]
>  endforeach
>  
> +
> +

One too many newlines?

>  run_target('html',
> command : ['/bin/true'],
> depends : html_targets)
> @@ -96,10 +111,6 @@ custom_target(
>  '-C', meson.current_build_dir(),
>] + html_files + [
>  '-C', meson.current_source_dir(),
> -'submitting-patches.html',
> -'translation-help.html',
> -'HACKING.html',
> -'index.html',
>  'asciidoc-override.css',
>  '-C', '/etc/asciidoc/stylesheets/',
>  'asciidoc.css',
> -- 
> 2.20.1

Re: [pacman-dev] [PATCH] Enable additional debug flags/logging with debugoptimized builds

[Dave Reisner]

On Tue, Dec 04, 2018 at 09:27:52AM -0500, Eli Schwartz wrote:
> On 12/4/18 7:56 AM, Dave Reisner wrote:
> > On Tue, Dec 04, 2018 at 05:14:50PM +1000, Allan McRae wrote:
> >> On 4/12/18 12:36 am, Dave Reisner wrote:
> >>> This lets developers run a local build with optimizations but also the
> >>> added debug logging that comes with PACMAN_DEBUG being defined.
> >>> ---
> >>
> >> So, we only test for "debug" to enable anything.  What options starting
> >> with debug would be used here?
> > 
> > meson understands --buildtype=debug and --buildtype=debugoptimized when
> > you configure the build. Before this patch, only the former buildtype is
> > accepted in order to trigger these extra flags. After, both buildtypes
> > are.
> > 
> >> Also, all those warning flags in this test are set with --enable-warning
> >> flags in autotools based builds.  Strangely, all the options from
> >> --enable-debug are not set here...
> > 
> > I chose not to add a -Ddebug=true flag to the meson build and instead
> > key off of the buildtype. With this patch, I'm essentially suggesting
> > that developers who want a suitable debugging build should just build as
> > --buildtype=(debug|debugoptimized). That gives you the extra warning
> > flags and extra timestamp logging just the same as --enable-debug and
> > --enable-warning-flags gave you with autotools.
> > 
> > Do you feel there's reason to separate these concerns?
> When using makepkg to build pacman-git, I use --buildtype=plain and let
> makepkg.conf handle debug builds. But maybe I still want the timestamping.

For c/c++ builds, --buildtype=debug isn't more than --buildtype=plain
with an added -g flag. I'm not sure how this would get in your way.

Re: [pacman-dev] [PATCH] Enable additional debug flags/logging with debugoptimized builds

[Dave Reisner]

On Tue, Dec 04, 2018 at 05:14:50PM +1000, Allan McRae wrote:
> On 4/12/18 12:36 am, Dave Reisner wrote:
> > This lets developers run a local build with optimizations but also the
> > added debug logging that comes with PACMAN_DEBUG being defined.
> > ---
> 
> So, we only test for "debug" to enable anything.  What options starting
> with debug would be used here?

meson understands --buildtype=debug and --buildtype=debugoptimized when
you configure the build. Before this patch, only the former buildtype is
accepted in order to trigger these extra flags. After, both buildtypes
are.

> Also, all those warning flags in this test are set with --enable-warning
> flags in autotools based builds.  Strangely, all the options from
> --enable-debug are not set here...

I chose not to add a -Ddebug=true flag to the meson build and instead
key off of the buildtype. With this patch, I'm essentially suggesting
that developers who want a suitable debugging build should just build as
--buildtype=(debug|debugoptimized). That gives you the extra warning
flags and extra timestamp logging just the same as --enable-debug and
--enable-warning-flags gave you with autotools.

Do you feel there's reason to separate these concerns?

> >  meson.build | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/meson.build b/meson.build
> > index 8837816f..394bd1f5 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -211,7 +211,7 @@ elif conf.has('HAVE_STRUCT_STATFS_F_FLAGS')
> >conf.set('FSSTATSTYPE', 'struct statfs')
> >  endif
> >  
> > -if get_option('buildtype') == 'debug'
> > +if get_option('buildtype').startswith('debug')
> >extra_cflags = [
> >  '-Wcast-align',
> >  '-Wclobbered',
> > 

[pacman-dev] [PATCH] Enable additional debug flags/logging with debugoptimized builds

[Dave Reisner]

This lets developers run a local build with optimizations but also the
added debug logging that comes with PACMAN_DEBUG being defined.
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 8837816f..394bd1f5 100644
--- a/meson.build
+++ b/meson.build
@@ -211,7 +211,7 @@ elif conf.has('HAVE_STRUCT_STATFS_F_FLAGS')
   conf.set('FSSTATSTYPE', 'struct statfs')
 endif
 
-if get_option('buildtype') == 'debug'
+if get_option('buildtype').startswith('debug')
   extra_cflags = [
 '-Wcast-align',
 '-Wclobbered',
-- 
2.19.2

[pacman-dev] [PATCH] scripts/meson: ensure wrapper scripts are executable

[Dave Reisner]

---
This is on top of my fixup branch

 scripts/meson.build | 21 +
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/scripts/meson.build b/scripts/meson.build
index aece7a22..344333be 100644
--- a/scripts/meson.build
+++ b/scripts/meson.build
@@ -52,21 +52,18 @@ foreach script : wrapped_scripts
 depend_files : library_files,
 build_by_default : true)
 
+  cdata = configuration_data()
+  cdata.set_quoted('BASH', BASH.path())
+  cdata.set_quoted('BUILDDIR', meson.current_build_dir())
+  cdata.set_quoted('REAL_PROGPATH', internal_script.full_path())
+
   # Create a wrapper script that bootstraps the real script within the build
-  # directory.
-  custom_target(
-'wrap_@0@'.format(script_shortname),
+  # directory. Use configure_file instead of a custom_target to ensure that
+  # permissions on the input script wrapper are preserved.
+  configure_file(
 input : join_paths(meson.source_root(), 'build-aux', 
'script-wrapper.sh.in'),
 output : script_shortname,
-build_by_default : true,
-command : [
-  SED,
-  '-e', 's,@BASH@,"@0@",'.format(BASH.path()),
-  '-e', 's,@BUILDDIR@,"@0@",'.format(meson.current_build_dir()),
-  '-e', 's,@REAL_PROGPATH@,"@0@",'.format(internal_script.full_path()),
-  '@INPUT@',
-],
-capture : true)
+configuration : cdata)
 
   # Install the real script
   meson.add_install_script(MESON_INSTALL_SCRIPT,
-- 
2.19.1

[pacman-dev] [PATCH 2/2] buildsys: remove size_to_human

[Dave Reisner]

This was only ever used by paccache, and paccache has since been moved
to pacman-contrib.
---
 scripts/Makefile.am  |  3 +--
 scripts/library/README   |  4 
 scripts/library/size_to_human.sh | 22 --
 scripts/meson.build  |  1 -
 scripts/po/POTFILES.in   |  1 -
 5 files changed, 1 insertion(+), 30 deletions(-)
 delete mode 100644 scripts/library/size_to_human.sh

diff --git a/scripts/Makefile.am b/scripts/Makefile.am
index c6b6220e..26c2e10d 100644
--- a/scripts/Makefile.am
+++ b/scripts/Makefile.am
@@ -35,8 +35,7 @@ EXTRA_DIST = \
$(LIBMAKEPKG_DIST)
 
 LIBRARY = \
-   library/human_to_size.sh \
-   library/size_to_human.sh
+   library/human_to_size.sh
 
 libmakepkgdir = $(datarootdir)/makepkg
 
diff --git a/scripts/library/README b/scripts/library/README
index 2b3a97bc..b99b0bc8 100644
--- a/scripts/library/README
+++ b/scripts/library/README
@@ -8,7 +8,3 @@ successful, the converted byte value is written to stdout and 
the function
 returns 0. If an error occurs, nothing in written and the function returns 1.
 Results may be inaccurate when using a broken implementation of awk, such
 as mawk or busybox awk.
-
-size_to_human.sh:
-The reverse of human_to_size, this function takes an integer byte size and
-prints its in human readable format, with SI prefixes (e.g. MiB, TiB).
diff --git a/scripts/library/size_to_human.sh b/scripts/library/size_to_human.sh
deleted file mode 100644
index 1d13eeb4..
--- a/scripts/library/size_to_human.sh
+++ /dev/null
@@ -1,22 +0,0 @@
-size_to_human() {
-   awk -v size="$1" '
-   BEGIN {
-   suffix[1] = "B"
-   suffix[2] = "KiB"
-   suffix[3] = "MiB"
-   suffix[4] = "GiB"
-   suffix[5] = "TiB"
-   suffix[6] = "PiB"
-   suffix[7] = "EiB"
-   count = 1
-
-   while (size > 1024) {
-   size /= 1024
-   count++
-   }
-
-   sizestr = sprintf("%.2f", size)
-   sub(/\.?0+$/, "", sizestr)
-   printf("%s %s", sizestr, suffix[count])
-   }'
-}
diff --git a/scripts/meson.build b/scripts/meson.build
index 50b0c34f..aece7a22 100644
--- a/scripts/meson.build
+++ b/scripts/meson.build
@@ -12,7 +12,6 @@ scripts = [
 
 library_files = [
   'library/human_to_size.sh',
-  'library/size_to_human.sh',
 ]
 
 SCRIPT_EDITOR = find_program(configure_file(
diff --git a/scripts/po/POTFILES.in b/scripts/po/POTFILES.in
index 37afc3b2..25eb9d3f 100644
--- a/scripts/po/POTFILES.in
+++ b/scripts/po/POTFILES.in
@@ -67,4 +67,3 @@ scripts/libmakepkg/util/pkgbuild.sh.in
 scripts/libmakepkg/util/source.sh.in
 scripts/libmakepkg/util/util.sh.in
 scripts/library/human_to_size.sh
-scripts/library/size_to_human.sh
-- 
2.19.1

[pacman-dev] [PATCH 1/2] meson: separate out wrapped from non-wrapped scripts

[Dave Reisner]

makepkg-template is a perl script and doesn't get wrapped by our shell
wrapper. It (wrongly) reads from the host machine rather than the build
root, but this is working as implemented.
---
wohoo first bug that affects one buildsys and not the other!

 scripts/meson.build | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/scripts/meson.build b/scripts/meson.build
index 535eccba..50b0c34f 100644
--- a/scripts/meson.build
+++ b/scripts/meson.build
@@ -1,5 +1,4 @@
-scripts = [
-  'makepkg-template.pl.in',
+wrapped_scripts = [
   'makepkg.sh.in',
   'pacman-db-upgrade.sh.in',
   'pacman-key.sh.in',
@@ -7,6 +6,10 @@ scripts = [
   'repo-add.sh.in'
 ]
 
+scripts = [
+  'makepkg-template.pl.in',
+]
+
 library_files = [
   'library/human_to_size.sh',
   'library/size_to_human.sh',
@@ -26,6 +29,19 @@ m4_edit = generator(
 foreach script : scripts
   script_shortname = script.split('.')[0]
 
+  custom_target(
+script,
+input : m4_edit.process(script),
+command : [ SCRIPT_EDITOR, '@INPUT@', '@OUTPUT@', '0755'],
+output : script_shortname,
+depend_files : library_files,
+install : true,
+install_dir : get_option('bindir'))
+endforeach
+
+foreach script : wrapped_scripts
+  script_shortname = script.split('.')[0]
+
   # Build the script, but don't install it. We want to keep it as a "private"
   # artifact that we reference from a wrapper script in order to bootstrap it
   # the build directory.
-- 
2.19.1

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

[Dave Reisner]

On Thu, Nov 01, 2018 at 06:38:03PM -0700, Andrew Gregory wrote:
> 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.

Ah yes, we're saying the same thing. Fixed on my branch.

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

[Dave Reisner]

On Thu, Nov 01, 2018 at 01:03:27AM -0700, Andrew Gregory wrote:
> 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?
> 

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.

> -- >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?

I'll think about how to do this... I'm not crazy about the idea of
making configuration of the meson build anything more than just 'meson
build', and this all needs to ready for processing at the time meson
generates the inputs for ninja.

[pacman-dev] [PATCH] meson: add a wrapper to bootstrap scripts from within build dir

[Dave Reisner]

This doesn't do quite as good of a job of "hiding away" the real script
as we did with autotools, but it satisfies the need for being able to
run scripts which depend on libmakepkg with the local copy within the
repo. We do, however, improve upon the autotools script by ensuring that
the bash path used in configuring pacman is the interpreter used to run
the underlying script.
---
Naturally, this applies on top of my previous meson patch.

 build-aux/meson-install-script.sh |  6 ++
 build-aux/script-wrapper.sh.in|  6 ++
 meson.build   |  1 +
 scripts/meson.build   | 33 +++
 4 files changed, 42 insertions(+), 4 deletions(-)
 create mode 100644 build-aux/meson-install-script.sh
 create mode 100755 build-aux/script-wrapper.sh.in

diff --git a/build-aux/meson-install-script.sh 
b/build-aux/meson-install-script.sh
new file mode 100644
index ..f5a42fca
--- /dev/null
+++ b/build-aux/meson-install-script.sh
@@ -0,0 +1,6 @@
+#!/bin/sh
+
+built_script=$1
+dest_path=$2
+
+install -Dm755 "$built_script" "$DESTDIR/$dest_path"
diff --git a/build-aux/script-wrapper.sh.in b/build-aux/script-wrapper.sh.in
new file mode 100755
index ..f87ae6f0
--- /dev/null
+++ b/build-aux/script-wrapper.sh.in
@@ -0,0 +1,6 @@
+#!/bin/bash
+
+# This script serves as a trampoline for running scripts which depend on
+# libmakepkg with the libmakepkg within the build tree.
+
+LIBRARY=@BUILDDIR@/libmakepkg exec @BASH@ -$- @REAL_PROGPATH@ "$@"
diff --git a/meson.build b/meson.build
index d81c86b7..1ec63a56 100644
--- a/meson.build
+++ b/meson.build
@@ -33,6 +33,7 @@ SED = find_program('sed')
 DU = find_program('du')
 LDCONFIG = get_option('ldconfig')
 MESON_MAKE_SYMLINK = join_paths(meson.source_root(), 
'build-aux/meson-make-symlink.sh')
+MESON_INSTALL_SCRIPT = join_paths(meson.source_root(), 
'build-aux/meson-install-script.sh')
 
 BASH = find_program('bash4', 'bash')
 if BASH.found()
diff --git a/scripts/meson.build b/scripts/meson.build
index 1fe3fb78..535eccba 100644
--- a/scripts/meson.build
+++ b/scripts/meson.build
@@ -24,14 +24,39 @@ m4_edit = generator(
   capture : true)
 
 foreach script : scripts
-  custom_target(
+  script_shortname = script.split('.')[0]
+
+  # Build the script, but don't install it. We want to keep it as a "private"
+  # artifact that we reference from a wrapper script in order to bootstrap it
+  # the build directory.
+  internal_script = custom_target(
 script,
 input : m4_edit.process(script),
 command : [ SCRIPT_EDITOR, '@INPUT@', '@OUTPUT@', '0755'],
-output : script.split('.')[0],
+output : script,
 depend_files : library_files,
-install : true,
-install_dir : get_option('bindir'))
+build_by_default : true)
+
+  # Create a wrapper script that bootstraps the real script within the build
+  # directory.
+  custom_target(
+'wrap_@0@'.format(script_shortname),
+input : join_paths(meson.source_root(), 'build-aux', 
'script-wrapper.sh.in'),
+output : script_shortname,
+build_by_default : true,
+command : [
+  SED,
+  '-e', 's,@BASH@,"@0@",'.format(BASH.path()),
+  '-e', 's,@BUILDDIR@,"@0@",'.format(meson.current_build_dir()),
+  '-e', 's,@REAL_PROGPATH@,"@0@",'.format(internal_script.full_path()),
+  '@INPUT@',
+],
+capture : true)
+
+  # Install the real script
+  meson.add_install_script(MESON_INSTALL_SCRIPT,
+   internal_script.full_path(),
+   join_paths(BINDIR, script_shortname))
 endforeach
 
 foreach symlink : ['repo-remove', 'repo-elephant']
-- 
2.19.1

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

[Dave Reisner]

On Sun, Oct 21, 2018 at 10:11:18PM -0400, Dave Reisner wrote:
> On Mon, Oct 22, 2018 at 11:16:25AM +1000, Allan McRae wrote:
> > On 22/10/18 10:03 am, Dave Reisner wrote:
> > > On Mon, Oct 22, 2018 at 09:56:04AM +1000, Allan McRae wrote:
> > >> On 22/10/18 8:57 am, Eli Schwartz wrote:
> > >>>> Also, most projects ship prebuilt man pages. I think some GNU ones
> > >>>> commit the built page to the tree, so that could be an option.
> > >>
> > >>> Both those things are also true about the configure and Makefile.in 
> > >>> files...
> > >>
> > >> That is another good reason not to use git archive to create release
> > >> tarballs.
> > >>
> > >> A
> > > 
> > > Consider that the reason we need 'make dist' and can't just use 'git
> > > archive' is because autotools *requires* extra tooling around just to
> > > generate the build system. This is no longer needed with meson.
> > 
> > Good point!  I'm not overly familiar with meson, so missed that completely.
> > 
> > > Pre-generating the manpages means that our substitutions are
> > > invalidated. For example, someone building from the tarball with
> > > --prefix=/some/where/else or no --prefix at all will have a
> > > pacman.conf(5) that still claims the right path is /etc/pacman.conf
> > > 
> > > I understand your point about python2 vs python3, but I think that's
> > > something we can work out based on a python3-based asciidoc in
> > > existance.
> > 
> > Does the meson build script allow us to not create docs if wanted?
> 
> Yes, see the meson_options.txt as part of this patch. One can invoke
> meson with -Ddoc=disabled to prevent manpages from being built.
> 
> > > What problems here would you like to see solved for the inclusion of
> > > meson? That will help me focus my time spent on this.
> > 
> > None need solved for inclusion in the current state.  I'm just trying to
> > understand what our future release process will look like.
> 
> Great!
> 
> > While we have autotools in parallel, if I run "make dist" with this
> > patch, do all the needed meson files get included too?   Can we get a
> > helper script to run doxygen?
> 
> No, meson.build files will not be included with the autotools-built
> 'make dist' tarball. It feels weird to do this. I suspect that if we
> want to switch the Arch build over while this dual build-system world
> exists, we should just build from a tag in the git repo.
> 
> Yes, I can add a script in build-aux to invoke doxygen. It's also
> possible I could create the Doxyfile as a .in file, and generate the
> right OUTPUT_DIRECTORY for invocation through the buildsys. There's
> actually an upstream example of exactly this:
> 
> https://github.com/mesonbuild/meson/blob/master/test%20cases/frameworks/14%20doxygen/doc/meson.build
> 
> I'll look into options here.

Figured this out, there's now a -Ddoxygen flag on the build system which
will build and install the doxygen crap if wanted. Adding this requires
a slight tweak on the autotools side to handle the Doxyfile ->
Doxyfile.in rename and subsequent generation.

dR

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

[Dave Reisner]

On Mon, Oct 22, 2018 at 11:16:25AM +1000, Allan McRae wrote:
> On 22/10/18 10:03 am, Dave Reisner wrote:
> > On Mon, Oct 22, 2018 at 09:56:04AM +1000, Allan McRae wrote:
> >> On 22/10/18 8:57 am, Eli Schwartz wrote:
> >>>> Also, most projects ship prebuilt man pages. I think some GNU ones
> >>>> commit the built page to the tree, so that could be an option.
> >>
> >>> Both those things are also true about the configure and Makefile.in 
> >>> files...
> >>
> >> That is another good reason not to use git archive to create release
> >> tarballs.
> >>
> >> A
> > 
> > Consider that the reason we need 'make dist' and can't just use 'git
> > archive' is because autotools *requires* extra tooling around just to
> > generate the build system. This is no longer needed with meson.
> 
> Good point!  I'm not overly familiar with meson, so missed that completely.
> 
> > Pre-generating the manpages means that our substitutions are
> > invalidated. For example, someone building from the tarball with
> > --prefix=/some/where/else or no --prefix at all will have a
> > pacman.conf(5) that still claims the right path is /etc/pacman.conf
> > 
> > I understand your point about python2 vs python3, but I think that's
> > something we can work out based on a python3-based asciidoc in
> > existance.
> 
> Does the meson build script allow us to not create docs if wanted?

Yes, see the meson_options.txt as part of this patch. One can invoke
meson with -Ddoc=disabled to prevent manpages from being built.

> > What problems here would you like to see solved for the inclusion of
> > meson? That will help me focus my time spent on this.
> 
> None need solved for inclusion in the current state.  I'm just trying to
> understand what our future release process will look like.

Great!

> While we have autotools in parallel, if I run "make dist" with this
> patch, do all the needed meson files get included too?   Can we get a
> helper script to run doxygen?

No, meson.build files will not be included with the autotools-built
'make dist' tarball. It feels weird to do this. I suspect that if we
want to switch the Arch build over while this dual build-system world
exists, we should just build from a tag in the git repo.

Yes, I can add a script in build-aux to invoke doxygen. It's also
possible I could create the Doxyfile as a .in file, and generate the
right OUTPUT_DIRECTORY for invocation through the buildsys. There's
actually an upstream example of exactly this:

https://github.com/mesonbuild/meson/blob/master/test%20cases/frameworks/14%20doxygen/doc/meson.build

I'll look into options here.

> Thanks,
> Allan

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

[Dave Reisner]

On Mon, Oct 22, 2018 at 09:56:04AM +1000, Allan McRae wrote:
> On 22/10/18 8:57 am, Eli Schwartz wrote:
> >> Also, most projects ship prebuilt man pages. I think some GNU ones
> >> commit the built page to the tree, so that could be an option.
> 
> > Both those things are also true about the configure and Makefile.in files...
> 
> That is another good reason not to use git archive to create release
> tarballs.
> 
> A

Consider that the reason we need 'make dist' and can't just use 'git
archive' is because autotools *requires* extra tooling around just to
generate the build system. This is no longer needed with meson.

Pre-generating the manpages means that our substitutions are
invalidated. For example, someone building from the tarball with
--prefix=/some/where/else or no --prefix at all will have a
pacman.conf(5) that still claims the right path is /etc/pacman.conf

I understand your point about python2 vs python3, but I think that's
something we can work out based on a python3-based asciidoc in
existance.

What problems here would you like to see solved for the inclusion of
meson? That will help me focus my time spent on this.

dR

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

[Dave Reisner]

On Mon, Oct 22, 2018 at 07:54:57AM +1000, Allan McRae wrote:
> On 22/10/18 7:46 am, Dave Reisner wrote:
> > 3) No 'make dist' equivalent. Just run 'git archive' to generate a
> > suitable tarball for distribution.
> 
> Is there a way to include pregenerated man pages with git archive?
> 
> A

No, git archive only considers what's in the git tree. Is there a reason
we're determined to pre-generate the manpages instead of just adding
makedepends on ascididoc?

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

[Dave Reisner]

Provide both build systems in parallel for now, to ensure that we work
out all the differences between the two. Some time from now, we'll give
up on autotools.

Meson tends to be faster and probably easier to read/maintain. On my
machine, the full meson configure+build+install takes a little under
half as long as a similar autotools-based invocation.

Building with meson is a two step process. First, configure the build:

  meson build

Then, compile the project:

  ninja -C build

There's some mild differences in functionality between meson and
autotools.  specifically:

1) No singular update-po target. meson only generates individual
update-po targets for each textdomain (of which we have 3).  To make
this easier, there's a build-aux/update-po script which finds all
update-po targets and runs them.

2) No doxygen support. Doxygen seems to be terrible and requires that
everything be built through the Doxyfile. Support for out-of-tree builds
(enforced by meson) appears to be nonexistant. I'd suggest just running
doxygen directly since we never package these files and just export them
for the website.

3) No 'make dist' equivalent. Just run 'git archive' to generate a
suitable tarball for distribution.
---
Yes, this is huge. All 330 tests pass, and I've been building/using
pacman from my 'meson' branch for a while now.

Feedback wanted.

 .ycm_extra_conf.py   | 250 ++
 build-aux/edit-script.sh.in  |  33 ++
 build-aux/meson-make-symlink.sh  |  12 +
 build-aux/tap-driver.py  | 296 +++
 build-aux/update-po  |  39 ++
 doc/meson.build  | 118 +
 lib/libalpm/meson.build  |  33 ++
 lib/libalpm/po/meson.build   |  15 +
 meson.build  | 487 +++
 meson_options.txt|  58 +++
 scripts/libmakepkg/integrity/meson.build |  20 +
 scripts/libmakepkg/lint_config/meson.build   |  18 +
 scripts/libmakepkg/lint_package/meson.build  |  20 +
 scripts/libmakepkg/lint_pkgbuild/meson.build |  37 ++
 scripts/libmakepkg/meson.build   |  31 ++
 scripts/libmakepkg/source/meson.build|  22 +
 scripts/libmakepkg/tidy/meson.build  |  23 +
 scripts/libmakepkg/util/meson.build  |  24 +
 scripts/meson.build  |  66 +++
 scripts/po/meson.build   |  15 +
 src/common/meson.build   |   4 +
 src/pacman/meson.build   |  23 +
 src/pacman/po/meson.build|  15 +
 src/util/meson.build |   3 +
 test/pacman/meson.build  | 357 ++
 test/scripts/meson.build |  12 +
 test/util/meson.build|   6 +
 27 files changed, 2037 insertions(+)
 create mode 100644 .ycm_extra_conf.py
 create mode 100644 build-aux/edit-script.sh.in
 create mode 100644 build-aux/meson-make-symlink.sh
 create mode 100644 build-aux/tap-driver.py
 create mode 100755 build-aux/update-po
 create mode 100644 doc/meson.build
 create mode 100644 lib/libalpm/meson.build
 create mode 100644 lib/libalpm/po/meson.build
 create mode 100644 meson.build
 create mode 100644 meson_options.txt
 create mode 100644 scripts/libmakepkg/integrity/meson.build
 create mode 100644 scripts/libmakepkg/lint_config/meson.build
 create mode 100644 scripts/libmakepkg/lint_package/meson.build
 create mode 100644 scripts/libmakepkg/lint_pkgbuild/meson.build
 create mode 100644 scripts/libmakepkg/meson.build
 create mode 100644 scripts/libmakepkg/source/meson.build
 create mode 100644 scripts/libmakepkg/tidy/meson.build
 create mode 100644 scripts/libmakepkg/util/meson.build
 create mode 100644 scripts/meson.build
 create mode 100644 scripts/po/meson.build
 create mode 100644 src/common/meson.build
 create mode 100644 src/pacman/meson.build
 create mode 100644 src/pacman/po/meson.build
 create mode 100644 src/util/meson.build
 create mode 100644 test/pacman/meson.build
 create mode 100644 test/scripts/meson.build
 create mode 100644 test/util/meson.build

diff --git a/.ycm_extra_conf.py b/.ycm_extra_conf.py
new file mode 100644
index ..f297deef
--- /dev/null
+++ b/.ycm_extra_conf.py
@@ -0,0 +1,250 @@
+#!/usr/bin/env python
+
+# SPDX-License-Identifier: Unlicense
+#
+# Based on the template file provided by the 'YCM-Generator' project authored 
by
+# Reuben D'Netto.
+# Jiahui Xie has re-reformatted and expanded the original script in accordance
+# to the requirements of the PEP 8 style guide and 'systemd' project,
+# respectively.
+#
+# The original license is preserved as it is.
+#
+#
+# This is free and unencumbered software released into the public domain.
+#
+# Anyone is free to copy, modify, publish, use, compile, sell, or
+# distribute this software, either in source code form or as a compiled
+# binary, for any purpose, 

[pacman-dev] [PATCH v2] Port pactest to python3

[Dave Reisner]

Use BytesIO instead of StringIO, and ensure that we unicode-encode data
where needed.
---
* Make sure pactest --review is happy

 configure.ac   | 2 +-
 test/pacman/pactest.py | 5 +++--
 test/pacman/pmdb.py| 4 ++--
 test/pacman/pmpkg.py   | 6 +++---
 test/pacman/util.py| 2 +-
 5 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/configure.ac b/configure.ac
index a569ffeb..c369ca74 100644
--- a/configure.ac
+++ b/configure.ac
@@ -179,7 +179,7 @@ AC_SUBST(LFS_CFLAGS)
 AC_PROG_AWK
 AC_PROG_CC_C99
 AC_PROG_INSTALL
-AC_CHECK_PROGS([PYTHON], [python2.7 python2 python], [false])
+AC_CHECK_PROGS([PYTHON], [python3 python], [false])
 AC_PATH_PROGS([BASH_SHELL], [bash bash4], [false])
 
 # check for perl 5.10.1 (needed by makepkg-template)
diff --git a/test/pacman/pactest.py b/test/pacman/pactest.py
index 1f5b8483..85cce6a1 100755
--- a/test/pacman/pactest.py
+++ b/test/pacman/pactest.py
@@ -1,4 +1,4 @@
-#! /usr/bin/python2
+#! /usr/bin/python3
 #
 #  pactest : run automated testing on the pacman binary
 #
@@ -45,7 +45,8 @@ def write(self, message):
 # duplicate stdout/stderr to a temporary file
 class OutputSaver():
 def __init__(self):
-self.save_file = tempfile.NamedTemporaryFile(prefix='pactest-output-')
+self.save_file = tempfile.NamedTemporaryFile(
+prefix='pactest-output-', mode='w')
 
 def __enter__(self):
 sys.stdout = MultiWriter(sys.stdout, self.save_file)
diff --git a/test/pacman/pmdb.py b/test/pacman/pmdb.py
index f7671987..95aa8756 100644
--- a/test/pacman/pmdb.py
+++ b/test/pacman/pmdb.py
@@ -15,9 +15,9 @@
 #  along with this program.  If not, see .
 
 
+from io import BytesIO
 import os
 import shutil
-from StringIO import StringIO
 import tarfile
 
 import pmpkg
@@ -251,7 +251,7 @@ def generate(self):
 filename = os.path.join(pkg.fullname(), name)
 info = tarfile.TarInfo(filename)
 info.size = len(data)
-tar.addfile(info, StringIO(data))
+tar.addfile(info, BytesIO(data.encode('utf8')))
 tar.close()
 # TODO: this is a bit unnecessary considering only one test uses it
 serverpath = os.path.join(self.root, util.SYNCREPO, self.treename)
diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py
index 5a32ccd6..4667ebc1 100644
--- a/test/pacman/pmpkg.py
+++ b/test/pacman/pmpkg.py
@@ -14,8 +14,8 @@
 #  You should have received a copy of the GNU General Public License
 #  along with this program.  If not, see .
 
+from io import BytesIO
 import os
-from StringIO import StringIO
 import tarfile
 
 import util
@@ -146,7 +146,7 @@ def makepkg(self, path):
 for name, data in archive_files:
 info = tarfile.TarInfo(name)
 info.size = len(data)
-tar.addfile(info, StringIO(data))
+tar.addfile(info, BytesIO(data.encode('utf8')))
 
 # Generate package file system
 for name in self.files:
@@ -167,7 +167,7 @@ def makepkg(self, path):
 # TODO wow what a hack, adding a newline to match mkfile?
 filedata = name + "\n"
 info.size = len(filedata)
-tar.addfile(info, StringIO(filedata))
+tar.addfile(info, BytesIO(filedata.encode('utf8')))
 
 tar.close()
 
diff --git a/test/pacman/util.py b/test/pacman/util.py
index 5fbe4c35..544bdd8d 100644
--- a/test/pacman/util.py
+++ b/test/pacman/util.py
@@ -152,7 +152,7 @@ def getmd5sum(filename):
 
 def mkmd5sum(data):
 checksum = hashlib.md5()
-checksum.update("%s\n" % data)
+checksum.update(("%s\n" % data).encode('utf8'))
 return checksum.hexdigest()
 
 
-- 
2.19.1

[pacman-dev] [PATCH 2/2] Drop vestiges of SIZECMD

[Dave Reisner]

SIZECMD was replaced in 1af766987f with a POSIX solution, and this token
is no longer used/needed.
---
 scripts/Makefile.am | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scripts/Makefile.am b/scripts/Makefile.am
index f83e16c0..462962e1 100644
--- a/scripts/Makefile.am
+++ b/scripts/Makefile.am
@@ -161,7 +161,6 @@ edit = sed \
-e "s|@INODECMD[@]|$(INODECMD)|g" \
-e "s|@OWNERCMD[@]|$(OWNERCMD)|g" \
-e "s|@MODECMD[@]|$(MODECMD)|g" \
-   -e 's|@SIZECMD[@]|$(SIZECMD)|g' \
-e 's|@SEDINPLACEFLAGS[@]|$(SEDINPLACEFLAGS)|g' \
-e 's|@SEDPATH[@]|$(SEDPATH)|g' \
-e 's|@DUFLAGS[@]|$(DUFLAGS)|g' \
-- 
2.19.0

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

[Dave Reisner]

Use BytesIO instead of StringIO, and ensure that we unicode-encode data
where needed.
---
 configure.ac   | 2 +-
 test/pacman/pactest.py | 2 +-
 test/pacman/pmdb.py| 5 +++--
 test/pacman/pmpkg.py   | 8 +---
 test/pacman/util.py| 2 +-
 5 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/configure.ac b/configure.ac
index a569ffeb..c369ca74 100644
--- a/configure.ac
+++ b/configure.ac
@@ -179,7 +179,7 @@ AC_SUBST(LFS_CFLAGS)
 AC_PROG_AWK
 AC_PROG_CC_C99
 AC_PROG_INSTALL
-AC_CHECK_PROGS([PYTHON], [python2.7 python2 python], [false])
+AC_CHECK_PROGS([PYTHON], [python3 python], [false])
 AC_PATH_PROGS([BASH_SHELL], [bash bash4], [false])
 
 # check for perl 5.10.1 (needed by makepkg-template)
diff --git a/test/pacman/pactest.py b/test/pacman/pactest.py
index 1f5b8483..628193ff 100755
--- a/test/pacman/pactest.py
+++ b/test/pacman/pactest.py
@@ -1,4 +1,4 @@
-#! /usr/bin/python2
+#! /usr/bin/python3
 #
 #  pactest : run automated testing on the pacman binary
 #
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
+
 import pmpkg
 import tap
 import util
@@ -251,7 +252,7 @@ def generate(self):
 filename = os.path.join(pkg.fullname(), name)
 info = tarfile.TarInfo(filename)
 info.size = len(data)
-tar.addfile(info, StringIO(data))
+tar.addfile(info, BytesIO(data.encode('utf8')))
 tar.close()
 # TODO: this is a bit unnecessary considering only one test uses it
 serverpath = os.path.join(self.root, util.SYNCREPO, self.treename)
diff --git a/test/pacman/pmpkg.py b/test/pacman/pmpkg.py
index 5a32ccd6..e42bd4d5 100644
--- a/test/pacman/pmpkg.py
+++ b/test/pacman/pmpkg.py
@@ -15,7 +15,9 @@
 #  along with this program.  If not, see .
 
 import os
-from StringIO import StringIO
+
+from io import BytesIO
+
 import tarfile
 
 import util
@@ -146,7 +148,7 @@ def makepkg(self, path):
 for name, data in archive_files:
 info = tarfile.TarInfo(name)
 info.size = len(data)
-tar.addfile(info, StringIO(data))
+tar.addfile(info, BytesIO(data.encode('utf8')))
 
 # Generate package file system
 for name in self.files:
@@ -167,7 +169,7 @@ def makepkg(self, path):
 # TODO wow what a hack, adding a newline to match mkfile?
 filedata = name + "\n"
 info.size = len(filedata)
-tar.addfile(info, StringIO(filedata))
+tar.addfile(info, BytesIO(filedata.encode('utf8')))
 
 tar.close()
 
diff --git a/test/pacman/util.py b/test/pacman/util.py
index 5fbe4c35..544bdd8d 100644
--- a/test/pacman/util.py
+++ b/test/pacman/util.py
@@ -152,7 +152,7 @@ def getmd5sum(filename):
 
 def mkmd5sum(data):
 checksum = hashlib.md5()
-checksum.update("%s\n" % data)
+checksum.update(("%s\n" % data).encode('utf8'))
 return checksum.hexdigest()
 
 
-- 
2.19.0

[pacman-dev] [PATCH 1/3] common/ini: Depend on util-common, not util

[Dave Reisner]

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

diff --git a/src/common/ini.c b/src/common/ini.c
index 9d656781..8e646528 100644
--- a/src/common/ini.c
+++ b/src/common/ini.c
@@ -24,7 +24,7 @@
 #include 
 
 #include "ini.h"
-#include "util.h"
+#include "util-common.h"
 
 /**
  * @brief Parse a pacman-style INI config file.
-- 
2.18.0

[pacman-dev] [PATCH 2/3] Remove unused checks for strcoll and mktime

[Dave Reisner]

We don't use these.
---
 configure.ac | 2 --
 1 file changed, 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5ca5bdd0..a569ffeb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -329,8 +329,6 @@ PATH_MAX_DEFINED
 AC_FUNC_FORK
 AC_FUNC_GETMNTENT
 AC_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK
-AC_FUNC_MKTIME
-AC_FUNC_STRCOLL
 AC_CHECK_FUNCS([dup2 getcwd getmntinfo gettimeofday memmove memset \
 mkdir realpath regcomp rmdir setenv setlocale strcasecmp \
 strchr strcspn strdup strerror strndup strnlen strrchr \
-- 
2.18.0

[pacman-dev] [PATCH 3/3] doc: use more implicit rules to build manpages

[Dave Reisner]

Use implicit dependency rules to translate asciidoc inputs to HTML and
manpage outputs. We should only have to declare explicit dependencies
for odd cases, e.g. the PKGBUILD documentation has an additional include
file and isn't a 1:1 conversion.
---
 doc/Makefile.am | 34 --
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/doc/Makefile.am b/doc/Makefile.am
index 8dec4ab1..2ac38cba 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -3,7 +3,7 @@
 # files listed in EXTRA_DIST no matter what. However, we only add them to
 # man_MANS if --enable-asciidoc and/or --enable-doxygen are used.
 
-ASCIIDOC_MANS = \
+MANPAGES = \
alpm-hooks.5 \
pacman.8 \
makepkg.8 \
@@ -66,11 +66,11 @@ EXTRA_DIST = \
submitting-patches.asciidoc \
translation-help.asciidoc \
Doxyfile \
-   $(ASCIIDOC_MANS) \
+   $(MANPAGES) \
$(DOXYGEN_MANS)
 
 # Files that should be removed, but which Automake does not know.
-MOSTLYCLEANFILES = *.xml $(ASCIIDOC_MANS) $(HTML_DOCS) repo-remove.8 
website.tar.gz
+MOSTLYCLEANFILES = *.xml $(MANPAGES) $(HTML_DOCS) repo-remove.8 website.tar.gz
 
 # Ensure manpages are fresh when building a dist tarball
 dist-hook:
@@ -85,7 +85,7 @@ REAL_PACKAGE_VERSION = $(PACKAGE_VERSION)
 endif
 
 man_MANS =
-dist_man_MANS = $(ASCIIDOC_MANS)
+dist_man_MANS = $(MANPAGES)
 
 if USE_DOXYGEN
 man_MANS += $(DOXYGEN_MANS)
@@ -96,6 +96,7 @@ doxygen.in:
$(DOXYGEN) $(srcdir)/Doxyfile
 endif
 
+man: $(MANPAGES)
 html: $(HTML_DOCS)
 
 website: website.tar.gz
@@ -129,11 +130,12 @@ A2X_OPTS = \
-f manpage \
--xsltproc-opts='-param man.endnotes.list.enabled 0 -param 
man.endnotes.are.numbered 0'
 
-# These rules are due to the includes and files of the asciidoc text
-$(ASCIIDOC_MANS): asciidoc.conf footer.asciidoc Makefile.am
+# Generate manpages
+%: %.asciidoc asciidoc.conf footer.asciidoc Makefile.am
$(AM_V_GEN)a2x $(A2X_OPTS) --asciidoc-opts="$(ASCIIDOC_OPTS) 
--out-file=./$@.xml" $(srcdir)/$@.asciidoc
 
-%.html: %.asciidoc
+# Generate HTML pages
+%.html: %.asciidoc asciidoc.conf footer.asciidoc Makefile.am
$(AM_V_GEN)asciidoc $(ASCIIDOC_OPTS) -o - $*.asciidoc | \
sed -e 's/\r$$//' > $@
 
@@ -142,27 +144,15 @@ HACKING.html: ../HACKING
sed -e 's/\r$$//' > $@
 
 # Customizations for certain HTML docs
-$(HTML_MANPAGES): asciidoc.conf footer.asciidoc Makefile.am
-$(HTML_OTHER): asciidoc.conf Makefile.am
 %.html: ASCIIDOC_OPTS += -a linkcss -a toc -a icons -a max-width=960px -a 
stylesheet=asciidoc-override.css
 %.8.html: ASCIIDOC_OPTS += -d manpage
 %.5.html: ASCIIDOC_OPTS += -d manpage
 %.3.html: ASCIIDOC_OPTS += -d manpage
 
-# Dependency rules
-alpm-hooks.5 alpm-hooks.5.html: alpm-hooks.5.asciidoc
-pacman.8 pacman.8.html: pacman.8.asciidoc
-makepkg.8 makepkg.8.html: makepkg.8.asciidoc
-makepkg-template.1 makepkg-template.1.html: makepkg-template.1.asciidoc
-repo-add.8 repo-add.8.html: repo-add.8.asciidoc
-vercmp.8 vercmp.8.html: vercmp.8.asciidoc
-pkgdelta.8 pkgdelta.8.html: pkgdelta.8.asciidoc
-pacman-key.8 pacman-key.8.html: pacman-key.8.asciidoc
+# Custom dependency rules
 PKGBUILD.5 PKGBUILD.5.html: PKGBUILD.5.asciidoc PKGBUILD-example.txt
-makepkg.conf.5 makepkg.conf.5.html: makepkg.conf.5.asciidoc
-pacman.conf.5 pacman.conf.5.html: pacman.conf.5.asciidoc
-libalpm.3 libalpm.3.html: libalpm.3.asciidoc
-# this one is just a symlink
+
+# Manpages as symlinks
 repo-remove.8: repo-add.8
$(RM) repo-remove.8
$(LN_S) repo-add.8 repo-remove.8
-- 
2.18.0

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

[Dave Reisner]

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

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

Anyone else have an opinion?

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

[pacman-dev] [PATCH v2] Merge expac into src/pacman

[Dave Reisner]

sum
+
+  %Vpackage validation method
+
+  %ihas install scriptlet (only with -Q)
+
+  %kdownload size (only with -S)
+
+  %linstall date (only with -Q)
+
+  %minstall size
+
+  %Mmodified backup files (only with -Q)
+
+  %npackage name
+
+  %ppackager name
+
+  %rrepo
+
+  %smd5sum
+
+  %uproject URL
+
+  %vversion
+
+  %winstall reason (only with -Q)
+
+  %!result number (auto-incremented counter, starts at 0)
+
+  %%literal %
+
+Note that for any lowercase tokens aside from %m and %k, full printf support is
+allowed, e.g. %-20n. This does not apply to any list based, date, or numerical
+output.
+
+Standard backslash escape sequences are supported, as per linkman:printf[1].
+
+Examples
+
+
+expac -Ss \'%r/%n %v\n%d\' ::
+   Emulate pacman\'s search function.
+
+expac --timefmt=%s \'%b\t%n\' | sort -n | head -10::
+   List the oldest 10 installed packages (by build date).
+
+See Also
+
+linkman:libalpm[3], linkman:pacman.conf[5]
+
+include::footer.asciidoc[]
diff --git a/src/pacman/.gitignore b/src/pacman/.gitignore
index 9889c35e..78b78159 100644
--- a/src/pacman/.gitignore
+++ b/src/pacman/.gitignore
@@ -5,3 +5,5 @@ pacman
 pacman.exe
 pacman-conf
 pacman-conf.exe
+expac
+expac.exe
diff --git a/src/pacman/Makefile.am b/src/pacman/Makefile.am
index 15cf20ce..deb386ab 100644
--- a/src/pacman/Makefile.am
+++ b/src/pacman/Makefile.am
@@ -21,7 +21,7 @@ libbasic_la_SOURCES = \
 libbasic_la_LIBADD = \
$(top_builddir)/lib/libalpm/.libs/libalpm.la
 
-bin_PROGRAMS = pacman pacman-conf
+bin_PROGRAMS = pacman pacman-conf expac
 
 AM_CPPFLAGS = \
-imacros $(top_builddir)/config.h \
@@ -66,3 +66,9 @@ pacman_conf_SOURCES = pacman-conf.c
 pacman_conf_LDADD = \
libbasic.la \
$(top_builddir)/lib/libalpm/.libs/libalpm.la
+
+expac_SOURCES = expac.c
+
+expac_LDADD = \
+   libbasic.la \
+   $(top_builddir)/lib/libalpm/.libs/libalpm.la
diff --git a/src/pacman/expac.c b/src/pacman/expac.c
new file mode 100644
index ..7d993cf6
--- /dev/null
+++ b/src/pacman/expac.c
@@ -0,0 +1,858 @@
+/*
+ *  expac.c
+ *
+ *  Copyright (c) 2018 Pacman Development Team 
+ *  Copyright (c) 2010-2018 by Dave Reisner 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "conf.h"
+#include "util.h"
+
+#define DEFAULT_DELIM"\n"
+#define DEFAULT_LISTDELIM"  "
+#define DEFAULT_TIMEFMT  "%c"
+#define SIZE_TOKENS  "BKMGTPEZY\0"
+
+#if defined(GIT_VERSION)
+#undef PACKAGE_VERSION
+#define PACKAGE_VERSION GIT_VERSION
+#endif
+
+static char const digits[] = "0123456789";
+static char const printf_flags[] = "'-+ #0I";
+
+typedef enum search_what_t {
+   SEARCH_EXACT,
+   SEARCH_GROUPS,
+   SEARCH_REGEX,
+} search_what_t;
+
+typedef struct expac_t {
+   config_t *config;
+} expac_t;
+
+bool opt_readone = false;
+bool opt_verbose = false;
+char opt_humansize = 'B';
+alpm_pkgfrom_t opt_corpus = ALPM_PKG_FROM_LOCALDB;
+search_what_t opt_what = SEARCH_EXACT;
+const char *opt_format = NULL;
+const char *opt_timefmt = DEFAULT_TIMEFMT;
+const char *opt_listdelim = DEFAULT_LISTDELIM;
+const char *opt_delim = DEFAULT_DELIM;
+const char *opt_config_file = CONFFILE;
+int opt_pkgcounter = 0;
+
+typedef const char *(*extractfn)(void*);
+
+static int is_valid_size_unit(char *u)
+{
+   return u[0] != '\0' && u[1] == '\0' &&
+   memchr(SIZE_TOKENS, *u, strlen(SIZE_TOKENS)) != NULL;
+}
+
+static const char *alpm_backup_get_name(alpm_backup_t *bkup)
+{
+   return bkup->name;
+}
+
+static char *size_to_string(off_t pkgsize)
+{
+   static char out[64];
+
+   if(opt_humansize == 'B') {
+   snprintf(out, sizeof(out), "%jd", (intmax_t)pkgsize);
+   } else {
+   snprintf(out, sizeof(out), "%.2f %ciB", humanize_size(pkgsize, 
opt_humansize, 0, NULL), opt_humansize);
+   }
+
+   return out;
+}
+
+static char *format_optdep(alpm_depend_t *optdep)
+{
+   char *out = NULL;
+
+   if(asprintf(, "%s: %s", optdep->name, optdep->desc) < 0) {
+   

Re: [pacman-dev] [PATCH 4/4] Merge expac into src/pacman

[Dave Reisner]

On Mon, Jul 09, 2018 at 06:37:35PM -0400, Andrew Gregory wrote:
> On 07/05/18 at 10:42am, Dave Reisner wrote:
> > This introduces code which has long been maintained at:
> > 
> > https://github.com/falconindy/expac
> > 
> > From the README:
> > 
> > expac is a data extraction tool for alpm databases. It features
> > printf-like flexibility and aims to be used as a simple tool for other
> > pacman based utilities which don't link against the library. It uses
> > pacman.conf as a config file for locating and loading your local and
> > sync databases.
> > ---
> > I expect that I'll get some pushback on the use of #pragma here. I don't
> > really know how portable it is to other compilers. Open to suggestions
> > on how to better avoid this (though I'd prefer not to disable the
> > warnings entirely for the file).
> 
> I'm not thrilled about adding #pragma to our codebase, but I don't see
> a better solution.  I would think it would be at least as portable as
> the function attributes we use elsewhere.
> 
> Just looking at expac.c, I don't see anything major, although I didn't
> look all that hard because I assume expac is pretty well tested at
> this point.  Since this is a previously external project, I'm fine if
> you want to incorporate changes as further patches to keep this one as
> close to original as possible.
> 

Happy to make most of these changes up front since your suggested
changes are fairly minor and very reasonable. Thanks for the review.

> >  doc/Makefile.am|   7 +-
> >  doc/expac.1.asciidoc   | 179 +
> >  src/pacman/.gitignore  |   2 +
> >  src/pacman/Makefile.am |   8 +-
> >  src/pacman/expac.c | 864 +
> >  5 files changed, 1057 insertions(+), 3 deletions(-)
> >  create mode 100644 doc/expac.1.asciidoc
> >  create mode 100644 src/pacman/expac.c
> 
> ...
> 
> > diff --git a/src/pacman/expac.c b/src/pacman/expac.c
> > new file mode 100644
> > index ..332414a0
> > --- /dev/null
> > +++ b/src/pacman/expac.c
> > @@ -0,0 +1,864 @@
> 
> ...
> 
> > +
> > +typedef enum package_corpus_t {
> > +   CORPUS_LOCAL,
> > +   CORPUS_SYNC,
> > +   CORPUS_FILE,
> > +} package_corpus_t;
> 
> Is there a reason you don't use the PKG_FROM constants from alpm?
> 

Nope. It's fine to reuse alpm_pkgfrom_t here.

> ...
> 
> > +static int print_time(time_t timestamp) {
> > +   char buffer[64];
> > +   int out = 0;
> > +
> > +   if(!timestamp) {
> > +   if(opt_verbose) {
> > +   out += printf("None");
> > +   }
> > +   return out;
> > +   }
> > +
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wformat-nonliteral"
> > +   /* no overflow here, strftime prints a max of 64 including null */
> > +   strftime([0], 64, opt_timefmt, localtime());
> > +#pragma GCC diagnostic pop
> > +
> > +   out += printf("%s", buffer);
> 
> I'm still trying to figure out how how the hell to correctly use
> strftime with its multiple undefined behaviors and lack of an error
> indicator, but we should at least skip the printf if it returns 0.  In
> that case, either there is nothing to print or buffer contains
> garbage.
> 
> ...
> 

Sounds good, though silently failing isn't my favorite. At least the
glibc implementation of strftime doesn't leave garbage in the buffer on
overflow. It'd also be lousy to print something like "error: timefmt
overflow" on every single error.

I've wanted for a while to do more sanity checking of the format strings
up front so that we can fail hard (and once) before gathering packages
and printing the results. I guess this is another situation that would
fix.

For now, let's just go with your suggestion and avoid printing an
undefined buffer on failure. In nearly 8 years (I can't believe it's
been that long) no one has complained that they've experienced
surprising behavior as a result of overflowing the 64 byte time format.

> > +static int read_targets_from_file(FILE *in, alpm_list_t **targets)
> > +{
> > +   char line[BUFSIZ];
> > +   int i = 0, end = 0, targets_added = 0;
> > +
> > +   while(!end) {
> > +   line[i] = fgetc(in);
> > +
> > +   if(feof(in))
> > +   end = 1;
> > +
> > +   if(isspace(line[i]) || end) {
> 
> This should be line-delimited to match pacman rather than
> space-delimited.
> 

Done.

> > +   line[i] = '\0';
> > +   /* avoid adding zero length a

Re: [pacman-dev] interest in using meson over autotools?

[Dave Reisner]

On Sun, Jul 08, 2018 at 06:10:09PM +0200, Andreas Baumann wrote:
> Hi,
> 
> Sorry, if this message is a little bit out of order, I only just
> subscribed to the pacman-dev group.
> 
> I want to raise the issue of bootstrapping:
> - automake, autoconf, libtool and friends need m4 and perl and bash
> - meson needs python
>
> Bootstrapping m4 and perl is comparatively easy to bootstrapping python.

So then build a minimal python as a "stage 1". You only need libffi and
expat, which themselves have no build dependencies beyond glibc.

Upstream has heard this argument before, thus:

http://mesonbuild.com/FAQ.html#why-is-meson-implemented-in-python-rather-than-programming-language-x
http://mesonbuild.com/Use-of-Python.html

> Porting to other architectures is easier if there are not too many
> dependencies.
> 
> See also my experiments here:
> 
> https://git.archlinux32.org/archlinux32/bootstrap32
> 
> and some porting work from oaken-source (Parabola):
> 
> https://github.com/oaken-source/parabola-riscv64-bootstrap

I'm not sure exactly what I'm supposed to get out of these. The reality
is that I don't really buy your argument. If you're bootstrapping Arch
Linux, you need to build Python anyways. You'll have to build Meson in
order to build systemd. You'll have to build Python in order to run
pacman's unit tests (you should absolutely care about these for
bootstrapping). Changing your bootstrap order to accomodate this doesn't
really seem that onerous.

> Cheers
> 
> Andreas
> 
> --
> Andreas Baumann
> Trottenstrasse 20
> CH-8037 Zuerich
> Telefon: +41(0)76/373 01 29
> E-mail: m...@andreasbaumann.cc
> Homepage: www.andreasbaumann.cc

Re: [pacman-dev] interest in using meson over autotools?

[Dave Reisner]

On Sat, Jul 07, 2018 at 07:58:36PM -0400, Andrew Gregory wrote:
> On 07/07/18 at 07:13pm, Dave Reisner wrote:
> > ...
> > Would there be interest in accepting patches to make these changes? I
> > propose we would carry both build systems in parallel for a major
> > release before eventually dropping autotools.
> > 
> > Feedback wanted,
> > dR
> 
> I've hated the experience every single time I've had to deal with our
> autotools setup, so I'm definitely open to change.  Our test suite is
> currently integrated with it, though.  What changes would we have to
> make to our tests for meson?
> 
> apg

This is one area where Meson could use some improvement. There's generic
support for tests, but no specific support for tests using TAP. There's
an open issue where someone explicitly has requested this:

https://github.com/mesonbuild/meson/issues/2923

Towards the end, there's some suggested workarounds for the current
limitations.

I think the answer is ideally that we have no make no changes at all to
our tests. We could do all of the work in the build system alone -- use
a harness to wrap each test and parse the TAP output.

[pacman-dev] interest in using meson over autotools?

[Dave Reisner]

Hi,

I'm probably not the first to suggest that Autotools is slow and
terrible, but here it is: Autotools is slow and terrible. Concretely, a
few things I find highly annoying about our current use of Autotools:

* We have to carry hacks to the build system itself just to ensure that
  our binaries aren't overlinked.
* Data is duplicated across Makefiles -- derived path configurations such
  as LOCALEDIR or CONFFILE.
* Code is duplicated across directories (symlinked to src/common). As a
  result, we have poor isolation and rebuild the code multiple times.
* './configure && make -C src/pacman pacman' fails. It seems absurd that
  the user at the command line should have to know to build libalpm
  first. Isn't that the whole point of a dependency-tracking build
  system?

Subjectively, the configuration itself is archaic and verbose. Does
anyone *really* want to spend time understanding the quoting semantics
of m4, or figure out where things like $ac_cv_sys_file_offset_bits come
from?

Yes, some of this could be resolved by fixing/rewriting the autotools
madness to not use recursive make, but I'd suggest we just drop
autotools and adopt meson. Meson can easily fix all of the above.

Why meson?

* It's fast. I wrote the initial meson.build files to compile libalpm.so
  and pacman (this took me under an hour). The resulting build takes 1/3
  of the time of the equivalent autotools build. The ./configure
  equivalent in meson will be faster too, but I can't compare that yet
  because my meson-equivalent is incomplete. Trust me, it'll be faster.
* It's modular. You can still constrain lots of subdirectory specifics in
  subdirectory meson.build files.
* It's easy to read. The config is python-ish, and it's very concise.
  The documentation is comprehensive and has plenty of examples.
* There's lots of upstream support, and the community is growing. In
  Arch Linux, 175 packages are built using meson (largely because gnome
  has decided to adopt meson). We would not be an early adopter of meson.
* It's portable. meson supports all the targets where we currently have
  actual users.

Would there be interest in accepting patches to make these changes? I
propose we would carry both build systems in parallel for a major
release before eventually dropping autotools.

Feedback wanted,
dR

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

[Dave Reisner]

This is shared between pacman and pacman-conf (and might be used by
other binaries in the future) -- no need to compile it once for each
consumer.
---
 src/pacman/.gitignore  |  3 ++-
 src/pacman/Makefile.am | 37 +++--
 2 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/src/pacman/.gitignore b/src/pacman/.gitignore
index 24e11ed3..9889c35e 100644
--- a/src/pacman/.gitignore
+++ b/src/pacman/.gitignore
@@ -1,6 +1,7 @@
 .deps
 .libs
+*.l[ao]
 pacman
 pacman.exe
 pacman-conf
-pacman-conf.exe
\ No newline at end of file
+pacman-conf.exe
diff --git a/src/pacman/Makefile.am b/src/pacman/Makefile.am
index 2344daff..15cf20ce 100644
--- a/src/pacman/Makefile.am
+++ b/src/pacman/Makefile.am
@@ -8,6 +8,19 @@ hookdir   = ${sysconfdir}/pacman.d/hooks/
 cachedir  = ${localstatedir}/cache/pacman/pkg/
 logfile   = ${localstatedir}/log/pacman.log
 
+noinst_LTLIBRARIES = \
+   libbasic.la
+
+libbasic_la_SOURCES = \
+   conf.h conf.c \
+   ini.h ini.c \
+   callback.h callback.c \
+   util.h util.c \
+   util-common.h util-common.c
+
+libbasic_la_LIBADD = \
+   $(top_builddir)/lib/libalpm/.libs/libalpm.la
+
 bin_PROGRAMS = pacman pacman-conf
 
 AM_CPPFLAGS = \
@@ -31,37 +44,25 @@ endif
 
 pacman_SOURCES = \
check.h check.c \
-   conf.h conf.c \
database.c \
deptest.c \
files.c \
-   ini.h ini.c \
package.h package.c \
pacman.h pacman.c \
query.c \
remove.c \
sighandler.h sighandler.c \
sync.c \
-   callback.h callback.c \
-   upgrade.c \
-   util.h util.c \
-   util-common.h util-common.c
+   upgrade.c
 
 pacman_LDADD = \
$(LTLIBINTL) \
+   libbasic.la \
$(top_builddir)/lib/libalpm/.libs/libalpm.la \
$(LIBARCHIVE_LIBS)
 
-pacman_conf_SOURCES = pacman-conf.c \
-   util.h \
-   util.c \
-   ini.h \
-   ini.c \
-   util-common.h \
-   util-common.c \
-   callback.h \
-   callback.c \
-   conf.h \
-   conf.c
+pacman_conf_SOURCES = pacman-conf.c
 
-pacman_conf_LDADD = $(top_builddir)/lib/libalpm/.libs/libalpm.la
+pacman_conf_LDADD = \
+   libbasic.la \
+   $(top_builddir)/lib/libalpm/.libs/libalpm.la
-- 
2.18.0

[pacman-dev] [PATCH 3/4] doc: use implicit rules to build manpages

[Dave Reisner]

Use implicit dependency rules to translate asciidoc inputs to HTML and
manpage outputs. We should only have to declare explicit dependencies
for odd cases, e.g. the PKGBUILD documentation has an additional include
file and isn't a 1:1 conversion.
---
 doc/Makefile.am | 34 --
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/doc/Makefile.am b/doc/Makefile.am
index 8dec4ab1..2ac38cba 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -3,7 +3,7 @@
 # files listed in EXTRA_DIST no matter what. However, we only add them to
 # man_MANS if --enable-asciidoc and/or --enable-doxygen are used.
 
-ASCIIDOC_MANS = \
+MANPAGES = \
alpm-hooks.5 \
pacman.8 \
makepkg.8 \
@@ -66,11 +66,11 @@ EXTRA_DIST = \
submitting-patches.asciidoc \
translation-help.asciidoc \
Doxyfile \
-   $(ASCIIDOC_MANS) \
+   $(MANPAGES) \
$(DOXYGEN_MANS)
 
 # Files that should be removed, but which Automake does not know.
-MOSTLYCLEANFILES = *.xml $(ASCIIDOC_MANS) $(HTML_DOCS) repo-remove.8 
website.tar.gz
+MOSTLYCLEANFILES = *.xml $(MANPAGES) $(HTML_DOCS) repo-remove.8 website.tar.gz
 
 # Ensure manpages are fresh when building a dist tarball
 dist-hook:
@@ -85,7 +85,7 @@ REAL_PACKAGE_VERSION = $(PACKAGE_VERSION)
 endif
 
 man_MANS =
-dist_man_MANS = $(ASCIIDOC_MANS)
+dist_man_MANS = $(MANPAGES)
 
 if USE_DOXYGEN
 man_MANS += $(DOXYGEN_MANS)
@@ -96,6 +96,7 @@ doxygen.in:
$(DOXYGEN) $(srcdir)/Doxyfile
 endif
 
+man: $(MANPAGES)
 html: $(HTML_DOCS)
 
 website: website.tar.gz
@@ -129,11 +130,12 @@ A2X_OPTS = \
-f manpage \
--xsltproc-opts='-param man.endnotes.list.enabled 0 -param 
man.endnotes.are.numbered 0'
 
-# These rules are due to the includes and files of the asciidoc text
-$(ASCIIDOC_MANS): asciidoc.conf footer.asciidoc Makefile.am
+# Generate manpages
+%: %.asciidoc asciidoc.conf footer.asciidoc Makefile.am
$(AM_V_GEN)a2x $(A2X_OPTS) --asciidoc-opts="$(ASCIIDOC_OPTS) 
--out-file=./$@.xml" $(srcdir)/$@.asciidoc
 
-%.html: %.asciidoc
+# Generate HTML pages
+%.html: %.asciidoc asciidoc.conf footer.asciidoc Makefile.am
$(AM_V_GEN)asciidoc $(ASCIIDOC_OPTS) -o - $*.asciidoc | \
sed -e 's/\r$$//' > $@
 
@@ -142,27 +144,15 @@ HACKING.html: ../HACKING
sed -e 's/\r$$//' > $@
 
 # Customizations for certain HTML docs
-$(HTML_MANPAGES): asciidoc.conf footer.asciidoc Makefile.am
-$(HTML_OTHER): asciidoc.conf Makefile.am
 %.html: ASCIIDOC_OPTS += -a linkcss -a toc -a icons -a max-width=960px -a 
stylesheet=asciidoc-override.css
 %.8.html: ASCIIDOC_OPTS += -d manpage
 %.5.html: ASCIIDOC_OPTS += -d manpage
 %.3.html: ASCIIDOC_OPTS += -d manpage
 
-# Dependency rules
-alpm-hooks.5 alpm-hooks.5.html: alpm-hooks.5.asciidoc
-pacman.8 pacman.8.html: pacman.8.asciidoc
-makepkg.8 makepkg.8.html: makepkg.8.asciidoc
-makepkg-template.1 makepkg-template.1.html: makepkg-template.1.asciidoc
-repo-add.8 repo-add.8.html: repo-add.8.asciidoc
-vercmp.8 vercmp.8.html: vercmp.8.asciidoc
-pkgdelta.8 pkgdelta.8.html: pkgdelta.8.asciidoc
-pacman-key.8 pacman-key.8.html: pacman-key.8.asciidoc
+# Custom dependency rules
 PKGBUILD.5 PKGBUILD.5.html: PKGBUILD.5.asciidoc PKGBUILD-example.txt
-makepkg.conf.5 makepkg.conf.5.html: makepkg.conf.5.asciidoc
-pacman.conf.5 pacman.conf.5.html: pacman.conf.5.asciidoc
-libalpm.3 libalpm.3.html: libalpm.3.asciidoc
-# this one is just a symlink
+
+# Manpages as symlinks
 repo-remove.8: repo-add.8
$(RM) repo-remove.8
$(LN_S) repo-add.8 repo-remove.8
-- 
2.18.0

[pacman-dev] [PATCH 4/4] Merge expac into src/pacman

[Dave Reisner]

ase
+
+  %ffilename (only with -S)
+
+  %gbase64 encoded PGP signature (only with -S)
+
+  %hsha256sum
+
+  %Vpackage validation method
+
+  %ihas install scriptlet (only with -Q)
+
+  %kdownload size (only with -S)
+
+  %linstall date (only with -Q)
+
+  %minstall size
+
+  %Mmodified backup files (only with -Q)
+
+  %npackage name
+
+  %ppackager name
+
+  %rrepo
+
+  %smd5sum
+
+  %uproject URL
+
+  %vversion
+
+  %winstall reason (only with -Q)
+
+  %!result number (auto-incremented counter, starts at 0)
+
+  %%literal %
+
+Note that for any lowercase tokens aside from %m and %k, full printf support is
+allowed, e.g. %-20n. This does not apply to any list based, date, or numerical
+output.
+
+Standard backslash escape sequences are supported, as per printf(1).
+
+Examples
+
+
+expac -Ss \'%r/%n %v\n%d\' ::
+   Emulate pacman\'s search function.
+
+expac --timefmt=%s \'%b\t%n\' | sort -n | head -10::
+   List the oldest 10 installed packages (by build date).
+
+See Also
+
+linkman:libalpm[3], linkman:pacman.conf[5]
+
+include::footer.asciidoc[]
diff --git a/src/pacman/.gitignore b/src/pacman/.gitignore
index 9889c35e..78b78159 100644
--- a/src/pacman/.gitignore
+++ b/src/pacman/.gitignore
@@ -5,3 +5,5 @@ pacman
 pacman.exe
 pacman-conf
 pacman-conf.exe
+expac
+expac.exe
diff --git a/src/pacman/Makefile.am b/src/pacman/Makefile.am
index 15cf20ce..deb386ab 100644
--- a/src/pacman/Makefile.am
+++ b/src/pacman/Makefile.am
@@ -21,7 +21,7 @@ libbasic_la_SOURCES = \
 libbasic_la_LIBADD = \
$(top_builddir)/lib/libalpm/.libs/libalpm.la
 
-bin_PROGRAMS = pacman pacman-conf
+bin_PROGRAMS = pacman pacman-conf expac
 
 AM_CPPFLAGS = \
-imacros $(top_builddir)/config.h \
@@ -66,3 +66,9 @@ pacman_conf_SOURCES = pacman-conf.c
 pacman_conf_LDADD = \
libbasic.la \
$(top_builddir)/lib/libalpm/.libs/libalpm.la
+
+expac_SOURCES = expac.c
+
+expac_LDADD = \
+   libbasic.la \
+   $(top_builddir)/lib/libalpm/.libs/libalpm.la
diff --git a/src/pacman/expac.c b/src/pacman/expac.c
new file mode 100644
index ..332414a0
--- /dev/null
+++ b/src/pacman/expac.c
@@ -0,0 +1,864 @@
+/*
+ *  expac.c
+ *
+ *  Copyright (c) 2018 Pacman Development Team 
+ *  Copyright (c) 2010-2018 by Dave Reisner 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "conf.h"
+#include "util.h"
+
+#define DEFAULT_DELIM  "\n"
+#define DEFAULT_LISTDELIM  "   "
+#define DEFAULT_TIMEFMT"%c"
+#define SIZE_TOKENS"BKMGTPEZY\0"
+
+static char const digits[] = "0123456789";
+static char const printf_flags[] = "'-+ #0I";
+
+typedef enum package_corpus_t {
+   CORPUS_LOCAL,
+   CORPUS_SYNC,
+   CORPUS_FILE,
+} package_corpus_t;
+
+typedef enum search_what_t {
+   SEARCH_EXACT,
+   SEARCH_GROUPS,
+   SEARCH_REGEX,
+} search_what_t;
+
+typedef struct expac_t {
+   config_t *config;
+} expac_t;
+
+bool opt_readone = false;
+bool opt_verbose = false;
+char opt_humansize = 'B';
+package_corpus_t opt_corpus = CORPUS_LOCAL;
+search_what_t opt_what = SEARCH_EXACT;
+const char *opt_format = NULL;
+const char *opt_timefmt = DEFAULT_TIMEFMT;
+const char *opt_listdelim = DEFAULT_LISTDELIM;
+const char *opt_delim = DEFAULT_DELIM;
+const char *opt_config_file = "/etc/pacman.conf";
+int opt_pkgcounter = 0;
+
+typedef const char *(*extractfn)(void*);
+
+static int is_valid_size_unit(char *u)
+{
+   return u[0] != '\0' && u[1] == '\0' &&
+   memchr(SIZE_TOKENS, *u, strlen(SIZE_TOKENS)) != NULL;
+}
+
+static const char *alpm_backup_get_name(alpm_backup_t *bkup)
+{
+   return bkup->name;
+}
+
+static char *size_to_string(off_t pkgsize)
+{
+   static char out[64];
+
+   if(opt_humansize == 'B') {
+   snprintf(out, sizeof(out), "%jd", (intmax_t)pkgsize);
+   } else {
+   snprintf(out, sizeof(out), "%.2f %ciB", humanize_size(pkgsize, 
opt_humansize, 0, NULL), opt_humansize);
+ 

[pacman-dev] [PATCH 1/4] pacman/conf: Remove unused include

[Dave Reisner]

---
 src/pacman/conf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/pacman/conf.c b/src/pacman/conf.c
index 9a37dcea..29f69052 100644
--- a/src/pacman/conf.c
+++ b/src/pacman/conf.c
@@ -35,7 +35,6 @@
 #include "conf.h"
 #include "ini.h"
 #include "util.h"
-#include "pacman.h"
 #include "callback.h"
 
 /* global config variable */
-- 
2.18.0

Re: [pacman-dev] [PATCH] libmakepkg/integrity: use more shared functions to generate signatures

[Dave Reisner]

On Tue, Jun 12, 2018 at 07:28:50AM -0400, Eli Schwartz wrote:
> The newly changed print_all_package_names function can iterate over the
> list of all package files that will be created; this avoids the need to
> independently recreate those filenames here.
> 
> Additionally, since debug packages may not actually exist, check if the
> package file exists first. If the main package does not exist then
> makepkg will have aborted before now, so there is no need to
> special-case that here.
> 
> Signed-off-by: Eli Schwartz 
> ---
>  .../integrity/generate_signature.sh.in  | 17 ++---
>  1 file changed, 2 insertions(+), 15 deletions(-)
> 
> diff --git a/scripts/libmakepkg/integrity/generate_signature.sh.in 
> b/scripts/libmakepkg/integrity/generate_signature.sh.in
> index 442fe031..3350ca50 100644
> --- a/scripts/libmakepkg/integrity/generate_signature.sh.in
> +++ b/scripts/libmakepkg/integrity/generate_signature.sh.in
> @@ -50,25 +50,12 @@ create_package_signatures() {
>   if [[ $SIGNPKG != 'y' ]]; then
>   return 0
>   fi
> - local pkg pkgarch pkg_file
> - local fullver=$(get_full_version)
>  
>   msg "$(gettext "Signing package(s)...")"
>  
> - for pkg in "${pkgname[@]}"; do
> - pkgarch=$(get_pkg_arch $pkg)
> - pkg_file="$PKGDEST/${pkg}-${fullver}-${pkgarch}${PKGEXT}"
> -
> - create_signature "$pkg_file"
> - done
> -
> - # check if debug package needs a signature
> - if ! check_option "debug" "y" || ! check_option "strip" "y"; then
> - pkg=$pkgbase-@DEBUGSUFFIX@
> - pkgarch=$(get_pkg_arch)
> - pkg_file="$PKGDEST/${pkg}-${fullver}-${pkgarch}${PKGEXT}"
> + print_all_package_names | while read pkg_file; do

read -r

>   if [[ -f $pkg_file ]]; then
>   create_signature "$pkg_file"
>   fi
> - fi
> + done
>  }
> -- 
> 2.17.1

[pacman-dev] [PATCH] makepkg: fix initialization when extracting arrays

[Dave Reisner]

Assuming that everything is a string leads to code which is effectively:

  a=
  a+=('bar')

This creates an array with 2 elements instead of one. Using proper array
initialization fixes this.

https://lists.archlinux.org/pipermail/pacman-dev/2018-June/022591.html
---
 scripts/libmakepkg/util/pkgbuild.sh.in | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/libmakepkg/util/pkgbuild.sh.in 
b/scripts/libmakepkg/util/pkgbuild.sh.in
index 10d154d1..3f8669ab 100644
--- a/scripts/libmakepkg/util/pkgbuild.sh.in
+++ b/scripts/libmakepkg/util/pkgbuild.sh.in
@@ -106,7 +106,11 @@ get_pkgbuild_attribute() {
 
local pkgname=$1 attrname=$2 isarray=$3 outputvar=$4
 
-   printf -v "$outputvar" %s ''
+   if (( isarray )); then
+   eval "$outputvar=()"
+   else
+   printf -v "$outputvar" %s ''
+   fi
 
if [[ $pkgname ]]; then
extract_global_variable "$attrname" "$isarray" "$outputvar"
-- 
2.17.1

Re: [pacman-dev] [PATCH 2/7] libmakepkg: stop printsrcinfo generating empty values

[Dave Reisner]

On Fri, Jun 08, 2018 at 09:37:37PM +0100, Morgan Adamiec wrote:
> > Please don't change this to paper over bad PKGBUILDs.
> 
> the thing is foo= is never declared in the pkgbuild
> 
> A pkgbuild like this:
> 
> pkgname=foo
> pkgver=1
> pkgrel=1
> arch=('any')
> 
> package() {
> depends+=("bar")
> }
> 
> Generates a srcinfo like this:
> 
> pkgbase = foo
> pkgver = 1
> pkgrel = 1
> arch = any
> 
> pkgname = foo
> depends =
> depends = bar
> 
> When you perform foo+=(bar) when foo is unset the array will only
> contain foo. So no I wouldn't put the blame on bad pkgbuilds here.
> 
> Infact the same thing happens if you do declare depends globally like such:
> 
> pkgname=foo
> pkgver=1
> pkgrel=1
> arch=('any')
> depends=()
> 
> package() {
> depends+=("bar")
> }

I see. Thanks for the clear reproduction case -- I think I see the
problem and I'll post a patch for the pkgbuild util code.

Re: [pacman-dev] [PATCH 2/7] libmakepkg: stop printsrcinfo generating empty values

[Dave Reisner]

On Fri, Jun 08, 2018 at 08:42:11PM +0100, Morgan Adamiec wrote:
> On Fri, 8 Jun 2018 at 20:33, Eli Schwartz  wrote:
> >
> > On 06/08/2018 02:18 PM, morganamilo wrote:
> > > When a split package overriddes an array using += and the array does not
> > > exist globally, makepkg --printsrcinfo will print the field with an
> > > empty vlaue before printing the acual values.
> > >
> > > For exampple: having `depends+=(foo bar)` will generate:
> > >   depends =
> > >   depends = foo
> > >   depends = bar
> > >
> > > Explicity check for empty array values and only print the values that
> > > are not empty.
> > >
> > > Signed-off-by: morganamilo 
> > > ---
> > >  scripts/libmakepkg/srcinfo.sh.in | 6 +-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/libmakepkg/srcinfo.sh.in 
> > > b/scripts/libmakepkg/srcinfo.sh.in
> > > index 509c4860..6a49be37 100644
> > > --- a/scripts/libmakepkg/srcinfo.sh.in
> > > +++ b/scripts/libmakepkg/srcinfo.sh.in
> > > @@ -44,7 +44,11 @@ srcinfo_write_attr() {
> > >   attrvalues=("${attrvalues[@]#[[:space:]]}")
> > >   attrvalues=("${attrvalues[@]%[[:space:]]}")
> > >
> > > - printf "\t$attrname = %s\n" "${attrvalues[@]}"
> > > + for val in "${attrvalues[@]}"; do
> > > + if [[ ! -z ${val// /} ]]; then
> > > + printf "\t$attrname = %s\n" "$val"
> > > + fi
> > > + done
> >
> > This is odd, I wonder why get_pkgbuild_attribute is returning an
> > array=('' foo bar) in this case? We should probably fix it more directly.
> >
> > --
> > Eli Schwartz
> > Bug Wrangler and Trusted User
> >
> 
> In my investigation it came down to lines like this
> https://git.archlinux.org/pacman.git/tree/scripts/libmakepkg/srcinfo.sh.in#n55
> 
> It seems appending an array to a non array value generates an array
> with the first value being an empty string then the values appended.
> 
> You can reproduce it with:
> foo=
> foo+=(bar)
> echo "${#foo[@]}"
> 
> I did think about fixing it there, but that function is used all over
> and I didn't want to break something.

That's just bad shell code. Using foo= to declare an array is the same
as writing foo=('').

Please don't change this to paper over bad PKGBUILDs. If anything, the
fix here is to leverage bash 4.4 in our lint rules when it's available
and use @a expansion to detect if something is an array or a string,
e.g.

$ licenses='MIT'
$ [[ ${licenses@a} = *a* ]] || echo "licenses must be an array"

Re: [pacman-dev] [PATCH] Fix using run_pacman to invoke -Qi with sudo

[Dave Reisner]

On Tue, May 15, 2018 at 11:13:09AM -0400, Eli Schwartz wrote:
> In commit 5698d7b66daa2a0bc99cab7a989cef1c806c3bf6 a new non-root use of
> pacman was added -- previously we used -T or -Qq, and run_pacman did not
> know how to special-case -Qi to skip being prepended with sudo.

Can we just be explicit about when we do and don't need elevated
privileges rather than trying to guess? Surely the caller knows the
requirements a priori.

> The result is:
> 
>   -> Generating .BUILDINFO file...
> ERROR: ld.so: object 'libfakeroot.so' from LD_PRELOAD cannot be preloaded 
> (cannot open shared object file): ignored.
> [sudo] password for eschwartz:
>   -> Adding changelog file...
> 
> Fix this by using a more generic glob since neither -Q nor -T will ever
> need sudo or PACMAN_OPTS
> 
> Signed-off-by: Eli Schwartz 
> ---
>  scripts/makepkg.sh.in | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 62b912e3..e9080a70 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -220,12 +220,12 @@ missing_source_file() {
>  
>  run_pacman() {
>   local cmd
> - if [[ $1 != -@(T|Qq) ]]; then
> + if [[ $1 != -@(T|Q)*([[:alpha:]]) ]]; then
>   cmd=("$PACMAN_PATH" "${PACMAN_OPTS[@]}" "$@")
>   else
>   cmd=("$PACMAN_PATH" "$@")
>   fi
> - if [[ $1 != -@(T|Qq|Q) ]]; then
> + if [[ $1 != -@(T|Q)*([[:alpha:]]) ]]; then
>   if type -p sudo >/dev/null; then
>   cmd=(sudo "${cmd[@]}")
>   else
> -- 
> 2.17.0

Re: [pacman-dev] [PATCH 2/2] Append '-$arch' to 'installed' array in .BUILDINFO

[Dave Reisner]

On Sun, Mar 18, 2018 at 11:40:53AM +1000, Allan McRae wrote:
> On 18/03/18 07:24, Robin Broda wrote:
> > This patch incurs a **severe** performance degradation when generating
> > the .BUILDINFO file, likely due to frequent usage of `pacman -Qi`
> > and `grep -E`. I haven't found a faster way to gather this information.
> > 
> > Signed-off-by: Robin Broda 
> 
> I will comment on the utility of this patch in another email.   This is
> just pointing out that it is broken...
> 
> > ---
> >  doc/BUILDINFO.5.txt   |  2 +-
> >  scripts/makepkg.sh.in | 12 ++--
> >  2 files changed, 11 insertions(+), 3 deletions(-)
> > 
> > diff --git a/doc/BUILDINFO.5.txt b/doc/BUILDINFO.5.txt
> > index 4734301e..2c74f9ff 100644
> > --- a/doc/BUILDINFO.5.txt
> > +++ b/doc/BUILDINFO.5.txt
> > @@ -61,7 +61,7 @@ BUILDINFO file format.
> >  
> >  *installed (array)*::
> > The installed packages at build time including the version information 
> > of
> > -   the package. Formatted as "$pkgname-$pkgver-$pkgrel".
> > +   the package. Formatted as "$pkgname-$pkgver-$pkgrel-$pkgarch".
> >  
> >  See Also
> >  
> > diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> > index ece53dca..10303417 100644
> > --- a/scripts/makepkg.sh.in
> > +++ b/scripts/makepkg.sh.in
> > @@ -694,8 +694,16 @@ write_buildinfo() {
> > write_kv_pair "buildenv" "${BUILDENV[@]}"
> > write_kv_pair "options" "${OPTIONS[@]}"
> >  
> > -   local pkglist=($(run_pacman -Q | sed "s# #-#"))
> > -   write_kv_pair "installed" "${pkglist[@]}"
> > +   local pkglist=($(run_pacman -Qq))
> > +   local installed=()
> > +   for pkg in "${pkglist[@]}"
> > +   do
> > +   pkginfo="$(pacman -Qi "${pkg}")"
> 
> What makes this call to pacman not need to use run_pacman like the others?
> 

Answer: run_pacman calls sudo, which means that a bare 'makepkg' will
require elevated privileges.

> > +   pkgver="$(grep -E '^Version' <<< "${pkginfo}" | tr -d ' ' | cut 
> > -d':' -f2-)"
> > +   pkgarch="$(grep -E '^Architecture' <<< "${pkginfo}" | tr -d ' ' 
> > | cut -d':' -f2-)"
> 
> Not every system runs in English...
> 
> > +   installed+=("${pkg}-${pkgver}-${pkgarch}")
> > +   done
> > +   write_kv_pair "installed" "${installed[@]}"
> >  }
> >  
> >  # build a sorted NUL-separated list of the full contents of the current
> > 

Re: [pacman-dev] Road to 5.1

[Dave Reisner]

On Mon, Jan 08, 2018 at 03:26:49PM +1000, Allan McRae wrote:
> Hi all,
> 
> We have not had a release in a long, long time.  Lets fix that!
> 
> There is a substantial number of bugs tagged for a 5.1 release [1].
> These were added when we were all younger and more optimistic about the
> world...  I have added a 5.2 target, so we can start bumping some to a
> future release.
> 
> Please reply here with anything you think is essential to the next
> release - even better if you provide a patch!
> 
> Cheers,
> Allan
> 
> 
> [1] https://bugs.archlinux.org/roadmap/proj3

Can we fix extraction based on lookups of user/group so that the
nonsense that Arch has been subjected to via systemd-sysusers can be
fixed? This should be a simple matter of calling
archive_read_disk_set_standard_lookup(archive) prior to extraction. I
think agregory has a branch that he's been working on to do this.

d

Re: [pacman-dev] [PATCH v2] Do not continuously try to open an invalid database

[Dave Reisner]

On Tue, Jan 09, 2018 at 10:07:13PM +1000, Allan McRae wrote:
> If you manage to download a bad database (e.g. an html file when
> behind a proxy or with a badly configured webserver), pacman makes
> sure you know about it.  Here is some example output:
> 
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized 
> archive format
> 
> I don't know how many times that gets printed because it goes beyond my 
> scrollback
> buffer.
> 
> Flag a database that we can "open" and "fstat" but not read from as invalid 
> to avoid
> this.
> 
> Signed-off-by: Allan McRae 
> ---
> 
> Review of v1:
> https://lists.archlinux.org/pipermail/pacman-dev/2017-May/022027.html
> 
> v2 - do much less stuff to achive the same result...
> 
> 
>  lib/libalpm/be_sync.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index 1b7c8b6f..389e964c 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -486,6 +486,7 @@ static int sync_db_populate(alpm_db_t *db)
>   fd = _alpm_open_archive(db->handle, dbpath, ,
>   , ALPM_ERR_DB_OPEN);
>   if(fd < 0) {
> + db->status &= DB_STATUS_INVALID;

Given that DB_STATUS_INVALID is a single bit, shouldn't this be a simple
assignment? A bitwise 'and' here will result in the status remaining
invalid if it's already invalid, or zero'ing it out (DB_STATUS_VALID) if
it's some other set of bits. That doesn't seem like what we want.

Semi-related thought: it's a bit odd that we semantically chose "0" as
valid, and "1" as invalid.

>   return -1;
>   }
>   est_count = estimate_package_count(, archive);
> -- 
> 2.15.1

Re: [pacman-dev] [PATCH v2 1/2] makepkg: implement error codes

[Dave Reisner]

On Sun, Sep 17, 2017 at 05:34:15PM +1000, Allan McRae wrote:
> On 16/09/17 06:54, Dave Reisner wrote:
> >> +Errors
> >> +--
> >> +On exit, makepkg will return one of the following error codes.
> >> +
> >> +**E_OK**=0::
> > I don't see the need to document internal details of how we refer to the
> > error codes through named variables if we aren't making these public API.
> > 
> 
> To clarify - you are saying to remove "E_OK" etc from the documentation,
> but keep the number and description? That seems fair enough to me.
> 
> Allan

Yep, that's correct. Similar to how curl(1) documents its exit codes.

Re: [pacman-dev] [PATCH v2 1/2] makepkg: implement error codes

[Dave Reisner]

I didn't go over this in detail, but I'll point out a few concerns I
have about this patch...

On Fri, Sep 15, 2017 at 01:30:51PM -0500, ivy.fos...@gmail.com wrote:
> From: Ivy Foster 
> 
> For your convenience, makepkg now has 16 distinct ways to fail.
> ---
>  doc/makepkg.8.txt   |  60 ++
>  scripts/Makefile.am |   1 +
>  scripts/libmakepkg/util/error.sh.in |  42 +
>  scripts/makepkg.sh.in   | 120 
> ++--
>  4 files changed, 162 insertions(+), 61 deletions(-)
>  create mode 100644 scripts/libmakepkg/util/error.sh.in
> 
> diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt
> index 2dff1b19..224292a2 100644
> --- a/doc/makepkg.8.txt
> +++ b/doc/makepkg.8.txt
> @@ -272,6 +272,66 @@ See linkman:makepkg.conf[5] for more details on 
> configuring makepkg using the
>  'makepkg.conf' file.
>  
>  
> +Errors
> +--
> +On exit, makepkg will return one of the following error codes.
> +
> +**E_OK**=0::

I don't see the need to document internal details of how we refer to the
error codes through named variables if we aren't making these public API.

> + Normal exit condition.
> +
> +**E_FAIL**=1::
> + Unknown cause of failure.
> +
> +// Exit code 2 is reserved by bash for misuse of shell builtins
> +
> +**E_CONFIG_ERROR**=3::
> + Error in configuration file.
> +
> +**E_INVALID_OPTION**=4::
> + User specified an invalid option
> +
> +**E_BUILD_FAILED**=5::
> + Error in PKGBUILD build function.
> +
> +**E_PACKAGE_FAILED**=6::
> + Failed to create a viable package.

What about failures in the prepare of pkgver functions (and any future
functions which haven't yet been defined)? I think this ought to be more
generic and be something like E_USER_FUNCTION_FAILED.

> +**E_MISSING_FILE**=7::
> + A source or auxiliary file specified in the PKGBUILD is
> + missing.
> +
> +**E_MISSING_PKGDIR**=8::
> + The PKGDIR is missing.
> +
> +**E_INSTALL_DEPS_FAILED**=9::
> + Failed to install dependencies.
> +
> +**E_REMOVE_DEPS_FAILED**=10::
> + Failed to remove dependencies.
> +
> +**E_ROOT**=11::
> + User attempted to run makepkg as root.
> +
> +**E_FS_PERMISSIONS**=12::
> + User lacks permissions to build or install to a given
> + location.
> +
> +**E_PKGBUILD_ERROR**=13::
> + Error parsing PKGBUILD.
> +
> +**E_ALREADY_BUILT**=14::
> + A package has already been built.
> +
> +**E_INSTALL_FAILED**=15::
> + The package failed to install.
> +
> +**E_MISSING_MAKEPKG_DEPS**=16::
> + Programs necessary to run makepkg are missing.
> +
> +**E_PRETTY_BAD_PRIVACY**=17::
> + Error signing package.

As implemented, this is only used when checking to see that the key
exists, not as a failure when signing the package. To do that, you'd
need to change scripts/libmakepkg/integrity/generate_signature.sh.in,
and then make sure the error code is propagated down the stack.

> +
> +
>  See Also
>  
>  linkman:makepkg.conf[5], linkman:PKGBUILD[5], linkman:pacman[8]
> diff --git a/scripts/Makefile.am b/scripts/Makefile.am
> index 4bb08a24..3e7689bf 100644
> --- a/scripts/Makefile.am
> +++ b/scripts/Makefile.am
> @@ -96,6 +96,7 @@ LIBMAKEPKG_IN = \
>   libmakepkg/tidy/strip.sh \
>   libmakepkg/tidy/zipman.sh \
>   libmakepkg/util.sh \
> + libmakepkg/util/error.sh \
>   libmakepkg/util/message.sh \
>   libmakepkg/util/option.sh \
>   libmakepkg/util/parseopts.sh \
> diff --git a/scripts/libmakepkg/util/error.sh.in 
> b/scripts/libmakepkg/util/error.sh.in
> new file mode 100644
> index ..eefd5652
> --- /dev/null
> +++ b/scripts/libmakepkg/util/error.sh.in
> @@ -0,0 +1,42 @@
> +#!/bin/bash
> +#
> +#   error.sh.in - error variable definitions for makepkg
> +#
> +#   Copyright (c) 2006-2017 Pacman Development Team 
> 
> +#   Copyright (c) 2002-2006 by Judd Vinet 
> +#
> +#   This program is free software; you can redistribute it and/or modify
> +#   it under the terms of the GNU General Public License as published by
> +#   the Free Software Foundation; either version 2 of the License, or
> +#   (at your option) any later version.
> +#
> +#   This program is distributed in the hope that it will be useful,
> +#   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +#   GNU General Public License for more details.
> +#
> +#   You should have received a copy of the GNU General Public License
> +#   along with this program.  If not, see .
> +#
> +
> +[[ -n "$LIBMAKEPKG_UTIL_ERROR_SH" ]] && return
> +LIBMAKEPKG_UTIL_ERROR_SH=1
> +
> +E_OK=0
> +E_FAIL=1 # Generic error
> +# exit code 2 reserved by bash for misuse of shell builtins

Not sure I understand this... When would this clash?

> +E_CONFIG_ERROR=3
> +E_INVALID_OPTION=4
> +E_BUILD_FAILED=5
> +E_PACKAGE_FAILED=6
> 

Re: [pacman-dev] Repository management

[Dave Reisner]

On Tue, May 09, 2017 at 10:54:44PM +1000, Allan McRae wrote:
> Hi all,
> 
> Every time I attempt to work on repo-add, I find it to be a very
> difficult endeavour.  Even though it is half the size of makepkg
> (without even including any of libmakepkg), it is much more convoluted
> to work on.
> 
> We also have a weird repository database system.  We have:
> - .db dbs with package information, signatures and delta information
> - .files dbs that are the same as .db dbs but additionally include filelists
> 
> There are two reasons the .files dbs replicate all information in the
> .db dbs
>  - .db and .files dbs getting out of sync could cause issues
>  - a complete database is useful for things like archweb, mostly to
> avoid the above
> 
> I would also like to include information on source packages to these
> databases.  The files information is separate due to wanting our primary
> database to be small.  Likewise, source package information needs to be
> separate (the signatures take most of the size in the .db dbs, so adding
> source package signatures effectively doubles the size).
> 
> So two points up for discussion:
> 
> 
> 1) Sync repository layout?  I don't see any point in leaving the tar
> based format, as reading of sync databases is not a bottleneck.  (The
> local db format can be a bottleneck, but that is a separate discussion...)

Isn't this a historical reversal? IIRC, the sync DBs used to be expanded
onto disk, and we decided to leave them as tarballs to address
performance/fragmentation concerns.

> Do we split the information in .db out of .files and add a .full db with
> complete information?  Then any .src db could follow suit and just have
> source package information.  How do we get around the out of sync issue
> (e.g., a package is removed from .db, but we have an old .files database
> with it).  Do we add timestamps, and print a warning on -F operations
> when the two are out of sync?
> 
> 
> 2) Do we need a better (read "more easily maintainable") tool for
> handling database generation and updates?  libalpm already can read in
> information package files, so we could add libalpm/db_write.c with the
> database creation functions.   Should we unify our repo format with our
> local database format which we already write?
> 

I'd urge you not to make this a part of pacman. It's too far off the
beaten path for most users to make it a part of an already complicated
tool.

> 
> I am looking for ideas here.  Please brainstorm to your hearts content.

WRT replacing repo-add, I'd suggest we come up with a the use cases we
want to support, design an interface to meet them, and then come up with
the implementation. Might be nice to start with the Arch Linux
repository layout as an example that we'd want to support (pooled
packages with symlinks into repo dirs).

> Cheers,
> Allan

Re: [pacman-dev] [PATCH 4/4] [RFC] makepkg: unify times for generated files in srcdir before packaging

[Dave Reisner]

On Mon, Apr 17, 2017 at 10:03:03PM +1000, Allan McRae wrote:
> From: Levente Polyak 
> 
> Signed-off-by: Allan McRae 
> ---
> 
> [Allan] I'm told his is useful for some python packages that generate pyo/pyc
> files during package...  I am undecided about its suitability for inclusion
> in makepkg yet.
> 
>  scripts/makepkg.sh.in | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index df4d6a06..84b83e7d 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -493,6 +493,8 @@ run_package() {
>   pkgfunc="package_$1"
>   fi
>  
> + # unify source times before package for reproducibility
> + find "$srcdir" -exec touch -h -d "@${SOURCE_DATE_EPOCH}" {} \;

Same as 3/4 -- prefer {} +.

If we accept this patch, the commit message should include an
explanation as to why this is useful.

>   run_function_safe "$pkgfunc"
>  }
>  
> -- 
> 2.12.0

Re: [pacman-dev] [PATCH 3/4] makepkg: unify source file times for improved build reproducibility

[Dave Reisner]

On Mon, Apr 17, 2017 at 10:03:02PM +1000, Allan McRae wrote:
> From: Levente Polyak 
> 
> Signed-off-by: Allan McRae 
> ---
>  scripts/makepkg.sh.in | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 7692ade5..df4d6a06 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -475,6 +475,9 @@ run_prepare() {
>  }
>  
>  run_build() {
> + # unify source times before building for reproducibility
> + find "$srcdir" -exec touch -h -d "@${SOURCE_DATE_EPOCH}" {} \;
> +

I'd use the '{} +' form of find here to avoid excessive forking.

>   run_function_safe "build"
>  }
>  
> -- 
> 2.12.0

Re: [pacman-dev] [PATCH 1/4] makepkg: extract parts of the write_pkginfo for use elsewhere

[Dave Reisner]

On Mon, Apr 17, 2017 at 10:03:00PM +1000, Allan McRae wrote:
> From: Levente Polyak 
> 
> Signed-off-by: Allan McRae 
> ---

Sorry, a lot of these comments are irrelevant to the actual patch, but I
couldn't help pointing them out...

>  scripts/makepkg.sh.in | 42 ++
>  1 file changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 42a76004..d61c7fff 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -608,6 +608,15 @@ find_libprovides() {
>   (( ${#libprovides[@]} )) && printf '%s\n' "${libprovides[@]}"
>  }
>  
> +get_packager() {
> + if [[ -n $PACKAGER ]]; then
> + local packager="$PACKAGER"
> + else
> + local packager="Unknown Packager"
> + fi
> + printf "%s\n" "$packager"

I was going to suggest that we simply make this:

  printf '%s\n' "${PACKAGER:-Unknown Packager}"

But then it occurred to me that if we just set this default value up
front, we don't need to treat this var as special...

Actually relevant to this patch, why not define this as
'write_kv_packager' to match other functions here, like
'write_kv_pkgname' and 'write_kv_pkgver'?

> +}
> +
>  write_kv_pair() {
>   local key="$1"
>   shift
> @@ -621,13 +630,22 @@ write_kv_pair() {
>   done
>  }
>  
> -write_pkginfo() {
> - if [[ -n $PACKAGER ]]; then
> - local packager="$PACKAGER"
> - else
> - local packager="Unknown Packager"
> +write_kv_pkgname() {
> + write_kv_pair "pkgname" "$pkgname"
> + if (( SPLITPKG )) || [[ "$pkgbase" != "$pkgname" ]]; then
> + write_kv_pair "pkgbase" "$pkgbase"
> + fi

Wouldn't it be nice if we just *always* wrote the pkgbase?

> +}
> +
> +write_kv_pkgver() {
> + local fullver=$(get_full_version)
> + write_kv_pair "pkgver" "$fullver"
> + if [[ "$fullver" != "$basever" ]]; then
> + write_kv_pair "basever" "$basever"
>   fi

Since 8a02abcf19, disallow pkgver overrides in package functions.
Therefore, I'm unclear on when we'd ever emit this basever attr.

> +}
>  
> +write_pkginfo() {
>   local size="$(@DUPATH@ @DUFLAGS@)"
>   size="$(( ${size%%[^0-9]*} * 1024 ))"
>  
> @@ -637,16 +655,8 @@ write_pkginfo() {
>   printf "# Generated by makepkg %s\n" "$makepkg_version"
>   printf "# using %s\n" "$(fakeroot -v)"
>  
> - write_kv_pair "pkgname" "$pkgname"
> - if (( SPLITPKG )) || [[ "$pkgbase" != "$pkgname" ]]; then
> - write_kv_pair "pkgbase" "$pkgbase"
> - fi
> -
> - local fullver=$(get_full_version)
> - write_kv_pair "pkgver" "$fullver"
> - if [[ "$fullver" != "$basever" ]]; then
> - write_kv_pair "basever" "$basever"
> - fi
> + write_kv_pkgname
> + write_kv_pkgver
>  
>   # TODO: all fields should have this treatment
>   local spd="${pkgdesc//+([[:space:]])/ }"
> @@ -656,7 +666,7 @@ write_pkginfo() {
>   write_kv_pair "pkgdesc" "$spd"
>   write_kv_pair "url" "$url"
>   write_kv_pair "builddate" "$SOURCE_DATE_EPOCH"
> - write_kv_pair "packager" "$packager"
> + write_kv_pair "packager" "$(get_packager)"
>   write_kv_pair "size" "$size"
>   write_kv_pair "arch" "$pkgarch"
>  
> -- 
> 2.12.0

Re: [pacman-dev] [PATCH v2] Provide a better guess about who the packager is.

[Dave Reisner]

On Thu, Feb 02, 2017 at 02:39:21AM +1000, Allan McRae wrote:
> On 02/02/17 01:59, Kieran Colford wrote:
> > The system usually has enough information in various places to guess
> > the name and email of the person running the makepkg script.  We can
> > use this to make the defaults more intelligent.  This particular
> > implemenation should provide relatively good guess that's compatible
> > with other programs (i.e. git).
> > 
> > Signed-off-by: Kieran Colford 
> 
> This will not be included in makepkg.  Any "guessing" will inevitably be
> wrong at some stage and we will have more and more fixes requested on
> top of this.

Perhaps we should bail if PACKAGER isn't set, then.

> We have a single place to configure this information.  That is enough.
> 
> Allan

Re: [pacman-dev] [PATCH] Provide a better guess about who the packager is.

[Dave Reisner]

On Wed, Feb 01, 2017 at 01:48:09PM +, Kieran Colford wrote:
> Exactly, if they have PACKAGER set then we don't make any guesses at all,
> they've already told us exactly what they want.

But this patch gets rid of a lovely feature in arch's db-scripts which
causes an unset PACKAGER variable (resulting in "Unknown Packager") to
reject packages from being pushed into the repos. Your patch breaks
that, and we end up having to hunt down whomever "noclaf@rampage" is to
make them fix their ~/.makepkg.conf.

> On Wed, Feb 1, 2017, 8:43 AM Dave Reisner, <d...@falconindy.com> wrote:
> 
> > On Feb 1, 2017 08:33, "Kieran Colford" <kie...@kcolford.com> wrote:
> >
> > Would you care to share with us what pile of shit it returns? I can't fix
> > something when I don't know what's wrong.
> >
> > I never suggested that we work around laziness, people should absolutely
> > configure their system properly (I personally think violators should be
> > punished with regular kernel panics, but I don't think Linus will accept my
> > patch).
> >
> > I'm just trying to give default values that conform to what a user expects
> > to see: if they added a real name on account creation then it should show
> > up wherever a real name is needed, if they set the EMAIL environment
> > variable then software should assume that's the user's email and use it.
> > Those are both recommended by the wiki too.
> >
> >
> > And if they set PACKAGER it leaks into makepkg just the same, but better?
> >
> >
> >
> > On Wed, Feb 1, 2017, 12:47 AM Allan McRae, <al...@archlinux.org> wrote:
> >
> > > On 01/02/17 08:10, Kieran Colford wrote:
> > > > The system usually has enough information in various places to guess
> > > > the name and email of the person running the makepkg script. Use these
> > > > instead of defaulting to "Unknown Packager" to minimize configuration
> > > > necessary by the user.  This particular implemenation should provide
> > > > relatively good guess that's compatible with other programs
> > > > (i.e. git).
> > > >
> > > > Signed-off-by: Kieran Colford <kie...@kcolford.com>
> > >
> > > This returns a pile of shit on my system  We are not going to work
> > > around laziness.
> > >
> > > A
> > >
> > --
> >
> > Signed, Kieran Colford
> >
> -- 
> 
> Signed, Kieran Colford

Re: [pacman-dev] [PATCH] Provide a better guess about who the packager is.

[Dave Reisner]

On Feb 1, 2017 08:33, "Kieran Colford"  wrote:

Would you care to share with us what pile of shit it returns? I can't fix
something when I don't know what's wrong.

I never suggested that we work around laziness, people should absolutely
configure their system properly (I personally think violators should be
punished with regular kernel panics, but I don't think Linus will accept my
patch).

I'm just trying to give default values that conform to what a user expects
to see: if they added a real name on account creation then it should show
up wherever a real name is needed, if they set the EMAIL environment
variable then software should assume that's the user's email and use it.
Those are both recommended by the wiki too.


And if they set PACKAGER it leaks into makepkg just the same, but better?



On Wed, Feb 1, 2017, 12:47 AM Allan McRae,  wrote:

> On 01/02/17 08:10, Kieran Colford wrote:
> > The system usually has enough information in various places to guess
> > the name and email of the person running the makepkg script. Use these
> > instead of defaulting to "Unknown Packager" to minimize configuration
> > necessary by the user.  This particular implemenation should provide
> > relatively good guess that's compatible with other programs
> > (i.e. git).
> >
> > Signed-off-by: Kieran Colford 
>
> This returns a pile of shit on my system  We are not going to work
> around laziness.
>
> A
>
--

Signed, Kieran Colford

[pacman-dev] [PATCH] dload: s/CURLOPT_WRITEHEADER/CURLOPT_HEADERDATA/

[Dave Reisner]

The former is really old, and should be avoided.
---
 lib/libalpm/dload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index e889609..b0f81b6 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -267,7 +267,7 @@ static void curl_set_handle_opts(struct dload_payload 
*payload,
curl_easy_setopt(curl, CURLOPT_LOW_SPEED_LIMIT, 1L);
curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME, 10L);
curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, dload_parseheader_cb);
-   curl_easy_setopt(curl, CURLOPT_WRITEHEADER, (void *)payload);
+   curl_easy_setopt(curl, CURLOPT_HEADERDATA, (void *)payload);
curl_easy_setopt(curl, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
curl_easy_setopt(curl, CURLOPT_TCP_KEEPALIVE, 1L);
curl_easy_setopt(curl, CURLOPT_TCP_KEEPIDLE, 60L);
-- 
2.10.2

[pacman-dev] [PATCH 1/2] configure.ac: Use POSIX compatible equality checks

[Dave Reisner]

---
 configure.ac | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 63f87d1..10e4415 100644
--- a/configure.ac
+++ b/configure.ac
@@ -222,13 +222,13 @@ PKG_CHECK_MODULES(LIBARCHIVE, [libarchive >= 2.8.0], ,
 # Check for OpenSSL
 have_openssl=no
 have_nettle=no
-if test "x$with_crypto" == "xnettle"; then
+if test "x$with_crypto" = "xnettle"; then
PKG_CHECK_MODULES(NETTLE, [nettle],
[AC_DEFINE(HAVE_LIBNETTLE, 1, [Define whether to use nettle]) 
have_nettle=yes], have_nettle=no)
if test "x$have_nettle" = xno -a "x$with_crypto" = xnettle; then
AC_MSG_ERROR([*** nettle support requested but libraries not 
found])
fi
-else if test "x$with_crypto" == "xopenssl"; then
+else if test "x$with_crypto" = "xopenssl"; then
PKG_CHECK_MODULES(LIBSSL, [libcrypto],
[AC_DEFINE(HAVE_LIBSSL, 1, [Define if libcrypto is available]) 
have_openssl=yes], have_openssl=no)
if test "x$have_openssl" = xno; then
-- 
2.10.2

[pacman-dev] [PATCH 2/2] pacman: ensure linkage against libarchive

[Dave Reisner]

Fixes build on ubuntu/debian platforms.
---
 src/pacman/Makefile.am | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/pacman/Makefile.am b/src/pacman/Makefile.am
index 14572cd..8999f2b 100644
--- a/src/pacman/Makefile.am
+++ b/src/pacman/Makefile.am
@@ -47,6 +47,9 @@ pacman_SOURCES = \
util.h util.c \
util-common.h util-common.c
 
-LDADD = $(LTLIBINTL) $(top_builddir)/lib/libalpm/.libs/libalpm.la
+pacman_LDADD = \
+   $(LTLIBINTL) \
+   $(top_builddir)/lib/libalpm/.libs/libalpm.la \
+   $(LIBARCHIVE_LIBS)
 
 # vim:set noet:
-- 
2.10.2

Re: [pacman-dev] [PATCH 2/2] Provide source files for useful debug packages

[Dave Reisner]

On Mon, Dec 05, 2016 at 03:02:10PM +1000, Allan McRae wrote:
> Debug packages are fairly useless currently because the soucre files needed
> for stepping through code etc are not packaged with them. This patch adds the
> needed source files to the debug package and adjusts the debug info to look at
> the /usr/src/debug/ directory for them rather than the build location.  This
> requires using the "debugedit" program which is provided as part of the RPM
> sources.

...will pacman provide a copy of debugedit?

> Signed-off-by: Allan McRae 
> ---
>  scripts/libmakepkg/tidy/strip.sh.in | 21 -
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/libmakepkg/tidy/strip.sh.in 
> b/scripts/libmakepkg/tidy/strip.sh.in
> index 9baed9f..ff75cb4 100644
> --- a/scripts/libmakepkg/tidy/strip.sh.in
> +++ b/scripts/libmakepkg/tidy/strip.sh.in
> @@ -35,6 +35,11 @@ build_id() {
>   LANG=C readelf -n $1 | sed -n '/Build ID/ { s/.*: //p; q; }'
>  }
>  
> +source_files() {
> + LANG=C readelf $1 --debug-dump | \

"$1"

> + awk '/DW_AT_name +:/{name=$8}/DW_AT_comp_dir +:/{print $8 "/" 
> name}'
> +}
> +
>  strip_file() {
>   local binary=$1; shift
>  
> @@ -50,6 +55,18 @@ strip_file() {
>   return
>   fi
>  
> + # copy source files to debug directory
> + local f t
> + for f in $(source_files "$binary"); do

http://mywiki.wooledge.org/DontReadLinesWithFor

while read -r f; do
  ...
done < <(source_files "$binary")

> + t=${f/$srcdir/$dbgsrc}

t=${f/"$srcdir"/$dbgsrc}

> + mkdir -p "${t%/*}"
> + cp "$f" "$t"

cp -- "$f" "$t"

> + done
> +
> + # adjust debug symbols to point at sources
> + debugedit -b "${srcdir}" -d /usr/src/debug/ -i "$binary" &> 
> /dev/null
> +
> + # copy debug symbols to debug directory
>   mkdir -p "$dbgdir/${binary%/*}"
>   objcopy --only-keep-debug "$binary" "$dbgdir/$binary.debug"
>   objcopy --add-gnu-debuglink="$dbgdir/${binary#/}.debug" 
> "$binary"
> @@ -88,8 +105,10 @@ tidy_strip() {
>   [[ -z ${STRIP_STATIC+x} ]] && STRIP_STATIC="-S"
>  
>   if check_option "debug" "y"; then
> +
>   
> dbgdir="$pkgdirbase/$pkgbase-@DEBUGSUFFIX@/usr/lib/debug"
> - mkdir -p "$dbgdir"
> + 
> dbgsrc="$pkgdirbase/$pkgbase-@DEBUGSUFFIX@/usr/src/debug"
> + mkdir -p "$dbgdir" "$dbgsrc"
>   fi
>  
>   local binary strip_flags
> -- 
> 2.10.2

Re: [pacman-dev] Download options

[Dave Reisner]

On Fri, Dec 02, 2016 at 04:47:41PM +1000, Allan McRae wrote:
> There was many thread and patches regarding this, so I am starting another!
> 
> So far there are two (maybe three) things people want options for to do
> with the downloader.  I see most of these options being set in
> pacman.conf and rarely specified on the command line.  Argue with me if
> I am wrong here...
> 
> Based on that assumption, here are my preferred option names (for both
> pacman.conf and the command line):
> 
> 1) Disable low speed timeout
> 
> DisableDownloadTimeout
> --disable-download-timeout

As a user, I would have no idea what this means. As the person who wrote
the code that this affects, the thing which is most closely associated
with time in this equation can hardly be called a "timeout". It's more
of a threshold.

Some other suggestions, because my shed is better than your shed:

--disabledeadnetworkcheck
--nocheckdeadnetwork
--nodeadconncheck
--nodeadconnectioncheck
--namingishard

> I see no need to separate out speed and time timeouts.
> 
> 
> 2) Set maximum download speed
> 
> MaxDownloadSpeed
> --max-download-speed

I think I've pointed out before that this is a lie at best. You aren't
actually throttling anything, you're just adding artificial delay. You
can also do this from outside of pacman:

https://lists.archlinux.org/pipermail/pacman-dev/2016-September/021367.html

FWIW, it's no longer true that curl uses a flat average -- it's now a
rolling window which should provide a smoother limiting mechanism.
However, that's a very recent change (7.50.2), and requires a libcurl
far newer than what we need for compilation (7.32.0).

> I would also accept replacing speed with limit.
> 
> 
> 3) (in a glorious future) Set maximum concurrent downloads
> 
> MaxConcurrentDownloads
> --max-concurrent-downloads
> 
> 
> Any argument about these option names will need to be very clearly
> justified.  I don't own a bike so have no need for a bikeshed.
> 
> Allan

Re: [pacman-dev] [PATCH 1/1] always accept untranslated answers 'Y' and 'N'

[Dave Reisner]

On Fri, Nov 11, 2016 at 09:34:05PM +0100, Christian Hesse wrote:
> Lukas Fleischer  on Fri, 2016/11/11 21:23:
> > On Fri, 11 Nov 2016 at 21:15:48, Christian Hesse wrote:
> > > From: Christian Hesse 
> > > 
> > > 'YES' translates to 'JA' in German, thus answer 'J' is expected for
> > > positive answer. This changes the behaviour to always accept 'Y'
> > > and 'N', in addition to the translated values.
> >
> > Not sure whether it is a problem in practice but what happens if "N" is
> > translated to "Y" in some language? Do we really want to accept if the
> > user enters "Y" in that case?
> 
> A valid point...
> Does such a language exist?

It does! Looks like Uzbek would break if we did this:

src/pacman/po/uz.po:msgid "No"
src/pacman/po/uz.po-msgstr "Yo'q"

> All my systems are configured with English locale, except my wife's and my
> mother's one. My blind typing for pacman commands breaks there. :-p
> 
> Well, possibly I should just set the root account to English locale... :D
> -- 
> main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
> "CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
> putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}

[pacman-dev] [PATCH 6/6] makepkg: unset potentially architecture-specific vars

[Dave Reisner]

I'm not convinced this is a worthwhile goal, but let's follow suit.
Since we can't know the names of all the vars that might exist, unset
them by pattern.
---
 scripts/makepkg.sh.in | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index c212ffc..e7a506f 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -1455,7 +1455,11 @@ fi
 
 unset pkgname pkgbase pkgver pkgrel epoch pkgdesc url license groups provides
 unset md5sums replaces depends conflicts backup source install changelog build
-unset makedepends optdepends options noextract validpgpkeys
+unset sha{1,224,256,384,512}sums makedepends optdepends options noextract 
validpgpkeys
+unset "${!makedepends_@}" "${!depends_@}" "${!source_@}" "${!checkdepends_@}"
+unset "${!optdepends_@}" "${!conflicts_@}" "${!provides_@}" "${!replaces_@}"
+unset "${!md5sums_@}" "${!sha1sums_@}" "${!sha224sums_@}" "${!sha256sums_@}"
+unset "${!sha384sums_@}" "${!sha512sums_@}"
 
 BUILDFILE=${BUILDFILE:-$BUILDSCRIPT}
 if [[ ! -f $BUILDFILE ]]; then
-- 
2.10.2

[pacman-dev] [PATCH 5/6] makepkg: fix quoting in calls to dependency checking

[Dave Reisner]

---
 scripts/makepkg.sh.in | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 0aabc25..c212ffc 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -243,7 +243,7 @@ handle_deps() {
 
(( $# == 0 )) && return $R_DEPS_SATISFIED
 
-   local deplist="$*"
+   local deplist=("$@")
 
if (( ! DEP_BIN )); then
return $R_DEPS_MISSING
@@ -253,7 +253,7 @@ handle_deps() {
# install missing deps from binary packages (using pacman -S)
msg "$(gettext "Installing missing dependencies...")"
 
-   if ! run_pacman -S --asdeps $deplist; then
+   if ! run_pacman -S --asdeps "${deplist[@]}"; then
error "$(gettext "'%s' failed to install missing 
dependencies.")" "$PACMAN"
exit 1 # TODO: error code
fi
@@ -276,10 +276,10 @@ resolve_deps() {
# deplist cannot be declared like this: local deplist=$(foo)
# Otherwise, the return value will depend on the assignment.
local deplist
-   deplist=$(check_deps "$@") || exit 1
+   deplist=($(check_deps "$@")) || exit 1
[[ -z $deplist ]] && return $R_DEPS_SATISFIED
 
-   if handle_deps $deplist; then
+   if handle_deps "${deplist[@]}"; then
# check deps again to make sure they were resolved
deplist=$(check_deps "$@") || exit 1
[[ -z $deplist ]] && return $R_DEPS_SATISFIED
-- 
2.10.2

[pacman-dev] [PATCH 4/6] makepkg: fix quoting in calls to check_deps

[Dave Reisner]

The inside needs quoting, and this is separate from the declartion,
which does not (in these cases).
---
 scripts/makepkg.sh.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index a97cdc2..0aabc25 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -276,12 +276,12 @@ resolve_deps() {
# deplist cannot be declared like this: local deplist=$(foo)
# Otherwise, the return value will depend on the assignment.
local deplist
-   deplist="$(check_deps $*)" || exit 1
+   deplist=$(check_deps "$@") || exit 1
[[ -z $deplist ]] && return $R_DEPS_SATISFIED
 
if handle_deps $deplist; then
# check deps again to make sure they were resolved
-   deplist="$(check_deps $*)" || exit 1
+   deplist=$(check_deps "$@") || exit 1
[[ -z $deplist ]] && return $R_DEPS_SATISFIED
fi
 
@@ -962,7 +962,7 @@ check_vcs_software() {
client=$(get_vcsclient "$proto") || 
exit $?
# ensure specified program is installed
local uninstalled
-   uninstalled="$(check_deps $client)" || 
exit 1
+   uninstalled=$(check_deps "$client") || 
exit 1
# if not installed, check presence in 
depends or makedepends
if [[ -n "$uninstalled" ]] && (( ! 
NODEPS || ( VERIFYSOURCE && !DEP_BIN ) )); then
if ! in_array "$client" 
${all_deps[@]}; then
-- 
2.10.2

[pacman-dev] [PATCH 1/6] makepkg.conf: add -g to default curl options

[Dave Reisner]

This disables globbing, which should never be used in source URL
specifications as it would lead to mismatches in the checksum mapping
and un-checked sources.
---
 etc/makepkg.conf.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/etc/makepkg.conf.in b/etc/makepkg.conf.in
index efac16a..7129397 100644
--- a/etc/makepkg.conf.in
+++ b/etc/makepkg.conf.in
@@ -8,9 +8,9 @@
 #
 #-- The download utilities that makepkg should use to acquire sources
 #  Format: 'protocol::agent'
-DLAGENTS=('ftp::/usr/bin/curl -qfC - --ftp-pasv --retry 3 --retry-delay 3 -o 
%o %u'
-  'http::/usr/bin/curl -qb "" -fLC - --retry 3 --retry-delay 3 -o %o 
%u'
-  'https::/usr/bin/curl -qb "" -fLC - --retry 3 --retry-delay 3 -o %o 
%u'
+DLAGENTS=('ftp::/usr/bin/curl -gqfC - --ftp-pasv --retry 3 --retry-delay 3 -o 
%o %u'
+  'http::/usr/bin/curl -gqb "" -fLC - --retry 3 --retry-delay 3 -o %o 
%u'
+  'https::/usr/bin/curl -gqb "" -fLC - --retry 3 --retry-delay 3 -o %o 
%u'
   'rsync::/usr/bin/rsync --no-motd -z %u %o'
   'scp::/usr/bin/scp -C %u %o')
 
-- 
2.10.2

[pacman-dev] [PATCH 0/6] Assorted makepkg cleanups

[Dave Reisner]

Here's a bunch of patches I've had sitting on a branch for a while, plus a new
one at the end which I thought would be a good idea after I found an instance of
"{$pkgver}" in a PKGBUILD.

These (and the other dload/curl patch) can be pulled from
git://github.com/falconindy/pacman.

Dave Reisner (6):
  makepkg.conf: add -g to default curl options
  makepkg: remove vestiges of global errexit
  makepkg: make run_function_safe more robust
  makepkg: fix quoting in calls to check_deps
  makepkg: fix quoting in calls to dependency checking
  makepkg: unset potentially architecture-specific vars

 etc/makepkg.conf.in   |  6 +++---
 scripts/makepkg.sh.in | 37 ++---
 2 files changed, 25 insertions(+), 18 deletions(-)

-- 
2.10.2

[pacman-dev] [PATCH 2/6] makepkg: remove vestiges of global errexit

[Dave Reisner]

These 'set +E' diversions haven't been needed since global errexit was
disabled in dca10b062f2 (January 2012).
---
 scripts/makepkg.sh.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 02398cf..a1385c1 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -276,12 +276,12 @@ resolve_deps() {
# deplist cannot be declared like this: local deplist=$(foo)
# Otherwise, the return value will depend on the assignment.
local deplist
-   deplist="$(set +E; check_deps $*)" || exit 1
+   deplist="$(check_deps $*)" || exit 1
[[ -z $deplist ]] && return $R_DEPS_SATISFIED
 
if handle_deps $deplist; then
# check deps again to make sure they were resolved
-   deplist="$(set +E; check_deps $*)" || exit 1
+   deplist="$(check_deps $*)" || exit 1
[[ -z $deplist ]] && return $R_DEPS_SATISFIED
fi
 
@@ -959,7 +959,7 @@ check_vcs_software() {
client=$(get_vcsclient "$proto") || 
exit $?
# ensure specified program is installed
local uninstalled
-   uninstalled="$(set +E; check_deps 
$client)" || exit 1
+   uninstalled="$(check_deps $client)" || 
exit 1
# if not installed, check presence in 
depends or makedepends
if [[ -n "$uninstalled" ]] && (( ! 
NODEPS || ( VERIFYSOURCE && !DEP_BIN ) )); then
if ! in_array "$client" 
${all_deps[@]}; then
-- 
2.10.2

[pacman-dev] [PATCH] dload: use curl's keepalive mechanism

[Dave Reisner]

This does exactly the same thing as it code it replaces, but punt to
curl to do it for brevity. Requires curl 7.25.0, which we already cover.
---
 lib/libalpm/dload.c | 46 +++---
 1 file changed, 3 insertions(+), 43 deletions(-)

diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 9d80358..ccd70d9 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -246,47 +246,6 @@ static size_t dload_parseheader_cb(void *ptr, size_t size, 
size_t nmemb, void *u
return realsize;
 }
 
-static int dload_sockopt_cb(void *userdata, curl_socket_t curlfd,
-   curlsocktype purpose)
-{
-   alpm_handle_t *handle = userdata;
-   int optval = 1;
-
-   /* this whole method is to prevent FTP control connections from going 
sour
-* during a long data transfer; crappy firewalls love to drop otherwise 
idle
-* connections if there is no traffic. */
-   if(purpose != CURLSOCKTYPE_IPCXN) {
-   return 0;
-   }
-
-   /* don't abort operation if any setsockopt fails, just log to debug */
-   if(setsockopt(curlfd, SOL_SOCKET, SO_KEEPALIVE, (void *),
-   sizeof(optval)) < 0) {
-   _alpm_log(handle, ALPM_LOG_DEBUG,
-   "Failed to set SO_KEEPALIVE on fd %d\n", 
curlfd);
-   }
-   else {
-#ifdef TCP_KEEPIDLE
-   optval = 60;
-   if(setsockopt(curlfd, IPPROTO_TCP, TCP_KEEPIDLE, (void 
*),
-   sizeof(optval)) < 0) {
-   _alpm_log(handle, ALPM_LOG_DEBUG,
-   "Failed to set TCP_KEEPIDLE on fd 
%d\n", curlfd);
-   }
-#endif
-#ifdef TCP_KEEPINTVL
-   optval = 60;
-   if(setsockopt(curlfd, IPPROTO_TCP, TCP_KEEPINTVL, (void 
*),
-   sizeof(optval)) < 0) {
-   _alpm_log(handle, ALPM_LOG_DEBUG,
-   "Failed to set TCP_KEEPINTVL on fd 
%d\n", curlfd);
-   }
-#endif
-   }
-
-   return 0;
-}
-
 static void curl_set_handle_opts(struct dload_payload *payload,
CURL *curl, char *error_buffer)
 {
@@ -310,8 +269,9 @@ static void curl_set_handle_opts(struct dload_payload 
*payload,
curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, dload_parseheader_cb);
curl_easy_setopt(curl, CURLOPT_WRITEHEADER, (void *)payload);
curl_easy_setopt(curl, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
-   curl_easy_setopt(curl, CURLOPT_SOCKOPTFUNCTION, dload_sockopt_cb);
-   curl_easy_setopt(curl, CURLOPT_SOCKOPTDATA, (void *)handle);
+   curl_easy_setopt(curl, CURLOPT_TCP_KEEPALIVE, 1L);
+   curl_easy_setopt(curl, CURLOPT_TCP_KEEPIDLE, 60L);
+   curl_easy_setopt(curl, CURLOPT_TCP_KEEPINTVL, 60L);
curl_easy_setopt(curl, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
 
_alpm_log(handle, ALPM_LOG_DEBUG, "url: %s\n", payload->fileurl);
-- 
2.10.2

Re: [pacman-dev] [PATCH] Add per-repo PinnedPubKey option

[Dave Reisner]

On Mon, Oct 31, 2016 at 04:36:23PM -0400, Travis Burtrum wrote:
> From abb057844eec0e5707c31b643d0f2187b4cf0eb6 Mon Sep 17 00:00:00 2001
> From: Travis Burtrum 
> Date: Mon, 31 Oct 2016 02:12:31 -0400
> Subject: [PATCH] Add per-repo PinnedPubKey option
> 
> This sets curl's CURLOPT_PINNEDPUBLICKEY option in the built-in
> downloader, or replaces %p in XferCommand.  This pins public
> keys to ensure your TLS connection is not man-in-the-middled
> without relying on CAs etc.  Probably most useful currently
> for very small groups or single servers.
> 
> It would obviously be best as a per-mirror option, but such
> a thing currently does not exist.

But perhaps as part of a larger scope, it could... As mentioned on IRC,
I'm not a huge fan of this.

> Signed-off-by: Travis Burtrum 
> ---
>  doc/pacman.conf.5.txt | 12 +++-
>  lib/libalpm/alpm.h| 10 +-
>  lib/libalpm/be_sync.c |  4 
>  lib/libalpm/db.c  | 12 
>  lib/libalpm/db.h  |  1 +
>  lib/libalpm/dload.c   |  8 +++-
>  lib/libalpm/dload.h   |  1 +
>  lib/libalpm/sync.c|  7 ---
>  src/pacman/conf.c | 26 --
>  src/pacman/conf.h |  1 +
>  10 files changed, 74 insertions(+), 8 deletions(-)
> 
> diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt
> index c665870..b1859c7 100644
> --- a/doc/pacman.conf.5.txt
> +++ b/doc/pacman.conf.5.txt
> @@ -126,7 +126,8 @@ Options
>   All instances of `%u` will be replaced with the download URL. If 
> present,
>   instances of `%o` will be replaced with the local filename, plus a
>   ``.part'' extension, which allows programs like wget to do file resumes
> - properly.
> + properly. If present, instances of `%p` will be replaced with the value 
> of
> + the repo specific PinnedPubKey directive, or `` if that is not set.
>   +
>   This option is useful for users who experience problems with built-in
>   HTTP/FTP support, or need the more advanced proxy support that comes 
> with
> @@ -276,6 +277,15 @@ even be used for different architectures.
>  Note that an enabled repository can be operated on explicitly, regardless of 
> the Usage
>  level set.
>  
> +*PinnedPubKey =* pinnedpubkey::
> + The string can be the file name of your pinned public key. The file 
> format expected
> + is "PEM" or "DER".  The string can also be any number of base64 encoded 
> sha256 
> + hashes preceded by "sha256//" and separated by ";"
> ++
> +When negotiating a TLS or SSL connection, the server sends a certificate 
> indicating
> +its identity. A public key is extracted from this certificate and if it does 
> not
> +exactly match the public key provided to this option, pacman will abort the
> +connection before sending or receiving any data.
>  
>  Package and Database Signature Checking[[SC]]
>  -
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index 2d2491d..e942559 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -750,11 +750,12 @@ typedef void (*alpm_cb_totaldl)(off_t total);
>   * @param url the URL of the file to be downloaded
>   * @param localpath the directory to which the file should be downloaded
>   * @param force whether to force an update, even if the file is the same
> + * @param pinnedpubkey a pinned public key string
>   * @return 0 on success, 1 if the file exists and is identical, -1 on
>   * error.
>   */
>  typedef int (*alpm_cb_fetch)(const char *url, const char *localpath,
> - int force);
> + int force, const char *pinnedpubkey);

If the signature of this must change, I'd strongly suggest we think
about some sort of more public payload struct which we can extend, and
shipping that across the callback. However, see below where I explain
more about why I think this is a Bad Idea.

>  
>  /** Fetch a remote pkg.
>   * @param handle the context handle
> @@ -1037,6 +1038,13 @@ alpm_list_t *alpm_db_get_groupcache(alpm_db_t *db);
>   */
>  alpm_list_t *alpm_db_search(alpm_db_t *db, const alpm_list_t *needles);
>  
> +/** Sets the pinned public key of a database.
> + * @param db pointer to the package database to set the status for
> + * @param pinnedpubkey a pinned public key string
> + * @return 0 on success, or -1 on error
> + */
> +int alpm_db_set_pinnedpubkey(alpm_db_t *db, char *pinnedpubkey);
> +
>  typedef enum _alpm_db_usage_ {
>   ALPM_DB_USAGE_SYNC = 1,
>   ALPM_DB_USAGE_SEARCH = (1 << 1),
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index 7774975..be2cd64 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -242,6 +242,8 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
>   payload.handle = handle;
>   payload.force = force;
>   payload.unlink_on_fail = 1;
> + 
> + payload.pinnedpubkey = db->pinnedpubkey;
>  
>  

Re: [pacman-dev] [PATCH v2] Parametrise the different ways in which the payload is reset

[Dave Reisner]

On Fri, Oct 14, 2016 at 09:26:46PM +0200, mar77i wrote:
> In FS#43434, Downloads which fail and are restarted on a different server
> will resume and may display a negative download speed. The payload's progress
> in libalpm was not properly reset which ultimately caused terminal noise
> because the line width calculation assumes positive download speeds.
> 
> This patch fixes the incomplete reset of the payload by mimicing what
> be_sync.c:alpm_db_update() does over in sync.c:download_single_file().
> dload.c:_alpm_dload_payload_reset() bundles and extends over the current
> behavior by updating initial_size and prevprogress for this case.
> This makes pacman reset the progress properly in the next invocation of the
> callback and display positive download speeds.
> 
> Fixes FS#43434.
> 
> Signed-off-by: Martin Kühne 
> ---
>  lib/libalpm/be_sync.c |  4 ++--
>  lib/libalpm/dload.c   | 15 +++
>  lib/libalpm/dload.h   |  2 +-
>  lib/libalpm/sync.c|  4 +---
>  4 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index a836277..2425359 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -244,7 +244,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
>   payload.unlink_on_fail = 1;
>  
>   ret = _alpm_download(, syncpath, NULL, _db_url);
> - _alpm_dload_payload_reset();
> + _alpm_dload_payload_reset(, 0);
>   updated = (updated || ret == 0);
>  
>   if(ret != -1 && updated && (level & ALPM_SIG_DATABASE)) {
> @@ -300,7 +300,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db)
>   sig_ret = _alpm_download(, syncpath, NULL, 
> NULL);
>   /* errors_ok suppresses error messages, but not the 
> return code */
>   sig_ret = payload.errors_ok ? 0 : sig_ret;
> - _alpm_dload_payload_reset();
> + _alpm_dload_payload_reset(, 0);

This is kind of opaque at a glance. What does 0 mean? Potential
modifications:

1) Add an inline comment:

  _alpm_dload_payload_reset(, 0 /* do not allow resume */);

2) Separate this out into 2 wrappers:

  _alpm_dload_payload_reset_for_retry();
  _alpm_dload_payload_reset();
  _alpm_dload_payload_reset_internal(, 0|1);

I don't feel too strongly about this, though, since this sort of
opaqueness is evident elsewhere in the codebase.

>   }
>  
>   if(ret != -1 && sig_ret != -1) {
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index dc57c92..55ff41f 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -697,7 +697,7 @@ char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t *handle, 
> const char *url)
>  
>   /* download the file */
>   ret = _alpm_download(, cachedir, _file, 
> _pkg_url);
> - _alpm_dload_payload_reset();
> + _alpm_dload_payload_reset(, 0);
>   if(ret == -1) {
>   _alpm_log(handle, ALPM_LOG_WARNING, _("failed to 
> download %s\n"), url);
>   free(final_file);
> @@ -738,7 +738,7 @@ char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t *handle, 
> const char *url)
>   FREE(sig_final_file);
>   }
>   free(sig_filepath);
> - _alpm_dload_payload_reset();
> + _alpm_dload_payload_reset(, 0);
>   }
>  
>   /* we should be able to find the file the second time around */
> @@ -750,15 +750,22 @@ char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t 
> *handle, const char *url)
>   return filepath;
>  }
>  
> -void _alpm_dload_payload_reset(struct dload_payload *payload)
> +void _alpm_dload_payload_reset(struct dload_payload *payload, int 
> allow_resume)
>  {
>   ASSERT(payload, return);
>  
> + FREE(payload->fileurl);
> + if(allow_resume) {
> + payload->initial_size += payload->prevprogress;
> + payload->prevprogress = 0;
> + payload->unlink_on_fail = 0;
> + return;
> + }
> +
>   FREE(payload->remote_name);
>   FREE(payload->tempfile_name);
>   FREE(payload->destfile_name);
>   FREE(payload->content_disp_name);
> - FREE(payload->fileurl);
>   memset(payload, '\0', sizeof(*payload));
>  }
>  
> diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h
> index 427c486..c9c94b8 100644
> --- a/lib/libalpm/dload.h
> +++ b/lib/libalpm/dload.h
> @@ -46,7 +46,7 @@ struct dload_payload {
>  #endif
>  };
>  
> -void _alpm_dload_payload_reset(struct dload_payload *payload);
> +void _alpm_dload_payload_reset(struct dload_payload *payload, int 
> allow_resume);
>  
>  int _alpm_download(struct dload_payload *payload, const char *localpath,
>   char **final_file, const char **final_url);
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index 00b68d0..81900df 100644
> --- 

Re: [pacman-dev] [PATCH][WIP] Allow replacing libcrypto with libnettle in pacman

[Dave Reisner]

On Tue, Oct 11, 2016 at 09:57:34PM +1000, Allan McRae wrote:
> Add a --with-nettle configure option that directs pacman to use the libnettle
> hashing functions. Only one of the --with-libssl and --with-nettle configure
> options can be specified.
> 
> Signed-off-by: Allan McRae 
> ---
> 
> I did not write this - it is based heavily off a patch submitted to flyspray.
> I will change the author information once it is provided.
> 
> Current caveats:
> 1) openssl is still used in makepkg.  This was also addressed in the original
> patch, but needs work
> 2) to use libnettle, you need to use --with-nettle --without-libssl due to
> setting openssl usage to be the default.  I'm not sure how I should address
> that.
> 
>  configure.ac  | 38 ++
>  lib/libalpm/Makefile.am   |  6 --
>  lib/libalpm/libalpm.pc.in |  2 +-
>  lib/libalpm/util.c| 41 +++--
>  4 files changed, 74 insertions(+), 13 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index f6b87e5..625ba68 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -120,10 +120,15 @@ AC_ARG_WITH(ldconfig,
>   [set the full path to ldconfig]),
>   [LDCONFIG=$withval], [LDCONFIG=/sbin/ldconfig])
>  
> -# Help line for using OpenSSL
> +# Help line for using OpenSSL (enabled by default)
>  AC_ARG_WITH(openssl,
> - AS_HELP_STRING([--with-openssl], [use OpenSSL crypto implementations 
> instead of internal routines]),
> - [], [with_openssl=check])
> + AS_HELP_STRING([--with-openssl], [use OpenSSL crypto implementations 
> @<:@default=yes@:>@]),
> + [], [with_openssl=yes])
> +
> +# Help line for using nettle (disabled by default)
> +AC_ARG_WITH(nettle,
> + AS_HELP_STRING([--with-nettle], [use nettle crypto implementations 
> @<:@default=no@:>@]),
> + [], [with_nettle=no])
>  
>  # Help line for using gpgme
>  AC_ARG_WITH(gpgme,
> @@ -218,9 +223,19 @@ AC_CHECK_LIB([m], [fabs], ,
>  PKG_CHECK_MODULES(LIBARCHIVE, [libarchive >= 2.8.0], ,
>   AC_MSG_ERROR([*** libarchive >= 2.8.0 is needed to compile pacman!]))
>  
> +# Check if at least one crypto backend is selected
> +AS_IF([test "x$with_openssl" == "xno" -a "x$with_nettle" == "xno"],
> + [AC_MSG_ERROR([*** pacman requires at least one crypto library to be 
> available])]
> +)
> +
> +# Check if exactly one crypto backend is selected
> +AS_IF([test "x$with_openssl" == "xyes" -a "x$with_nettle" == "xyes"],
> + [AC_MSG_ERROR([*** --with-openssl and --with-nettle are mutually 
> exclusive.])]
> +)
> +
>  # Check for OpenSSL
>  have_openssl=no
> -if test "x$with_openssl" != "xno"; then
> +if test "x$with_openssl" == "xyes"; then
>   PKG_CHECK_MODULES(LIBSSL, [libcrypto],
>   [AC_DEFINE(HAVE_LIBSSL, 1, [Define if libcrypto is available]) 
> have_openssl=yes], have_openssl=no)
>   if test "x$have_openssl" = xno -a "x$with_openssl" = xyes; then
> @@ -229,10 +244,16 @@ if test "x$with_openssl" != "xno"; then
>  fi
>  AM_CONDITIONAL(HAVE_LIBSSL, [test "$have_openssl" = "yes"])
>  
> -# Ensure one library for generating checksums is present
> -if test "$have_openssl" != "yes"; then
> - AC_MSG_ERROR([*** no library for checksum generation found])
> +# Check for nettle
> +have_nettle=no
> +if test "x$with_nettle" == "xyes"; then
> + PKG_CHECK_MODULES(NETTLE, [nettle],
> + [AC_DEFINE(HAVE_LIBNETTLE, 1, [Define whether to use nettle]) 
> have_nettle=yes], have_nettle=no)
> + if test "x$have_nettle" = xno -a "x$with_nettle" = xyes; then
> + AC_MSG_ERROR([*** nettle support requested but libraries not 
> found])
> + fi
>  fi
> +AM_CONDITIONAL(HAVE_LIBNETTLE, [test "$have_nettle" = "yes"])
>  
>  # Check for libcurl
>  have_libcurl=no
> @@ -542,7 +563,7 @@ ${PACKAGE_NAME}:
>  compiler   : ${CC}
>  preprocessor flags : ${CPPFLAGS}
>  compiler flags : ${WARNING_CFLAGS} ${CFLAGS}
> -library flags  : ${LIBS} ${LIBSSL_LIBS} ${LIBARCHIVE_LIBS} 
> ${LIBCURL_LIBS} ${GPGME_LIBS}
> +library flags  : ${LIBS} ${LIBSSL_LIBS} ${NETTLE_LIBS} 
> ${LIBARCHIVE_LIBS} ${LIBCURL_LIBS} ${GPGME_LIBS}
>  linker flags   : ${LDFLAGS}
>  
>  Architecture   : ${CARCH}
> @@ -569,6 +590,7 @@ ${PACKAGE_NAME}:
>  Use libcurl: ${have_libcurl}
>  Use GPGME  : ${have_gpgme}
>  Use OpenSSL: ${have_openssl}
> +Use nettle : ${have_nettle}
>  Run make in doc/ dir   : ${wantdoc} ${asciidoc}
>  Doxygen support: ${usedoxygen}
>  debug support  : ${debug}
> diff --git a/lib/libalpm/Makefile.am b/lib/libalpm/Makefile.am
> index 945a612..f4f20e6 100644
> --- a/lib/libalpm/Makefile.am
> +++ b/lib/libalpm/Makefile.am
> @@ -65,13 +65,15 @@ libalpm_la_CFLAGS = \
>   $(GPGME_CFLAGS) \
>   $(LIBARCHIVE_CFLAGS) \
>   $(LIBCURL_CFLAGS) \
> - $(LIBSSL_CFLAGS)
> + 

Re: [pacman-dev] [RFC v3 06/13] paccache: streamline usage function

[Dave Reisner]

On Fri, Sep 30, 2016 at 01:47:54PM +0200, Gordian Edenhofer wrote:
> Signed-off-by: Gordian Edenhofer 
> ---
>  contrib/paccache.sh.in | 83 
> --
>  1 file changed, 53 insertions(+), 30 deletions(-)
> 
> diff --git a/contrib/paccache.sh.in b/contrib/paccache.sh.in
> index 02fae52..78f566f 100644
> --- a/contrib/paccache.sh.in
> +++ b/contrib/paccache.sh.in
> @@ -30,6 +30,17 @@ declaredelim=$'\n' keep=3 movedir= scanarch=
>  QUIET=0
>  USE_COLOR='y'
>  
> +# gettext initialization
> +export TEXTDOMAIN='pacman'
> +export TEXTDOMAINDIR='@localedir@'
> +
> +# Determine whether we have gettext; make it a no-op if we do not
> +if ! type -p gettext >/dev/null; then
> + gettext() {
> + printf "%s\n" "$@"
> + }
> +fi
> +
>  m4_include(../scripts/library/output_format.sh)
>  m4_include(../scripts/library/parseopts.sh)
>  
> @@ -172,36 +183,48 @@ summarize() {
>  }
>  
>  usage() {
> - cat < -${myname} (pacman) v${myver}
> -
> -A flexible pacman cache cleaning utility.
> -
> -Usage: ${myname}  [options] [targets...]
> -
> -  Operations:
> --d, --dryrun  perform a dry run, only finding candidate packages.
> --m, --move   move candidate packages to "dir".
> --r, --remove  remove candidate packages.
> -
> -  Options:
> --a, --arch  scan for "arch" (default: all architectures).
> --c, --cachedir   scan "dir" for packages. can be used more than 
> once.
> -  (default: read from @sysconfdir@/pacman.conf).
> --f, --force   apply force to mv(1) and rm(1) operations.
> --h, --helpdisplay this help message and exit.
> --i, --ignoreignore "pkgs", comma-separated. Alternatively, 
> specify
> -  "-" to read package names from stdin, newline-
> -  delimited.
> --k, --keep   keep "num" of each package in the cache (default: 
> 3).
> ---nocolor remove color from output.
> --q, --quiet   minimize output
> --u, --uninstalled target uninstalled packages.
> --v, --verbose increase verbosity. specify up to 3 times.
> --z, --nulluse null delimiters for candidate names (only with 
> -v
> -  and -vv).
> -
> -EOF
> + printf "%s (pacman) %s\n" "$myname" "$myver"
> + echo
> + printf -- "$(gettext "A flexible pacman cache cleaning utility")\n"
> + echo
> + printf -- "$(gettext "Usage: %s  [options] [targets...]")\n" 
> "$0"
> + echo
> + printf -- "$(gettext "Operations:")\n"
> + printf -- "  -d, --dryrun  "
> + printf -- "$(gettext "Perform a dry run, only finding candidate 
> packages")\n"
> + printf -- "  -m, --move   "
> + printf -- "$(gettext "Move candidate packages to \"dir\"")\n"
> + printf -- "  -r, --remove  "
> + printf -- "$(gettext "Remove candidate packages")\n"
> + echo
> + printf -- "$(gettext "Options:")\n"
> + printf -- "  -a, --arch  "
> + printf -- "$(gettext "Scan for \"arch\" (default: all 
> architectures)")\n"
> + printf -- "  -c, --cachedir   "
> + printf -- "$(gettext "Scan \"dir\" for packages. Can be used more than 
> once.\n\
> + (default: read from %s).")\n" 
> "@sysconfdir@/pacman.conf"
> + printf -- "  -f, --force   "
> + printf -- "$(gettext "Apply force to mv(1) and rm(1) operations")\n" 
> "\$srcdir/"
> + printf -- "  -h, --help"
> + printf -- "$(gettext "Display this help message and exit")\n"
> + printf -- "  -i, --ignore"
> + printf -- "$(gettext "Ignore \"pkgs\", comma-separated. Alternatively, 
> specify\n\
> + \"-\" to read package names from stdin, 
> newline-\n\
> + delimited.")\n" "\$srcdir/"
> + printf -- "  -k, --keep   "
> + printf -- "$(gettext "Keep \"num\" of each package in the cache 
> (default: 3)")\n"
> + printf -- "  -q, --quiet   "
> + printf -- "$(gettext "Minimize output")\n"
> + printf -- "  -u, --uninstalled "
> + printf -- "$(gettext "Target uninstalled packages")\n"
> + printf -- "  -v, --verbose "
> + printf -- "$(gettext "Increase verbosity. specify up to 3 times.")\n"
> + printf -- "  -z, --null"
> + printf -- "$(gettext "Use null delimiters for candidate names (only 
> with -v\n\
> + and -vv)")\n"
> + printf -- "  --nocolor "
> + printf -- "$(gettext "Remove color from output")\n"
> + echo

I'm going to need convincing that this fits the definition of
"streamlined" compared to the current implementation.

  stream·line
  /ˈstrēmˌlīn/

  1.  design or provide with a form that presents very little resistance to a
  flow of air or water, increasing speed and ease of movement.
  2.  

Re: [pacman-dev] [PATCH 00/11] Streamline syntax of contrib scripts

[Dave Reisner]

On Thu, Sep 29, 2016 at 03:30:30PM +0200, Gordian Edenhofer wrote:
> On Thu, 2016-09-29 at 09:04 -0400, Dave Reisner wrote:
> > On Thu, Sep 29, 2016 at 12:23:00PM +0200, Gordian Edenhofer wrote:
> > > 
> > > Reorganize scripts in contrib to fit the syntax and style of
> > > makepkg.
> > > Unify multiple different approaches to e.g. write a usage function,
> > > handle
> > > options and differences in naming schemas for options.
> > 
> > Again, you're still putting flag names in the translatable strings.
> > This
> > will only ever end badly.
> 
> It has worked great for makepkg, see the git repo for dozens of
> examples [1].
> 
> [1] https://git.archlinux.org/pacman.git/tree/scripts/makepkg.sh.in#n11
> 66

Has it worked great?

- id.po contains --noprapare instead of --noprepare
- da.po contains --finer instead of --finger
- sv.po contains --inout instead of --input

Repeating this mistake of allowing flags to be translated is purely
cargo-cult nonsense. Please don't do it.

Re: [pacman-dev] [PATCH 00/11] Streamline syntax of contrib scripts

[Dave Reisner]

On Thu, Sep 29, 2016 at 12:23:00PM +0200, Gordian Edenhofer wrote:
> Reorganize scripts in contrib to fit the syntax and style of makepkg.
> Unify multiple different approaches to e.g. write a usage function, handle
> options and differences in naming schemas for options.

Again, you're still putting flag names in the translatable strings. This
will only ever end badly.

> Gordian Edenhofer (11):
>   checkupdates: streamline usage function
>   paccache: streamline usage function
>   pacdiff: streamline usage function
>   pacdiff: proper option handling
>   pacdiff: streamline option syntax with makepkg
>   paclist: streamline usage function
>   paclog-pkglist: streamline usage function
>   pacscripts: streamline usage function
>   updpkgsums: streamline usage function
>   rankmirrors: streamline usage function
>   rankmirrors: proper option handling
> 
>  contrib/checkupdates.sh.in   |   9 +--
>  contrib/paccache.sh.in   |  61 ++--
>  contrib/pacdiff.sh.in|  76 +++--
>  contrib/paclist.sh.in|  10 ++--
>  contrib/paclog-pkglist.sh.in |  12 ++--
>  contrib/pacscripts.sh.in |  18 +++---
>  contrib/rankmirrors.sh.in| 132 
> +--
>  contrib/updpkgsums.sh.in |  21 ---
>  8 files changed, 179 insertions(+), 160 deletions(-)
> 
> --
> 2.10.0