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

Use changeset tab for reinstall #3726

Merged
merged 1 commit into from
Nov 25, 2022

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Nov 24, 2022

Problem

  1. Install xScienceContinued (which will also install three dependencies)
  2. Right-click it and select "Reinstall"
  3. Click Yes in the confirmation popup
  4. Click Continue on the recommendations screen
  5. Install flow fails:
    reinstall

Cause

The changeset passed to the install flow for reinstallation includes the dependencies, so they're added to the resolver once as dependencies and then again later as user-selected, which triggers an exception.

Changes

  • Now the reinstall option jumps to the changeset tab instead of popping up a confirmation dialog, so you can see what it's going to do
  • Now the changeset tab saves the changeset it displays and passes it to the install flow upon confirmation, rather than re-calculating the changeset from the mod list
  • Now the reinstall option generates a changeset that includes removing the depending/dependency mods but excludes installation of dependencies that will be pulled in automatically

After this, reinstallation succeeds for these mods. I also tried installing RP-1 express and re-installing some of it, and that worked as well. 🤞

Fixes #3725.

@HebaruSan HebaruSan added Bug Something is not working as intended GUI Issues affecting the interactive GUI Pull request labels Nov 24, 2022
@HebaruSan
Copy link
Member Author

@linuxgurugamer, when the build finishes, could you please take it for a test drive? It'll be under the Artifacts dropdown here:

https://github.com/KSP-CKAN/CKAN/pull/3726/checks

Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That all seems to make sense 🎉

@HebaruSan
Copy link
Member Author

HebaruSan commented Nov 25, 2022

I need to review the pull requests that added the dependencies and dependent mods here before merging, to make sure we haven't broken some obscure known use cases...

I guess we can rework this more freely than I thought.

@HebaruSan HebaruSan added In progress We're still working on this and removed In progress We're still working on this labels Nov 25, 2022
@HebaruSan HebaruSan merged commit 2b447ed into KSP-CKAN:master Nov 25, 2022
@HebaruSan HebaruSan deleted the fix/reinstall-changeset branch November 25, 2022 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended GUI Issues affecting the interactive GUI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Trying to reinstall a mod which has a dependency doesn't work due to the dependency
2 participants