Make WordPress Core

Opened 6 years ago

Closed 4 years ago

Last modified 4 years ago

#42594 closed task (blessed) (fixed)

Clean up svn properties on develop

Reported by: dd32's profile dd32 Owned by: desrosj's profile desrosj
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch
Focuses: Cc:

Description

A number of files have random svn: properties set that they don't need - it's probably time to clean these out.

svn:executable is set on 23 files, none of which need it.

svn:keywords is set on 96 files, again, none of which need it - and may actually cause issue in the future if we ever end up using an $Author variable in a file with it (It does substitution). Thankfully there's a 0-intersect between the the keywords and files that have the property currently.

svn:eol-style doesn't appear to be set on all files consistently either, and should be set on all text/editable files.

Attachments (1)

42594.diff (2.6 KB) - added by desrosj 4 years ago.

Download all attachments as: .zip

Change History (25)

#1 @dd32
6 years ago

  • Milestone changed from Awaiting Review to 5.0
  • Owner set to dd32
  • Status changed from new to accepted

#2 @dd32
6 years ago

In 42200:

Remove the svn:executable property from files that don't need it.

See #42594

#3 @dd32
6 years ago

In 42201:

Remove the svn:keywords property from files that don't need it.

See #42594

#4 follow-up: @johnbillion
5 years ago

Anything else left to do here @dd32 ?

#5 @pento
5 years ago

  • Milestone changed from 5.0 to 5.1

#6 in reply to: ↑ 4 @dd32
5 years ago

  • Owner dd32 deleted
  • Status changed from accepted to assigned

Replying to johnbillion:

Anything else left to do here @dd32 ?

Not sure, a lot has happened in develop.svn since then - AFAIK there were still a lot of stray properties after [42201] that still needed cleaning up, if anyone cares enough to comb through it.

#8 @afercia
5 years ago

#45412 was marked as a duplicate.

#9 @afercia
5 years ago

The svn:eol-style was reported also in #45412, closed as duplicate.

#10 @pento
5 years ago

It'd help to add auto-props to the repo, so we don't keep having to do this. These are all the extensions in the repo, a few extra pairs of eyes to check that I got them all would be helpful.

### Source files
*.css = svn:eol-style=native
*.js = svn:eol-style=native
*.php = svn:eol-style=native
*.py = svn:eol-style=native

### Font files
*.eot = svn:mime-type=application/octet-stream
*.otf = svn:mime-type=application/octet-stream
*.ttf = svn:mime-type=application/octet-stream
*.woff = svn:mime-type=application/octet-stream
*.woff2 = svn:mime-type=application/octet-stream

### Image files
*.bmp = svn:mime-type=application/octet-stream
*.gif = svn:mime-type=application/octet-stream
*.ico = svn:mime-type=application/octet-stream
*.jp2 = svn:mime-type=application/octet-stream
*.jpg = svn:mime-type=application/octet-stream
*.pct = svn:mime-type=application/octet-stream
*.png = svn:mime-type=application/octet-stream
*.psd = svn:mime-type=application/octet-stream
*.sgi = svn:mime-type=application/octet-stream
*.svg = svn:eol-style=native
*.tga = svn:mime-type=application/octet-stream
*.tiff = svn:mime-type=application/octet-stream

### Video files
*.mkv = svn:mime-type=application/octet-stream
*.mov = svn:mime-type=application/octet-stream
*.mp4 = svn:mime-type=application/octet-stream
*.webm = svn:mime-type=application/octet-stream

### Audio files
*.flac = svn:mime-type=application/octet-stream
*.mp3 = svn:mime-type=application/octet-stream

### Document files
*.csv = svn:eol-style=native
*.dfxp = svn:eol-style=native
*.docx = svn:mime-type=application/octet-stream
*.html = svn:eol-style=native
*.md = svn:eol-style=native
*.pdf = svn:mime-type=application/octet-stream
*.rtf = svn:mime-type=application/octet-stream
*.tsv = svn:eol-style=native
*.txt = svn:eol-style=native
*.vtt = svn:eol-style=native

### Translation files
*.mo = svn:eol-style=native
*.po = svn:mime-type=application/octet-stream
*.pot = svn:eol-style=native

### Config files
*.dist = svn:eol-style=native
*.jshintrc = svn:eol-style=native
*.json = svn:eol-style=native
*.lock = svn:eol-style=native
*.xml = svn:eol-style=native
*.yml = svn:eol-style=native

### Miscellaneous
*.crt = svn:eol-style=native
*.map = svn:eol-style=native
*.nodelete = svn:eol-style=native

To fix up the existing problems, something like these commands will do:

find -E . -regex '.*\.(css|js|php|py)' | xargs -I {} svn propset svn:eol-style native '{}'
find -E . -regex '.*\.(bmp|gif|ico|jp2|jpg)' | xargs -I {} svn propset svn:mime-type application/octet-stream '{}'

#11 @pento
5 years ago

  • Milestone changed from 5.1 to 5.2

Also, as much as I enjoy changing pretty much every file in the repo, I'm going to leave this until 5.2.

#12 @johnbillion
5 years ago

  • Milestone changed from 5.2 to Future Release
  • Type changed from defect (bug) to task (blessed)

#13 @desrosj
4 years ago

  • Milestone changed from Future Release to 5.4
  • Owner set to desrosj

I'll follow up on this and wrap it up after 5.3 is branched.

#14 @desrosj
4 years ago

In 46585:

Remove the svn:executable property from files that don't need it.

See #42594.

#15 @desrosj
4 years ago

In 46586:

Ensure svn:eol-style is consistently set for all files.

See #42594.

#16 @desrosj
4 years ago

In 46587:

Ensure svn:eol-style is consistently set for all files (part 2).

Continuation of [46586]

See #42594.

#17 @desrosj
4 years ago

In 46588:

Ensure svn:mime-type is consistently set for appropriate files.

Continuation of [46586], [46587]

See #42594.

@desrosj
4 years ago

#18 @desrosj
4 years ago

  • Keywords has-patch added

Using find . -type f -path '*/.svn*' -prune -o -print | perl -ne 'print $1 if m/\.([^.\/]+)$/' | sort -u, I audited the list of file types in trunk since 42594#comment:10 and found these new formats:

  • editorconfig
  • env
  • gitignore
  • ini
  • scss
  • sql
  • template

I think that all of these can just be svn:eol-style=native.

[46585-46588] fixes the issues for all file types in trunk today.

Here is an updated snippet to run for double checking trunk (should result in no changes):

find -E . -regex '.*\.(crt|css|csv|dfxp|dist|editorconfig|env|gitignore|html|ini|js|jshintrc|json|lock|map|md|mo|nodelete|php|pot|py|scss|sql|svg|template|tsv|txt|vtt|xml|yml)' | xargs -I {} svn propset svn:eol-style native '{}'
find -E . -regex '.*\.(bmp|docx|eot|flac|gif|ico|jp2|jpg|mkv|mov|mp3|mp4|otf|pct|pdf|png|po|psd|rtf|sgi|tga|tiff|ttf|webm|woff|woff2)' | xargs -I {} svn propset svn:mime-type application/octet-stream '{}'

Here is an updated list all inclusive list:

### Source files
*.css = svn:eol-style=native
*.js = svn:eol-style=native
*.php = svn:eol-style=native
*.py = svn:eol-style=native
*.scss = svn:eol-style=native
*.template = svn:eol-style=native

### Font files
*.eot = svn:mime-type=application/octet-stream
*.otf = svn:mime-type=application/octet-stream
*.ttf = svn:mime-type=application/octet-stream
*.woff = svn:mime-type=application/octet-stream
*.woff2 = svn:mime-type=application/octet-stream

### Image files
*.bmp = svn:mime-type=application/octet-stream
*.gif = svn:mime-type=application/octet-stream
*.ico = svn:mime-type=application/octet-stream
*.jp2 = svn:mime-type=application/octet-stream
*.jpg = svn:mime-type=application/octet-stream
*.pct = svn:mime-type=application/octet-stream
*.png = svn:mime-type=application/octet-stream
*.psd = svn:mime-type=application/octet-stream
*.sgi = svn:mime-type=application/octet-stream
*.svg = svn:eol-style=native
*.tga = svn:mime-type=application/octet-stream
*.tiff = svn:mime-type=application/octet-stream

### Video files
*.mkv = svn:mime-type=application/octet-stream
*.mov = svn:mime-type=application/octet-stream
*.mp4 = svn:mime-type=application/octet-stream
*.webm = svn:mime-type=application/octet-stream

### Audio files
*.flac = svn:mime-type=application/octet-stream
*.mp3 = svn:mime-type=application/octet-stream

### Document files
*.csv = svn:eol-style=native
*.dfxp = svn:eol-style=native
*.docx = svn:mime-type=application/octet-stream
*.html = svn:eol-style=native
*.md = svn:eol-style=native
*.pdf = svn:mime-type=application/octet-stream
*.rtf = svn:mime-type=application/octet-stream
*.tsv = svn:eol-style=native
*.txt = svn:eol-style=native
*.vtt = svn:eol-style=native

### Translation files
*.mo = svn:eol-style=native
*.po = svn:mime-type=application/octet-stream
*.pot = svn:eol-style=native

### Config files
*.dist = svn:eol-style=native
*.editorconfig = svn:eol-style=native
*.env = svn:eol-style=native
*.gitignore = svn:eol-style=native
*.ini = svn:eol-style=native
*.jshintrc = svn:eol-style=native
*.json = svn:eol-style=native
*.lock = svn:eol-style=native
*.xml = svn:eol-style=native
*.yml = svn:eol-style=native

### Miscellaneous
*.crt = svn:eol-style=native
*.map = svn:eol-style=native
*.nodelete = svn:eol-style=native
*.sql = svn:eol-style=native

42594.diff adds the above list to svn:auto-props. Would love a sanity check before committing that.

#19 @SergeyBiryukov
4 years ago

Note that [46586/trunk/wp-config-sample.php] changed svn:eol-style from CRLF to native.

Previously, it was intentionally set to CRLF in #12775 (see also #20890 and #28187).

That said, there might be arguments for reconsidering the previous decision, see comment:3:ticket:45138.

Just wanted to make sure the change is given a proper consideration. If so, #45138 can be closed as fixed.

#20 @SergeyBiryukov
4 years ago

Just a note that [46586] made #31432 much more prominent. Basically any test with a multiline string output now fails on Windows, seeing 47 failures on trunk.

Last edited 4 years ago by SergeyBiryukov (previous) (diff)

#21 @SergeyBiryukov
4 years ago

It's also a bit awkward that phpcbf and phpcs convert all files to \n and enforce Unix EOLs on Windows (see #47411), then SVN converts them back to \r\n, and so on.

Seems like we have two options here:

  1. Don't enforce a particular EOL style.
  2. If enforcing, don't not change it back and forth.
Last edited 4 years ago by SergeyBiryukov (previous) (diff)

#22 @SergeyBiryukov
4 years ago

In 46612:

Build/Test Tools: Ignore EOL differences in tests using multiline string assertions.

Unix vs. Windows EOL style mismatches can cause misleading failures in tests using the heredoc syntax (<<<) or multiline strings as the expected result.

Fixes #31432. See #42594, #47411.

#23 @desrosj
4 years ago

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

In 46636:

Add svn:auto-props for eol-style and mime-type to trunk.

svn:eol-style and svn:mime-type property default values are now provided for all file types currently in trunk.

This also reverts the eol-style change to wp-config-sample.php made in [46586].

Fixes #42594.

#24 @desrosj
4 years ago

I rechecked all files in trunk using the commands above, and all files looked correct.

[46636] should fix eol-style and mime-type going forward, provided new file types are not added. When that happens, the auto-props will need to be updated.

@SergeyBiryukov I reverted the change to wp-config-sample.php in [46636]. Until we can confirm that changing it is appropriate, I think it's best to investigate in the other tickets that discuss the specific problems with that change.

Note: See TracTickets for help on using tickets.