Merge lp:~sjakthol/software-properties/fix-1037916 into lp:software-properties

[Sami Jaktholm]

Construct paths for new sources list entries properly. This caused some weird behavior together with bug 1042916 in python-apt when adding a PPA that already had a file with entries present in the system.

Basically python-apt failed to match the new PPA to the existing one and apt-add-repository provided it with a confusing path to save the PPA to (the path had double-slash in it, for example '/etc/apt/sourceslist.d//ppa.list'). When python-apt saved the sources entries, it opened the same file twice: the first for the old entry with proper path and the second for the new entry. And when the changes were flushed at closing time the contents of the one closed first would be overridden by the contents of the one closed second.

This caused all kind of weird results to be written in the sources list file. Some results can be seen in bugs 972617, 994515 1018327 and 1037916.

Although this particular problem will be gone once the fix for bug 1042916 is released I think it's better to fix the other half of the problem too. This branch uses os.path.join to make sure the path is constructed properly.

[Colin Watson (cjwatson)]

Thanks for the explanation. Using os.path.join does seem reasonable.
However, a style point: using "from os.path import join" inside a
function isn't very PEP-8-ish - you only need to do that kind of thing
in the case where the import must be deferred until the function is
first called (usually only for circular or conditional imports).

I suggest instead:

 * 'import os' at the top of the file
 * call os.path.join rather than join

Could you make and test that change and I'll be happy to sponsor it?

[Colin Watson (cjwatson)]

review: Needs Fixing

[lp:~sjakthol/software-properties/fix-1037916]

800. By Sami Jaktholm on 2012-09-17

Fix coding style

[Sami Jaktholm (sjakthol)]

Fixed.

review: Needs Resubmitting

[Colin Watson (cjwatson)]

Thanks!

review: Approve

=== modified file 'softwareproperties/ppa.py'
--- softwareproperties/ppa.py	2012-09-07 20:08:44 +0000
+++ softwareproperties/ppa.py	2012-09-17 13:15:24 +0000
@@ -26,6 +26,7 @@
 import re
 import subprocess
 from threading import Thread
+import os
 
 try:
   import urllib.request
@@ -74,8 +75,8 @@
     sourceslistd = apt_pkg.config.find_dir("Dir::Etc::sourceparts")
     line = "deb http://ppa.launchpad.net/%s/%s/ubuntu %s main" % (
         ppa_owner, ppa_name, distro_codename)
-    filename = "%s/%s-%s-%s.list" % (
-        sourceslistd, encode(ppa_owner), encode(ppa_name), distro_codename)
+    filename = os.path.join(sourceslistd, "%s-%s-%s.list" % (
+        encode(ppa_owner), encode(ppa_name), distro_codename))
     return (line, filename)
 
 def get_ppa_info_from_lp(owner_name, ppa_name):