Merge lp:~nacc/extract-changelogs/changelog-components into lp:extract-changelogs

[Nish Aravamudan]

Reorganize c.u.c for sources in main and binaries in universe

Currently, for packages such as qemu, `apt-get changelog qemu` fails because the source is in main but the binary is in universe. `apt` uses the binary package to create the URI, but c.u.c uses the source for extracting the changelog files.

This may not be accetable for merging yet, but I'd like a review.

LP: #1672555

[Nish Aravamudan (nacc)]

Note that also I do recognize there is already the binary/ namespace that solves this problem too. `apt` does not use it, though, and I think the concern is while it could be changed to do so (I have tested this locally while investigating the code), it would not work with another mirror of c.u.c that did not have binary/.

[Brian Murray (brian-murray)]

What concerns do you have that leads you to say "this may not be acceptable for merging yet"?

The log messages for "removing broken symlink" are not distinct between binary or component symlinks, while the creation messages are. That seems worth fixing to me.

Thanks for working on this!

review: Needs Fixing

[Nish Aravamudan (nacc)]

Hi Brian,

My primary concern is making sure I got the B-C right.

My thinking is:

A) old apt + new layout = will still use pool/component/

B) new apt + new layout = will use pool/

C) old apt + old layout = will still work as it is unchanged

But I think we do have to worry about:

D) new apt + old layout = will look in pool/ and not find it and thus break.

at least initially until we do a pass through all the changelogs and update them with links?

I don't know enough about the various use-cases for apt -- but as I write this, I'm realizing it shouldn't matter yet as there is only the "old apt" case, which should be fine with browsing to a real file or to a symlink in pool/component.

I'm just being overly cautious with new code that is very visible :)

I will fixup the log messages for sure, thanks for catching that!

Unmerged revisions

53. By Nish Aravamudan on 2017-03-14

_create_{binary,component}_symlinks: specify type of broken symlink to remove

52. By Nish Aravamudan on 2017-03-14

Move pool/ changelogs into the root directory, symlink in component subdirs

If a srcpkg is in main but some binaries are in universe, `apt changelog
$binpkg` will fail because `apt` has the component in the URL and the
component comes from the binary package, but the changelog is stored
according to the source package. There isn't really any reason for the
component to be in the URI, but we need to maintain
backwards-compatibility.

For new changelogs, elide the component from the URI for the actual
file. Make symlinks in the old component/ path for compatibility.

LP: #1672555

51. By Nish Aravamudan on 2017-03-13

_create_binary_symlinks: create symlinks regardless of existing changelog

r45 change the behavior in attempting to fix the code, by creating the
binary/ symlinks even if the target changelog already exists. However,
it removed the invocation of (what is now) _create_binary_symlinks that
occurs when the target changelog is initially created.

50. By Nish Aravamudan on 2017-03-13

create_binary_symlinks: log symlinks correctly

Right now, the symlink deletion and creation logs appear to be reversed
(referring to the target not to the link).

=== modified file 'lp-extract-changelogs.py'
--- lp-extract-changelogs.py	2016-04-13 23:40:29 +0000
+++ lp-extract-changelogs.py	2017-03-14 19:07:44 +0000
@@ -131,7 +131,8 @@
         # statistics
         self.skipped = 0
         self.extracted = 0
-        self.symlinked = 0
+        self.binary_symlinked = 0
+        self.component_symlinked = 0
         # date
         self._time_of_last_check = 0
         if os.path.exists(self.LAST_CHECK_FILE):
@@ -234,8 +235,7 @@
         if os.path.exists(os.path.join(dest, "changelog")):
             logging.debug("skipping already existing '%s'" % dest)
             self.skipped += 1
-            if self._create_binary_symlinks(s, dest):
-                self.symlinked += 1
+            self._create_symlinks(s, dest)
             return False
 
         # fetch/unpack
@@ -272,6 +272,7 @@
                           (s.srcname, s.srcversion))
             return False
 
+        self._create_symlinks(s, dest)
         # remove unpack dir
         shutil.rmtree(tmpdir)
         return True
@@ -291,12 +292,11 @@
                 continue
 
             # main extraction
-            dest = "%s/pool/%s/%s/%s/%s_%s" % (self.targetdir,
-                                               s.srccomponent,
-                                               poolhash(s.srcname),
-                                               s.srcname,
-                                               s.srcname,
-                                               s.srcversion)
+            dest = "%s/pool/%s/%s/%s_%s" % (self.targetdir,
+                                            poolhash(s.srcname),
+                                            s.srcname,
+                                            s.srcname,
+                                            s.srcversion)
 
             if not self._unpack_changelogs_to_target(s, dest):
                 continue
@@ -318,15 +318,43 @@
             if not os.path.exists(lnk):
                 # broken symlink, get rid of them
                 if os.path.islink(lnk):
-                    logging.debug("removing broken symlink '%s'" % dest)
-                    os.remove(lnk)
-                logging.debug("create compat symlink '%s' -> '%s'" %
-                              (dest, lnk))
-                if not os.path.exists(lnkdir):
-                    os.makedirs(lnkdir)
-                os.symlink(os.path.abspath(dest), lnk)
-                linked = True
-        return linked
+                    logging.debug("removing broken binary symlink '%s'" % lnk)
+                    os.remove(lnk)
+                logging.debug("create compat binary symlink '%s' -> '%s'" %
+                              (lnk, dest))
+                if not os.path.exists(lnkdir):
+                    os.makedirs(lnkdir)
+                os.symlink(os.path.abspath(dest), lnk)
+                linked = True
+        return linked
+
+    def _create_component_symlinks(self, s, dest):
+        linked = False
+        for comp in set(b[2] for b in
+                        s.binary_packages_versions_components):
+            lnkdir = "%s/pool/%s/%s/%s" % (self.targetdir,
+                                           comp,
+                                           poolhash(s.srcname),
+                                           s.srcname)
+            lnk = "%s/%s_%s" % (lnkdir, s.srcname, s.srcversion)
+            if not os.path.exists(lnk):
+                # broken symlink, get rid of them
+                if os.path.islink(lnk):
+                    logging.debug("removing broken component symlink '%s'" % lnk)
+                    os.remove(lnk)
+                logging.debug("create compat component symlink '%s' -> '%s'" %
+                              (lnk, dest))
+                if not os.path.exists(lnkdir):
+                    os.makedirs(lnkdir)
+                os.symlink(os.path.abspath(dest), lnk)
+                linked = True
+        return linked
+
+    def _create_symlinks(self, s, dest):
+        if self._create_binary_symlinks(s, dest):
+            self.binary_symlinked += 1
+        if self._create_component_symlinks(s, dest):
+            self.component_symlinked += 1
 
     def write_last_check_date(self):
         with open(self.LAST_CHECK_FILE, "w+") as fd:
@@ -352,8 +380,8 @@
     if c.get_changelogs():
         c.write_last_check_date()
     os.close(lock)
-    logging.info("skipped: %i, extracted: %i, symlinked: %i" % \
-                 (c.skipped, c.extracted, c.symlinked))
+    logging.info("skipped: %i, extracted: %i, binary symlinked: %i, compoment_symlinked: %i" % \
+                 (c.skipped, c.extracted, c.binary_symlinked, c.component_symlinked))
 
     # test code
     #ubuntu = c._launchpad.distributions[DISTRIBUTION]