Skip to content

feat: Implement Canister Build Adapter Framework and Script Adapter#10

Merged
rikonor merged 54 commits intomainfrom
or-adapters
Jun 16, 2025
Merged

feat: Implement Canister Build Adapter Framework and Script Adapter#10
rikonor merged 54 commits intomainfrom
or-adapters

Conversation

@rikonor
Copy link
Copy Markdown
Contributor

@rikonor rikonor commented Jun 4, 2025

Summary:

This PR introduces canister build adapters within the icp-adapter crate. It defines a common Adapter trait and provides an initial, fully functional implementation for a ScriptAdapter. This adapter allows canister builds to be performed by executing arbitrary shell commands. Placeholders for MotokoAdapter and RustAdapter are also included for future development.

Detailed Changes:

  • Build Adapter Framework (lib/icp-adapter/src/lib.rs):

    • Introduced an async Adapter trait with a compile method, serving as the core abstraction for different build mechanisms.
    • Established modules for script, motoko, and rust adapters.
    • Implemented a unified AdapterCompileError enum using snafu for consistent error handling across adapters.
  • Script Adapter (lib/icp-adapter/src/script.rs):

    • The ScriptAdapter allows defining canister build processes via one or more shell commands.
    • Commands are parsed using shellwords, enabling complex commands with quoted arguments.
    • The adapter executes commands within the canister's specified directory.
    • Standard input, output, and error streams are inherited from the executing process, making build logs directly visible.
    • Comprehensive error handling is in place for command parsing, invocation failures, and non-zero exit codes.
    • The ScriptAdapter configuration (e.g., the command(s) to run) is designed to be deserializable from a configuration file (like canister.yaml). For example:
      # canister.yaml (example snippet)
      build:
        adapter: script
        command: "npm run build"
      # or for multiple commands:
      # commands:
      #   - "npm install"
      #   - "npm run build"
  • Placeholder Adapters:

Future Work:

  • Implement the compilation logic for MotokoAdapter.
  • Implement the compilation logic for RustAdapter.
  • Integrate these adapters into the broader canister build lifecycle.

This PR description was generated by Roo.

rikonor added 30 commits May 29, 2025 12:17
@rikonor rikonor requested review from a user and adamspofford-dfinity June 6, 2025 16:25
Copy link
Copy Markdown
Contributor

@adamspofford-dfinity adamspofford-dfinity left a comment

Choose a reason for hiding this comment

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

If this PR is meant to fully implement the adapter, it should contain the build cache directory logic, and a test for a script canister that invokes cargo build.

Comment thread lib/icp-adapter/src/script.rs
Comment thread lib/icp-adapter/src/script.rs
@adamspofford-dfinity adamspofford-dfinity dismissed their stale review June 9, 2025 16:25

answer: no, it is not

@rikonor
Copy link
Copy Markdown
Contributor Author

rikonor commented Jun 9, 2025

If this PR is meant to fully implement the adapter, it should contain the build cache directory logic, and a test for a script canister that invokes cargo build.

Hi @adamspofford-dfinity, as discussed in the team meeting, it doesn't seem like we've reached a consensus on how those things would work yet. I think it'd be valid to proceed with reviewing this code regardless since the things you've brought up would be additions to this code, rather than replace it. What is here works and is needed, so I don't see a need to wait for the design discussions to conclude.

Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Let's start with a test or tests, since it looks like this is at the point where it's possible to add them and verify some functionality.

Comment thread lib/icp-adapter/src/lib.rs Outdated
Comment thread lib/icp-adapter/src/script.rs
Comment thread lib/icp-adapter/src/script.rs Outdated
Comment thread lib/icp-adapter/src/script.rs
@rikonor
Copy link
Copy Markdown
Contributor Author

rikonor commented Jun 12, 2025

Let's start with a test or tests, since it looks like this is at the point where it's possible to add them and verify some functionality.

@ericswanson-dfinity Tests have been added.

Comment thread bin/icp-cli/tests/adapter_script_tests.rs Outdated
Comment thread lib/icp-adapter/src/script.rs
Comment thread bin/icp-cli/tests/adapter_script_tests.rs Outdated
Comment thread .github/workflows/test.yml Outdated
Comment thread lib/icp-adapter/src/script.rs Outdated
Comment thread bin/icp-cli/src/commands/build.rs
@rikonor rikonor removed the wip Work in progress label Jun 13, 2025
@rikonor rikonor force-pushed the or-adapters branch 2 times, most recently from 4b2e483 to 2d66a9b Compare June 13, 2025 19:19
@rikonor rikonor merged commit 33b347c into main Jun 16, 2025
11 checks passed
@rikonor rikonor deleted the or-adapters branch June 16, 2025 14:33
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