Caution! The merge cmdlets might merge incorrectly!

NAV2015 introduced new object merge functionality to ease upgrading of NAV databases dramatically. For information about the cmdlets you could start here: http://blogs.msdn.com/b/nav/archive/2014/10/03/merging-application-objects-using-windows-powershell-in-microsoft-dynamics-nav-2015.aspx

I reported an issue to Microsoft back in the middle of December 2014, and after more than one month of e-mails back and forth I got this response:

(…)
This by design and won’t be fixed. We are applying a 3-way merge on the function body including comments. The 3 merge searches for similar code when doing a merge an the out-commented code gives the best match. The customer would see a similar issue by using other 3-way merge tools like Beyond Compare. The current design also supports merging of comments which can contain valuable information that should also be merge.
(…)

I now give up, and instead of trying to convince Microsoft to fix the cmdlets, I instead wish to warn the NAV community about this issue. The main issue is, that the tool claim to show which objects with conflicts the NAV developer needs to handle manually and which objects was merged automatically without conflicts. However; in my example below, it claims there are no conflicts, but the merge is incorrect. 😦

My first error report was rejected with the argument my C/AL code did not comply with the guidelines specified here: https://msdn.microsoft.com/en-us/library/ee414237.aspx That meant Microsoft initially said the merge cmdlets might fail if your code was indented incorrect or your variables were not in the right order πŸ™‚

Enough teasing, here is the problem: If either the Modified or Target object set uses braces (”{” and ”}”) to disable code, then it might merge incorrectly and claim everything is ok. The usage of braces isΒ not recommended by Microsoft, but it is not until NAV2015 we have been able to easy block comment code with “//”. Using braces has always been possible and even described here: https://msdn.microsoft.com/en-us/library/dd301180(v=nav.70).aspx

Many NAV customers have had several VARs modifying their objects and their current VAR might not know if braces are used or not. My current company has a company policy to use braces when code is changed. There are pros and cons with this approach, but the point is, that there is no (easy) way to check if anyone ever used this approach in a NAV database.

Here is an example with a dummy modification in a simple codeunit using braces: (left=base, right=modified)

BaseVsModified

Let us imagine Microsoft also made a change in the codeunit: (left=base, right=target)
BaseVsTarget

Here is the result from the merge cmdlets: (left=base, right=result)
BaseVsResult

As you can see, the cmdlets simply merges Microsoft’s code changes into the comment section of the Modified object. I do not claim the cmdlets should necessarily merge the code into the code section, but it should at least consider this to be conflict. As it is right now, then no developer would notice this potentially critical problem.

What do you think – is this just a theoretical problem, or does this make you more careful using the merge cmdlets?

Advertisements

10 thoughts on “Caution! The merge cmdlets might merge incorrectly!

  1. Hi GrumpyNavDev,

    this is definitely not a theoretical issue only. I would bet almost every real-world NAV application has comments with curly brackets. All that I know do, and not because I inserted them. If the cmdlet merges them wrongly and as not conflicted, it renders the tool practically useless. So… good to know, thank you for sharing it.

    with best regards

    Jens

    Like

    • Hi Waldo,
      I kept hitting a brick wall when I tried to convince Microsoft of the impact of this issue. (Created a support incident at Partnersource) They simply didn’t see it as a problem, as people should always test the merge result…
      Do you know anyone who might be able to convince the right people of this issue, or maybe bring more awareness to the issue using your blog?
      I think the solution could be rather simple. Just internally prefix all comment lines with something during the merge. This would prevent merging code with comments and vice versa.

      Like

  2. I agree that there should be a conflict, however I agree also with MS that curly brackets, as a best practice, should not be used for – to me – obvious reasons.
    If code has to be outcommented when going live this means to me this was old code that has been replaced. This should be done using slashes.
    If it’s outcommented “just because …” (we might need it later), it should have been deleted.

    Like

    • Before the introduction of the merge cmdlets, I only knew one problem using braces / curly brackets, and that was the missing green color in the designer. The debugger knew how to color these comments – it was only the designer that did not.
      However; the point is that we often do not know if somebody else (maybe even another VAR or one of their add-ons) might have used it before we got the client. To us it makes the cmdlets useless, and that is a shame.

      Like

    • Hi Luc,

      obvious reasons or not, it’s a real issue. C/AL syntax allows commenting with curly brackets. It has been used for 20+ years. Unless that is changed for all existing NAV installations out there, any new tool handling C/AL must know about it and and should handle it as a comment. This, or the tool would be a hazard / useless for all existing NAV installations. “Best practice” is a really weak argument in the face of reality. It smacks of avoidance, and along the lines of “no, it is hard, ticket is already closed, no budget left, I don’t care, why don’t they upgrade” thinking. This doesn’t solve the issue. πŸ™‚ So, Mr. MVP, do you somebody at MDCC to talk sense to about it?

      with best regards

      Jens

      Like

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s