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

gitignore: ignore .DS_Store for macOS #14721

Closed
wants to merge 1 commit into from
Closed

Conversation

kfarnung
Copy link
Contributor

@kfarnung kfarnung commented Aug 9, 2017

Porting upstream from nodejs/node-chakracore#363 on behalf of @tommyZZM.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

gitignore

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Aug 9, 2017
@refack refack self-assigned this Aug 9, 2017
@Fishrock123
Copy link
Contributor

Fishrock123 commented Aug 9, 2017

@tommyZZM Please add .DS_Store to your global gitignore. It belongs there rather than per project.

Copy link
Contributor

@Fishrock123 Fishrock123 left a comment

Choose a reason for hiding this comment

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

I'm not sure why it didn't ignore properly, but we already ignore all dotfiles and whitelist the ones we need. Check the top of the file.

(This change in and of itself should not land.)

@Fishrock123 Fishrock123 added the duplicate Issues and PRs that are duplicates of other issues or PRs. label Aug 9, 2017
@Fishrock123
Copy link
Contributor

(Aside: duplicate of various past issues pre-whitelist: https://github.com/nodejs/node/pulls?q=is%3Apr+ds_store+is%3Aclosed)

@Fishrock123
Copy link
Contributor

Also, summoning @claudiorodriguez who wrote that patch: 15cc7c0

@refack
Copy link
Contributor

refack commented Aug 9, 2017

@tommyZZM did you by any chance run git add . -A -f at some point?

@kfarnung
Copy link
Contributor Author

kfarnung commented Aug 9, 2017

Thanks @Fishrock123, it does seem like that should cover it, I'll ping @tommyZZM and try to get more information.

@kfarnung
Copy link
Contributor Author

kfarnung commented Aug 9, 2017

I see the issue now, the .DS_Store files are in the deps folder (from @tommyZZM in nodejs/node-chakracore#363):

node-chakracore$ git status
On branch master
Your branch is up-to-date with 'origin/master'.
Untracked files:
  (use "git add <file>..." to include in what will be committed

        .DS_Store
	chakrashim/.DS_Store
	npm/.DS_Store
	openssl/.DS_Store
	openssl/openssl/.DS_Store
	v8/.DS_Store
	v8/test/.DS_Store
	v8/test/mjsunit/.DS_Store
	../tools/eslint/.DS_Store
	../tools/eslint/node_modules/.DS_Store

Which makes sense given the commit that @Fishrock123 referenced, it explicitly allows all dotfiles in the deps directories:

!deps/**/.*

I'm personally not opposed to this change if it simplifies life for potential contributors, but as it stands this appears to be the intended behavior.

@kfarnung
Copy link
Contributor Author

kfarnung commented Aug 9, 2017

@Fishrock123
Copy link
Contributor

Ah, right. Ordinarily those folders do not need to be touched, so this isn't usually an issue.

I suggest making a refined whitelist for the chakrashim parts specifically and ignoring everything else. The other folders I think it is best to leave up to the end user, lest we repeat the never-ending-meta-file-ignoring again.

@kfarnung
Copy link
Contributor Author

kfarnung commented Aug 9, 2017

I'm starting to think that having a global .gitignore is the correct solution. Even refining using /deps/chakrashim/.gitignore wouldn't really help @tommyZZM since the files also show up under v8 and openssl.

@Fishrock123
Copy link
Contributor

IMO, if you edit the deps directly this is kinda on you.

@kfarnung
Copy link
Contributor Author

I think the consensus was not to land this change, so let's just close the PR.

@kfarnung kfarnung closed this Aug 11, 2017
@kfarnung kfarnung deleted the pr363 branch August 11, 2017 00:57
@tommyZZM
Copy link

tommyZZM commented Aug 12, 2017

hi @Fishrock123 @kfarnung

i have added .DS_Store to .gitignore_global with git config --global core.excludesfile

.gitignore_global

.DS_Store
*.DS_Store
**/.DS_Store

it works fine in other remotes , but not working in node and node-chakracore

my git version is 2.8.1, i tried update to 2.14.1, but still not get rid of .DS_Store

i found these rules may be the reason cause some .DS_Store not ignored

.gitignore

 # Whitelist dotfiles
 .*
-!deps/**/.*
-!test/fixtures/**/.*
-!tools/eslint/**/.*
-!tools/doc/node_modules/**/.*
+#!deps/**/.*
+#!test/fixtures/**/.*
+#!tools/eslint/**/.*
+#!tools/doc/node_modules/**/.*

i commented these lines and .DS_Stores are gone

then i tired may the most simplest modification is just add .DS_Store to .gitignore directly

.gitignore

+.DS_Store

and it works


JUST UPDATED

i found a solution without remove or add anything

the sequence of rules in #Whitelist dotfiles causes the problem 😄

.gitignore

# Whitelist dotfiles
-.*
!deps/**/.*
!test/fixtures/**/.*
!tools/eslint/**/.*
!tools/doc/node_modules/**/.*
!.editorconfig
!.eslintignore
!.eslintrc.yaml
!.gitattributes
!.github
!.gitignore
!.gitkeep
!.mailmap
!.remarkrc
+.*

move the .* to bottom solve the problem!

@tommyZZM
Copy link

tommyZZM commented Aug 12, 2017

hi @Fishrock123 @kfarnung

But modify sequence of rules will causes more problem

This overrides all the preceding rules. Previously, adding .foo to the test/fixtures directory would be reported by git, which is what we want. With this change, it will be ignored.

add .DS_Store at bottom may be better

# Whitelist dotfiles
.*
!deps/**/.*
!test/fixtures/**/.*
!tools/eslint/**/.*
!tools/doc/node_modules/**/.*
!.editorconfig
!.eslintignore
!.eslintrc.yaml
!.gitattributes
!.github
!.gitignore
!.gitkeep
!.mailmap
!.remarkrc
+.DS_Store

my current macOS(10.12.6) will auto generate .DS_Store without actually opening the folder. so this problem nervers me ...

i do not thing besides git clone the remote ,and then after some times reading code, git status will show this untracked changes of .DS_Stores

@gibfahn
Copy link
Member

gibfahn commented Aug 12, 2017

Yeah okay, this happens for me as well if I open up Finder and click around the deps/ subdirectories. I guess I've never done that before so didn't notice.

I suggest making a refined whitelist for the chakrashim parts specifically and ignoring everything else. The other folders I think it is best to leave up to the end user, lest we repeat the never-ending-meta-file-ignoring again.

I think @Fishrock123 's suggestion makes most sense. Ignore all dotfiles in deps/ except for a whitelist. Or if that's not feasible we could take the nuclear option of ignoring deps/ completely, and requiring people updating deps to use git add -f deps/.

@tommyZZM interesting that .DS_Store files get generated without you going into the folder, I'm on 10.12.6 as well, and I don't see that. Maybe your IDE is navigating into those folders or something.

@tommyZZM
Copy link

@gibfahn maybe some solfware (VisualStudioCoreSourceTree) make them get generated

@refack
Copy link
Contributor

refack commented Aug 13, 2017

Y'all know about .git/info/exclude

@gibfahn
Copy link
Member

gibfahn commented Aug 13, 2017

Y'all know about .git/info/exclude

Doesn't local .gitignore take precedence over it? See docs: https://git-scm.com/docs/gitignore#_description

@refack
Copy link
Contributor

refack commented Aug 13, 2017

Doesn't local .gitignore take precedence over it? See docs: https://git-scm.com/docs/gitignore#_description

Boop, read it upside down

@refack refack removed their assignment Oct 20, 2018
@xgqfrms-GitHub
Copy link

.gitignore & .DS_Store

https://stackoverflow.com/questions/107701/how-can-i-remove-ds-store-files-from-a-git-repository

$ vi ~/.gitignore_global


# modify configs with below `ignore_macos.js` file

$ git config --global core.excludesfile ~/.gitignore_global

ignore_macos.js

# OS generated files #
######################
.DS_Store
.DS_Store?
._*
.Spotlight-V100
.Trashes
ehthumbs.db
Thumbs.db

https://stackoverflow.com/questions/18393498/gitignore-all-the-ds-store-files-in-every-folder-and-subfolder

$ find . -name .DS_Store -print0 | xargs -0 git rm --ignore-unmatch

echo ".DS_Store" >> ~/.gitignore_global
echo "._.DS_Store" >> ~/.gitignore_global
echo "**/.DS_Store" >> ~/.gitignore_global
echo "**/._.DS_Store" >> ~/.gitignore_global

git config --global core.excludesfile ~/.gitignore_global
#Ignore folder mac
.DS_Store


# OR
.DS_Store
._.DS_Store
**/.DS_Store
**/._.DS_Store

@refack refack added the stalled Issues and PRs that are stalled. label Mar 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate Issues and PRs that are duplicates of other issues or PRs. meta Issues and PRs related to the general management of the project. stalled Issues and PRs that are stalled.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants