Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Hi Julian, I've read the bug history and I'm sending you the remaining patches one by one so that we don't end up in a total mess again. Sorry for that previous mess btw. I'm starting with what was patch 1/5 last year. Best, Michael -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Sat, May 03, 2014 at 04:09:42PM +0200, Michael Schaller wrote: Another friendly ping to Julian... Julian, can you at least commit patch 4/5 as it fixes a docstring? Hi sorry, I'm busy most of the time, and should not work on it today either, but here I am: I committed the docstring patches 4 and 5. Could you resend the remaining patches (possibly in one email, as attachments)? I lost a bit track of which patch is which. Thanks. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Be friendly, do not top-post, and follow RFC 1855 Netiquette. - If you don't I might ignore you. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Hi Julian, I'll need time for that because I've also lost track after all this time... ;-) Hopefully I'll manage this week. If not, it can be up to two months. Please give me a ping if it takes even longer... Best, Michael PS: Thanks for picking this up again. How about getting more maintainers? Would you know some more folks who care about python-apt? On 06/10/2015 06:04 PM, Julian Andres Klode wrote: On Sat, May 03, 2014 at 04:09:42PM +0200, Michael Schaller wrote: Another friendly ping to Julian... Julian, can you at least commit patch 4/5 as it fixes a docstring? Hi sorry, I'm busy most of the time, and should not work on it today either, but here I am: I committed the docstring patches 4 and 5. Could you resend the remaining patches (possibly in one email, as attachments)? I lost a bit track of which patch is which. Thanks. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Another friendly ping to Julian... Julian, can you at least commit patch 4/5 as it fixes a docstring? -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Friendly ping... -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Sat, Mar 08, 2014 at 01:04:35PM +0100, michael wrote: And another friendly ping to Julian...I would really like to continue on this one. ;-) Cool. I commented on github. I do prefer patches by email for commenting, though. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Please do not top-post if possible. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On 03/10/2014 06:04 PM, Julian Andres Klode wrote: Cool. I commented on github. Thank you for reviewing the patches. I've largely reworked the patches, removed the changelog, updated the version, removed awkward stuff and made them simpler. Unfortunately now you have to review them all over again. :-/ I do prefer patches by email for commenting, though. So just git send-email? -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Mon, Mar 10, 2014 at 8:28 PM, Michael Schaller mich...@5challer.de wrote: On 03/10/2014 06:04 PM, Julian Andres Klode wrote: Cool. I commented on github. Thank you for reviewing the patches. I've largely reworked the patches, removed the changelog, updated the version, removed awkward stuff and made them simpler. Unfortunately now you have to review them all over again. :-/ I do prefer patches by email for commenting, though. So just git send-email? git-send-email is great. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Oh. I just realized that there was an issue with patch 3.I'll send out a new version of the patch in a minute.
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
And I've sent the wrong patch. *sigh*I should call it a day...
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
And another friendly ping to Julian...I would really like to continue on this one. ;-)
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Friendly ping to Julian...
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Four more patches:https://github.com/michael-schaller/python-apt/commits/comp_type_deb The first commit includes Julian's preliminary CompTypeDeb patch and bumps the version to 0.9.3 in the changelog. Julian, please let me know what you think about these patches.
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Julian, did you see my reply on your comment?https://github.com/michael-schaller/python-apt/commit/ef6202e2f54dc2085e4d87a01aa2a66ff7c1a437 I'm not sure if GitHub sent you a notificaction...
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Sat, Jan 25, 2014 at 12:41:14PM +0100, michael wrote: Julian, did you see my reply on your comment?https://github.com/michael-schaller/python-apt/commit/ef6202e2f54dc2085e4d87a01aa2a66ff7c1a437 I'm not sure if GitHub sent you a notificaction... Yes, but I'm happy with that part now, it seems more readable the way you have it. I'm not that happy about the tests part and decided to try my own way to simplify the test code. Looks slightly more functional-style. -- 8 -- From 3e33aaa11c0062b6bf97601e3221a9858a533ad2 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode j...@debian.org Date: Sat, 25 Jan 2014 15:46:57 +0100 Subject: [PATCH] tests/test_deps.py: test_dstr cleanup and fixes Simplify the code by not stating all test cases in the code, but using a loop. Also adds more tests this way. --- tests/test_deps.py | 35 +-- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/tests/test_deps.py b/tests/test_deps.py index 0035fa8..96bcd57 100644 --- a/tests/test_deps.py +++ b/tests/test_deps.py @@ -6,6 +6,7 @@ # are permitted in any medium without royalty provided the copyright # notice and this notice are preserved. Unit tests for verifying the correctness of check_dep, etc in apt_pkg. +import itertools import unittest import apt_pkg @@ -114,28 +115,18 @@ class TestDependencies(unittest.TestCase): def test_dstr(self): Test apt.package.BaseDependency.__dstr dstr = apt.package.BaseDependency._BaseDependency__dstr -self.assertEqual(dstr(), ) -self.assertEqual(dstr(), ) -self.assertEqual(, dstr()) -self.assertEqual(, dstr()) -self.assertEqual(dstr(), ) -self.assertEqual(dstr(), ) -self.assertEqual(, dstr()) -self.assertEqual(, dstr()) -self.assertNotEqual(dstr(), ) -self.assertNotEqual(dstr(), =) -self.assertNotEqual(dstr(), ) -self.assertNotEqual(dstr(), !=) -self.assertNotEqual(dstr(), =) -self.assertNotEqual(dstr(), ) -self.assertNotEqual(dstr(), =) -self.assertNotEqual(dstr(), ) -self.assertNotEqual(dstr(), !=) -self.assertNotEqual(dstr(), =) -self.assertFalse(dstr() != ) -self.assertFalse(dstr() != ) -self.assertFalse(dstr() != ) -self.assertFalse(dstr() != ) +equal = {: {, }, + =: {==, =}, + : {, }} +operators = [, , =, !=, =, ==, =, , ] + +for a, b in itertools.product(equal.keys(), operators): +if b in equal[a]: +self.assertEqual(dstr(a), b) +self.assertEqual(b, dstr(a)) +else: +self.assertNotEqual(dstr(a), b) +self.assertNotEqual(b, dstr(a)) def testParseDepends(self): dependencies: Test apt_pkg.ParseDepends(). -- 1.8.5.2 -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Please do not top-post if possible. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Those tests clearly look better! ^^ When it comes to https://github.com/michael-schaller/python-apt/commit/ef6202e2f54dc2085e4d87a01aa2a66ff7c1a437 I would be already happy if you would include the updated docstring. The rest is really just readability. ;-) What do you think of the second commit? https://github.com/michael-schaller/python-apt/commit/928814bbc43ed50ab956271a514baee49c7a452b
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Sat, Jan 25, 2014 at 04:10:54PM +0100, michael wrote: Those tests clearly look better! ^^ When it comes to https://github.com/michael-schaller/python-apt/commit/ef6202e2f54dc2085e4d87a01aa2a66ff7c1a437 I would be already happy if you would include the updated docstring. The rest is really just readability. ;-) OK, I merged the remaining stuff in that commit. What do you think of the second commit? https://github.com/michael-schaller/python-apt/commit/928814bbc43ed50ab956271a514baee49c7a452b I think I prefer something that uses properties if we only pass an apt_pkg.Dependency, like the following one; on the grounds that this is more common in apt.*. -- 8 -- From 6233a7ce270971665e88bf38c28a09f2e75e5545 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode j...@debian.org Date: Sat, 25 Jan 2014 17:02:10 +0100 Subject: [PATCH] apt/package.py: Pass an apt_pkg.Dependency to BaseDependency Instead of passing the properties to __init__(), let's just pass the apt_pkg.Dependency. Store that in a _dep attribute, and provide properties to provide the API. The other classes do it this way as well. Reported-by: Michael Schaller mich...@5challer.de --- apt/package.py | 60 ++ doc/source/library/apt.package.rst | 23 ++- 2 files changed, 43 insertions(+), 40 deletions(-) diff --git a/apt/package.py b/apt/package.py index d1a7274..2703216 100644 --- a/apt/package.py +++ b/apt/package.py @@ -61,15 +61,7 @@ class FetchError(Exception): class BaseDependency(object): -A single dependency. - -Attributes defined here: -name - The name of the dependency -relation - The relation (,=,=,!=,=,,) -version- The version depended on -rawtype - The type of the dependendy (e.g. 'Recommends') -pre_depend - Boolean value whether this is a pre-dependency. - +A single dependency. class __dstr(str): Compare helper for compatibility with old third-party code. @@ -95,12 +87,45 @@ class BaseDependency(object): def __ne__(self, other): return not self.__eq__(other) -def __init__(self, name, rel, ver, pre, rawtype=None): -self.name = name -self.relation = self.__dstr(rel) -self.version = ver -self.pre_depend = pre -self.rawtype = rawtype +def __init__(self, dep): +self._dep = dep # apt_pkg.Dependency + +@property +def name(self): +The name of the target package. +return self._dep.target_pkg.name + +@property +def relation(self): +The relation (, =, !=, =, =, , ). + +Note that the empty string is a valid string as well, if no version +is specified. + +return self.__dstr(self._dep.comp_type) + +@property +def version(self): +The target version or None. + +It is None if and only if relation is the empty string. +return self._dep.target_ver + +@property +def rawtype(self): +Type of the dependency. + +This should be one of 'Breaks', 'Conflicts', 'Depends', 'Enhances', +'PreDepends', 'Recommends', 'Replaces', 'Suggests'. + +Additional types might be added in the future. + +return self._dep.dep_type_untranslated + +@property +def pre_depend(self): +Whether this is a PreDepends. +return self._dep.dep_type_untranslated == 'PreDepends' def __repr__(self): return ('BaseDependency: name:%r relation:%r version:%r preDepend:%r' @@ -433,10 +458,7 @@ class Version(object): for dep_ver_list in depends[type_]: base_deps = [] for dep_or in dep_ver_list: -base_deps.append(BaseDependency(dep_or.target_pkg.name, - dep_or.comp_type, dep_or.target_ver, - (type_ == PreDepends), - rawtype=type_)) +base_deps.append(BaseDependency(dep_or)) depends_list.append(Dependency(base_deps)) except KeyError: pass diff --git a/doc/source/library/apt.package.rst b/doc/source/library/apt.package.rst index 825bf3d..1a69c1e 100644 --- a/doc/source/library/apt.package.rst +++ b/doc/source/library/apt.package.rst @@ -23,27 +23,8 @@ The Version class Dependency Information -- -.. class:: BaseDependency - -The :class:`BaseDependency` class defines various attributes for accessing -the parts of a dependency. The attributes are as follows: - -.. attribute:: name - -The name of the dependency - -.. attribute:: relation - -The relation (,=,=,!=,=,,), as string. Note that the empty string -is a valid string as well, if no version is specified. - -.. attribute:: version - -The version or None. -
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Sat, 25 Jan 2014 17:17:18 +0100 Julian Andres Klodelt;j...@debian.orggt; wrote I think I prefer something that uses properties if we only pass an apt_pkg.Dependency, like the following one; on the grounds that this is more common in apt.*. Julian, I've planned to rework this from instance variables to properties with the next commit. ;-) I wanted to rework the constructor first because I wasn't sure if you were happy with the API change... Anyhow... Thanks for taking care of it. ^^
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Sat, Jan 18, 2014 at 5:31 PM, Michael Schaller mich...@5challer.de wrote: Friendly ping to to Julian. I'll try to look at it this week, but university started again, so I'm more busy than before. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Sat, Jan 18, 2014 at 5:31 PM, Michael Schaller lt;mich...@5challer.degt; wrote: I'll try to look at it this week, but university started again, so I'm more busy than before. Oh. Your studies keep you busy! No worries then. Take your time... ;-)
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Friendly ping to to Julian. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Before I start with the BaseDependency properties here two more proposals: apt/package.py: Improved readability and documentation of BaseDependency.__dstr(). https://github.com/michael-schaller/python-apt/commit/ef6202e2f54dc2085e4d87a01aa2a66ff7c1a437 apt/package.py: Simplified BaseDependency constructor. https://github.com/michael-schaller/python-apt/commit/928814bbc43ed50ab956271a514baee49c7a452b Julian, how do you like these two patches? -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Julian, another question regarding this change... Do you have any problems with changing the constructor of apt.package.BaseDependency? I would like it to change it so that it only takes a apt_pkg.Dependency object as argument. As far as I see it the constructor is not documented and so I would assume that the constructor is not part of the public API. Am I correct? -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Mon, Jan 06, 2014 at 05:45:20PM +0100, Michael Schaller wrote: On 01/06/2014 05:35 PM, Julian Andres Klode wrote: It still needs test cases and documentation in doc, but should work. I thus did not commit it. That looks good to me. Do you intend to submit it? If so I can make the needed apt_get.Dependency and apt.package.BaseDependency changes... Well, you can already start. I'm only missing stuff not relevant for you (tests docs), so there's no problem if you go ahead and do the remaining stuff before I commit my part. It does something completely different. The point of __dstr is not to make the self.relation member something modern, but to make client code that does dep.relation == checks work. You are correct. My proposal wouldn't work for this use case. Now that we've discussed all this how about documenting BaseDependency.__dstr properly so that a reader knows what it is for? What else do you want documented there? It seems to be explained Helper to make match and match -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Please do not top-post if possible. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On 01/07/2014 02:33 PM, Julian Andres Klode wrote: Well, you can already start. I'm only missing stuff not relevant for you (tests docs), so there's no problem if you go ahead and do the remaining stuff before I commit my part. Will do but that will most likely be on the weekend as my vacation is over. :-( What else do you want documented there? It seems to be explained Helper to make match and match A lot comes to mind... It doesn't explain the = matches == case. It also doesn't explain that this is for compatibility with older third-party code that still checks relations against or . I would also comment/document when this compatibility code should be deprecated and when it should be removed. This code is nearly 4 years in place by now and so I think there should be a deprecation notice for this compatibility code by now... Do you think that this all is not document-worthy? ;-) -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Mon, Jan 06, 2014 at 08:09:43AM +0100, Michael Schaller wrote: Awesome! My first patch in Debian. ^^ Thanks for all your patience so far... Next thing on the agenda is the relation handling in BaseDependency. I'm wondering if I'm the only one trying to use this but the relation handling seems to be totally broken at the moment. There is a huge difference between the Debian manual (what I should get) and the actual results from python-apt.. python-apt does not use Debian-style dependencies, it uses the style everyone else uses. And this is documented. Before the new API was introduced in 0.7.9X, some parts were using Debian-style and some were using these normal ones. I then unified them to only use the normal ones because (IIRC) most parts were using them already. APT itself exports both variants, the one we are using is pkgCache::CompType, the other one is pkgCache::CompTypeDeb. Just view it as an abstraction. We can introduce a comp_type_deb in Dependency, though, if you like. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Please do not top-post if possible. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Having two styles explains totally why I'm confused. ^^ Some more questions/comments regarding this... What do you mean by it uses the style everyone else uses? Isn't Debian-style THE common denominator? If not, do you have a link where I can read more about the style everyone else uses? Maybe this link should be added to the documentation as background information. A comp_type_deb does sound like a good idea but it would need to be properly documented so that people understand the difference. I also think that this shouldn't be in apt_pkg.Dependency but rather in apt.package.BaseDependency. What do you think? IMHO the documentation for apt_pkg.Version.comp_type needs fixing because it lists and == albeit apt_pkg never uses , or ==. It only uses ['', '=', '=', '=', '', ''] as you can see in my previous post. IMHO the documentation for apt.package.BaseDependency needs fixing because it lists == albeit apt never uses ==. It only uses ['', '=', '=', '=', '', ''] as you can see in my previous post. Furthermore it also allows or better said should be able to handle , and == (via apt.package.BaseDependency.__dstr) but and aren't listed in the docstring. Can it be that apt.package.BaseDependency.__dstr has a bug? Shouldn't __ne__ only use __ne__ calls? If you ask me then: def __ne__(self, other): return str.__eq__(self, other) and str.__ne__(2 * self, other) should be: def __ne__(self, other): return str.__ne__(self, other) and str.__ne__(2 * self, other) Am I the only one who thinks that apt.package.BaseDependency.__dstr is a really weird helper? Why not just use a dict and translate all possible strings to the expected strings? That would IMHO increase the readability and would only make the expected strings available. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Mon, Jan 06, 2014 at 12:31:52PM +0100, Michael Schaller wrote: Having two styles explains totally why I'm confused. ^^ Some more questions/comments regarding this... What do you mean by it uses the style everyone else uses? Isn't Debian-style THE common denominator? If not, do you have a link where I can read more about the style everyone else uses? Maybe this link should be added to the documentation as background information. Outside the Debian world, the Debian style is not used. The style used here is just the common ASCII mathematical notation. A comp_type_deb does sound like a good idea but it would need to be properly documented so that people understand the difference. I also think that this shouldn't be in apt_pkg.Dependency but rather in apt.package.BaseDependency. What do you think? No. It would have to be in both. IMHO the documentation for apt_pkg.Version.comp_type needs fixing because it lists and == albeit apt_pkg never uses , or ==. It only uses ['', '=', '=', '=', '', ''] as you can see in my previous post. Fixed. And it also uses !=, apt's source code tells me. IMHO the documentation for apt.package.BaseDependency needs fixing because it lists == albeit apt never uses ==. It only uses ['', '=', '=', '=', '', ''] as you can see in my previous post. Furthermore it also allows or better said should be able to handle , and == (via apt.package.BaseDependency.__dstr) but and aren't listed in the docstring.i Fixed. Can it be that apt.package.BaseDependency.__dstr has a bug? Shouldn't __ne__ only use __ne__ calls? If you ask me then: def __ne__(self, other): return str.__eq__(self, other) and str.__ne__(2 * self, other) should be: def __ne__(self, other): return str.__ne__(self, other) and str.__ne__(2 * self, other) Am I the only one who thinks that apt.package.BaseDependency.__dstr is a really weird helper? Why not just use a dict and translate all possible strings to the expected strings? That would IMHO increase the readability and would only make the expected strings available. I don't know. I did not write IIRC. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Please do not top-post if possible. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On 01/06/2014 02:04 PM, Julian Andres Klode wrote: Outside the Debian world, the Debian style is not used. The style used here is just the common ASCII mathematical notation. Oh. Right! Now that you mention it APT also supports RPMs and thus it would make sense to use the math-notation. Got it! Thanks! No. It would have to be in both. Right. Makes sense. So you would propose to add the properties apt_pkg.Dependency.comp_type_deb and apt.package.BaseDependency.relation_deb? Fixed. And it also uses !=, apt's source code tells me. Makes sense. Thanks for updating the documentation. I have one nitpick though... apt_pkg.Dependency doesn't list . Can it be that apt.package.BaseDependency.__dstr has a bug? Shouldn't __ne__ only use __ne__ calls? If you ask me then: def __ne__(self, other): return str.__eq__(self, other) and str.__ne__(2 * self, other) should be: def __ne__(self, other): return str.__ne__(self, other) and str.__ne__(2 * self, other) You didn't answer this one. Do you think this is a bug? Am I the only one who thinks that apt.package.BaseDependency.__dstr is a really weird helper? Why not just use a dict and translate all possible strings to the expected strings? That would IMHO increase the readability and would only make the expected strings available. I don't know. I did not write IIRC. Actually you did write it. ;-) http://anonscm.debian.org/gitweb/?p=apt/python-apt.git;a=commit;h=28fcce3fe36ab2c3855d4c3766490272a7310392 Can you elaborate what's the rationale behind this compatibility code? -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Mon, Jan 06, 2014 at 03:43:25PM +0100, Michael Schaller wrote: On 01/06/2014 02:04 PM, Julian Andres Klode wrote: Outside the Debian world, the Debian style is not used. The style used here is just the common ASCII mathematical notation. Oh. Right! Now that you mention it APT also supports RPMs and thus it would make sense to use the math-notation. Got it! Thanks! We do not support RPM. There was a fork apt-rpm but nobody ever merged it back. No. It would have to be in both. Right. Makes sense. So you would propose to add the properties apt_pkg.Dependency.comp_type_deb and apt.package.BaseDependency.relation_deb? If you think they are needed, go ahead. We have some other places using CompType() in the C++ code, though, and adding _deb variants for them would be complicated. Fixed. And it also uses !=, apt's source code tells me. Makes sense. Thanks for updating the documentation. I have one nitpick though... apt_pkg.Dependency doesn't list . Fixed. I updated all lists everywhere so the dependencies are sorted now. Can it be that apt.package.BaseDependency.__dstr has a bug? Shouldn't __ne__ only use __ne__ calls? If you ask me then: def __ne__(self, other): return str.__eq__(self, other) and str.__ne__(2 * self, other) should be: def __ne__(self, other): return str.__ne__(self, other) and str.__ne__(2 * self, other) You didn't answer this one. Do you think this is a bug? Yes, I fixed it by making def __ne__(self, other): -return str.__eq__(self, other) and str.__ne__(2 * self, other) +return not self.__eq__(other) Am I the only one who thinks that apt.package.BaseDependency.__dstr is a really weird helper? Why not just use a dict and translate all possible strings to the expected strings? That would IMHO increase the readability and would only make the expected strings available. I don't know. I did not write IIRC. Actually you did write it. ;-) http://anonscm.debian.org/gitweb/?p=apt/python-apt.git;a=commit;h=28fcce3fe36ab2c3855d4c3766490272a7310392 It's very simple. We only use __dstr() and __dstr(), so everything works as expected this way. Can you elaborate what's the rationale behind this compatibility code? In old python-apt versions, this was one of the cases where we returned instead of . Using this compatibility code, old code keeps working. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Please do not top-post if possible. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On 01/06/2014 04:28 PM, Julian Andres Klode wrote: We do not support RPM. There was a fork apt-rpm but nobody ever merged it back. That's why I haven't heard of it in ages. ;-) If you think they are needed, go ahead. We have some other places using CompType() in the C++ code, though, and adding _deb variants for them would be complicated. BaseDependency.relation_deb would only be a nice to have. I'm more interested in having a string that represents the original dependency string. A property that returns something like python (= 2.7) for a BaseDependency. Would it be okay to add something like this to BaseDependency? Maybe BaseDependency.rawstr similar to BaseDependency.rawtype. What do you think? Yes, I fixed it by making def __ne__(self, other): -return str.__eq__(self, other) and str.__ne__(2 * self, other) +return not self.__eq__(other) Thanks. That also does the trick. ^^ It's very simple. We only use __dstr() and __dstr(), so everything works as expected this way. It is also used for = to let == equal =. Is that also intentional? Can you elaborate what's the rationale behind this compatibility code? In old python-apt versions, this was one of the cases where we returned instead of . Using this compatibility code, old code keeps working. I see. Is this code still needed? Shouldn't all these issues be fixed in python-apt by now...? Furthermore the code is hard to read and so no one realized that there was a bug in it for nearly 4 years. I personally would write it this way: # Relation verification and replaces for compatibility '' with '' # and '' with ''. RELATION_COMPAT = { '': '', '': '', '': '', '': '', '=': '=', '=': '=', '=': '=', '!=': '!=', '': '', } def __init__(self, name, rel, ver, pre, rawtype=None): self.name = name self.relation = RELATION_COMPAT[rel] ... What do you think about this code? -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Mon, Jan 06, 2014 at 04:56:27PM +0100, Michael Schaller wrote: If you think they are needed, go ahead. We have some other places using CompType() in the C++ code, though, and adding _deb variants for them would be complicated. BaseDependency.relation_deb would only be a nice to have. I'm more interested in having a string that represents the original dependency string. A property that returns something like python (= 2.7) for a BaseDependency. Would it be okay to add something like this to BaseDependency? Maybe BaseDependency.rawstr similar to BaseDependency.rawtype. What do you think? You'd still need a comp_type_deb in apt_pkg.Dependency in order to construct that string, or you simply replace by and by manually. You can write this, and make it the __str__() method of the dependency. While writing this I noticed that there is no DepIterator::CompTypeDeb, so I need to do it a bit more complicated if I want to implement comp_type_deb. diff --git a/python/cache.cc b/python/cache.cc index c0f1ede..443710b 100644 --- a/python/cache.cc +++ b/python/cache.cc @@ -1431,6 +1431,12 @@ static PyObject *DependencyGetCompType(PyObject *Self,void*) return PyString_FromString(Dep.CompType()); } +static PyObject *DependencyGetCompTypeDeb(PyObject *Self,void*) +{ + pkgCache::DepIterator Dep = GetCpppkgCache::DepIterator(Self); + return PyString_FromString(pkgCache::CompTypeDeb(Dep-CompareOp)); +} + static PyObject *DependencyGetDepType(PyObject *Self,void*) { pkgCache::DepIterator Dep = GetCpppkgCache::DepIterator(Self); @@ -1459,6 +1465,9 @@ static PyGetSetDef DependencyGetSet[] = { {comp_type,DependencyGetCompType,0, The type of comparison, as a string, namely one of:\n '', '=', '=', '!=', '=', '', ''.}, + {comp_type_deb,DependencyGetCompTypeDeb,0, +The type of comparison in Debian format, as a string, namely one of:\n +'', '=', '=', '!=', '=', '', ''.}, {dep_type,DependencyGetDepType,0, The type of the dependency; may be translated}, {dep_type_untranslated,DependencyGetDepTypeUntranslated,0, It still needs test cases and documentation in doc, but should work. I thus did not commit it. It's very simple. We only use __dstr() and __dstr(), so everything works as expected this way. It is also used for = to let == equal =. Is that also intentional? Most likely yes. Some docs mentioned == previously. Can you elaborate what's the rationale behind this compatibility code? In old python-apt versions, this was one of the cases where we returned instead of . Using this compatibility code, old code keeps working. I see. Is this code still needed? Shouldn't all these issues be fixed in python-apt by now...? No issues in python-apt, but potential issues in client code. Removing it might silently break code. Furthermore the code is hard to read and so no one realized that there was a bug in it for nearly 4 years. I personally would write it this way: # Relation verification and replaces for compatibility '' with '' # and '' with ''. RELATION_COMPAT = { '': '', '': '', '': '', '': '', '=': '=', '=': '=', '=': '=', '!=': '!=', '': '', } def __init__(self, name, rel, ver, pre, rawtype=None): self.name = name self.relation = RELATION_COMPAT[rel] ... What do you think about this code? It does something completely different. The point of __dstr is not to make the self.relation member something modern, but to make client code that does dep.relation == checks work. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Please do not top-post if possible. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On 01/06/2014 05:35 PM, Julian Andres Klode wrote: It still needs test cases and documentation in doc, but should work. I thus did not commit it. That looks good to me. Do you intend to submit it? If so I can make the needed apt_get.Dependency and apt.package.BaseDependency changes... It does something completely different. The point of __dstr is not to make the self.relation member something modern, but to make client code that does dep.relation == checks work. You are correct. My proposal wouldn't work for this use case. Now that we've discussed all this how about documenting BaseDependency.__dstr properly so that a reader knows what it is for? -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Sat, Jan 04, 2014 at 07:08:58PM +0100, Michael Vogt wrote: Good catch, thanks! Updated that too. The patchset includes your pep8/pyflakes patch as well to avoid conflicts. Lets see if Julian has further feedback and if he is also ok with the changes I will commit to the python-apt git upstream branch. This includes an older version of other-Michael's patch. The newest one has an additional paragraph in the commit message specifying what changes where made. But apart from that, it's fine. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Please do not top-post if possible. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Sun, Jan 05, 2014 at 07:15:05PM +0100, Julian Andres Klode wrote: On Sat, Jan 04, 2014 at 07:08:58PM +0100, Michael Vogt wrote: Good catch, thanks! Updated that too. The patchset includes your pep8/pyflakes patch as well to avoid conflicts. Lets see if Julian has further feedback and if he is also ok with the changes I will commit to the python-apt git upstream branch. This includes an older version of other-Michael's patch. The newest one has an additional paragraph in the commit message specifying what changes where made. But apart from that, it's fine. Excellent, thanks! I fixed that and used Michaels latest patch as the base and applied the others on top. Thanks for your review! Sorry for having hijacked the original bugreport, but the pep8/pyflakes got me carried away a bit. Time to get back to the original topic of the bugreport :) Cheers, Michael -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Awesome! My first patch in Debian. ^^ Thanks for all your patience so far... Next thing on the agenda is the relation handling in BaseDependency. I'm wondering if I'm the only one trying to use this but the relation handling seems to be totally broken at the moment. There is a huge difference between the Debian manual (what I should get) and the actual results from python-apt... According to the Debian Manual [1] the only allowed relations are , =, =, = and . Back in the days there have been which maps to = and which maps to =. and have been deprecated because they were confusingly used. python-apt on the other side behaves totally different: --- #!/usr/bin/python import apt cache = apt.cache.Cache() relations = set() for pkg in cache: # apt.package.Package ver = pkg.candidate # apt.package.Version or_deps = ver.get_dependencies(PreDepends, Depends, Recommends) for or_dep in or_deps: # apt.package.Dependency for dep in or_dep: # apt.package.BaseDependency relations.add(dep.relation) print relations --- Output: set(['', '=', '=', '=', '', '']) Looking a bit deeper this behaviour is already present in apt_pkg: --- #!/usr/bin/python import apt cache = apt.cache.Cache() relations = set() for pkg in cache: # apt.package.Package ver = pkg.candidate # apt.package.Version _ver = ver._cand # apt_pkg.Version dep_dict = _ver.depends_list pre_deps = dep_dict.get('PreDepends', []) deps = dep_dict.get('Depends', []) recs = dep_dict.get('Recommends', []) for or_dep in pre_deps: for dep in or_dep: # apt_pkg.Dependency relations.add(dep.comp_type) for or_dep in deps: for dep in or_dep: # apt_pkg.Dependency relations.add(dep.comp_type) for or_dep in recs: for dep in or_dep: # apt_pkg.Dependency relations.add(dep.comp_type) print relations --- Output: set(['', '=', '=', '=', '', '']) The documentation of apt_pkg.Dependency.comp_type reads like this: .. attribute:: comp_type The type of comparison (=, ==, , =), as string. dpkg seems to be only adhering to what is written in the Debian Manual: $ egrep -o '\( |\( |\(= |\( |\( |\(= |\(= |\(== ' /var/lib/dpkg/status | tr -d '(' | sort | uniq = = = I hope you are confused too, so can we fix this in python-apt? I know that this would break compatibility but the current implementation is plain wrong IMHO. Also the question would be where this needs fixing... (libapt-pkg, apt_pkg.Dependency or apt.Package.BaseDependency) [1] http://www.debian.org/doc/debian-policy/ch-relationships.html -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Tue, Dec 31, 2013 at 11:58:17PM +0100, Julian Andres Klode wrote: On Tue, Dec 31, 2013 at 11:06:15PM +0100, Michael Vogt wrote: [..] You really need to include the original copyright statement and the license here, otherwise you violate the license. If the original code does not include the license or a copyright statement above it, it might not be distributable at all, as the license clearly says: The *above* copyright notice and *this permission notice* shall be included in all copies or substantial portions of the Software. (I just added some emphasis) I'd even say that including the copyright notice *below* the license terms is wrong, as the license clearly says *above*. But not including them is definitely wrong. We don't want legal issues. Indeed! The header was cargo-culted from the update-manager test_pyflakes.py but I rewrote the file for python-apt completely so we are clean here (and I fixed the comment). I will followup with the update-manager test too. Cheers, Michael -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On 01/01/2014 05:44 PM, Michael Vogt wrote: On Wed, Jan 01, 2014 at 12:30:16PM +0100, Michael Schaller wrote: A few comments on the patches: Thanks for your review and the points you raised. You're welcome. I really like the idea to use Travis CI. 1) tests/test_pep8.py Can you document in a comment or docstring why you ignore certain issues and what these issues are? This should help readers to better understand what the test does and more importantly what it ignores. Furthermore do you plan to reduce the number of ignores in the future? If so you should add a TODO for yourself. 2) tests/test_pep8.py Can you use instead of 'self.assertEqual(res, 0)' a better fail message like you did in 'tests/test_pyflakes.py'? 3) tests/test_pyflakes.py You don't need the 'from __future__ import print_function' import with Python 3. Those are fixed in my git branch now, thanks! Thanks. Could you include the detailed messages for the error codes? (taken from --show-pep8) E125 continuation line does not distinguish itself from next logical line E126 continuation line over-indented for hanging indent E127 continuation line over-indented for visual indent E128 continuation line under-indented for visual indent IMHO those should be fixed too because they increase the readability of the source code. You can try to fix those with autopep8. 4) debian/control (nitpick) I prefer to sort dependencies alphabetically. I have no opinion about this :) I don't really mind either way. Especially for packages with many dependencies I think it helps to check if a dependency is already there or missing. But given that one doesn't look often into debian/control I can really understand that there is no strong opinion on it. ;-) 5) .travis.yml I would really like to see a rather lengthy comment in the beginning of the YAML file to explain what purpose it serves and what it does. I also think you should give a link here and that you should explain what should happen after Trusty. Maybe it would be even better to use SID instead of Trusty. 6) .travis.yml Why do you run './debian/rules binary' and not 'dpkg-buildpackage'? 7) .travis.yml Why not also ensure here that Lintian and if possible piuparts have no issues with the newly built package? Good points, I added a explaination to the top now that hopefully addresses the points. I renamed the added sources.list entry as its misleading. In my branch its using distro-info --stable to get test latest python3.3 and apt/libapt. Using sid here is probably a bit too fragile, the chroots of the travis-ci.org are ubuntu based. But I would certainly love to see a travis-ci instance that is debian based. I changed the debian/rules binary in my branch now to be ./debian/rules build-arch. This will build and run the tests. We can't use dpkg-buildpackage currently as its using fakeroot and the tests (iirc in the test_auth.py code) also use fakeroot and nesting is not possible AFAIK. But I do like the idea of building the deb and doing additional checks like lintian/piuparts. Thanks for the good explanation. ^^ BTW: Don't you need a '-q' for 'sudo apt-get install build-essential $(gdebi -q --apt-line ./debian/control)', too? Cheers and happy new year! Michael Oh yeah. Happy new year! -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Tue, Dec 31, 2013 at 11:06:15PM +0100, Michael Vogt wrote: On Tue, Dec 31, 2013 at 01:09:39PM +0100, Michael Schaller wrote: [..] It also adds a .travis.yml file that runs the tests. Next step is to actually make the pep8/pylakes test to pass :) [..] That is now done, my branch is pyflakes and pep8 clean. The patch is a bit big (mostly whitespace of course), I pushed it to https://github.com/mvo5/python-apt/tree/feature/travis (but I guess the name should be changed :) I had to update the minimal python3 version to 3.3 because test_paths.py would not work otherwise. We might consider rewriting this test to work on 3.2 (but honestly, I'm not very motivated to do that). Cheers, Michael -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On 12/31/2013 11:52 PM, Julian Andres Klode wrote: It's still a git patch, so it should still have a summary line. git tools are becoming a bit useless otherwise. I did not write “remove the summary line”. * apt/cache.py: - Fixed PEP8 issues - Fixed pyflakes issue: Removed unused local variable 'transient' * apt/package.py: - Fixed PEP8 issues - Fixed pyflakes issue: Removed unused import 'warnings' And git patches usually have prose text. I know that the latest commit does not follow the usual git conventions, but I'm trying to get us completely adopt them, because it just looks unnatural otherwise. Done. Please see the attached patch in revision 3. From 2da21a6e4161799474d996f2530aad03ec9ee674 Mon Sep 17 00:00:00 2001 From: Michael Schaller mich...@5challer.de Date: Wed, 1 Jan 2014 12:08:21 +0100 Subject: [PATCH] apt/cache.py, apt/package.py: Fixed PEP8 and pyflakes issues This commit removed the unused local variable 'transient' in 'apt/cache.py' and the unused import 'warnings' in 'apt/package.py'. --- apt/cache.py | 53 + apt/package.py | 40 +++- debian/changelog | 9 + 3 files changed, 53 insertions(+), 49 deletions(-) diff --git a/apt/cache.py b/apt/cache.py index 43fb55d..6b1e2bd 100644 --- a/apt/cache.py +++ b/apt/cache.py @@ -40,6 +40,7 @@ class FetchFailedException(IOError): class LockFailedException(IOError): Exception that is thrown when locking fails. + class CacheClosedException(Exception): Exception that is thrown when the cache is used after close(). @@ -53,7 +54,7 @@ class Cache(object): list of available packages. The cache can be used like a mapping from package names to Package -objects (although only getting items is supported). +objects (although only getting items is supported). Keyword arguments: progress -- a OpProgress object @@ -74,7 +75,7 @@ class Cache(object): self._fullnameset = set() self._changes_count = -1 self._sorted_set = None - + self.connect(cache_post_open, self._inc_changes_count) self.connect(cache_post_change, self._inc_changes_count) if memonly: @@ -86,17 +87,17 @@ class Cache(object): apt_pkg.config.clear(APT) apt_pkg.config.set(Dir, rootdir) apt_pkg.init_config() -if os.path.exists(rootdir+/etc/apt/apt.conf): +if os.path.exists(rootdir + /etc/apt/apt.conf): apt_pkg.read_config_file(apt_pkg.config, - rootdir + /etc/apt/apt.conf) -if os.path.isdir(rootdir+/etc/apt/apt.conf.d): + rootdir + /etc/apt/apt.conf) +if os.path.isdir(rootdir + /etc/apt/apt.conf.d): apt_pkg.read_config_dir(apt_pkg.config, - rootdir + /etc/apt/apt.conf.d) +rootdir + /etc/apt/apt.conf.d) apt_pkg.config.set(Dir::State::status, rootdir + /var/lib/dpkg/status) # also set dpkg to the rootdir path so that its called for the # --print-foreign-architectures call -apt_pkg.config.set(Dir::bin::dpkg, +apt_pkg.config.set(Dir::bin::dpkg, os.path.join(rootdir, usr, bin, dpkg)) # create required dirs/files when run with special rootdir # automatically @@ -105,7 +106,6 @@ class Cache(object): # recognized (LP: #320665) apt_pkg.init_system() self.open(progress) - def _inc_changes_count(self): Increase the number of changes @@ -118,12 +118,12 @@ class Cache(object): files = [/var/lib/dpkg/status, /etc/apt/sources.list, -] + ] dirs = [/var/lib/dpkg, /etc/apt/, /var/cache/apt/archives/partial, /var/lib/apt/lists/partial, - ] +] for d in dirs: if not os.path.exists(rootdir + d): #print creating: , rootdir + d @@ -165,8 +165,8 @@ class Cache(object): i = last = 0 size = len(self._cache.packages) for pkg in self._cache.packages: -if progress is not None and last+100 i: -progress.update(i/float(size)*100) +if progress is not None and last + 100 i: +progress.update(i / float(size) * 100) last = i # drop stuff with no versions (cruft) if pkg.has_versions: @@ -259,7 +259,8 @@ class Cache(object): def required_download(self): Get the size of the packages that are required to download. if self._records is None: -raise CacheClosedException(Cache
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
A few comments on the patches: 1) tests/test_pep8.py Can you document in a comment or docstring why you ignore certain issues and what these issues are? This should help readers to better understand what the test does and more importantly what it ignores. Furthermore do you plan to reduce the number of ignores in the future? If so you should add a TODO for yourself. 2) tests/test_pep8.py Can you use instead of 'self.assertEqual(res, 0)' a better fail message like you did in 'tests/test_pyflakes.py'? 3) tests/test_pyflakes.py You don't need the 'from __future__ import print_function' import with Python 3. 4) debian/control (nitpick) I prefer to sort dependencies alphabetically. 5) .travis.yml I would really like to see a rather lengthy comment in the beginning of the YAML file to explain what purpose it serves and what it does. I also think you should give a link here and that you should explain what should happen after Trusty. Maybe it would be even better to use SID instead of Trusty. 6) .travis.yml Why do you run './debian/rules binary' and not 'dpkg-buildpackage'? 7) .travis.yml Why not also ensure here that Lintian and if possible piuparts have no issues with the newly built package? -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Wed, Jan 01, 2014 at 12:30:16PM +0100, Michael Schaller wrote: A few comments on the patches: Thanks for your review and the points you raised. 1) tests/test_pep8.py Can you document in a comment or docstring why you ignore certain issues and what these issues are? This should help readers to better understand what the test does and more importantly what it ignores. Furthermore do you plan to reduce the number of ignores in the future? If so you should add a TODO for yourself. 2) tests/test_pep8.py Can you use instead of 'self.assertEqual(res, 0)' a better fail message like you did in 'tests/test_pyflakes.py'? 3) tests/test_pyflakes.py You don't need the 'from __future__ import print_function' import with Python 3. Those are fixed in my git branch now, thanks! 4) debian/control (nitpick) I prefer to sort dependencies alphabetically. I have no opinion about this :) I don't really mind either way. 5) .travis.yml I would really like to see a rather lengthy comment in the beginning of the YAML file to explain what purpose it serves and what it does. I also think you should give a link here and that you should explain what should happen after Trusty. Maybe it would be even better to use SID instead of Trusty. 6) .travis.yml Why do you run './debian/rules binary' and not 'dpkg-buildpackage'? 7) .travis.yml Why not also ensure here that Lintian and if possible piuparts have no issues with the newly built package? Good points, I added a explaination to the top now that hopefully addresses the points. I renamed the added sources.list entry as its misleading. In my branch its using distro-info --stable to get test latest python3.3 and apt/libapt. Using sid here is probably a bit too fragile, the chroots of the travis-ci.org are ubuntu based. But I would certainly love to see a travis-ci instance that is debian based. I changed the debian/rules binary in my branch now to be ./debian/rules build-arch. This will build and run the tests. We can't use dpkg-buildpackage currently as its using fakeroot and the tests (iirc in the test_auth.py code) also use fakeroot and nesting is not possible AFAIK. But I do like the idea of building the deb and doing additional checks like lintian/piuparts. Cheers and happy new year! Michael -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Tue, Dec 31, 2013 at 11:04:47PM +0100, Julian Andres Klode wrote: (Addressed to Michael Vogt, so don't wonder, other Michael...) [..] I thought we wanted to use git-dch now (meaning after I released 0.9.1) for changelog manipulation. I did commit 56ed099558a9f6e7137a8c113ca9efb2b2c1a1d2 without a changelog entry for that reason, but I see that you did the commit afterwards with a changelog entry. Indeed, sorry for this, I'm not used to this yet, but I agree that its a good idea and we should stick with git-dch. It would also be cooler to have some more useful summary lines in your commits. The last one just had * apt/cache.py:. If you use debcommit, you can use debcommit -e to adjust the summary. Again agreed, I was using debcommit and will be more carefull about this in the future. Cheers, Michael -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Hi Julian, On 12/31/2013 01:29 AM, Julian Andres Klode wrote: On Sun, Dec 29, 2013 at 12:06:18PM +0100, Michael Schaller wrote: Package: python-apt Version: 0.9.2 Severity: wishlist Tags: patch This patch does far too many things that are not even remotely connected. Fine by me. It's always hard for me to judge the right patch size if I contribute the first time to a project. I'm already happy that the overall idea of the patch seems to be acceptable. ;-) From 0d295006a98769cd6151c78b3a078ad92d8047ee Mon Sep 17 00:00:00 2001 From: Michael Wisheu mich...@5challer.de Did you change your surname after writing the patch? Good catch! I've got married two months ago and took my wife's last name. I'm still hunting down all the occurrences of my old last name and it looks like I've still had the old name in my git config. I wonder how I've managed to change my mail address but not my name... *sigh* Anyhow... If you think this name change is fishy please let me know and I'll proof to you my identity and that the name change happened. Date: Sun, 29 Dec 2013 11:57:19 +0100 Subject: [PATCH] * apt/cache.py: - Fixed PEP8 linter and pyflakes issues - Added 'InstalledFilter' to get a filtered cache that only contains the currently installed packages. * apt/packages.py: - Fixed PEP8 linter issues The PEP8 stuff should be separate. The InstalledFilter should be separate - Removed special handling of 'collections' import as all supported distributions have Python 2.6 or newer by now. This should be separate - Replaced faulty 'BaseDependency.__dstr' with easier to read compat code. This should be separate - Added new properties to 'Dependency' and 'BaseDependency' to get the target package versions that could satisfy a dependency. Only this should be the patch. Before I start sending smaller patches could you please tell me if I should open new bugs or if we can handle all this in this one bug? Best, Michael -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Tue, Dec 31, 2013 at 01:29:53AM +0100, Julian Andres Klode wrote: On Sun, Dec 29, 2013 at 12:06:18PM +0100, Michael Schaller wrote: [..] - Fixed PEP8 linter issues The PEP8 stuff should be separate. [..] Thanks for bringing up better pep8 compliance. I think something like the attached patch would nice to ensure that the code stays in the pep8 style. If there is consensus on this, I'm happy to make the test pass (i.e. make the rest of the code pep8 compatible). Cheers, Michael -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Hi Michael, From my experience PEP8 and pyflakes are great tools with nearly no false positives - especially compared to pylint - and by now they are available for Python 2 and 3. I personally like to add a lint command to setup.py to my projects. Maybe this would be beneficial for python-apt too. Please let me know if you would like to see a patch for that. Best, Michael -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Tue, Dec 31, 2013 at 12:23:54PM +0100, Michael Schaller wrote: Hi Julian, On 12/31/2013 01:29 AM, Julian Andres Klode wrote: On Sun, Dec 29, 2013 at 12:06:18PM +0100, Michael Schaller wrote: Package: python-apt Version: 0.9.2 Severity: wishlist Tags: patch This patch does far too many things that are not even remotely connected. Fine by me. It's always hard for me to judge the right patch size if I contribute the first time to a project. I'm already happy that the overall idea of the patch seems to be acceptable. ;-) The overall idea seems right, I'm not so sure about the implementation details, though. But I first need smaller patches to be able to read the functional changes without the other changes getting in the way. From 0d295006a98769cd6151c78b3a078ad92d8047ee Mon Sep 17 00:00:00 2001 From: Michael Wisheu mich...@5challer.de Did you change your surname after writing the patch? Good catch! I've got married two months ago and took my wife's last name. I'm still hunting down all the occurrences of my old last name and it looks like I've still had the old name in my git config. I wonder how I've managed to change my mail address but not my name... *sigh* Anyhow... If you think this name change is fishy please let me know and I'll proof to you my identity and that the name change happened. That's not a problem. [...] Before I start sending smaller patches could you please tell me if I should open new bugs or if we can handle all this in this one bug? You can just git-send-email them to the mailing list or even push a git branch somewhere. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Please do not top-post if possible. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
You can just git-send-email them to the mailing list or even push a git branch somewhere. Alrighty. Then let's stick to the bug for the time being as it contains already the context for the patches. See attached the patch with the PEP8 linter and pyflakes fixes. PEP8 only showed typical minor issues (indentation, line continuation, ...) and pyflakes only found an unused import and an unused variable. From 865b39a4f2ed566e7dd7fc13ff83ea26edbab000 Mon Sep 17 00:00:00 2001 From: Michael Schaller mich...@5challer.de Date: Tue, 31 Dec 2013 13:55:09 +0100 Subject: [PATCH] apt/cache.py, apt/package.py: Fixed PEP8 and pyflakes issues --- apt/cache.py | 53 + apt/package.py | 40 +++- debian/changelog | 3 +++ 3 files changed, 47 insertions(+), 49 deletions(-) diff --git a/apt/cache.py b/apt/cache.py index 43fb55d..6b1e2bd 100644 --- a/apt/cache.py +++ b/apt/cache.py @@ -40,6 +40,7 @@ class FetchFailedException(IOError): class LockFailedException(IOError): Exception that is thrown when locking fails. + class CacheClosedException(Exception): Exception that is thrown when the cache is used after close(). @@ -53,7 +54,7 @@ class Cache(object): list of available packages. The cache can be used like a mapping from package names to Package -objects (although only getting items is supported). +objects (although only getting items is supported). Keyword arguments: progress -- a OpProgress object @@ -74,7 +75,7 @@ class Cache(object): self._fullnameset = set() self._changes_count = -1 self._sorted_set = None - + self.connect(cache_post_open, self._inc_changes_count) self.connect(cache_post_change, self._inc_changes_count) if memonly: @@ -86,17 +87,17 @@ class Cache(object): apt_pkg.config.clear(APT) apt_pkg.config.set(Dir, rootdir) apt_pkg.init_config() -if os.path.exists(rootdir+/etc/apt/apt.conf): +if os.path.exists(rootdir + /etc/apt/apt.conf): apt_pkg.read_config_file(apt_pkg.config, - rootdir + /etc/apt/apt.conf) -if os.path.isdir(rootdir+/etc/apt/apt.conf.d): + rootdir + /etc/apt/apt.conf) +if os.path.isdir(rootdir + /etc/apt/apt.conf.d): apt_pkg.read_config_dir(apt_pkg.config, - rootdir + /etc/apt/apt.conf.d) +rootdir + /etc/apt/apt.conf.d) apt_pkg.config.set(Dir::State::status, rootdir + /var/lib/dpkg/status) # also set dpkg to the rootdir path so that its called for the # --print-foreign-architectures call -apt_pkg.config.set(Dir::bin::dpkg, +apt_pkg.config.set(Dir::bin::dpkg, os.path.join(rootdir, usr, bin, dpkg)) # create required dirs/files when run with special rootdir # automatically @@ -105,7 +106,6 @@ class Cache(object): # recognized (LP: #320665) apt_pkg.init_system() self.open(progress) - def _inc_changes_count(self): Increase the number of changes @@ -118,12 +118,12 @@ class Cache(object): files = [/var/lib/dpkg/status, /etc/apt/sources.list, -] + ] dirs = [/var/lib/dpkg, /etc/apt/, /var/cache/apt/archives/partial, /var/lib/apt/lists/partial, - ] +] for d in dirs: if not os.path.exists(rootdir + d): #print creating: , rootdir + d @@ -165,8 +165,8 @@ class Cache(object): i = last = 0 size = len(self._cache.packages) for pkg in self._cache.packages: -if progress is not None and last+100 i: -progress.update(i/float(size)*100) +if progress is not None and last + 100 i: +progress.update(i / float(size) * 100) last = i # drop stuff with no versions (cruft) if pkg.has_versions: @@ -259,7 +259,8 @@ class Cache(object): def required_download(self): Get the size of the packages that are required to download. if self._records is None: -raise CacheClosedException(Cache object used after close() called) +raise CacheClosedException( +Cache object used after close() called) pm = apt_pkg.PackageManager(self._depcache) fetcher = apt_pkg.Acquire() pm.get_archives(fetcher, self._list, self._records) @@ -289,16 +290,14 @@ class Cache(object): # now check the result (this is the code from apt-get.cc)
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Tue, Dec 31, 2013 at 02:05:36PM +0100, Michael Schaller wrote: You can just git-send-email them to the mailing list or even push a git branch somewhere. Alrighty. Then let's stick to the bug for the time being as it contains already the context for the patches. See attached the patch with the PEP8 linter and pyflakes fixes. PEP8 only showed typical minor issues (indentation, line continuation, ...) and pyflakes only found an unused import and an unused variable. This looks good to me, thanks for doing this work. One minor note below: [..] diff --git a/debian/changelog b/debian/changelog index 52a1000..a7b037d 100644 --- a/debian/changelog +++ b/debian/changelog @@ -3,6 +3,9 @@ python-apt (0.9.2) UNRELEASED; urgency=low * apt/cache.py: - when using apt.Cache(rootdir=/some/dir) only read the APT configuration from this rootdir instead of /etc (closes: #728274) +- Fixed PEP8 and pyflakes issues + * apt/package.py: +- Fixed PEP8 and pyflakes issues Feel free to add yourself in the changelog, i.e. something like: + [ Michael Schaller ] + * apt/cache.py: +- Fixed PEP8 and pyflakes issues + * apt/package.py: +- Fixed PEP8 and pyflakes issues Cheers, Michael -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Tue, Dec 31, 2013 at 02:05:36PM +0100, Michael Schaller wrote: You can just git-send-email them to the mailing list or even push a git branch somewhere. Alrighty. Then let's stick to the bug for the time being as it contains already the context for the patches. See attached the patch with the PEP8 linter and pyflakes fixes. PEP8 only showed typical minor issues (indentation, line continuation, ...) and pyflakes only found an unused import and an unused variable. From 865b39a4f2ed566e7dd7fc13ff83ea26edbab000 Mon Sep 17 00:00:00 2001 From: Michael Schaller mich...@5challer.de Date: Tue, 31 Dec 2013 13:55:09 +0100 Subject: [PATCH] apt/cache.py, apt/package.py: Fixed PEP8 and pyflakes issues It would be great if you could explain what the pyflakes issues were in a separate paragraph of the commit message. Basically the second sentence of the previous paragraph. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Please do not top-post if possible. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
(Addressed to Michael Vogt, so don't wonder, other Michael...) On Tue, Dec 31, 2013 at 10:23:49PM +0100, Michael Vogt wrote: On Tue, Dec 31, 2013 at 02:05:36PM +0100, Michael Schaller wrote: You can just git-send-email them to the mailing list or even push a git branch somewhere. Alrighty. Then let's stick to the bug for the time being as it contains already the context for the patches. See attached the patch with the PEP8 linter and pyflakes fixes. PEP8 only showed typical minor issues (indentation, line continuation, ...) and pyflakes only found an unused import and an unused variable. This looks good to me, thanks for doing this work. One minor note below: [..] diff --git a/debian/changelog b/debian/changelog index 52a1000..a7b037d 100644 --- a/debian/changelog +++ b/debian/changelog @@ -3,6 +3,9 @@ python-apt (0.9.2) UNRELEASED; urgency=low * apt/cache.py: - when using apt.Cache(rootdir=/some/dir) only read the APT configuration from this rootdir instead of /etc (closes: #728274) +- Fixed PEP8 and pyflakes issues + * apt/package.py: +- Fixed PEP8 and pyflakes issues Feel free to add yourself in the changelog, i.e. something like: + [ Michael Schaller ] + * apt/cache.py: +- Fixed PEP8 and pyflakes issues + * apt/package.py: +- Fixed PEP8 and pyflakes issues Cheers, Michael I thought we wanted to use git-dch now (meaning after I released 0.9.1) for changelog manipulation. I did commit 56ed099558a9f6e7137a8c113ca9efb2b2c1a1d2 without a changelog entry for that reason, but I see that you did the commit afterwards with a changelog entry. It would also be cooler to have some more useful summary lines in your commits. The last one just had * apt/cache.py:. If you use debcommit, you can use debcommit -e to adjust the summary. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Please do not top-post if possible. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Tue, Dec 31, 2013 at 01:09:39PM +0100, Michael Schaller wrote: Hi Michael, Hi Michael, From my experience PEP8 and pyflakes are great tools with nearly no false positives - especially compared to pylint - and by now they are available for Python 2 and 3. Indeed, I tend to use this combo (pep8 and pyflakes) in my projects as well, works great! I personally like to add a lint command to setup.py to my projects. Maybe this would be beneficial for python-apt too. Please let me know if you would like to see a patch for that. Indeed, I attached (this time for real :) patches to include them as part of the tests. I like it this way as a CI system (like travis) can alert me if I accidentally break pep8/pyflakes with a commit. It also adds a .travis.yml file that runs the tests. Next step is to actually make the pep8/pylakes test to pass :) Cheers, Michael From f84aaa308f0f79d4a1407d8fd5cd849de8d50263 Mon Sep 17 00:00:00 2001 From: Michael Vogt m...@debian.org Date: Tue, 31 Dec 2013 10:06:22 +0100 Subject: [PATCH 1/4] make pep8 test part of the unittests --- tests/test_pep8.py | 20 1 file changed, 20 insertions(+) create mode 100755 tests/test_pep8.py diff --git a/tests/test_pep8.py b/tests/test_pep8.py new file mode 100755 index 000..f72a2ea --- /dev/null +++ b/tests/test_pep8.py @@ -0,0 +1,20 @@ +import os +import subprocess +import unittest + + +class PackagePep8TestCase(unittest.TestCase): + +def test_all_code(self): +res = 0 +py_dir = os.path.join(os.path.dirname(__file__), ..) +res += subprocess.call( +[pep8, + --ignore=E121,E123,E124,E125,E126,E127,E128, + --exclude, build,tests/old, + --repeat, py_dir]) +self.assertEqual(res, 0) + + +if __name__ == __main__: +unittest.main() -- 1.8.3.2 From e751078fb9a532a3e8ef411e88d95adbe05ea0a5 Mon Sep 17 00:00:00 2001 From: Michael Vogt m...@debian.org Date: Tue, 31 Dec 2013 21:36:26 +0100 Subject: [PATCH 2/4] add pyflakes test --- tests/test_pyflakes.py | 45 + 1 file changed, 45 insertions(+) create mode 100644 tests/test_pyflakes.py diff --git a/tests/test_pyflakes.py b/tests/test_pyflakes.py new file mode 100644 index 000..7404987 --- /dev/null +++ b/tests/test_pyflakes.py @@ -0,0 +1,45 @@ +#!/usr/bin/python3 +# -*- Mode: Python; indent-tabs-mode: nil; tab-width: 4; coding: utf-8 -*- + +# Partly based on a script from Review Board, MIT license; but modified to +# act as a unit test. + +from __future__ import print_function + +import os +import subprocess +import unittest + + +class TestPyflakesClean(unittest.TestCase): + +EXCLUDES = [build, tests/old] +TOPLEVEL = os.path.normpath( +os.path.join(os.path.dirname(os.path.abspath(__file__)), ..)) + +def is_excluded_path(self, path): +for exclude in self.EXCLUDES: +if path.startswith(os.path.join(self.TOPLEVEL, exclude)): +return True +return False + +def get_py_files(self, toplevel): +files = [] +for path, dirnames, filenames in os.walk(self.TOPLEVEL): +if self.is_excluded_path(path): +continue +for filename in filenames: +if os.path.splitext(filename)[1] == .py: +files.append(os.path.join(path, filename)) +return files + +def test_pyflakes_clean(self): +cmd = [pyflakes] + self.get_py_files(self.TOPLEVEL) +res = subprocess.call(cmd) +if res != 0: +self.fail(pyflakes failed with: %s % res) + +if __name__ == __main__: +import logging +logging.basicConfig(level=logging.DEBUG) +unittest.main() -- 1.8.3.2 From f8fb88aadc908fbabacc89169dc7dce197e71a51 Mon Sep 17 00:00:00 2001 From: Michael Vogt m...@debian.org Date: Tue, 31 Dec 2013 22:34:54 +0100 Subject: [PATCH 3/4] add new pep8,pyflakes dependencies --- debian/control | 4 +++- debian/tests/control | 2 +- po/python-apt.pot| 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/debian/control b/debian/control index 8c2f2ce..f4f5211 100644 --- a/debian/control +++ b/debian/control @@ -18,7 +18,9 @@ Build-Depends: apt (= 0.9.6), python-distutils-extra (= 2.0), python-sphinx (= 0.5), python-debian, - python-unittest2 + python-unittest2, + pep8, + pyflakes Vcs-Git: git://anonscm.debian.org/apt/python-apt.git Vcs-Browser: http://anonscm.debian.org/gitweb/?p=apt/python-apt.git XS-Testsuite: autopkgtest diff --git a/debian/tests/control b/debian/tests/control index 2ca0a40..bdca7d6 100644 --- a/debian/tests/control +++ b/debian/tests/control @@ -1,2 +1,2 @@ Tests: run-tests -Depends: @, apt-utils, python-debian, fakeroot, intltool +Depends: @, apt-utils, python-debian, fakeroot, intltool, pep8, pyflakes diff --git a/po/python-apt.pot b/po/python-apt.pot
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On 12/31/2013 10:23 PM, Michael Vogt wrote: Feel free to add yourself in the changelog, i.e. something like: + [ Michael Schaller ] Done. On 12/31/2013 11:00 PM, Julian Andres Klode wrote: It would be great if you could explain what the pyflakes issues were in a separate paragraph of the commit message. Basically the second sentence of the previous paragraph. Done. See the new patch attached. From 2d29cf5f820cd96b7776d08c05362a65acaa9e90 Mon Sep 17 00:00:00 2001 From: Michael Schaller mich...@5challer.de Date: Tue, 31 Dec 2013 23:22:32 +0100 Subject: [PATCH] * apt/cache.py: - Fixed PEP8 issues - Fixed pyflakes issue: Removed unused local variable 'transient' * apt/package.py: - Fixed PEP8 issues - Fixed pyflakes issue: Removed unused import 'warnings' --- apt/cache.py | 53 + apt/package.py | 40 +++- debian/changelog | 9 + 3 files changed, 53 insertions(+), 49 deletions(-) diff --git a/apt/cache.py b/apt/cache.py index 43fb55d..6b1e2bd 100644 --- a/apt/cache.py +++ b/apt/cache.py @@ -40,6 +40,7 @@ class FetchFailedException(IOError): class LockFailedException(IOError): Exception that is thrown when locking fails. + class CacheClosedException(Exception): Exception that is thrown when the cache is used after close(). @@ -53,7 +54,7 @@ class Cache(object): list of available packages. The cache can be used like a mapping from package names to Package -objects (although only getting items is supported). +objects (although only getting items is supported). Keyword arguments: progress -- a OpProgress object @@ -74,7 +75,7 @@ class Cache(object): self._fullnameset = set() self._changes_count = -1 self._sorted_set = None - + self.connect(cache_post_open, self._inc_changes_count) self.connect(cache_post_change, self._inc_changes_count) if memonly: @@ -86,17 +87,17 @@ class Cache(object): apt_pkg.config.clear(APT) apt_pkg.config.set(Dir, rootdir) apt_pkg.init_config() -if os.path.exists(rootdir+/etc/apt/apt.conf): +if os.path.exists(rootdir + /etc/apt/apt.conf): apt_pkg.read_config_file(apt_pkg.config, - rootdir + /etc/apt/apt.conf) -if os.path.isdir(rootdir+/etc/apt/apt.conf.d): + rootdir + /etc/apt/apt.conf) +if os.path.isdir(rootdir + /etc/apt/apt.conf.d): apt_pkg.read_config_dir(apt_pkg.config, - rootdir + /etc/apt/apt.conf.d) +rootdir + /etc/apt/apt.conf.d) apt_pkg.config.set(Dir::State::status, rootdir + /var/lib/dpkg/status) # also set dpkg to the rootdir path so that its called for the # --print-foreign-architectures call -apt_pkg.config.set(Dir::bin::dpkg, +apt_pkg.config.set(Dir::bin::dpkg, os.path.join(rootdir, usr, bin, dpkg)) # create required dirs/files when run with special rootdir # automatically @@ -105,7 +106,6 @@ class Cache(object): # recognized (LP: #320665) apt_pkg.init_system() self.open(progress) - def _inc_changes_count(self): Increase the number of changes @@ -118,12 +118,12 @@ class Cache(object): files = [/var/lib/dpkg/status, /etc/apt/sources.list, -] + ] dirs = [/var/lib/dpkg, /etc/apt/, /var/cache/apt/archives/partial, /var/lib/apt/lists/partial, - ] +] for d in dirs: if not os.path.exists(rootdir + d): #print creating: , rootdir + d @@ -165,8 +165,8 @@ class Cache(object): i = last = 0 size = len(self._cache.packages) for pkg in self._cache.packages: -if progress is not None and last+100 i: -progress.update(i/float(size)*100) +if progress is not None and last + 100 i: +progress.update(i / float(size) * 100) last = i # drop stuff with no versions (cruft) if pkg.has_versions: @@ -259,7 +259,8 @@ class Cache(object): def required_download(self): Get the size of the packages that are required to download. if self._records is None: -raise CacheClosedException(Cache object used after close() called) +raise CacheClosedException( +Cache object used after close() called) pm = apt_pkg.PackageManager(self._depcache) fetcher = apt_pkg.Acquire() pm.get_archives(fetcher,
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Tue, Dec 31, 2013 at 11:40:34PM +0100, Michael Schaller wrote: On 12/31/2013 10:23 PM, Michael Vogt wrote: Feel free to add yourself in the changelog, i.e. something like: + [ Michael Schaller ] Done. On 12/31/2013 11:00 PM, Julian Andres Klode wrote: It would be great if you could explain what the pyflakes issues were in a separate paragraph of the commit message. Basically the second sentence of the previous paragraph. Done. See the new patch attached. From 2d29cf5f820cd96b7776d08c05362a65acaa9e90 Mon Sep 17 00:00:00 2001 From: Michael Schaller mich...@5challer.de Date: Tue, 31 Dec 2013 23:22:32 +0100 Subject: [PATCH] It's still a git patch, so it should still have a summary line. git tools are becoming a bit useless otherwise. I did not write “remove the summary line”. * apt/cache.py: - Fixed PEP8 issues - Fixed pyflakes issue: Removed unused local variable 'transient' * apt/package.py: - Fixed PEP8 issues - Fixed pyflakes issue: Removed unused import 'warnings' And git patches usually have prose text. I know that the latest commit does not follow the usual git conventions, but I'm trying to get us completely adopt them, because it just looks unnatural otherwise. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Please do not top-post if possible. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Tue, Dec 31, 2013 at 11:06:15PM +0100, Michael Vogt wrote: From e751078fb9a532a3e8ef411e88d95adbe05ea0a5 Mon Sep 17 00:00:00 2001 From: Michael Vogt m...@debian.org Date: Tue, 31 Dec 2013 21:36:26 +0100 Subject: [PATCH 2/4] add pyflakes test --- tests/test_pyflakes.py | 45 + 1 file changed, 45 insertions(+) create mode 100644 tests/test_pyflakes.py diff --git a/tests/test_pyflakes.py b/tests/test_pyflakes.py new file mode 100644 index 000..7404987 --- /dev/null +++ b/tests/test_pyflakes.py @@ -0,0 +1,45 @@ +#!/usr/bin/python3 +# -*- Mode: Python; indent-tabs-mode: nil; tab-width: 4; coding: utf-8 -*- + +# Partly based on a script from Review Board, MIT license; but modified to +# act as a unit test. You really need to include the original copyright statement and the license here, otherwise you violate the license. If the original code does not include the license or a copyright statement above it, it might not be distributable at all, as the license clearly says: The *above* copyright notice and *this permission notice* shall be included in all copies or substantial portions of the Software. (I just added some emphasis) I'd even say that including the copyright notice *below* the license terms is wrong, as the license clearly says *above*. But not including them is definitely wrong. We don't want legal issues. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Please do not top-post if possible. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
On Sun, Dec 29, 2013 at 12:06:18PM +0100, Michael Schaller wrote: Package: python-apt Version: 0.9.2 Severity: wishlist Tags: patch This patch does far too many things that are not even remotely connected. From 0d295006a98769cd6151c78b3a078ad92d8047ee Mon Sep 17 00:00:00 2001 From: Michael Wisheu mich...@5challer.de Did you change your surname after writing the patch? Date: Sun, 29 Dec 2013 11:57:19 +0100 Subject: [PATCH] * apt/cache.py: - Fixed PEP8 linter and pyflakes issues - Added 'InstalledFilter' to get a filtered cache that only contains the currently installed packages. * apt/packages.py: - Fixed PEP8 linter issues The PEP8 stuff should be separate. The InstalledFilter should be separate - Removed special handling of 'collections' import as all supported distributions have Python 2.6 or newer by now. This should be separate - Replaced faulty 'BaseDependency.__dstr' with easier to read compat code. This should be separate - Added new properties to 'Dependency' and 'BaseDependency' to get the target package versions that could satisfy a dependency. Only this should be the patch. -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. Please do not top-post if possible. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Package: python-apt Version: 0.9.2 Severity: wishlist Tags: patch From 0d295006a98769cd6151c78b3a078ad92d8047ee Mon Sep 17 00:00:00 2001 From: Michael Wisheu mich...@5challer.de Date: Sun, 29 Dec 2013 11:57:19 +0100 Subject: [PATCH] * apt/cache.py: - Fixed PEP8 linter and pyflakes issues - Added 'InstalledFilter' to get a filtered cache that only contains the currently installed packages. * apt/packages.py: - Fixed PEP8 linter issues - Removed special handling of 'collections' import as all supported distributions have Python 2.6 or newer by now. - Replaced faulty 'BaseDependency.__dstr' with easier to read compat code. - Added new properties to 'Dependency' and 'BaseDependency' to get the target package versions that could satisfy a dependency. --- apt/cache.py | 52 ++- apt/package.py | 279 --- debian/changelog | 16 +++- 3 files changed, 268 insertions(+), 79 deletions(-) diff --git a/apt/cache.py b/apt/cache.py index 43fb55d..897c2be 100644 --- a/apt/cache.py +++ b/apt/cache.py @@ -40,6 +40,7 @@ class FetchFailedException(IOError): class LockFailedException(IOError): Exception that is thrown when locking fails. + class CacheClosedException(Exception): Exception that is thrown when the cache is used after close(). @@ -53,7 +54,7 @@ class Cache(object): list of available packages. The cache can be used like a mapping from package names to Package -objects (although only getting items is supported). +objects (although only getting items is supported). Keyword arguments: progress -- a OpProgress object @@ -74,7 +75,7 @@ class Cache(object): self._fullnameset = set() self._changes_count = -1 self._sorted_set = None - + self.connect(cache_post_open, self._inc_changes_count) self.connect(cache_post_change, self._inc_changes_count) if memonly: @@ -86,17 +87,17 @@ class Cache(object): apt_pkg.config.clear(APT) apt_pkg.config.set(Dir, rootdir) apt_pkg.init_config() -if os.path.exists(rootdir+/etc/apt/apt.conf): +if os.path.exists(rootdir + /etc/apt/apt.conf): apt_pkg.read_config_file(apt_pkg.config, rootdir + /etc/apt/apt.conf) -if os.path.isdir(rootdir+/etc/apt/apt.conf.d): +if os.path.isdir(rootdir + /etc/apt/apt.conf.d): apt_pkg.read_config_dir(apt_pkg.config, rootdir + /etc/apt/apt.conf.d) apt_pkg.config.set(Dir::State::status, rootdir + /var/lib/dpkg/status) # also set dpkg to the rootdir path so that its called for the # --print-foreign-architectures call -apt_pkg.config.set(Dir::bin::dpkg, +apt_pkg.config.set(Dir::bin::dpkg, os.path.join(rootdir, usr, bin, dpkg)) # create required dirs/files when run with special rootdir # automatically @@ -105,7 +106,6 @@ class Cache(object): # recognized (LP: #320665) apt_pkg.init_system() self.open(progress) - def _inc_changes_count(self): Increase the number of changes @@ -165,8 +165,8 @@ class Cache(object): i = last = 0 size = len(self._cache.packages) for pkg in self._cache.packages: -if progress is not None and last+100 i: -progress.update(i/float(size)*100) +if progress is not None and last + 100 i: +progress.update(i / float(size) * 100) last = i # drop stuff with no versions (cruft) if pkg.has_versions: @@ -289,16 +289,14 @@ class Cache(object): # now check the result (this is the code from apt-get.cc) failed = False -transient = False err_msg = for item in fetcher.items: if item.status == item.STAT_DONE: continue if item.STAT_IDLE: -transient = True continue err_msg += Failed to fetch %s %s\n % (item.desc_uri, - item.error_text) +item.error_text) failed = True # we raise a exception if the download failed or it was cancelt @@ -349,7 +347,6 @@ class Cache(object): if fetcher is None: fetcher = apt_pkg.Acquire(progress) - return self._fetch_archives(fetcher, apt_pkg.PackageManager(self._depcache)) @@ -362,12 +359,12 @@ class Cache(object): else: return bool(pkg.has_provides and not pkg.has_versions) -def get_providing_packages(self, pkgname, candidate_only=True, +def
Bug#733489: python-apt: Improve 'Dependency' and 'BaseDependency' to get target package versions that satisfy dependencies
Hi everyone, This is a feature request bug to improve the 'apt.package.Dependency' and 'apt.package.BaseDependency' classes in 'python-apt'. The current implementation of both classes is rather scarce and so I've refactored them to to include better properties. Most notably the information from 'apt_pkg.Dependency.all_targets()' is now available as a property on both classes. While I was at it I've fixed PEP8 and pyflakes issues, fixed bugs and got rid of obsolete code. Please review the patch carefully and let me know what you think. Best, Michael Schaller