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

fix: Use lower case for Wasm persistence modes #4854

Merged
merged 8 commits into from
Jan 17, 2025

Conversation

luc-blaeser
Copy link
Contributor

@luc-blaeser luc-blaeser commented Jan 16, 2025

Related to dfinity/ic#3479.

This changes the variant names for the Wasm memory persistence modes used for enhanced orthogonal persistence to lower case to align it with the IC specification, i.e. using keep and replace instead of Keep and Replace.

For installed Motoko programs using enhanced orthogonal persistence, the change only affects the programmatic upgrades of Motoko actor class instances. Existing such programs would need to be recompiled with a new Motoko compiler and upgraded. The IC detects mismatching variant names of Wasm memory persistence by raising an error while still preserving persistent memory.

@crusso
Copy link
Contributor

crusso commented Jan 16, 2025

Might have been easier to just change the spec to match the implementation ;->

@luc-blaeser
Copy link
Contributor Author

Might have been easier to just change the spec to match the implementation ;->

Yes, I was also tempted. But better going this more painful path to keep IC specification more consistent (reducing "technical debt").

@luc-blaeser luc-blaeser changed the title Lower case persistence modes fix: Use lower case for Wasm persistence modes Jan 17, 2025
@luc-blaeser luc-blaeser marked this pull request as ready for review January 17, 2025 08:44
@luc-blaeser luc-blaeser requested a review from a team as a code owner January 17, 2025 08:44
@luc-blaeser luc-blaeser requested review from ggreif and crusso January 17, 2025 08:44
@crusso
Copy link
Contributor

crusso commented Jan 17, 2025

LGTM.
Should we mark it DO_NOT_MERGE for now?

Copy link

github-actions bot commented Jan 17, 2025

Comparing from a9bc214 to bef6e4b:
In terms of gas, 5 tests regressed and the mean change is +0.6%.
In terms of size, no changes are observed in 5 tests.

github-merge-queue bot pushed a commit to dfinity/ic that referenced this pull request Jan 17, 2025
Change variant names for the Wasm memory persistence to lower case in
line with the IC specification, i.e. using `keep` and `replace` instead
of `Keep` and `Replace`. The Wasm memory persistence option is used for
Motoko's enhanced orthogonal persistence, which is currently still in
beta testing.

The following components need to be updated once these changes are
released on IC mainnet and `dfx`:
* The Motoko compiler (dfinity/motoko#4854)
* The Motoko playground
(dfinity/motoko-playground#275)

No change is needed for `dfx`.

For installed Motoko programs using enhanced orthogonal persistence, the
change only affects the programmatic upgrades of Motoko actor class
instances. Existing such programs would need to be recompiled with a new
Motoko compiler and upgraded. The IC detects mismatching variant names
of Wasm memory persistence by raising an error while still preserving
persistent memory.
github-merge-queue bot pushed a commit to dfinity/ic that referenced this pull request Jan 17, 2025
Change variant names for the Wasm memory persistence to lower case in
line with the IC specification, i.e. using `keep` and `replace` instead
of `Keep` and `Replace`. The Wasm memory persistence option is used for
Motoko's enhanced orthogonal persistence, which is currently still in
beta testing.

The following components need to be updated once these changes are
released on IC mainnet and `dfx`:
* The Motoko compiler (dfinity/motoko#4854)
* The Motoko playground
(dfinity/motoko-playground#275)

No change is needed for `dfx`.

For installed Motoko programs using enhanced orthogonal persistence, the
change only affects the programmatic upgrades of Motoko actor class
instances. Existing such programs would need to be recompiled with a new
Motoko compiler and upgraded. The IC detects mismatching variant names
of Wasm memory persistence by raising an error while still preserving
persistent memory.
@luc-blaeser
Copy link
Contributor Author

We can merge. It is supported in next IC release.

@luc-blaeser luc-blaeser added the automerge-squash When ready, merge (using squash) label Jan 17, 2025
@ggreif
Copy link
Contributor

ggreif commented Jan 17, 2025

We can merge. It is supported in next IC release.

Yep, but let's not forget that dfx must pull the new replica along with moc.

Copy link
Contributor

@ggreif ggreif left a comment

Choose a reason for hiding this comment

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

LGTM!

@mergify mergify bot merged commit 6dcbc9b into master Jan 17, 2025
11 checks passed
@mergify mergify bot removed the automerge-squash When ready, merge (using squash) label Jan 17, 2025
@mergify mergify bot deleted the luc/lower-case-persistence-modes branch January 17, 2025 22:04
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.

3 participants