Merge lp://staging/~barry/ubuntu-system-image/lp1444347 into lp://staging/ubuntu-system-image/server
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 271 | ||||
Proposed branch: | lp://staging/~barry/ubuntu-system-image/lp1444347 | ||||
Merge into: | lp://staging/ubuntu-system-image/server | ||||
Diff against target: |
433 lines (+254/-63) 2 files modified
lib/systemimage/diff.py (+89/-51) lib/systemimage/tests/test_diff.py (+165/-12) |
||||
To merge this branch: | bzr merge lp://staging/~barry/ubuntu-system-image/lp1444347 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Steve Langasek (community) | Needs Fixing | ||
Caio Begotti (community) | Approve | ||
Stéphane Graber | Pending | ||
Registry Administrators | Pending | ||
Review via email: mp+260597@code.staging.launchpad.net |
Description of the change
Addresses LP: #1444347 by ensuring that any hardlinks which at first appear not to change between the source and target, but which their underlying link target *does* change, gets properly modified in the diff. This handles the following cases (among others):
source:
a (regular file)
b -> a (via hardlink)
target
a (regular file, modified)
b -> a (via hardlink)
If the diff doesn't not also include b then after application of the diff, b will point to a's *old* inode, but not the "new" a. This branch ensures that in this case, b also gets modified by the diff so that it will point to a's new inode.
Coincidentally, this also fixes a related previously broken case:
source:
a (regular file)
b -> a (via hardlink)
target:
b (regular file)
a -> b (via hardlink)
In this case, both a and b need to be modified in the diff.
This branch includes a few other drive-by improvements.
* Rather than use hard to read nested indexes to get at the bits of information, we use a namedtuple for the internal data structures, which allow us to use easy to read attribute access to ensure we're getting at the bits and pieces we intend to get to.
* Along those lines, this actually fixes some broken indexing in the big "switched hardlinks" conditional. E.g. devminor wasn't being checked previously, nor was gid. Those are now being checked.
* Lots of comments :)
* A few minor code style improvements.
One more important note as you're reviewing this branch. There is a change in an existing test, but I believe it is legitimate. In the original test, the hardlink c/h -> d did not get modified, but this is exactly the case we're trying to fix above. In the test, d does change but the hardlink to d (i.e. c/h) wasn't being changed. That means that after the delta, c/h would point to the old d inode, and we want it to point to the new d.
Looks alright given the bug info, even though I can't reproduce or test it in a device myself :-)