Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

On update, automatically delete from filesystem all Piwik files that are not in the latest release #5936

Closed
dertuxmalwieder opened this issue Aug 5, 2014 · 19 comments
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Milestone

Comments

@dertuxmalwieder
Copy link

After having used Piwik since 2011 or 2012, I auto-updated to the newest beta today.

Well, damn.

Oops… there was a problem during the request. Maybe the server had a temporary issue, or maybe you requested a report with too much data.

This happens only for a couple of widgets (the one I had before), deleting and readding them doesn't work. PHP's memory limit is "high enough". ;)

What happened?

of7k9c jpg

@mattab mattab added the Bug label Aug 6, 2014
@mattab mattab added this to the Short term milestone Aug 6, 2014
@mattab
Copy link
Member

mattab commented Aug 6, 2014

Check your server error logs, do you find an error listed there? Then look for error in FAQ: http://piwik.org/faq/

@dertuxmalwieder
Copy link
Author

Yes, but the error isn't there:

mod_fcgid: stderr: PHP Fatal error: Cannot use Piwik\Visualization\Config as Config because the name is already in use in /(...)/html/piwik/plugins/CoreVisualizations/Visualizations/HtmlTable/Goals.php on line 16, referer: http://(...)/piwik/index.php?module=CoreHome&action=index&idSite=1&period=range&date=last7

(I removed absolute paths here.)

@sgiehl
Copy link
Member

sgiehl commented Aug 6, 2014

Please remove the file plugins/CoreVisualizations/Visualizations/HtmlTable/Goals.php. That file was moved in d0cb345

@mattab maybe we should add an unlink in an update script to ensure the file is removed

@dertuxmalwieder
Copy link
Author

Nice. That seems to have worked, thanks.
Why doesn't the Updater check that yet?

@sgiehl
Copy link
Member

sgiehl commented Aug 6, 2014

@tsteur @mattab could anyone of you have a look at that and check if an update script make sense for such issues

@tsteur
Copy link
Member

tsteur commented Aug 6, 2014

Maybe we can remove no longer existing files from core and core plugins during update automatically to not have to create such updates everytime?

@sgiehl
Copy link
Member

sgiehl commented Aug 6, 2014

+1 for an automatic solution during updates

@mattab mattab changed the title Update to 2.5b2 destroyed my Piwik Automatically delete from filesystem all files that are not in the latest Piwik release Aug 7, 2014
@mattab mattab changed the title Automatically delete from filesystem all files that are not in the latest Piwik release On update, automatically delete from filesystem all Piwik files that are not in the latest release Aug 7, 2014
@mattab
Copy link
Member

mattab commented Aug 7, 2014

@sgiehl @tsteur I renamed the ticket to On update, automatically delete from filesystem all Piwik files that are not in the latest release in an effort to propose a solution to this problem.

@tsteur
Copy link
Member

tsteur commented Aug 8, 2014

shouldn't this be in the next release as otherwise some installations break?

@dertuxmalwieder
Copy link
Author

Mine won't anymore, but ...

@mattab
Copy link
Member

mattab commented Aug 8, 2014

@tsteur moved to 2.5.0

@mattab mattab modified the milestones: Short term, Piwik 2.5.0 Aug 8, 2014
@tsteur
Copy link
Member

tsteur commented Aug 10, 2014

Wrote a lot of tests and just tested it for a while to make sure it works which it does in general. Basically I compare all files in the core and the plugins/* directory and remove those files that are no longer present in latest.zip. Of course I do not remove any custom plugins etc. I do currently not compare/sync misc, js and other folders although would be easy to do.

Was not easy to test since there is no beta for it yet. Created by own latest.zip and used this for manual testing. Problem is it won't work with the next update or whenever a user updates to a future version from Piwik <=2.4 to Piwik >=2.5. I need to remove all the gone files in the controller method oneClick_copy as all the files are copied there and the downloaded/extracted files are removed at the end of the method. The problem is if you update now, the current controller does not have this code yet since not the controller code of the next update will be used. Any ideas for this problem? I guess we maybe have to create a command in addition that creates an update file automatically as well?

@tsteur
Copy link
Member

tsteur commented Aug 11, 2014

I was just working on an additional update file to remove no longer needed files to remove those files:

'misc/others/test_cookies_GenerateHundredsWebsitesAndVisits.php',
'misc/others/test_generateLotsVisitsWebsites.php',
'core/Tracker/ActionEvent.php',
'plugins/Actions/Widgets.php',
'plugins/CustomVariables/Menu.php',
'plugins/CustomVariables/Widgets.php',
'plugins/DevicesDetection/Widgets.php',
'plugins/Events/Widgets.php',
'plugins/ExampleRssWidget/Controller.php',
'plugins/Live/Menu.php',
'plugins/Provider/Widgets.php',
'plugins/SEO/Controller.php',
'plugins/SegmentEditor/Controller.php',
'plugins/UserCountry/Widgets.php',
'plugins/UserSettings/Widgets.php',
'plugins/VisitTime/Widgets.php',
'plugins/VisitorInterest/Widgets.php'

Problem is the following: Those files might be added in a future version of Piwik again. Imagine we add a plugins/VisitorInterest/Widgets.php file to Piwik in 2.6.0 and a user updates from 2.4.0 to 2.6.0.

What will happen is that plugins/VisitorInterest/Widgets.php will be copied to the users Piwik installation but afterwards removed by an update in 2.5.0 although the file should not be removed. So this solution does not really work either. Maybe we could check the age of the file that is supposed to be deleted and then decide whether to delete or not but even this might not work in all cases (eg if Update executed later etc)

The only solution that seems to work for me is by comparing the new Piwik version with the current Piwik version of the user and remove no longer needed files as explained and implemented above. But users will have to update at least twice until it works (see above).

I just tested an update from 2.4.0 to 2.5.0-b2 and those Widget files above still exist but it is not that much of a problem as same widgets that are added multiple times are only shown once. They are defined in Reports and Widgets now because Widgets are not removed. This can be a problem in the future once the widgets are defined differently in Reports and Widgets class. Same for Menu.

Any further ideas? Or is solution maybe ok for now?

@tsteur tsteur self-assigned this Aug 11, 2014
tsteur added a commit that referenced this issue Aug 11, 2014
…s directories that are no longer present in the new build
tsteur added a commit that referenced this issue Aug 11, 2014
…re they only delete the files they are supposed to do
tsteur added a commit that referenced this issue Aug 11, 2014
tsteur added a commit that referenced this issue Aug 11, 2014
… also brings the advantage to make it better testable. Controller should be always simple/small... removeGoneFiles() is not really testable as it uses the static filesystem and I did not want to make the code ugly to mock it... Looking forward to dependency injection
tsteur added a commit that referenced this issue Aug 11, 2014
@mattab
Copy link
Member

mattab commented Aug 11, 2014

Those files might be added in a future version of Piwik again.

Good catch and well done for thinking of this important use case ahead!

tsteur added a commit that referenced this issue Aug 12, 2014
…s directories that are no longer present in the new build
tsteur added a commit that referenced this issue Aug 12, 2014
…re they only delete the files they are supposed to do
tsteur added a commit that referenced this issue Aug 12, 2014
tsteur added a commit that referenced this issue Aug 12, 2014
… also brings the advantage to make it better testable. Controller should be always simple/small... removeGoneFiles() is not really testable as it uses the static filesystem and I did not want to make the code ugly to mock it... Looking forward to dependency injection
tsteur added a commit that referenced this issue Aug 12, 2014
tsteur added a commit that referenced this issue Aug 12, 2014
@tsteur
Copy link
Member

tsteur commented Aug 12, 2014

Is it ok to close this issue for now? I've created another issue #5985 to refactor the updater completely one day. Then we can maybe find a solution that will work in all cases. For now it will only work if a user performs an update and then performs an update again.

Would be nice to see two betas to actually test it (again). Tested it only with my local instance so far and with custom built latest.zip etc.

@dertuxmalwieder
Copy link
Author

Sure, you can close it, my issue is "fixed" for now. :)

@mattab
Copy link
Member

mattab commented Aug 12, 2014

@tsteur the 2.5.0-b3 was released.
I'll release 2.5.0-rc1 today.

Do you want to wait for this release maybe to close it? anyway your call to close this issue or not. Cheers

@tsteur
Copy link
Member

tsteur commented Aug 12, 2014

Well we could simply reopen it any time and close it now. Would be nice if someone else had a quick look at the code or better the general behavior. The code should work as I have added many tests there (very critical part as we do not want to delete any random files) and I tested it manually as well. This should work but one never knows. So if someone comes a problem into mind why we shouldn't delete no longer needed files in '/core' and '/plugins' that'd be helpful. I can't think of any since we leave all third party plugins untouched. Even the Piwik plugins installed via Marketplace remain untouched.

@mattab
Copy link
Member

mattab commented Aug 12, 2014

So if someone comes a problem into mind why we shouldn't delete no longer needed files in '/core' and '/plugins' that'd be helpful.

there's no file being created or modified in core/ or plugins/ so it's safe to delete all files not in latest release from there (besides third party plugins and themes which should be un-deleted of course )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Projects
None yet
Development

No branches or pull requests

4 participants