Bug 764677

Summary: PackageKit get the wrong package size that due to libzypp changes
Product: [openSUSE] openSUSE 12.2 Reporter: Max Lin <mlin>
Component: GNOMEAssignee: Vincent Untz <vuntz>
Status: RESOLVED FIXED QA Contact: E-mail List <qa-bugs>
Severity: Normal    
Priority: P5 - None CC: badshah400, dimstar, mlin, sreeves
Version: Factory   
Target Milestone: ---   
Hardware: x86-64   
OS: openSUSE 12.2   
Whiteboard:
Found By: --- Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---
Attachments: wrong package size in apper
a simple patch to fix the size calculation
correct calculate package size
Reworked patch, based on vuntz' comments

Description Max Lin 2012-05-30 09:31:15 UTC
Created attachment 492931 [details]
wrong package size in apper

These days, I see apper(a package management tool for KDE) get the wrong package size from PackageKit that such as the attachment. And running "pkcon get-details" also get the weird package size, for example:

kdeman@linux-c5b2:/tmp> pkcon get-details upower
More than one package matches:
1. upower-0.9.16-1.1.x86_64 [@System]
2. upower-0.9.16-1.1.i586 [factory-oss]
Please choose the correct package: 2
...
  size:        101938176 bytes
...

apparently the size looks wrong, it is not true big as pkcon said, I found the same behavior via dbus-monitor btw.

Reproducible: Always

Steps to Reproduce:
1.start apper(but I guess that it also can reproduce in gnome-packagekit)
2.enter any categories or check the new updates
3.watch the package size what it get

Actual Results:  
see attachment

Expected Results:  
valid size
Comment 1 Max Lin 2012-05-30 09:40:50 UTC
err..forgot say about libzypp changes.

I saw a libzypp commit https://github.com/openSUSE/libzypp/commit/1e29b99a9dd54344ac3d7486e32b0fe9803ae28f

since the return size no longer in kbytes, and packagekit zypp backend still have do size calculation as size * 1024, thus I guess it cause this problem, I guess...
Comment 2 Max Lin 2012-05-30 09:50:08 UTC
Created attachment 492937 [details]
a simple patch to fix the size calculation

I have build PacakgeKit with this patch, the patch just do that give raw size value to pk_backend_details() rather than give size * 1024, it looks fix the issue.

Any comment?
Comment 3 Max Lin 2012-06-04 04:02:43 UTC
Hi Scott,

I guess that you are the right man for this issue, so I added you to CC. :p
Any comment?
Comment 4 Dominique Leuenberger 2012-06-16 20:01:37 UTC
If we can get a patch that can correctly identify WHICH of the API variants of libzypp is being used, I am willing to merge it.

The current patch will cause issues when this PK build is being used on an 'older' version of libzypp, which does not have that change implemented yet (thus only shifting the problem).

@Max,and chance to work up something like that?
Comment 5 Max Lin 2012-06-18 07:57:25 UTC
Created attachment 495099 [details]
correct calculate package size

Well, hope I didn't missed somewhere, but I could not found any function which return libzypp version, and also could not found any macro defined in devel header...

so I tried another way what like I attached. Although its looks weird(check libzypp twice), but it doesn't need modify libzypp itself and works on my 12.1 and Factory machines. comment?
Comment 6 Dominique Leuenberger 2012-06-20 16:53:19 UTC
Thanks Max,

I think that patch is as good as it gets without zypp introducing version information in its headers (which would be SO MUCH nicer :) )

I'll spin up a package with the patch included
Comment 7 Vincent Untz 2012-06-20 19:00:36 UTC
Comment on attachment 495099 [details]
correct calculate package size

A few comments on the patch:

>--- a/configure.ac
>+++ b/configure.ac
>@@ -742,8 +742,16 @@ fi
> 
> if test x$enable_zypp = xyes; then
> 	PKG_CHECK_MODULES(ZYPP, libzypp >= 6.16.0)
>+	PKG_CHECK_MODULES([LIBZYPP_RETURN_BYTES], libzypp >= 11.4.0, ZYPP_RETURN_BYTES="yes", ZYPP_RETURN_BYTES="no")

PKG_CHECK_EXISTS is better than PKG_CHECK_MODULES here. Just do:
PKG_CHECK_EXISTS(libzypp >= 11.4.0, [ ZYPP_RETURN_BYTES="yes" ], [ ZYPP_RETURN_BYTES="no" ])

>+	if test "x$ZYPP_RETURN_BYTES" = "xyes"; then
>+	    AC_DEFINE(ZYPP_RETURN_BYTES, 1, [define if libzypp return package size is bytes])
>+	fi
>+else
>+        ZYPP_RETURN_BYTES=no
> fi
> 
>+AM_CONDITIONAL(ZYPP_RETURN_BYTES, test x$ZYPP_RETURN_BYTES = xyes)
>+

There's no need for AM_CONDITIONAL, I think.
Comment 8 Dominique Leuenberger 2012-06-20 19:38:55 UTC
Created attachment 495686 [details]
Reworked patch, based on vuntz' comments
Comment 9 Dominique Leuenberger 2012-06-20 21:14:14 UTC
Patch integrated in SR 125578

Vincent, it would be great if you could assist in upstreaming this patch.
Comment 10 Vincent Untz 2012-06-20 21:33:47 UTC
Committed upstream: https://gitorious.org/packagekit/packagekit/commit/e6191bb910bca7f47726be2ee273b29e9d74442d

I guess we can close the bug?
Comment 11 Dominique Leuenberger 2012-06-21 18:08:31 UTC
All done.. thanks Vincent!
Comment 12 Ludwig Nussel 2012-06-29 12:19:30 UTC
*** Bug 769345 has been marked as a duplicate of this bug. ***
Comment 13 Michael Andres 2012-07-19 16:44:09 UTC
(In reply to comment #1)
> err..forgot say about libzypp changes.
> 
> I saw a libzypp commit
> https://github.com/openSUSE/libzypp/commit/1e29b99a9dd54344ac3d7486e32b0fe9803ae28f
> 
> since the return size no longer in kbytes, and packagekit zypp backend still
> have do size calculation as size * 1024, thus I guess it cause this problem, I
> guess...

Actually 'no' ;) 

The underlying libsolv changed it's representation of size, that's why zypp::ResObject::downloadSize() was adapted. zypp::ResObject::downloadSize() always returns Byte.


Unfortunately the zypp-backend is retrieving libsolv's raw attributes instead of using zypp::ResObject. That's why the backend code broke.

https://github.com/openSUSE/PackageKit/pull/3 suggests a fix for this.
Comment 14 Dominique Leuenberger 2012-07-19 17:41:48 UTC
(In reply to comment #13)

> Unfortunately the zypp-backend is retrieving libsolv's raw attributes instead
> of using zypp::ResObject. That's why the backend code broke.
> 
> https://github.com/openSUSE/PackageKit/pull/3 suggests a fix for this.

Just to make this clear: the submitted patch there is the one that came out from this bug report (and is currently already part of the openSUSE:Factory PackageKit package).
Comment 15 Dominique Leuenberger 2012-07-19 17:42:32 UTC
Ups.. sorry.. my bad... pull 3 is one from you...