Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-7594

Merges across named branches should not be ignored

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      By fixing JENKINS-4972, the mercurial plugin discards merges for the purpose of triggering builds and determining culprits.

      This makes sense when the merge is within a single named branch (i.e. it is during the regular flow of development).

      It does not make a lot of sense to drop the merge when it is a merge across named branches. These merges are typically merges of a fix from one version branch to another version branch, and if this merge introduces errors, the merger wants to know about it.

        Attachments

          Issue Links

            Activity

            Hide
            willemv Willem Verstraeten added a comment -

            jglick is mentioned as the maintainer of the mercurial plugin, and he fixed JENKINS-4972, so I am assigning it to him

            Show
            willemv Willem Verstraeten added a comment - jglick is mentioned as the maintainer of the mercurial plugin, and he fixed JENKINS-4972 , so I am assigning it to him
            Hide
            jglick Jesse Glick added a comment -

            Not sure what the best way to fix is without regressing JENKINS-4972. Could return to considering merges as poll triggers if and only if you have configured no module list, which would be right for everyone except people who are triggering on submodules and merging across named branches, probably a small minority.

            I don't know how to tell that a merge changeset changed certain files in the effective manifest for the effective branch head, without doing some complicated and expensive hg commands (i.e. beyond simple hg incoming). If caching is in effect, I suppose that after updating the cache you could take the current workspace revision, then run in the cache repo: hg di -stat --rev $wspace --rev $branch. (-stat available only in newer Hg releases.) This summary can then be matched against the module list (if any). This trick will not work without a cache repo, unless perhaps you use the RDiff extension. And you will still need to run hg incoming to produce a changelog, even though some of the resulting changesets might not be "used" in the actual head-to-head diff.

            Show
            jglick Jesse Glick added a comment - Not sure what the best way to fix is without regressing JENKINS-4972 . Could return to considering merges as poll triggers if and only if you have configured no module list, which would be right for everyone except people who are triggering on submodules and merging across named branches, probably a small minority. I don't know how to tell that a merge changeset changed certain files in the effective manifest for the effective branch head, without doing some complicated and expensive hg commands (i.e. beyond simple hg incoming). If caching is in effect, I suppose that after updating the cache you could take the current workspace revision, then run in the cache repo: hg di - stat --rev $wspace --rev $branch. ( -stat available only in newer Hg releases.) This summary can then be matched against the module list (if any). This trick will not work without a cache repo, unless perhaps you use the RDiff extension. And you will still need to run hg incoming to produce a changelog, even though some of the resulting changesets might not be "used" in the actual head-to-head diff.
            Hide
            willemv Willem Verstraeten added a comment -

            Couldn't you simply determine whether one of the two parents of the merge came from a different named branch and, if so, simply include it for triggering ?

            Show
            willemv Willem Verstraeten added a comment - Couldn't you simply determine whether one of the two parents of the merge came from a different named branch and, if so, simply include it for triggering ?
            Hide
            jglick Jesse Glick added a comment -

            That's fine if there is no module list. It will trigger unnecessary builds if there is a module list. That may be an acceptable conservative compromise.

            Show
            jglick Jesse Glick added a comment - That's fine if there is no module list. It will trigger unnecessary builds if there is a module list. That may be an acceptable conservative compromise.
            Hide
            foogly Anthony Brown added a comment -

            I have to admit that I would rather have a few unnecessary builds, than not having it build when it should.

            Show
            foogly Anthony Brown added a comment - I have to admit that I would rather have a few unnecessary builds, than not having it build when it should.
            Hide
            ridesmet Ringo De Smet added a comment -

            We are more and more using named branches as feature branches. However, when the branch is merged back into the parent branch, no build is triggered in Jenkins for the parent branch. This is becoming problematic.

            What is the status of this ticket?

            Show
            ridesmet Ringo De Smet added a comment - We are more and more using named branches as feature branches. However, when the branch is merged back into the parent branch, no build is triggered in Jenkins for the parent branch. This is becoming problematic. What is the status of this ticket?
            Hide
            marcsanfacon Marc Sanfacon added a comment -

            We have the same issue here.

            I suggest adding an option 'Ignore Merges' for each mercurial configuration so that a Jenkins admin can decide or not to trigger builds on merge.

            Show
            marcsanfacon Marc Sanfacon added a comment - We have the same issue here. I suggest adding an option 'Ignore Merges' for each mercurial configuration so that a Jenkins admin can decide or not to trigger builds on merge.
            Hide
            jglick Jesse Glick added a comment -

            Adding a configuration option is probably not a good idea; the plugin should work sensibly without configuration.

            Show
            jglick Jesse Glick added a comment - Adding a configuration option is probably not a good idea; the plugin should work sensibly without configuration.
            Hide
            jglick Jesse Glick added a comment -

            Not actively working on the plugin at the moment.

            Show
            jglick Jesse Glick added a comment - Not actively working on the plugin at the moment.
            Hide
            haraldme haraldme added a comment -

            As I understand it, it isn't reliable to base the "trigger build or not" decision based on whatever Mercurial reports as "modified files" in a merge commit. I'm not entirely sure how this "modified files" list is generated, but from my jobs' console output I suspect it is done using something a la

            hg log --rev <branch>:0 --prune <rev-after-previous-pull>
            

            ... which means we're leaving it up to Mercurial's innards to fill in the blanks for added/modified/deleted files.

            Wouldn't it be better to ask Mercurial what files changed between the revision we were at after the last pull, and the revision we've ended up at after a new pull? That is, do something closer to

            hg status --rev <rev-after-previous-pull> --rev <branch>
            

            to obtain the list of "modified files".

            Show
            haraldme haraldme added a comment - As I understand it, it isn't reliable to base the "trigger build or not" decision based on whatever Mercurial reports as "modified files" in a merge commit. I'm not entirely sure how this "modified files" list is generated, but from my jobs' console output I suspect it is done using something a la hg log --rev <branch>:0 --prune <rev-after-previous-pull> ... which means we're leaving it up to Mercurial's innards to fill in the blanks for added/modified/deleted files. Wouldn't it be better to ask Mercurial what files changed between the revision we were at after the last pull, and the revision we've ended up at after a new pull? That is, do something closer to hg status --rev <rev-after-previous-pull> --rev <branch> to obtain the list of "modified files".
            Hide
            willemv Willem Verstraeten added a comment -

            haraldme : I've been thinking the same thing. I've been working on implementing it, I will likely create a push request for it next week.

            Show
            willemv Willem Verstraeten added a comment - haraldme : I've been thinking the same thing. I've been working on implementing it, I will likely create a push request for it next week.
            Hide
            jglick Jesse Glick added a comment -

            @haraldme: the devil is in the "something closer to" details. Please read my original comments. There are two basic problems to solve:

            1. Dealing with workspaces not using a cache repository. In that case the remote head, i.e. the proposed new tip of the workspace, is not locally available, so a plain call to hg diff or the like will not work. If you have already run hg incoming and thus saved a bundle somewhere, it may work to use the little-known "bundlerepo" syntax: hg -R incoming.hg ... will run on an effective repo consisting of the current (workspace) repo plus the specified bundle.

            2. Figuring out what command to run to actually get the list of files changed between two revisions. You need to know the exact list, so you can filter by the modules config option (if defined) and check to see if there are any files left. hg diff --stat, available in recent Mercurial versions (but not older versions), gives a decent summary for people but is difficult to parse mechanically. It also does not show file renames meaningfully. hg status --rev gives much more parsable output but wastes time walking the working copy and will in fact mix in junk if there are any local modifications in the workspace (from a poorly written build script) and the clean flag is not set, since unlike diff it only takes one revision argument. The best approach I can come up with is to run hg manifest --debug --rev on both the local and remote heads, and manually compare the output; this should tell you what exact files will be modified, added, or removed (including permissions!) by cutting at column 47.

            The current code relies on enumerating files changed in particular (non-merge) revisions, under the simplifying assumption that the set of files effectively changed by an incoming bundle is equal to the union of those files changed in the non-merge changesets in that bundle. Of course this fails in various corner cases.

            Show
            jglick Jesse Glick added a comment - @haraldme: the devil is in the "something closer to" details. Please read my original comments. There are two basic problems to solve: 1. Dealing with workspaces not using a cache repository. In that case the remote head, i.e. the proposed new tip of the workspace, is not locally available, so a plain call to hg diff or the like will not work. If you have already run hg incoming and thus saved a bundle somewhere, it may work to use the little-known "bundlerepo" syntax: hg -R incoming.hg ... will run on an effective repo consisting of the current (workspace) repo plus the specified bundle. 2. Figuring out what command to run to actually get the list of files changed between two revisions. You need to know the exact list, so you can filter by the modules config option (if defined) and check to see if there are any files left. hg diff --stat , available in recent Mercurial versions (but not older versions), gives a decent summary for people but is difficult to parse mechanically. It also does not show file renames meaningfully. hg status --rev gives much more parsable output but wastes time walking the working copy and will in fact mix in junk if there are any local modifications in the workspace (from a poorly written build script) and the clean flag is not set, since unlike diff it only takes one revision argument. The best approach I can come up with is to run hg manifest --debug --rev on both the local and remote heads, and manually compare the output; this should tell you what exact files will be modified, added, or removed (including permissions!) by cutting at column 47. The current code relies on enumerating files changed in particular (non-merge) revisions, under the simplifying assumption that the set of files effectively changed by an incoming bundle is equal to the union of those files changed in the non-merge changesets in that bundle. Of course this fails in various corner cases.
            Hide
            marcsanfacon Marc Sanfacon added a comment -

            @jglick, I understand that trying to support everything automatically is probably what makes more sense. However, in our cases, the main Mercurial server does not allow multiple heads so all the merge operations are coming from other branches, which means that they include new features or bug fixes and we want this to trigger a new build. I think it makes sense to let the administrator decide if a build must be triggered or not by merge operations.

            I have coded a new option in the mercurial plug-in to trigger builds on merge operations. I am using this new version on our production server and it works exactly the way I want it.

            The repository containing the modifications is here: https://github.com/marcsanfacon/mercurial-plugin

            Show
            marcsanfacon Marc Sanfacon added a comment - @jglick, I understand that trying to support everything automatically is probably what makes more sense. However, in our cases, the main Mercurial server does not allow multiple heads so all the merge operations are coming from other branches, which means that they include new features or bug fixes and we want this to trigger a new build. I think it makes sense to let the administrator decide if a build must be triggered or not by merge operations. I have coded a new option in the mercurial plug-in to trigger builds on merge operations. I am using this new version on our production server and it works exactly the way I want it. The repository containing the modifications is here: https://github.com/marcsanfacon/mercurial-plugin
            Hide
            jglick Jesse Glick added a comment -

            This should not be left to the administrator to have to figure out. Polling should trigger a build if and only if the incoming changesets would change the content of the working copy - which they would in your case.

            Show
            jglick Jesse Glick added a comment - This should not be left to the administrator to have to figure out. Polling should trigger a build if and only if the incoming changesets would change the content of the working copy - which they would in your case.
            Hide
            haraldme haraldme added a comment -

            Regarding your claim that hg status takes only one revision argument: The status┬ácommand accepts up to two --rev arguments, and has done so since Mercurial version 0.9.2 (added in commit 8b55c0ba8048 of the hg-stable repo, in October 2006). When given two revision arguments, I don't see why it would walk the working copy (although I haven't checked the code) – as it will produce the list of changes between those two revisions, unsullied by any local modifications in the working tree.

            Not sure what a "cache repository" is; is that something specific to the Mercurial plugin of Jenkins?

            Show
            haraldme haraldme added a comment - Regarding your claim that hg status takes only one revision argument: The status ┬ácommand accepts up to two --rev arguments, and has done so since Mercurial version 0.9.2 (added in commit 8b55c0ba8048 of the hg-stable repo, in October 2006). When given two revision arguments, I don't see why it would walk the working copy (although I haven't checked the code) – as it will produce the list of changes between those two revisions, unsullied by any local modifications in the working tree. Not sure what a "cache repository" is; is that something specific to the Mercurial plugin of Jenkins?
            Hide
            jglick Jesse Glick added a comment -

            @haraldme: good catch, this is mentioned in the freeform help text (though not in the line for this option). Should simplify the impl, though manifest --debug does work.

            Cache repository is specific to the plugin. In fact it seems #1 does not matter because after c148799 the plugin runs pull rather than incoming.

            Show
            jglick Jesse Glick added a comment - @haraldme: good catch, this is mentioned in the freeform help text (though not in the line for this option). Should simplify the impl, though manifest --debug does work. Cache repository is specific to the plugin. In fact it seems #1 does not matter because after c148799 the plugin runs pull rather than incoming .
            Hide
            jglick Jesse Glick added a comment -

            Fixing with 3b5409.

            Show
            jglick Jesse Glick added a comment - Fixing with 3b5409.
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Jesse Glick
            Path:
            src/main/java/hudson/plugins/mercurial/HgExe.java
            src/main/java/hudson/plugins/mercurial/MercurialSCM.java
            src/test/java/hudson/plugins/mercurial/MercurialSCMTest.java
            http://jenkins-ci.org/commit/mercurial-plugin/3b54094736fc31fe159c2f5fa80d91b64db47851
            Log:
            [FIXED JENKINS-7594] Merges across named branches should not be ignored.
            Rewriting compareRemoteRevisionWith to use 'hg st --rev ... --rev ...' which is simpler and more robust than examining intermediate csets.

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: src/main/java/hudson/plugins/mercurial/HgExe.java src/main/java/hudson/plugins/mercurial/MercurialSCM.java src/test/java/hudson/plugins/mercurial/MercurialSCMTest.java http://jenkins-ci.org/commit/mercurial-plugin/3b54094736fc31fe159c2f5fa80d91b64db47851 Log: [FIXED JENKINS-7594] Merges across named branches should not be ignored. Rewriting compareRemoteRevisionWith to use 'hg st --rev ... --rev ...' which is simpler and more robust than examining intermediate csets.
            Hide
            dogfood dogfood added a comment -

            Integrated in plugins_mercurial #84
            [FIXED JENKINS-7594] Merges across named branches should not be ignored.

            Jesse Glick :
            Files :

            • src/test/java/hudson/plugins/mercurial/MercurialSCMTest.java
            • src/main/java/hudson/plugins/mercurial/HgExe.java
            • src/main/java/hudson/plugins/mercurial/MercurialSCM.java
            Show
            dogfood dogfood added a comment - Integrated in plugins_mercurial #84 [FIXED JENKINS-7594] Merges across named branches should not be ignored. Jesse Glick : Files : src/test/java/hudson/plugins/mercurial/MercurialSCMTest.java src/main/java/hudson/plugins/mercurial/HgExe.java src/main/java/hudson/plugins/mercurial/MercurialSCM.java
            Hide
            haraldme haraldme added a comment -

            This issue was resolved some weeks ago – but I've been unable to find any info about plans for making the fix part of a new release of the plugin. The list of issues under the "Version 1.39 (Under Development)" heading on https://wiki.jenkins-ci.org/display/JENKINS/Mercurial+Plugin does not mention this bug.

            The "don't trigger build on merge commits" misfeature regularly causes my team to miss out on Jenkins builds (which in turn means that build-breaking culprits may get out of bringing the otherwise compulsory "I broke the build" cake), so we're eagerly looking forward to having this fixed.

            Show
            haraldme haraldme added a comment - This issue was resolved some weeks ago – but I've been unable to find any info about plans for making the fix part of a new release of the plugin. The list of issues under the "Version 1.39 (Under Development)" heading on https://wiki.jenkins-ci.org/display/JENKINS/Mercurial+Plugin does not mention this bug. The "don't trigger build on merge commits" misfeature regularly causes my team to miss out on Jenkins builds (which in turn means that build-breaking culprits may get out of bringing the otherwise compulsory "I broke the build" cake), so we're eagerly looking forward to having this fixed.
            Hide
            ridesmet Ringo De Smet added a comment -

            Like harald, I would like to see these fixes rolled into a release of the plugin.

            Show
            ridesmet Ringo De Smet added a comment - Like harald, I would like to see these fixes rolled into a release of the plugin.
            Hide
            jglick Jesse Glick added a comment -

            Many significant changes and refactorings have made in dev sources so I am not surprised the wiki is out of date.

            I am not currently doing plugin releases (pending info on how to simultaneously publish for Hudson), but one of the other devs on the project might.

            Show
            jglick Jesse Glick added a comment - Many significant changes and refactorings have made in dev sources so I am not surprised the wiki is out of date. I am not currently doing plugin releases (pending info on how to simultaneously publish for Hudson), but one of the other devs on the project might.

              People

              • Assignee:
                jglick Jesse Glick
                Reporter:
                willemv Willem Verstraeten
              • Votes:
                5 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: