-
Notifications
You must be signed in to change notification settings - Fork 3
Open
Description
@internet-diglett ran into this issue in oxidecomputer/omicron#9533. I believe what happened is:
- His branch branched from
mainfrom f72589001e66c2136a6c37def8f25084403fca82 (Dec 15) - A few weeks later, he merged with
mainvia this commit, which pulled in:- openapi/nexus/nexus-2025122300.0.0-a54e5e.json
- openapi/nexus/nexus-2026010100.0.0-943ffd.json
- openapi/sled-agent/sled-agent-12.0.0-ffacab.json
- During that merge (i.e., after running
git mergebut before committing the merge commit), he rancargo xtask openapi generate(which makes sense, to check one's resolution of conflicts that came up).- Because this tool uses the merge base with
mainto figure out what's blessed, and the merge base at this point would have been the older commit (f7258, not the one he was merging with at that point), it would have seen these three OpenAPI documents as locally-added. - Separately: this change also pulled in changes to code that affected blessed documents in compatible ways (i.e., changed doc strings).
- On
main, the blessed documents did not reflect these changes because we never update blessed documents. We simply allow them to diverge from what's generated as long as it's compatible. - In the context of the merge, since the tool thought these were locally-added, it rewrote these documents (incorrectly).
- Because this tool uses the merge base with
- After committing the merge, the tool (correctly) reported errors for each of these documents:
Failure nexus (versioned v2025122300.0.0 (blessed)): Oxide Region API
error: This version is blessed, and it's a supported version, but it's
missing a local OpenAPI document. This is unusual. If you
intended to remove this version, you must also update the list of
supported versions in Rust. If you didn't, restore the file from
git: nexus/nexus-2025122300.0.0-a54e5e.json
I found that if I followed these directions, restoring those files and then rerunning generate, it produced the correct result.
The net result of all this was that:
- The tool definitely did the wrong thing during the merge.
- Although it figured it out after the merge was committed, fixing it required separate action from the user to fix the on-disk state.
And the real upshot was that together we spent awhile just trying to figure out what had happened.
I think to hit this though you need all these to be true:
- you're doing a merge
- you haven't yet committed it
- you're pulling in new blessed versions
- there are changes subsequent to those blessed versions being added that would change them in fully-compatible ways (i.e., doc changes)
I suspect the fix is to use MERGE_COMMIT from the environment as the source of blessed specs, if it's set.
This appears fixed in #39 but I'm filing this anyway to detail the issue in case others hit it.
Metadata
Metadata
Assignees
Labels
No labels