search

tarides / ocaml-platform-installer

The best way for developers to write software in OCaml
ISC License
60 stars 8 forks source link

Cache migration #137

Closed Julow closed 1 year ago

Julow commented 1 year ago

Rebased on top of both https://github.com/tarides/ocaml-platform-installer/pull/135 and https://github.com/tarides/ocaml-platform-installer/pull/136 (must be rebased and merged last)

Fix https://github.com/tarides/ocaml-platform-installer/issues/131

This adds a mechanism to migrate the cache after the tool is upgraded. The cache has an incrementing version number written to it, which is checked before doing anything with it.

TODO:

panglesd commented 1 year ago

I rebased on master (not on #134 as we should wait for it to be merged in case we modify it, even though it depends on it). I implemented the 0 -> 1 migration.

It remains to test everything...

panglesd commented 1 year ago

I added a test, which will normally fail until we rebase on #134 (currently we do the migration but do not know how to use the new repo structure...)

panglesd commented 1 year ago

It is rebased and CI is green: ready for review!

panglesd commented 1 year ago

I think that while this cache migration is not the most important, later cache migration will be required. Since it is only for few users, it hopefully won't require that much maintenance, will avoid losing users out of frustration, and be a good practice for later migration function.

(Note that I know we might lose users with frustration if the migration cache does not work as well)

Julow commented 1 year ago

I agree that migration is something we should have. I'm considering whether this specific migration is useful.

Is it enough to rename the packages after https://github.com/tarides/ocaml-platform-installer/pull/134 ?

I don't remember if it is important to replace all occurrences of the old string.

This is a sign that we are writing bugs. I'm not confident in writing such code without better, faster tests (https://github.com/tarides/ocaml-platform-installer/issues/143), which are also not something we can do before the planned release.

Julow commented 1 year ago

I added a missing migration. I'll merge when the CI is green.