WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 5 months ago

#27207 closed enhancement (fixed)

Extend core's .gitignore file

Reported by: TobiasBg Owned by: markjaquith
Milestone: 3.9 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch
Focuses: Cc:

Description

In [25001] for #24976, we added a gitignore file to trunk. With the recent additions of the git mirrors for the SVN repo, this is becoming more and more useful.
However, the .gitignore file is out of sync with the svn:ignore property in many places (also noted in some comments in #24976).
It also uses entries in a "greedy" way (regexp speak) (for example, build will match a build folder in any subdirectory, and not just the ./build directory).
Additionally, it contains exclusions for some folders and file types that don't appear in core natively.

I therefore propose to clean up and extend the .gitignore file to include all files/folders in core that have a meaning and might appear throughout core development, but that should not be version-controlled/tracked.

The attached patch does that by

  • adding
    • all files that are svn:ignored (list from svn propget svn:ignore -R .)
    • our config files (the versions without -sample in the file name)
    • .htaccess (per azaozz's suggestion)
    • debug.log (result of WP_DEBUG_LOG, but not svn:ignored),
  • removing most of the initial entries from [25001] as ignoring those file types should be done on a per-developer basis (e.g. by adding them to .git/info/exclude), if desired, but not on the core repo by default,
  • being more strict with folder and file names, by prepending the / before the entries. (With that, /build matches only ./build, but not for example ./src/wp-admin/build if it existed (in untracked state)). (I left out the / for the config files, to be on the safe side by really ignoring all of them.)

Additionally, I added a .gitignore entry, to allow developers to ignore other/custom stuff with further .gitignore files, e.g. in subfolders.

Attachments (5)

27207.diff (1.4 KB) - added by TobiasBg 3 years ago.
27207.2.diff (1.6 KB) - added by markjaquith 3 years ago.
Do not ignore new twenty* themes
ticket-27207-gitignore.2.patch (386 bytes) - added by bpetty 3 years ago.
27207.3.diff (665 bytes) - added by markjaquith 3 years ago.
explicitly ignore default themes
27207.4.diff (655 bytes) - added by TobiasBg 3 years ago.

Download all attachments as: .zip

Change History (26)

@TobiasBg
3 years ago

#1 follow-up: @nacin
3 years ago

I'm game. svn:ignore should be updated to be kept in sync.

I will say, though, the generic ones are helpful. At the very least, *.log (wp-content/debug.log is technically a thing, as much as I loathe to admit it).

#2 @TobiasBg
3 years ago

Yes, the generic file types can be helpful, but they might (at a later time) also hide stuff that should be noticed. I therefore tend to prefer a minimalist approach, to only add specific files/folders that we know have a meaning to core or build/test tools. Generic types can always be added by a dev, if he prefers, via those custom .gitignore files or .git/info/exclude.

I agree with wp-content/debug.log, which is why it's added separately.

/src/wp-content/debug.log, the potential .gitignore files in subfolders, .htaccess, and tests/phpunit/data/plugins/wordpress-importer are the only ones from the suggested .gitignore that are not svn:ignored at the moment.

@markjaquith
3 years ago

Do not ignore new twenty* themes

#3 @markjaquith
3 years ago

  • Owner set to markjaquith
  • Resolution set to fixed
  • Status changed from new to closed

In 27295:

More robust .gitignore file

fixes #27207. props TobiasBg, markjaquith.

#4 @bpetty
3 years ago

Definitely agree with scoping build properly. Personally, I always anchor my excludes. I'd rather keep adding new ones when I actually run into them rather than specifying loosely defined excludes - it really doesn't happen that often. I have a bunch in my own .git/info/exclude file, but ignoring the more personal ones (like my IDE project files), I still have these core-specific ones:

/src/.htaccess
/src/wp-content/uploads/

/src/wp-content/plugins/*
!/src/wp-content/plugins/index.php
!/src/wp-content/plugins/hello.php

Well, .htaccess isn't actually WP intentionally placing it in src, but I just find myself placing it there every time since running grunt build clears the entire build folder, and I don't want to keep fixing my permalinks while running from the build directory every time I make a change. So I just copy it into src so it's brought over automatically. Of course, the same applies to the plugin excludes, and really anything else that would have normally been specified for https://core.svn.wordpress.org/trunk.

#5 @bpetty
3 years ago

Well, if you had given me a minute to review the patch, I could have spotted this for you too...

#6 @bpetty
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#7 @markjaquith
3 years ago

bpetty — why did you add index.php and hello.php? The files are already tracked.

#8 @bpetty
3 years ago

You could say the same about core themes, but it's still helpful to be explicit.

#9 follow-up: @TobiasBg
3 years ago

I also don't see the benefit of adding index.php and hello.php.
@bpetty: Mark's idea with the themes is for new core themes (like twentyfifteen), I think.
IMO, that should also not be necessary, as the new theme slug could be temporarily reincluded locally before the initial commit of the new theme. This exclude rule in fact prevents things like twentythirteen-child, which is probably not uncommon.

#10 in reply to: ↑ 1 @TobiasBg
3 years ago

Replying to nacin:

svn:ignore should be updated to be kept in sync.

We could handle that in this ticket, too, but I don't think that this can be prepared in a patch.

#11 in reply to: ↑ 9 @markjaquith
3 years ago

Replying to TobiasBg:

@bpetty: Mark's idea with the themes is for new core themes (like twentyfifteen), I think.

Indeed.

IMO, that should also not be necessary, as the new theme slug could be temporarily reincluded locally before the initial commit of the new theme. This exclude rule in fact prevents things like twentythirteen-child, which is probably not uncommon.

A good point. You can -f it to force add. We add new themes once a year. Not a problem.

@markjaquith
3 years ago

explicitly ignore default themes

#12 @markjaquith
3 years ago

27207.3.diff switches to explicit default theme specifying.

#13 @TobiasBg
3 years ago

But those are also already committed (except for twentyfifteen), so there's no need to reinclude them.
Or am I missing something here?

@TobiasBg
3 years ago

#14 @TobiasBg
3 years ago

27207.4.diff should be all that's necessary.
Once twentyfifteen development starts, it can be easily added with git add -f src/wp-content/themes/twentyfifteen or temporarily reincluded on the committer's machine. Once it's committed, all is good again for everybody.

#15 @markjaquith
3 years ago

The other case is adding a file to a twenty* theme, which happens a good deal during theme development. So might only need a twentyfifteen exclusion override. But not yet. Can add it during new theme development if the forcing requirement is annoying anyone. So yeah: 27207.4.diff is good.

#16 @markjaquith
3 years ago

In 27298:

Remove twenty* ignore-exclusion lines. Already tracked.

see #27207. props TobiasBg.

#17 follow-up: @markjaquith
3 years ago

Now, svn:ignore probably needs some updating. I haven't done SVN ignores in a while. Can someone list the deficiencies in our current svn:ignore?

#18 in reply to: ↑ 17 @TobiasBg
3 years ago

The other case is adding a file to a twenty* theme, which happens a good deal during theme development.

That's a very good point, hadn't thought about that. Maybe 27207.3.diff is better then... Argh, I'm undecided! :-)

Can someone list the deficiencies in our current svn:ignore?

Missing for sure:

  • /src/wp-content/debug.log
  • tests/phpunit/data/plugins/wordpress-importer

Nice to have (if already possible on the SVN repo with azaozz's suggestion):

  • .gitignore (in subfolders)
  • .htaccess (everywhere)

#19 @nacin
3 years ago

SVN actually has tests/phpunit/data/plugins/wordpress-importer as an external, so svn:ignore isn't necessary for that.

#20 @markjaquith
3 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 27382:

Edit svn:ignore and svn:global-ingnores properties to match .gitignore

fixes #27207

#21 @westonruter
5 months ago

In 39362:

Git: Prevent untracked files from being ignored by git in bundled themes.

See #27207.
Fixes #38779.

Note: See TracTickets for help on using tickets.