Skip to content

Commit

Permalink
Move idls onto the main repo, rather than an unknown SHA, and sync wi…
Browse files Browse the repository at this point in the history
…th go module (cadence-workflow#6241)

`idls/` and the `github.com/uber/cadence-idl` version in `go.mod` are currently out of sync and more than a little screwed up:
- the go module is on `0482c040f91d17be35cccee2bc1cf883f7344992`
- the IDL submodule is on `1cd936eb8e24d42f8d8cd38d6216e619c14c48d5`

The go module is currently on cadence-workflow/cadence-idl@0482c04
which is the latest `master` and seems fine.

The current IDL submodule: https://github.com/uber/cadence-idl/tree/1cd936eb8e24d42f8d8cd38d6216e619c14c48d5
doesn't belong to any branch (much less `master`), and is in danger of being GC'd if we don't do something about that.

It's probably from a temporary PR branch that was cleaned up as part of cadence-workflow#6178 (the contents are similar to https://github.com/uber/cadence-idl/tree/d92bb53292d64b698c0c5600bbd6e258e728a020 which was approved and _is_ on `master`), but I can't find `1cd936eb8e24d42f8d8cd38d6216e619c14c48d5` in any PR in either https://github.com/uber/cadence-idl or https://github.com/timl3136/cadence-idl so I'm really not sure.

So this PR contains a few changes:
- moves `idls/` to match `go.mod` SHA: `0482c040f91d17be35cccee2bc1cf883f7344992`
  - `d92bb53292d64b698c0c5600bbd6e258e728a020` did not build, as the service code for `ListAll...` was removed, which is also why I didn't just revert the related commits.
- adds a `make .idl-status` and CI steps to verify that this all stays in sync in the future, because this is all VERY easy to miss in PR reviews, and github's UI just makes it worse by hiding many of these details.
  • Loading branch information
Groxx authored Aug 20, 2024
1 parent 67fcf12 commit 3e3b40c
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 1 deletion.
12 changes: 12 additions & 0 deletions .buildkite/pipeline-master.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,18 @@ agents:

steps:

- label: ":on: IDL submodule points to master"
plugins:
- kubernetes:
<<: *kubernetes
podSpec:
<<: *podSpec
containers:
- <<: *commandContainer
command:
- |-
make .idl-status
- label: ":golang: unit test"
artifact_paths:
- ".build/coverage/*.out"
Expand Down
12 changes: 12 additions & 0 deletions .buildkite/pipeline-pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,18 @@ agents:

steps:

- label: ":on: IDL submodule points to master"
plugins:
- kubernetes:
<<: *kubernetes
podSpec:
<<: *podSpec
containers:
- <<: *commandContainer
command:
- |-
make .idl-status
- label: ":golang: unit test"
artifact_paths:
- ".build/coverage/*.out"
Expand Down
24 changes: 24 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,30 @@ $(STABLE_BIN)/$(PROTOC_VERSION_BIN): | $(STABLE_BIN)
$Q unzip -q $(STABLE_BIN)/protoc.zip -d $(PROTOC_UNZIP_DIR)
$Q cp $(PROTOC_UNZIP_DIR)/bin/protoc $@

# checks that the idl submodule points to a commit on master, and that it matches the go module (which must be a pseudo version).
# this is only used in an explicit CI step, because it's expected to fail when developing.
#
# `git ls-tree HEAD idls` is selected because this only cares about the committed/checked-out target,
# not whatever the current status is, because only the committed value will exist for others.
#
# and last but not least: this avoids using `go` to make this check take only a couple seconds in CI,
# so the whole docker container doesn't have to be prepared.
.idl-status:
branches="$$(git submodule foreach git branch master --contains HEAD)"; \
if ! (echo "$$branches" | grep -q master); then \
>&2 echo "IDL submodule points to a commit ($$(git submodule foreach git rev-parse HEAD | tail -n 1)) that is not on master."; \
>&2 echo "Make sure the IDL PR has been merged, and this PR is updated, before merging here."; \
exit 1; \
fi
idlsha="$$(git ls-tree HEAD idls | awk '{print substr($$3,0,12)}')"; \
gosha="$$(grep github.com/uber/cadence-idl go.mod | tr '-' '\n' | tail -n1)"; \
if [[ "$$idlsha" != "$$gosha" ]]; then \
>&2 echo "IDL submodule sha ($$idlsha) does not match go module sha ($$gosha)."; \
>&2 echo "Make sure the IDL PR has been merged, and this PR is updated, before merging here."; \
exit 1; \
fi


# ====================================
# Codegen targets
# ====================================
Expand Down
2 changes: 1 addition & 1 deletion idls
Submodule idls updated from 1cd936 to 0482c0

0 comments on commit 3e3b40c

Please sign in to comment.