Skip to content

Use "filesystem" encoding for PATH as in MRI. Fixes #3907.#3923

Closed
headius wants to merge 6 commits intojruby:masterfrom
headius:filesystem_encoding_for_path
Closed

Use "filesystem" encoding for PATH as in MRI. Fixes #3907.#3923
headius wants to merge 6 commits intojruby:masterfrom
headius:filesystem_encoding_for_path

Conversation

@headius
Copy link
Member

@headius headius commented May 25, 2016

See #3907.

@headius headius added this to the JRuby 9.1.2.0 milestone May 25, 2016

putRubyKeyValuePair(runtime, rubyMap, key, (String) val, encoding);
Encoding valueEncoding = keyEncoding;
if ( key.equals("PATH") ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if ( key.equalsIgnoreCase("PATH") ) {

Copy link
Member Author

Choose a reason for hiding this comment

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

Mmm, I suppose this would be in line with environment vars being case-insensitive on Windows, wouldn't it.

@headius
Copy link
Member Author

headius commented May 25, 2016

The failure was valid; my changes were a bit overreaching in trying to get encodings right. I've reverted the modified behavior for "locale" encoding and everything is green again.

headius added 6 commits May 25, 2016 15:55
This fixes the one regression in my previous changes. Because MRI
always creates new strings when you read from ENV, they just
normalize to locale encoding for setenv, and then either use
locale encoding or filesystem encoding for getenv. The equivalent
for us here is to just let the setenv string decode to Java String
and then use the default getBytes to match it up with the default
Charset used for locale, rather than attempting to transcode the
string directly.
@headius headius force-pushed the filesystem_encoding_for_path branch from 46bce29 to 9d7295d Compare May 25, 2016 20:56
@headius
Copy link
Member Author

headius commented May 25, 2016

Merged to master in 221ba4f. Merging to 9.1.2.0 branch now.

@headius headius closed this May 25, 2016
@headius headius deleted the filesystem_encoding_for_path branch May 26, 2016 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants