mergiraf/mergiraf
mergiraf/mergiraf
9
213
Fork 45
Code Issues 51 Pull requests 7 Releases 19 Activity Actions

Bad merge for YAML conflict with double-quoted string #379

New issue
Closed
opened 2025-05-11 19:03:26 +02:00 by carlwr · 2 comments
carlwr commented 2025-05-11 19:03:26 +02:00
Copy link

It seems mergiraf solves a conflict incorrectly for a YAML file with double-quoted values in it.

Quoted mapping values: wrong

With the following conflicted YAML file, mergiraf picks Left instead of combining Left and Right:


cat >quot-confl.yaml <<'EOF'
l:
<<<<<<< HEAD
- k: "b"
||||||| base
  - k: "b"
=======
  - k: "RIGHT"
>>>>>>>
EOF

mergiraf solve --stdout quot-confl.yaml

# ACTUAL OUTPUT:
# --------------------------
# INFO Solved all conflicts.
# l:
# - k: "b"
# --------------------------

# EXPECTED OUTPUT:
# --------------------------
# INFO Solved all conflicts.
# l:
# - k: "RIGHT"
# --------------------------

The conflicted file above represents this situation:

  • introduced changes vs. base:
    • Left: reduced the indentation (stylistic)
    • Right: modified the value of the mapping
  • note: these two changes are orthogonal - the resolved file should have both

Non-quoted mapping values: correct

If the mapping values are not quoted, mergiraf does combine the changes, i.e. does what I expect:


cat >bare-confl.yaml <<'EOF'
l:
<<<<<<< HEAD
- k: b
||||||| base
  - k: b
=======
  - k: RIGHT
>>>>>>>
EOF

mergiraf solve --stdout bare-confl.yaml

# ACTUAL + EXPECTED OUTPUT:
# --------------------------
# INFO Solved all conflicts.
# l:
# - k: RIGHT
# --------------------------

Details

mergiraf --version
# mergiraf 0.7.0
# (0.6.0 has the same behaviour)

tree-sitter syntax tree + mergiraf debug output

In case helpful.

TL; DR

  • the tree-sitter syntax tree has no obvious problem with either the quoted or unqoted version (I tested the Left version)
  • mergiraf doesn't do much at all with the quoted (= problematic) version

commands:


cat >quot-left.yaml <<'EOF'
l:
- k: "b"
EOF

cat >bare-left.yaml <<'EOF'
l:
- k: b
EOF

for stem in quot bare
do
  echo
  echo ---$stem---
  echo
  echo tree-sitter, $stem-left.yaml:
  tree-sitter parse --debug-graph $stem-left.yaml
  echo
  echo mergiraf:
  mergiraf solve -v --stdout $stem-confl.yaml 2>&1
  echo
done

...prints:


---quot---

tree-sitter, quot-left.yaml:
(stream [0, 0] - [2, 0]
  (document [0, 0] - [2, 0]
    (block_node [0, 0] - [2, 0]
      (block_mapping [0, 0] - [2, 0]
        (block_mapping_pair [0, 0] - [2, 0]
          key: (flow_node [0, 0] - [0, 1]
            (plain_scalar [0, 0] - [0, 1]
              (string_scalar [0, 0] - [0, 1])))
          value: (block_node [1, 0] - [2, 0]
            (block_sequence [1, 0] - [2, 0]
              (block_sequence_item [1, 0] - [2, 0]
                (block_node [1, 2] - [2, 0]
                  (block_mapping [1, 2] - [2, 0]
                    (block_mapping_pair [1, 2] - [1, 8]
                      key: (flow_node [1, 2] - [1, 3]
                        (plain_scalar [1, 2] - [1, 3]
                          (string_scalar [1, 2] - [1, 3])))
                      value: (flow_node [1, 5] - [1, 8]
                        (double_quote_scalar [1, 5] - [1, 8])))))))))))))

mergiraf:
DEBUG re-constructing revisions from parsed merge took 334ns
DEBUG initializing the parser took 4.292µs
DEBUG parsing all three files took 70.208µs
DEBUG matching base to left
DEBUG matching base to right
DEBUG top-down phase yielded 24 matches
DEBUG top-down phase yielded 24 matches
DEBUG matching took 20.125µs
DEBUG matching took 29.375µs
DEBUG matching left to right
DEBUG top-down phase yielded 0 matches
DEBUG matching took 28µs
DEBUG matching all three pairs took 128.625µs
DEBUG constructing the classmapping took 10.375µs
DEBUG generating PCS triples
DEBUG generating PCS triples took 4.791µs
DEBUG number of triples: 2
DEBUG cleaning up PCS triples took 2.458µs
DEBUG ⊥ build_subtree_from_changeset
DEBUG Exact(stream:0…14@Base/.L./)
DEBUG deleted and modified: 
DEBUG really deleted children: 
DEBUG parents to recompute: 
DEBUG constructing the merged tree took 24.833µs
DEBUG post-processing the merged tree for signature conflicts took 167ns
DEBUG Exact(stream:0…14@Base/.L./)
INFO Solved all conflicts.
l:
- k: "b"


---bare---

tree-sitter, bare-left.yaml:
(stream [0, 0] - [2, 0]
  (document [0, 0] - [2, 0]
    (block_node [0, 0] - [2, 0]
      (block_mapping [0, 0] - [2, 0]
        (block_mapping_pair [0, 0] - [2, 0]
          key: (flow_node [0, 0] - [0, 1]
            (plain_scalar [0, 0] - [0, 1]
              (string_scalar [0, 0] - [0, 1])))
          value: (block_node [1, 0] - [2, 0]
            (block_sequence [1, 0] - [2, 0]
              (block_sequence_item [1, 0] - [2, 0]
                (block_node [1, 2] - [2, 0]
                  (block_mapping [1, 2] - [2, 0]
                    (block_mapping_pair [1, 2] - [1, 6]
                      key: (flow_node [1, 2] - [1, 3]
                        (plain_scalar [1, 2] - [1, 3]
                          (string_scalar [1, 2] - [1, 3])))
                      value: (flow_node [1, 5] - [1, 6]
                        (plain_scalar [1, 5] - [1, 6]
                          (string_scalar [1, 5] - [1, 6]))))))))))))))

mergiraf:
DEBUG re-constructing revisions from parsed merge took 333ns
DEBUG initializing the parser took 4.333µs
DEBUG parsing all three files took 67µs
DEBUG matching base to left
DEBUG matching base to right
DEBUG top-down phase yielded 23 matches
DEBUG top-down phase yielded 3 matches
DEBUG matching took 21.75µs
DEBUG discarding match with similarity 0.375, close to threshold 0.4
DEBUG discarding match with similarity 0.33333334, close to threshold 0.4
DEBUG discarding match with similarity 0.36842105, close to threshold 0.4
DEBUG discarding match with similarity 0.35, close to threshold 0.4
DEBUG discarding match with similarity 0.33333334, close to threshold 0.4
DEBUG discarding match with similarity 0.3181818, close to threshold 0.4
DEBUG matching took 85.167µs
DEBUG matching left to right
DEBUG top-down phase yielded 0 matches
DEBUG matching took 24.292µs
DEBUG matching all three pairs took 181.291µs
DEBUG constructing the classmapping took 9.625µs
DEBUG generating PCS triples
DEBUG generating PCS triples took 38.917µs
DEBUG number of triples: 23
DEBUG number of conflicting triples: 1
DEBUG eliminating (plain_scalar:10…11@Base, ⊣, string_scalar:10…11@Base, Base) by (plain_scalar:10…11@Base, ⊣, string_scalar:10…15@Right, Right)
DEBUG number of conflicting triples: 1
DEBUG eliminating (plain_scalar:10…11@Base, string_scalar:10…11@Base, ⊢, Base) by (plain_scalar:10…11@Base, string_scalar:10…15@Right, ⊢, Right)
DEBUG cleaning up PCS triples took 25µs
DEBUG ⊥ build_subtree_from_changeset
DEBUG  stream:0…12@Base build_subtree_from_changeset
DEBUG   _bgn_imp_doc:0…11@Base build_subtree_from_changeset
DEBUG    block_node:0…11@Base build_subtree_from_changeset
DEBUG     block_mapping:0…11@Base build_subtree_from_changeset
DEBUG      block_mapping_pair:0…11@Base build_subtree_from_changeset
DEBUG       flow_node:0…1@Base build_subtree_from_changeset
DEBUG        plain_scalar:0…1@Base build_subtree_from_changeset
DEBUG         string_scalar:0…1@Base build_subtree_from_changeset
DEBUG       ::1…2@Base build_subtree_from_changeset
DEBUG       block_node:5…11@Base build_subtree_from_changeset
DEBUG        block_sequence:5…11@Base build_subtree_from_changeset
DEBUG         block_sequence_item:5…11@Base build_subtree_from_changeset
DEBUG          -:5…6@Base build_subtree_from_changeset
DEBUG          block_node:7…11@Base build_subtree_from_changeset
DEBUG           block_mapping:7…11@Base build_subtree_from_changeset
DEBUG            block_mapping_pair:7…11@Base build_subtree_from_changeset
DEBUG             ::8…9@Base build_subtree_from_changeset
DEBUG             flow_node:10…11@Base build_subtree_from_changeset
DEBUG              plain_scalar:10…11@Base build_subtree_from_changeset
DEBUG               string_scalar:10…15@Right build_subtree_from_changeset
DEBUG              plain_scalar:10…11@Base Checking unvisited base node string_scalar:10…11@Base
DEBUG               string_scalar:10…11@Base build_subtree_from_changeset
DEBUG Mixed(stream:0…12@Base
  Mixed(_bgn_imp_doc:0…11@Base
    Mixed(block_node:0…11@Base
      Mixed(block_mapping:0…11@Base
        Mixed(block_mapping_pair:0…11@Base
          Exact(flow_node:0…1@Base/BLR/)
          Exact(::1…2@Base/BLR/)
          Exact(block_node:5…11@Base/..R/)
        )
      )
    )
  )
)
DEBUG deleted and modified: 
DEBUG really deleted children: 
DEBUG parents to recompute: 
DEBUG constructing the merged tree took 87.25µs
DEBUG post-processing the merged tree for signature conflicts took 2.042µs
DEBUG Mixed(stream:0…12@Base
  Mixed(_bgn_imp_doc:0…11@Base
    Mixed(block_node:0…11@Base
      Mixed(block_mapping:0…11@Base
        Mixed(block_mapping_pair:0…11@Base
          Exact(flow_node:0…1@Base/BLR/)
          Exact(::1…2@Base/BLR/)
          Exact(block_node:5…11@Base/..R/)
        )
      )
    )
  )
)
INFO Solved all conflicts.
l:
- k: RIGHT
It seems `mergiraf` solves a conflict incorrectly for a YAML file with double-quoted values in it. ## Quoted mapping values: wrong With the following conflicted YAML file, `mergiraf` picks _Left_ instead of combining _Left_ and _Right_: ```bash cat >quot-confl.yaml <<'EOF' l: <<<<<<< HEAD - k: "b" ||||||| base - k: "b" ======= - k: "RIGHT" >>>>>>> EOF mergiraf solve --stdout quot-confl.yaml # ACTUAL OUTPUT: # -------------------------- # INFO Solved all conflicts. # l: # - k: "b" # -------------------------- # EXPECTED OUTPUT: # -------------------------- # INFO Solved all conflicts. # l: # - k: "RIGHT" # -------------------------- ``` The conflicted file above represents this situation: * introduced changes vs. `base`: * _Left_: reduced the indentation (stylistic) * _Right_: modified the value of the mapping * note: these two changes are orthogonal - the resolved file should have both ## Non-quoted mapping values: correct If the mapping values are _not quoted_, `mergiraf` does combine the changes, i.e. does what I expect: ```bash cat >bare-confl.yaml <<'EOF' l: <<<<<<< HEAD - k: b ||||||| base - k: b ======= - k: RIGHT >>>>>>> EOF mergiraf solve --stdout bare-confl.yaml # ACTUAL + EXPECTED OUTPUT: # -------------------------- # INFO Solved all conflicts. # l: # - k: RIGHT # -------------------------- ``` ## Details ```bash mergiraf --version # mergiraf 0.7.0 # (0.6.0 has the same behaviour) ``` ### tree-sitter syntax tree + mergiraf debug output In case helpful. **TL; DR** * the `tree-sitter` syntax tree has no obvious problem with either the quoted or unqoted version (I tested the _Left_ version) * `mergiraf` doesn't do much at all with the quoted (= problematic) version --- commands: ```bash cat >quot-left.yaml <<'EOF' l: - k: "b" EOF cat >bare-left.yaml <<'EOF' l: - k: b EOF for stem in quot bare do echo echo ---$stem--- echo echo tree-sitter, $stem-left.yaml: tree-sitter parse --debug-graph $stem-left.yaml echo echo mergiraf: mergiraf solve -v --stdout $stem-confl.yaml 2>&1 echo done ``` ...prints: ``` ---quot--- tree-sitter, quot-left.yaml: (stream [0, 0] - [2, 0] (document [0, 0] - [2, 0] (block_node [0, 0] - [2, 0] (block_mapping [0, 0] - [2, 0] (block_mapping_pair [0, 0] - [2, 0] key: (flow_node [0, 0] - [0, 1] (plain_scalar [0, 0] - [0, 1] (string_scalar [0, 0] - [0, 1]))) value: (block_node [1, 0] - [2, 0] (block_sequence [1, 0] - [2, 0] (block_sequence_item [1, 0] - [2, 0] (block_node [1, 2] - [2, 0] (block_mapping [1, 2] - [2, 0] (block_mapping_pair [1, 2] - [1, 8] key: (flow_node [1, 2] - [1, 3] (plain_scalar [1, 2] - [1, 3] (string_scalar [1, 2] - [1, 3]))) value: (flow_node [1, 5] - [1, 8] (double_quote_scalar [1, 5] - [1, 8]))))))))))))) mergiraf: DEBUG re-constructing revisions from parsed merge took 334ns DEBUG initializing the parser took 4.292µs DEBUG parsing all three files took 70.208µs DEBUG matching base to left DEBUG matching base to right DEBUG top-down phase yielded 24 matches DEBUG top-down phase yielded 24 matches DEBUG matching took 20.125µs DEBUG matching took 29.375µs DEBUG matching left to right DEBUG top-down phase yielded 0 matches DEBUG matching took 28µs DEBUG matching all three pairs took 128.625µs DEBUG constructing the classmapping took 10.375µs DEBUG generating PCS triples DEBUG generating PCS triples took 4.791µs DEBUG number of triples: 2 DEBUG cleaning up PCS triples took 2.458µs DEBUG ⊥ build_subtree_from_changeset DEBUG Exact(stream:0…14@Base/.L./) DEBUG deleted and modified: DEBUG really deleted children: DEBUG parents to recompute: DEBUG constructing the merged tree took 24.833µs DEBUG post-processing the merged tree for signature conflicts took 167ns DEBUG Exact(stream:0…14@Base/.L./) INFO Solved all conflicts. l: - k: "b" ---bare--- tree-sitter, bare-left.yaml: (stream [0, 0] - [2, 0] (document [0, 0] - [2, 0] (block_node [0, 0] - [2, 0] (block_mapping [0, 0] - [2, 0] (block_mapping_pair [0, 0] - [2, 0] key: (flow_node [0, 0] - [0, 1] (plain_scalar [0, 0] - [0, 1] (string_scalar [0, 0] - [0, 1]))) value: (block_node [1, 0] - [2, 0] (block_sequence [1, 0] - [2, 0] (block_sequence_item [1, 0] - [2, 0] (block_node [1, 2] - [2, 0] (block_mapping [1, 2] - [2, 0] (block_mapping_pair [1, 2] - [1, 6] key: (flow_node [1, 2] - [1, 3] (plain_scalar [1, 2] - [1, 3] (string_scalar [1, 2] - [1, 3]))) value: (flow_node [1, 5] - [1, 6] (plain_scalar [1, 5] - [1, 6] (string_scalar [1, 5] - [1, 6])))))))))))))) mergiraf: DEBUG re-constructing revisions from parsed merge took 333ns DEBUG initializing the parser took 4.333µs DEBUG parsing all three files took 67µs DEBUG matching base to left DEBUG matching base to right DEBUG top-down phase yielded 23 matches DEBUG top-down phase yielded 3 matches DEBUG matching took 21.75µs DEBUG discarding match with similarity 0.375, close to threshold 0.4 DEBUG discarding match with similarity 0.33333334, close to threshold 0.4 DEBUG discarding match with similarity 0.36842105, close to threshold 0.4 DEBUG discarding match with similarity 0.35, close to threshold 0.4 DEBUG discarding match with similarity 0.33333334, close to threshold 0.4 DEBUG discarding match with similarity 0.3181818, close to threshold 0.4 DEBUG matching took 85.167µs DEBUG matching left to right DEBUG top-down phase yielded 0 matches DEBUG matching took 24.292µs DEBUG matching all three pairs took 181.291µs DEBUG constructing the classmapping took 9.625µs DEBUG generating PCS triples DEBUG generating PCS triples took 38.917µs DEBUG number of triples: 23 DEBUG number of conflicting triples: 1 DEBUG eliminating (plain_scalar:10…11@Base, ⊣, string_scalar:10…11@Base, Base) by (plain_scalar:10…11@Base, ⊣, string_scalar:10…15@Right, Right) DEBUG number of conflicting triples: 1 DEBUG eliminating (plain_scalar:10…11@Base, string_scalar:10…11@Base, ⊢, Base) by (plain_scalar:10…11@Base, string_scalar:10…15@Right, ⊢, Right) DEBUG cleaning up PCS triples took 25µs DEBUG ⊥ build_subtree_from_changeset DEBUG stream:0…12@Base build_subtree_from_changeset DEBUG _bgn_imp_doc:0…11@Base build_subtree_from_changeset DEBUG block_node:0…11@Base build_subtree_from_changeset DEBUG block_mapping:0…11@Base build_subtree_from_changeset DEBUG block_mapping_pair:0…11@Base build_subtree_from_changeset DEBUG flow_node:0…1@Base build_subtree_from_changeset DEBUG plain_scalar:0…1@Base build_subtree_from_changeset DEBUG string_scalar:0…1@Base build_subtree_from_changeset DEBUG ::1…2@Base build_subtree_from_changeset DEBUG block_node:5…11@Base build_subtree_from_changeset DEBUG block_sequence:5…11@Base build_subtree_from_changeset DEBUG block_sequence_item:5…11@Base build_subtree_from_changeset DEBUG -:5…6@Base build_subtree_from_changeset DEBUG block_node:7…11@Base build_subtree_from_changeset DEBUG block_mapping:7…11@Base build_subtree_from_changeset DEBUG block_mapping_pair:7…11@Base build_subtree_from_changeset DEBUG ::8…9@Base build_subtree_from_changeset DEBUG flow_node:10…11@Base build_subtree_from_changeset DEBUG plain_scalar:10…11@Base build_subtree_from_changeset DEBUG string_scalar:10…15@Right build_subtree_from_changeset DEBUG plain_scalar:10…11@Base Checking unvisited base node string_scalar:10…11@Base DEBUG string_scalar:10…11@Base build_subtree_from_changeset DEBUG Mixed(stream:0…12@Base Mixed(_bgn_imp_doc:0…11@Base Mixed(block_node:0…11@Base Mixed(block_mapping:0…11@Base Mixed(block_mapping_pair:0…11@Base Exact(flow_node:0…1@Base/BLR/) Exact(::1…2@Base/BLR/) Exact(block_node:5…11@Base/..R/) ) ) ) ) ) DEBUG deleted and modified: DEBUG really deleted children: DEBUG parents to recompute: DEBUG constructing the merged tree took 87.25µs DEBUG post-processing the merged tree for signature conflicts took 2.042µs DEBUG Mixed(stream:0…12@Base Mixed(_bgn_imp_doc:0…11@Base Mixed(block_node:0…11@Base Mixed(block_mapping:0…11@Base Mixed(block_mapping_pair:0…11@Base Exact(flow_node:0…1@Base/BLR/) Exact(::1…2@Base/BLR/) Exact(block_node:5…11@Base/..R/) ) ) ) ) ) INFO Solved all conflicts. l: - k: RIGHT ```
wetneb commented 2025-05-11 21:23:57 +02:00
Owner
Copy link

Thanks a lot for reporting this, and for the thorough investigation! I'll likely have a try at solving this soon.

Thanks a lot for reporting this, and for the thorough investigation! I'll likely have a try at solving this soon.
👍 1
wetneb self-assigned this 2025-05-12 08:54:14 +02:00
carlwr commented 2025-05-15 11:15:26 +02:00
Author
Copy link

I installed mergiraf head and can confirm that my example conflict is now solved correctly.

Great with the quick reaction + fix! :D

I installed mergiraf `head` and can confirm that my example conflict is now solved correctly. Great with the quick reaction + fix! :D
🎉 2
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
marker_length_detection
comm-parents-check-have-all-sigs
benchmark_utils
v0.15.0
v0.14.0
v0.13.0
v0.12.1
v0.12.0
v0.11.0
v0.10.0
v0.9.0
v0.8.1
v0.8.0
v0.7.0
v0.6.0
v0.5.1
v0.5.0
v0.4.0
preview-v0.4.0-alpha1
v0.3.1
v0.3.0
v0.2.0
v0.1.0
Labels
Clear labels   
Compat/Breaking

Breaking change that won't be backward compatible

  
Kind
Bad merge
Examples where the merging algorithm doesn't work as well as it should

  
Kind
Bug
Something is not working

  
Kind
Documentation
Documentation changes

  
Kind
Enhancement
Improve existing functionality

  
Kind
Feature
New functionality

  
Kind
New language

  
Kind
Security
This is security issue

  
Kind
Testing
Issue or pull request related to testing

  
Priority
Critical
The priority is critical

  
Priority
High
The priority is high

  
Priority
Low
The priority is low

  
Priority
Medium
The priority is medium

  
Reviewed
Confirmed
Issue has been confirmed

  
Reviewed
Duplicate
This issue or pull request already exists

  
Reviewed
Invalid
Invalid issue

  
Reviewed
Won't Fix
This issue won't be fixed

  
Status
Abandoned
Somebody has started to work on this but abandoned work

  
Status
Blocked
Something is blocking this issue or pull request

  
Status
Need More Info
Feedback is required to reproduce issue or to continue work

No labels
Compat/Breaking
Kind
Bad merge
Kind
Bug
Kind
Documentation
Kind
Enhancement
Kind
Feature
Kind
New language
Kind
Security
Kind
Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
wetneb
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: mergiraf/mergiraf#379
No description provided.
Delete branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?