This is an archive of the discontinued Mercurial Phabricator instance.

merge: allow to merge non-conflicting changes outside narrowspec
Needs RevisionPublic

Authored by pulkit on Dec 11 2018, 6:21 AM.

Details

Reviewers
durin42
martinvonz
Group Reviewers
hg-reviewers
Summary

This patch allows merging of non-conflicting changes outside narrowspec.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit created this revision.Dec 11 2018, 6:21 AM

I am dubiuos that my fix is correct. I went through the history and didn't find any explanation why we don't allow merging non-conflicting changes outside narrowspec except TODO's. @martinvonz do you know why we don't allow merging of non-conflicting changes here?

martinvonz requested changes to this revision.Dec 11 2018, 9:47 AM

I'm pretty sure this doesn't actually perform the merge, it just drops the changes outside the narrowspec. On commit, we need to record that outside/ has the new nodeid that we got from the side we merged in. To do that, we need to remember what that nodeid is, from the time of hg merge to the time of hg commit. That probably means storing the nodeid in the dirstate (like git does), or maybe in the merge state.

This revision now requires changes to proceed.Dec 11 2018, 9:47 AM
pulkit added a comment.Feb 4 2019, 8:48 AM

I'm pretty sure this doesn't actually perform the merge, it just drops the changes outside the narrowspec. On commit, we need to record that outside/ has the new nodeid that we got from the side we merged in. To do that, we need to remember what that nodeid is, from the time of hg merge to the time of hg commit. That probably means storing the nodeid in the dirstate (like git does), or maybe in the merge state.

IIUC, dirstate does not contains files outside narrowspec. Maybe we need to do this in merge state?

In D5410#85232, @pulkit wrote:

I'm pretty sure this doesn't actually perform the merge, it just drops the changes outside the narrowspec. On commit, we need to record that outside/ has the new nodeid that we got from the side we merged in. To do that, we need to remember what that nodeid is, from the time of hg merge to the time of hg commit. That probably means storing the nodeid in the dirstate (like git does), or maybe in the merge state.

IIUC, dirstate does not contains files outside narrowspec. Maybe we need to do this in merge state?

I think neither of them currently contains files outside narrows, so we'd need to extend whichever we decide. It's probably easier to extend the merge state. I'm not sure which I think is cleaner to extend. There's precedent for populating the commit based on the dirstate (index) in git.

In D5410#85232, @pulkit wrote:

I'm pretty sure this doesn't actually perform the merge, it just drops the changes outside the narrowspec. On commit, we need to record that outside/ has the new nodeid that we got from the side we merged in. To do that, we need to remember what that nodeid is, from the time of hg merge to the time of hg commit. That probably means storing the nodeid in the dirstate (like git does), or maybe in the merge state.

IIUC, dirstate does not contains files outside narrowspec. Maybe we need to do this in merge state?

I think neither of them currently contains files outside narrows, so we'd need to extend whichever we decide. It's probably easier to extend the merge state. I'm not sure which I think is cleaner to extend. There's precedent for populating the commit based on the dirstate (index) in git.

Or not touch any of them and introduce a new file?

In D5410#87526, @pulkit wrote:
In D5410#85232, @pulkit wrote:

I'm pretty sure this doesn't actually perform the merge, it just drops the changes outside the narrowspec. On commit, we need to record that outside/ has the new nodeid that we got from the side we merged in. To do that, we need to remember what that nodeid is, from the time of hg merge to the time of hg commit. That probably means storing the nodeid in the dirstate (like git does), or maybe in the merge state.

IIUC, dirstate does not contains files outside narrowspec. Maybe we need to do this in merge state?

I think neither of them currently contains files outside narrows, so we'd need to extend whichever we decide. It's probably easier to extend the merge state. I'm not sure which I think is cleaner to extend. There's precedent for populating the commit based on the dirstate (index) in git.

Or not touch any of them and introduce a new file?

Sure, that's another option. It might be the easiest one since it won't involve making the format compatible (although I think that's pretty easy for the merge state).