Skip to content

Commit 90f6c78

Browse files
authored
Merge pull request #5605 from unisonweb/25-03-04-merge-bugfix
bugfix: merge name-pick bug
2 parents d2aa2f5 + ea9e72e commit 90f6c78

File tree

3 files changed

+143
-3
lines changed

3 files changed

+143
-3
lines changed

unison-core/src/Unison/Names.hs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,9 +233,9 @@ preferring :: Names -> Names -> Names
233233
preferring xs ys =
234234
Names (preferring1 xs.terms ys.terms) (preferring1 xs.types ys.types)
235235
where
236-
preferring1 :: (Ord a, Ord b) => Relation a b -> Relation a b -> Relation a b
236+
preferring1 :: (Ord ref) => Relation Name ref -> Relation Name ref -> Relation Name ref
237237
preferring1 =
238-
Relation.unionRangeWith (\_ x _ -> x)
238+
Relation.unionRangeWith (\_ref xs _ys -> xs)
239239

240240
-- | TODO: get this from database. For now it's a constant.
241241
numHashChars :: Int

unison-merge/src/Unison/Merge/Mergeblob3.hs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,12 @@ makePrettyPrintEnv names libdepsNames lcaLibdeps =
374374
PPED.makePPED
375375
( PPE.namer
376376
( Names.preferring
377-
(Names.preferring names.alice names.bob <> libdepsNames)
377+
-- Here it might be slightly more comfortable to Alice if we prefer her names and _her_ libdeps, not the
378+
-- combined Alice+Bob libdep, because that might bring in a Bob name that Alice isn't yet familiar with
379+
-- (even though it will be in her merge result at the end). However, that would require a bit of simple
380+
-- refactoring (just need to delay the combining of libdeps until at least here), and doesn't seem worth it
381+
-- over this quick fix of just "prefer Alice + any libdep name over names that only Bob's project has".
382+
(Names.preferring (names.alice <> libdepsNames) names.bob)
378383
(names.lca <> lcaLibdeps)
379384
)
380385
)
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
``` ucm
2+
scratch/main> builtins.mergeio lib.builtin
3+
4+
Done.
5+
```
6+
7+
The situation:
8+
9+
- topic has two names for a thing (foo and lib.bar)
10+
- main delete one alias (foo, leaving lib.bar)
11+
- When merging topic into main, for some reason we render topic's stuff that depends on that thing as referring to
12+
`foo`, which main deleted\!
13+
14+
``` ucm
15+
scratch/main> alias.type lib.builtin.Nat MyNat
16+
17+
Done.
18+
```
19+
20+
``` unison
21+
foo : Nat
22+
foo = 17
23+
```
24+
25+
``` ucm :added-by-ucm
26+
Loading changes detected in scratch.u.
27+
28+
I found and typechecked these definitions in scratch.u. If you
29+
do an `add` or `update`, here's how your codebase would
30+
change:
31+
32+
⍟ These new definitions are ok to `add`:
33+
34+
foo : MyNat
35+
```
36+
37+
``` ucm
38+
scratch/main> add
39+
40+
⍟ I've added these definitions:
41+
42+
foo : MyNat
43+
44+
scratch/main> branch topic
45+
46+
Done. I've created the topic branch based off of main.
47+
48+
Tip: To merge your work back into the main branch, first
49+
`switch /main` then `merge /topic`.
50+
51+
scratch/topic> switch /main
52+
53+
scratch/main> delete.type MyNat
54+
55+
Done.
56+
```
57+
58+
``` unison
59+
foo : Nat
60+
foo = 18
61+
```
62+
63+
``` ucm :added-by-ucm
64+
Loading changes detected in scratch.u.
65+
66+
I found and typechecked these definitions in scratch.u. If you
67+
do an `add` or `update`, here's how your codebase would
68+
change:
69+
70+
⍟ These names already exist. You can `update` them to your
71+
new definition:
72+
73+
foo : Nat
74+
```
75+
76+
``` ucm
77+
scratch/main> update
78+
79+
Okay, I'm searching the branch for code that needs to be
80+
updated...
81+
82+
Done.
83+
84+
scratch/main> switch /topic
85+
```
86+
87+
``` unison
88+
bar : MyNat
89+
bar = foo + foo
90+
```
91+
92+
``` ucm :added-by-ucm
93+
Loading changes detected in scratch.u.
94+
95+
I found and typechecked these definitions in scratch.u. If you
96+
do an `add` or `update`, here's how your codebase would
97+
change:
98+
99+
⍟ These new definitions are ok to `add`:
100+
101+
bar : MyNat
102+
```
103+
104+
``` ucm
105+
scratch/topic> add
106+
107+
⍟ I've added these definitions:
108+
109+
bar : MyNat
110+
111+
scratch/topic> switch /main
112+
```
113+
114+
This merge used to fail because it rendered `bar` using the deleted name `MyNat` (rather than the still-existing name
115+
`Nat`).
116+
117+
``` ucm
118+
scratch/main> merge /topic
119+
120+
Loading branches...
121+
122+
Loading definitions...
123+
124+
Computing diffs...
125+
126+
Loading dependents of changes...
127+
128+
Loading and merging library dependencies...
129+
130+
Rendering Unison file...
131+
132+
Typechecking Unison file...
133+
134+
I merged scratch/topic into scratch/main.
135+
```

0 commit comments

Comments
 (0)