Re: [pacman-dev] Versioned packages

2016-09-22 Thread Eli Schwartz
On 09/22/2016 03:53 AM, Gordian Edenhofer wrote:
> I wouldn't assume that the user has done anything wrong, rather
> something with the system is wrong if one feels the need to 'backup'
> packages.

Awesome, we must be in agreement, because I don't think people should
feel the need to backup package*s* either.

> Since one is intending to backup a package one should be highly aware
> of the purpose of the package.

What does that have to do with anything???

> The provide field makes sure that it is still treated the same. But to
> allow backups for all packages, the user has to live with them not
> being signed-off.

Okay. What about reinstalling this mysterious ghost-like package?
And when, precisely, did you decide that "backups for all packages" was
a desirable feature, rather than a highly kludgy workaround for the edge
case of the linux kernel?

> I dislike dummy packages and therefore would keep the current way of
> creating e.g. the linux package.
> If using dummy packages the main question becomes where to draw the
> line. You are currently referring to linux as "for that specific
> package" but what if users need different versions for another
> package... The system of everyone else would become more and more
> cluttered with dummy packages.

To add to my previous objections: Arch Linux doesn't believe in running
old software, and the linux is a unique exception whereby we don't want
to delete a currently-running kernel, the solution to which is not
"let's modify pacman".


Maybe the source of this confusion is, you should read the bugtracker to
see why this thread was initially proposed, and then decide if backing
up package*s* is a good solution.
Rather than presupposing it is a wanted feature. (feature creep)
https://bugs.archlinux.org/task/16702

-- 
Eli Schwartz


Re: [pacman-dev] Versioned packages

2016-09-22 Thread Gordian Edenhofer
On Wed, 2016-09-21 at 23:28 -0400, Eli Schwartz wrote:
> On 09/21/2016 05:56 AM, Gordian Edenhofer wrote:
> > 
> > You bar for insanity is whether the debian folks do it? Please
> > explain
> > yourself why do you think this is the wrong approach for the
> > problem.
> > My reasoning would be that packaging a dummy package would result
> > in
> > the same 'renamed' package on the system but would require the
> > package
> > maintainer to take action.
> 
> Well, I can usually count on them to be over-engineered.
> 
> But in all seriousness, then. :)
> Building in to the package manager, a capacity for retroactively
> modifying the definition of a specific package, seems to me to be a
> clear case of over-engineering. And just a bad idea in principle.
> 
> If you want to do that, it is because you are already doing
> *something*
> wrong.

I wouldn't assume that the user has done anything wrong, rather
something with the system is wrong if one feels the need to 'backup'
packages.

> Because, you should not have to change your mind about what these
> particular files actually are, and start calling them something else
> on

Since one is intending to backup a package one should be highly aware
of the purpose of the package.

> a whim. Particularly as it disappears from the repositories, stops
> being
> trusted as signed by a central authority (the TUs), cannot be
> reinstalled...

The provide field makes sure that it is still treated the same. But to
allow backups for all packages, the user has to live with them not
being signed-off.

> > Btw. my Arch system is rock solid and I am not especially
> > interested in
> > dummy packages on my system but for those who need it it can be a
> > huge
> > difference. Therefore it makes sense to give those specific user
> > base
> > the ability to do so on their own. It wouldn't even be a dangerous
> > operation since by adding a provide field one should be unable to
> > brick
> > the system.
> > However I admit that it would require some additions to pacman
> > which is
> > obviously an argument against this but the way packages are treated
> > would stay the same.
> > Currently I don't see a reason why one should change the way of
> > packaging just to circumvent the underlying problem.
> 
> I am a bit confused as to what this all means...
> 
> Do you prefer dummy packages which pull in versioned kernels and
> depend
> on changing the packaging (for that specific package)?
> Or do you prefer additions to pacman while "treating the packages the
> same"?

I dislike dummy packages and therefore would keep the current way of
creating e.g. the linux package.
If using dummy packages the main question becomes where to draw the
line. You are currently referring to linux as "for that specific
package" but what if users need different versions for another
package... The system of everyone else would become more and more
cluttered with dummy packages.

> I think we may have a drastically different idea of what constitutes
> "circumventing the underlying problem".
> To be specific, we both seem to think the other is the one
> circumventing
> the underlying problem.

That's true :D

signature.asc
Description: This is a digitally signed message part


[pacman-dev] [PATCH 2/2] Change type of count in be_sync

2016-09-22 Thread Rikard Falkeborn
Making it size_t matches the return value of alpm_list_count() and
avoids the implicit cast to int.

Signed-off-by: Rikard Falkeborn 
---
 lib/libalpm/be_sync.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index f127ed8..079db19 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -452,8 +452,8 @@ static size_t estimate_package_count(struct stat *st, 
struct archive *archive)
 static int sync_db_populate(alpm_db_t *db)
 {
const char *dbpath;
-   size_t est_count;
-   int count, fd;
+   size_t est_count, count;
+   int fd;
int ret = 0;
struct stat buf;
struct archive *archive;
@@ -510,10 +510,10 @@ static int sync_db_populate(alpm_db_t *db)
count = alpm_list_count(db->pkgcache->list);
if(count > 0) {
db->pkgcache->list = alpm_list_msort(db->pkgcache->list,
-   (size_t)count, _alpm_pkg_cmp);
+   count, _alpm_pkg_cmp);
}
_alpm_log(db->handle, ALPM_LOG_DEBUG,
-   "added %d packages to package cache for db '%s'\n",
+   "added %zu packages to package cache for db '%s'\n",
count, db->treename);
 
 cleanup:
-- 
2.10.0


[pacman-dev] [PATCH 1/2] Return boolean from db_populate

2016-09-22 Thread Rikard Falkeborn
Since the number of packages is not used anywhere, just return a
boolean to avoid the implicit cast from size_t to int in be_local.c.
Use 0 as success to be consistent with db_validate.

Signed-off-by: Rikard Falkeborn 
---

This patch is made on top of the "Fix Gcc overflow patch". It will
apply to master directly, but the diff doesn't match fully.

As suggested by Andrew after concern of casts from size_t to
int, triggered by the "Fix Gcc overflow patch", change the
"populate" functions to return a flag instead of the number
of packages to avoid implicit casting from size_t to int
in local_db_populate().

The next patch changes the sync_db_populate() function to use
size_t for count in order to not implicitly cast the return
value from alpm_list_count() to int.

 lib/libalpm/be_local.c | 2 +-
 lib/libalpm/be_sync.c  | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c
index 3ef1975..43c6bc9 100644
--- a/lib/libalpm/be_local.c
+++ b/lib/libalpm/be_local.c
@@ -612,7 +612,7 @@ static int local_db_populate(alpm_db_t *db)
_alpm_log(db->handle, ALPM_LOG_DEBUG, "added %zu packages to package 
cache for db '%s'\n",
count, db->treename);
 
-   return count;
+   return 0;
 }
 
 /* Note: the return value must be freed by the caller */
diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 32a669d..f127ed8 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -454,6 +454,7 @@ static int sync_db_populate(alpm_db_t *db)
const char *dbpath;
size_t est_count;
int count, fd;
+   int ret = 0;
struct stat buf;
struct archive *archive;
struct archive_entry *entry;
@@ -487,7 +488,7 @@ static int sync_db_populate(alpm_db_t *db)
db->pkgcache = _alpm_pkghash_create(est_count);
if(db->pkgcache == NULL) {
db->handle->pm_errno = ALPM_ERR_MEMORY;
-   count = -1;
+   ret = -1;
goto cleanup;
}
 
@@ -520,7 +521,7 @@ cleanup:
if(fd >= 0) {
close(fd);
}
-   return count;
+   return ret;
 }
 
 /* This function validates %FILENAME%. filename must be between 3 and
-- 
2.10.0