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

Is it possible for the windows version of the download script to use os.cp & os.rm instead of os.mv? #6020

Open
ReneMuala opened this issue Dec 30, 2024 · 5 comments

Comments

@ReneMuala
Copy link

In the past moths i had a lot of problems like:

error: cannot move source.tmp\gettext-0.22.3 to source Permission denied
On windows, sometimes restarting the os solved the issue, other it didnt.
After taking a look at the following instructions i modified the download script "xmake\modules\private\action\require\impl\actions\download.lua", changing every os.mv instruction to a pair of os.cp and os.rm. After that, all the issues seem to be gone.

Now i wonder if is it possible for this patch to be applied for windows in the repo.

          > ![source tmp_20221017145323](https://user-images.githubusercontent.com/15936923/196115168-319a9aa0-1d0d-4286-9071-cac7cafe71be.png)

Xmake Version v2.5.6 Operating System Version and Architecture Windows 11 21H2 Git Version git version 2.34.0.windows.1 did not work git version 2.38.0.windows.1 also did not work

edit C:\Program Files\xmake\modules\private\action\require\impl\actions\download.lua os.mv => os.cp can work image

Then how about this one? This issue happens with curl, not git

Originally posted by @xq114 in #2902 (comment)

@waruqi
Copy link
Member

waruqi commented Dec 30, 2024

os.mv has been improved, I don't know why you would need to replace it.

If it does not work, we should try to fix os.mv instead of replacing it with os.cp

@SirLynix
Copy link
Member

I guess os.mv could try to fall back to cp + optional rm (warning instead of error if it fails) in case of failure

@ReneMuala
Copy link
Author

os.mv has been improved, I don't know why you would need to replace it.

If it does not work, we should try to fix os.mv instead of replacing it with os.cp

No problem, i'm open to contribute on that.

@ReneMuala
Copy link
Author

I guess os.mv could try to fall back to cp + optional rm (warning instead of error if it fails) in case of failure

I agree

@waruqi
Copy link
Member

waruqi commented Jan 3, 2025

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

No branches or pull requests

3 participants