You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Fetch commit from origin when not in local repo for ddev port-commit (DataDog#23703)
* Resolve ddev port-commit input by fetching from origin when not local
When the commit passed to `ddev release port-commit` is not in the local
object database, fall back to `git fetch origin <sha>` before aborting.
GitHub allows fetching any reachable commit by SHA, so this lets us port
commits that live on branches the local `remote.origin.fetch` refspec
does not track (a common configuration in this repo).
The fetch is precise — we never need to know which branch the commit is
on, since cherry-pick only needs the commit object.
* Add changelog entry for DataDog#23703
* Abort early on abbreviated SHAs and clarify test names
Address review feedback:
- Abbreviated SHAs cannot be fetched from GitHub via uploadpack
(allowReachableSHA1InWant only honours full 40-char SHAs), so detect
the abbreviated-hex case before attempting the fetch and abort with a
clear "pass the full 40-character SHA" message. Avoids the confusing
`fatal: couldn't find remote ref` stderr from git on a doomed fetch.
- Rename `test_command_aborts_when_commit_does_not_exist` to
`test_command_aborts_when_commit_missing_after_fetch`. The original
name suggested it covered the "commit truly missing" case, but with
the mock setup it actually exercises the edge where the fetch
succeeds but rev-parse still fails. The new name reflects that.
- Add `test_command_aborts_on_abbreviated_sha_not_local` for the new
early-abort path.
- Use a full 40-char SHA constant for the fetch-fallback tests so they
reach the fetch path under the new early-abort check.
* Use contextlib.suppress instead of except: pass
* Annotate Application.abort as NoReturn
The original `app.abort` calls `ctx.exit` (which raises `SystemExit`),
so the method never returns. Mark it with `-> NoReturn` so callers
don't need a trailing `raise AssertionError('unreachable')` to satisfy
mypy when the abort is the last statement on a code path. Also add an
explicit `raise SystemExit(code)` so mypy can see the no-return at the
implementation level (the `__exit_func` field is untyped) and as a
safety net if a buggy `exit_func` is ever passed in.
Removes the two `raise AssertionError('unreachable')` lines from
`_resolve_commit_or_fetch` that were only there for type checking.
* Type Application.exit_func and abort so callers don't need a trailing raise
Annotate the `exit_func` callback as `Callable[[int], NoReturn]`
(matching click's `Context.exit`) and mark `Application.abort` as
`-> NoReturn`. Mypy now infers that calling `self.__exit_func(code)`
doesn't return, so the abort path is correctly recognised as terminal
without needing an unreachable `raise SystemExit(code)` inside the
method body.
Typing the `__init__` body surfaced two pre-existing issues:
- `self.__config = {}` needed an explicit `dict[str, Any]` annotation.
- `serve_openmetrics_payload.py` was constructing `DockerAgent` with
`app.platform` (a `Platform`) where it expects an `Application`.
This has been broken since 2024 — `AgentInterface.platform` accesses
`self.app.platform`, which would raise `AttributeError` on a Platform
instance at runtime. Pass `app` directly.
Typing-only imports (`Any`, `Callable`, `NoReturn`) live behind
`TYPE_CHECKING` since the file already uses `from __future__ import
annotations`.
* Skip fetch fallback in --dry-run mode
Preserves the dry-run contract: when the commit is not in the local
object database, abort with a clear message instead of running
`git fetch origin <sha>` during plan resolution. Tells the user to
re-run without --dry-run or pre-fetch the commit manually.
Adds `test_command_aborts_in_dry_run_when_commit_not_local` covering
the new path and asserting no fetch is attempted.
* Add PR-or-commit input support to ddev port-commit
The command now accepts a PR number, an explicit `PR-<number>` token, or
a GitHub PR URL in addition to a commit SHA. Detection is tiered:
- `PR-<digits>` or a PR URL is treated as an explicit PR reference.
- Pure-digit inputs are tried as a PR first when a GitHub token is
configured; a 404 falls back to commit resolution.
- Anything else goes straight to commit resolution.
When the input resolves to a PR, the command verifies the PR is merged
and that its merge commit has a single parent (i.e. the PR was
squash-merged); anything else aborts with a clear message instructing
the user to pass the specific commit they want to backport.
Refactors `_resolve_commit_or_fetch` to raise `_CommitNotResolvable`
instead of aborting, so the new orchestrator can wrap the error with a
unified "PR or commit" message when pure-digit input also fails commit
resolution. The single abort happens at the call site in
`resolve_port_plan`.
Adds:
- `AsyncGitHubClient.get_pull_request` for the GitHub lookup.
- `FakeAsyncGitHubClient.get_pull_request` plus a built-in 404 default
so tests that don't care about PR lookup auto-fall-through.
- New regex constants and helpers.
- `[COMMIT_OR_PR]` metavar and updated docstring on the click command.
* Add tests and changelog entry for PR-or-commit input
* Address round-1 review feedback for PR-or-commit input
Apply reviewer findings on top of the PR-or-commit input change:
- Wrap `_resolve_commit_or_fetch` inside `_resolve_pr_to_commit` so a
non-resolvable merge commit reports `PR #N ...` instead of leaking the
raw SHA the user never typed.
- Handle `pydantic.ValidationError` from `get_pull_request`, and split
401 / 403 out of the generic HTTP error arm with an actionable hint
about `github.token`.
- Move the abbreviated-SHA check ahead of the dry-run gate in
`_resolve_commit_or_fetch` so abbreviated input gives the real
diagnosis on the first run instead of forcing a retry.
- Guard the `rev-list --parents` call in `_abort_if_merge_commit`
against `OSError`, matching the rest of the module's careful style.
- Tighten the token-required abort for explicit PR inputs to flag the
`--no-pr` carve-out (the lookup is needed regardless of PR creation).
- Drop `_fetch_pr`'s dependency on the full `Application`; only the
token is needed.
- Annotate `Application.abort` parameters (`text: str`, `code: int`,
`**kwargs: Any`) to finish the typing surface.
- Strip the raw input in `_resolve_input` so trailing whitespace from a
copy-paste no longer drops the user to commit resolution.
- Drop `[ ]` from the click metavar; click adds them for optional
arguments and they were rendering doubled in --help.
- Replace the test helper's `lambda` exit function with a real
`NoReturn` callable so it matches the new `Application.__init__`
signature. Also corrects the pre-existing `Applicatione` typo.
- Add `http://`, fragment, and trailing-whitespace variants to the
parametrized PR-input test.
- Remove an em-dash from a test comment.
* Address round-2 review feedback
- Convert `_abort_if_merge_commit`'s try/except to try/else so the
parent-count binding is structurally explicit rather than relying on
`Application.__exit_func` being `NoReturn` at runtime. The typing
contract still holds, but a future caller wiring a non-terminating
exit function would now surface as a structural error instead of an
`UnboundLocalError`.
- Add `test_command_emits_pr_context_when_pure_digit_pr_merge_commit_missing`
pinning the path the round-1 f-1 fix was filed for: pure-digit input
+ PR found + non-resolvable merge commit should abort with a message
that names `PR #N`, not the raw SHA the user never typed.
* Drop redundant negative assertion in PR-context regression test
The positive assertion already validates that the PR-context message
fires. The negative assertion was meant to guard against the raw
commit-resolution message firing alone, but that message is in fact
suffixed onto the PR-context wrap, so any `not in` / `startswith` form
either passed trivially (Rich console wrapping) or failed for the
wrong reason. Drop it.
* Consolidate the PR's changelog into a single entry
* Quiet httpx and show a progress line during PR resolution
The httpx logger emits an INFO line for every request, which leaks
through to the CLI as `INFO: HTTP Request: GET ... "HTTP/1.1 200 OK"`
just before our own status messages. Drop httpx to WARNING at command
entry so only failures are surfaced.
Also print `Resolving PR #N via GitHub...` before the lookup so the
user sees something happen when the command pauses for the API call,
matching the existing "Commit X not found locally; fetching from
origin" pattern.
* Force `git add` when resolving `.in-toto/` conflicts
`.in-toto/` is gitignored in this repo (and most repos that ship signed
packages), so `git add <path>` refuses to stage the file even though
the path is already tracked in HEAD. `_resolve_in_toto_conflict` gets
here only for paths git itself flagged as `--diff-filter=U`, so forcing
the add is safe and matches the rest of the function's "keep target
branch's .in-toto" intent.
Without this, cherry-picking any commit that touches a `.in-toto/`
file (i.e. every release commit) aborts mid-workflow with `Command
'['git', 'add', '.in-toto/...']' returned non-zero exit status 1`.
Accept a PR number, ``PR-<number>`` token, or GitHub PR URL as input to ``port-commit``, and fetch the target commit from origin when it is not in the local object database.
0 commit comments