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

FileNotFoundException on Windows caused by underlying coursier #3563

Closed
lefou opened this issue Sep 16, 2024 · 14 comments · Fixed by #3568
Closed

FileNotFoundException on Windows caused by underlying coursier #3563

lefou opened this issue Sep 16, 2024 · 14 comments · Fixed by #3568
Milestone

Comments

@lefou
Copy link
Member

lefou commented Sep 16, 2024

Under Windows, I had the following cousier download issues. Rerunning seems to help, so it could be a race condition. Probably a regression.

CI build: https://github.com/com-lihaoyi/mill/actions/runs/10883643568/job/30197520523?pr=3560

1 targets failed
contrib.playlib.worker[2.6].resolvedRunIvyDeps java.io.FileNotFoundException: C:\Users\runneradmin\AppData\Local\Coursier\cache\v1\https\repo1.maven.org\maven2\com\lihaoyi\os-lib_2.12\0.10.7\os-lib_2.12-0.10.7.jar (The process cannot access the file because it is being used by another process)
@lefou
Copy link
Member Author

lefou commented Sep 16, 2024

It removed the retry handler for this specific exception.

cc @alexarchambault

@lefou
Copy link
Member Author

lefou commented Sep 16, 2024

Retry-related, but a different exception:

https://github.com/com-lihaoyi/mill/actions/runs/10884321338/job/30199843323?pr=3562

scalalib.resolvedIvyDeps java.nio.file.AccessDeniedException: C:\Users\runneradmin\AppData\Local\Coursier\cache\v1\https\repo1.maven.org\maven2\org\scalameta\scalafmt-dynamic_2.13\3.8.3\.scalafmt-dynamic_2.13-3.8.3.jar__sha1.computed18409881239238064357.tmp -> C:\Users\runneradmin\AppData\Local\Coursier\cache\v1\https\repo1.maven.org\maven2\org\scalameta\scalafmt-dynamic_2.13\3.8.3\.scalafmt-dynamic_2.13-3.8.3.jar__sha1.computed

and

https://github.com/com-lihaoyi/mill/actions/runs/10884321338/job/30199843621?pr=3562

scalalib.resolvedRunIvyDeps java.nio.file.AccessDeniedException: C:\Users\runneradmin\AppData\Local\Coursier\cache\v1\https\repo1.maven.org\maven2\com\typesafe\config\1.4.3\.config-1.4.3.jar__sha1.computed13153222522663641967.tmp -> C:\Users\runneradmin\AppData\Local\Coursier\cache\v1\https\repo1.maven.org\maven2\com\typesafe\config\1.4.3\.config-1.4.3.jar__sha1.computed

@lefou
Copy link
Member Author

lefou commented Sep 16, 2024

I think we should re-add Mills own retry mechanism until we can prove it's fixed in coursier. We could disable it via a feature-flag via an env variable, so that testing without retry-handling in Mill is easy. But for the better user experience, we better keep retry-handling in Mill for now. It doesn't hurt unless it's needed, then it's really helpful.

@lefou lefou changed the title FillNotFoundException on Windows in caused by underlying coursier FileNotFoundException on Windows in caused by underlying coursier Sep 16, 2024
@lefou lefou changed the title FileNotFoundException on Windows in caused by underlying coursier FileNotFoundException on Windows caused by underlying coursier Sep 16, 2024
@alexarchambault
Copy link
Contributor

I should be able to have a look tomorrow Europe time (morning or afternoon, I'm not sure yet).

I'm not sure about defaulting to retries by default, unless it really hampers development on Mill for you right now. A benefit of removing them in Mill is precisely to unearth those issues, to fix the root cause (or at least, adding a retry right around where it should be added in coursier).

@alexarchambault
Copy link
Contributor

alexarchambault commented Sep 16, 2024

Post links to CI failures or stack traces here as soon as you run into one, the more we have, the better.

@lefou
Copy link
Member Author

lefou commented Sep 16, 2024

Yeah, I agree in general. But the price pays the user. Wouldn't it be better to print out some big fat warning when we retry. That way, you can still see all issue but users don't end up with broken workflows. Some users reported and handled these kind of issues for years, so I'd rather not mess with their user experience.

@lefou
Copy link
Member Author

lefou commented Sep 16, 2024

@lefou
Copy link
Member Author

lefou commented Sep 16, 2024

@lefou
Copy link
Member Author

lefou commented Sep 16, 2024

@alexarchambault
Copy link
Contributor

To precise my point of view, this has to be addressed before 0.12.0 final, so that Mill users at large don't pay its price. Or earlier if the impact on Mill development is too high. Else, it's best to notice when it happens IMO, so that it can be fixed properly.

@lihaoyi
Copy link
Member

lihaoyi commented Sep 16, 2024

Let's go ahead without Mill retries for now. If it remains a problem long term we can put them back, but the point of the RC period is to surface issues, and we don't have infra for out-of-band error reporting set up so we cant just log to ELK or Sentry. For now we have reason to believe the issues can be fixed in a reasonable period of time so surfacing them is great. Unless they're really so painful that development is impossible because of them

@lefou
Copy link
Member Author

lefou commented Sep 16, 2024

I hope for an upstream fix, I really do. I'm also tired of arguing this topic. To give some perspective: these upstream issues are known for years and I lost lots of work hours until I landed these workarounds. Experiencing these regressions is frustrating and unnecessary. I don't think our early adopters (me included) should experience these issues (again).

@lihaoyi
Copy link
Member

lihaoyi commented Sep 17, 2024

Hit another AccessDeniedException here https://github.com/com-lihaoyi/mill/actions/runs/10894796574/job/30232196351

@alexarchambault
Copy link
Contributor

I have hope that coursier/coursier#3076 fixes that. It's being released right now.

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 a pull request may close this issue.

3 participants