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

[WIP] Conda install openwpm dependencies #429

Closed

Conversation

birdsarah
Copy link
Contributor

Task: make dependencies conda installable
Why: so we can get going on windows
Side benefit: installation for all platforms will be simplified and unified

Depends on #426

…ss-osx' into conda-install-openwpm-dependencies"

This reverts commit 7ff5bcb, reversing
changes made to f40ca82.
@birdsarah
Copy link
Contributor Author

@englehardt are there tests that are a little flaky? I just have one failing test and i'm a little surprised. i can't kick off a rerun, so if it is a flaky would you mind kicking it?

@@ -421,41 +437,11 @@ Once installed, execute `py.test -vv` in the test directory to run all tests.

### Mac OSX (Limited support for developers)

We've added an installation file to make it easier to run tests and develop on
Copy link
Contributor

@motin motin Aug 2, 2019

Choose a reason for hiding this comment

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

This part of the readme (including the linked wiki page) needs to be updated to reflect the Conda-based installation.

Copy link
Contributor

@motin motin left a comment

Choose a reason for hiding this comment

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

I welcome this clean-up! I left some suggestions in comments, and we should wait for #426 to be merged and re-base this PR before merging (or make this not depend on #426).

@@ -68,15 +68,16 @@ def get_geckodriver_exec_path():
If the geckodriver executable does not exist next to the Firefox binary,
we throw a RuntimeError.
"""
firefox_binary_path = get_firefox_binary_path()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is commenting this out a temporary change?

sites = ['http://www.example.com',
'http://www.princeton.edu',
'http://citp.princeton.edu/']
NUM_BROWSERS = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

This PR should probably not change the default number of browsers

@@ -29,8 +31,8 @@
browser_params[0]['headless'] = True # Launch only browser 0 headless

# Update TaskManager configuration (use this for crawl-wide settings)
manager_params['data_directory'] = '~/Desktop/'
manager_params['log_directory'] = '~/Desktop/'
manager_params['data_directory'] = 'demo-data'
Copy link
Contributor

Choose a reason for hiding this comment

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

This PR should probably not change the default paths

- tblib
- pip:
- publicsuffix
- plyvel
Copy link
Contributor

Choose a reason for hiding this comment

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

Lovely if Conda makes the Mac-plyvel-installation issues go away! :)


conda activate openwpm

Get a copy of firefox and place it in the directory firefox-bin
Copy link
Contributor

Choose a reason for hiding this comment

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

The Mac install script does not place it in firefox-bin, we can opportunistically update the docs to reflect that here now :)

instrumentation extension or run tests you will also need to install the
development dependencies included in `install-dev.sh`.

It is likely that OpenWPM will work on platforms other than Ubuntu, however
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment should remain since this PR does not add full support for Mac and Windows.

@@ -16,18 +16,31 @@ version of Firefox.
Installation
------------

OpenWPM has been developed and tested on Ubuntu 14.04/16.04. An installation
Copy link
Contributor

Choose a reason for hiding this comment

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

The first sentence here should remain, since we do not add tests for Mac and Windows.

@@ -46,68 +82,6 @@ jobs:
install:
script:
- docker build -f Dockerfile -t openwpm .
- ./.deploy-to-dockerhub.sh
- language: node_js
Copy link
Contributor

Choose a reason for hiding this comment

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

Please restore the tests for webext-instrumentation. If you find a cleaner way, it'd be great, otherwise just restore as they were. Thanks :)

@motin motin changed the title Conda install openwpm dependencies [WIP] [WIP] Conda install openwpm dependencies Aug 6, 2019
@motin motin requested review from englehardt and nhnt11 October 9, 2019 06:11
@birdsarah birdsarah closed this Apr 10, 2020
@nhnt11 nhnt11 removed their request for review August 4, 2020 15:26
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