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

Proposed by Andrea Corbellini on 2012-12-14
Status: Merged
Merged at revision: 825
Proposed branch: lp:~andrea.corbellini/software-properties/fix-621977-1
Merge into: lp:software-properties
Diff against target: 130 lines (+41/-9)
3 files modified
add-apt-repository (+4/-1)
softwareproperties/SoftwareProperties.py (+33/-5)
tests/test_dbus.py (+4/-3)
To merge this branch: bzr merge lp:~andrea.corbellini/software-properties/fix-621977-1
Reviewer Review Type Date Requested Status
Michael Terry 2012-12-14 Approve on 2013-01-24
Review via email: mp+139919@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Michael Terry (mterry) wrote :

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

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'add-apt-repository'
2--- add-apt-repository 2012-11-09 12:21:58 +0000
3+++ add-apt-repository 2012-12-14 15:05:45 +0000
4@@ -98,6 +98,9 @@
5 parser.add_option("-k", "--keyserver",
6 dest="keyserver", default=DEFAULT_KEYSERVER,
7 help=_("URL of keyserver. Default: %default"))
8+ parser.add_option("-s", "--enable-source", action="store_true",
9+ dest="enable_source", default=False,
10+ help=_("Allow downloading of the source packages from the repository"))
11 parser.add_option("-y", "--yes", action="store_true",
12 dest="assume_yes", default=False,
13 help=_("Assume yes to all queries"))
14@@ -198,7 +201,7 @@
15 print(_("Error: '%s' doesn't exist in a sourcelist file") % debsrc_line)
16
17 else:
18- if not sp.add_source_from_line(line):
19+ if not sp.add_source_from_line(line, options.enable_source):
20 print(_("Error: '%s' invalid") % line)
21 sys.exit(1)
22 sp.sourceslist.save()
23
24=== modified file 'softwareproperties/SoftwareProperties.py'
25--- softwareproperties/SoftwareProperties.py 2012-10-01 09:24:55 +0000
26+++ softwareproperties/SoftwareProperties.py 2012-12-14 15:05:45 +0000
27@@ -26,6 +26,7 @@
28
29 import apt
30 import apt_pkg
31+import copy
32 from hashlib import md5
33 import re
34 import os
35@@ -696,14 +697,15 @@
36 line = "deb %s %s %s" % ( repo, self.distro.codename, areas )
37 return line
38
39- def add_source_from_line(self, line):
40+ def add_source_from_line(self, line, enable_source_code=False):
41 """
42 Add a source with the given apt line and auto-add
43 signing key if we have it in the whitelist
44 """
45 (deb_line, file) = expand_ppa_line(line.strip(), self.distro.codename)
46 deb_line = self.expand_http_line(deb_line)
47- debsrc_line = 'deb-src' + deb_line[3:]
48+ debsrc_entry_type = 'deb-src' if enable_source_code else '# deb-src'
49+ debsrc_line = debsrc_entry_type + deb_line[3:]
50 new_deb_entry = SourceEntry(deb_line, file)
51 new_debsrc_entry = SourceEntry(debsrc_line, file)
52 if new_deb_entry.invalid or new_debsrc_entry.invalid:
53@@ -716,7 +718,7 @@
54 comment=new_deb_entry.comment,
55 file=new_deb_entry.file,
56 architectures=new_deb_entry.architectures)
57- self.sourceslist.add(new_debsrc_entry.type,
58+ self.sourceslist.add(debsrc_entry_type,
59 new_debsrc_entry.uri,
60 new_debsrc_entry.dist,
61 new_debsrc_entry.comps,
62@@ -729,18 +731,44 @@
63 worker.join(30)
64 return True
65
66- def remove_source(self, source):
67+ def remove_source(self, source, remove_source_code=True):
68 """Remove the given source"""
69+ if remove_source_code:
70+ if isinstance(source, str):
71+ # recall this method giving a SourceEntry as an argument
72+ source = self._find_source_from_string(source)
73+ self.remove_source(source, True)
74+ elif source is not None:
75+ self.remove_source(source, False)
76+ # remove the deb-src lines (enabled or not) associated to
77+ # this source entry
78+ source = copy.copy(source)
79+ source.type = 'deb-src'
80+ source.disabled = True
81+ self.remove_source(source, False)
82+ source.disabled = False
83+ self.remove_source(source, False)
84+ return
85+
86 # first find the source object if we got a string
87 if isinstance(source, str):
88 source = self._find_source_from_string(source)
89+ if source is None:
90+ return
91 # if its a sources.list.d file and it contains only a single line
92 # (the line that we just remove), then all references to that
93 # file are gone and the file is not saved. we work around that
94 # here
95 if source.file != apt_pkg.config.find_file("Dir::Etc::sourcelist"):
96 self.sourceslist.list.append(SourceEntry("", file=source.file))
97- self.sourceslist.remove(source)
98+ try:
99+ self.sourceslist.remove(source)
100+ except ValueError:
101+ # this exception is raised if trying to remove an entry that does
102+ # not exist. in this case we suppress the error because there's no
103+ # need to propagate it (the aim of this method is to ensure that
104+ # the given entries are not listed in the sourceslist)
105+ pass
106 self.set_modified_sourceslist()
107
108 def add_key(self, path):
109
110=== modified file 'tests/test_dbus.py'
111--- tests/test_dbus.py 2012-07-05 13:32:44 +0000
112+++ tests/test_dbus.py 2012-12-14 15:05:45 +0000
113@@ -243,13 +243,14 @@
114 with open("./aptroot/etc/apt/sources.list") as f:
115 sourceslist = f.read()
116 self.assertTrue(s in sourceslist)
117- # remove again, needs to remove both deb and deb-src lines
118+ self.assertTrue(s.replace("deb", "# deb-src") in sourceslist)
119+ # remove again
120 self.iface.RemoveSource(s)
121- self.iface.RemoveSource(s.replace("deb", "deb-src"))
122 with open("./aptroot/etc/apt/sources.list") as f:
123 sourceslist = f.read()
124 self.assertFalse(s in sourceslist)
125- # wait for the _on_modified_sources_list signal to arrive"
126+ self.assertFalse(s.replace("deb", "# deb-src") in sourceslist)
127+ # wait for the _on_modified_sources_list signal to arrive
128 self.loop.run()
129
130 def test_add_gpg_key(self):

Subscribers

People subscribed via source and target branches

to status/vote changes: