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

Spurious .DS_Store entries when running ./mach update-manifest on macOS #15725

Closed
nox opened this issue Feb 24, 2017 · 7 comments
Closed

Spurious .DS_Store entries when running ./mach update-manifest on macOS #15725

nox opened this issue Feb 24, 2017 · 7 comments

Comments

@nox
Copy link
Member

@nox nox commented Feb 24, 2017

diff --git i/tests/wpt/mozilla/meta/MANIFEST.json w/tests/wpt/mozilla/meta/MANIFEST.json
index fc2c7b6818..907e06b8d8 100644
--- i/tests/wpt/mozilla/meta/MANIFEST.json
+++ w/tests/wpt/mozilla/meta/MANIFEST.json
@@ -6633,6 +6633,11 @@
    ]
   },
   "support": {
+   "./.DS_Store": [
+    [
+     {}
+    ]
+   ],
    "./lint.whitelist": [
     [
      {}
@@ -9333,6 +9338,11 @@
      {}
     ]
    ],
+   "mozilla/.DS_Store": [
+    [
+     {}
+    ]
+   ],
    "mozilla/2x2.png": [
     [
      {}
@simartin
Copy link
Contributor

@simartin simartin commented Mar 11, 2017

I'll take a stab at this one

@jdm
Copy link
Member

@jdm jdm commented Nov 15, 2017

#16562 was the last attempt, and it contained a suggestion for how to improve it.

@jdm jdm added L-python and removed C-assigned labels Nov 15, 2017
@highfive
Copy link

@highfive highfive commented Nov 15, 2017

@jdm jdm added the E-less easy label Nov 15, 2017
@anrddh
Copy link
Contributor

@anrddh anrddh commented Nov 16, 2017

I can work on this if I can get some guidance.

@jdm
Copy link
Member

@jdm jdm commented Nov 16, 2017

@anrdh Sure! The problem is that ./mach update-manifest looks for all files that exist under tests/wpt/web-platform-tests and updates tests/wpt/metadata/MANIFEST.json based on that list. If you view the directory in macOS' Finder, that can unintentionally create a new .DS_Store file in the directory that gets picked up by the manifest. #16562 was a good start at solving this, and contained a suggestion for how to improve the implementation.

@kregoslup
Copy link
Contributor

@kregoslup kregoslup commented Dec 10, 2017

Since there is no asignee, can I work on this?

@jdm
Copy link
Member

@jdm jdm commented Dec 10, 2017

@kregoslup Yes.

@jdm jdm added the C-assigned label Dec 10, 2017
kregoslup added a commit to kregoslup/servo that referenced this issue Dec 10, 2017
@jdm jdm added the C-has open PR label Jan 3, 2018
bors-servo added a commit that referenced this issue Mar 27, 2018
Issue #15723: Skip git ignored files in update-manifest.

<!-- Please describe your changes on the following line: -->
This patch makes "./mach update-manifest" aware of .gitignore.

---
<!-- 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
- [X] These changes fix #15725 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because I don't know how to test it, I have tried mocking opening file and checking whether it exists but it's difficult to handle both in py2.7 and py3 and I think it would say too much about `PathFilter` internals. I have also tried mocking `PathFilter` but then I would be duplicating logic in tests.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- 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="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19550)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.