diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 1e5e127072eb78a515bfc98f8e81c4d9dfb75052..de6e2f81595a4eb0cf10ba681e27b55d791444d0 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -20,8 +20,11 @@ def initialize( end def execute - # TODO: Update this message with the removal of FF merge_trains_create_ref_service and update tests - # This is for compatibility with MergeToRefService during the rollout. + # The "3:" prefix is for compatibility with the output of + # MergeToRefService, which is still used to create merge refs and some + # merge train refs. The prefix can be dropped once MergeToRefService is no + # longer used. See https://gitlab.com/gitlab-org/gitlab/-/issues/455421 + # and https://gitlab.com/gitlab-org/gitlab/-/issues/421025 return ServiceResponse.error(message: '3:Invalid merge source') unless first_parent_sha.present? result = { diff --git a/app/views/projects/settings/merge_requests/_merge_request_merge_method_settings.html.haml b/app/views/projects/settings/merge_requests/_merge_request_merge_method_settings.html.haml index 891bd62c0a412d508445cab65e60b4836e2a592f..3ce9630a333ffad80f91264c3210aacbe40b92d7 100644 --- a/app/views/projects/settings/merge_requests/_merge_request_merge_method_settings.html.haml +++ b/app/views/projects/settings/merge_requests/_merge_request_merge_method_settings.html.haml @@ -14,9 +14,7 @@ - ffTrains = s_('ProjectSettings|If merge trains are enabled, merging is only possible if the branch can be rebased without conflicts.') - ffTrainsHelp = link_to s_('ProjectSettings|What are merge trains?'), help_page_path('ci/pipelines/merge_trains', anchor: 'enable-merge-trains'), target: '_blank', rel: 'noopener noreferrer' -- ffTrainsWithFastForward = (noMergeCommit + "<br />" + ffOnly + "<br />" + ffConflictRebase + "<br />" + ffTrains + " " + ffTrainsHelp).html_safe -- ffTrainsWithoutFastForward = (noMergeCommit + "<br />" + ffOnly + "<br />" + ffConflictRebase).html_safe -- ffTrainsHelpFullHelpText = Feature.enabled?(:fast_forward_merge_trains_support) ? ffTrainsWithFastForward : ffTrainsWithoutFastForward +- ffTrainsHelpFullHelpText = (noMergeCommit + "<br />" + ffOnly + "<br />" + ffConflictRebase + "<br />" + ffTrains + " " + ffTrainsHelp).html_safe .form-group %b= s_('ProjectSettings|Merge method') diff --git a/config/feature_flags/development/fast_forward_merge_trains_support.yml b/config/feature_flags/development/fast_forward_merge_trains_support.yml deleted file mode 100644 index 9c814bb658e621f7175aa30e9c7ef0bd028120f1..0000000000000000000000000000000000000000 --- a/config/feature_flags/development/fast_forward_merge_trains_support.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: fast_forward_merge_trains_support -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/122787 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/282442 -milestone: '16.1' -type: development -group: group::pipeline execution -default_enabled: true diff --git a/config/feature_flags/development/merge_trains_create_ref_service.yml b/config/feature_flags/development/merge_trains_create_ref_service.yml deleted file mode 100644 index cdbe6813210904d973e056be121665008c097b50..0000000000000000000000000000000000000000 --- a/config/feature_flags/development/merge_trains_create_ref_service.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: merge_trains_create_ref_service -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/127531 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/420161 -milestone: '16.3' -type: development -group: group::pipeline execution -default_enabled: true diff --git a/doc/ci/pipelines/merge_trains.md b/doc/ci/pipelines/merge_trains.md index cd1b92e9dd7ee7433d41c66d8d7e3662ba85702c..e66fd70f285738688356d21b63c52e0d1dc2d32b 100644 --- a/doc/ci/pipelines/merge_trains.md +++ b/doc/ci/pipelines/merge_trains.md @@ -12,6 +12,7 @@ DETAILS: > - [In GitLab 16.0 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/359057), the **Start merge train** and **Start merge train when pipeline succeeds** buttons became **Set to auto-merge**. **Remove from merge train** became **Cancel auto-merge**. > - Support for [fast-forward](../../user/project/merge_requests/methods/index.md#fast-forward-merge) and [semi-linear](../../user/project/merge_requests/methods/index.md#merge-commit-with-semi-linear-history) merge methods [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/282442) in GitLab 16.5 [with a flag](../../administration/feature_flags.md) named `fast_forward_merge_trains_support`. Enabled by default. +> - [Feature flag `fast_forward_merge_trains_support` removed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148964#note_1855981445) in GitLab 16.11. In projects with frequent merges to the default branch, changes in different merge requests might conflict with each other. Use merge trains to put merge requests in a queue. diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb index a4874529686884e8b570a347dab25cc289a1588f..30ffc2bd2c52dd59ae045c006e7295204a0a916b 100644 --- a/ee/app/models/ee/merge_request.rb +++ b/ee/app/models/ee/merge_request.rb @@ -483,8 +483,6 @@ def merge_train override :should_be_rebased? def should_be_rebased? - return super if ::Feature.disabled?(:fast_forward_merge_trains_support) - return false if MergeTrains::Train.project_using_ff?(target_project) return false if merge_train_car&.on_ff_train? diff --git a/ee/app/models/merge_trains/car.rb b/ee/app/models/merge_trains/car.rb index 630302b06a762e415a80e2d2ce44e40f2246aed2..a015db46206e8500af152e0f7a0bba61277a6020 100644 --- a/ee/app/models/merge_trains/car.rb +++ b/ee/app/models/merge_trains/car.rb @@ -173,14 +173,10 @@ def active? end def on_ff_train? - if ::Feature.enabled?(:fast_forward_merge_trains_support, target_project) - commit_sha = merge_request.merge_params.dig('train_ref', 'commit_sha') - return false unless commit_sha.present? + commit_sha = merge_request.merge_params.dig('train_ref', 'commit_sha') + return false unless commit_sha.present? - active? && pipeline&.sha == commit_sha - else - false - end + active? && pipeline&.sha == commit_sha end def train diff --git a/ee/app/models/merge_trains/merge_commit_message.rb b/ee/app/models/merge_trains/merge_commit_message.rb index f3c4624ede502b8ebf51ab8659719620539d0f64..01fc9cb9f43d3a94531d66acb950621fe36f1690 100644 --- a/ee/app/models/merge_trains/merge_commit_message.rb +++ b/ee/app/models/merge_trains/merge_commit_message.rb @@ -2,7 +2,10 @@ module MergeTrains module MergeCommitMessage - # Remove with merge_trains_create_ref_service feature flag + # TODO: Remove with GitLab 18.0, or whenever we can relatively + # safely assume that no merge trains predating fast-forward merge + # support exist. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/455421 def self.legacy_value(merge_request, previous_ref) "Merge branch #{merge_request.source_branch} with #{previous_ref} " \ "into #{merge_request.train_ref_path}" diff --git a/ee/app/models/merge_trains/train.rb b/ee/app/models/merge_trains/train.rb index dd187f3e419b92f33e0b0e8f52bc44946467576a..a0f97a3eaefb1a489673a32f8616e2403b6cab83 100644 --- a/ee/app/models/merge_trains/train.rb +++ b/ee/app/models/merge_trains/train.rb @@ -12,8 +12,7 @@ def self.all_for_project(project) end def self.project_using_ff?(project) - ::Feature.enabled?(:fast_forward_merge_trains_support, project) && - project.merge_trains_enabled? && + project.merge_trains_enabled? && project.ff_merge_must_be_possible? end diff --git a/ee/app/services/merge_trains/create_pipeline_service.rb b/ee/app/services/merge_trains/create_pipeline_service.rb index 4358797cdffc02090a07ab311ffd1688ab565f6d..3a88513616ee6d933eed07c8e3cb3ddad560a779 100644 --- a/ee/app/services/merge_trains/create_pipeline_service.rb +++ b/ee/app/services/merge_trains/create_pipeline_service.rb @@ -31,7 +31,7 @@ def create_train_ref(merge_request, previous_ref, create_mergeable_ref) source_sha: merge_request.diff_head_sha, first_parent_ref: previous_ref ).execute.to_h.transform_keys do |key| - # TODO: Remove this transformation with FF merge_trains_create_ref_service + # TODO: Remove this transformation with https://gitlab.com/gitlab-org/gitlab/-/issues/455421 case key when :commit_sha then :commit_id when :source_sha then :source_id @@ -40,6 +40,7 @@ def create_train_ref(merge_request, previous_ref, create_mergeable_ref) end end else + # TODO: Remove in https://gitlab.com/gitlab-org/gitlab/-/issues/455421 ::MergeRequests::MergeToRefService.new( project: merge_request.target_project, current_user: merge_request.merge_user, diff --git a/ee/app/services/merge_trains/refresh_merge_request_service.rb b/ee/app/services/merge_trains/refresh_merge_request_service.rb index cd9b90a067383e0ecce8997bd7ce5b4accb8e9e0..d361b2b3018a1e32965e2195883f19526ca7d0bb 100644 --- a/ee/app/services/merge_trains/refresh_merge_request_service.rb +++ b/ee/app/services/merge_trains/refresh_merge_request_service.rb @@ -51,16 +51,12 @@ def merge_from_train_ref? # Although it should be technically safe to merge from any mergeable train # ref, do so for fast-forward and semi-linear merge trains to avoid # disruption to standard merge commit merge trains. - return false unless project.ff_merge_must_be_possible? && Feature.enabled?(:fast_forward_merge_trains_support, project) + return false unless project.ff_merge_must_be_possible? mergeable_sha_and_message?(merge_train_car) end def create_mergeable_train_ref? - unless Feature.enabled?(:merge_trains_create_ref_service, merge_request.target_project) - return false - end - # The two checks below ensure that by construction, we can safely # fast-forward merge from any train ref satisfying # #mergeable_from_train_ref? diff --git a/ee/spec/models/merge_request_spec.rb b/ee/spec/models/merge_request_spec.rb index 98ec2bd2b14f1aa96f4cfa888c07c56dfdf508e6..06070fd1489617135de660ecf92487bcb888fff9 100644 --- a/ee/spec/models/merge_request_spec.rb +++ b/ee/spec/models/merge_request_spec.rb @@ -2144,18 +2144,6 @@ def create_mr(metrics_data = {}) it_behaves_like 'ff car on train' end - - context 'when the ff merge train feature is disabled' do - before do - stub_feature_flags(fast_forward_merge_trains_support: false) - end - - it { is_expected.to eq true } - - it 'will not run ff related code' do - expect(MergeTrains::Train).not_to receive(:project_using_ff?) - end - end end def stub_foss_conditions_met diff --git a/ee/spec/models/merge_trains/car_spec.rb b/ee/spec/models/merge_trains/car_spec.rb index 50646b483d2da9df135ea04979130b2be2c4df84..5391fb801d836d093ac79f59cd7461dc622d03cb 100644 --- a/ee/spec/models/merge_trains/car_spec.rb +++ b/ee/spec/models/merge_trains/car_spec.rb @@ -539,14 +539,6 @@ end it { is_expected.to eq(true) } - - context 'when the feature flag is disabled' do - before do - stub_feature_flags(fast_forward_merge_trains_support: false) - end - - it { is_expected.to eq(false) } - end end end diff --git a/ee/spec/models/merge_trains/train_spec.rb b/ee/spec/models/merge_trains/train_spec.rb index 7ff7682a470ba51a90e548f326f7b281c261c22b..9c99076ea64255fe7bc755c17ddbbd134147828d 100644 --- a/ee/spec/models/merge_trains/train_spec.rb +++ b/ee/spec/models/merge_trains/train_spec.rb @@ -31,19 +31,15 @@ describe '.project_using_ff?' do subject { described_class.project_using_ff?(target_project) } - where(:merge_trains_enabled, :ff_trains_enabled, :ff_merge_method, :expected) do - true | true | true | true - true | true | false | false - true | false | true | false - false | true | true | false - false | false | true | false - true | false | false | false + where(:merge_trains_enabled, :ff_merge_method, :expected) do + true | true | true + true | false | false + false | true | false end with_them do before do allow(target_project).to receive(:merge_trains_enabled?).and_return(merge_trains_enabled) - stub_feature_flags(fast_forward_merge_trains_support: ff_trains_enabled) allow(target_project).to receive(:ff_merge_must_be_possible?).and_return(ff_merge_method) end diff --git a/ee/spec/services/merge_trains/refresh_merge_request_service_spec.rb b/ee/spec/services/merge_trains/refresh_merge_request_service_spec.rb index a472a72ad79350cfae4d5b5f7e49c876fe70185d..868bc1c661799fb42efa0791b8839e6400098548 100644 --- a/ee/spec/services/merge_trains/refresh_merge_request_service_spec.rb +++ b/ee/spec/services/merge_trains/refresh_merge_request_service_spec.rb @@ -180,16 +180,6 @@ end end end - - context 'when FF merge_trains_create_ref_service is disabled' do - let(:expected_create_mergeable_ref) { false } - - before do - stub_feature_flags(merge_trains_create_ref_service: false) - end - - it_behaves_like 'creates a pipeline for merge train' - end end context 'when pipeline for merge train is running' do @@ -312,20 +302,6 @@ expect(merge_request.state).to eq("merged") end end - - context 'with feature flag fast_forward_merge_trains_support disabled' do - before do - stub_feature_flags(fast_forward_merge_trains_support: false) - end - - it 'uses the default merge strategy' do - expect_next_instance_of(MergeRequests::MergeService, project: project, current_user: maintainer, params: instance_of(HashWithIndifferentAccess)) do |service| - expect(service).to receive(:execute).with(merge_request, skip_discussions_check: true, check_mergeability_retry_lease: true) - end - - subject - end - end end context 'when it failed to merge the merge request' do diff --git a/spec/features/projects/settings/merge_requests_settings_spec.rb b/spec/features/projects/settings/merge_requests_settings_spec.rb index da54fbeaee4560f25bb67acf91d210ac2ac7e38d..1227767683156b1b9c438a6fe02a20f5b0ed9352 100644 --- a/spec/features/projects/settings/merge_requests_settings_spec.rb +++ b/spec/features/projects/settings/merge_requests_settings_spec.rb @@ -98,33 +98,9 @@ end end - describe 'With the fast_forward_merge_trains_support feature flag turned off' do - before do - sign_in(user) - stub_feature_flags(fast_forward_merge_trains_support: false) - - visit(project_settings_merge_requests_path(project)) - end - - it 'does not display the fast forward merge train message' do - page.within '.merge-request-settings-form' do - expect(page).not_to have_content 'merging is only possible if the branch can be rebased without conflicts.' - end - end - end - - describe 'With the fast_forward_merge_trains_support feature flag turned on' do - before do - sign_in(user) - stub_feature_flags(fast_forward_merge_trains_support: true) - - visit(project_settings_merge_requests_path(project)) - end - - it 'displays the fast forward merge train message' do - page.within '.merge-request-settings-form' do - expect(page).to have_content 'merging is only possible if the branch can be rebased without conflicts.' - end + it 'displays the fast forward merge train message' do + page.within '.merge-request-settings-form' do + expect(page).to have_content 'merging is only possible if the branch can be rebased without conflicts.' end end