Project

General

Profile

Actions

Feature #36320

open

Migrate to Rails 7.1

Added by Go MAEDA about 2 years ago. Updated about 24 hours ago.

Status:
Reopened
Priority:
Normal
Category:
Rails support
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed

Files

0001-Upgrade-to-Rails-7.patch (5.05 KB) 0001-Upgrade-to-Rails-7.patch Takashi Kato, 2022-04-10 15:35
0002-Fix-deprecation-warning.patch (5.73 KB) 0002-Fix-deprecation-warning.patch Takashi Kato, 2022-04-10 15:35
0004-Use-Selenium-WebDriver-Wait.patch (1.58 KB) 0004-Use-Selenium-WebDriver-Wait.patch Takashi Kato, 2022-04-10 15:35
0003-Replace-to_s-db-to-to_fs-db.patch (10.9 KB) 0003-Replace-to_s-db-to-to_fs-db.patch Takashi Kato, 2022-04-10 15:35
0001-Upgrade-to-rails-7.1-beta1.patch (25.7 KB) 0001-Upgrade-to-rails-7.1-beta1.patch patch for r22317 Takashi Kato, 2023-09-24 17:42
20231117_36320.patch (26.1 KB) 20231117_36320.patch r22429@trunk Minoru Maeda, 2023-11-17 06:13
0001-Fix-autoloading-plugin-classes.patch (2.83 KB) 0001-Fix-autoloading-plugin-classes.patch Takashi Kato, 2023-11-28 13:12
0001v2-Fix-autoloading-plugin-classes.patch (4.66 KB) 0001v2-Fix-autoloading-plugin-classes.patch patch for r22488 Takashi Kato, 2023-11-28 15:44

Related issues

Related to Redmine - Patch #36317: Set default protect from forgery trueClosedMarius BĂLTEANU

Actions
Related to Redmine - Patch #36355: Update roadie-rails to 3.0ClosedGo MAEDA

Actions
Related to Redmine - Patch #35536: Use webpack to improve javascripts and stylesheets managementNew

Actions
Related to Redmine - Patch #39110: Replacing request_store with ActiveSupport::CurrentAttributesClosedMarius BĂLTEANU

Actions
Related to Redmine - Defect #37732: Fix "DEPRECATION WARNING: Rendering actions with '.' in the name is deprecated" in RepositoriesControllerClosedMarius BĂLTEANU

Actions
Related to Redmine - Defect #39760: Some API tests fail with Ruby 2.7ClosedMarius BĂLTEANU

Actions
Related to Redmine - Defect #39747: Diff of a javascript file in repository module is not displayed with layout ClosedMarius BĂLTEANU

Actions
Related to Redmine - Defect #39803: Plugin modules no longer workingNeeds feedbackGuus Teley

Actions
Related to Redmine - Defect #39834: Extract tests for plugin autoloading and Restore Plugin directory settingsClosedMarius BĂLTEANU

Actions
Related to Redmine - Patch #40148: Update activerecord-sqlserver-adapter to 7.1ClosedGo MAEDA

Actions
Related to Redmine - Patch #40210: Remove overrides that inserts a non-breaking space (nbsp) to empty option elementsClosedMarius BĂLTEANU

Actions
Actions #1

Updated by Go MAEDA about 2 years ago

  • Related to Patch #36317: Set default protect from forgery true added
Actions #2

Updated by Go MAEDA about 2 years ago

Actions #3

Updated by Marius BĂLTEANU about 2 years ago

  • Related to Patch #35536: Use webpack to improve javascripts and stylesheets management added
Actions #4

Updated by Go MAEDA almost 2 years ago

  • Target version changed from Unplanned backlogs to 6.0.0
Actions #6

Updated by Go MAEDA almost 2 years ago

Thank you for posting the patches.

I think we have to wait until the release of Redmine 5.1.0 to upgrade Rails because 5.1.0 should be Rails 6.1 based.

Actions #7

Updated by Takashi Kato 5 months ago

I've updated a patch that is compatible with Rails 7.1 beta. As for the WebDriver patch, I have linked it to another issue 37558.

Actions #8

Updated by Jun NAITOH 3 months ago

https://endoflife.date/rails

6.1
(01 Jun 2024)

https://github.com/rails/rails/pull/46895#issuecomment-1673353127

6.1 is not getting anymore releases unless it's a severe vulnerability and will be EOL next June.

I think Redmine needs to upgrade to Rails 7 before Rails 6.1 EOL.

Actions #9

Updated by M T 3 months ago

Not a bad effort there Takashi Kato, I gave your patch a quick go with Rails 7.1 and it seemed to work with standard Redmine 5.1 release branch.

However one thing that got broke is the autoloading of classes and modules from plugin folders, not sure why. Perhaps you could check this out... I know there have been some subtle changes in this area in Rails. (Also is there a reason you deleted the zeitwerk config line?)

Cheers
MT

Actions #10

Updated by Minoru Maeda 3 months ago

Takashi Kato wrote in #note-7:

I've updated a patch that is compatible with Rails 7.1 beta. As for the WebDriver patch, I have linked it to another issue 37558.

Thank you for the great patch. I applied the 0001-Upgrade-to-rails-7.1-beta1.patch to r22429@trunk and ran tests, but some of them failed.

  • test/integration/sudo_mode_test.rb
  • test/integration/api_test/my_test.rb and test/integration/api_test/news_test.rb
    • It seems there is a problem where XML and JSON requests don't return the expected content-type.
  • test/unit/project_nested_set_concurrency_test.rb:34
    • It looks like assert_difference is evaluated without waiting for the Thread to finish.

Based on the patch you posted, I made corrections so that test/integration/api_test would pass. Also, I updated to Rails v7.1.2, which was released on November 10, 2023.
However, the failures in test/integration/sudo_mode_test.rb and test/unit/project_nested_set_concurrency_test.rb have not been resolved.

I hope this helps.

Actions #11

Updated by Marius BĂLTEANU 3 months ago

  • Assignee set to Marius BĂLTEANU
Actions #12

Updated by Marius BĂLTEANU 3 months ago

  • Related to Patch #39110: Replacing request_store with ActiveSupport::CurrentAttributes added
Actions #13

Updated by Marius BĂLTEANU 3 months ago

  • Subject changed from Migrate to Rails 7 to Migrate to Rails 7.1
  • Status changed from New to Resolved
  • Resolution set to Fixed

I've committed the patch that updates Rails to 7.1.2 on trunk. Thanks Takashi Kato for your work!

Actions #14

Updated by Takashi Kato 3 months ago

A fix included in rails 7.1.2 (https://github.com/rails/rails/pull/49636) causes autoloading of plugins to fail.
A patch to fix this is attached.

Actions #15

Updated by Marius BĂLTEANU 3 months ago

Thanks!

Can you take a look also on the failing tests from here? I will do it as well this week.

Actions #16

Updated by Takashi Kato 3 months ago

Sorry for your time.
It is difficult to fix autoload_paths after initialize, so I've changed the method.

Actions #17

Updated by Marius BĂLTEANU 3 months ago

  • Related to Defect #37732: Fix "DEPRECATION WARNING: Rendering actions with '.' in the name is deprecated" in RepositoriesController added
Actions #18

Updated by Mischa The Evil 3 months ago

Takashi Kato wrote in #note-16:

Sorry for your time.
It is difficult to fix autoload_paths after initialize, so I've changed the method.

Just my two cents on this patch:
  • This somewhat seems to converge with #24007 (albeit not as a user config option directly) and as such has the issue mentioned in #24007#note-13 that 'plugins' directory remains hard-coded in source:/trunk/Gemfile@22489#L125.
  • The comments included in this patch contain a typo (s/tesr/test).
Actions #19

Updated by Mizuki ISHIKAWA 3 months ago

Takashi Kato wrote in #note-16:

Sorry for your time.
It is difficult to fix autoload_paths after initialize, so I've changed the method.

I was conducting an investigation without realizing that a patch had already been posted. Akira Matsuda assisted me in this research.
The problem is likely to have been caused by the changes to https://github.com/rails/rails/commit/ebfca905db .

The following changes will also solve the problem:

diff --git a/lib/redmine/plugin_loader.rb b/lib/redmine/plugin_loader.rb
index 2151e949b..e1b58baa7 100644
--- a/lib/redmine/plugin_loader.rb
+++ b/lib/redmine/plugin_loader.rb
@@ -126,7 +126,7 @@ module Redmine
         # Add the plugin directories to rails autoload paths
         engine_cfg = Rails::Engine::Configuration.new(directory.to_s)
         engine_cfg.paths.add 'lib', eager_load: true
-        engine_cfg.eager_load_paths.each do |dir|
+        engine_cfg.all_eager_load_paths.each do |dir|
           Rails.autoloaders.main.push_dir dir
           Rails.application.config.watchable_dirs[dir] = [:rb]
         end

https://github.com/redmica/redmica_ui_extension plugin can reproduce autoloading-related problems.

Actions #20

Updated by Marius BĂLTEANU 3 months ago

Thanks Takashi Kato and Mizuki ISHIKAWA for investigating this issue, I've committed the fix with all_eager_load_paths because it seems to call both methods eager_load_paths + paths.eager_load.

Actions #21

Updated by Marius BĂLTEANU 3 months ago

Committed also the test and the changes required for the test to pass, thanks!

Actions #22

Updated by Marius BĂLTEANU 3 months ago

Mischa The Evil wrote in #note-18:

[...]

Just my two cents on this patch:

It looks like we are close to support changing the "plugins" directory path from the configuration file, but we still need more time to invest in order to properly support and document this change.

Actions #23

Updated by Marius BĂLTEANU 3 months ago

  • Related to Defect #39760: Some API tests fail with Ruby 2.7 added
Actions #24

Updated by Marius BĂLTEANU 3 months ago

  • Related to Defect #39747: Diff of a javascript file in repository module is not displayed with layout added
Actions #25

Updated by Marius BĂLTEANU 3 months ago

It seems that after this update, some concurrency tests randomly fail, for example:

Is someone able to reproduce the problem on the local environment?

Actions #26

Updated by Marius BĂLTEANU 3 months ago

  • Related to Defect #39803: Plugin modules no longer working added
Actions #27

Updated by Minoru Maeda 3 months ago

Marius BALTEANU wrote in #note-25:

Is someone able to reproduce the problem on the local environment?

I encountered a test failure in project_nested_set_concurrency_test.rb on my M2 chip MacBook Air using a Docker environment (with postgres:14.9 and ruby:2.7.8).

I'm not sure of the reason, but it seems to be related to https://github.com/rack/rack-session#compatibility. After modifying the Gemfile as follows and running bundle update, the test for project_nested_set_concurrency_test.rb passed with the seed value that previously led to failure.

diff --git a/Gemfile b/Gemfile
index 908c32343..d5870ad95 100644
--- a/Gemfile
+++ b/Gemfile
@@ -3,6 +3,7 @@ source 'https://rubygems.org'
 ruby '>= 2.7.0', '< 3.3.0'

 gem 'rails', '7.1.2'
+gem 'rack-session', '~> 1.0'
 gem 'rouge', '~> 4.2.0'
 gem 'mini_mime', '~> 1.1.0'
 gem "actionpack-xml_parser" 
Actions #28

Updated by Marius BĂLTEANU 3 months ago

  • Related to Defect #39834: Extract tests for plugin autoloading and Restore Plugin directory settings added
Actions #29

Updated by Minoru Maeda 3 months ago

Minoru Maeda wrote in #note-27:

I'm not sure of the reason, but it seems to be related to https://github.com/rack/rack-session#compatibility. After modifying the Gemfile as follows and running bundle update, the test for project_nested_set_concurrency_test.rb passed with the seed value that previously led to failure.

I've discovered the reason for the failure of the test in project_nested_set_concurrency_test.rb. The issue appears to be that assert_difference is evaluating Project.count based on cached SQL values, leading to the test failure.

Implementing the following modifications to trunk@ r22518 should resolve the issue, allowing the test in project_nested_set_concurrency_test.rb to pass. Note that my environment with PostgreSQL 14 and Ruby 2.7.

diff --git a/Gemfile b/Gemfile
index d5870ad95..908c32343 100644
--- a/Gemfile
+++ b/Gemfile
@@ -3,7 +3,6 @@ source 'https://rubygems.org'
 ruby '>= 2.7.0', '< 3.3.0'

 gem 'rails', '7.1.2'
-gem 'rack-session', '~> 1.0'
 gem 'rouge', '~> 4.2.0'
 gem 'mini_mime', '~> 1.1.0'
 gem "actionpack-xml_parser" 
diff --git a/test/unit/project_nested_set_concurrency_test.rb b/test/unit/project_nested_set_concurrency_test.rb
index e835d2b50..077731040 100644
--- a/test/unit/project_nested_set_concurrency_test.rb
+++ b/test/unit/project_nested_set_concurrency_test.rb
@@ -38,7 +38,7 @@ class ProjectNestedSetConcurrencyTest < ActiveSupport::TestCase
     p = generate_project!
     p.destroy

-    assert_difference 'Project.count', 60 do
+    assert_difference 'Project.async_count.value', 60 do
       threads = []
       3.times do |i|
         threads << Thread.new(i) do
Actions #30

Updated by Marius BĂLTEANU 2 months ago

Minoru Maeda wrote in #note-29:

Minoru Maeda wrote in #note-27:

I'm not sure of the reason, but it seems to be related to https://github.com/rack/rack-session#compatibility. After modifying the Gemfile as follows and running bundle update, the test for project_nested_set_concurrency_test.rb passed with the seed value that previously led to failure.

I've discovered the reason for the failure of the test in project_nested_set_concurrency_test.rb. The issue appears to be that assert_difference is evaluating Project.count based on cached SQL values, leading to the test failure.

Implementing the following modifications to trunk@ r22518 should resolve the issue, allowing the test in project_nested_set_concurrency_test.rb to pass. Note that my environment with PostgreSQL 14 and Ruby 2.7.

[...]

Thanks, committed!

Actions #31

Updated by Marius BĂLTEANU 2 months ago

  • Status changed from Resolved to Closed

The update to Rails 7.1 is finally completed, please report any problem in new issue.

Thanks again to all contributors involved in this update!

Actions #34

Updated by Mischa The Evil 2 months ago

  • Private changed from Yes to No
Actions #35

Updated by Go MAEDA 28 days ago

  • Related to Patch #40148: Update activerecord-sqlserver-adapter to 7.1 added
Actions #36

Updated by Go MAEDA 26 days ago

  • Status changed from Closed to Reopened

A test is failing after updating Rails to 7.1.3 in r22665.

Error:
ProjectNestedSetConcurrencyTest#test_concurrency:
NoMethodError: undefined method `-' for #<ActiveRecord::Promise status=complete>
    test/unit/project_nested_set_concurrency_test.rb:41:in `test_concurrency'

bin/rails test test/unit/project_nested_set_concurrency_test.rb:34

This is caused by the following Rails issue. I am waiting for the release of Rails 7.1.4.
Rails 7.1.3: async_count returns a completed Promise instead of the value · Issue #50776 · rails/rails

Actions #37

Updated by Go MAEDA 19 days ago

  • Related to Patch #40210: Remove overrides that inserts a non-breaking space (nbsp) to empty option elements added
Actions #38

Updated by Marius BĂLTEANU about 24 hours ago

Go MAEDA wrote in #note-36:

A test is failing after updating Rails to 7.1.3 in r22665.

[...]

This is caused by the following Rails issue. I am waiting for the release of Rails 7.1.4.
Rails 7.1.3: async_count returns a completed Promise instead of the value · Issue #50776 · rails/rails

I've reverted the update to 7.1.3 until Rails 7.1.4 is released in order to have all the tests pass.

Actions

Also available in: Atom PDF