[pacman-dev] [RFC] _RESERVED_IDS & enum assignments, take 2

2016-09-03 Thread ivy . foster
I've had another go at the patches from yesterday.

The first patch, which eliminates use of #define
_RESERVED_IDENTIFIERS, has one problem keeping me from adding
-Wreserved-id-macro to configure.ac: the autoconf-generated file
config.h.in contains one, _DARWIN_USE_64_BIT_INODE. I'm honestly not
sure how to prevent autoconf from adding this, or how to make it print
it at the very bottom after a "#pragma GCC system_header".

The second is a much more complete version of the patch that added
ALPM_ERR_OK. This version handles *every* enum type that is used by
functions that return either a member of that enum or an error, and
allows building cleanly with clang -Wassign-enum.

The RFC in the subject is because for patch #2, I discovered that a
common code pattern in pacman is using enum types to generate
bitfields. Technically, the resulting bitfield isn't a member of the
enum*, which caused many a warning with -Wassign-enum. As a quick and
dirty solution, I declared these bitfield variables to be ints rather
than (say) _alpm_siglevel_t. It encodes less information than
declaring them as the enum type, but is technically more accurate.
Would it be better to use unions or something?

Comments welcome. Cheers.

Ivy

*:
enum foo {
BAR = 1 << 0, /* 01 */
BAZ = 1 << 1; /* 10 */
/* Not included: 11 */
}


[pacman-dev] [PATCH v2 1/2] Do not #define _RESERVED_IDENTIFIERS

2016-09-03 Thread ivy . foster
From: Ivy Foster 

Signed-off-by: Ivy Foster 
---
 lib/libalpm/add.h   | 6 +++---
 lib/libalpm/alpm.h  | 6 +++---
 lib/libalpm/alpm_list.h | 6 +++---
 lib/libalpm/backup.h| 6 +++---
 lib/libalpm/base64.h| 4 ++--
 lib/libalpm/conflict.h  | 6 +++---
 lib/libalpm/db.h| 6 +++---
 lib/libalpm/delta.h | 6 +++---
 lib/libalpm/deps.h  | 6 +++---
 lib/libalpm/diskspace.h | 6 +++---
 lib/libalpm/dload.h | 6 +++---
 lib/libalpm/filelist.h  | 6 +++---
 lib/libalpm/graph.h | 6 +++---
 lib/libalpm/group.h | 6 +++---
 lib/libalpm/handle.h| 6 +++---
 lib/libalpm/hook.h  | 6 +++---
 lib/libalpm/libarchive-compat.h | 6 +++---
 lib/libalpm/log.h   | 6 +++---
 lib/libalpm/md5.h   | 4 ++--
 lib/libalpm/package.h   | 6 +++---
 lib/libalpm/pkghash.h   | 6 +++---
 lib/libalpm/remove.h| 6 +++---
 lib/libalpm/sha2.h  | 4 ++--
 lib/libalpm/signing.h   | 6 +++---
 lib/libalpm/sync.h  | 6 +++---
 lib/libalpm/trans.h | 6 +++---
 lib/libalpm/util.h  | 6 +++---
 src/common/ini.h| 6 +++---
 src/common/util-common.h| 6 +++---
 src/pacman/callback.h   | 6 +++---
 src/pacman/check.h  | 6 +++---
 src/pacman/conf.h   | 6 +++---
 src/pacman/package.h| 6 +++---
 src/pacman/pacman.h | 6 +++---
 src/pacman/sighandler.h | 6 +++---
 src/pacman/util.h   | 6 +++---
 36 files changed, 105 insertions(+), 105 deletions(-)

diff --git a/lib/libalpm/add.h b/lib/libalpm/add.h
index aa707fa..c1ab62a 100644
--- a/lib/libalpm/add.h
+++ b/lib/libalpm/add.h
@@ -17,8 +17,8 @@
  *  You should have received a copy of the GNU General Public License
  *  along with this program.  If not, see .
  */
-#ifndef _ALPM_ADD_H
-#define _ALPM_ADD_H
+#ifndef ALPM_ADD_H
+#define ALPM_ADD_H
 
 #include "db.h"
 #include "alpm_list.h"
@@ -26,6 +26,6 @@
 
 int _alpm_upgrade_packages(alpm_handle_t *handle);
 
-#endif /* _ALPM_ADD_H */
+#endif /* ALPM_ADD_H */
 
 /* vim: set noet: */
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 168d71b..7955585 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -20,8 +20,8 @@
  *  You should have received a copy of the GNU General Public License
  *  along with this program.  If not, see .
  */
-#ifndef _ALPM_H
-#define _ALPM_H
+#ifndef ALPM_H
+#define ALPM_H
 
 #ifdef __cplusplus
 extern "C" {
@@ -1624,6 +1624,6 @@ void alpm_conflict_free(alpm_conflict_t *conflict);
 #ifdef __cplusplus
 }
 #endif
-#endif /* _ALPM_H */
+#endif /* ALPM_H */
 
 /* vim: set noet: */
diff --git a/lib/libalpm/alpm_list.h b/lib/libalpm/alpm_list.h
index 5af84e1..cf7d463 100644
--- a/lib/libalpm/alpm_list.h
+++ b/lib/libalpm/alpm_list.h
@@ -17,8 +17,8 @@
  *  You should have received a copy of the GNU General Public License
  *  along with this program.  If not, see .
  */
-#ifndef _ALPM_LIST_H
-#define _ALPM_LIST_H
+#ifndef ALPM_LIST_H
+#define ALPM_LIST_H
 
 #include  /* size_t */
 
@@ -90,6 +90,6 @@ void *alpm_list_to_array(const alpm_list_t *list, size_t n, 
size_t size);
 #ifdef __cplusplus
 }
 #endif
-#endif /* _ALPM_LIST_H */
+#endif /* ALPM_LIST_H */
 
 /* vim: set noet: */
diff --git a/lib/libalpm/backup.h b/lib/libalpm/backup.h
index 2e11dbc..5cf3f90 100644
--- a/lib/libalpm/backup.h
+++ b/lib/libalpm/backup.h
@@ -17,8 +17,8 @@
  *  You should have received a copy of the GNU General Public License
  *  along with this program.  If not, see .
  */
-#ifndef _ALPM_BACKUP_H
-#define _ALPM_BACKUP_H
+#ifndef ALPM_BACKUP_H
+#define ALPM_BACKUP_H
 
 #include "alpm_list.h"
 #include "alpm.h"
@@ -28,6 +28,6 @@ alpm_backup_t *_alpm_needbackup(const char *file, alpm_pkg_t 
*pkg);
 void _alpm_backup_free(alpm_backup_t *backup);
 alpm_backup_t *_alpm_backup_dup(const alpm_backup_t *backup);
 
-#endif /* _ALPM_BACKUP_H */
+#endif /* ALPM_BACKUP_H */
 
 /* vim: set noet: */
diff --git a/lib/libalpm/base64.h b/lib/libalpm/base64.h
index df684ab..9edb864 100644
--- a/lib/libalpm/base64.h
+++ b/lib/libalpm/base64.h
@@ -22,8 +22,8 @@
  *  along with this program.  If not, see .
  */
 
-#ifndef _BASE64_H
-#define _BASE64_H
+#ifndef BASE64_H
+#define BASE64_H
 
 #include 
 
diff --git a/lib/libalpm/conflict.h b/lib/libalpm/conflict.h
index e17d552..801c201 100644
--- a/lib/libalpm/conflict.h
+++ b/lib/libalpm/conflict.h
@@ -17,8 +17,8 @@
  *  You should have received a copy of the GNU General Public License
  *  along with this program.  If not, see .
  */
-#ifndef _ALPM_CONFLICT_H
-#define _ALPM_CONFLICT_H
+#ifndef ALPM_CONFLICT_H
+#define ALPM_CONFLICT_H
 
 

[pacman-dev] [PATCH v2 2/2] Make pacman build cleanly with clang's -Wassign-enum

2016-09-03 Thread ivy . foster
From: Ivy Foster 

In many cases, it was enough to add error or "none" values to enums,
to account for cases where functions returned either an enumerated
value or 0/-1.

A common code pattern in pacman is to use enums to create bitfields
representing various option combinations. Since they are not
technically part of the enumerated type used to build them, these
bitfields have been reassigned to type int.

Signed-off-by: Ivy Foster 
---
 configure.ac |  1 +
 lib/libalpm/alpm.c   |  4 +--
 lib/libalpm/alpm.h   | 15 +++--
 lib/libalpm/be_package.c |  2 +-
 lib/libalpm/be_sync.c|  4 +--
 lib/libalpm/db.c | 20 ++--
 lib/libalpm/db.h |  2 +-
 lib/libalpm/dload.c  |  2 +-
 lib/libalpm/package.c| 84 
 lib/libalpm/package.h|  2 +-
 lib/libalpm/signing.c|  6 ++--
 lib/libalpm/sync.c   | 10 +++---
 lib/libalpm/trans.c  |  2 +-
 lib/libalpm/util.h   |  2 +-
 src/pacman/conf.c|  4 +--
 src/pacman/conf.h| 12 +++
 src/pacman/database.c|  2 +-
 src/util/cleanupdelta.c  |  2 +-
 src/util/pactree.c   |  2 +-
 src/util/testpkg.c   |  2 +-
 20 files changed, 96 insertions(+), 84 deletions(-)

diff --git a/configure.ac b/configure.ac
index 51288e7..2f9aeab 100644
--- a/configure.ac
+++ b/configure.ac
@@ -430,6 +430,7 @@ fi
 AC_MSG_CHECKING(for excessive compiler warning flags)
 if test "x$warningflags" = "xyes" ; then
AC_MSG_RESULT(yes)
+   CFLAGS_ADD([-Wassign-enum], [WARNING_CFLAGS])
CFLAGS_ADD([-Wcast-align], [WARNING_CFLAGS])
CFLAGS_ADD([-Wclobbered], [WARNING_CFLAGS])
CFLAGS_ADD([-Wempty-body], [WARNING_CFLAGS])
diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c
index 6b7fa7a..6338429 100644
--- a/lib/libalpm/alpm.c
+++ b/lib/libalpm/alpm.c
@@ -152,9 +152,9 @@ const char SYMEXPORT *alpm_version(void)
 /** Get the capabilities of the library.
  * @return a bitmask of the capabilities
  * */
-enum alpm_caps SYMEXPORT alpm_capabilities(void)
+int SYMEXPORT alpm_capabilities(void)
 {
-   return 0
+   return ALPM_CAPABILITY_NONE
 #ifdef ENABLE_NLS
| ALPM_CAPABILITY_NLS
 #endif
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 7955585..cb3aa51 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -53,7 +53,8 @@ typedef struct __alpm_trans_t alpm_trans_t;
  * @{
  */
 typedef enum _alpm_errno_t {
-   ALPM_ERR_MEMORY = 1,
+   ALPM_ERR_OK = 0,
+   ALPM_ERR_MEMORY,
ALPM_ERR_SYSTEM,
ALPM_ERR_BADPERMS,
ALPM_ERR_NOT_A_FILE,
@@ -143,6 +144,7 @@ typedef int64_t alpm_time_t;
 
 /** Package install reasons. */
 typedef enum _alpm_pkgreason_t {
+   ALPM_PKG_REASON_ERROR = -1,
/** Explicitly requested by the user. */
ALPM_PKG_REASON_EXPLICIT = 0,
/** Installed as a dependency for another package. */
@@ -151,6 +153,7 @@ typedef enum _alpm_pkgreason_t {
 
 /** Location a package object was loaded from. */
 typedef enum _alpm_pkgfrom_t {
+   ALPM_PKG_FROM_ERROR = -1,
ALPM_PKG_FROM_FILE = 1,
ALPM_PKG_FROM_LOCALDB,
ALPM_PKG_FROM_SYNCDB
@@ -158,6 +161,7 @@ typedef enum _alpm_pkgfrom_t {
 
 /** Method used to validate a package. */
 typedef enum _alpm_pkgvalidation_t {
+   ALPM_PKG_VALIDATION_ERROR = -1,
ALPM_PKG_VALIDATION_UNKNOWN = 0,
ALPM_PKG_VALIDATION_NONE = (1 << 0),
ALPM_PKG_VALIDATION_MD5SUM = (1 << 1),
@@ -193,6 +197,9 @@ typedef enum _alpm_fileconflicttype_t {
 
 /** PGP signature verification options */
 typedef enum _alpm_siglevel_t {
+   ALPM_SIG_ERROR = -1,
+   ALPM_SIG_NONE = 0,
+
ALPM_SIG_PACKAGE = (1 << 0),
ALPM_SIG_PACKAGE_OPTIONAL = (1 << 1),
ALPM_SIG_PACKAGE_MARGINAL_OK = (1 << 2),
@@ -1037,6 +1044,7 @@ 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);
 
 typedef enum _alpm_db_usage_ {
+   ALPM_DB_USAGE_NONE = 0,
ALPM_DB_USAGE_SYNC = 1,
ALPM_DB_USAGE_SEARCH = (1 << 1),
ALPM_DB_USAGE_INSTALL = (1 << 2),
@@ -1439,6 +1447,8 @@ alpm_pkg_t *alpm_sync_newversion(alpm_pkg_t *pkg, 
alpm_list_t *dbs_sync);
 
 /** Transaction flags */
 typedef enum _alpm_transflag_t {
+   ALPM_TRANS_FLAG_ERROR = -1,
+   ALPM_TRANS_FLAG_NONE = 0,
/** Ignore dependency checks. */
ALPM_TRANS_FLAG_NODEPS = 1,
/** Ignore file conflicts and overwrite files. */
@@ -1606,13 +1616,14 @@ int alpm_release(alpm_handle_t *handle);
 int alpm_unlock(alpm_handle_t *handle);
 
 enum alpm_caps {
+   ALPM_CAPABILITY_NONE = 0,
ALPM_CAPABILITY_NLS = (1 << 0),
ALPM_CAPABILITY_DOWNLOADER = (1 << 1),
ALPM_CAPABILITY_SIGNATURES = (1 << 2)
 };
 
 const char *alpm_version(void);
-enum alpm_caps alpm_capabilities(void);
+int alpm_capabilities(void);
 
 void 

[pacman-dev] [PATCH v7 12/12] bacman man page: describe the jobs option

2016-09-03 Thread Gordian Edenhofer
Signed-off-by: Gordian Edenhofer 
---
 contrib/doc/bacman.8.txt | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/contrib/doc/bacman.8.txt b/contrib/doc/bacman.8.txt
index a9d7ba9..4738189 100644
--- a/contrib/doc/bacman.8.txt
+++ b/contrib/doc/bacman.8.txt
@@ -35,6 +35,9 @@ Options
 *-m, \--nocolor*::
Disable colored output.
 
+*-j, \--jobs *::
+   Assemble multiple packages in parallel.
+
 *-o, \--out *::
Write the assembled package(s) to the specified directory.
 
@@ -53,10 +56,11 @@ Recreate the package ``linux-headers''.
 Assemble the packages gzip munge binutils and place the packages at
 ``~/Downloads''.
 
-'bacman' --nocolor --pacnew -o ~/backup $(pacman -Qsq)
+'bacman' --nocolor --pacnew -o ~/backup -j 5 $(pacman -Qsq)
 
-Assemble all currently installed packages using ``.pacnew'' whenever available,
-suppress colored output and place the desired packages in ``~/backup''.
+Assemble all currently installed packages using 5 concurrent jobs, use
+``.pacnew'' whenever available, suppress colored output and place the desired
+packages in ``~/backup''.
 
 
 See Also
-- 
2.9.3


[pacman-dev] [PATCH v7 10/12] bacman: add option to specify the number of jobs

2016-09-03 Thread Gordian Edenhofer
Signed-off-by: Gordian Edenhofer 
---
 contrib/bacman.sh.in | 33 +++--
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/contrib/bacman.sh.in b/contrib/bacman.sh.in
index b92b4e4..7ae9058 100644
--- a/contrib/bacman.sh.in
+++ b/contrib/bacman.sh.in
@@ -57,12 +57,13 @@ usage() {
printf -- "$(gettext "  -h, --help   Show this help message and 
exit")\n"
printf -- "$(gettext "  -q, --quiet  Silence most of the status 
reporting")\n"
printf -- "$(gettext "  -m, --nocolorDisable colorized output 
messages")\n"
+   printf -- "$(gettext "  -j, --jobsBuild in parallel with N jobs 
- you may want to set XZ_OPT")\n"
printf -- "$(gettext "  -o, --out   Write output to specified 
directory (instead of \$PKGDEST)")\n"
printf -- "$(gettext "  --pacnew Package .pacnew files")\n"
echo
printf -- "$(gettext "Examples:  %s linux-headers")\n" "$myname"
printf -- "$(gettext "  %s gzip munge binutils -o ~/Downloads")\n" 
"$myname"
-   printf -- "$(gettext "  %s --nocolor --pacnew -o /tmp gzip munge 
binutils")\n" "$myname"
+   printf -- "$(gettext "  %s --nocolor --pacnew -o /tmp -j 5 gzip munge 
binutils")\n" "$myname"
printf -- "$(gettext "  %s \$(pacman -Qsq)")\n" "$myname"
echo
 }
@@ -89,8 +90,8 @@ for option in "$@"; do
 done
 
 # Parse arguments
-OPT_SHORT='o:qmv'
-OPT_LONG=('out:' 'quiet' 'nocolor' 'pacnew' 'version')
+OPT_SHORT='o:j:qmv'
+OPT_LONG=('out:' 'jobs:' 'quiet' 'nocolor' 'pacnew' 'version')
 if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then
usage
exit 1
@@ -104,6 +105,14 @@ while :; do
pkg_dest=$2
[[ ! -d "$2" ]] && echo -e "The directory 
\e[39;1m$2\e[0m does not exist!" && exit 3
shift ;;
+   -j|--jobs)
+   if [[ $2 =~ ^-?[0-9]+$ ]]; then
+   MAX_JOBS=$2
+   else
+   echo -e "\e[39;1m$2\e[0m is not a valid 
integer!"
+   exit -1
+   fi
+   shift ;;
-q|--quiet)
QUIET=1 ;;
-m|--nocolor)
@@ -190,7 +199,11 @@ fakebuild() {
cd "$work_dir" || exit 1
 
# Assemble list of files which belong to the package and tar them
-   msg2 "Copying package files..."
+   if [[ $MAX_JOBS -gt 1 ]]; then
+   msg2 "${pkg_name}: Copying package files..."
+   else
+   msg2 "Copying package files..."
+   fi
 
while read i; do
if [[ -z $i ]]; then
@@ -265,7 +278,11 @@ fakebuild() {
 
# Reconstruct .PKGINFO from database
# TODO adopt makepkg's write_pkginfo() into this or scripts/library
-   msg2 "Generating .PKGINFO metadata..."
+   if [[ $MAX_JOBS -gt 1 ]]; then
+   msg2 "${pkg_name}: Generating .PKGINFO metadata..."
+   else
+   msg2 "Generating .PKGINFO metadata..."
+   fi
echo "# Generated by $myname $myver"> .PKGINFO
if [[ $INFAKEROOT == "1" ]]; then
echo "# Using $(fakeroot -v)">> .PKGINFO
@@ -353,7 +370,11 @@ fakebuild() {
chmod 644 "$work_dir"/{.PKGINFO,.CHANGELOG,.INSTALL} 2> /dev/null
 
# Generate the package
-   msg2 "Generating the package..."
+   if [[ $MAX_JOBS -gt 1 ]]; then
+   msg2 "${pkg_name}: Generating the package..."
+   else
+   msg2 "Generating the package..."
+   fi
 
pkg_file="$pkg_dest/$pkg_namver-$pkg_arch${PKGEXT}"
ret=0
-- 
2.9.3


[pacman-dev] [PATCH v7 09/12] bacman: add otpion to alter the output directory

2016-09-03 Thread Gordian Edenhofer
Signed-off-by: Gordian Edenhofer 
---
 contrib/bacman.sh.in | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/contrib/bacman.sh.in b/contrib/bacman.sh.in
index 94b09d7..b92b4e4 100644
--- a/contrib/bacman.sh.in
+++ b/contrib/bacman.sh.in
@@ -57,10 +57,12 @@ usage() {
printf -- "$(gettext "  -h, --help   Show this help message and 
exit")\n"
printf -- "$(gettext "  -q, --quiet  Silence most of the status 
reporting")\n"
printf -- "$(gettext "  -m, --nocolorDisable colorized output 
messages")\n"
+   printf -- "$(gettext "  -o, --out   Write output to specified 
directory (instead of \$PKGDEST)")\n"
printf -- "$(gettext "  --pacnew Package .pacnew files")\n"
echo
printf -- "$(gettext "Examples:  %s linux-headers")\n" "$myname"
-   printf -- "$(gettext "  %s --nocolor --pacnew gzip munge binutils")\n" 
"$myname"
+   printf -- "$(gettext "  %s gzip munge binutils -o ~/Downloads")\n" 
"$myname"
+   printf -- "$(gettext "  %s --nocolor --pacnew -o /tmp gzip munge 
binutils")\n" "$myname"
printf -- "$(gettext "  %s \$(pacman -Qsq)")\n" "$myname"
echo
 }
@@ -87,8 +89,8 @@ for option in "$@"; do
 done
 
 # Parse arguments
-OPT_SHORT='qmv'
-OPT_LONG=('quiet' 'nocolor' 'pacnew' 'version')
+OPT_SHORT='o:qmv'
+OPT_LONG=('out:' 'quiet' 'nocolor' 'pacnew' 'version')
 if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then
usage
exit 1
@@ -98,6 +100,10 @@ unset OPT_SHORT OPT_LONG OPTRET
 
 while :; do
case "$1" in
+   -o|--out)
+   pkg_dest=$2
+   [[ ! -d "$2" ]] && echo -e "The directory 
\e[39;1m$2\e[0m does not exist!" && exit 3
+   shift ;;
-q|--quiet)
QUIET=1 ;;
-m|--nocolor)
@@ -149,7 +155,8 @@ source "@sysconfdir@/makepkg.conf"
 if [[ -r ~/.makepkg.conf ]]; then
source ~/.makepkg.conf
 fi
-pkg_dest="${PKGDEST:-$PWD}"
+PKGDEST="${PKGDEST:-$PWD}"
+pkg_dest="${pkg_dest:-$PKGDEST}"
 pkg_pkger="${PACKAGER:-'Unknown Packager'}"
 
 # Check for an existing database
-- 
2.9.3


[pacman-dev] [PATCH v7 11/12] bacman: add manual page

2016-09-03 Thread Gordian Edenhofer
Signed-off-by: Gordian Edenhofer 
---
 contrib/doc/.gitignore   |  1 +
 contrib/doc/Makefile.am  |  5 +++-
 contrib/doc/bacman.8.txt | 66 
 3 files changed, 71 insertions(+), 1 deletion(-)
 create mode 100644 contrib/doc/bacman.8.txt

diff --git a/contrib/doc/.gitignore b/contrib/doc/.gitignore
index c5612bc..3ab2035 100644
--- a/contrib/doc/.gitignore
+++ b/contrib/doc/.gitignore
@@ -1 +1,2 @@
 verify-pacman-repo-db.1
+bacman.8
diff --git a/contrib/doc/Makefile.am b/contrib/doc/Makefile.am
index 4c316bb..d5725b1 100644
--- a/contrib/doc/Makefile.am
+++ b/contrib/doc/Makefile.am
@@ -4,12 +4,14 @@
 # man_MANS if --enable-asciidoc and/or --enable-doxygen are used.
 
 ASCIIDOC_MANS = \
-   verify-pacman-repo-db.1
+   verify-pacman-repo-db.1 \
+   bacman.8
 
 EXTRA_DIST = \
asciidoc.conf \
footer.txt \
verify-pacman-repo-db.1.txt \
+   bacman.8.txt \
$(ASCIIDOC_MANS)
 
 # Files that should be removed, but which Automake does not know.
@@ -53,5 +55,6 @@ $(ASCIIDOC_MANS): asciidoc.conf footer.txt Makefile.am
 
 # Dependency rules
 verify-pacman-repo-db.1: verify-pacman-repo-db.1.txt
+bacman.8: bacman.8.txt
 
 # vim:set noet:
diff --git a/contrib/doc/bacman.8.txt b/contrib/doc/bacman.8.txt
new file mode 100644
index 000..a9d7ba9
--- /dev/null
+++ b/contrib/doc/bacman.8.txt
@@ -0,0 +1,66 @@
+/
+vim:set ts=4 sw=4 syntax=asciidoc noet spell spelllang=en_us:
+/
+bacman(8)
+==
+
+Name
+
+bacman - recreate installed packages
+
+
+Synopsis
+
+'bacman' [options] 
+
+
+Description
+---
+'bacman' was designed to reassemble installed packages from its deliverd files.
+It comes in handy if there is no internet connection available and you have no
+access to a up-to-date package cache.
+
+'bacman' fully honors linkman:makepkg.conf[8] and all compression environment
+variables, notably ``XZ_OPT''.
+
+
+Options
+---
+*-h, \--help*::
+   Display usage information.
+
+*-q, \--quiet*::
+   Silence most of the status reporting.
+
+*-m, \--nocolor*::
+   Disable colored output.
+
+*-o, \--out *::
+   Write the assembled package(s) to the specified directory.
+
+*\--pacnew*::
+   Package .pacnew files instead of the concerning files currently in 
place.
+
+
+Examples
+
+'bacman' linux-headers
+
+Recreate the package ``linux-headers''.
+
+'bacman' gzip munge binutils -o ~/Downloads
+
+Assemble the packages gzip munge binutils and place the packages at
+``~/Downloads''.
+
+'bacman' --nocolor --pacnew -o ~/backup $(pacman -Qsq)
+
+Assemble all currently installed packages using ``.pacnew'' whenever available,
+suppress colored output and place the desired packages in ``~/backup''.
+
+
+See Also
+
+linkman:makepkg[8], linkman:pacman[8]
+
+include::footer.txt[]
-- 
2.9.3


[pacman-dev] [PATCH v7 08/12] bacman: add option to print fewer status updates

2016-09-03 Thread Gordian Edenhofer
Signed-off-by: Gordian Edenhofer 
---
Move indentation fixes into its own commit.

 contrib/bacman.sh.in | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/contrib/bacman.sh.in b/contrib/bacman.sh.in
index 6f943a1..94b09d7 100644
--- a/contrib/bacman.sh.in
+++ b/contrib/bacman.sh.in
@@ -28,6 +28,7 @@ declare -r myname='bacman'
 declare -r myver='@PACKAGE_VERSION@'
 USE_COLOR='y'
 INCLUDE_PACNEW='n'
+QUIET=0
 # Required for fakeroot because options are shifted off the array.
 ARGS=("$@")
 
@@ -54,6 +55,7 @@ usage() {
echo
printf -- "$(gettext "Options:")\n"
printf -- "$(gettext "  -h, --help   Show this help message and 
exit")\n"
+   printf -- "$(gettext "  -q, --quiet  Silence most of the status 
reporting")\n"
printf -- "$(gettext "  -m, --nocolorDisable colorized output 
messages")\n"
printf -- "$(gettext "  --pacnew Package .pacnew files")\n"
echo
@@ -85,8 +87,8 @@ for option in "$@"; do
 done
 
 # Parse arguments
-OPT_SHORT='mv'
-OPT_LONG=('nocolor' 'pacnew' 'version')
+OPT_SHORT='qmv'
+OPT_LONG=('quiet' 'nocolor' 'pacnew' 'version')
 if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then
usage
exit 1
@@ -96,6 +98,8 @@ unset OPT_SHORT OPT_LONG OPTRET
 
 while :; do
case "$1" in
+   -q|--quiet)
+   QUIET=1 ;;
-m|--nocolor)
USE_COLOR='n' ;;
--pacnew)
-- 
2.9.3


[pacman-dev] [PATCH v7 05/12] bacman: rewrite usage function

2016-09-03 Thread Gordian Edenhofer
Signed-off-by: Gordian Edenhofer 
---
Move indentation fixes into its own commit.

 contrib/bacman.sh.in | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/contrib/bacman.sh.in b/contrib/bacman.sh.in
index 639a538..c853958 100644
--- a/contrib/bacman.sh.in
+++ b/contrib/bacman.sh.in
@@ -48,13 +48,21 @@ trap clean_up SIGHUP SIGINT SIGTERM
 # User Friendliness
 #
 usage() {
-   echo "${myname} (pacman) v${myver}"
+   printf "%s (pacman) %s\n" "$myname" "$myver"
echo
-   echo "Recreate a package using pacman's database and system files"
+   printf -- "$(gettext "Recreate packages using pacman's database and 
system files")\n"
echo
-   echo "Usage: ${myname} [--nocolor] [--pacnew] "
+   printf -- "$(gettext "Usage: %s [options] ")\n" "$0"
+   echo
+   printf -- "$(gettext "Options:")\n"
+   printf -- "$(gettext "  -h, --help   Show this help message and 
exit")\n"
+   printf -- "$(gettext "  -m, --nocolorDisable colorized output 
messages")\n"
+   printf -- "$(gettext "  --pacnew Package .pacnew files")\n"
+   echo
+   printf -- "$(gettext "Examples:  %s linux-headers")\n" "$myname"
+   printf -- "$(gettext "  %s --nocolor --pacnew gzip munge binutils")\n" 
"$myname"
+   printf -- "$(gettext "  %s \$(pacman -Qsq)")\n" "$myname"
echo
-   echo "Example: ${myname} linux-headers"
 }
 
 version() {
-- 
2.9.3


[pacman-dev] [PATCH v7 06/12] bacman: code structuring

2016-09-03 Thread Gordian Edenhofer
Adding and clarifying comments.
Adding and removing some new lines.

Signed-off-by: Gordian Edenhofer 
---
 contrib/bacman.sh.in | 25 -
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/contrib/bacman.sh.in b/contrib/bacman.sh.in
index c853958..d0309b6 100644
--- a/contrib/bacman.sh.in
+++ b/contrib/bacman.sh.in
@@ -44,9 +44,7 @@ clean_up() {
 # Trap termination signals
 trap clean_up SIGHUP SIGINT SIGTERM
 
-#
-# User Friendliness
-#
+# Print usage information
 usage() {
printf "%s (pacman) %s\n" "$myname" "$myver"
echo
@@ -65,6 +63,7 @@ usage() {
echo
 }
 
+# Print version information
 version() {
printf "%s %s\n" "$myname" "$myver"
echo 'Copyright (C) 2008 locci '
@@ -118,9 +117,7 @@ if [[ ${#pkg_list[@]} == 0 ]]; then
exit 1
 fi
 
-#
-# Fakeroot support
-#
+# Run with fake root privileges if EUID is not root
 if (( EUID )); then
if [[ -f /usr/bin/fakeroot ]]; then
msg "Entering fakeroot environment"
@@ -133,33 +130,25 @@ if (( EUID )); then
fi
 fi
 
-#
-# Setting environmental variables
-#
+# Source environmental variables and specify fallbacks
 if [[ ! -r @sysconfdir@/pacman.conf ]]; then
error "unable to read @sysconfdir@/pacman.conf"
exit 1
 fi
-
 eval $(awk '/DBPath/ {print $1$2$3}' @sysconfdir@/pacman.conf)
 pac_db="${DBPath:-@localstatedir@/lib/pacman/}/local"
-
 if [[ ! -r @sysconfdir@/makepkg.conf ]]; then
error "unable to read @sysconfdir@/makepkg.conf"
exit 1
 fi
-
 source "@sysconfdir@/makepkg.conf"
 if [[ -r ~/.makepkg.conf ]]; then
source ~/.makepkg.conf
 fi
-
 pkg_dest="${PKGDEST:-$PWD}"
 pkg_pkger=${PACKAGER:-'Unknown Packager'}
 
-#
-# Checks everything is in place
-#
+# Check for an existing database
 if [[ ! -d $pac_db ]]; then
error "pacman database directory ${pac_db} not found"
exit 1
@@ -191,6 +180,7 @@ fakebuild() {
 
# Assemble list of files which belong to the package and tar them
msg2 "Copying package files..."
+
while read i; do
if [[ -z $i ]]; then
continue
@@ -357,6 +347,7 @@ fakebuild() {
pkg_file="$pkg_dest/$pkg_namver-$pkg_arch${PKGEXT}"
ret=0
 
+   # Move compressed package to destination
# TODO: Maybe this can be set globally for robustness
shopt -s -o pipefail
bsdtar -cf - $comp_files * |
@@ -370,7 +361,7 @@ fakebuild() {
"$PKGEXT"; cat ;;
esac > "${pkg_file}"; ret=$?
 
-   # Move compressed package to destination
+   # Evaluate return code
if (( ret )); then
error "Unable to write package to $pkg_dest"
plain "   Maybe the disk is full or you do not have write 
access"
-- 
2.9.3


[pacman-dev] [PATCH v7 07/12] bacman: quote pkg_pkger variable

2016-09-03 Thread Gordian Edenhofer
Signed-off-by: Gordian Edenhofer 
---
 contrib/bacman.sh.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/bacman.sh.in b/contrib/bacman.sh.in
index d0309b6..6f943a1 100644
--- a/contrib/bacman.sh.in
+++ b/contrib/bacman.sh.in
@@ -146,7 +146,7 @@ if [[ -r ~/.makepkg.conf ]]; then
source ~/.makepkg.conf
 fi
 pkg_dest="${PKGDEST:-$PWD}"
-pkg_pkger=${PACKAGER:-'Unknown Packager'}
+pkg_pkger="${PACKAGER:-'Unknown Packager'}"
 
 # Check for an existing database
 if [[ ! -d $pac_db ]]; then
-- 
2.9.3


[pacman-dev] [PATCH v7 04/12] bacman: proper option handling

2016-09-03 Thread Gordian Edenhofer
Switch to parseopts instead of merely checking the first argument.

Signed-off-by: Gordian Edenhofer 
---
 contrib/bacman.sh.in | 62 +++-
 1 file changed, 42 insertions(+), 20 deletions(-)

diff --git a/contrib/bacman.sh.in b/contrib/bacman.sh.in
index 0c71541..639a538 100644
--- a/contrib/bacman.sh.in
+++ b/contrib/bacman.sh.in
@@ -32,6 +32,7 @@ INCLUDE_PACNEW='n'
 ARGS=("$@")
 
 m4_include(../scripts/library/output_format.sh)
+m4_include(../scripts/library/parseopts.sh)
 
 # Lazy recursive clean up of temporary dirs
 work_dir_root="${TMPDIR:-/tmp}/bacman"
@@ -62,32 +63,51 @@ version() {
echo 'Copyright (C) 2008-2016 Pacman Development Team 
'
 }
 
-while [[ ! -z $1 ]]; do
-   if [[ $1 == "--nocolor" ]]; then
-   USE_COLOR='n'
-   shift
-   elif [[ $1 == "--pacnew" ]]; then
-   INCLUDE_PACNEW='y'
-   shift
-   else
-   break
-   fi
-done
-
+# Configure colored output
 m4_include(../scripts/library/term_colors.sh)
 
 # Break if no argument was given
-if (( $# < 1 )); then
+if (( $# == 0 )); then
+   usage
+   exit 1
+fi
+
+# Printing the usage information takes precedence over every other parameter
+for option in "$@"; do
+   [[ $option == "-h" || $option == "--help" ]] && usage && exit 0
+done
+
+# Parse arguments
+OPT_SHORT='mv'
+OPT_LONG=('nocolor' 'pacnew' 'version')
+if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then
usage
exit 1
 fi
+set -- "${OPTRET[@]}"
+unset OPT_SHORT OPT_LONG OPTRET
+
+while :; do
+   case "$1" in
+   -m|--nocolor)
+   USE_COLOR='n' ;;
+   --pacnew)
+   INCLUDE_PACNEW='y' ;;
+   -v|--version)
+   version
+   exit 0 ;;
+   --)
+   shift
+   break 2 ;;
+   esac
+   shift
+done
 
-if [[ $1 = -@(h|-help) ]]; then
+# Retrieve the list of packages to be assembled and break if none was specified
+pkg_list=($*)
+if [[ ${#pkg_list[@]} == 0 ]]; then
usage
-   exit 0
-elif [[ $1 = -@(V|-version) ]]; then
-   version
-   exit 0
+   exit 1
 fi
 
 #
@@ -373,9 +393,11 @@ parallelize() {
 
 # Initiate assembly function
 if [[ $MAX_JOBS -gt "1" ]]; then
-   parallelize "$@"
+   parallelize "${pkg_list[@]}"
 else
-   for PKG in $@; do fakebuild $PKG; done
+   for PKG in ${pkg_list[@]}; do
+   fakebuild $PKG
+   done
 fi
 msg "Done."
 
-- 
2.9.3


[pacman-dev] [PATCH v7 02/12] bacman: handle SIGHUP, SIGINT, SIGTERM signals

2016-09-03 Thread Gordian Edenhofer
Trap SIGHUP, SIGINT, SIGTERM and remove working directories accordingly.

Signed-off-by: Gordian Edenhofer 
---
 contrib/bacman.sh.in | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/contrib/bacman.sh.in b/contrib/bacman.sh.in
index 5e6fce5..e7a7f57 100644
--- a/contrib/bacman.sh.in
+++ b/contrib/bacman.sh.in
@@ -33,6 +33,16 @@ ARGS=("$@")
 
 m4_include(../scripts/library/output_format.sh)
 
+# Lazy recursive clean up of temporary dirs
+work_dir_root="${TMPDIR:-/tmp}/bacman"
+clean_up() {
+   rm -rf "$work_dir_root".*
+   echo
+   exit
+}
+# Trap termination signals
+trap clean_up SIGHUP SIGINT SIGTERM
+
 #
 # User Friendliness
 #
-- 
2.9.3


[pacman-dev] [PATCH v7 01/12] bacman: allow for multiple packages as arguments

2016-09-03 Thread Gordian Edenhofer
To enable the creation of multiple packages with one command move the
assembly process into its own function.

Signed-off-by: Gordian Edenhofer 
---
Split up first commit into three distinct parts:
Moving stuff into function, handle abort signals, write and alter some comments.
Merge the altered comments into the code structuring commit.

 contrib/bacman.sh.in | 405 +--
 1 file changed, 200 insertions(+), 205 deletions(-)

diff --git a/contrib/bacman.sh.in b/contrib/bacman.sh.in
index a611c1a..5e6fce5 100644
--- a/contrib/bacman.sh.in
+++ b/contrib/bacman.sh.in
@@ -66,7 +66,8 @@ done
 
 m4_include(../scripts/library/term_colors.sh)
 
-if (( $# != 1 )); then
+# Break if no argument was given
+if (( $# < 1 )); then
usage
exit 1
 fi
@@ -118,10 +119,6 @@ fi
 pkg_dest="${PKGDEST:-$PWD}"
 pkg_pkger=${PACKAGER:-'Unknown Packager'}
 
-pkg_name="$1"
-pkg_dir=("$pac_db/$pkg_name"-+([^-])-+([^-]))
-pkg_namver=("${pkg_dir[@]##*/}")
-
 #
 # Checks everything is in place
 #
@@ -130,227 +127,225 @@ if [[ ! -d $pac_db ]]; then
exit 1
 fi
 
-if (( ${#pkg_dir[@]} != 1 )); then
-   error "%d entries for package %s found in pacman database" \
-   ${#pkg_dir[@]} "${pkg_name}"
-   msg2 "%s" "${pkg_dir[@]}"
-   exit 1
-fi
-
-if [[ ! -d $pkg_dir ]]; then
-   error "package %s is found in pacman database," "${pkg_name}"
-   plain "   but '%s' is not a directory" "${pkg_dir}"
-   exit 1
-fi
-
-#
-# Begin
-#
-msg "Package: ${pkg_namver}"
-work_dir=$(mktemp -d "${TMPDIR:-/tmp}/bacman.XX")
-cd "$work_dir" || exit 1
-
-#
-# File copying
-#
-msg2 "Copying package files..."
-
-while read i; do
-   if [[ -z $i ]]; then
-   continue
+# Assemble a single package: $1 = pkgname
+fakebuild() {
+   pkg_name="$1"
+   pkg_dir=("$pac_db/$pkg_name"-+([^-])-+([^-]))
+   pkg_namver=("${pkg_dir[@]##*/}")
+
+   # Checks database for specified package
+   if (( ${#pkg_dir[@]} != 1 )); then
+   error "%d entries for package %s found in pacman database" \
+   ${#pkg_dir[@]} "${pkg_name}"
+   msg2 "%s" "${pkg_dir[@]}"
+   exit 1
fi
-
-   if [[ $i == %+([A-Z])% ]]; then
-   current=$i
-   continue
+   if [[ ! -d $pkg_dir ]]; then
+   error "package %s is found in pacman database," "${pkg_name}"
+   plain "   but '%s' is not a directory" "${pkg_dir}"
+   exit 1
fi
 
-   case "$current" in
-   %FILES%)
-   local_file="/$i"
-   package_file="$work_dir/$i"
+   # Create working directory
+   msg "Package: ${pkg_namver}"
+   work_dir=$(mktemp -d "${work_dir_root}.XX")
+   cd "$work_dir" || exit 1
 
-   if [[ ! -e $local_file ]]; then
-   warning "package file $local_file is missing"
-   continue
-   fi
-   ;;
-
-   %BACKUP%)
-   # Get the MD5 checksum.
-   original_md5="${i##*$'\t'}"
-   # Strip the md5sum after the tab.
-   i="${i%$'\t'*}"
-   local_file="/$i.pacnew"
-   package_file="$work_dir/$i"
-
-   # Include unmodified .pacnew files.
-   local_md5="$(md5sum "$local_file" | cut -d' ' -f1)"
-   if [[ $INCLUDE_PACNEW == 'n' ]] \
-   || [[ ! -e $local_file ]] \
-   || [[ $local_md5 != $original_md5 ]]; then
-   # Warn about modified files.
-   local_md5="$(md5sum "/$i" | cut -d' ' -f1)"
-   if [[ $local_md5 != $original_md5 ]]; then
-   warning "package file /$i has been 
modified"
-   fi
-   # Let the normal file be included in the 
%FILES% list.
-   continue
-   fi
-   ;;
+   # Assemble list of files which belong to the package and tar them
+   msg2 "Copying package files..."
+   while read i; do
+   if [[ -z $i ]]; then
+   continue
+   fi
 
-   *)
+   if [[ $i == %+([A-Z])% ]]; then
+   current=$i
continue
-   ;;
-   esac
+   fi
 
-   ret=0
-   bsdtar -cnf - -s'/.pacnew$//' "$local_file" 2> /dev/null | bsdtar -xpf 
- 2> /dev/null
+   case "$current" in
+   %FILES%)
+   local_file="/$i"
+   package_file="$work_dir/$i"
+
+   

Re: [pacman-dev] Downloader options - Was: Add configuration options for libcurl's "low speed" timeout

2016-09-03 Thread Olivier Brunel
On Sat, 3 Sep 2016 09:40:52 -0400
Dave Reisner  wrote:

> On Sat, Sep 03, 2016 at 10:45:42PM +1000, Allan McRae wrote:
> > On 30/08/16 22:48, Christian Hesse wrote:  
> > > Dave Reisner  on Tue, 2016/08/30 08:46:  
> > >> On Tue, Aug 30, 2016 at 02:12:23PM +0200, Christian Hesse
> > >> wrote:  
> > >>> From: Christian Hesse 
> > >>>
> > >>> Add LowSpeedLimit and LowSpeedTime configuration options to
> > >>> correspond to libcurl's CURLOPT_LOW_SPEED_LIMIT and
> > >>> CURLOPT_LOW_SPEED_TIME options. This allows, e.g., transfers
> > >>> behind corporate virus-scanning firewalls to survive the
> > >>> delays. Increasing the timeout may not be desirable in all
> > >>> situations; similarly, disabling the check prevents detection
> > >>> of disappearing networks.
> > >>
> > >> FWIW, I'm strongly opposed to having a 1:1 mapping between pacman
> > >> options and curl config options. Please look at the bigger
> > >> picture -- it's dead connection detection. We might reimplement
> > >> that in the future in some other way (e.g. via the progress
> > >> callback or by some other transfer library entirely).
> > >>
> > >> To that end, I think it would be reasonable to add a boolean
> > >> toggle for the dead connection detection (default on). The patch
> > >> in its current state makes me rather itchy from an API
> > >> perspective.  
> > > 
> > > That is what my stupid-proxy patch does...
> > > Now it is up to Allan to decide. ;)  
> > 
> > Crap...  Why do I need to make decisions?
> > 
> > OK - lets think on the go here...  The two options we "want" to
> > support based on submitted patches are:
> > 1) Disabling the low speed timeout
> > 2) Setting maximum download speed
> > 
> > In the future, we might also add a parallel download option.
> > 
> > I'm guessing these want both global config values and command line
> > options.  Anyone want to suggest what these could be.  I'll choose
> > the colour of the bikeshed.
> > 
> > Allan  
> 
> 1) DetectDeadConnections
> 2) MaxDownloadKBps
> future) MaxConcurrentDownloads
> 
> For #2, I'll suggest that you can already do this with a program like
> netbrake. Curl's internal rate limiting is not good and only uses a
> flat average, rather than a rolling window of recent samples. It's

FWIW I've actually submitted a patch[1] recently to improve curl's
limiting algo that should improve this. (Of course that'll only apply
to users with a quite recent libcurl.)

In my patch[2] (for pacman this time) I used MaxDlSpeed as name so one
can specify a unit, whereas MaxDownloadKBps implies one (though likely
the most common one I suppose).

Using "Dl" instead of "Download" in the option name seemed like still
pretty common/understandable, all the while making things a bit
shorter, especially nice for the command line option.

With similar intent, maybe a shorter one for #1 might be better... e.g.
NoLowSpeedTimeout, or simply NoSlowTimeout ?
(Also I guess yours should be NoDetectDeadConnections or
similar, as the option is actually meant to *disable* the low speed
timeout.)

> idea of "limiting" bandwidth is inserting an artificial delay before
> the next recv call, which might just be draining a buffer of already
> received data from the kernel. This is particularly true if you're
> behind a router and not the actual gateway device (and I suspect this
> is the common case).
> 
> d

[1] https://github.com/curl/curl/pull/971
[2]
https://lists.archlinux.org/pipermail/pacman-dev/2016-August/021265.html


Re: [pacman-dev] Downloader options - Was: Add configuration options for libcurl's "low speed" timeout

2016-09-03 Thread Dave Reisner
On Sat, Sep 03, 2016 at 10:45:42PM +1000, Allan McRae wrote:
> On 30/08/16 22:48, Christian Hesse wrote:
> > Dave Reisner  on Tue, 2016/08/30 08:46:
> >> On Tue, Aug 30, 2016 at 02:12:23PM +0200, Christian Hesse wrote:
> >>> From: Christian Hesse 
> >>>
> >>> Add LowSpeedLimit and LowSpeedTime configuration options to correspond
> >>> to libcurl's CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME options.
> >>> This allows, e.g., transfers behind corporate virus-scanning firewalls
> >>> to survive the delays. Increasing the timeout may not be desirable in
> >>> all situations; similarly, disabling the check prevents detection of
> >>> disappearing networks.  
> >>
> >> FWIW, I'm strongly opposed to having a 1:1 mapping between pacman
> >> options and curl config options. Please look at the bigger picture --
> >> it's dead connection detection. We might reimplement that in the future
> >> in some other way (e.g. via the progress callback or by some other
> >> transfer library entirely).
> >>
> >> To that end, I think it would be reasonable to add a boolean toggle for
> >> the dead connection detection (default on). The patch in its current
> >> state makes me rather itchy from an API perspective.
> > 
> > That is what my stupid-proxy patch does...
> > Now it is up to Allan to decide. ;)
> 
> Crap...  Why do I need to make decisions?
> 
> OK - lets think on the go here...  The two options we "want" to support
> based on submitted patches are:
> 1) Disabling the low speed timeout
> 2) Setting maximum download speed
> 
> In the future, we might also add a parallel download option.
> 
> I'm guessing these want both global config values and command line
> options.  Anyone want to suggest what these could be.  I'll choose the
> colour of the bikeshed.
> 
> Allan

1) DetectDeadConnections
2) MaxDownloadKBps
future) MaxConcurrentDownloads

For #2, I'll suggest that you can already do this with a program like
netbrake. Curl's internal rate limiting is not good and only uses a flat
average, rather than a rolling window of recent samples. It's idea of
"limiting" bandwidth is inserting an artificial delay before the next
recv call, which might just be draining a buffer of already received
data from the kernel. This is particularly true if you're behind a
router and not the actual gateway device (and I suspect this is the
common case).

d


[pacman-dev] Downloader options - Was: Add configuration options for libcurl's "low speed" timeout

2016-09-03 Thread Allan McRae
On 30/08/16 22:48, Christian Hesse wrote:
> Dave Reisner  on Tue, 2016/08/30 08:46:
>> On Tue, Aug 30, 2016 at 02:12:23PM +0200, Christian Hesse wrote:
>>> From: Christian Hesse 
>>>
>>> Add LowSpeedLimit and LowSpeedTime configuration options to correspond
>>> to libcurl's CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME options.
>>> This allows, e.g., transfers behind corporate virus-scanning firewalls
>>> to survive the delays. Increasing the timeout may not be desirable in
>>> all situations; similarly, disabling the check prevents detection of
>>> disappearing networks.  
>>
>> FWIW, I'm strongly opposed to having a 1:1 mapping between pacman
>> options and curl config options. Please look at the bigger picture --
>> it's dead connection detection. We might reimplement that in the future
>> in some other way (e.g. via the progress callback or by some other
>> transfer library entirely).
>>
>> To that end, I think it would be reasonable to add a boolean toggle for
>> the dead connection detection (default on). The patch in its current
>> state makes me rather itchy from an API perspective.
> 
> That is what my stupid-proxy patch does...
> Now it is up to Allan to decide. ;)

Crap...  Why do I need to make decisions?

OK - lets think on the go here...  The two options we "want" to support
based on submitted patches are:
1) Disabling the low speed timeout
2) Setting maximum download speed

In the future, we might also add a parallel download option.

I'm guessing these want both global config values and command line
options.  Anyone want to suggest what these could be.  I'll choose the
colour of the bikeshed.

Allan


Re: [pacman-dev] [PATCH v6 2/9] bacman: parallel packaging

2016-09-03 Thread Allan McRae
On 03/09/16 00:50, Gordian Edenhofer wrote:
> Signed-off-by: Gordian Edenhofer 
> ---

To be completely explicit, I am not going to accept parallel packaging
patches (2, 7, and 9 in patchset v6).  There are many other tools
available to handle running scripts in parallel, and the boilerplate
overhead in bacman is minimal.  This is not a feature that we should be
providing.

Allan


Re: [pacman-dev] [PATCH v6 5/9] bacman: add option to print fewer status updates

2016-09-03 Thread Allan McRae
On 03/09/16 00:50, Gordian Edenhofer wrote:
> Signed-off-by: Gordian Edenhofer 
> ---
>  contrib/bacman.sh.in | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/contrib/bacman.sh.in b/contrib/bacman.sh.in
> index f5807b5..a567e7d 100644
> --- a/contrib/bacman.sh.in
> +++ b/contrib/bacman.sh.in
> @@ -28,6 +28,7 @@ declare -r myname='bacman'
>  declare -r myver='@PACKAGE_VERSION@'
>  USE_COLOR='y'
>  INCLUDE_PACNEW='n'
> +QUIET=0
>  # Required for fakeroot because options are shifted off the array.
>  ARGS=("$@")
>  
> @@ -54,6 +55,7 @@ usage() {
>   echo
>   printf -- "$(gettext "Options:")\n"
>   printf -- "$(gettext "  -h, --help   Show this help message and 
> exit")\n"
> + printf -- "$(gettext "  -q, --quiet  Silence most of the status 
> reporting")\n"
>   printf -- "$(gettext "  -m, --nocolorDisable colorized output 
> messages")\n"
>   printf -- "$(gettext "  --pacnew Package .pacnew files")\n"
>   echo
> @@ -85,8 +87,8 @@ for option in "$@"; do
>  done
>  
>  # Parse arguments
> -OPT_SHORT='mv'
> -OPT_LONG=('nocolor' 'pacnew' 'version')
> +OPT_SHORT='qmv'
> +OPT_LONG=('quiet' 'nocolor' 'pacnew' 'version')
>  if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then
>   usage
>   exit 1
> @@ -96,6 +98,8 @@ unset OPT_SHORT OPT_LONG OPTRET
>  
>  while :; do
>   case "$1" in
> + -q|--quiet)
> + QUIET=1 ;;
>   -m|--nocolor)
>   USE_COLOR='n' ;;
>   --pacnew)
> @@ -180,6 +184,7 @@ fakebuild() {
>  
>   # Assemble list of files which belong to the package and tar them
>   msg2 "Copying package files..."
> +

Unrelated change

>   while read i; do
>   if [[ -z $i ]]; then
>   continue
> 


Re: [pacman-dev] [PATCH] bash-completion: fix leaking "files" array into shell environment

2016-09-03 Thread Allan McRae
On 02/09/16 13:40, Eli Schwartz wrote:
> Signed-off-by: Eli Schwartz 
> ---


Thanks,
Allan


Re: [pacman-dev] [PATCH v5 4/8] bacman: rewrite usage page and version information

2016-09-03 Thread Allan McRae
On 03/09/16 01:07, Gordian Edenhofer wrote:
> On Fri, 2016-09-02 at 23:37 +1000, Allan McRae wrote:
>> On 02/09/16 22:51, Gordian Edenhofer wrote:
>>>
>>> echo 'Copyright (C) 2008 locci
>>> '
>>> +   echo 'Copyright (C) 2016 Gordian Edenhofer >> f...@gmail.com>'
>>> echo 'Copyright (C) 2008-2016 Pacman Development Team >> man-...@archlinux.org>'
>>
>>
>> You want your name included beyond the "Pacman Development
>> Team".  There
>> are currently 11 other contributors to that script beyond the
>> original
>> author (who has their own copyright line).
> 
> I did not intended to insult anyone or diminish other contributions.
> I just though of the patch as a significant revamp considering the
> patch allows for multiple packages, handles abort signals, added
> various options and introduced a man page.
> Since apparently this seems to offend you, I removed my name.

I wasn't being insulted.  My email was more intended to point out the
usual process that is followed in this code base.  I was probably
blunter than intended as I was rushing through patch review late at night.

Allan


Re: [pacman-dev] [RFC, PATCHES]: Very preliminary work towards clean build with clang -Weverything

2016-09-03 Thread Allan McRae
On 03/09/16 12:08, ivy.fos...@gmail.com wrote:
> First of all, don't worry; I'm under no illusion that the
> pie-in-the-sky Subject line is anything that will happen in the near
> future, nor even necessarily a goal for the project (given just how
> finicky -Weverything is).
> 
> Basically, I'm wondering whether sporadic patches addressing the sorts
> of things "clang -Weverything" gripes about would be considered
> welcome contributions, or nitpicking. By "sporadic patches", I mean
> that any of this sort from me would come when I felt like working on a
> coding project but didn't have anything particularly in mind.
> 
> As a sample, I've made two patches addressing some of its earliest
> warnings. The first changes several headers to (say) #define
> IDENTIFIERS rather than _IDENTIFIERS. The second adds ALPM_ERR_OK = 0
> to the _alpm_err_t enum. Some functions which return an _alpm_err_t
> return value were returning 0 on success, but there was no 0 value
> enumerated--not actively harmful, but still an oddity.
> 
> If this sort of thing isn't of interest, I definitely get it. I just
> figured that an RFC with patches attached would provide something more
> concrete to discuss than one without.

I am happy to accept these.  When you fix a specific set of warnings
from clang/gcc, you can add that specific warning flag (i.e. not
-Weverything...) to our configure.ac at this section:

# Enable or disable compiler warning flags
AC_MSG_CHECKING(for excessive compiler warning flags)
if test "x$warningflags" = "xyes" ; then
AC_MSG_RESULT(yes)
CFLAGS_ADD([-Wcast-align], [WARNING_CFLAGS])
CFLAGS_ADD([-Wclobbered], [WARNING_CFLAGS])
...

That way we will not regress in the future.

Thanks,
Allan


[pacman-dev] [PATCH] Fix memory leak in remove_notify_needed_optdepends

2016-09-03 Thread Allan McRae
Also add pactest which captures this leak when run under valgrind.

Reported-by: Sergey Petrenko
Signed-off-by: Allan McRae 
---
 lib/libalpm/remove.c  |  1 +
 test/pacman/tests/TESTS   |  1 +
 .../pacman/tests/remove-optdepend-of-installed-package.py | 15 +++
 3 files changed, 17 insertions(+)
 create mode 100644 test/pacman/tests/remove-optdepend-of-installed-package.py

diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c
index 45f7c2f..173dbc6 100644
--- a/lib/libalpm/remove.c
+++ b/lib/libalpm/remove.c
@@ -189,6 +189,7 @@ static void remove_notify_needed_optdepends(alpm_handle_t 
*handle, alpm_list_t *
};
EVENT(handle, );
}
+   free(optstring);
}
}
}
diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS
index bd5a0b6..2d87796 100644
--- a/test/pacman/tests/TESTS
+++ b/test/pacman/tests/TESTS
@@ -109,6 +109,7 @@ TESTS += test/pacman/tests/querycheck002.py
 TESTS += test/pacman/tests/querycheck_fast_file_type.py
 TESTS += test/pacman/tests/reason001.py
 TESTS += test/pacman/tests/remove-assumeinstalled.py
+TESTS += test/pacman/tests/remove-optdepend-of-installed-package.py
 TESTS += test/pacman/tests/remove-recursive-cycle.py
 TESTS += test/pacman/tests/remove001.py
 TESTS += test/pacman/tests/remove002.py
diff --git a/test/pacman/tests/remove-optdepend-of-installed-package.py 
b/test/pacman/tests/remove-optdepend-of-installed-package.py
new file mode 100644
index 000..4973df5
--- /dev/null
+++ b/test/pacman/tests/remove-optdepend-of-installed-package.py
@@ -0,0 +1,15 @@
+self.description = "Remove packages which is an optdepend of another package"
+
+p1 = pmpkg("dep")
+self.addpkg2db("local", p1)
+
+p2 = pmpkg("pkg")
+p2.optdepends = ["dep: for foobar"]
+self.addpkg2db("local", p2)
+
+self.args = "-R %s" % p1.name
+
+self.addrule("PACMAN_RETCODE=0")
+self.addrule("!PKG_EXIST=%s" % p1.name)
+self.addrule("PKG_EXIST=%s" % p2.name)
+self.addrule("PACMAN_OUTPUT=%s optionally requires %s" % (p2.name, p1.name))
-- 
2.9.3


Re: [pacman-dev] Memleak

2016-09-03 Thread Allan McRae
On 03/09/16 19:26, Sergey Petrenko via pacman-dev wrote:
> libalpm/remove.c:
>  
> in remove_notify_needed_optdepends():
>  
> char *optstring = alpm_dep_compute_string(optdep);
> 
> isn't freed after being used.
> 
> Ought to thank Valgrind for this one.
> 

Thanks.  We did not cover this path in our pactest suite.  Adding this test:

diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS
index bd5a0b6..2d87796 100644
--- a/test/pacman/tests/TESTS
+++ b/test/pacman/tests/TESTS
@@ -109,6 +109,7 @@ TESTS += test/pacman/tests/querycheck002.py
 TESTS += test/pacman/tests/querycheck_fast_file_type.py
 TESTS += test/pacman/tests/reason001.py
 TESTS += test/pacman/tests/remove-assumeinstalled.py
+TESTS += test/pacman/tests/remove-optdepend-of-installed-package.py
 TESTS += test/pacman/tests/remove-recursive-cycle.py
 TESTS += test/pacman/tests/remove001.py
 TESTS += test/pacman/tests/remove002.py
diff --git a/test/pacman/tests/remove-optdepend-of-installed-package.py
b/test/pacman/tests/remove-optdepend-of-installed-package.py
new file mode 100644
index 000..4973df5
--- /dev/null
+++ b/test/pacman/tests/remove-optdepend-of-installed-package.py
@@ -0,0 +1,15 @@
+self.description = "Remove packages which is an optdepend of another
package"
+
+p1 = pmpkg("dep")
+self.addpkg2db("local", p1)
+
+p2 = pmpkg("pkg")
+p2.optdepends = ["dep: for foobar"]
+self.addpkg2db("local", p2)
+
+self.args = "-R %s" % p1.name
+
+self.addrule("PACMAN_RETCODE=0")
+self.addrule("!PKG_EXIST=%s" % p1.name)
+self.addrule("PKG_EXIST=%s" % p2.name)
+self.addrule("PACMAN_OUTPUT=%s optionally requires %s" % (p2.name,
p1.name))
-- 
2.9.3


Re: [pacman-dev] [RFC, PATCHES]: Very preliminary work towards clean build with clang -Weverything

2016-09-03 Thread Martin Kühne
All identifiers that begin with an underscore and either an uppercase
letter or another underscore are always reserved for any use.
All identifiers that begin with an underscore are always reserved for
use as identifiers with file scope in both the ordinary and tag
name spaces. (ISO 9899:1999, 7.1.3) Don't use such identifiers.

As a matter of fact, typedefs, struct names and function names fall
under this terminology. Despite the fact that the compiler accepts
leading underscores, per standard they are explicitly discouraged.
So, what we should consider is to move _alpm_errno_t along with all
other names in the source tree away from leading underscores. Their
visibility should be controlled by the presence of a static keyword,
-fvisibility and such instead.

You might want to write a sed script for this, and it looks like a
major invasion into the codebase. The last word is with the
developers, though, hence I'm not sure this is going to be accepted
for traditional reasons.

cheers!
mar77i