This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8529

flags: introduce explicit testing for merging change to exec flag
ClosedPublic

Authored by marmoute on May 16 2020, 4:11 PM.

Download Raw Diff

Details

Reviewers

durin42

Group Reviewers

hg-reviewers

Commits

rHGbf5ed664f467: flags: introduce explicit testing for merging change to exec flag

Summary

It turns out that we do not seems to test the simple case for merging exec flag
changes. More advanced case are test (merging exec flag without a common
ancestors, merging with a symlink, etc…) but not the basic.

We are about introduce various fixes to merging flag change across renames,
having the most basic case tested first seems useful.

note: We are only testing "adding" an exec flag here, not removing it. We
introduce basic test on stable and will consolidate them on default.

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

marmoute created this revision.May 16 2020, 4:11 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptMay 16 2020, 4:11 PM

Herald added a subscriber: mercurial-patches. · View Herald Transcript

marmoute added a child revision: D8530: flags: also test merging a rename with and exec flag change.May 16 2020, 4:11 PM

I'm okay with the series, but I'm a little hesitant to put something this big on stable. Can you sway me one way or the other?

In D8529#127681, @durin42 wrote:

I'm okay with the series, but I'm a little hesitant to put something this big on stable. Can you sway me one way or the other?

Most of the "size" is about adding new tests which is less dangerous than code.

Patch-1: test only
Patch-2: test only
Patch-3: small test change, easy to double check (we if flag changed, this is not the same content)
Patch-4: test only
Patch-5: a score of new line, but the fundamental logic is quite simple: we do a three way merge on the flags.
Patch-6: test only
Patch-7: The most "slippery" one because we change existing code. However, the previous code was obviously wrong and no test barked when I changed it
Patch-8: test only, could easilly go on default later
Patch-9: test only, could easilly go on default later

Everything up to Patch-3 is straightforward and worries free. Up to Patch-5 seems simple and fine. Up to Patch 7 is fixing an anoying bug that I would rather see fixed on stable.

Does that help ?

marmoute added a commit: rHGbf5ed664f467: flags: introduce explicit testing for merging change to exec flag.May 28 2020, 12:38 PM

This revision was not accepted when it landed; it landed in state Needs Review.

Closed by commit rHGbf5ed664f467: flags: introduce explicit testing for merging change to exec flag (authored by marmoute). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
A	M		tests/test-merge-exec.t (82 lines)

Status	Author	Revision
Closed	marmoute	D8536 flags: also test merge with executable bit removed
Closed	marmoute	D8535 flags: also test the removal of the exec flag
Closed	marmoute	D8534 flags: read flag from dirstate/disk for workingcopyctx (issue5743)
Closed	marmoute	D8533 flags: actually merge flags in simplemerge
Closed	marmoute	D8532 flags: add a test for merging exec flag change with rename and file change
Closed	marmoute	D8531 flags: account for flag change when tracking rename relevant to merge
Closed	marmoute	D8530 flags: also test merging a rename with and exec flag change
Closed	marmoute	D8529 flags: introduce explicit testing for merging change to exec flag

Diff 21497

tests/test-merge-exec.t

This file was added.

				===============================================
				Testing merge involving change to the exec flag
				===============================================

				#require execbit


				Initial setup
				==============


				$ hg init base-repo
				$ cd base-repo
				$ cat << EOF > a
				> 1
				> 2
				> 3
				> 4
				> 5
				> 6
				> 7
				> 8
				> 9
				> EOF
				$ touch b
				$ hg add a b
				$ hg commit -m "initial commit"
				$ cd ..

				Testing merging mode change
				===========================

				setup

				Change on one side, executable bit on the other

				$ hg clone base-repo simple-merge-repo
				updating to branch default
				2 files updated, 0 files merged, 0 files removed, 0 files unresolved
				$ cd simple-merge-repo
				$ chmod +x a
				$ hg ci -m "make a executable, no change"
				$ [ -x a ] \|\| echo "executable bit not recorded"
				$ hg up ".^"
				1 files updated, 0 files merged, 0 files removed, 0 files unresolved
				$ cat << EOF > a
				> 1
				> 2
				> 3
				> 4
				> 5
				> 6
				> 7
				> x
				> 9
				> EOF
				$ hg commit -m "edit end of file"
				created new head

				merge them (from the update side)

				$ hg merge 'desc("make a executable, no change")'
				1 files updated, 0 files merged, 0 files removed, 0 files unresolved
				(branch merge, don't forget to commit)
				$ hg st
				M a
				$ [ -x a ] \|\| echo "executable bit lost"

				merge them (from the chmod side)

				$ hg up -C 'desc("make a executable, no change")'
				1 files updated, 0 files merged, 0 files removed, 0 files unresolved
				$ hg merge 'desc("edit end of file")'
				1 files updated, 0 files merged, 0 files removed, 0 files unresolved
				(branch merge, don't forget to commit)
				$ hg st
				M a
				$ [ -x a ] \|\| echo "executable bit lost"


				$ cd ..

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	21393		May 16 2020, 4:11 PM	★	★
Diff 2	21497	rHGbf5ed664f467ce58877cce67c142537e8aa420b4	May 16 2020, 2:37 PM	★	★