classification
Title: gitignore file is too broad
Type: Stage: resolved
Components: Build Versions: Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Greg Price, brett.cannon, miss-islington, zach.ware
Priority: normal Keywords: patch

Created on 2019-08-24 07:15 by Greg Price, last changed 2019-09-11 10:31 by zach.ware. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 15451 merged Greg Price, 2019-08-24 07:30
PR 15542 merged Greg Price, 2019-08-27 05:34
PR 15552 merged miss-islington, 2019-08-27 18:16
PR 15553 merged miss-islington, 2019-08-27 18:16
PR 15747 merged miss-islington, 2019-09-09 09:35
PR 15748 merged miss-islington, 2019-09-09 09:35
PR 15823 merged Greg Price, 2019-09-10 07:10
PR 15900 merged zach.ware, 2019-09-11 09:32
Messages (12)
msg350355 - (view) Author: Greg Price (Greg Price) * Date: 2019-08-24 07:15
There are a number of files that we track in the repo, but are nevertheless covered by `.gitignore`.

This *mostly* doesn't change anything, because Git itself only cares what `.gitignore` has to say about files that aren't already tracked.  But:

* It affects any new files someone might add that are covered by the same unintentionally-broad patterns.  In that case it'd be likely to cause some confused debugging into why Git wasn't seeing the file; or possibly loss of work, if the person didn't notice that the file had never been committed to Git.

* More immediately, other tools that aren't Git but consult the Git ignore rules don't necessarily implement this wrinkle. In particular this is unfortunately a WONTFIX bug in ripgrep / `rg`: https://github.com/BurntSushi/ripgrep/issues/1127 .

I learned of the `rg` bug (and, for that matter, refreshed myself on just how Git itself handles this case) after some confusion today where I was looking with for references to a given macro, thought I'd looked at all of them... and then later noticed through `git log -p -S` a reference in `PC/pyconfig.h` with no subsequent change deleting it. Turned out it was indeed there and I needed to take account of it.

Here's the list of affected files:

$ git ls-files -i --exclude-standard 
.gitignore
Doc/Makefile
Lib/test/data/README
Modules/Setup
PC/pyconfig.h
Tools/freeze/test/Makefile
Tools/msi/core/core.wixproj
Tools/msi/core/core.wxs
Tools/msi/core/core_d.wixproj
Tools/msi/core/core_d.wxs
Tools/msi/core/core_en-US.wxl
Tools/msi/core/core_files.wxs
Tools/msi/core/core_pdb.wixproj
Tools/msi/core/core_pdb.wxs
Tools/unicode/Makefile

Fortunately this is not hard to fix.  The semantics of `.gitignore` have a couple of gotchas, but once you know them it's not really any more complicated to get the behavior exactly right.  And I've previously spent the hour or two to read up on it... and when I forget, I just consult my own short notes :), at the top of this file:
https://github.com/zulip/zulip/blob/master/.gitignore

I have a minimal fix which takes care of all the files above. I'll post that shortly, and I may also write up a more thorough fix that tries to make it easy not to fall into the same Git pitfall again.
msg350600 - (view) Author: Greg Price (Greg Price) * Date: 2019-08-27 05:40
> I have a minimal fix which takes care of all the files above. I'll post that shortly, and I may also write up a more thorough fix that tries to make it easy not to fall into the same Git pitfall again.

Both now done.
* GH-15451 is that minimal fix.
* Just now I've posted https://github.com/gnprice/cpython/pull/1 , which is the more thorough fix. It's written on top of the minimal fix, but I could also squash them together as one PR.

Also just posted GH-15542 . This is a small edit, deleting some lines whose author evidently never intended them to get committed to the repo.  I'd noticed those lines before and been mildly puzzled; writing up the thorough fix caused me to look in the history and see how they were added, and resolve the mystery :).  I've kept that as a separate PR just to reduce the number of moving parts in the main one, because the reasoning for it is quite independent.
msg350640 - (view) Author: miss-islington (miss-islington) Date: 2019-08-27 18:16
New changeset 8c9e9b0cd5b24dfbf1424d1f253d02de80e8f5ef by Miss Islington (bot) (Greg Price) in branch 'master':
bpo-37936: Remove some .gitignore rules that were intended locally. (GH-15542)
https://github.com/python/cpython/commit/8c9e9b0cd5b24dfbf1424d1f253d02de80e8f5ef
msg350641 - (view) Author: miss-islington (miss-islington) Date: 2019-08-27 18:35
New changeset caf7a30efecef21def997ab214dc6c1604d90a84 by Miss Islington (bot) in branch '3.8':
bpo-37936: Remove some .gitignore rules that were intended locally. (GH-15542)
https://github.com/python/cpython/commit/caf7a30efecef21def997ab214dc6c1604d90a84
msg350642 - (view) Author: miss-islington (miss-islington) Date: 2019-08-27 18:41
New changeset 2f0440e6dba248ff1e2b46c063adfd8fcb923a34 by Miss Islington (bot) in branch '3.7':
bpo-37936: Remove some .gitignore rules that were intended locally. (GH-15542)
https://github.com/python/cpython/commit/2f0440e6dba248ff1e2b46c063adfd8fcb923a34
msg351379 - (view) Author: miss-islington (miss-islington) Date: 2019-09-09 09:34
New changeset 5e5e9515029f70836003a8cfb30433166fcc8db7 by Miss Islington (bot) (Greg Price) in branch 'master':
bpo-37936: Avoid ignoring files that we actually do track. (GH-15451)
https://github.com/python/cpython/commit/5e5e9515029f70836003a8cfb30433166fcc8db7
msg351406 - (view) Author: miss-islington (miss-islington) Date: 2019-09-09 10:25
New changeset c837ad408e85eed9d20ba8331751df15e14f6aef by Miss Islington (bot) in branch '3.8':
bpo-37936: Avoid ignoring files that we actually do track. (GH-15451)
https://github.com/python/cpython/commit/c837ad408e85eed9d20ba8331751df15e14f6aef
msg351453 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2019-09-09 13:10
New changeset 9ae9ac1061c76d637ebe9c1d6c6c802311d6ddba by Zachary Ware (Miss Islington (bot)) in branch '3.7':
bpo-37936: Avoid ignoring files that we actually do track. (GH-15451) (GH-15748)
https://github.com/python/cpython/commit/9ae9ac1061c76d637ebe9c1d6c6c802311d6ddba
msg351581 - (view) Author: Greg Price (Greg Price) * Date: 2019-09-10 07:19
Thanks @zach.ware for the review and merge of GH-15451!

That was the "minimal" fix, fixing rules that apply to files we have in the repo now.  So `rg` will no longer ignore `PC/pyconfig.h`. :-)

I've just sent GH-15823 with the "more thorough" fix on top of that, which tries to make it easy not to fall into the same Git pitfall again.
msg351776 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2019-09-11 09:25
New changeset 455122a0094c8cfdf7e062eccc5e5b5885c75e8b by Zachary Ware (Greg Price) in branch 'master':
bpo-37936: Systematically distinguish rooted vs. unrooted in .gitignore (GH-15823)
https://github.com/python/cpython/commit/455122a0094c8cfdf7e062eccc5e5b5885c75e8b
msg351779 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2019-09-11 09:38
Ok, now it's done :)

Thanks, Greg.
msg351801 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2019-09-11 10:31
New changeset df935b5f0bcc40522c7aac1e844aa176cd8bbdef by Zachary Ware in branch '3.8':
[3.8] bpo-37936: Systematically distinguish rooted vs. unrooted in .gitignore (GH-15823) (GH-15900)
https://github.com/python/cpython/commit/df935b5f0bcc40522c7aac1e844aa176cd8bbdef
History
Date User Action Args
2019-09-11 10:31:15zach.waresetmessages: + msg351801
2019-09-11 09:38:15zach.waresetstatus: open -> closed

stage: patch review -> resolved
messages: + msg351779
versions: + Python 3.8
2019-09-11 09:32:41zach.waresetstage: resolved -> patch review
pull_requests: + pull_request15541
2019-09-11 09:25:29zach.waresetmessages: + msg351776
2019-09-10 07:19:18Greg Pricesetstatus: closed -> open

messages: + msg351581
2019-09-10 07:10:11Greg Pricesetpull_requests: + pull_request15472
2019-09-09 13:10:25zach.waresetnosy: + zach.ware
messages: + msg351453
2019-09-09 10:25:07miss-islingtonsetmessages: + msg351406
2019-09-09 09:43:57benjamin.petersonsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2019-09-09 09:35:08miss-islingtonsetpull_requests: + pull_request15402
2019-09-09 09:35:02miss-islingtonsetpull_requests: + pull_request15401
2019-09-09 09:34:53miss-islingtonsetmessages: + msg351379
2019-08-27 18:41:41miss-islingtonsetmessages: + msg350642
2019-08-27 18:35:15miss-islingtonsetmessages: + msg350641
2019-08-27 18:16:51miss-islingtonsetpull_requests: + pull_request15228
2019-08-27 18:16:47miss-islingtonsetnosy: + miss-islington
messages: + msg350640
2019-08-27 18:16:44miss-islingtonsetpull_requests: + pull_request15227
2019-08-27 05:40:04Greg Pricesetmessages: + msg350600
2019-08-27 05:34:27Greg Pricesetpull_requests: + pull_request15219
2019-08-26 17:50:12brett.cannonsetnosy: + brett.cannon
2019-08-24 07:30:06Greg Pricesetkeywords: + patch
stage: patch review
pull_requests: + pull_request15143
2019-08-24 07:15:12Greg Pricecreate