Actually fix File.symlink on Windows#6762
Merged
headius merged 3 commits intojruby:masterfrom Jul 26, 2021
Merged
Conversation
It seems like this code was broken before I messed with it. The FFI::Config constant does not exist, so this was not actually defining the FFI-based File.symlink. As a result, my change to load that logic lazily caused a stack overflow. The changes here fix this: * Always define the Windows-specific String function present here. * Only check for the FFI constant to know if FFI is available. * If FFI is not available *or* it cannot bind the necessary functions, instead define File.symlink to raise NotImplemented. See jruby#6751.
In jruby#4349 we saw that this logic does not load properly when native function calls have been disabled, due to FFI partially loading and defining an FFI module that was not fully functional. This is why the previous code here checked for the existence of FFI::Config rather than just FFI. However this is still error-prone, so I am restructuring this logic to do all FFI juggling within the same rescue block as the require of 'ffi'. If that require raises LoadError or if the required functions are not present, it immediately falls back on defining the NotImplemented version of File.symlink. The same change has been done to the Solaris File#flock logic below.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It seems like this code was broken before I messed with it. The
FFI::Config constant does not exist, so this was not actually
defining the FFI-based File.symlink. As a result, my change to
load that logic lazily caused a stack overflow.
The changes here fix this:
functions, instead define File.symlink to raise NotImplemented.
See #6751.