Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(SwingSet): adapt to promise tagging support #8403

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

erights
Copy link
Member

@erights erights commented Sep 28, 2023

The current agoric-sdk depends on an endo before endojs/endo#1785 ,

At #8312 , @FUDCo made use of the previous kludge for tagging passable promises. But this kludge no longer works after endojs/endo#1785.

Therefore, agoric-sdk would break if upgraded to depend on current endo. with the CI errors shown at #7937 .

This PR alters the relevant code that @FUDCo added in #8312 so that it checks which technique works, and then use only that one. The CI on this PR checks that this change works in the current agoric-sdk, which currently depends on an endo before endojs/endo#1785 . This same change is also incorporated and tested by #8340 , which is like #7937 but with some fixes needed to work with the current endo. Its CI is not yet clean, but the errors from #7937 due to endojs/endo#1785 seems to have gone away.

Reviewers, in the github diff view, try both "show whitespace" and "hide whitespace" because they adapt to the moved lines in opposite ways.

Copy link
Contributor

@FUDCo FUDCo left a comment

Choose a reason for hiding this comment

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

Seems fine, albeit unavoidably ugly. I've got a colliding PR in flight which moves the kmarshal.js file to a different package, so I'm sure hoping Git's merge/rebase logic does the right thing!

packages/SwingSet/src/lib/kmarshal.js Outdated Show resolved Hide resolved
packages/SwingSet/src/lib/kmarshal.js Show resolved Hide resolved
@erights erights force-pushed the markm-adapt-to-promise-tagging-support branch from 7d941ee to 630d203 Compare September 28, 2023 21:09
@erights erights added the automerge:squash Automatically squash merge label Sep 28, 2023
@erights erights force-pushed the markm-adapt-to-promise-tagging-support branch from 630d203 to 2e9d97d Compare September 28, 2023 21:32
@mergify mergify bot merged commit af6e3fd into master Sep 28, 2023
71 checks passed
@mergify mergify bot deleted the markm-adapt-to-promise-tagging-support branch September 28, 2023 22:12
anilhelvaci pushed a commit to Jorge-Lopes/agoric-sdk that referenced this pull request Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants