This is an archive of the discontinued Mercurial Phabricator instance.

tests: run test-copies-chain-merge.t also with copies in changesets
ClosedPublic

Authored by martinvonz on Oct 8 2020, 1:23 PM.

Details

Summary

We have these tests already and it seems like a waste to not run them
in the changesets case. The biggest differences stem from
hg log --follow not working with copies stored in the changeset
extras.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Oct 8 2020, 1:23 PM

Reviewers: Up to here is ready for review (and uncontroversial, I would think)

pulkit accepted this revision.Oct 13 2020, 3:41 AM
This revision is now accepted and ready to land.Oct 13 2020, 3:41 AM

This adds a non negligible amount of churn that conflict with an import bug series I submitted (See D9195 and above).

Can we drop it for now and restart review on a new diff? In particular, I do not understand why the changeset version of the algorithm is not giving use the same result as the compatibility one.

(cc @pulkit )

This adds a non negligible amount of churn that conflict with an import bug series I submitted (See D9195 and above).
Can we drop it for now and restart review on a new diff?

That was sent for review after this series, so I don't think it's fair to drop this one for that reason.

In particular, I do not understand why the changeset version of the algorithm is not giving use the same result as the compatibility one.

I think I explained that in the commit message: hg log --follow <file> doesn't work with copy information (only) in the changelog, since it has a fast-path that follows the revisions in the filelog. We (probably I) will need to fix that some day...

This adds a non negligible amount of churn that conflict with an import bug series I submitted (See D9195 and above).
Can we drop it for now and restart review on a new diff?

That was sent for review after this series, so I don't think it's fair to drop this one for that reason.

You probably noticed that I have been sending a pretty regular stream of patch on this topic over the last month. This series is just the continuation of this. I would have appreciated if you would have coordinated with me before starting sending arbitrary patch on this topic. The lack of communication is probably going to cost too many hours for both of us.

In particular, I do not understand why the changeset version of the algorithm is not giving use the same result as the compatibility one.

I think I explained that in the commit message: hg log --follow <file> doesn't work with copy information (only) in the changelog, since it has a fast-path that follows the revisions in the filelog. We (probably I) will need to fix that some day...

We should probably explicitly add this information around the if block.

This adds a non negligible amount of churn that conflict with an import bug series I submitted (See D9195 and above).
Can we drop it for now and restart review on a new diff?

That was sent for review after this series, so I don't think it's fair to drop this one for that reason.

You probably noticed that I have been sending a pretty regular stream of patch on this topic over the last month. This series is just the continuation of this. I would have appreciated if you would have coordinated with me before starting sending arbitrary patch on this topic. The lack of communication is probably going to cost too many hours for both of us.

In particular, I do not understand why the changeset version of the algorithm is not giving use the same result as the compatibility one.

I think I explained that in the commit message: hg log --follow <file> doesn't work with copy information (only) in the changelog, since it has a fast-path that follows the revisions in the filelog. We (probably I) will need to fix that some day...

We should probably explicitly add this information around the if block.

Yes, it will be nice to get this documented around that block.

This adds a non negligible amount of churn that conflict with an import bug series I submitted (See D9195 and above).
Can we drop it for now and restart review on a new diff?

That was sent for review after this series, so I don't think it's fair to drop this one for that reason.

You probably noticed that I have been sending a pretty regular stream of patch on this topic over the last month. This series is just the continuation of this. I would have appreciated if you would have coordinated with me before starting sending arbitrary patch on this topic. The lack of communication is probably going to cost too many hours for both of us.

In particular, I do not understand why the changeset version of the algorithm is not giving use the same result as the compatibility one.

I think I explained that in the commit message: hg log --follow <file> doesn't work with copy information (only) in the changelog, since it has a fast-path that follows the revisions in the filelog. We (probably I) will need to fix that some day...

We should probably explicitly add this information around the if block.

Yes, it will be nice to get this documented around that block.

Agreed. I've sent D9204 for that.