@tsteur opened this Issue on December 2nd 2021 Member

Seen eg in https://github.com/matomo-org/matomo/issues/18411#issuecomment-984201606 and https://github.com/matomo-org/matomo/issues/18415 https://github.com/matomo-org/matomo/issues/18418

At least some of these files (I'm thinking node_modules/iframe-resizer/.eslintrc and node_modules/ng-dialog/.eslintrc, and vendor/php-di/php-di/.gitstats.yml and vendor/php-di/php-di/.phpstorm.meta.php) we would have expected to be deleted automatically eg in https://github.com/matomo-org/matomo/blob/4.6.1/plugins/CoreUpdater/Model.php#L26-L36 Unless there's a different problem that these files shouldn't have been in the manifest file or so.

Also maybe we can check if we can delete some files in the Matomo root directory which we haven't done in the past. This can be tricky though as some files like a .htaccess we wouldn't want to delete. As part of this issue we investigate if we can delete some of these files automatically, or whether tests can help us make aware that we need to delete certain files manually if they are gone in the release (might be tricky as it's done in the build script I assume). If we can't delete them automatically, and can't test for it, then we need to make sure that when we make changes to the package script that we are made aware in any way to not forget to change this.

Maybe even a test could otherwise fetch the build script and parse for rm -rf calls etc.

@sgiehl commented on December 3rd 2021 Member

@tsteur We are actually ignoring all files starting with . on update. Those file won't be deleted if they exist.
The reason for ignoring is actually the glob(*) we are using. It doesn't catch hidden files. This can actually be fixed and I will prepare a PR for it.

@sgiehl commented on December 3rd 2021 Member

@tsteur regarding the other part of this issue. I wouldn't start trying to sync/remove other parts. Guess we had a good reason not to do that. To circumvent we have similar issues in the future we could create a github action, that automatically checks the changes in a PR. And if a file is deleted, that wouldn't be caught by the updater we could add a comment on the PR to check if an update script is provided to remove those files.
Guess it should be quite simple to do that. Just something like git log {BASESHA}..{PRSHA} --diff-filter=D --summary | grep "delete mode" and then go through the list and check if it is in certain directories or not...
Or would you prefer another approach?

@tsteur commented on December 5th 2021 Member

Would that action run on the archive repository? As we might delete a file from root directory in the script that builds the release? And it would run in this repository?

could maybe also a regular test run the above command so it runs as part of the tests?

@sgiehl commented on December 6th 2021 Member

No, that action would run in this repository for all pull request.
I don't yet have a good idea how to write a test for that. We would actually need to build a release. Download the latest release, perform an update, and check if the file integrity check complains about anything. Imho that sounds a bit oversized for a normal test. It might possibly be something I could implement it #17594 and let a release fail if the integrity check complains 🤔

@tsteur commented on December 6th 2021 Member

👍 OK. I was hoping we could just run git log {BASESHA}..{PRSHA} --diff-filter=D --summary | grep "delete mode" in a regular test but maybe something like that doesn't work

@atom-box commented on February 9th 2022

A user noticed this issue:
"The one thing I want to see improved? The ... file checks during the update. Every ... time the updater nags me about supposedly unexpected files that I'm supposed to manually delete before I can continue. each. .... Just. These are files that Matomo put in there himself during the last update, why are they unexpected? And if they suddenly bother you, why doesn't the updater delete the ... itself or at least offer me a button that I can use to do this conveniently so that I don't have to log in to the server myself? I updated Matomo two days ago and of course had to delete files manually again. Now I get another mail about an update to 4.7.1, and of course I have to delete a few other files manually. Get your build process on track or teach the updater not to be such a chump"

Powered by GitHub Issue Mirror