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

bugfix: Redownload artifacts if missing #2604

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tgodzik
Copy link
Contributor

@tgodzik tgodzik commented Feb 12, 2025

Fixes #821

This might cause loading configuration a bit, but that should not be an issue compared to potential user trouble when the artifacts are missing.

The alternative is to only react to missing main scala jars, which would indicate that a cache was cleaned, but not if it was deleted only partly

Fixes scalacenter#821

This might cause loading configuration a bit, but that should not be an issue compared to potential user trouble when the artifacts are missing.
@tgodzik tgodzik requested a review from adpi2 February 12, 2025 19:00
Copy link
Member

@adpi2 adpi2 left a comment

Choose a reason for hiding this comment

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

I am not sure about this. Cleaning the cache is not something we can or should do blindly. When we decide to do it anyway, for whatever reason, maybe it is reasonable that we have to re-import each project.

Also technically I have a few concerns:

  • Resolving each artifact one after the other can be very slow: Coursier needs to do the conflict resolution of transitive dependencies again and again. Can we bypass the resolution and directly download the missing JAR? Or, should we do one resolution of all the missing libraries at once?
  • We don't even know if Coursier is going to put the JAR in the right folder. Maybe the project does not use Coursier, or it has some custom cache configuration. Should we check the output files and copy them if necessary?

@tgodzik
Copy link
Contributor Author

tgodzik commented Feb 13, 2025

You're right about the points raised, I should fix that.

As for the validity of the approach I just felt it's not that big of an issue to add it, since currently users are quite confused about what is happening. And sometimes those artifacts are removed by the build tool like in case of gradle.

The alternative approach would be to have that check and add a better error message so that the users know they need to reimport.

What do you think?

@adpi2
Copy link
Member

adpi2 commented Feb 13, 2025

And sometimes those artifacts are removed by the build tool like in case of gradle.

Do you know why Gradle does such thing?

@tgodzik
Copy link
Contributor Author

tgodzik commented Feb 13, 2025

I think it was local cache inside the project, but not sure if that is a standard behaviour even

@adpi2
Copy link
Member

adpi2 commented Feb 13, 2025

I think it was local cache inside the project, but not sure if that is a standard behaviour even

Then I think it would be weird for Bloop to write files inside Gradle local cache.

Maybe it is safer for Bloop not to interact too much with project dependencies. So I would opt for a better error message.

Although I would not be mad if you disagree :)

@tgodzik
Copy link
Contributor Author

tgodzik commented Feb 13, 2025

I think it was local cache inside the project, but not sure if that is a standard behaviour even

Then I think it would be weird for Bloop to write files inside Gradle local cache.

Maybe it is safer for Bloop not to interact too much with project dependencies. So I would opt for a better error message.

Although I would not be mad if you disagree :)

Not super sure if I disagree 😅 Potentially we could only do it for things in coursier cache, but yeah it all kind of falls apart

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.

Deleting cached jars triggers NoSuchFileException
2 participants