Skip to content

Remove stale cchost mentions across tests, samples, schema, and host config#7883

Open
Copilot wants to merge 7 commits into
mainfrom
copilot/remove-cchost-mentions
Open

Remove stale cchost mentions across tests, samples, schema, and host config#7883
Copilot wants to merge 7 commits into
mainfrom
copilot/remove-cchost-mentions

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 13, 2026

The cchost binary was removed in 7.x (each app is now its own executable), but stale references remained throughout the tree.

Tests / infra (Python)

  • Replace # Set to cchost default value comments with # Set to node default value in tests/infra/{runner,piccolo_driver}.py, tests/historical_query_{perf,cache}.py, tests/lts_compatibility.py.
  • Update --binary-dir help text in tests/infra/e2e_args.py.
  • Remove the CCFRemote.BIN = "" class attribute and instead require the binary name on construction: added a binary_name keyword argument to CCFRemote.__init__. For major_version >= 7 (or unspecified), the binary is enclave_file; for major_version < 7, binary_name is asserted to be set and resolved via build_bin_path. infra.node.Node passes binary_name="cchost" for pre-7 nodes.

Host config schema

  • Rename doc/host_config_schema/cchost_config.jsonhost_config.json; update CMakeLists.txt and doc/conf.py.
  • Rename C++ struct host::CCHostConfighost::HostConfig (only used in src/host/configuration.h and src/host/run.cpp).

samples/minimal_ccf

  • Drop the obsolete mv .../bin/cchost /usr/bin/cchost step from the runtime Dockerfile and replace it with a mv .../bin/js_generic /usr/bin/js_generic step, with a comment noting js_generic is a sample application binary intended to be replaced with the actual application binary when adapting the image.
  • Replace the cchost invocation in run_app.sh with js_generic.
  • Rename cchost_config_virtual_js.jsonconfig_virtual_js.json.

Intentionally retained

  • CHANGELOG.md historical entries.
  • The inline comment in tests/infra/remote.py __init__ explaining the 7.x binary consolidation (kept per review feedback).

Comment thread tests/infra/remote.py Outdated
Agent-Logs-Url: https://github.com/microsoft/CCF/sessions/16102018-130f-491a-87f7-26ad84e003bf

Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>
Copilot AI requested a review from maxtropets May 13, 2026 16:29
@maxtropets maxtropets marked this pull request as ready for review May 13, 2026 17:59
@maxtropets maxtropets requested a review from a team as a code owner May 13, 2026 17:59
Copilot AI review requested due to automatic review settings May 13, 2026 17:59
Comment thread samples/minimal_ccf/run_app.sh
Comment thread tests/infra/remote.py
Agent-Logs-Url: https://github.com/microsoft/CCF/sessions/944eaa0f-7d80-4808-b871-e5a88fd00c0e

Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>
Copilot AI requested a review from maxtropets May 13, 2026 18:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes stale references to the deprecated cchost entrypoint across the repo (tests/infra, samples, and the host config schema/docs), reflecting the 7.x+ model where each app is its own executable. It also renames the host config schema file and the corresponding C++ config struct to remove cchost-specific naming.

Changes:

  • Update Python test/infra comments and CLI help to refer to “node” rather than cchost, and adjust CCFRemote comments/defaults.
  • Rename host config schema to host_config.json and update build/docs wiring; rename host::CCHostConfig to host::HostConfig.
  • Update samples/minimal_ccf scripts/config naming and remove a Dockerfile step that staged /usr/bin/cchost.

Custom instructions used:

  • .github/copilot-instructions.md
  • .github/instructions/reviewing.instructions.md

Reviewed changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/lts_compatibility.py Update comment referring to node defaults.
tests/infra/runner.py Update comments referring to node defaults.
tests/infra/remote.py Adjust CCFRemote.BIN default/comment and inline note for 7.x+.
tests/infra/piccolo_driver.py Update comment referring to node defaults.
tests/infra/e2e_args.py Update --binary-dir help text to refer to node executable.
tests/historical_query_perf.py Update comment referring to node defaults.
tests/historical_query_cache.py Update comment referring to node defaults.
src/host/run.cpp Rename config type usage from CCHostConfig to HostConfig.
src/host/configuration.h Rename host::CCHostConfig struct to host::HostConfig and JSON declarations accordingly.
samples/minimal_ccf/run_app.sh Switch sample invocation from cchost to js_generic and rename config reference.
samples/minimal_ccf/ccf_runtime/Dockerfile Remove step that moved cchost into /usr/bin.
samples/minimal_ccf/app/config_virtual_js.json Add/rename sample config JSON for minimal sample.
doc/host_config_schema/host_config.json Add/rename host config JSON schema under new name.
doc/conf.py Point schema-based doc generation to host_config.json.
CMakeLists.txt Point embedded/installed schema source to host_config.json.

Comment thread tests/infra/remote.py Outdated
Comment thread samples/minimal_ccf/run_app.sh
Comment thread samples/minimal_ccf/ccf_runtime/Dockerfile Outdated
Copilot AI requested a review from achamayou May 13, 2026 19:30
Copy link
Copy Markdown
Member

@achamayou achamayou left a comment

Choose a reason for hiding this comment

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

Thank you for tidying this up, there are a couple of niggles but this is going in the right direction.

Comment thread tests/infra/remote.py Outdated
@maxtropets
Copy link
Copy Markdown
Collaborator

@copilot

Run set -ex

  • git config --global --add safe.directory /__w/CCF/CCF
  • ./scripts/ci-checks.sh
    Includes
    Release notes
    TODOs
    Copyright notice headers
    Python lint
    CMake format
    Shell scripts
    C/C++ format
    Python format
    OpenAPI
    TypeScript, JavaScript, Markdown, TypeSpec, YAML and JSON format
    Python types
    Timing
    Summary
    The following checks failed: CMake format

Run formatting script in the way we do in CI, fix this PR

Create a separate PR with your review instructions or skils edited in such a way thay you never forget doing this again (I'm angry)

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 14, 2026

@maxtropets I've opened a new pull request, #7884, to work on those changes. Once the pull request is ready, I'll request review from you.

@maxtropets
Copy link
Copy Markdown
Collaborator

@maxtropets I've opened a new pull request, #7884, to work on those changes. Once the pull request is ready, I'll request review from you.

That PR is against main now , as it should be. Formatting to be fixed in this PR, as per CI.yml

@copilot

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 14, 2026

@maxtropets I've opened a new pull request, #7886, to work on those changes. Once the pull request is ready, I'll request review from you.

@maxtropets
Copy link
Copy Markdown
Collaborator

@copilot do not wait for any other PRs, they are not blocking this. Fix formatting in this PR by looking into ci.yml to see how it's done.

Agent-Logs-Url: https://github.com/microsoft/CCF/sessions/3276533c-eba0-4d87-ab5d-d9a7791c9a5c

Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 14, 2026

@copilot do not wait for any other PRs, they are not blocking this. Fix formatting in this PR by looking into ci.yml to see how it's done.

Fixed in 379f54a — ran scripts/cmake-format-checks.sh -f (the same gersemi check as CI) which reformatted the file(READ ...) call in CMakeLists.txt (now fits on one line after the cchost_config.jsonhost_config.json rename). scripts/cmake-format-checks.sh now passes cleanly.

Copilot AI requested a review from maxtropets May 14, 2026 13:23
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.

4 participants