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.

Sign up for GitHub

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

Jump to bottom

Update emscripten Dockerfile to 3.1.19 #13424

Merged
merged 2 commits into from
Aug 25, 2022
Merged

Update emscripten Dockerfile to 3.1.19 #13424

merged 2 commits into from
Aug 25, 2022

Conversation

Marenz
Copy link
Contributor

@Marenz Marenz commented Aug 22, 2022

No description provided.

@Marenz Marenz mentioned this pull request Aug 22, 2022
Closed
@Marenz Marenz force-pushed the update-emscripten-dockerfile branch from a24a7f3 to 1e16d6d Compare August 23, 2022 11:27
@Marenz Marenz mentioned this pull request Aug 23, 2022
Merged
@Marenz Marenz force-pushed the update-emscripten-dockerfile branch 3 times, most recently from e0222d0 to 63903a6 Compare August 23, 2022 17:23
@Marenz Marenz changed the title Update emscripten Dockerfile to 3.1.18 Update emscripten Dockerfile to 3.1.19 Aug 23, 2022
@Marenz Marenz force-pushed the update-emscripten-dockerfile branch from 63903a6 to 16fb3bc Compare August 23, 2022 17:28
@Marenz Marenz force-pushed the update-emscripten-dockerfile branch from 16fb3bc to a397d41 Compare August 23, 2022 17:44
@github-actions
Copy link

solbuildpackpusher/solidity-buildpack-deps:emscripten-12 [solbuildpackpusher/solidity-buildpack-deps@sha256:b61a992179d8e937cdeb5113dd8cdacd7b53889b4c946ad30a221f90900e7f9f].

@github-actions
Copy link

solbuildpackpusher/solidity-buildpack-deps:emscripten-12 [solbuildpackpusher/solidity-buildpack-deps@sha256:70c37e541c518eb726bb0dfe0a4e03d7c331f182de1eef82d54e43e6f99a2d95].

@github-actions
Copy link

solbuildpackpusher/solidity-buildpack-deps:emscripten-12 [solbuildpackpusher/solidity-buildpack-deps@sha256:688fc4bd697c97ec1c4fe25d1ac658cffc3bb4f060c7a05f18ffadc4375d3047].

@github-actions
Copy link

solbuildpackpusher/solidity-buildpack-deps:emscripten-12 [solbuildpackpusher/solidity-buildpack-deps@sha256:ed0b64eef5463b56a17e1c906573fd2e6e7eb29de58d06b19a7dabd3bd41830d].

@github-actions
Copy link

solbuildpackpusher/solidity-buildpack-deps:emscripten-12 [solbuildpackpusher/solidity-buildpack-deps@sha256:c07fdb46c8e66cb1129ab7bfb703ea0741ce6f6b280ae9c781092b6fff5c4f89].

@Marenz Marenz force-pushed the update-emscripten-dockerfile branch from a397d41 to 7a1e39c Compare August 23, 2022 19:31
@github-actions
Copy link

solbuildpackpusher/solidity-buildpack-deps:emscripten-12 [solbuildpackpusher/solidity-buildpack-deps@sha256:e36fbddc13a1a199c42ce638d50a5e57da49742b7b771e6d90a825bf69ff8a49].

@github-actions
Copy link

solbuildpackpusher/solidity-buildpack-deps:emscripten-12 [solbuildpackpusher/solidity-buildpack-deps@sha256:b0059109479b3a0dd40a56d092726edd5550487bb39f124d2af03713f54514ca].

cameel
cameel reviewed Aug 24, 2022
View reviewed changes
scripts/docker/buildpack-deps/Dockerfile.emscripten Outdated Show resolved Hide resolved
scripts/docker/buildpack-deps/Dockerfile.emscripten Outdated Show resolved Hide resolved
scripts/docker/buildpack-deps/Dockerfile.emscripten Outdated Show resolved Hide resolved
scripts/docker/buildpack-deps/Dockerfile.emscripten Outdated Show resolved Hide resolved
scripts/ci/build_emscripten.sh Show resolved Hide resolved
@Marenz Marenz force-pushed the update-emscripten-dockerfile branch from e6698de to 1dbef27 Compare August 24, 2022 14:45
@github-actions
Copy link

solbuildpackpusher/solidity-buildpack-deps:emscripten-12 [solbuildpackpusher/solidity-buildpack-deps@sha256:d59441964e5b1ea26f08871dea1cc4f3a3e9fb14604684ba47ac97ef62eafc1d].

@Marenz Marenz force-pushed the update-emscripten-dockerfile branch from 1dbef27 to 3587054 Compare August 25, 2022 11:08
@github-actions
Copy link

solbuildpackpusher/solidity-buildpack-deps:emscripten-12 [solbuildpackpusher/solidity-buildpack-deps@sha256:65a82268792a5a2ee85ad432baf04a056c3a4006941ab3a4416eb1a0614883f3].

cameel
cameel approved these changes Aug 25, 2022
View reviewed changes
Copy link
Member

@cameel cameel left a comment

Choose a reason for hiding this comment

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

The only remaining problem is a missing TODO note so I'm approving but it would be best to have it IMO.

scripts/ci/build_emscripten.sh
Comment on lines +65 to +69
# Disable warnings for unqualified "move()" calls, introduced and enabled by
# default in clang-16 which is what the emscripten docker image uses.
# Additionally, disable the warning for unknown warnings here, as this script is
# also used with earlier clang versions.
CMAKE_CXX_FLAGS="-Wno-unqualified-std-cast-call -Wno-unknown-warning-option"
Copy link
Member

Choose a reason for hiding this comment

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

Can you add the TODO comment I mentioned earlier? I really think we need something that clearly indicates when the hack can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The hack can be removed literally right after this PR and I will do that once it's merged <3 so I don't think we need the comment

@Marenz Marenz merged commit c4d9cdf into develop Aug 25, 2022
@Marenz Marenz deleted the update-emscripten-dockerfile branch August 25, 2022 14:08
@cameel cameel mentioned this pull request Aug 25, 2022
Merged
This was referenced Sep 8, 2022
Merged
Closed
@cameel cameel mentioned this pull request Sep 21, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cameel cameel cameel approved these changes

Assignees
No one assigned
Labels
build system 🏗️
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Marenz @cameel @timweri