Skip to content

Conversation

@steven-johnson
Copy link
Contributor

The API for WABT has changed in top-of-tree. This fixes the incompatibilities. It also (temporarily) changes to pulling WABT from top-of-tree (rather than a known release) as the most recent labeled release doesn't have these changes. (We'll change back to a fixed WABT release when the next one is released.)

The API for WABT has changed in top-of-tree. This fixes the incompatibilities. It also (temporarily) changes to pulling WABT from top-of-tree (rather than a known release) as the most recent labeled release doesn't have these changes. (We'll change back to a fixed WABT release when the next one is released.)

if (WITH_WABT)
set(WABT_VER 1.0.25)
set(WABT_VER main)
Copy link
Member

Choose a reason for hiding this comment

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

I am worried about this making our builds unpredictable. I think better would be to hard-code an override-able git commit hash, like so:

Suggested change
set(WABT_VER main)
set(WABT_VER "09c40635207d42dd30ffaca22477fd3491dd9e7d"
CACHE STRING "WABT commit to download (tag, branch name, or hash)")

and then in CI we can pull nightlies by setting the WABT_VER cache variable to main.

Copy link
Member

Choose a reason for hiding this comment

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

(09c40635207d42dd30ffaca22477fd3491dd9e7d is the current top-of-tree hash)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair, keep in mind this should be very short term

@steven-johnson steven-johnson merged commit 832efeb into master Feb 11, 2022
@steven-johnson steven-johnson deleted the srj/wabt-fixes branch February 11, 2022 22:46
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.

4 participants