Control: tags -1 + patch On Fri, 2015-12-04 at 18:09 +0000, Adam D. Barratt wrote: > On Sat, 2011-11-26 at 20:38 +0000, Adam D. Barratt wrote: > > hint's automagic binary to source mapping has unintended and undesired > > side-effects in some circumstances. The situation where issues have > > been noticed most is with "hint clean" and renamed source packages, but > > it will likely manifest in other cases. > [...] > > I haven't found an easy way of resolving this yet, so am filing this in > > the BTS so that the issue is documented and in case anyone else has any > > thoughts. > > After another of my periodic pokes at this, the main problem is that I > can't see an easy way of fixing it without bending the encapsulation. > > "clean" uses the HintFileParagraph class to represent the lines it has > read from the hint file, which in turn instantiates a number of Hint > objects. The Hint class's constructor then parses the package list, > including performing the binary to source mappings. By this point, the > knowledge of which subcommand of "hint" was used is well and truly > invisible. > > We could extend the Hint class to take keyword arguments so that the > package mapping could be disabled when required, but this would then > somehow need to be communicated via HintFileParagraph, which feels a > little dirty. > > Admittedly, the HintFileParagraph is only used by the clean action, so > we could simply say that the class will not modify its input, and thus > have it pass a "don't do binary to source mappings" flag to the Hint > class. > > Thoughts welcome, before I either decide that the above is a silly idea, > or just do it. :-)
fwiw, here's a patch that does that. The second "don't update" hunk also changes the behaviour in another way - currently, if you have a hint for foo and foo is then removed from unstable, a subsequent "hint clean" will drop the hint; after my update, that will no longer happen. However, I think this is actually better, for two reasons: - currently "easy foo" would simply be replaced by "# No packages left in hint", which isn't particularly helpful - "easy foo bar" will be replaced by "easy bar", with no indication that the hint has been changed. I don't think "clean" should make changes like that. Regards, Adam
diff --git a/scripts/hint b/scripts/hint index 9d38274..3b3b1ee 100755 --- a/scripts/hint +++ b/scripts/hint @@ -513,9 +513,11 @@ class Hint(object): ## - def __init__(self, hintname, *args): + def __init__(self, hintname, *args, **kwargs): starts_with_letter = re.compile(r'(?i)^[a-z]') + self.kwargs = kwargs + self.name = hintname self.pkgs = [] @@ -562,7 +564,8 @@ class Hint(object): suite = self.SOURCE_DISTRIBUTION except AttributeError: suite = 'unstable' - if not projectb.is_source_pkg(suite, src): + if not projectb.is_source_pkg(suite, src) and \ + ('noupdate' not in kwargs or kwargs['noupdate'] != 1): tmpsrc = src src = projectb.get_source_pkg(suite, src) if src is None: @@ -634,7 +637,7 @@ class Hint(object): words = [ ] return ' '.join(words) - def __new__(cls, hintname, *args): + def __new__(cls, hintname, *args, **kwargs): if hintname in \ [ 'easy', 'hint', 'force', 'force-hint', 'unblock', 'urgent', 'unblock-udeb' ]: subclass = UnstableToTestingHint @@ -685,8 +688,8 @@ class UnversionedHint(Hint): class MigrationHint(Hint): """A hint whose packages are meant to move from one distribution to another.""" - def __init__(self, *args): - Hint.__init__(self, *args) + def __init__(self, *args, **kwargs): + Hint.__init__(self, *args, **kwargs) self._versions = None @property @@ -699,12 +702,13 @@ class MigrationHint(Hint): 'target': get_version_numbers(self.TARGET_DISTRIBUTION, self.pkgs), } - for pkg, ver in self._versions['source'].items(): - if ver is None and pkg not in self.remove_pkgs: - print >>sys.stderr, ( - 'W: package %s not in %s, skipping.' - % (pkg, self.SOURCE_DISTRIBUTION)) - self.pkgs.remove(pkg) + if 'noupdate' not in self.kwargs or self.kwargs['noupdate'] != 1: + for pkg, ver in self._versions['source'].items(): + if ver is None and pkg not in self.remove_pkgs: + print >>sys.stderr, ( + 'W: package %s not in %s, skipping.' + % (pkg, self.SOURCE_DISTRIBUTION)) + self.pkgs.remove(pkg) return self._versions @@ -783,8 +787,8 @@ class TpuToTestingHint(MigrationHint): class AgeDaysHint(UnstableToTestingHint): - def __init__(self, hintname, days, *args): - UnstableToTestingHint.__init__(self, '%s %s' % (hintname, days), *args) + def __init__(self, hintname, days, *args, **kwargs): + UnstableToTestingHint.__init__(self, '%s %s' % (hintname, days), *args, **kwargs) class RemovalHint(Hint): @@ -865,7 +869,7 @@ class HintfileParagraph(object): else: if seen_hint and split_out: self._groups.append([]) - hint = Hint(*line.strip().split()) + hint = Hint(*line.strip().split(), noupdate=1) self._groups[-1].append(hint) seen_hint = True