diff mbox series

[pyalpm] Rename function to match renamed name in pacman master

Message ID 20190118145257.26900-1-eschwartz@archlinux.org
State New
Headers show
Series [pyalpm] Rename function to match renamed name in pacman master | expand

Commit Message

Emil Velikov via arch-projects Jan. 18, 2019, 2:52 p.m. UTC 
alpm_sync_newversion becomes alpm_sync_get_new_version and changes
behavior slightly. See for details:
https://git.archlinux.org/pacman.git/commit/?id=e9d91a688d1a2ebe58e8a895853debf745a529cf
---

DO NOT MERGE THIS until after pacman 5.2 or 6 is released.

pacman does not have any good preprocessor check for this -- the only
way to handle both methods would be to implement some sort of test in
setup.py and create a DEFINE based on that.

Posted here in order to be prepared, though I wouldn't mind having it
exist in the pyalpm repo in a "pacman-git" branch. ;)

 src/db.c     | 4 ++--
 src/db.h     | 2 +-
 src/pyalpm.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

Comments

Emil Velikov via arch-projects Jan. 18, 2019, 3:16 p.m. UTC | #1 
On Fri, 18 Jan 2019 at 14:53, Eli Schwartz via arch-projects
<arch-projects@archlinux.org> wrote:
>
> alpm_sync_newversion becomes alpm_sync_get_new_version and changes
> behavior slightly. See for details:
> https://git.archlinux.org/pacman.git/commit/?id=e9d91a688d1a2ebe58e8a895853debf745a529cf
> ---
>
> DO NOT MERGE THIS until after pacman 5.2 or 6 is released.
>
> pacman does not have any good preprocessor check for this -- the only
> way to handle both methods would be to implement some sort of test in
> setup.py and create a DEFINE based on that.
>
> Posted here in order to be prepared, though I wouldn't mind having it
> exist in the pyalpm repo in a "pacman-git" branch. ;)
>
>  src/db.c     | 4 ++--
>  src/db.h     | 2 +-
>  src/pyalpm.c | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/db.c b/src/db.c
> index 3015d61..f6c5da4 100644
> --- a/src/db.c
> +++ b/src/db.c
> @@ -304,7 +304,7 @@ PyObject* pyalpm_find_grp_pkgs(PyObject* self, PyObject *args) {
>  }
>
>  /** Finds an available upgrade for a package in a list of databases */
> -PyObject* pyalpm_sync_newversion(PyObject *self, PyObject* args) {
> +PyObject* pyalpm_sync_get_new_version(PyObject *self, PyObject* args) {
>    PyObject *pkg;
>    PyObject *dbs;
>    alpm_list_t *db_list;
> @@ -320,7 +320,7 @@ PyObject* pyalpm_sync_newversion(PyObject *self, PyObject* args) {
>    {
>      alpm_pkg_t *rawpkg = pmpkg_from_pyalpm_pkg(pkg);
>      if (rawpkg) {
> -      result = alpm_sync_newversion(rawpkg, db_list);
> +      result = alpm_sync_get_new_version(rawpkg, db_list);
>      }
>      alpm_list_free(db_list);
>    }
> diff --git a/src/db.h b/src/db.h
> index 4ed02e6..be35b78 100644
> --- a/src/db.h
> +++ b/src/db.h
> @@ -29,6 +29,6 @@ PyObject *pyalpm_db_from_pmdb(void* data);
>  int pylist_db_to_alpmlist(PyObject *list, alpm_list_t **result);
>
>  PyObject* pyalpm_find_grp_pkgs(PyObject* self, PyObject* args);
> -PyObject* pyalpm_sync_newversion(PyObject *self, PyObject* args);
> +PyObject* pyalpm_sync_get_new_version(PyObject *self, PyObject* args);
>
>  #endif
> diff --git a/src/pyalpm.c b/src/pyalpm.c
> index 56c9639..7391191 100644
> --- a/src/pyalpm.c
> +++ b/src/pyalpm.c
> @@ -87,7 +87,7 @@ static PyMethodDef methods[] = {
>      "args: a list of packages, a dependency string\n"
>      "returns: a Package object or None" },
>
> -  {"sync_newversion", pyalpm_sync_newversion, METH_VARARGS,
> +  {"sync_newversion", pyalpm_sync_get_new_version, METH_VARARGS,
>      "finds an available upgrade for a package in a list of databases\n"
>      "args: a package, a list of databases\n"
>      "returns: an upgrade candidate or None" },
> --
> 2.20.1

The function name changed to highlight a breaking change in behaviour.
Would it therefore make sense to also rename the function on the python side?
Emil Velikov via arch-projects Jan. 18, 2019, 4:55 p.m. UTC | #2 
On 1/18/19 10:16 AM, Morgan Adamiec via arch-projects wrote:
> On Fri, 18 Jan 2019 at 14:53, Eli Schwartz via arch-projects
> <arch-projects@archlinux.org> wrote:
>>
>> alpm_sync_newversion becomes alpm_sync_get_new_version and changes
>> behavior slightly. See for details:
>> https://git.archlinux.org/pacman.git/commit/?id=e9d91a688d1a2ebe58e8a895853debf745a529cf
>> ---
>>
>> DO NOT MERGE THIS until after pacman 5.2 or 6 is released.
>>
>> pacman does not have any good preprocessor check for this -- the only
>> way to handle both methods would be to implement some sort of test in
>> setup.py and create a DEFINE based on that.
>>
>> Posted here in order to be prepared, though I wouldn't mind having it
>> exist in the pyalpm repo in a "pacman-git" branch. ;)
>>
>>  src/db.c     | 4 ++--
>>  src/db.h     | 2 +-
>>  src/pyalpm.c | 2 +-
>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/db.c b/src/db.c
>> index 3015d61..f6c5da4 100644
>> --- a/src/db.c
>> +++ b/src/db.c
>> @@ -304,7 +304,7 @@ PyObject* pyalpm_find_grp_pkgs(PyObject* self, PyObject *args) {
>>  }
>>
>>  /** Finds an available upgrade for a package in a list of databases */
>> -PyObject* pyalpm_sync_newversion(PyObject *self, PyObject* args) {
>> +PyObject* pyalpm_sync_get_new_version(PyObject *self, PyObject* args) {
>>    PyObject *pkg;
>>    PyObject *dbs;
>>    alpm_list_t *db_list;
>> @@ -320,7 +320,7 @@ PyObject* pyalpm_sync_newversion(PyObject *self, PyObject* args) {
>>    {
>>      alpm_pkg_t *rawpkg = pmpkg_from_pyalpm_pkg(pkg);
>>      if (rawpkg) {
>> -      result = alpm_sync_newversion(rawpkg, db_list);
>> +      result = alpm_sync_get_new_version(rawpkg, db_list);
>>      }
>>      alpm_list_free(db_list);
>>    }
>> diff --git a/src/db.h b/src/db.h
>> index 4ed02e6..be35b78 100644
>> --- a/src/db.h
>> +++ b/src/db.h
>> @@ -29,6 +29,6 @@ PyObject *pyalpm_db_from_pmdb(void* data);
>>  int pylist_db_to_alpmlist(PyObject *list, alpm_list_t **result);
>>
>>  PyObject* pyalpm_find_grp_pkgs(PyObject* self, PyObject* args);
>> -PyObject* pyalpm_sync_newversion(PyObject *self, PyObject* args);
>> +PyObject* pyalpm_sync_get_new_version(PyObject *self, PyObject* args);
>>
>>  #endif
>> diff --git a/src/pyalpm.c b/src/pyalpm.c
>> index 56c9639..7391191 100644
>> --- a/src/pyalpm.c
>> +++ b/src/pyalpm.c
>> @@ -87,7 +87,7 @@ static PyMethodDef methods[] = {
>>      "args: a list of packages, a dependency string\n"
>>      "returns: a Package object or None" },
>>
>> -  {"sync_newversion", pyalpm_sync_newversion, METH_VARARGS,
>> +  {"sync_newversion", pyalpm_sync_get_new_version, METH_VARARGS,
>>      "finds an available upgrade for a package in a list of databases\n"
>>      "args: a package, a list of databases\n"
>>      "returns: an upgrade candidate or None" },
>> --
>> 2.20.1
> 
> The function name changed to highlight a breaking change in behaviour.
> Would it therefore make sense to also rename the function on the python side?
In libalpm, the function alpm_sync_newversion() is/was documented as
"Check for new version of pkg in sync repos"

In pyalpm, the function sync_newversion() is documented as "finds an
available upgrade for a package in a list of databases".

So I made the rationale that pyalpm users would expect to be passing the
right syncdbs.

Worth noting: pyalpm only really makes use of this function in
pycman-query, which does *not* filter the list of repos, but also does
not print [ignored] even for ignored packages from active repos.

OTOH, attempting to iterate over a PacmanConfig instance leads me to the
somewhat surprising conclusion that after commit
f445d66e34846fab28d773de2c57314fbb76cf77 which implemented the Usage
option, it seems that pacman_conf_enumerator shows this value only for
PacmanConfig to throw it away. This might explain why a code search on
Github shows only people using get_syncdbs() as a parameter to
sync_newversion. There's currently no sane way to handle this at all,
with or without my patch, regardless of pacman or pyalpm version.

This will require considerable thought and many changes.

However, renaming the function is a free move, since pyalpm does not
support filtering the database under any conditions, basically, and in
order to do it, you need to reimplement the PacmanConfig class which is
nontrivial. The API for this is crippled enough that it might as well be
an implementation detail.

Going forward,

alpm.DB needs to get new usage/siglevel attributes, and/or the
PacmanConfig().repos dict needs to be completely reimplemented by
exposing members which instead of ('name', ['server']) contain ('name',
{'servers': ['server']}) and then further expanded with additional options.

This will allow pyalpm consumers to, for the first time, get the full
range of options relevant to libalpm.
diff mbox series

Patch

diff --git a/src/db.c b/src/db.c
index 3015d61..f6c5da4 100644
--- a/src/db.c
+++ b/src/db.c
@@ -304,7 +304,7 @@  PyObject* pyalpm_find_grp_pkgs(PyObject* self, PyObject *args) {
 }
 
 /** Finds an available upgrade for a package in a list of databases */
-PyObject* pyalpm_sync_newversion(PyObject *self, PyObject* args) {
+PyObject* pyalpm_sync_get_new_version(PyObject *self, PyObject* args) {
   PyObject *pkg;
   PyObject *dbs;
   alpm_list_t *db_list;
@@ -320,7 +320,7 @@  PyObject* pyalpm_sync_newversion(PyObject *self, PyObject* args) {
   {
     alpm_pkg_t *rawpkg = pmpkg_from_pyalpm_pkg(pkg);
     if (rawpkg) {
-      result = alpm_sync_newversion(rawpkg, db_list);
+      result = alpm_sync_get_new_version(rawpkg, db_list);
     }
     alpm_list_free(db_list);
   }
diff --git a/src/db.h b/src/db.h
index 4ed02e6..be35b78 100644
--- a/src/db.h
+++ b/src/db.h
@@ -29,6 +29,6 @@  PyObject *pyalpm_db_from_pmdb(void* data);
 int pylist_db_to_alpmlist(PyObject *list, alpm_list_t **result);
 
 PyObject* pyalpm_find_grp_pkgs(PyObject* self, PyObject* args);
-PyObject* pyalpm_sync_newversion(PyObject *self, PyObject* args);
+PyObject* pyalpm_sync_get_new_version(PyObject *self, PyObject* args);
 
 #endif
diff --git a/src/pyalpm.c b/src/pyalpm.c
index 56c9639..7391191 100644
--- a/src/pyalpm.c
+++ b/src/pyalpm.c
@@ -87,7 +87,7 @@  static PyMethodDef methods[] = {
     "args: a list of packages, a dependency string\n"
     "returns: a Package object or None" },
 
-  {"sync_newversion", pyalpm_sync_newversion, METH_VARARGS,
+  {"sync_newversion", pyalpm_sync_get_new_version, METH_VARARGS,
     "finds an available upgrade for a package in a list of databases\n"
     "args: a package, a list of databases\n"
     "returns: an upgrade candidate or None" },