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

Use shopify fork instead of original memcached #14

Closed
wants to merge 43 commits into from

Conversation

tomasv
Copy link
Member

@tomasv tomasv commented Apr 5, 2024

This should give us a better maintained memcached with more fixes.

hirocaster and others added 30 commits April 1, 2016 16:49
- memcached/lib/memcached/exceptions.rb:52: warning: instance variable @no_backtrace not initialized
- memcached/lib/memcached/exceptions.rb:61: warning: instance variable @no_backtrace not initialized
- memcached/lib/memcached/experimental.rb:25: warning: assigned but unused variable - flags
- test/unit/memcached_experimental_test.rb:92: warning: assigned but unused variable - result
- test/unit/memcached_experimental_test.rb:122: warning: assigned but unused variable - result
- test/unit/memcached_experimental_test.rb:176: warning: assigned but unused variable - result
- test/unit/memcached_experimental_test.rb:196: warning: assigned but unused variable - result
- test/unit/memcached_experimental_test.rb:194: warning: assigned but unused variable - value
- test/unit/memcached_test.rb:277: warning: assigned but unused variable - result
- test/unit/memcached_test.rb:295: warning: assigned but unused variable - result
- test/unit/memcached_test.rb:302: warning: assigned but unused variable - result
- test/unit/memcached_test.rb:309: warning: assigned but unused variable - result
- test/unit/memcached_test.rb:321: warning: assigned but unused variable - result
- test/unit/memcached_test.rb:328: warning: assigned but unused variable - result
- test/unit/memcached_test.rb:339: warning: assigned but unused variable - result
- test/unit/memcached_test.rb:949: warning: assigned but unused variable - response
- test/unit/memcached_test.rb:963: warning: assigned but unused variable - response
- memcached/lib/memcached/memcached.rb:637: warning: shadowing outer local variable - server
- memcached/lib/memcached/memcached.rb:700: warning: shadowing outer local variable - key
…ctions

Fix tests and migrate to Github Actions
The warning was

.../lib/memcached/memcached.rb:653: warning: method redefined; discarding old set_credentials
.../lib/memcached/auth.rb:8: warning: previous definition of set_credentials was here

Also, define all the methods for this class together in the same file,
since auth.rb is always required anyways.
The warnings was

.../lib/memcached/rails.rb:1: warning: .../lib/memcached/rails.rb:1: warning: loading in progress, circular require considered harmful - .../lib/memcached.rb
The warning was

test/unit/rails_test.rb:91: warning: instance variable @called not initialized
…arthurnn#199)

Avoids a "RSTRING_PTR is returning NULL!!" warning on ruby 3
This was an experimental feature that was disabled in JRuby 1.7 and
then removed.
We are no longer using OBJSETUP to create a ruby string and rubinius
has rb_external_str_new_with_enc, so we can just unconditionally use
that function now.
Regenerate the Swig bindings using Swig 4
…rthurnn#198)

Since the `!tries` check was never true (0 is truthy in ruby).
The tests for this method were getting skipped because the server
doesn't actually support this command.  I'm not sure when it was
supported. Support for this was already removed from libmemcached.
Since it was experimental in this library, we may as well just remove
it now.
Since the libmemcached upgrade is a breaking change, so we can't keep
the Rlibmemcached interface stable and memcached_get_from_last won't
be available.
Add deprecations and remove an experimental method
dylanahsmith and others added 13 commits May 13, 2021 09:35
I thought rb_external_str_new_with_enc did what I wanted, which was to
pass ownership of the C string allocation and have ruby free it when it
was done with it.  However, it actually just copies the string contents
like with rb_str_new.
…-touch

Clean vendor files before touching all of them to avoid missing changes
Fix unreleased memory leak from use of rb_external_str_new_with_enc
Patch libmemcached to handle EINTR in poll(2)
Specifically the problem is that when libmemcached is compiled, it can't
find libsasl.  Libsasl exists, but the function it's checking for on
macOS has been deprecated and that causes a compiler error.  Since
libmemcached is compiled without sasl support that makes the Ruby
extension blow up.
This code was likely fine back when it was written against Ruby 1.9.3,
but since then `Kernel#raise` now automatically attach the currently rescued error
as the new exception `cause`.

Since this code was re-raising the same instance over and once a `cause` was
attached it would never be de-associated, which in some contrived scenario could
leak to information leak across requests / test etc.

I ran a benchmark and the fastest way to raise exception I could find is:

```ruby
raise ErrorClass, "message".freeze, EMPTY_ARRAY, cause: nil
```
@tomasv tomasv closed this Apr 5, 2024
@tomasv tomasv deleted the feature/release-shopify-gem branch April 5, 2024 12:22
@tomasv tomasv restored the feature/release-shopify-gem branch April 5, 2024 12:27
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.

6 participants