| Message ID | 1465814502-14108-11-git-send-email-stephen.finucane@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
On 06/13/2016 05:41 AM, Stephen Finucane wrote: > Users can send a new revision of a single patch from a series. When > this happens there is no immediate context: one must grok the > previous series to understand that this new patch is in fact a minor > change to an existing series and not a new, standalone hanges. This is > particularly impactful for things like CI which _need_ to understand > this dependency trail in order to successfully apply and test a patch. > > Resolve this issue through the "auto-completion" of series. This is > achieved by examining previous series and taking only the patches that > have not be replaced in the current version. For example, if a user > sends a three patch series and then submits a revision to patch two, > only patches one and three of the prior series will be used and the > resulting group presented as an entirely new series. > > This is done entirely dynamically. The reason for this is that series > relationships themselves are dynamic: sometimes it may not be possible > to identify a relationship until the last patch of a new series is > submitted (say there's a three patch series and the first two parsed > patches are significantly reworked). > > This won't catch situations where the order of a patch has been moved > or the patch has been significantly reworked. However, one can hope > users are not silly enough to do something like that. > > Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> I'm not sure I'm understanding this properly. Here's how I tried to test this: 1) import this entire series with parsearchive.py 2) took this patch, and updated its headers to: - have a new "Message-Id" - make In-Reply-To and References point to the original 10/10 patch. 3) imported this new hand-crafted patch The result was the new patch being added to the same series revision. The way I read this commit message, I was expecting it to generate a new SeriesRevision. Am I missing something?
On 23 Jun 13:22, Andy Doan wrote: > On 06/13/2016 05:41 AM, Stephen Finucane wrote: > >Users can send a new revision of a single patch from a series. When > >this happens there is no immediate context: one must grok the > >previous series to understand that this new patch is in fact a minor > >change to an existing series and not a new, standalone hanges. This is > >particularly impactful for things like CI which _need_ to understand > >this dependency trail in order to successfully apply and test a patch. > > > >Resolve this issue through the "auto-completion" of series. This is > >achieved by examining previous series and taking only the patches that > >have not be replaced in the current version. For example, if a user > >sends a three patch series and then submits a revision to patch two, > >only patches one and three of the prior series will be used and the > >resulting group presented as an entirely new series. > > > >This is done entirely dynamically. The reason for this is that series > >relationships themselves are dynamic: sometimes it may not be possible > >to identify a relationship until the last patch of a new series is > >submitted (say there's a three patch series and the first two parsed > >patches are significantly reworked). > > > >This won't catch situations where the order of a patch has been moved > >or the patch has been significantly reworked. However, one can hope > >users are not silly enough to do something like that. > > > >Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> > > I'm not sure I'm understanding this properly. Here's how I tried to > test this: > > 1) import this entire series with parsearchive.py > 2) took this patch, and updated its headers to: > - have a new "Message-Id" > - make In-Reply-To and References point to the original 10/10 patch. > > 3) imported this new hand-crafted patch > > The result was the new patch being added to the same series > revision. The way I read this commit message, I was expecting it to > generate a new SeriesRevision. Am I missing something? So what this is supposed to do is use patches from previous revisions to complete the new revision. Say you have the following: [PATCH 1/2] Test [PATCH 2/2] Test123 [PATCH v2 2/2] Test123 This is a series, then a single patch from the new series. This should autocomplete and use v1 of [1/2] in place of the missing v2. However, like the previous patch it doesn't work that well and I'd like to spend more time on this. I'll drop for the next revision and address in a separate series.
diff --git a/patchwork/models.py b/patchwork/models.py index d2d4079..ce8ac05 100644 --- a/patchwork/models.py +++ b/patchwork/models.py @@ -250,19 +250,50 @@ class SeriesRevision(models.Model): def complete(self): return self.total == self.actual_total + @cached_property + def ancestors(self): + if self.group: + # NOTE(stephenfin) Under normal usage dates should be more + # than adequate. However, if patches were submitted in an + # unusual order, e.g. using parsearchive tool, then this + # can be invalid. This is a corner case that we + # unfortunately do not cover + return self.group.revisions.filter(date__lt=self.date) + + @cached_property + def ancestor(self): + if self.ancestors: + return self.ancestors.reverse()[0] + @property def patches(self): - """Return patches associated with this series . + """Build a complete list of patches. - Eventually this will "autofill" a series revision by pulling in - missing patches from prior revisions, where possible. For now, - however, this just lets us retrieve the patches created for - this given revision. - - Returns: - The patches in the revision. + This works by recursively searching through ancestor series to + "fill in the gaps". """ - return self.unique_patches.all() + if self.complete or not self.ancestor: + return self.unique_patches + + # TODO(stephenfin) We should compare on more than one field to + # identify duplicates. The 'name' field itself is fairly useless + # in the case of typos etc. + # TODO(stephenfin) This won't handle different version numbers! + # We need to strip the tags to ensure 'xxx' and '[v2] xxx' are + # identified as different versions of the same patch. This will + # fix series linking also + ids = self.unique_patches.values('name') + ancestor_patches = self.ancestor.patches + + # TODO(stephenfin) Is the below correct? + patches_ = ancestor_patches.exclude(name__in=ids) + return patches_ | self.unique_patches.all() + + @cached_property + def actual_version(self): + if not self.group: + return 1 + return self.ancestors.count() + 1 def __str__(self): return self.name
Users can send a new revision of a single patch from a series. When this happens there is no immediate context: one must grok the previous series to understand that this new patch is in fact a minor change to an existing series and not a new, standalone hanges. This is particularly impactful for things like CI which _need_ to understand this dependency trail in order to successfully apply and test a patch. Resolve this issue through the "auto-completion" of series. This is achieved by examining previous series and taking only the patches that have not be replaced in the current version. For example, if a user sends a three patch series and then submits a revision to patch two, only patches one and three of the prior series will be used and the resulting group presented as an entirely new series. This is done entirely dynamically. The reason for this is that series relationships themselves are dynamic: sometimes it may not be possible to identify a relationship until the last patch of a new series is submitted (say there's a three patch series and the first two parsed patches are significantly reworked). This won't catch situations where the order of a patch has been moved or the patch has been significantly reworked. However, one can hope users are not silly enough to do something like that. Signed-off-by: Stephen Finucane <stephen.finucane@intel.com> --- patchwork/models.py | 49 ++++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 40 insertions(+), 9 deletions(-)