Skip to content

/ servo

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

Check for Resources on case sensitive filesystem #12137

Merged
merged 1 commit into from Jul 15, 2016
Merged

Check for Resources on case sensitive filesystem #12137

merged 1 commit into from Jul 15, 2016

Conversation

@cbrewster
Copy link
Member

cbrewster commented Jul 1, 2016 •
edited

Case where this is needed:
http://logs.glob.uno/?c=mozilla%23servo&s=1+Jul+2016&e=1+Jul+2016#c471192

Another option is to make the Resources folder on the mac .app bundle be lower case; however, it is standard to have Resources in a .app bundle.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #__ (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable
@cbrewster
Check for Resources on case sensitive fs.
Loading status checks…
292a5e9
@aneeshusa
Copy link
Member

aneeshusa commented Jul 1, 2016

If Resources is customary, can we only check for Resources and remove the check for resources?
@cbrewster
Copy link
Member Author

cbrewster commented Jul 1, 2016

@aneeshusa we would need to rename the resource folder to Resource everywhere. Currently it is resource everywhere but in the mac bundle.
@aneeshusa
Copy link
Member

aneeshusa commented Jul 1, 2016

Can we have the Mac packaging rename it to Resources?
@cbrewster
Copy link
Member Author

cbrewster commented Jul 1, 2016

@aneeshusa it currently does, but then servo can't find it on a case sensitive filesystem(as it was only checking for resources).
@perlun
Copy link
Contributor

perlun commented Jul 4, 2016

Good to try and get this fixed. (Like @aneeshusa, I also strongly dislike case-insensitive filesystems; they are really the root of many problems in our world, because they tuck you into a false sense of "all is fine" when it is in fact not at all...)
@emilio
Copy link
Member

emilio commented Jul 15, 2016

ping @aneeshusa and @ConnorGBrewster, is there any better solution for this? If not, feel free to r=me
@cbrewster
Copy link
Member Author

cbrewster commented Jul 15, 2016

@bors-servo r=emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2016

📌 Commit 292a5e9 has been approved by emilio
@aneeshusa
Copy link
Member

aneeshusa commented Jul 15, 2016

I can't think of a better way to do this, so @bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2016

💡 This pull request was already approved, no need to approve it again.

  • There's another pull request that is currently being tested, blocking this pull request: #12458
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2016

📌 Commit 292a5e9 has been approved by aneeshusa
@highfive highfive assigned aneeshusa and unassigned emilio Jul 15, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2016

⌛ Testing commit 292a5e9 with merge 175340d...
bors-servo added a commit that referenced this pull request Jul 15, 2016
@bors-servo
Auto merge of #12137 - ConnorGBrewster:resources_case_sensitive, r=an…
Loading status checks…
175340d 
…eeshusa

Check for Resources on case sensitive filesystem

<!-- Please describe your changes on the following line: -->
Case where this is needed:
http://logs.glob.uno/?c=mozilla%23servo&s=1+Jul+2016&e=1+Jul+2016#c471192

Another option is to make the Resources folder on the mac .app bundle be lower case; however, it is standard to have `Resources` in a .app bundle.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/12137)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2016

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows
@bors-servo bors-servo merged commit 292a5e9 into servo:master Jul 15, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@aneeshusa aneeshusa

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
@cbrewster @aneeshusa @perlun @emilio @bors-servo @highfive
You can’t perform that action at this time.