Merge fail and call to GenerateMergeLogMessage()

[Daniel Sahlberg]

Prompted by the discussion in the users group [1], I've been investigating why the prompt to merge doesn't appear sometimes.

When merging with a specified range, CSVNProgressDlg::CmdMerge() is used while merging without a range (or All revisions) is using CSVNProgressDlg::CmdMergeAll().

CSVNProgressDlg::CheckUpdateAndRetry() should check if an update is required:

    if (GetSVNError() && GetSVNError()->apr_err == SVN_ERR_CLIENT_MERGE_UPDATE_REQUIRED)

        ... prompt the user to update or cancel ...

In CmdMerge, the call to GenerateMergeLogMessage() (line 3384) seems to reset the latest error (SVNBase::m_err) so GetSVNError() will return false.

A naive fix would be to condition the call to GenerateMergeLogMessage() with if (!bFailed), however that is a direct revert of r27728 "Generate the merge log message even if the merge returned with an error: a merge conflict also returns an error.".

I'm instead considering to check GetSVNError() and the actual error message but I don't have enough knowledge about the different error messages that could appear.

Ideally, I'd like to do check for a list of error messages that indicate a merge failure and create a log message in these cases (it seems unneccesary to create a log message in case of a network failure). Unfortunately I don't know how to figure out this list of error messages.

Another option would be to check for only SVN_ERR_CLIENT_MERGE_UPDATE_REQUIRED. That would solve the current bug but could open up for other bugs in the future if/when we want to look for other error messages later in the code.

Any opinion on this?

On a side note, CmdMergeReintegrate() (and CmdMergeReintegrateOldStyle() and  CmdMergeReintegrateOldStyle()) should probably also be changed to run GenerateMergeLogMessage() in case of a merge conflict to give similar behaviour in all kind of merges.

Kind regards,

Daniel

[1] https://groups.google.com/g/tortoisesvn/c/i15g_7BYhgI

[Stefan]

On Saturday, August 5, 2023 at 12:21:54 PM UTC+2 daniel.l...@gmail.com wrote:

Prompted by the discussion in the users group [1], I've been investigating why the prompt to merge doesn't appear sometimes.

When merging with a specified range, CSVNProgressDlg::CmdMerge() is used while merging without a range (or All revisions) is using CSVNProgressDlg::CmdMergeAll().

CSVNProgressDlg::CheckUpdateAndRetry() should check if an update is required:

    if (GetSVNError() && GetSVNError()->apr_err == SVN_ERR_CLIENT_MERGE_UPDATE_REQUIRED)

        ... prompt the user to update or cancel ...

In CmdMerge, the call to GenerateMergeLogMessage() (line 3384) seems to reset the latest error (SVNBase::m_err) so GetSVNError() will return false.

A naive fix would be to condition the call to GenerateMergeLogMessage() with if (!bFailed), however that is a direct revert of r27728 "Generate the merge log message even if the merge returned with an error: a merge conflict also returns an error.".

I'm instead considering to check GetSVNError() and the actual error message but I don't have enough knowledge about the different error messages that could appear.

I'm not against that, but in my experience that is hard to maintain. Because new errors are added frequently to svn, which means we'd have to keep up.

Since the problem appears to be that GenerateMergeLogMessage() clears the current error, why not just do:

auto oldErr = svn_error_dup(m_err);

// do the GenerateMergeLogMessage part

svn_error_clear(m_err);

m_err = oldErr;

 

Ideally, I'd like to do check for a list of error messages that indicate a merge failure and create a log message in these cases (it seems unneccesary to create a log message in case of a network failure). Unfortunately I don't know how to figure out this list of error messages.

Another option would be to check for only SVN_ERR_CLIENT_MERGE_UPDATE_REQUIRED. That would solve the current bug but could open up for other bugs in the future if/when we want to look for other error messages later in the code.

Any opinion on this?

On a side note, CmdMergeReintegrate() (and CmdMergeReintegrateOldStyle() and  CmdMergeReintegrateOldStyle()) should probably also be changed to run GenerateMergeLogMessage() in case of a merge conflict to give similar behaviour in all kind of merges.

agreed.

Stefan

[Daniel Sahlberg]

söndag 6 augusti 2023 kl. 18:56:39 UTC+2 skrev Stefan:

agreed.

Thanks for the suggestions. All done in r29601 and r29602.

/Daniel