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

Fixes to get GHunt working in WSL 1 #14

Closed
wants to merge 2 commits into from

Conversation

christianboyle
Copy link
Contributor

I don't expect this to be merged, but figured I'd create it in case it helps anyone else save some time when trying to get GHunt working in the following:

  • WSL 1
  • Windows 10, Version: 10.0.19041 / Build 19041

Notes:

The reason for the hardcoding of chromedriver.exe in driverpath:

  • You want to use chromedriver.exe rather than chromedriver, since you are invoking Chrome via Windows path, but this code prevents this from happening:
  • Line 33-37 in lib/utils.py:
def get_driverpath():
    if system() == "Windows":
        driverpath = "./chromedriver.exe"
    else:
        driverpath = "./chromedriver"

This never returns Windows when using WSL 1, this can be confirmed by running the following test, which results in linux being printed:

from sys import platform
if platform == "linux" or platform == "linux2":
    print ('linux')
elif platform == "darwin":
    print ('os x')
elif platform == "win32":
    print ('win32')

The reason for adding all of the arguments in lib/gmaps.py:

  • To be perfectly honest I'm not sure if all eight of the arguments added in lib/gmaps.py (if cfg["headless"]: ...) are necessary, but I didn't have time to test removal of each one-by-one and this set has traditionally worked for me when using headless Chrome in other contexts (like Puppeteer). It's possible it would work with some being omitted.

@ItsIgnacioPortal
Copy link
Contributor

ItsIgnacioPortal commented Oct 3, 2020

There are countless ways of getting to now wich system a script is running on.
It seems, for now, the cleanest solution would be use implement os_detect.py

https://www.scivision.co/python-fine-grained-os-detection/

Also, those headers should be loaded dinamically (i.e. only if running on WSL)

@@ -70,7 +70,7 @@

tmprinter.out("Starting browser...")

driverpath = get_driverpath()
driverpath = "./chromedriver.exe"
Copy link
Contributor

Choose a reason for hiding this comment

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

Using a hardcoded path is unacceptable

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, we can't do this. :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wasn't intended to be merged, did you two read the first sentence of the PR?

I don't expect this to be merged, but figured I'd create it in case it helps anyone else save some time when trying to get GHunt working in the following:

@christianboyle christianboyle marked this pull request as draft October 3, 2020 22:38
Copy link
Owner

@mxrch mxrch left a comment

Choose a reason for hiding this comment

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

I'll reorganize the driver process later and by the way checking if we can support the WSL or even checking it, but I don't think hardcoding a path and adding a lot of options without checking it is a good idea.
Thanks anyway for your edit, I add the WSL support in my todo list.

@@ -70,7 +70,7 @@

tmprinter.out("Starting browser...")

driverpath = get_driverpath()
driverpath = "./chromedriver.exe"
Copy link
Owner

Choose a reason for hiding this comment

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

Yes, we can't do this. :(

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.

3 participants