Fix frozen string literal issue for ruby 3.4.0

[chaadow]

To prepare for ruby 3.4.0 and making strings frozen by default, I've stumbled upon this string mutation.

Let me know if you need anything ( I didn't add tests since I assume tests exist for this feature )

[geemus]

Rather than changing away from concatenation below (which has memory usage impacts), could we change this to ensure it isn't frozen (since we really do want a non-frozen here). I think the syntax would be canonical_headers = +''.

[chaadow]

You are definitely right, let me change this real quick @geemus

[geemus]

@chaadow Thanks for catching this and providing a potential fix. I think I might like a slightly different approach, commented above, happy to discuss further. Thanks!

[chaadow]

@geemus done, thanks for the review!

[chaadow]

I would have preferred to enable frozen string literal for the entire gem, but I see that the CI uses a shared one.

Here is an example PR that implements it for rails-i18n (using RUBYOPT) https://github.com/svenfuchs/rails-i18n/pull/1120/files

Maybe you'd be interested to implement it there?

[geemus]

I could definitely see how setting RUBYOPTS like that could be a good way to find out sooner rather than later where we will need to fix things. Do you know what happens with those options on presently? Is in warnings, errors, other?

It could make sense to directly update the shared stuff, or we could add separate things for fog-aws specifically around this potentially. As I type this, it feels like maybe just doing it here locally to start and work out any issues might be good (and then we could move it to the shared stuff once it was proven out a bit more). What do you think?

[chaadow]

I could definitely see how setting RUBYOPTS like that could be a good way to find out sooner rather than later where we will need to fix things. Do you know what happens with those options on presently? Is in warnings, errors, other?

short answer:

• --enable-frozen-string-literal will raise an error.
• and in ruby 3.4 only with -W:depreacted ( or Warning[:deprecated] = true ) it will emit warnings.

Back when frozen string literals were introduced ( in ruby 2.3.0 in dec. 2015 ) you could run an entire ruby script ( or a rails app ) with the --enable-frozen-string-literal flag
Back then it was nearly and realistically impossible for any production app to enable this flag, as gems were not prepared.

However gems ( and gem maintainers), in isolation, could, in their CI only, make sure to run their unit tests with that flag, because probably soon ( with ruby 3.5 if i'm not mistaken ) it will be enabled by default.

Ruby 3.4 will start to emit deprecation warnings ( can already be tested with ruby 3.4.0-preview1 version) if the deprecation flag is enabled ( as described above)

==> here is a gist from the creator of zeitwerk and co-written by Jean Boussier, ruby commiter, and both rails core members that better explains the future of frozen string literals in ruby.

It could make sense to directly update the shared stuff, or we could add separate things for fog-aws specifically around this potentially. As I type this, it feels like maybe just doing it here locally to start and work out any issues might be good (and then we could move it to the shared stuff once it was proven out a bit more). What do you think?

Yes I agree, I think we can create a temporary .yml with the shared CI code + the RUBYOPTS part. ( we can test that it works by manually removing the +"" that i've added) and once the yml setup works, we can move it upstream to the shared CI, and create PRs for each gem of the fog ecosystem 👌🏼 @geemus

[geemus]

Thanks for the extra details, I hadn't been following this and had to read up on it around the original issue. Trying it out here before moving upstream sounds good to me. Do you have capacity to give that a try?

[chaadow]

@geemus Yes I do, I started a PR here #711