Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

msi: fix slow start issue on Windows #631

Merged
merged 1 commit into from
Mar 13, 2024

Conversation

kenhys
Copy link
Contributor

@kenhys kenhys commented Mar 7, 2024

Until chef/win32-service#85 is merged,
use forked version of win32-service.

This fix should be applied to fluent-package not to block starting
fluentdwinsvc service on Windows.

See #618

Closes: #630

NOTE: even though just putting the following line
does not install forked version of win32-service, so
install it explicitly as same as fluentd gem.

  gem "win32-service", github: "fluent-plugins-nursery/win32-service",
  branch: "fluent-package", platforms: [:mingw, :x64_mingw]

@kenhys kenhys force-pushed the use-win32-service-fork branch from 28c4287 to c2059bf Compare March 7, 2024 04:05
@kenhys
Copy link
Contributor Author

kenhys commented Mar 7, 2024

forked version was fetched.


2024-03-07T04:35:47.5844462Z �[?25h�[?25lUsing win32-service 2.3.2 from https://github.com/fluent-plugins-nursery/win32-s��[?25h�[?25l
2024-03-07T04:35:47.5845782Z ervice.git (at fix-slow-start-with-ruby3@cfcc200)�[31X�[31C�[25;50H�[?25h�[?25l
2024-03-07T04:35:47.5847685Z �[80X�[80C

@daipom
Copy link
Contributor

daipom commented Mar 7, 2024

Thanks for this!
The branch fix-slow-start-with-ruby3 is being used for fixing the upstream (chef/win32-service#85).
Should we make a specific branch or tag for our package?

@kenhys kenhys force-pushed the use-win32-service-fork branch from c2059bf to c71f898 Compare March 7, 2024 05:06
@kenhys
Copy link
Contributor Author

kenhys commented Mar 7, 2024

The branch fix-slow-start-with-ruby3 is being used for fixing the upstream (chef/win32-service#85).
Should we make a specific branch or tag for our package?

fix-slow-start-with-ruby may be removed by default (when that PR was merged), so it is better to create a specific branch.

@kenhys kenhys force-pushed the use-win32-service-fork branch from c71f898 to 49263d4 Compare March 7, 2024 05:23
@kenhys
Copy link
Contributor Author

kenhys commented Mar 7, 2024

Added fluent-package branch and refer it from Gemfile.
https://github.com/fluent-plugins-nursery/win32-service/tree/fluent-package

@kenhys kenhys force-pushed the use-win32-service-fork branch from 49263d4 to c95b121 Compare March 7, 2024 05:34
fluent-package/Gemfile Outdated Show resolved Hide resolved
@daipom
Copy link
Contributor

daipom commented Mar 7, 2024

Added fluent-package branch and refer it from Gemfile. https://github.com/fluent-plugins-nursery/win32-service/tree/fluent-package

Thanks!!

@kenhys kenhys force-pushed the use-win32-service-fork branch from c95b121 to fa99bc3 Compare March 11, 2024 05:52
@kenhys
Copy link
Contributor Author

kenhys commented Mar 11, 2024

When processing :ruby_gems task, it raises Bundler::GemNotFound: Could not find gem 'win32-service' on Linux.
It seems a bit strange... 🤔

@kenhys
Copy link
Contributor Author

kenhys commented Mar 11, 2024

bundler v2.3.27 contains fix for regression. check it.
https://github.com/rubygems/rubygems/releases/tag/bundler-v2.3.27

@kenhys
Copy link
Contributor Author

kenhys commented Mar 12, 2024

win32-service", platforms: [:mingw, :x64_mingw] with install gem from local repo may not work as expected.

with install_gem_from_local_repo

  • bundler 2.3.26 failed
  • bundler 2.3.27 failed
  • bundler 2.4.22 failed
  • bundler 2.5.6 failed

It try to install win32-service on linux.

@kenhys
Copy link
Contributor Author

kenhys commented Mar 12, 2024

got it. It seems that the following context does not work.

source ... do
   gem ... , platforms: [:mingw, :x64_mingw] 
end

@kenhys kenhys force-pushed the use-win32-service-fork branch from fa99bc3 to dabb717 Compare March 12, 2024 05:12
@kenhys
Copy link
Contributor Author

kenhys commented Mar 12, 2024

Hmm, not reproducible failure yet about minimum Gemfile.

set -xe

rm -fr /tmp/local_gem_repo
mkdir -p /tmp/local_gem_repo
cp Gemfile Gemfile.lock /tmp/local_gem_repo

cd /tmp/local_gem_repo 
git clone https://github.com/fluent-plugins-nursery/win32-service.git --branch fluent-package
(cd win32-service && \
    rake build && \
    cp pkg/win32-service*.gem /tmp/local_gem_repo)
gem generate_index
TEST=yes bundle _2.3.26_ install

@kenhys
Copy link
Contributor Author

kenhys commented Mar 12, 2024

win32-service gem was built but not installed from local repo.

*** RSpec not available. (sudo) gem install rspec to run unit tests. ***        
^[[80X^[[80C
win32-service 2.3.2 built to pkg/win32-service-2.3.2.gem.^[[23X^[[23C
cd C:/fluent-package-5.0.2/fluent-package/local_gem_repo^[[24X^[[24C
gem generate_index^[[62X^[[62C
^[[80X^[[80C
^[[?25h^[[?25lGenerating Marshal quick index gemspecs for 2 gems^[[?25h^[[?25l^[[Hw^[[25;51H^[[?25h^[[?25l
..^[[78X^[[78C^[[25;3H^[[?25h^[[?25l
Complete^[[72X^[[72C^[[25;9H^[[?25h^[[?25l
Generated Marshal quick index gemspecs: 0.003s^[[34X^[[34C^[[25;47H^[[?25h^[[?25l
Generating specs index^[[58X^[[58C^[[25;23H^[[?25h^[[?25l
Generated specs index: 0.000s^[[51X^[[51C^[[25;30H^[[?25h^[[?25l
Generating latest specs index^[[51X^[[51C^[[25;30H^[[?25h^[[?25l
Generated latest specs index: 0.000s^[[44X^[[44C^[[25;37H^[[?25h^[[?25l
Generating prerelease specs index^[[47X^[[47C^[[25;34H^[[?25h^[[?25l
Generated prerelease specs index: 0.000s^[[40X^[[40C^[[25;41H^[[?25h^[[?25l
Compressing indices^[[61X^[[61C^[[25;20H^[[?25h^[[?25l
^[[80X^[[80C
^[[?25h^[[?25lCompressed indices: 0.003s^[[?25h^[[?25l
^[[80X^[[80C
^[[?25h^[[?25lcd -^[[?25h^[[?25l
cd C:/opt/fluent/share^[[58X^[[58C^[[25;23H^[[?25h^[[?25l

@kenhys kenhys force-pushed the use-win32-service-fork branch from dabb717 to e7d822d Compare March 12, 2024 09:45
@kenhys kenhys marked this pull request as ready for review March 12, 2024 11:19
@kenhys kenhys added this to the 5.0.3 milestone Mar 13, 2024
@kenhys
Copy link
Contributor Author

kenhys commented Mar 13, 2024

NOTE: sudo apt install colorized-logs and use ansi2txt < ... to eliminate it.

fluent-package/Rakefile Outdated Show resolved Hide resolved
@kenhys kenhys force-pushed the use-win32-service-fork branch 5 times, most recently from 14d465b to 7e873b8 Compare March 13, 2024 06:39
Until chef/win32-service#85 is merged,
use forked version of win32-service.

This fix should be applied to fluent-package not to block starting
fluentdwinsvc service on Windows.

See fluent#618

Closes: fluent#630

NOTE: even though just putting the following line
does not install forked version of win32-service, so
install it explicitly as same as fluentd gem.

  gem "win32-service", github: "fluent-plugins-nursery/win32-service",
  branch: "fluent-package", platforms: [:mingw, :x64_mingw]

Signed-off-by: Kentaro Hayashi <[email protected]>
@kenhys kenhys force-pushed the use-win32-service-fork branch from 7e873b8 to ca4bea6 Compare March 13, 2024 07:41
@kenhys
Copy link
Contributor Author

kenhys commented Mar 13, 2024

checking only end_with?("/lib/win32/windows/functions.rb") is not enough:

 Find.find("c:/opt/fluent/lib/ruby/gems/") do |f|
irb(main):061:1*   if f.end_with?("functions.rb")  then p f; end
irb(main):062:0> end
"c:/opt/fluent/lib/ruby/gems/3.2.0/gems/rexml-3.2.5/lib/rexml/functions.rb"
"c:/opt/fluent/lib/ruby/gems/3.2.0/gems/rexml-3.2.6/lib/rexml/functions.rb"
"c:/opt/fluent/lib/ruby/gems/3.2.0/gems/win32-eventlog-0.6.7/lib/win32/windows/functions.rb"
"c:/opt/fluent/lib/ruby/gems/3.2.0/gems/win32-service-2.3.2/lib/win32/windows/functions.rb"
=> nil

@kenhys
Copy link
Contributor Author

kenhys commented Mar 13, 2024

CI has passed !

@daipom
Copy link
Contributor

daipom commented Mar 13, 2024

Thanks so much!

Copy link
Member

@ashie ashie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it would be better to generalize the tasks to be applicable to other gems, I don't always require it in this pull request. Because we shouldn't use forked version of gems in usual.

@kenhys kenhys merged commit 44dfadd into fluent:master Mar 13, 2024
22 checks passed
@kenhys kenhys deleted the use-win32-service-fork branch March 13, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Install win32-service gem from a forked repository
3 participants