LegacyDiffNote st_diff being set incorrectly some of the time
From https://gitlab.com/gitlab-org/gitlab/-/issues/343345#note_710950301
We want to directly address the issue where st_diff
is sometimes correct on GitHub->GitLab->GitLab
imports and sometimes it is not.
It seems like it is somehow related to the merger_request_diff_files
perhaps not being set correctly somehow on export from GitLab and then not correctly imported.
Reproduce
- Import Nodejs repo from GitHub
- Look at the Diff notes for merge request
40009
, specifically the one with aline_code = 59863e22002f8881745543fff366759cb74364cc_374_374
- Export that project and then import file based again and look at the same
note
from merge request40009
- you can delete the
issues.ndjson
file and keep only the last1000
lines from themerge_requests.ndjson
to make it go faster.
- you can delete the
From my investigations the note from the original import(note
below) and the one from the export/import(bad_note
below) seem to have mistmatching merge_requests_diff_files
info(which is where the data below is eventually parsed from)
Notes...
current note diff files...
irb(main):015:0> note.diff_file.diff_lines.map(&:line_code)
=> [nil, "59863e22002f8881745543fff366759cb74364cc_371_371", "59863e22002f8881745543fff366759cb74364cc_372_372", "59863e22002f8881745543fff366759cb74364cc_373_373", "59863e22002f8881745543fff366759cb74364cc_374_374"]
last one matches
irb(main):016:0> note.line_code
st_diff to diff file, then that is used to lookup - many from the bad_note...when running the same and I don't know why
"59863e22002f8881745543fff366759cb74364cc_234_327", "59863e22002f8881745543fff366759cb74364cc_235_328", nil, "59863e22002f8881745543fff366759cb74364cc_1977_2070
irb(main):132:0> note.diff_file.diff_lines.size
=> 5
has the line_code in the diff_lines we need
irb(main):133:0> bad_note.diff_file.diff_lines.size
=> 150
does not have the line we need...
=> "59863e22002f8881745543fff366759cb74364cc_374_374"
2 version of raw_diffs for fs.md file on good note...one on bad note - did it export the wrong one somehow???
irb(main):146:0> note.noteable.merge_request_diff.raw_diffs.size
=> 4
irb(main):147:0> bad_note.noteable.merge_request_diff.raw_diffs.size
=> 3
irb(main):148:0> bad_note.noteable.merge_request_diff.raw_diffs.map(&:new_path)
=> ["doc/api/fs.md", "lib/internal/fs/promises.js", "test/parallel/test-fs-promises-file-handle-stream.js"]
irb(main):149:0> note.noteable.merge_request_diff.raw_diffs.map(&:new_path)
=> ["doc/api/fs.md", "doc/api/fs.md", "lib/internal/fs/promises.js", "test/parallel/test-fs-promises-file-handle-stream.js"]
irb(main):150:0> note.noteable.merge_request_diff.merge_request_diff_files.size
=> 3
- path to look at for variance in exact case...
- http://gitlab.com/gitlab-org/gitlab/blob/c56b1af721280e02910e9f5857b12b1a01c0f1c8/lib/gitlab/github_import/importer/pull_request_importer.rb#L72-72 -> http://gitlab.com/gitlab-org/gitlab/blob/c56b1af721280e02910e9f5857b12b1a01c0f1c8/lib/gitlab/import/merge_request_helpers.rb#L62-62 -> http://gitlab.com/gitlab-org/gitlab/blob/c56b1af721280e02910e9f5857b12b1a01c0f1c8/app/models/merge_request_diff.rb#L208-208
Related to #343345.
Edited by Doug Stull