Initial support for wasmtime package #6083

redradist · 2021-06-29T09:13:47Z

Specify library name and version: wasmtime/0.29.0

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the
conan-center hook activated.

prince-chrismc

Typically we a hesitant about adding binary downloads https://github.com/conan-io/conan-center-index/blob/41d4fbb8c218f9d938ccb8b9d888bff52002e62e/docs/packaging_policy.md

https://github.com/conan-io/conan-center-index/blob/10d383fc84cf69517484605eb4316a10ce9ffaf6/docs/faqs.md#what-is-the-policy-on-creating-packages-from-pre-compiled-binaries

You'll need to wait for the CCI teams to move this forward of not

recipes/wasmtime/all/conanfile.py

prince-chrismc · 2021-07-03T12:42:14Z

recipes/wasmtime/all/conanfile.py

+                self.cpp_info.libs = ["wasmtime"]
+        else:
+            if self.settings.os == "Windows":
+                self.cpp_info.defines= ["/DWASM_API_EXTERN=", "/DWASI_API_EXTERN="]


conan should handle the rest

Suggested change

self.cpp_info.defines= ["/DWASM_API_EXTERN=", "/DWASI_API_EXTERN="]

self.cpp_info.defines.extend(["WASM_API_EXTERN", "WASI_API_EXTERN"])

Without /D conan generates some strange defines and test_package does not work:

set(CONAN_DEFINES_WASMTIME "-DWASM_API_EXTERN" "-DWASI_API_EXTERN") set(CONAN_BUILD_MODULES_PATHS_WASMTIME ) # COMPILE_DEFINITIONS are equal to CONAN_DEFINES without -D, for targets set(CONAN_COMPILE_DEFINITIONS_WASMTIME "WASM_API_EXTERN" "WASI_API_EXTERN")

This works for a lot of other recipes. https://github.com/conan-io/conan-center-index/search?p=2&q=cpp_info+defines

You need to use it in tandem with the other comment https://github.com/conan-io/conan-center-index/pull/6083/files#r663361146

I update the suggestion with a better syntax

It is better syntax, but it is not working ...
Please, check it locally in Windows machine

recipes/wasmtime/all/conanfile.py

prince-chrismc · 2021-07-03T12:43:34Z

recipes/wasmtime/all/test_package/CMakeLists.txt

+
+add_executable(example example.cpp)
+target_link_libraries(example PRIVATE wasmtime::wasmtime)
+target_compile_options(example PRIVATE ${CONAN_COMPILE_DEFINITIONS_WASMTIME})


Should be included with the targets

Suggested change

target_compile_options(example PRIVATE ${CONAN_COMPILE_DEFINITIONS_WASMTIME})

I cannot do this, because wasmtime::wasmtime do not have compile definitions for this target ...
By some reason conan do not add target_compile_definitions(...) for this target

Also very strange behavior:

set(CONAN_DEFINES_WASMTIME "-D/DWASM_API_EXTERN=" "-D/DWASI_API_EXTERN=") set(CONAN_BUILD_MODULES_PATHS_WASMTIME ) # COMPILE_DEFINITIONS are equal to CONAN_DEFINES without -D, for targets set(CONAN_COMPILE_DEFINITIONS_WASMTIME "/DWASM_API_EXTERN=" "/DWASI_API_EXTERN=")

CONAN_COMPILE_DEFINITIONS_WASMTIME generated properly, but CONAN_DEFINES_WASMTIME is incorrect

With https://github.com/conan-io/conan-center-index/pull/6083/files#r670901897, this should be correct now.

No it is not, see comment above for it

recipes/wasmtime/all/conanfile.py

prince-chrismc · 2021-07-03T12:46:32Z

recipes/wasmtime/all/conanfile.py

+
+class WasmtimeConan(ConanFile):
+    name = 'wasmtime'
+    homepage = 'https://github.com/bytecodealliance/wasmtime'


Suggested change

homepage = 'https://github.com/bytecodealliance/wasmtime'

homepage = 'https://wasmtime.dev/'

Disagree, because later I will add package for wasmtime-cpp and it would be weird to have two projects with the same home directory

Those should be the same recipe.

It would be separate receipts wasmtime (only C API) and wasmtime-cpp (C++ API)

Also https://wasmtime.dev/ is not descriptive enough for understanding how to use this package

No, you will still need this package
As I mentioned previously I will create support with dependency on this package when I will create wasmtime-cpp
See the issue:
bytecodealliance/wasmtime-cpp#1

It's okay to have two recipes share the same homepage.

Home page should be meaningful
https://wasmtime.dev/ does not

Perhaps you're right.
The c api lives in the main/crates/c-api subfolder.

How does this project relate to wasm-c-api at https://github.com/WebAssembly/wasm-c-api/?

@madebr It is unrelated project

recipes/wasmtime/all/conanfile.py

recipes/wasmtime/all/CMakeLists.txt

redradist · 2021-07-03T13:57:25Z

@prince-chrismc

Typically we a hesitant about adding binary downloads https://github.com/conan-io/conan-center-index/blob/41d4fbb8c218f9d938ccb8b9d888bff52002e62e/docs/packaging_policy.md

https://github.com/conan-io/conan-center-index/blob/10d383fc84cf69517484605eb4316a10ce9ffaf6/docs/faqs.md#what-is-the-policy-on-creating-packages-from-pre-compiled-binaries

You'll need to wait for the CCI teams to move this forward of not

This policy that you mentioned looks bad ... Because there are lots of ways to signal that package has vulnerabilities, for example to mark it with some flag, also source code could be analyzed by homepage

Official support will be provided by me bytecodealliance/wasmtime-cpp#1

Biulding from sources is not available, because this package is build by cargo for Rust language

prince-chrismc · 2021-07-03T16:23:36Z

You can build the rust compiler https://github.com/rust-lang/rust#installing-from-source add it as a build requirements and compile this project from source

redradist · 2021-07-03T17:24:31Z

@prince-chrismc

You can build the rust compiler https://github.com/rust-lang/rust#installing-from-source add it as a build requirements and compile this project from source

What is the point of doing this ? Why conan have binary package ?
Is not it enough to have sha256 for a package ?

prince-chrismc · 2021-07-03T22:54:45Z

Conan produces binary packages from source code to be consumed... by downloading from other services theres a risk of incompatiblity with ABI and platform variations. Not all binaries are permitted to be distrusted which has also been an issue in the past.

The last PR of this nature is stalled so I just wanted to warn you this topic has not been resolved. Packaging pre compiled binaries is possible it's just an on going issue in CCI

…x into add-wasmtime

Update license type Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

prince-chrismc · 2021-08-22T21:22:28Z

recipes/wasmtime/all/conanfile.py

        except KeyError:
            raise ConanInvalidConfiguration("Binaries for this combination of architecture/version/os not available")

+        if (self.settings.compiler, self.settings.os) == ("gcc", "Windows") and self.options.shared:
+            # FIXME: https://github.com/bytecodealliance/wasmtime/issues/3168


This was merged in

Indeed, but we will need to wait for the next release to have shared mingw avaialble.

Ahh I did not look, but thats for checking!

recipes/wasmtime/config.yml

redradist · 2021-08-30T20:30:32Z

@Croydon @prince-chrismc @SSE4 @uilianries @jgsogo
Can you review this PR again ?
Most of the comments answered again :)

conan-center-bot · 2021-08-30T21:51:17Z

All green in build 33 (3012cca0c38c32b1e064e9ea178d8cd0c111f6c0):

wasmtime/0.29.0@:
All packages built successfully! (All logs)

jgsogo

I'm worried about the issue with self.cpp_info.defines, but it's probably more related to a Conan client missing/incomplete feature. Now with the new toolchains effort, it's the time to try and fix these errors.

Anyway, I see value having this recipe available to build on top of it. As always, details and improvements will arrive in following PRs 💪

prince-chrismc · 2021-08-31T23:34:36Z

this is blocked by an old review

PR	By	Opened	Recipe	Reviews	Last	🛑 Blockers	🌟 Approvers
#6083	redradist	Jun 29	🆕 wasmtime	139	Aug 31	@uilianries	madebr, jgsogo, prince-chrismc

github-actions bot mentioned this pull request Jun 29, 2021

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

redradist mentioned this pull request Jun 29, 2021

[QUESTION] Conan C++ support bytecodealliance/wasmtime-cpp#1

Closed

redradist changed the title ~~Initial support for wasmtime package~~ [WIP] Initial support for wasmtime package Jun 29, 2021

redradist changed the title ~~[WIP] Initial support for wasmtime package~~ Initial support for wasmtime package Jun 29, 2021

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Jun 30, 2021

This comment has been minimized.

Sign in to view

redradist force-pushed the add-wasmtime branch from 031d05e to f3ef60b Compare July 1, 2021 18:29

This comment has been minimized.

Sign in to view

redradist force-pushed the add-wasmtime branch 2 times, most recently from 50fbaa9 to 7663797 Compare July 1, 2021 18:57

This comment has been minimized.

Sign in to view

redradist force-pushed the add-wasmtime branch from 7663797 to fa64d49 Compare July 1, 2021 19:20

This comment has been minimized.

Sign in to view

redradist force-pushed the add-wasmtime branch from fa64d49 to dc8865a Compare July 1, 2021 21:50

This comment has been minimized.

Sign in to view

Initial support for wasmtime package

455e9a4

redradist force-pushed the add-wasmtime branch from dc8865a to 455e9a4 Compare July 2, 2021 22:25

This comment has been minimized.

Sign in to view

Fixed build package on Linux

15736c9

This comment has been minimized.

Sign in to view

conan-center-bot removed the Failed label Jul 3, 2021

prince-chrismc suggested changes Jul 3, 2021

View reviewed changes

redradist and others added 3 commits July 4, 2021 12:32

Updates according the comments in review #0

30ad84c

Merge branch 'add-wasmtime' of github.com:redradist/conan-center-inde…

638e944

…x into add-wasmtime

Update recipes/wasmtime/all/conanfile.py

326e85d

Update license type Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

redradist added 2 commits August 14, 2021 16:56

Updated receipt for new comments from @madebr

728690c

Fixed calling method tools.cross_building(...)

59f9c9f

This comment has been minimized.

Sign in to view

Update def package(self) according the comments in review

dca562b

This comment has been minimized.

Sign in to view

prince-chrismc reviewed Aug 22, 2021

View reviewed changes

recipes/wasmtime/config.yml Outdated Show resolved Hide resolved

Updated versions of wasmtime

7eeed40

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Aug 30, 2021

Updated forgot version ugrade

c1fa065

madebr previously approved these changes Aug 30, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

Fixed sha256sum for macos

20d43a9

redradist dismissed madebr’s stale review via 20d43a9 August 30, 2021 20:38

This comment has been minimized.

Sign in to view

redradist added 2 commits August 31, 2021 00:27

Fixed SHA256 for Linux

ed7f493

Updated all SHA256 to proer values

3012cca

conan-center-bot removed the Failed label Aug 30, 2021

madebr approved these changes Aug 30, 2021

View reviewed changes

jgsogo approved these changes Aug 31, 2021

View reviewed changes

prince-chrismc approved these changes Aug 31, 2021

View reviewed changes

SSE4 approved these changes Sep 2, 2021

View reviewed changes

uilianries approved these changes Sep 6, 2021

View reviewed changes

conan-center-bot merged commit cc01ea9 into conan-io:master Sep 6, 2021

This was referenced Sep 7, 2021

wasmtime: need to download binaries in build #7190

Merged

Conan for simplifing dependency management openvinotoolkit/openvino#7194

Closed

	self.cpp_info.defines= ["/DWASM_API_EXTERN=", "/DWASI_API_EXTERN="]
	self.cpp_info.defines.extend(["WASM_API_EXTERN", "WASI_API_EXTERN"])

	homepage = 'https://github.com/bytecodealliance/wasmtime'
	homepage = 'https://wasmtime.dev/'

Initial support for wasmtime package #6083

Initial support for wasmtime package #6083

Conversation

redradist commented Jun 29, 2021 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

prince-chrismc left a comment

Choose a reason for hiding this comment

prince-chrismc Jul 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prince-chrismc Jul 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

redradist Jul 26, 2021 • edited Loading

Choose a reason for hiding this comment

redradist commented Jul 3, 2021 • edited Loading

prince-chrismc commented Jul 3, 2021

redradist commented Jul 3, 2021

prince-chrismc commented Jul 3, 2021

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

redradist commented Aug 30, 2021

This comment has been minimized.

This comment has been minimized.

conan-center-bot commented Aug 30, 2021

jgsogo left a comment

Choose a reason for hiding this comment

prince-chrismc commented Aug 31, 2021

redradist commented Jun 29, 2021 •

edited

Loading

prince-chrismc Jul 3, 2021 •

edited

Loading

prince-chrismc Jul 3, 2021 •

edited

Loading

redradist Jul 26, 2021 •

edited

Loading

redradist commented Jul 3, 2021 •

edited

Loading