Skip to content

fix(xtest): make otdf-sdk-mgr check out source refs where make builds them - #681

Draft
dmihalcik-virtru wants to merge 2 commits into
mainfrom
DSPX-4372-sdkmgr-checkout-slug
Draft

dmihalcik-virtru wants to merge 2 commits into
mainfrom
DSPX-4372-sdkmgr-checkout-slug

Conversation

@dmihalcik-virtru

Copy link
Copy Markdown
Member

What

otdf-sdk-mgr install tip checks a ref out under xtest/sdk/<lang>/src/ and then runs make with VERSIONS=<slug>; the SDK Makefiles build src/$VERSIONS into dist/$VERSIONS. cmd_tip computes the slug by flattening / to -- and stripping both sdk-- and otdfctl--, but the checkouts named the worktree differently:

  • checkout_sdk_branch (js, java) stripped only sdk--, so otdfctl/<x> went to src/otdfctl--<x> while make built src/<x>;
  • checkout_go_from_platform stripped only otdfctl--, so sdk/v1.2.3 went to platform-src/sdk--v1.2.3 (and src/sdk--v1.2.3) while make built src/v1.2.3.

Both checkouts now name the worktree with one private helper, checkout._worktree_name, using the same expression as cmd_tip.

Why it matters

When src/<slug> didn't exist, make failed with an error that never mentioned the mismatch. When an earlier checkout had left a src/<slug>, make silently built that stale tree into the dist/ directory the caller expected, so tests ran against the wrong source.

User-visible effect

  • dist/ names are unchanged.
  • src/ worktree names change only for the two combinations that were broken: go with sdk/... refs (now src/<x> and platform-src/<x>), and js/java with otdfctl/... refs (now src/<x>). Existing sdk--* / otdfctl--* directories from earlier checkouts are no longer used; otdf-sdk-mgr clean --src-only removes them.

Validation

Follow-up

Found in review of #668, which adds refs.dist_slug with this same expression and switches cmd_tip to it. Once both land, both checkout call sites switch to refs.dist_slug and _worktree_name goes away. This PR keeps the change local to checkout.py so the two don't conflict. Recorded in the DSPX-4372-t1 plan.

… them

`install tip` passes make VERSIONS=<slug>, and the SDK Makefiles build
src/$VERSIONS into dist/$VERSIONS. cmd_tip strips both the `sdk--` and
`otdfctl--` prefixes from the flattened ref, but checkout_sdk_branch
(js, java) stripped only `sdk--` and checkout_go_from_platform only
`otdfctl--`. So go `sdk/v1.2.3` was checked out into src/sdk--v1.2.3
and js or java `otdfctl/<x>` into src/otdfctl--<x>, while make built
src/v1.2.3 and src/<x>. With nothing there, make failed with an error
that didn't mention the mismatch; with a tree an earlier checkout left
there, it built that stale tree into the dist/ directory the caller
expected.

Both checkouts now name the worktree with _worktree_name, the same
expression cmd_tip uses. dist/ names don't change, and src/ names
change only for the two broken combinations. For go that moves the
platform-src/ worktree too, and the src/ symlink follows it.

The new test runs cmd_tip for go, java and js with git and make
stubbed at the subprocess layer, for main, feature/x, sdk/v1.2.3,
otdfctl/v0.24.0, refs/pull/123/head and a full SHA. It checks that
src/ holds exactly src/$VERSIONS and, for go, that the link resolves
into platform-src/$VERSIONS. Without the fix, go-sdk/v1.2.3,
java-otdfctl/v0.24.0 and js-otdfctl/v0.24.0 fail.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
@coderabbitai

coderabbitai Bot commented Oct 26, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sonarqubecloud

Copy link
Copy Markdown

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.

1 participant