Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ Follow the repository's automated style configuration in
- Keep changes consistent with existing Polaris patterns.
- For Python, follow the path-specific instructions in
`.github/instructions/python.instructions.md`.
- Use the repository's Pixi environment before considering any other Python
environment. Prefer `pixi run -e py314 <command>` for `python`, `pytest`,
`pre-commit`, `ruff`, and `mypy`, and check `.pixi/` plus `pixi.toml`
before concluding a tool is missing.
- pre-commit on changed files is required before finishing; if sandboxed
execution fails, request escalation and do not close the task until it has
run or the user declines.
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/copilot-setup-steps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,6 @@ jobs:
- name: Verify the agent environment
run: |
pixi run -e py314 python --version
pixi run -e py314 pytest --version
pixi run -e py314 pre-commit --version
pixi run -e py314 mache --help
17 changes: 17 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,18 @@ These instructions apply to the whole repository unless a deeper
- If an instruction here conflicts with automated tooling, follow the
automated tooling.

## Python environment

- This repository uses Pixi as the primary development environment manager.
- Check the root `.pixi/` and `pixi.toml` before creating or selecting any
other Python environment.
- Prefer `pixi run -e py314 <command>` for Python tools such as `python`,
`pytest`, `pre-commit`, `ruff`, and `mypy`.
- If you need an interactive shell, use `pixi shell -e py314` from the repo
root instead of creating a new virtual environment.
- Do not treat `pytest: command not found` in a plain shell as a missing
dependency until you have tried the command through Pixi.

## Python style

- Keep Python lines at 79 characters or fewer whenever possible.
Expand All @@ -28,7 +40,12 @@ These instructions apply to the whole repository unless a deeper

## Validation

- Run tests and linting through Pixi unless the task explicitly requires a
different environment.
- Prefer `pixi run -e py314 pytest` for tests.
- pre-commit on changed files is required before finishing; if sandboxed
execution fails, request escalation and do not close the task until it has
run or the user declines.
- Prefer `pixi run -e py314 pre-commit run --files ...` for required
validation.
- Prefer fixing lint and formatting issues rather than suppressing them.
46 changes: 33 additions & 13 deletions docs/developers_guide/config_machines_updates.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ The update path is:
2. The workflow creates or refreshes a GitHub issue.
3. Copilot is assigned to that issue.
4. Copilot opens a pull request against `main`.
5. That PR updates `mache/cime_machine_config/config_machines.xml` first, then
any related Spack templates or version strings that the report indicates
should be reviewed.
5. That PR replaces `mache/cime_machine_config/config_machines.xml` with the
latest version from `E3SM-Project/E3SM`, then updates any related Spack
templates or version strings that the report indicates should be reviewed.
6. A maintainer reviews and merges the PR.
7. The next daily run compares the merged repository state against upstream
again.
Expand All @@ -112,8 +112,16 @@ Copilot receives instructions from two places.
`agent_assignment` payload:

- Use the issue body as the task definition.
- Update `config_machines.xml` first.
- Then update related Spack templates and version strings.
- Run `pixi run -e py314 python utils/update_cime_machine_config.py
--work-dir .`.
- Replace `mache/cime_machine_config/config_machines.xml` with the generated
`upstream_config_machines.xml`, then remove that temporary file before
committing.
- State the upstream E3SM commit hash in the PR summary.
- Then update related Spack templates and version strings under
`mache/spack/templates/<machine>*.yaml`,
`mache/spack/templates/<machine>*.sh`, and
`mache/spack/templates/<machine>*.csh`.
- Add TODO comments in the PR when prefix or path changes need reviewer
confirmation.

Expand All @@ -123,6 +131,7 @@ Copilot receives instructions from two places.
drift and includes:

- the timestamp and upstream source URL,
- the upstream revision when it could be resolved from GitHub,
- the workflow run URL,
- the list of affected supported machines,
- the required work list,
Expand All @@ -131,9 +140,16 @@ drift and includes:

The required work section tells Copilot to:

- update `mache/cime_machine_config/config_machines.xml` for the affected
supported machines,
- update Spack templates and version strings when module or environment drift
- run `pixi run -e py314 python utils/update_cime_machine_config.py
--work-dir .`,
- replace `mache/cime_machine_config/config_machines.xml` with the generated
`upstream_config_machines.xml`,
- remove `upstream_config_machines.xml` before committing,
- state the upstream E3SM commit hash in the PR summary,
- update Spack templates and version strings in
`mache/spack/templates/<machine>*.yaml`,
`mache/spack/templates/<machine>*.sh`, and
`mache/spack/templates/<machine>*.csh` when module or environment drift
implies different package versions,
- keep the PR focused when the change is only version or module drift,
- add a TODO in the PR instead of guessing when a new prefix or path is not
Expand Down Expand Up @@ -173,13 +189,17 @@ in this order.

### 1. `config_machines.xml` changes

Verify that the PR updates
`mache/cime_machine_config/config_machines.xml` only for supported machines
reported by the workflow, and that those changes match the current upstream
E3SM machine definitions.
Verify that the PR replaces
`mache/cime_machine_config/config_machines.xml` with the current upstream file
from `E3SM-Project/E3SM`, rather than hand-editing only selected machine
blocks.

The supported-machine report from the workflow tells you which machine entries
caused the drift and which sections deserve the closest review.

In practice, the easiest cross-check is to compare the PR against the report
artifact from the workflow run that opened or refreshed the issue.
artifact and the upstream XML source from the workflow run that opened or
refreshed the issue.

### 2. Related Spack updates

Expand Down
33 changes: 30 additions & 3 deletions mache/cime_machine_config/report.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ def has_updates(self) -> bool:
class UpdateReport:
generated_at: str
upstream_url: str
upstream_revision: str | None
supported_machines: list[str]
machines: list[MachineUpdate]

Expand All @@ -64,7 +65,13 @@ def to_dict(self) -> dict[str, object]:
return data


def build_update_report(old_xml, new_xml, supported_machines, upstream_url):
def build_update_report(
old_xml,
new_xml,
supported_machines,
upstream_url,
upstream_revision=None,
):
"""Build a structured report for supported machine config changes."""

old_root = etree.parse(old_xml).getroot()
Expand All @@ -87,6 +94,7 @@ def build_update_report(old_xml, new_xml, supported_machines, upstream_url):
return UpdateReport(
generated_at=datetime.now(timezone.utc).isoformat(),
upstream_url=upstream_url,
upstream_revision=upstream_revision,
supported_machines=list(supported_machines),
machines=machines,
)
Expand All @@ -113,6 +121,9 @@ def render_update_issue(report, run_url=None):
f'- Upstream source: {report.upstream_url}',
]

if report.upstream_revision is not None:
lines.append(f'- Upstream revision: {report.upstream_revision}')

if run_url:
lines.append(f'- Workflow run: {run_url}')

Expand All @@ -121,14 +132,24 @@ def render_update_issue(report, run_url=None):
'',
'Required work:',
'',
'- Update mache/cime_machine_config/config_machines.xml for the',
' affected supported machines.',
'- Run `pixi run -e py314 python '
'utils/update_cime_machine_config.py --work-dir .` from the',
' repository root.',
'- Replace mache/cime_machine_config/config_machines.xml with',
' upstream_config_machines.xml generated by that command, then',
' remove upstream_config_machines.xml before committing.',
'- If module or environment changes imply different',
' system-package',
' versions, update the corresponding mache Spack templates and',
' version strings for the affected toolchains.',
'- Look for related Spack template files under',
' mache/spack/templates/<machine>*.yaml,',
' mache/spack/templates/<machine>*.sh, and',
' mache/spack/templates/<machine>*.csh.',
'- If the only follow-up is module or version drift, keep the PR',
' focused on those updates.',
'- In the PR summary, state which upstream E3SM commit hash was',
' used for the config_machines.xml replacement.',
'- If any prefix or path values changed and the correct',
' replacement',
' is not obvious, add a TODO comment in the PR for the reviewer',
Expand Down Expand Up @@ -350,6 +371,12 @@ def _render_machine_section(machine):
'- Spack templates to review: '
f'{", ".join(machine.spack_templates_to_review)}'
)
lines.append(
'- Spack template globs: '
f'mache/spack/templates/{machine.machine}*.yaml, '
f'mache/spack/templates/{machine.machine}*.sh, '
f'mache/spack/templates/{machine.machine}*.csh'
)
else:
lines.append('- Spack templates to review: none matched')

Expand Down
12 changes: 11 additions & 1 deletion tests/test_cime_machine_config_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ def test_build_update_report_detects_spack_related_drift(
new_xml=new_xml,
supported_machines=['test-machine'],
upstream_url='https://example.invalid/config_machines.xml',
upstream_revision='deadbeef1234',
)

assert report.has_updates is True
Expand Down Expand Up @@ -97,13 +98,22 @@ def test_render_update_issue_includes_required_instructions(
new_xml=new_xml,
supported_machines=['test-machine'],
upstream_url='https://example.invalid/config_machines.xml',
upstream_revision='deadbeef1234',
)
markdown = render_update_issue(
report,
run_url='https://github.example/actions/runs/1',
)

assert 'Update mache/cime_machine_config/config_machines.xml' in markdown
assert (
'pixi run -e py314 python utils/update_cime_machine_config.py'
in markdown
)
assert 'upstream_config_machines.xml' in markdown
assert 'deadbeef1234' in markdown
assert 'state which upstream E3SM commit hash was' in markdown
assert 'mache/spack/templates/<machine>*.yaml' in markdown
assert 'mache/spack/templates/test-machine*.sh' in markdown
assert 'TODO comment in the PR for the reviewer' in markdown
assert 'test-machine_gnu_mpich.yaml' in markdown
assert 'https://github.example/actions/runs/1' in markdown
69 changes: 69 additions & 0 deletions tests/test_update_cime_machine_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
from importlib.util import module_from_spec, spec_from_file_location
from pathlib import Path


def _load_update_module():
module_path = (
Path(__file__).resolve().parents[1]
/ 'utils'
/ 'update_cime_machine_config.py'
)
spec = spec_from_file_location('update_cime_machine_config', module_path)
assert spec is not None
assert spec.loader is not None

module = module_from_spec(spec)
spec.loader.exec_module(module)
return module


def test_parse_github_raw_url_supports_default_e3sm_url():
update_module = _load_update_module()

source = update_module._parse_github_raw_url(
'https://raw.githubusercontent.com/E3SM-Project/E3SM/'
'refs/heads/master/cime_config/machines/config_machines.xml'
)

assert source == (
'E3SM-Project',
'E3SM',
'master',
'cime_config/machines/config_machines.xml',
)


def test_get_latest_commit_sha_uses_github_commits_api(monkeypatch):
update_module = _load_update_module()
captured = {}

class FakeResponse:
def raise_for_status(self):
return None

def json(self):
return [{'sha': 'abc123'}]

def fake_get(url, *, params, timeout):
captured['url'] = url
captured['params'] = params
captured['timeout'] = timeout
return FakeResponse()

monkeypatch.setattr(update_module.requests, 'get', fake_get)

sha = update_module._get_latest_commit_sha(
owner='E3SM-Project',
repo='E3SM',
ref='master',
path='cime_config/machines/config_machines.xml',
)

assert sha == 'abc123'
assert captured['url'].endswith('/repos/E3SM-Project/E3SM/commits')
assert captured['params'] == {
'sha': 'master',
'path': 'cime_config/machines/config_machines.xml',
'per_page': 1,
}
assert captured['timeout'] == 60
19 changes: 11 additions & 8 deletions utils/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,17 @@ mache package.
## update CIME machine config

The `update_cime_machine_config.py` script can be used to download the latest
version of the `config_machines.xml` file from E3SM's `master` branch, then
compare it from the previous version stored in `mache` and show changes related
to supported machines.

A developer can then copy `new_config_machines.xml` into
`mache/cime_machine_config/config_machines.xml` as part of a PR that makes
relevant updates. They should also make the changes associated with the
differences that this utility displays in the appropriate `mache/spack/templates` files.
version of the `config_machines.xml` file from `E3SM-Project/E3SM`, compare it
with the copy stored in `mache`, and show changes related to supported
machines.

When drift is detected, the follow-up PR should replace
`mache/cime_machine_config/config_machines.xml` with the latest upstream file
from `E3SM-Project/E3SM`. This utility helps confirm the drift and review the
affected supported machines; it does not rewrite the repository copy in place.

The PR should also make the changes associated with the differences that this
utility displays in the appropriate `mache/spack/templates` files.

## extract spack shell scripts from CIME machine config

Expand Down
17 changes: 13 additions & 4 deletions utils/manage_cime_machine_config_issue.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,19 @@ def build_issue_payload(
'target_repo': f'{owner}/{repo}',
'base_branch': base_branch,
'custom_instructions': (
'Use the issue body as the task definition. Update '
'config_machines.xml first, then the related Spack '
'templates and version strings. Add TODO comments in the '
'PR when prefix or path changes need reviewer confirmation.'
'Use the issue body as the task definition. Run `pixi run '
'-e py314 python utils/update_cime_machine_config.py '
'--work-dir .`, replace '
'mache/cime_machine_config/config_machines.xml with '
'upstream_config_machines.xml, remove '
'upstream_config_machines.xml before committing, and state '
'the upstream E3SM commit hash in the PR summary. Then '
'update the related Spack templates and version strings in '
'mache/spack/templates/<machine>*.yaml, '
'mache/spack/templates/<machine>*.sh, and '
'mache/spack/templates/<machine>*.csh. '
'Add TODO comments in the PR when prefix or path changes '
'need reviewer confirmation.'
),
'custom_agent': '',
'model': '',
Expand Down
Loading
Loading