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

2018-10-18 Thread 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


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

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

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

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

No need to break lines under 80 characters.

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

No space between if and (.

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


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

2018-10-18 Thread morganamilo
Currently when attempting to sync a group where all packages are
ignored (either by ignorepkg, ignoregroup or --needed) pacman
will error with "target not found".

Instead, if a group has no packages, check if the group exists
and only throw an error if it does not.

Signed-off-by: morganamilo 

diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 2d3d198a..316853bb 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -1446,6 +1446,8 @@ int alpm_extract_keyid(alpm_handle_t *handle, const char 
*identifier,
 
 alpm_list_t *alpm_find_group_pkgs(alpm_list_t *dbs, const char *name);
 
+int alpm_group_exists(alpm_list_t *dbs, const char *name);
+
 /*
  * Sync
  */
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index 05f58fad..57058782 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -313,6 +313,26 @@ alpm_list_t SYMEXPORT *alpm_find_group_pkgs(alpm_list_t 
*dbs,
return pkgs;
 }
 
+/** Check if a group exists across a list of databases.
+ * @param dbs the list of alpm_db_t *
+ * @param name the name of the group
+ * @return 1 if the group exists, 0 if it does not
+ */
+int SYMEXPORT alpm_group_exists(alpm_list_t *dbs,
+   const char *name)
+{
+   alpm_list_t *i;
+   for(i = dbs; i; i = i->next) {
+   alpm_db_t *db = i->data;
+
+   if (alpm_db_get_group(db, name)) {
+   return 1;
+   }
+   }
+
+   return 0;
+}
+
 /** Compute the size of the files that will be downloaded to install a
  * package.
  * @param newpkg the new package to upgrade to
diff --git a/src/pacman/sync.c b/src/pacman/sync.c
index ef8faedf..32df6e04 100644
--- a/src/pacman/sync.c
+++ b/src/pacman/sync.c
@@ -543,6 +543,10 @@ static int process_group(alpm_list_t *dbs, const char 
*group, int error)
int count = alpm_list_count(pkgs);
 
if(!count) {
+   if(alpm_group_exists(dbs, group)) {
+   return 0;
+   }
+
pm_printf(ALPM_LOG_ERROR, _("target not found: %s\n"), group);
return 1;
}
-- 
2.19.1


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

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

I did say the patch set was incomplete ;)
group_exists does seem like the best idea though so will do. Would you
prefer that function be added to the backend or frontend?