Merge lp:~rom1-chal/bzr-builder/reporting_conflict_from_merge into lp:bzr-builder

Proposed by Romain Chalumeau
Status: Work in progress
Proposed branch: lp:~rom1-chal/bzr-builder/reporting_conflict_from_merge
Merge into: lp:bzr-builder
Diff against target: 125 lines (+69/-5)
2 files modified
recipe.py (+64/-2)
tests/test_recipe.py (+5/-3)
To merge this branch: bzr merge lp:~rom1-chal/bzr-builder/reporting_conflict_from_merge
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) code Needs Fixing
James Westby Pending
Review via email: mp+30127@code.launchpad.net

Description of the change

Hi,

I am release manager for a company with many developers.
I massively use recipes to manage my integration plans of feature branches.

My concern is about reporting on merges producing conflicts.

As a release mgr, I have to know :
 - which feature branch has introduced the conflict
 - and who are the developers that are concerned by the conflicted modifications

To do so, i have written the ConflictsFromMerge exception which can be raised by merge_branch (instead of BzrCommandError).
The exception gets in parameter the ChildBranch.url which has stopped the build.
To detect the developers, I parse conflicted parts on the annotated conflicted files.

Very useful for my integration team and myself.

The tests "test_build_tree_merged_with_conflicts" and "test_pull_or_branch_pull_with_conflicts" have been updated to take into account the

Rgds/Romain

To post a comment you must log in.
Revision history for this message
James Westby (james-w) wrote :

Hi,

Thanks for this, it looks great.

I have a few formatting things that I would like to clean up when I merge,
but they are minor.

The thing that is stopping me doing it right now is that I'm not sure how
generally applicable the output is. I'm trying to think of things that would
both be widely useful, and still give you the information that you need.

Perhaps not showing the developers, but the revisions (like you would get from
bzr log) for each revision involved in the conflict would work?

I'm going to ponder this over the weekend, and we can get it landed next week.

Thanks,

James

Revision history for this message
Romain Chalumeau (rom1-chal) wrote :

Le vendredi 16 juillet 2010 à 20:10 +0000, James Westby a écrit :
> Hi,
>
Hi James

> Thanks for this, it looks great.

Thx
>
> I have a few formatting things that I would like to clean up when I merge,
> but they are minor.
>
> The thing that is stopping me doing it right now is that I'm not sure how
> generally applicable the output is. I'm trying to think of things that would
> both be widely useful, and still give you the information that you need.
>
> Perhaps not showing the developers, but the revisions (like you would get from
> bzr log) for each revision involved in the conflict would work?
>

I agree. So far, it is a helper for my integration team.
To explain the context, the development teams wants to go towards
continuous integration (of course in a single branch).
As a release manager, i want feature branches.

The bzr-builder helped making coexist the two methods.
I use daily crons based on recipes to build the integration or release
branch and send it to hudson.
With conflicts, I catch the merge exceptions and put the related msgs in
emails sent only to the targetted developers. It helps me avoiding to
spam a mailing where the emails are not read anymore. That's the reason
why i need the developers concerned by a conflict.

I agree that returning the conflicted revisions objects would be more
flexible to generate custom msg or filter the information.

Yet, the other piece of information i need to communicate is the
branches that are concerned by the conflict so that the developers can
easily reproduce the conflict locally. I did not find a "sexy" way of
listing the branches from a shared repository that contain such or such
revision. As the recipe contains the urls, i add it to the exception
msg.

> I'm going to ponder this over the weekend, and we can get it landed next week.
>
> Thanks,

Thx to you
Romain
>
> James
>
>
>

--

Revision history for this message
James Westby (james-w) wrote :

On Mon, 19 Jul 2010 08:03:58 -0000, Romain Chalumeau <email address hidden> wrote:
> I agree. So far, it is a helper for my integration team.
> To explain the context, the development teams wants to go towards
> continuous integration (of course in a single branch).
> As a release manager, i want feature branches.
>
> The bzr-builder helped making coexist the two methods.
> I use daily crons based on recipes to build the integration or release
> branch and send it to hudson.
> With conflicts, I catch the merge exceptions and put the related msgs in
> emails sent only to the targetted developers. It helps me avoiding to
> spam a mailing where the emails are not read anymore. That's the reason
> why i need the developers concerned by a conflict.

Interesting approach.

> I agree that returning the conflicted revisions objects would be more
> flexible to generate custom msg or filter the information.
>
> Yet, the other piece of information i need to communicate is the
> branches that are concerned by the conflict so that the developers can
> easily reproduce the conflict locally. I did not find a "sexy" way of
> listing the branches from a shared repository that contain such or such
> revision. As the recipe contains the urls, i add it to the exception
> msg.

I'm perfectly happy to have the urls in there, it's just including the
list of developers that I find a little specific.

Thanks,

James

Revision history for this message
Romain Chalumeau (rom1-chal) wrote :

OK, i understand.

What do you think of the approach of getting in the exception an array
of revision objects in conflict. In these objects, we can find any
information necessary to build a reporting on the conflict status
(revno, revid, dates, files, developers, etc...)

As the exception is typed, i can easily extend the build command to
catch the branch and revisions in conflict, and so customize my
reporting. The bzr-builder can keep a standard exception msg.

For a future feature, I was thinking about a report template in
a .bazaar directory for instance. If none is present, the standard error
is reported. If a template is present, it replaces the values for the
revisions in conflict.

Keywords might be something like :
revfrom_no / revto_no, revfrom_author / revto_author, etc...

An option --email-report may exist to automatically to email the report
to the one launching the command and the concerned developers.

Rgds/Romain

>
> Thanks,
>
> James

--

Revision history for this message
Martin Pool (mbp) wrote :

On 19 July 2010 18:43, Romain Chalumeau <email address hidden> wrote:
> OK, i understand.
>
> What do you think of the approach of getting in the exception an array
> of revision objects in conflict. In these objects, we can find any
> information necessary to build a reporting on the conflict status
> (revno, revid, dates, files, developers, etc...)
>
> As the exception is typed, i can easily extend the build command to
> catch the branch and revisions in conflict, and so customize my
> reporting. The bzr-builder can keep a standard exception msg.
>
>
>
>
> For a future feature, I was thinking about a report template in
> a .bazaar directory for instance. If none is present, the standard error
> is reported. If a template is present, it replaces the values for the
> revisions in conflict.
>
> Keywords might be something like :
> revfrom_no / revto_no, revfrom_author / revto_author, etc...

I think this basic concept would be useful to have within bzr itself.

> An option --email-report may exist to automatically to email the report
> to the one launching the command and the concerned developers.

I think we should have a post-merge-conflicts hook, which can be
configured in a particular context to send mail to whoever.
--
Martin

Revision history for this message
Romain Chalumeau (rom1-chal) wrote :

Le lundi 19 juillet 2010 à 19:27 +0000, Martin Pool a écrit :
> On 19 July 2010 18:43, Romain Chalumeau <email address hidden> wrote:
> > For a future feature, I was thinking about a report template in
> > a .bazaar directory for instance. If none is present, the standard error
> > is reported. If a template is present, it replaces the values for the
> > revisions in conflict.
> >
> > Keywords might be something like :
> > revfrom_no / revto_no, revfrom_author / revto_author, etc...
>
> I think this basic concept would be useful to have within bzr itself.
>
> > An option --email-report may exist to automatically to email the report
> > to the one launching the command and the concerned developers.
>
> I think we should have a post-merge-conflicts hook, which can be
> configured in a particular context to send mail to whoever.

Hi,

I really like the idea of post-merge-conflicts.
Would help a lot for reporting with an automation of continuous
merging.

Shall i open a blue print in bzr ?

--

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

We don't really use blue prints in Bazaar. Perhaps it's a useful topic to bring up on the mailing list ?

I agree with James that it would be nice to get this into Bazaar itself.

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Voting Needs Fixing, as I think this is something that is more appropriate for the regular bzr merge code in one form or another - it's not really bzr-builder specific.

review: Needs Fixing (code)

Unmerged revisions

99. By Romain Chalumeau

Tests updates
Take into account the new exceptions raised ConflictsDetected and ConflictsFromMerge
in tests :
test_build_tree_merged_with_conflicts
test_pull_or_branch_pull_with_conflicts

98. By Romain Chalumeau

ConflictsDetected exception

97. By Romain Chalumeau

ConflictFromMerge exception

When a merge directive produces conflicts, a conflictFromMerge exception is raised.
The msg of the exception gives a complete report to ease the integrator solving the conflict ie :
- branch url which causes the conflict
- files in conflict
- authors who have committed the conflicted modifications
- path where to handle the conflicts

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'recipe.py'
--- recipe.py 2010-07-09 20:51:37 +0000
+++ recipe.py 2010-07-16 16:32:03 +0000
@@ -68,6 +68,68 @@
68 % (base_branch.deb_version, available_tokens))68 % (base_branch.deb_version, available_tokens))
6969
7070
71# Class to handle exception when merge produces conflicts
72# The msg is targetted to give the necessary information to resolve the conflict
73class ConflictsFromMerge(errors.BzrError) :
74 _fmt = "Conflicts while merging\n\nThe build had to be stopped due to conflicts detected while merging %(childbranch)s\nYou will find the conflicted files in %(workingdir)s\n\n%(conflicts)s"
75
76
77 def __init__(self, childbranch, tree):
78 errors.BzrError.__init__(self)
79 self.childbranch = childbranch
80 self.tree = tree
81 self.conflicts = self.analyse_conflicts()
82 self.workingdir = tree.basedir
83
84 def analyse_conflicts(self):
85
86 from bzrlib import annotate, osutils, revision
87 import time
88
89 msg = ''
90 # Init of the current revision : _expand_annotations does not work with None
91 current_rev = revision.Revision(revision.CURRENT_REVISION)
92 current_rev.parent_ids = self.tree.get_parent_ids()
93 current_rev.committer = self.tree.branch.get_config().username()
94 current_rev.message = "?"
95 current_rev.timestamp = round(time.time(), 3)
96 current_rev.timezone = osutils.local_time_offset()
97
98 for conflicted_file in self.tree._iter_conflicts() :
99 # Annotate conflicted files
100 fileid = self.tree.path2id(conflicted_file)
101 annotations = list(self.tree.annotate_iter(fileid))
102 annotation = list(annotate._expand_annotations(annotations, self.tree.branch, current_rev))
103
104 in_conflict_part = False
105 developers = []
106 # Retrieve the developpers name from the annotation inside the conflict block
107 for (revno_str, author, date_str, origin, text) in annotation :
108 if text.startswith( "=======") :
109 continue
110 if text.startswith(">>>>>>> MERGE-SOURCE"):
111 in_conflict_part = False
112 continue
113 if in_conflict_part and len(author) > 0 :
114 developers.append(author)
115 if text.startswith("<<<<<<< TREE"):
116 in_conflict_part = True
117
118 developers_set = set(developers)
119 msg += "The conflicts in %s are related to changes from %s \n" % (conflicted_file, ', '.join(developers_set))
120
121 msg+= "You may contact them to correctly handle the resolution."
122 return msg
123
124# Unresolved conflicts are still present in the working dir the build is being done
125class ConflictsDetected(errors.BzrError) :
126
127 _fmt = "Unresolved conflicts are still present in the working directory %(workingdir)s. \nPlease resolve those conflicts before relaunching the build on this working dir."
128
129 def __init__(self, tree):
130 errors.BzrError.__init__(self)
131 self.workingdir = tree.basedir
132
71class CommandFailedError(errors.BzrError):133class CommandFailedError(errors.BzrError):
72134
73 _fmt = "The command \"%(command)s\" failed."135 _fmt = "The command \"%(command)s\" failed."
@@ -163,7 +225,7 @@
163 conflicts = tree_to.conflicts()225 conflicts = tree_to.conflicts()
164 if len(conflicts) > 0:226 if len(conflicts) > 0:
165 # FIXME: better reporting227 # FIXME: better reporting
166 raise errors.BzrCommandError("Conflicts... aborting.")228 raise ConflictsDetected(tree_to)
167 except:229 except:
168 if created_br_to:230 if created_br_to:
169 br_to.unlock()231 br_to.unlock()
@@ -215,7 +277,7 @@
215 merger.set_pending()277 merger.set_pending()
216 if conflict_count:278 if conflict_count:
217 # FIXME: better reporting279 # FIXME: better reporting
218 raise errors.BzrCommandError("Conflicts from merge")280 raise ConflictsFromMerge(child_branch.url, tree_to)
219 tree_to.commit("Merge %s" %281 tree_to.commit("Merge %s" %
220 urlutils.unescape_for_display(282 urlutils.unescape_for_display(
221 child_branch.url, 'utf-8'))283 child_branch.url, 'utf-8'))
222284
=== modified file 'tests/test_recipe.py'
--- tests/test_recipe.py 2010-07-09 20:51:37 +0000
+++ tests/test_recipe.py 2010-07-16 16:32:03 +0000
@@ -34,6 +34,8 @@
34 RecipeParser,34 RecipeParser,
35 RecipeBranch,35 RecipeBranch,
36 RecipeParseError,36 RecipeParseError,
37 ConflictsFromMerge,
38 ConflictsDetected,
37 resolve_revisions,39 resolve_revisions,
38 RUN_INSTRUCTION,40 RUN_INSTRUCTION,
39 )41 )
@@ -550,7 +552,7 @@
550 base_branch = BaseRecipeBranch("source1", "1", 0.2)552 base_branch = BaseRecipeBranch("source1", "1", 0.2)
551 merged_branch = RecipeBranch("merged", "source2")553 merged_branch = RecipeBranch("merged", "source2")
552 base_branch.merge_branch(merged_branch)554 base_branch.merge_branch(merged_branch)
553 self.assertRaises(errors.BzrCommandError, build_tree,555 self.assertRaises(ConflictsFromMerge, build_tree,
554 base_branch, "target")556 base_branch, "target")
555 self.failUnlessExists("target")557 self.failUnlessExists("target")
556 tree = workingtree.WorkingTree.open("target")558 tree = workingtree.WorkingTree.open("target")
@@ -694,10 +696,10 @@
694 source.add(["b"])696 source.add(["b"])
695 rev_id = source.commit("two")697 rev_id = source.commit("two")
696 source.branch.tags.set_tag("one", rev_id)698 source.branch.tags.set_tag("one", rev_id)
697 e = self.assertRaises(errors.BzrCommandError,699 e = self.assertRaises(ConflictsDetected,
698 pull_or_branch, tree_to, br_to, source.branch,700 pull_or_branch, tree_to, br_to, source.branch,
699 to_transport, rev_id, accelerator_tree=source)701 to_transport, rev_id, accelerator_tree=source)
700 self.assertEqual("Conflicts... aborting.", str(e))702 self.assertStartsWith(str(e), "Unresolved conflicts are still present in the working directory")
701 tree_to.unlock()703 tree_to.unlock()
702 br_to.unlock()704 br_to.unlock()
703 tree_to = workingtree.WorkingTree.open("target")705 tree_to = workingtree.WorkingTree.open("target")

Subscribers

People subscribed via source and target branches