Skip to content

Add checkout.flags support#75

Open
petetomasik wants to merge 37 commits into
mainfrom
sup-6207-checkout-flags
Open

Add checkout.flags support#75
petetomasik wants to merge 37 commits into
mainfrom
sup-6207-checkout-flags

Conversation

@petetomasik
Copy link
Copy Markdown
Contributor

Summary

Adds checkout.flags support to CommandStep so pipeline YAML can specify per-step git flag overrides for clone, fetch, checkout, and clean.

Example:

steps:
  - command: build.sh
    checkout:
      flags:
        clone: "--depth 1"
        fetch: "--prune"
        checkout: ""       # explicit empty = remove flags
        clean: "-fdx"

Changes

  • New Checkout and CheckoutFlags types in step_command_checkout.go, following the existing Cache/Matrix/Secret pattern (inline RemainingFields, MarshalJSON via inlineFriendlyMarshalJSON).
  • New Checkout *Checkout field on CommandStep.
  • Interpolation hooks on both types, wired into CommandStep.interpolate outside the type switch so both env and matrix passes run (mirrors Secrets).
  • *string for the four flag values to distinguish unset (nil, omitted from output) from explicit empty ("", preserved as flag removal).
  • Test coverage for parsing shapes (subset, empty map, unknown keys, whitespace, special chars, YAML null), YAML and JSON round-trips, env and matrix interpolation, RemainingFields interpolation at both levels, long flag values, and nil-receiver safety on all three layers.

Test plan

  • go test ./... passes
  • go vet ./... clean
  • Existing pipelines without a checkout block produce no checkout key in YAML/JSON output (covered by TestCommandStepCheckoutOmittedWhenNil)

- Drop boilerplate doc comments on interpolate methods and tighten
  MarshalJSON comments to match step_command_matrix.go.
- Drop redundant "Standard caveats apply" line from Checkout doc;
  sibling sub-block types (Cache, Matrix) don't carry it.
- Tighten struct-tag whitespace to single-space separators.
- Extend TestCommandStepCheckoutYAMLRoundTrip with an actual
  yaml.Marshal assertion so the test name reflects what it checks.
- Add a "flags: {}" parsing case to pin down non-nil-empty Flags.
Lock down the current safe behavior so a future change to
ordered.Unmarshal null handling doesn't silently regress the
wire format: top-level "checkout: null" leaves Checkout nil,
nested "flags: null" leaves Flags nil under a non-nil Checkout.
Replace the grouped slice-form interface assertion with two single-type
blocks to match the Cache/Matrix/MatrixAdjustment convention, and
document that only the nested shape (flags: <map>) is recognized.
- Drop TestCommandStepCheckoutRoundTrip: subset of YAMLRoundTrip.
- Drop TestCommandStepCheckoutLongFlagValue: parse-only, no real
  yaml.v3 quirk verified by manual round-trip.
- Fix TestCommandStepCheckoutInterpolationNilSafety to use a real
  Matrix and non-empty MatrixPermutation so it actually reaches
  Checkout.interpolate instead of short-circuiting in
  InterpolateMatrixPermutation.
- Add TestCommandStepCheckoutEnvThenMatrixInterpolation covering the
  combined env-then-matrix pass on a single flag value.
Add UnmarshalOrdered to Checkout so scalar (bool/string/int) and
sequence values fail Parse with errUnsupportedCheckoutType instead
of silently degrading the surrounding step to UnknownStep (step
level) or producing an ordered-package error message (pipeline
level). Lock the contract in with parsing tests at both layers.
inlineFriendlyMarshalJSON reads only the yaml tag, and CommandStep.
UnmarshalJSON routes through ordered.Unmarshal which also reads only
yaml tags. The json tags on CheckoutFlags fields had no behavioral
effect. Matches Cache/Matrix/MatrixAdjustment.
Match the single-block form used in step_command_matrix.go where a
parent type and a closely related child type share a file.
The previous exact-string assertion tied the test to yaml.v3's
indent and quoting defaults, so a dep bump could fail the test
without indicating a real regression. Marshal, reparse, and diff
the resulting *Checkout, matching TestPipelineCheckoutYAMLRoundTrip.
Cover the interaction the PR is built around: pipeline.env values
interpolating into both checkout blocks, pipeline-level and step-
level checkout coexisting in the same YAML, and YAML round-trip
preserving both blocks plus the empty-string ("flag removal")
distinction.
Resolve schema conflict between the two branches' Checkout types by adding
Submodules to the Flags-based Checkout struct on this branch, dropping the
duplicate checkout.go from feat in favor of step_command_checkout.go, and
extending the existing tests with Submodules coverage. Combine the cache
interpolation pass from feat with this branch's checkout interpolation pass
in CommandStep.interpolate.
- Per-flag null (clone: null) leaves the pointer nil, distinct from
  explicit empty ("", flag removal).
- flags: <scalar> and flags: <sequence> are rejected, since
  CheckoutFlags has no UnmarshalOrdered and the behavior is left to
  generic ordered.Unmarshal today.
- Pipeline-level Submodules: parse cases for true/false/null and
  combined with flags, plus a YAML round-trip.
- Reword the Checkout type doc so it covers both Submodules (top-level)
  and Flags (nested) instead of reading as if the nested shape were the
  only one recognized.
- Replace the placeholder "is needed to use inlineFriendlyMarshalJSON"
  comments on both MarshalJSON helpers with the WHY phrasing used on
  Cache (the sibling that introduced inlineFriendlyMarshalJSON).
@petetomasik petetomasik marked this pull request as ready for review May 21, 2026 15:19
Copy link
Copy Markdown

@omehegan omehegan left a comment

Choose a reason for hiding this comment

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

I just note that you've refactored/removed checkout.go here, so some of the other open PRs (e.g. #73) will need a refactor before they can merge. Otherwise looks good.

Base automatically changed from feat/git-checkout-features to main May 26, 2026 08:11
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