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

fix(psmodules): Remove folder recursively when unlinking previous module path #5127

Merged
merged 3 commits into from
Aug 29, 2022

Conversation

AkariiinMKII
Copy link
Contributor

@AkariiinMKII AkariiinMKII commented Aug 26, 2022

Description

Add a -Recurse parameter to Remove-Item cmdlet when unlinking psmodule path.

Motivation and Context

How Has This Been Tested?

I've tested my codes by performing update for the same app, everything seems ok

Linking ~\scoop\modules\WhoAteMyRAM => ~\scoop\apps\WhoAteMyRAM\current
WARN  ~\scoop\modules\WhoAteMyRAM already exists. It will be replaced.
Running post_install script...
'WhoAteMyRAM' (1.0.10) was installed successfully!

Changes may effect:

uninstall_psmodule $manifest $refdir $global

install_psmodule $manifest $dir $global

Checklist:

  • I have read the Contributing Guide.
  • I have ensured that I am targeting the develop branch.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.
  • I have added an entry in the CHANGELOG.

@rashil2000
Copy link
Member

Could this also solve #4844?

@AkariiinMKII
Copy link
Contributor Author

AkariiinMKII commented Aug 26, 2022

Could this also solve #4844?

Nope I guess, his issue happens in uninstallation process, while mine happens in installation process.

@rashil2000
Copy link
Member

Which PS module do you face this error in? Does it happen in any/all PS modules?

@AkariiinMKII
Copy link
Contributor Author

Which PS module do you face this error in? Does it happen in any/all PS modules?

Yes, it happens in all modules.

@rashil2000
Copy link
Member

I am unable to repro this issue:

image

@AkariiinMKII
Copy link
Contributor Author

AkariiinMKII commented Aug 28, 2022

I am unable to repro this issue:

image

Maybe it's because of PS version, it appears in PS 5.1, which is included in Win 10 by default.
image
image

@AkariiinMKII
Copy link
Contributor Author

AkariiinMKII commented Aug 28, 2022

There is another solution that we run uninstall_psmodule during update process, even the warning message won't appear if we act like this, but without -Recurse parameter, confirm is still required if there is conflict folder, which is not empty, while installing.
Maybe we need to change both?

image

rashil2000
rashil2000 previously approved these changes Aug 28, 2022
Copy link
Member

@rashil2000 rashil2000 left a comment

Choose a reason for hiding this comment

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

Right, the prompt appears in PS 5.1

This should fix it.

niheaven
niheaven previously approved these changes Aug 29, 2022
@niheaven niheaven dismissed stale reviews from rashil2000 and themself via 5ad7677 August 29, 2022 15:37
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.

3 participants