search

Nuzair46 / BlockTheSpot-Mac

Spotify Ad blocker for MacOS
GNU General Public License v3.0
1.35k stars 82 forks source link

Avoid overwriting original xpui.bak #11

Closed jetfir3 closed 1 year ago

jetfir3 commented 1 year ago

Checks if xpui.bak already exists before trying to create xpui.bak -- this will avoid cases where script is run multiple times on the same Spotify install.

Nuzair46 commented 1 year ago

When user updates Spotify, the xpui.bak from previous version will be still there. and if the user runs the patch, it wont create the backup of new xpui. So incase the user uninstalls the patch, they will have a broken spotify with old xpui.

ideally the script is supposed to run only once. so i dont see a point where this change is more useful. let me know your thoughts

jetfir3 commented 1 year ago

I have not observed app directory behavior when updating through the app's own updater. If you have observed that the Spotify updater does not touch existing files which are not being overwritten then your point is indeed valid.

My concern was users who run the script twice and they end up with a xpui.bak which is then a backup of the patched xpui.spa -- but the update handling situation would be the more important variable to cater to.

I am not sure if there is a way to handle both variables and in that case, we would just have to hope no one runs the script twice and then wants to revert to stock with uninstall.sh.

Nuzair46 commented 1 year ago

Yea. the script only needs to run once. In the future we can try something to know if the client was patched previously. closing this for now

jetfir3 commented 1 year ago

Yea. the script only needs to run once. In the future we can try something to know if the client was patched previously. closing this for now

As a test, I installed a fresh v1.1.94, ran SpotX, let the client update itself to 1.1.95 and restarted app after the update -- no xpui.bak file was found in /Applications/Spotify.app/Contents/Resources/Apps/ so it appears that updating the app takes care of the xpui.bak itself.

Nuzair46 commented 1 year ago

@jetfir3 so the xpui.bak is cleared when Spotify is updated? in that case this pr makes sense. Confirm this and ill merge.

amd64fox commented 1 year ago

I feel like this is not correct logic

If the user runs the SpotX installation again, without updating the client, the stock backup will be overwritten with the modified version.

jetfir3 commented 1 year ago

@jetfir3 so the xpui.bak is cleared when Spotify is updated? in that case this pr makes sense. Confirm this and ill merge.

Just tested again to cover all bases and, yes, when the app is updated, the xpui.bak file is removed. Conditions tested were: -Updating app via in-app updater -- Upon clicking the "restart app" button after update was finished... the app closes, app files are replaced (xpui.bak is removed) and app relaunches on new stock version.

-Updating app manually via official DMG distribution from Spotify.com -- Mounted DMG, copied app to Applications shortcut, gave permission to overwrite existing files. App is then copied to /Applications/ directory and the existing xpui.bak file is removed.

So using either "official" method of updating the app will remove the xpui.bak file.

The only possible condition where this system could fail is people manually copying over updated app files from a TBZ archive. Such behavior is not something we should plan around.

So I think this is good to merge.