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

Append to classpath not safe without separator #11

Merged
merged 2 commits into from Feb 12, 2022

Conversation

jlberner
Copy link

It is not safe to append a new path to the CLASSPATH environment variable without a path separator that is suitable for the platform. I could not think of a good/concise way to determine the appropriate path separator, but it should not be necessary to have anything remain in it, so changing from append to assignment.

This resolves the "Class t16subclass not found" outstanding issue.

It is not safe to append a new path to the CLASSPATH environment variable without a path separator that is suitable for the platform. I could not think of a good/concise way to determine the appropriate path separator, but it should not be necessary to have anything remain in it, so changing from append to assignment.

This resolves the "Class t16subclass not found" outstanding issue.
@zmughal
Copy link

zmughal commented Jan 18, 2022

You can use https://perldoc.perl.org/Env which internally uses $Config::Config{path_sep}

@mohawk2
Copy link
Collaborator

mohawk2 commented Jan 18, 2022

Or, it would work to use something like:

$ENV{CLASSPATH} = join $Config{path_sep}, grep defined, $ENV{CLASSPATH}, File::Spec->catfile(qw(t t16subclass.jar));

Please update to that and we can merge and make a release!

Updated to combine with $Config{path_sep} at the suggestion of @mohawk2
@jlberner
Copy link
Author

Update done as requested. Thanks!

Copy link
Collaborator

@mohawk2 mohawk2 left a comment

Choose a reason for hiding this comment

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

Thank you!

@mohawk2 mohawk2 merged commit 8a05efd into ingydotnet:master Feb 12, 2022
@mohawk2
Copy link
Collaborator

mohawk2 commented Feb 12, 2022

This works a lot better if one goes ahead and loads Config and File::Spec first (and actually tests one's changes).

mohawk2 added a commit that referenced this pull request Feb 12, 2022
@mohawk2
Copy link
Collaborator

mohawk2 commented Feb 12, 2022

This module really needs us to bring in CI which would help everyone. @jlberner Thank you for your contribution :-)

@mohawk2
Copy link
Collaborator

mohawk2 commented Feb 12, 2022

Released as 0.67!

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.

None yet

3 participants