Merge lp:~andrea.corbellini/software-properties/fix-621977-1 into lp:software-properties

[Andrea Corbellini]

This is an updated version of my previous merge proposal:

  https://code.launchpad.net/~andrea.corbellini/software-properties/fix-621977/+merge/134815

This branch fixes bug #621977 in the following way:

1. deb-src lines are added, but commented out, so that Software Properties still lets you enable them later;

2. a new -s, --enable-source command line option is added so that you can add uncommented deb-src line with a single command;

Also:

3. when removing a source entry, all related deb-src lines are automatically removed.

I fixed the tests to work with the new code. test_dbus.test_updates_automation fails, but it also fails in trunk.

The point #3 (automatically remove deb-src entries) is required to do a "clean" job by default. Having deb-src lines without correspective deb lines makes sense, however it doesn't make much sense in most situations.

It'd be useful to expose via the DBus API the new parameters enable_source_code (of add_source_from_line) and remove_source_code (from remove_source). However, modifying the signature of the existing DBus methods will break backward compatibility. I suggest adding new methods for that. If you like the idea, I can create a bug task and work on it.

[Michael Terry (mterry)]

This was already approved before. Still looks good and works fine for me.

review: Approve

=== modified file 'add-apt-repository'
--- add-apt-repository	2012-11-09 12:21:58 +0000
+++ add-apt-repository	2012-12-14 15:05:45 +0000
@@ -98,6 +98,9 @@
     parser.add_option("-k", "--keyserver",
         dest="keyserver", default=DEFAULT_KEYSERVER,
         help=_("URL of keyserver. Default: %default"))
+    parser.add_option("-s", "--enable-source", action="store_true",
+        dest="enable_source", default=False,
+        help=_("Allow downloading of the source packages from the repository"))
     parser.add_option("-y", "--yes", action="store_true",
         dest="assume_yes", default=False,
         help=_("Assume yes to all queries"))
@@ -198,7 +201,7 @@
             print(_("Error: '%s' doesn't exist in a sourcelist file") % debsrc_line)
 
     else:
-        if not sp.add_source_from_line(line):
+        if not sp.add_source_from_line(line, options.enable_source):
             print(_("Error: '%s' invalid") % line)
             sys.exit(1)
         sp.sourceslist.save()
 
=== modified file 'softwareproperties/SoftwareProperties.py'
--- softwareproperties/SoftwareProperties.py	2012-10-01 09:24:55 +0000
+++ softwareproperties/SoftwareProperties.py	2012-12-14 15:05:45 +0000
@@ -26,6 +26,7 @@
 
 import apt
 import apt_pkg
+import copy
 from hashlib import md5
 import re
 import os
@@ -696,14 +697,15 @@
     line = "deb %s %s %s" % ( repo, self.distro.codename, areas )
     return line
 
-  def add_source_from_line(self, line):
+  def add_source_from_line(self, line, enable_source_code=False):
     """
     Add a source with the given apt line and auto-add
     signing key if we have it in the whitelist
     """
     (deb_line, file) = expand_ppa_line(line.strip(), self.distro.codename)
     deb_line = self.expand_http_line(deb_line)
-    debsrc_line = 'deb-src' + deb_line[3:]
+    debsrc_entry_type = 'deb-src' if enable_source_code else '# deb-src'
+    debsrc_line = debsrc_entry_type + deb_line[3:]
     new_deb_entry = SourceEntry(deb_line, file)
     new_debsrc_entry = SourceEntry(debsrc_line, file)
     if new_deb_entry.invalid or new_debsrc_entry.invalid:
@@ -716,7 +718,7 @@
                          comment=new_deb_entry.comment,
                          file=new_deb_entry.file,
                          architectures=new_deb_entry.architectures)
-    self.sourceslist.add(new_debsrc_entry.type,
+    self.sourceslist.add(debsrc_entry_type,
                          new_debsrc_entry.uri,
                          new_debsrc_entry.dist,
                          new_debsrc_entry.comps,
@@ -729,18 +731,44 @@
         worker.join(30)
     return True
 
-  def remove_source(self, source):
+  def remove_source(self, source, remove_source_code=True):
     """Remove the given source"""
+    if remove_source_code:
+      if isinstance(source, str):
+        # recall this method giving a SourceEntry as an argument
+        source = self._find_source_from_string(source)
+        self.remove_source(source, True)
+      elif source is not None:
+        self.remove_source(source, False)
+        # remove the deb-src lines (enabled or not) associated to
+        # this source entry
+        source = copy.copy(source)
+        source.type = 'deb-src'
+        source.disabled = True
+        self.remove_source(source, False)
+        source.disabled = False
+        self.remove_source(source, False)
+      return
+
     # first find the source object if we got a string
     if isinstance(source, str):
       source = self._find_source_from_string(source)
+    if source is None:
+      return
     # if its a sources.list.d file and it contains only a single line
     # (the line that we just remove), then all references to that
     # file are gone and the file is not saved. we work around that
     # here
     if source.file != apt_pkg.config.find_file("Dir::Etc::sourcelist"):
       self.sourceslist.list.append(SourceEntry("", file=source.file))
-    self.sourceslist.remove(source)
+    try:
+      self.sourceslist.remove(source)
+    except ValueError:
+      # this exception is raised if trying to remove an entry that does
+      # not exist. in this case we suppress the error because there's no
+      # need to propagate it (the aim of this method is to ensure that
+      # the given entries are not listed in the sourceslist)
+      pass
     self.set_modified_sourceslist()
 
   def add_key(self, path):
 
=== modified file 'tests/test_dbus.py'
--- tests/test_dbus.py	2012-07-05 13:32:44 +0000
+++ tests/test_dbus.py	2012-12-14 15:05:45 +0000
@@ -243,13 +243,14 @@
         with open("./aptroot/etc/apt/sources.list") as f:
             sourceslist = f.read()
             self.assertTrue(s in sourceslist)
-        # remove again, needs to remove both deb and deb-src lines
+            self.assertTrue(s.replace("deb", "# deb-src") in sourceslist)
+        # remove again
         self.iface.RemoveSource(s)
-        self.iface.RemoveSource(s.replace("deb", "deb-src"))
         with open("./aptroot/etc/apt/sources.list") as f:
             sourceslist = f.read()
             self.assertFalse(s in sourceslist)
-        # wait for the _on_modified_sources_list signal to arrive"
+            self.assertFalse(s.replace("deb", "# deb-src") in sourceslist)
+        # wait for the _on_modified_sources_list signal to arrive
         self.loop.run()
 
     def test_add_gpg_key(self):