Skip to content

jruby extension#30

Open
kares wants to merge 5 commits into
net-ssh:mainfrom
kares:jruby-ext
Open

jruby extension#30
kares wants to merge 5 commits into
net-ssh:mainfrom
kares:jruby-ext

Conversation

@kares
Copy link
Copy Markdown

@kares kares commented May 29, 2026

adds a Java implementation (LLM generated based on current C sources)

the rest is plumbing for the JRuby extension -> -java variant of the gem

generated based on C sources
@kares kares marked this pull request as ready for review May 29, 2026 07:52
@kares kares marked this pull request as draft May 29, 2026 10:55
@kares kares marked this pull request as ready for review May 29, 2026 11:14
Copy link
Copy Markdown

@headius headius left a comment

Choose a reason for hiding this comment

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

Looks good to me overall. Will be worth running some benchmarks to clean up excess allocation. A few minor review comments should be addressed.

import java.util.Arrays;

public final class BCryptPbkdf {
private static final int BLF_N = 16;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fix indentation to match Java standard, otherwise this will be a constant maintenance headache.

engine.getSingletonClass().defineAnnotatedMethods(BCryptPbkdfExt.class);
}

@JRubyMethod(required = 4, rest = true)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These don't appear to be rest-arg methods so remove rest = true.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

unfortunately, failing when rest = true is removed

}


@JRubyMethod(required = 2, rest = true)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remove rest = true and use two explicit arguments.

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.

2 participants