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

Unable to reference kernel32 dll within cygwin64 #559

Closed
philwalk opened this issue Jun 21, 2018 · 11 comments
Closed

Unable to reference kernel32 dll within cygwin64 #559

philwalk opened this issue Jun 21, 2018 · 11 comments
Labels
bug A defect or misbehaviour. nailgun windows

Comments

@philwalk
Copy link
Contributor

philwalk commented Jun 21, 2018

In my cygwin64 environment, I was unable to run the bloop script as-is. Here's the error I was seeing:

$ bloop
Traceback (most recent call last):
  File "/home/philwalk/.bloop/bloop", line 726, in <module>
    k32 = ctypes.CDLL('Kernel32', use_errno=True)
  File "/usr/lib/python2.7/ctypes/__init__.py", line 366, in __init__
    self._handle = _dlopen(self._name, mode)
OSError: No such file or directory

Experimentation led to the following fix : bloop script section near line 726 change from:

elif sys.platform == 'cygwin':
    k32 = ctypes.CDLL('Kernel32', use_errno=True)
    perf_frequency = ctypes.c_uint64()
    k32.QueryPerformanceFrequency(ctypes.byref(perf_frequency))

to


elif sys.platform == 'cygwin':
    k32 = ctypes.CDLL('Kernel32.dll', use_errno=True)
    perf_frequency = ctypes.c_uint64()
    k32.QueryPerformanceFrequency(ctypes.byref(perf_frequency))

In summary, changing the first argument to ctypes.CDLL from 'Kernel32' to 'Kernel32.dll' fixes the problem in my environment.

What I don't know is whether the original problem might be due to my environment being non-standard in some way. If anyone else is able to use the script as-is in a cygwin environment, then this is not the right fix. Anyone have such an experience?

I can do a pull request if this is a universal fix, I just need some confirmation from other cygwin users.

@Duhemm
Copy link
Collaborator

Duhemm commented Jun 21, 2018

Thanks a lot for the report. I've never tried Bloop with Cygwin on Windows, so I can't tell if the issue comes from there or not, and I really can't tell whether the changes that you propose would solve the issues for good.

The Bloop client bloop is actually a very slightly modified version of the stock python Nailgun client (see https://github.com/facebook/nailgun). Could you try using the unmodified Nailgun client on your machine and see if the problem disappears? If it does not (I strongly suspect it won't), it would probably be best to open an issue or PR with your fix on https://github.com/facebook/nailgun.

You can download the latest version of the Python Nailgun client here: https://github.com/facebook/nailgun/blob/master/pynailgun/ng.py

To use it with Bloop, the invocation should look like this (assuming you're running the Bloop server on port 8212, which is the default port):

python ng.py --nailgun-port 8212 about # or any other command

@philwalk
Copy link
Contributor Author

I downloaded nailgun and did the suggested experiment, here's the output:

$  python pynailgun/ng.py --nailgun-port 8212 about
Traceback (most recent call last):
  File "pynailgun/ng.py", line 775, in <module>
    k32 = ctypes.CDLL("Kernel32", use_errno=True)
  File "/usr/lib/python2.7/ctypes/__init__.py", line 366, in __init__
    self._handle = _dlopen(self._name, mode)
OSError: No such file or directory

I have searched online for references to python/cygwin/dll and have found almost nothing. It could be that very few are using cygwin/python/nailgun, and that the bug exists also in nailgun. Since filing the report I also tested my Windows 10 laptop with the same result. My python skills are limited, but I'll see if I can come up with a fix that only applies the modified code if the original throws an exception.

@philwalk
Copy link
Contributor Author

philwalk commented Jun 22, 2018

The following fix (replacing bloop script line 725) avoids the question of whether this is a bug in nailgun or not.

    try:
        k32 = ctypes.CDLL("Kernel32", use_errno=True)
    except OSError:
        k32 = ctypes.CDLL("Kernel32.dll", use_errno=True)

@philwalk
Copy link
Contributor Author

I also filed a bug with the nailgun project.

@Duhemm
Copy link
Collaborator

Duhemm commented Jun 26, 2018

(See: facebookarchive/nailgun#145)

@jvican jvican changed the title bloop script might need a tweak to run on cygwin; unable to reference kernel32 dll Unable to reference kernel32 dll within cygwin64 Jul 2, 2018
@jvican
Copy link
Contributor

jvican commented Jul 2, 2018

Hey Phil, do you want to submit a fix to our bloop plugin script so that we unblock you? It looks like the nailgun bug won't be fixed any time soon. It would be a pity you cannot try bloop out because of that.

@philwalk
Copy link
Contributor Author

philwalk commented Jul 3, 2018

If the following approach seems acceptable, I'll submit a pull request.

In the installer script at line 166, the nailgun script is transferred as-is to the client:

download_and_install(NAILGUN_CLIENT_URL, BLOOP_CLIENT_TARGET, 0o755)
print("Installed bloop client in '%s'" % BLOOP_CLIENT_TARGET)

One approach would be to transfer first to a temporary file, and then copy-with-edits to the correct location:

import tempfile
BLOOP_CLIENT_TARGET_TEMPFILE = join(tempfile.gettempdir(), "bloop")
download(NAILGUN_CLIENT_URL, BLOOP_CLIENT_TARGET_TEMPFILE)
copy_with_edits(BLOOP_CLIENT_TARGET_TEMPFILE, BLOOP_CLIENT_TARGET, 0o755)
print("Installed bloop client in '%s'" % BLOOP_CLIENT_TARGET)

Here's a simple version of 'copy_with_edits':

# If the EXACT target_line is encountered in the input file, the following
# edit is performed:
#  + write the edits_prefix line
#  + write the target_line with one additional level of indentation
#  + write the edits_suffix lines
#
# This is fragile in the sense that it will do nothing unless the
# input file has an exact match with the target_line.

def copy_with_edits(src,dest,permissions=0o644):

    target_line  = "    k32 = ctypes.CDLL('Kernel32', use_errno=True)"

    edits_prefix = "    try:\n"
    edits_suffix = "    except OSError:\n" \
                   "        k32 = ctypes.CDLL('Kernel32.dll', use_errno=True)\n"

    with open(src) as f1:
        with open(dest, 'w') as f2:
            for line in f1:
                linetrim = line.rstrip('[\r\n\s]+')
                if target_line == linetrim:
                    f2.write(edits_prefix) # try:
                    f2.write('    '+line)         # with one additional level of indentation
                    f2.write(edits_suffix) # else:
                else:
                    f2.write(line)
    os.chmod(dest, permissions)
    os.remove(src)

This works if the target line doesn't change.

@jvican
Copy link
Contributor

jvican commented Jul 3, 2018

I wonder if instead of doing this workaround you would be happy with making this change in facebook/nailgun, and then we would upgrade our nailgun version to it. The change would only need to be the addition of the extra try clause in the python script. Note that we have our own fork of nailgun (https://github.com/scalacenter/nailgun).

@jvican
Copy link
Contributor

jvican commented Jul 3, 2018

Actually, I just did the change in the linked nailgun commit. Assuming the fix you linked at the beginning works, this will be fixed in 1.0.0 for you.

@philwalk
Copy link
Contributor Author

philwalk commented Jul 3, 2018

Wow, a much better approach, thanks! I didn't know we had our own nailgun version to work with.

@jvican
Copy link
Contributor

jvican commented Jul 3, 2018

Yeah, I realized that from your response! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A defect or misbehaviour. nailgun windows
Projects
None yet
Development

No branches or pull requests

3 participants