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: remind to build before install #3446

Merged
merged 4 commits into from
Nov 20, 2023
Merged

fix: remind to build before install #3446

merged 4 commits into from
Nov 20, 2023

Conversation

lwshang
Copy link
Contributor

@lwshang lwshang commented Nov 16, 2023

Description

SDK-971

When installing a canister, if the canister is not built yet, a more informative message will be shown:

"The canister must be built before install. Please run `dfx build`."

The original request was to allow directly installing custom canisters which specify wasm path. However, for such custom canisters, dfx may do some necessary post-processing as a part of dfx build and install the processed wasm later. Directly installing the exact wasm specified in dfx.json will be an inconsistent behavior.

Also, from the source code perspective. The canister type specific information including whether the wasm field is specified are not available to the install_canister function. We'll have to expose too much specific information at this level to allow a special rule here.

Therefore, I improve the install_canister function by giving better error message in such case.
Before this change, users may got confused that dfx reports "Fail to read the wasm file" even though they specified it in dfx.json. The new error message is more helpful that it provides the command to fix the issue.

How Has This Been Tested?

e2e

Checklist:

  • The title of this PR complies with Conventional Commits.
  • I have edited the CHANGELOG accordingly.
  • I have made corresponding changes to the documentation.

@lwshang lwshang marked this pull request as ready for review November 17, 2023 15:14
@lwshang lwshang requested review from chenyan-dfinity and a team as code owners November 17, 2023 15:14
@lwshang lwshang requested a review from sesi200 November 17, 2023 15:16
@lwshang lwshang enabled auto-merge (squash) November 20, 2023 14:59
@lwshang lwshang merged commit 4c712a4 into master Nov 20, 2023
167 checks passed
@lwshang lwshang deleted the lwshang/SDK-971 branch November 20, 2023 15:41
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.

2 participants