-
Notifications
You must be signed in to change notification settings - Fork 237
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 coursier's default cache directory when fetching jars to update the install.json file #407
Use coursier's default cache directory when fetching jars to update the install.json file #407
Conversation
coursier.bzl
Outdated
|
||
# locations from https://get-coursier.io/docs/1.1.0-M14-1/cache.html#default-location | ||
if _is_windows(os_name): | ||
return "%s\\Coursier\\Cache\\v1" % os_env.get("LOCALAPPDATA") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay, I just managed to try this on a Windows machine.
While pinning artifacts on Windows is still pretty broken with the pin
script not working properly (#324), this should be "%s/Coursier/cache/v1" % os_env.get("LOCALAPPDATA").replace("\\", "/")
, based on what I've tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for testing it on Windows! Is it supposed to be lower case cache
in the path or captilized Cache
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is cache
on my machine.
coursier.bzl
Outdated
return location | ||
return coursier_cache_env_var | ||
|
||
# locations from https://get-coursier.io/docs/1.1.0-M14-1/cache.html#default-location |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current version is https://get-coursier.io/docs/2.0.0-RC5-3/cache.html#default-location
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
Thank you so much for this! Just tried it out on this repo itself, it is a big speedup. |
…he install.json file
4e47cd3
to
3118e15
Compare
Every time I run
bazel run @unpinned_maven//:pin
it downloads every jar defined in themaven_install
target because the @unpinned_maven repository is deleted every time it is "fetched". Basically the Coursier cache used for fetching atexternal/unpinned_maven/v1
is never re-used when you are updating themaven_install.json
file.This PR changes the unpinned fetch so it uses Coursier's default cache locations or the COURSIER_CACHE env var for the cache.
By re-using the cache a
bazel run @unpinned_maven//:pin
update now takes around 60 seconds instead of 10-15 minutes when the cache is empty.