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

Use branch'd ink-wrapper for compatibility with aleph-node 12.1 release #23

Merged
merged 5 commits into from
Nov 23, 2023

Conversation

deuszx
Copy link
Collaborator

@deuszx deuszx commented Nov 17, 2023

This PR updates ink-wrapper, installed in the ink-dev image, to install the version from the feature branch where it's compatible with aleph-node release r-12.1. We cannot release ink-wrapper to crates ATM since aleph_client's releases after version 3.0.0 are no longer pushed to the crates. I chose to install via --rev so that later changes to the feature branch (which will definitely come) don't alter the state of the image.

It also adds docker labels and bumps toolchain used to install ink-wrapper.

Here you can find changes made to the ink-wrapper.

@deuszx deuszx requested a review from obrok November 17, 2023 18:53
@deuszx deuszx force-pushed the ink-wrapper-aleph-node-12.1 branch from c815ebb to d8cc950 Compare November 17, 2023 18:53
INK_WRAPPER_VERSION=0.5.0
CARGO_CONTRACT_VERSION=3.2.0

LABEL cargo-contract="$CARGO_CONTRACT_VERSION" \
Copy link
Collaborator Author

@deuszx deuszx Nov 17, 2023

Choose a reason for hiding this comment

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

You can now inspect the labels:

docker image inspect --format='{{json .Config.Labels}}' cardinal-cryptography/ink-dev-arm64

{"cargo-contract":"3.2.0","ink-wrapper":"rev-57fde9f0e31f1de610efe1879715c028849f4bc0","rust":"1.71.0"}

@deuszx deuszx force-pushed the ink-wrapper-aleph-node-12.1 branch from d8cc950 to 1c65f5f Compare November 20, 2023 20:12
Makefile Outdated
@@ -4,7 +4,7 @@

MAKEFILE_NAME := Ink development docker
DOCKER_NAME_INK_DEV := cardinal-cryptography/ink-dev
DOCKER_TAG := 1.8.0
DOCKER_TAG := 1.8.0-r12.1-57fde9
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm like meh, if you can inspect the version and it's documented just bump semver?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So 1.9.0 or 2.0.0? I am not really sure how I should go about the versioning here. The variables are:

  1. cargo-contract
  2. ink-wrapper
  3. rust
  4. aleph-node version this particular image is compatible with.

Not every aleph-node change breaks the backwards compatibility, so the same ink-dev image can work with many deployments. Same with other packages here.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's not backward compatible, ie ink-dev won't work with aleph-node 11.X anymore, then 2.0.0 is a must

Copy link
Collaborator Author

@deuszx deuszx Nov 23, 2023

Choose a reason for hiding this comment

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

It's tricky b/c it will work and won't work at the same time. Since we added ink-wrapper to this image (and we hadn't initially) this adds another option to break backwards compatibility. For example, this particular image's cargoc-contract will work with 12 and 11 while its ink-wrapper won't.

I'll bump to 2.0.0 anyway.

I think it's more important to be clear about versions in the labels than correct numbering anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makefile Outdated
@@ -4,7 +4,7 @@

MAKEFILE_NAME := Ink development docker
DOCKER_NAME_INK_DEV := cardinal-cryptography/ink-dev
DOCKER_TAG := 1.8.0
DOCKER_TAG := 1.8.0-r12.1-57fde9
Copy link
Contributor

Choose a reason for hiding this comment

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

If it's not backward compatible, ie ink-dev won't work with aleph-node 11.X anymore, then 2.0.0 is a must

Dockerfile Show resolved Hide resolved
@deuszx deuszx merged commit a862d63 into main Nov 23, 2023
2 checks passed
@deuszx deuszx deleted the ink-wrapper-aleph-node-12.1 branch November 23, 2023 12:07
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