Skip to content

fix: update Makefile with correct paths#1095

Merged
sonic2kk merged 1 commit into
sonic2kk:masterfrom
koppchen:fix-make-uninstall
Apr 28, 2024
Merged

fix: update Makefile with correct paths#1095
sonic2kk merged 1 commit into
sonic2kk:masterfrom
koppchen:fix-make-uninstall

Conversation

@koppchen

Copy link
Copy Markdown
Contributor

Small fix to update the Makefile with correct paths so make uninstall removes "steamtinkerlaunch.svg" and "steamtinkerlaunch.desktop". This also removes the unnecessarily -r (recursive) flag, while removing those two files.

Testing (before)

Shows two files are left behind after make uninstall.

steamtinkerlaunch (master =)$ rm -rf /tmp/test; mkdir /tmp/test

steamtinkerlaunch (master =)$ PREFIX=/tmp/test make install
sed "s:^PREFIX=\"/usr\":PREFIX=\"/tmp/test\":" -i steamtinkerlaunch
install -Dm755 steamtinkerlaunch -t "/tmp/test/bin"
install -d "/tmp/test/share/steamtinkerlaunch"
cp -r collections eval guicfgs lang misc "/tmp/test/share/steamtinkerlaunch"
install -Dm644 README.md -t "/tmp/test/share/doc/steamtinkerlaunch"
install -Dm644 "misc/steamtinkerlaunch.desktop" -t "/tmp/test/share/applications"
install -Dm644 "misc/steamtinkerlaunch.svg" -t "/tmp/test/share/icons/hicolor/scalable/apps"

steamtinkerlaunch (master =)$ find /tmp/test -type f | wc -l
181

steamtinkerlaunch (master =)$ PREFIX=/tmp/test make uninstall
rm -rf "/tmp/test/share/icons/hicolor/scalable/apps/misc/steamtinkerlaunch.svg"
rm -rf "/tmp/test/share/applications/misc/steamtinkerlaunch.desktop"
rm -rf "/tmp/test/share/doc/steamtinkerlaunch"
rm -rf "/tmp/test/share/steamtinkerlaunch"
rm -f "/tmp/test/bin/steamtinkerlaunch"

steamtinkerlaunch (master =)$ find /tmp/test -type f | wc -l
2

steamtinkerlaunch (master =)$ find /tmp/test -type f
/tmp/test/share/applications/steamtinkerlaunch.desktop
/tmp/test/share/icons/hicolor/scalable/apps/steamtinkerlaunch.svg

Testing (after)

Shows no files are left behind after make uninstall.

steamtinkerlaunch (fix-make-uninstall)$ rm -rf /tmp/test; mkdir /tmp/test

steamtinkerlaunch (fix-make-uninstall)$ PREFIX=/tmp/test make install
sed "s:^PREFIX=\"/usr\":PREFIX=\"/tmp/test\":" -i steamtinkerlaunch
install -Dm755 steamtinkerlaunch -t "/tmp/test/bin"
install -d "/tmp/test/share/steamtinkerlaunch"
cp -r collections eval guicfgs lang misc "/tmp/test/share/steamtinkerlaunch"
install -Dm644 README.md -t "/tmp/test/share/doc/steamtinkerlaunch"
install -Dm644 "misc/steamtinkerlaunch.desktop" -t "/tmp/test/share/applications"
install -Dm644 "misc/steamtinkerlaunch.svg" -t "/tmp/test/share/icons/hicolor/scalable/apps"

steamtinkerlaunch (fix-make-uninstall)$ find /tmp/test -type f | wc -l
181

steamtinkerlaunch (fix-make-uninstall)$ PREFIX=/tmp/test make uninstall
rm -f "/tmp/test/share/icons/hicolor/scalable/apps/steamtinkerlaunch.svg"
rm -f "/tmp/test/share/applications/steamtinkerlaunch.desktop"
rm -rf "/tmp/test/share/doc/steamtinkerlaunch"
rm -rf "/tmp/test/share/steamtinkerlaunch"
rm -f "/tmp/test/bin/steamtinkerlaunch"

steamtinkerlaunch (fix-make-uninstall)$ find /tmp/test -type f | wc -l
0

Thanks!

Update Makefile with correct paths so "make uninstall" removes
"steamtinkerlaunch.svg" and "steamtinkerlaunch.desktop".
@sonic2kk

Copy link
Copy Markdown
Owner

Wow! Thank you for catching those incorrect paths. I'm not sure how this went undetected for so long, you might've saw in the blame that it's been this way since I added this way back in #608. I triple-checked because I could not believe these were wrong.

The change to use -f is also very welcome. We already just use -f for the steamtinkerlaunch script so it's strange that even this was missed! Past sonic2kk was a silly, silly person.

Thanks a bunch for finding and fixing this. I'll merge it now :-)

@sonic2kk sonic2kk merged commit 02e49ab into sonic2kk:master Apr 28, 2024
@sonic2kk

Copy link
Copy Markdown
Owner

Changelog has been updated to give credit, under the "Fixes" section: https://github.com/sonic2kk/steamtinkerlaunch/wiki/Changelog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants