# HG changeset patch # User Georges Racinet # Date 1646326059 -3600 # Thu Mar 03 17:47:39 2022 +0100 # Branch heptapod-stable # Node ID fd9d65989b10cabf4292169f666c88891695bff1 # Parent f29df4bad5a25823690aca9436e47ad6efcca988 NativeMercurial::Migrator: separate method for Mercurial state files It will make it easier to shuffle things around diff --git a/lib/heptapod/native_mercurial/migrator.rb b/lib/heptapod/native_mercurial/migrator.rb --- a/lib/heptapod/native_mercurial/migrator.rb +++ b/lib/heptapod/native_mercurial/migrator.rb @@ -128,6 +128,15 @@ ) end + # ensure all gitlab specific state files are present + def hg_ensure_state_files + renew_exclusive_lease + output, status = @project.repository.hg_call( + ["hpd-ensure-all-gitlab-specific-state-files"], force_system_user: true + ) + raise StandardError, output.strip unless status == 0 + end + def do_migrate Gitlab::AppLogger.info( message: "Starting native hg migration", @@ -140,13 +149,7 @@ migrate_pipelines migrate_notes migrate_releases - - # ensure all gitlab specific state files are present - renew_exclusive_lease - output, status = @project.repository.hg_call( - ["hpd-ensure-all-gitlab-specific-state-files"], force_system_user: true - ) - raise StandardError, output.strip unless status == 0 + hg_ensure_state_files @project.vcs_type = 'hg' timeless_save(@project) # HG changeset patch # User Georges Racinet # Date 1646326398 -3600 # Thu Mar 03 17:53:18 2022 +0100 # Branch heptapod-stable # Node ID 8ce0cdc7b8e796e6695ce9dfedb9df4f402e2d71 # Parent fd9d65989b10cabf4292169f666c88891695bff1 Mercurial native migration: doing notes last Notes are the most numerous and (up to now) the most likely to break. See for instance heptapod#630 diff --git a/lib/heptapod/native_mercurial/migrator.rb b/lib/heptapod/native_mercurial/migrator.rb --- a/lib/heptapod/native_mercurial/migrator.rb +++ b/lib/heptapod/native_mercurial/migrator.rb @@ -147,8 +147,8 @@ migrate_issues migrate_merge_requests migrate_pipelines + migrate_releases migrate_notes - migrate_releases hg_ensure_state_files @project.vcs_type = 'hg' # HG changeset patch # User Georges Racinet # Date 1646326530 -3600 # Thu Mar 03 17:55:30 2022 +0100 # Branch heptapod-stable # Node ID 15684bb2e6e8540ad8f71399ecba5c4f7b6e31e7 # Parent 8ce0cdc7b8e796e6695ce9dfedb9df4f402e2d71 Mercurial native migration: doing repo state files first This is needed for proper repository operation, even if in practice the files are supposed to already exist for repositories with recent activity. diff --git a/lib/heptapod/native_mercurial/migrator.rb b/lib/heptapod/native_mercurial/migrator.rb --- a/lib/heptapod/native_mercurial/migrator.rb +++ b/lib/heptapod/native_mercurial/migrator.rb @@ -144,12 +144,12 @@ project_full_path: @project.full_path ) + hg_ensure_state_files migrate_issues migrate_merge_requests migrate_pipelines migrate_releases migrate_notes - hg_ensure_state_files @project.vcs_type = 'hg' timeless_save(@project) # HG changeset patch # User Georges Racinet # Date 1646326690 -3600 # Thu Mar 03 17:58:10 2022 +0100 # Branch heptapod-stable # Node ID 7679b25695b8c64b2239a2ca6b1d9f3ab7551ddd # Parent 15684bb2e6e8540ad8f71399ecba5c4f7b6e31e7 NativeMercurial::Migrator: separate method to set vcs_type Will make it easier to change when and if it is done. diff --git a/lib/heptapod/native_mercurial/migrator.rb b/lib/heptapod/native_mercurial/migrator.rb --- a/lib/heptapod/native_mercurial/migrator.rb +++ b/lib/heptapod/native_mercurial/migrator.rb @@ -137,6 +137,11 @@ raise StandardError, output.strip unless status == 0 end + def set_final_vcs_type + @project.vcs_type = 'hg' + timeless_save(@project) + end + def do_migrate Gitlab::AppLogger.info( message: "Starting native hg migration", @@ -150,9 +155,7 @@ migrate_pipelines migrate_releases migrate_notes - - @project.vcs_type = 'hg' - timeless_save(@project) + set_final_vcs_type rescue StandardError => err msg = "Native mercurial migration failed: #{err}" log_error(err, msg) # HG changeset patch # User Georges Racinet # Date 1646327111 -3600 # Thu Mar 03 18:05:11 2022 +0100 # Branch heptapod-stable # Node ID c8c5ec7e295a9dbd2d10ef5b237e0032f80aea4a # Parent 7679b25695b8c64b2239a2ca6b1d9f3ab7551ddd Native Mercurial migration: always set VCS type if MRs are done If the Merge Requests are already migrated, not setting `vcs_type` to `hg` is counter-productive and harms current reviews by breaking all diffs and commit references. Therefore, even in the face of errors, if we reach that point, it's better to make the project native and wait for resolution of problems. diff --git a/lib/heptapod/native_mercurial/migrator.rb b/lib/heptapod/native_mercurial/migrator.rb --- a/lib/heptapod/native_mercurial/migrator.rb +++ b/lib/heptapod/native_mercurial/migrator.rb @@ -149,9 +149,12 @@ project_full_path: @project.full_path ) + rollback_vcs_type = true hg_ensure_state_files + migrate_merge_requests + rollback_vcs_type = false + migrate_issues - migrate_merge_requests migrate_pipelines migrate_releases migrate_notes @@ -160,6 +163,8 @@ msg = "Native mercurial migration failed: #{err}" log_error(err, msg) raise + ensure + set_final_vcs_type unless rollback_vcs_type end def migrate_issues diff --git a/spec/lib/heptapod/native_mercurial/migrator_spec.rb b/spec/lib/heptapod/native_mercurial/migrator_spec.rb --- a/spec/lib/heptapod/native_mercurial/migrator_spec.rb +++ b/spec/lib/heptapod/native_mercurial/migrator_spec.rb @@ -291,6 +291,22 @@ end end + describe '#do_migrate' do + it "sets VCS Type to native even if there are errors if Merge Requests are migrated" do + error = StandardError.new("oops") + expect(subject).to receive(:migrate_issues).and_raise(error) + expect { subject.do_migrate }.to raise_error(error) + expect(project.vcs_type).to eq('hg') + end + + it "does not set VCS Type to native if Merge Requests are not migrated" do + error = StandardError.new("oops") + expect(subject).to receive(:migrate_merge_requests).and_raise(error) + expect { subject.do_migrate }.to raise_error(error) + expect(project.vcs_type).to eq('hg_git') + end + end + # Best approximation to equality of timestamps # # True equality of timestamps is probably impossible, because # HG changeset patch # User Georges Racinet # Date 1646327619 -3600 # Thu Mar 03 18:13:39 2022 +0100 # Branch heptapod-stable # Node ID 324b6a46e4078e3ac72e7901943f7eb53dc2d44d # Parent c8c5ec7e295a9dbd2d10ef5b237e0032f80aea4a Mercurial native migration: swallow errors on Notes It is problematic to crash the process if a single MR or Commit comment cannot be migrated (see heptapod#630). We don't have means to provide proper user feedback (heptapod#643), but there are two cases depending on how the migration is triggered - API (expected to be the most common case): crash resulted in a failed background job, and is therefore not visible by the end user. The end user cannot access logs. Hence user feedback is the same with this changeset. - Rake task: crash was visible, the end user can access logs. A simple user feedback is implemented here by returning errors in Notes and outputing them in the rake task. So it seems that there is litlle value in crashing in this case, despite the "fail early, fail often" slogan that we otherwise adhere to. diff --git a/lib/heptapod/native_mercurial/migrator.rb b/lib/heptapod/native_mercurial/migrator.rb --- a/lib/heptapod/native_mercurial/migrator.rb +++ b/lib/heptapod/native_mercurial/migrator.rb @@ -95,13 +95,15 @@ # take care of all startup initializations and final cleanups, # like a Python context manager would. def enter + res = nil try_obtain_exclusive_lease do with_project_readonly do vcs_type_enforcer do - yield + res = yield end end end + res end def migrate! @@ -142,12 +144,17 @@ timeless_save(@project) end + # perform the migration, returning a mapping of errors per type + # + # Most errors will raise an exception instead of being added to + # the return. def do_migrate Gitlab::AppLogger.info( message: "Starting native hg migration", project_id: @project.id, project_full_path: @project.full_path ) + errors = {} rollback_vcs_type = true hg_ensure_state_files @@ -157,8 +164,10 @@ migrate_issues migrate_pipelines migrate_releases - migrate_notes + errors[:notes] = migrate_notes set_final_vcs_type + + errors rescue StandardError => err msg = "Native mercurial migration failed: #{err}" log_error(err, msg) @@ -212,6 +221,7 @@ def migrate_notes count = 0 + failed = [] @project.notes.each do |note| count += 1 renew_exclusive_lease if count % EXCLUSIVE_LEASE_NOTES_RENEWAL_RATE == 0 @@ -222,8 +232,9 @@ end rescue StandardError => err log_error(err, "Failed to migrate Note id=#{note.id}") - raise + failed << note.id end + failed end def migrate_note_positions(note) diff --git a/lib/tasks/migrate/migrate_to_native_hg.rake b/lib/tasks/migrate/migrate_to_native_hg.rake --- a/lib/tasks/migrate/migrate_to_native_hg.rake +++ b/lib/tasks/migrate/migrate_to_native_hg.rake @@ -33,8 +33,12 @@ check_gitaly_connection check_hgitaly_connection migrator = Heptapod::NativeMercurial::Migrator.new(project, user) - migrator.migrate! - puts 'done!'.color(:green) + errors = migrator.migrate! + if errors.each_value.all? { |errs| errs.empty? } + puts 'done!'.color(:green) + else + puts "there are errors for #{errors}. Please check application log for details".color(:yellow) + end end end end diff --git a/spec/lib/heptapod/native_mercurial/migrator_spec.rb b/spec/lib/heptapod/native_mercurial/migrator_spec.rb --- a/spec/lib/heptapod/native_mercurial/migrator_spec.rb +++ b/spec/lib/heptapod/native_mercurial/migrator_spec.rb @@ -305,6 +305,26 @@ expect { subject.do_migrate }.to raise_error(error) expect(project.vcs_type).to eq('hg_git') end + + context "about Notes" do + let!(:issue) do + create(:issue, + project: project, + description: "Sample description", + title: "Sample title") + end + + let!(:my_comment) do + create(:note_on_issue, project: project, noteable: issue, + note: "Sample comment") + end + + it "returns errors" do + expect(subject).to receive(:migrate_note_positions) + .and_raise(StandardError.new('failed')) + expect(subject.migrate!).to eq({ notes: [my_comment.id] }) + end + end end # Best approximation to equality of timestamps