Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdded detection for case-sensitive file systems #11740
Conversation
highfive
commented
Jun 13, 2016
|
Heads up! This PR modifies the following files:
|
|
Looks good. r=me with a better comment. Reviewed 1 of 1 files at r1. python/mach_bootstrap.py, line 164 [r1] (raw file):
This commment should be reworded for clarity. I read it three times and I"m still not sure about it. Maybe "Check for the wrong path deliberately, which will fail on case-sensitive systems" or something like that. Comments from Reviewable |
This is needed for the moment because of a bug in virtualenv (reported upstream).
perlun
force-pushed the
perlun:detect-case-sensitive-file-system
branch
from
e04450f
to
22fddac
highfive
commented
Jun 14, 2016
|
New code was committed to pull request. |
|
Thanks for the constructive suggestion, Jack! How about this? |
| @@ -160,7 +160,22 @@ def _activate_virtualenv(topdir): | |||
| open(marker_path, 'w').close() | |||
|
|
|||
|
|
|||
| def _ensure_case_insensitive_if_windows(): | |||
| # The folder is called 'python'. By deliberately checking for it with the wrong case, we determine if the file | |||
This comment has been minimized.
This comment has been minimized.
wafflespeanut
Jun 14, 2016
Member
This could just be, "By deliberately checking for the wrong case, we determine ..."
This comment has been minimized.
This comment has been minimized.
perlun
Jun 15, 2016
Author
Contributor
Yes, assuming that the reader has a basic knowledge of the folder structure in the project. I don't know, I feel like it's better to be "too explicit" in cases like this. It feels more friendly to the casual reader that way.
|
Ping @metajack - are we fine with the comment as it's written now or do you want it to be adjusted? |
|
@bors-servo r+ Reviewed 1 of 1 files at r2. Comments from Reviewable |
|
|
…tajack Added detection for case-sensitive file systems - [X] `./mach test-tidy` does not report any errors - [X] These changes do not require tests because they only change Mach code This is needed for the moment because of a bug in virtualenv (reported upstream). r? @metajack (you were the one who suggested that we check this. I did it in a slightly simpler way since I realized we over-complicated things a bit when talking about it the other day.) <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11740) <!-- Reviewable:end -->
|
|
|
@bors-servo retry |
|
cc @larsbergstrom @edunham @aneeshusa Have any of you seen this openssl failure before? |
|
It started happening during the mozlondon week. |
|
|
…tajack Added detection for case-sensitive file systems - [X] `./mach test-tidy` does not report any errors - [X] These changes do not require tests because they only change Mach code This is needed for the moment because of a bug in virtualenv (reported upstream). r? @metajack (you were the one who suggested that we check this. I did it in a slightly simpler way since I realized we over-complicated things a bit when talking about it the other day.) <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11740) <!-- Reviewable:end -->
|
|
|
@bors-servo retry |
|
@metajack No, I've never seen that openssl error before. All these intermittents with zero output (other than a failure error code of |
|
|
|
|
perlun commentedJun 13, 2016
•
edited by larsbergstrom
./mach test-tidydoes not report any errorsThis is needed for the moment because of a bug in virtualenv (reported upstream).
r? @metajack (you were the one who suggested that we check this. I did it in a slightly simpler way since I realized we over-complicated things a bit when talking about it the other day.)
This change is