Skip to content

Support frozen strings#75

Merged
mfazekas merged 4 commits intonet-ssh:masterfrom
npezza93:master
Feb 16, 2026
Merged

Support frozen strings#75
mfazekas merged 4 commits intonet-ssh:masterfrom
npezza93:master

Conversation

@npezza93
Copy link
Copy Markdown
Contributor

No description provided.

Comment thread net-scp.gemspec Outdated
spec.add_runtime_dependency(%q<net-ssh>, [">= 2.6.5", "< 8.0.0"])
spec.add_development_dependency(%q<test-unit>, [">= 0"])
spec.add_development_dependency(%q<mocha>, [">= 0"])
spec.add_dependency(%q<mocha>, [">= 0", "<2.1"])
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why was it converted to non dev dependency?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Weird, no idea. hah. Reverted

@robertcheramy
Copy link
Copy Markdown
Collaborator

Why should frozen-string-literals be enabled in the CI?
If we want to see the warnings about frozen string literals in the CI, we should use -W:deprecated and add ruby 3.4 as a target.

I also think it is better to have command = String.new('scp ') than the complicated test about the ruby version.

@npezza93
Copy link
Copy Markdown
Contributor Author

npezza93 commented Jan 14, 2025

Why should frozen-string-literals be enabled in the CI?

To catch any other places that would fail if frozen strings were enabled. And to also catch regressions

-W:deprecated

I think this just outputs warnings. In CI we would want actual failures.

Looks like the failures are due to net-ssh not yet supporting frozen strings

@robertcheramy
Copy link
Copy Markdown
Collaborator

This is not a good solution to fail all tests as long as Net::SCP didn't fix its frozen string literals, because no CI test will work and we will not be able to see if a PR is OK or not. Frozen string literals will only be default in Ruby 4.0, so we have no rush and out CI will fail against ruby-head as soon as it is make default there.

I propose not to enable frozen string literals, and to modify Rakefile to warn about them:

Rake::TestTask.new do |t|
  t.ruby_opts = ['-W:deprecated']
  t.libs = ["lib", "test"]
end

This way, we can see where there is work to do, and we still can use the CI to check against all ruby versions.

@nodeg
Copy link
Copy Markdown

nodeg commented Oct 2, 2025

Any updates on this?

@morgoth
Copy link
Copy Markdown

morgoth commented Nov 5, 2025

It would be very helpful to be string-freeze safe.
In our app we enabled frozen strings by default.

Frozen string literals will only be default in Ruby 4.0, so we have no rush

Let's get ready :-)

@npezza93
Copy link
Copy Markdown
Contributor Author

npezza93 commented Nov 5, 2025

Tests are failing due to net-ssh not supporting frozen strings. I have addressed that here: net-ssh/net-ssh#958

Comment thread lib/net/scp.rb Outdated
@morgoth
Copy link
Copy Markdown

morgoth commented Dec 9, 2025

@mfazekas Looks like ready to be merged

@mfazekas mfazekas merged commit 8b3a4e9 into net-ssh:master Feb 16, 2026
willnet added a commit to willnet/net-scp that referenced this pull request Apr 3, 2026
Support for frozen string literals was merged in net-ssh#75, but it seems some places still need fixes.

`FrozenError` still occurs on CI:
https://github.com/willnet/net-scp/actions/runs/23930369904/job/69796023235#logs

There are destructive string operations in places such as https://github.com/net-ssh/net-scp/blob/f9cc5acef1af70061508a54b004ce17ef19028e8/lib/net/scp/download.rb#L43 , so make it explicit that the affected strings are not frozen.
willnet added a commit to willnet/net-scp that referenced this pull request Apr 3, 2026
Support for frozen string literals was merged in net-ssh#75, but it seems some places still need fixes.

`FrozenError` still occurs on CI:
https://github.com/willnet/net-scp/actions/runs/23931026231/job/69797935847#step:4:36

There are destructive string operations in places such as https://github.com/net-ssh/net-scp/blob/f9cc5acef1af70061508a54b004ce17ef19028e8/lib/net/scp/download.rb#L43 , so make it explicit that the affected strings are not frozen.
willnet added a commit to willnet/net-scp that referenced this pull request Apr 3, 2026
Support for frozen string literals was merged in net-ssh#75, but it seems some places still need fixes.

`FrozenError` still occurs on CI:
https://github.com/willnet/net-scp/actions/runs/23931026231/job/69797935847#step:4:36

There are destructive string operations in places such as https://github.com/net-ssh/net-scp/blob/f9cc5acef1af70061508a54b004ce17ef19028e8/lib/net/scp/download.rb#L43 , so make it explicit that the affected strings are not frozen.
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.

5 participants