search

microsoft / winget-pkgs

The Microsoft community Windows Package Manager manifest repository
MIT License
8.74k stars 4.56k forks source link

.gitattributes changes causing validation errors #72254

Open Trenly opened 2 years ago

Trenly commented 2 years ago

This has had the unintentional effect of creating validation errors. Any time a user is not cautious when creating a PR, it causes additional files to have their line endings updated to CRLF. Because the pipelines can only handle changes to one manifest at a time, this results in the Pull-Request-Error label being assigned to PR's.

This change also has un-intended implications for other integrations to winget-pkgs. It means that MSFTBot may need to be updated to use CRLF, or it will cause files to be added which create the Pull-Request-Error label on others' PRs. It means that WingetCreate may need to be updated to use CRLF. YamlCreate will need to be validated and potentially updated. Validation will need to be added to catch LF-only files.

All in all, what should have been a simple change to a single file has a rippling effect throughout the WinGet community.

To resolve the ripple effects, I think there are a few options.

1) Just wait. Scripts have been kicked off to change line endings on the files to CRLF. It will take upwards of 60 hours to complete (due to PR rate limits), but it will eventually get to a state of equilibrium. However, this doesn't address any of the other rippling effects to MSFTBot / WingetCreate / YamlCreate / etc. 2) BIIIIIG PR. We're talking tens of thousands of changes across all the files. Granted, because it is "Just line endings" it would be low risk, an engineer would be required to create the PR, one to review it, and a rebuild run afterwards to verify and validate. Again, doesn't address the rippling effects. Because of the scope of changes this PR would encompass, I think this is not a good option. Possible - yes; Practical - no. 3) Rollback the change to .gitattributes. This would revert the consistency requirement of line endings, but immediately resolves the issue.

I suggest we roll back the changes to the .gitattributes file and take a more careful approach to ensuring consistency.

Thoughts?

cc @denelon @OfficialEsco @jedieaston @ItzLevvie @ImJoakim @KaranKad @quhxl and anyone else who has an opinion

Okeanos commented 2 years ago

Sorry about that – that definitely wasn't my intention :(.

Your proposed plan of action as outlined in the 6 steps at the end sounds good.

Trenly commented 2 years ago

Sorry about that – that definitely wasn't my intention :(.

It's all good! I don't think anyone realized what would happen until validation started acting funny. It's not your fault (regardless of what git blame says)

jedieaston commented 2 years ago

My opinion: Go for option 3, and maybe we can add a GitHub Action that verifies the line endings until the validation service can be updated to look for that, similar to the spell checker. Unless a kind engineer feels like deploying a change on a Friday.

edit: nvm, just do the steps at the end of @Trenly's comment. Sounds good to me, although I'd still like a way to get a line endings warning in the short term so we can start cleaning it up as needed when we see it in PRs.