Skip to content

Conversation

@MSP-Greg
Copy link
Contributor

@nagachika

Thanks for your work with this.

Before the first revert, an encoding error caused a test-all failure.

For a number of years, Windows MSYS2 (mingw & ucrt) build's CI needed encoding changes in the shell. Removed from test-all, and also changed test & test-all to run from a cmd shell instead of a bash shell.

After doing so, readline-ext had one encoding failure. It is not installed in Ruby 3.3, so I removed the file readline.so, which then uses reline.

All tests pass. Also, this should be the case, as now the mingw CI is identical to the Windows/mswin CI...

@nobu nobu changed the title Revert "Revert "Ruby 3.2 - Speed up rebuilding the loaded feature index and realpath cache (#8023) Re-apply "Ruby 3.2 - Speed up rebuilding the loaded feature index and realpath cache (#8023)" Aug 20, 2023
@MSP-Greg
Copy link
Contributor Author

@nobu

Thank you, much better than the double 'revert'.

Re the changes in the workflow file, the encoding code dates back to the commit running mingw on Actions, which was when 3.0 just started development (2019-12-31). I still had similar code in ruby-loco, and removed all of it this morning. CI passes for all three builds (mingw, ucrt, & mswin). I think it probably existed due to readline (now readline-ext), which now isn't used on Windows?

So, maybe add the changes to master?

@nagachika
Copy link
Member

@MSP-Greg It's OK to change the shell in GitHub Actions for MinGW. I would prefer to skip the failing readline test instead of removing readline.so on MinGW.
Please see my branch.
https://github.com/nagachika/ruby/commits/00-win-ruby
I have tried to create a PR for your branch, but I cannot find the base repository on the GitHub comparing page. 🤔

@MSP-Greg
Copy link
Contributor Author

@nagachika

Re the shell change, glad that's ok. I keep an eye on a few extension gems, and also https://github.com/oneclick/rubyinstaller2. Most people are using Windows shells, not the Git or MSYS2 bash shells. Also, most run CI from the default Actions shell, which is PowerShell. There are exceptions...

Re the readline issue, I checked readline-ext, and it doesn't run CI on Windows.

Skipping the test is fine by me.

I have tried to create a PR for your branch, but I cannot find the base repository on the GitHub comparing page.

Hmm. Not sure. When I've been in your position, I don't think I've ever done a PR, I've only added commits. I think I've done so both from cmd line and the desktop app. But, force pushes made a mess of things.

So, should I change the first commit title/subject to match @nobu's suggestion, squash the CI commits, and add your nagachika@c7a3de8?

BTW, thanks.

@MSP-Greg
Copy link
Contributor Author

@nagachika

The changes I mentioned above are at https://github.com/MSP-Greg/ruby/commits/00-win-ruby-2

Running CI now. Does that look ok? Three commits, rebased also...

@nagachika
Copy link
Member

Thanks for rebasing. LGTM.

@nagachika nagachika enabled auto-merge August 21, 2023 15:26
@nagachika nagachika added this pull request to the merge queue Aug 21, 2023
Merged via the queue into ruby:ruby_3_2 with commit 788b03d Aug 21, 2023
@MSP-Greg MSP-Greg deleted the 00-win-ruby branch August 22, 2023 02:04
nagachika added a commit that referenced this pull request Aug 24, 2023
nagachika added a commit that referenced this pull request Aug 24, 2023
@hsbt hsbt added the Backport label Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants