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

Make virtualenv more tolerant of spaces in directory names #8394

Merged
merged 1 commit into from Nov 8, 2015

Conversation

@frewsxcv
Copy link
Member

frewsxcv commented Nov 7, 2015

Fixes #8390

Review on Reviewable

@frewsxcv
Copy link
Member Author

frewsxcv commented Nov 7, 2015

I confirmed this works locally

@Manishearth
Copy link
Member

Manishearth commented Nov 7, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2015

📌 Commit 4ff8d3a has been approved by Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2015

Testing commit 4ff8d3a with merge 6e8071f...

bors-servo added a commit that referenced this pull request Nov 7, 2015
Make virtualenv more tolerant of spaces in directory names

Fixes #8390

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8394)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2015

💔 Test failed - gonk

@frewsxcv
Copy link
Member Author

frewsxcv commented Nov 7, 2015

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2015

Testing commit 4ff8d3a with merge abf2dfe...

bors-servo added a commit that referenced this pull request Nov 7, 2015
Make virtualenv more tolerant of spaces in directory names

Fixes #8390

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8394)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2015

💔 Test failed - linux-rel

@frewsxcv
Copy link
Member Author

frewsxcv commented Nov 7, 2015

@bors-servo retry

yet another buildbot issue

@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2015

Previous build results for android, gonk, linux-dev, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt are reusable. Rebuilding only linux-rel...

@bors-servo
Copy link
Contributor

bors-servo commented Nov 8, 2015

@bors-servo bors-servo merged commit 4ff8d3a into servo:master Nov 8, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@frewsxcv frewsxcv deleted the frewsxcv:virtualenv-spaces branch Nov 8, 2015
upsuper added a commit to upsuper-forks/servo that referenced this pull request Jul 19, 2016
There are two changes:
 * remove quoting which causes virtuaenv not activate
 * check virtualenv actually activated

If the quoting added in the fix in servo#8394 (4ff8d3a) kicks in, it
causes virtualenv to fail to activate.

For the common case it is no op:
```python
>>> from pipes import quote
>>> print quote('common/case')
common/case
```

When the path actually needs quoting, this is what happens:

```python
>>> print quote('test spaces')
'test spaces'
>>> print quote('windows\\path')
'windows\\path'
```

Note the embedded quotes.

Virtualenv in activate_this.py uses __file__ to build the path that
should be added to PATH:

```python
>>> print os.getcwd()
C:\software\git
>>> print os.path.abspath(quote('windows\\path'))
C:\software\git\'windows\path'
>>>
```

The constructed path is not valid. Adding it at the beginning of PATH
has no effect. This issue affects any case when the call to `quote`
kicks in.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.