igrigorik / em-http-request

Asynchronous HTTP Client (EventMachine + Ruby)
1.22k stars 220 forks source link

Using cookiejar2 fork? #354

Closed dorianmariecom closed 7 months ago

dorianmariecom commented 2 years ago

Hi, I made a fork to fix warnings, what do you think of adding it as a dependency?

https://rubygems.org/gems/cookiejar2

https://github.com/dorianmariefr/cookiejar2

See https://github.com/dwaite/cookiejar/pulls

And https://github.com/dwaite/cookiejar/commit/5f85661eb661673460e7ad1a04a98111566e2537 that was not released

dorianmariecom commented 2 years ago

See https://github.com/faye/faye/issues/537 for the other dependency that depends on cookiejar from rails

mtasaka commented 11 months ago

Now the original cookiejar site is archived so it is no longer maintained, cookiejar upstream suggests to use cookiejar2: https://github.com/dwaite/cookiejar/commit/36f692bfb66808a40285f47ae536a81f29a502dc

voxik commented 11 months ago

@dwaite I am just curious, wouldn't it be possible to takeover the original project and keep the name?

mattbrictson commented 10 months ago

FYI, it looks like em-http-request completely breaks on Ruby 3.3 due to a problem in the unmaintained cookiejar gem, which is required by em-http/client.rb.

ArgumentError:
  wrong number of arguments (given 3, expected 1..2)
# ./vendor/bundle/ruby/3.3.0+0/gems/cookiejar-0.3.3/lib/cookiejar/cookie_validation.rb:48:in `initialize'
# ./vendor/bundle/ruby/3.3.0+0/gems/cookiejar-0.3.3/lib/cookiejar/cookie_validation.rb:48:in `new'
# ./vendor/bundle/ruby/3.3.0+0/gems/cookiejar-0.3.3/lib/cookiejar/cookie_validation.rb:48:in `<module:CookieValidation>'
# ./vendor/bundle/ruby/3.3.0+0/gems/cookiejar-0.3.3/lib/cookiejar/cookie_validation.rb:25:in `<module:CookieJar>'
# ./vendor/bundle/ruby/3.3.0+0/gems/cookiejar-0.3.3/lib/cookiejar/cookie_validation.rb:5:in `<top (required)>'
# ./vendor/bundle/ruby/3.3.0+0/gems/cookiejar-0.3.3/lib/cookiejar/cookie.rb:3:in `<top (required)>'
# ./vendor/bundle/ruby/3.3.0+0/gems/cookiejar-0.3.3/lib/cookiejar.rb:1:in `<top (required)>'
# ./vendor/bundle/ruby/3.3.0+0/gems/em-http-request-1.1.7/lib/em-http/client.rb:1:in `<top (required)>'
# ./vendor/bundle/ruby/3.3.0+0/gems/em-http-request-1.1.7/lib/em-http.rb:17:in `<top (required)>'

We are seeing ruby-head CI failures due to this in the webmock project. It would be great to have this fixed before Ruby 3.3 is released in December. Is there anything I can do to help?

mtasaka commented 10 months ago

FYI, it looks like em-http-request completely breaks on Ruby 3.3 due to a problem in the unmaintained cookiejar gem, which is required by em-http/client.rb.

This is actually fixed in forked cookiejar2 : https://github.com/dorianmariefr/cookiejar2/commit/2b3670f6558637dab5402546c92fae4f57da1401

dwaite commented 10 months ago

@dwaite I am just curious, wouldn't it be possible to takeover the original project and keep the name?

I have not been able to find advice from the ruby ecosystem on how to do this without risking supply chain attacks. Thus preferring a fork by a motivated new maintainer over transferring the project.

mattbrictson commented 10 months ago

I have taken over ownership of a few Ruby projects. In my experience, it was as simple as writing the owner a polite message (either email or in a GitHub issue), and the answer was "sure, here you go". That requires a certain level of trust, and I can totally understand an owner being wary about handing over gem publishing rights to a complete stranger.

The other option is to put the project up for "adoption" on rubygems: https://blog.rubygems.org/2022/01/19/rubygems-adoptions.html

paddor commented 8 months ago

@dwaite Just curious, what keeps you from unarchiving the repo, implementing this one-line fix, and releasing v0.3.4?

MatheusRich commented 8 months ago

I've opened a PR (#358) and emailed Ilya to see if we can get it reviewed and merged. 🤞🏽

dwaite commented 8 months ago

@dwaite Just curious, what keeps you from unarchiving the repo, implementing this one-line fix, and releasing v0.3.4?

Done. I would still recommend migration to the fork.

paddor commented 8 months ago

@dwaite Thanks a lot. It's much appreciated. The longterm goal is to migrate away from EM to Async but things take time... Thanks again.

MatheusRich commented 8 months ago

@dwaite TY for releasing a new version! Do you think you could put up the gem for adoption so @dorianmariefr can be the new maintainer?

dorianmarie commented 7 months ago

I got added as a gem owner (just accepted the invitation), thanks @dwaite