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

setup-ocaml/cache: allow creating cache key out of local repos #656

Merged
merged 1 commit into from Mar 20, 2023

Conversation

psafont
Copy link
Contributor

@psafont psafont commented Mar 16, 2023

Do not try to parse the location as url as it may be a relative filepath, this is a potential fix for #649

Not sure if it's the correct choice for all cases. Another option would be to read the URL from opam once it's been added, or generate it in an ad-hoc way from the relative filepath, then it could be passed to the function and in theory it should work, although you may want to add the u.hash as well to the urn

src/setup-ocaml/cache.ts Outdated Show resolved Hide resolved
@psafont psafont force-pushed the local-urls branch 2 times, most recently from f59f021 to 6bbfe72 Compare March 17, 2023 11:33
@psafont
Copy link
Contributor Author

psafont commented Mar 17, 2023

Current commit created this cache key:
v1-setup-ocaml-2.0.12-opam-download-/home/runner/work/xs-opam/xs-opam-4_08_1-2023-2-17-4437730611-20

Is this an acceptable name?

@psafont psafont changed the title setup-ocaml/cache: use repo names for cache key setup-ocaml/cache: allow creating cache key out of local repos Mar 17, 2023
@smorimoto
Copy link
Member

That looks good!

@smorimoto
Copy link
Member

@psafont Could you add a change log entry?

@smorimoto smorimoto added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Mar 17, 2023
The location might be a relative filepath instead of a url, create a
fallback path for these, sidestepping opam to create the url for all
cases.
@psafont
Copy link
Contributor Author

psafont commented Mar 20, 2023

Should be good to go now!

@smorimoto
Copy link
Member

Fantastic!

@smorimoto smorimoto merged commit 7ce8524 into ocaml:master Mar 20, 2023
9 checks passed
@psafont psafont deleted the local-urls branch March 20, 2023 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants