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

uvwasi: add recipe #18522

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

uvwasi: add recipe #18522

wants to merge 8 commits into from

Conversation

toge
Copy link
Contributor

@toge toge commented Jul 13, 2023

Specify library name and version: uvwasi/*

uvwasi is required by wasm-micro-runtime with libc=uvwasi.


@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

AbrilRBS
AbrilRBS previously approved these changes Jul 14, 2023
Copy link
Member

@AbrilRBS AbrilRBS left a comment

Choose a reason for hiding this comment

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

Looks good! The patch seems to be a subset of the changes in the linked URL, but I think that's ok :)

Other than that, I like this simple recipe, congrats!

@AbrilRBS AbrilRBS self-assigned this Jul 14, 2023
danimtb
danimtb previously approved these changes Jul 14, 2023
@jcar87 jcar87 dismissed stale reviews from danimtb and AbrilRBS July 14, 2023 12:36

pending team discussion

@jcar87 jcar87 self-assigned this Jul 14, 2023
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/out)

-if(CMAKE_C_COMPILER_ID MATCHES "AppleClang|Clang|GNU")
+if(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

@toge, thanks for your contribution.

What is the motivation behind this change? The patch mentions nodejs/uvwasi#210 but I cannot see this change there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the wrong information.
uswasi#210 only adds the installer.

I split the patch file in two.

endif()

-if(APPLE)
+if(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably unnecessary - CMAKE_MACOSX_RPATH is ON by default if the cmake minimum required is recent enough, and ON is the value we want - I suspect that even without this change, the dylib would have an install name that starts with @rpath

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jcar87
Thank you for your review!
I disable this patch.

@jcar87
Copy link
Contributor

jcar87 commented Jul 17, 2023

Thanks @toge ! It's still not clear why disabling visibility=hidden is needed?

@conan-center-bot

This comment has been minimized.

@toge
Copy link
Contributor Author

toge commented Jul 17, 2023

@jcar87
Sorry, I overlooked this.
When compiled with visibility=hidden, there is no uvwasi symbol in the shared library.
I think the only solution is to remove visibility=hidden.

@jcar87
Copy link
Contributor

jcar87 commented Jul 17, 2023

@jcar87 Sorry, I overlooked this. When compiled with visibility=hidden, there is no uvwasi symbol in the shared library. I think the only solution is to remove visibility=hidden.

I see @toge, thanks for providing a rationale behind this. In this case, and given that the upstream uvwasi repository already bundles a shared library, this seems like an issue in the upstream code that has nothing to do with Conan. My concern with the patch (that is, removing the hidden visibility compiler flag), is that if in the future this issue is addressed by the library authors by marking the exported symbols with the default visibility attribute, Conan Center would be generating a library that exports all symbols. In this case before accepting this particular patch, I would report this issue to upstream first, since it appears their shared library is not usable due to a bug (and if that is the case, I'd rather this recipe not provide a shared library at all).

@toge
Copy link
Contributor Author

toge commented Jul 17, 2023

I agree with you.
Please wait a moment to do so.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@AbrilRBS
Copy link
Member

AbrilRBS commented Aug 9, 2023

Any updates @toge? :)

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@toge
Copy link
Contributor Author

toge commented Sep 18, 2023

In gcc <= 10.0, uvwasi's test_package.cpp returns 134 error code.
It worked fine on uvwasi/0.0.18.

I'm investigating.

uvwasi/0.0.19 (test package): RUN: ./test_package
Segmentation fault (core dumped)

ERROR: uvwasi/0.0.19 (test package): Error in test() method, line 26
        self.run(bin_path, env="conanrun")
        ConanException: Error 139 while executing

Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Apr 20, 2024
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Jun 4, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Jun 4, 2024
cmake_layout(self, src_folder="src")

def requirements(self):
self.requires("libuv/1.48.0", transitive_headers=True)
Copy link
Member

Choose a reason for hiding this comment

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

We're going to allow version ranges for this library in the form of [>=1 <2] :)

@github-actions github-actions bot removed the stale label Jun 14, 2024
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Jul 14, 2024
@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ❌

Failure in build 3 (9329996b9e720ec7a2e2dd59ace6cb5c90dc2724):

  • uvwasi/0.0.21:
    CI failed to create some packages (All logs)

    Logs for packageID a29621e36b84bd974c7a9e90e81864d910970f1a:
    [settings]
    arch=x86_64
    build_type=Debug
    compiler=gcc
    compiler.libcxx=libstdc++11
    compiler.version=11
    os=Linux
    [options]
    uvwasi:shared=True
    
    [...]
    [HOOK - conan-center.py] pre_build(): [FPIC MANAGEMENT (KB-H007)] 'fPIC' option not found
    [HOOK - conan-center.py] pre_build(): [FPIC MANAGEMENT (KB-H007)] OK
    uvwasi/0.0.21 (test package): Calling build()
    uvwasi/0.0.21 (test package): CMake command: cmake -G "Unix Makefiles" -DCMAKE_TOOLCHAIN_FILE="/home/conan/workspace/prod-v1/bsr/cci-86e52200/recipes/uvwasi/all/test_package/build/Debug/generators/conan_toolchain.cmake" -DCMAKE_POLICY_DEFAULT_CMP0091="NEW" -DCMAKE_BUILD_TYPE="Debug" "/home/conan/workspace/prod-v1/bsr/cci-86e52200/recipes/uvwasi/all/test_package/."
    
    ----Running------
    > cmake -G "Unix Makefiles" -DCMAKE_TOOLCHAIN_FILE="/home/conan/workspace/prod-v1/bsr/cci-86e52200/recipes/uvwasi/all/test_package/build/Debug/generators/conan_toolchain.cmake" -DCMAKE_POLICY_DEFAULT_CMP0091="NEW" -DCMAKE_BUILD_TYPE="Debug" "/home/conan/workspace/prod-v1/bsr/cci-86e52200/recipes/uvwasi/all/test_package/."
    -----------------
    -- Using Conan toolchain: /home/conan/workspace/prod-v1/bsr/cci-86e52200/recipes/uvwasi/all/test_package/build/Debug/generators/conan_toolchain.cmake
    -- The C compiler identification is GNU 11.4.0
    -- Check for working C compiler: /usr/local/bin/cc
    -- Check for working C compiler: /usr/local/bin/cc -- works
    -- Detecting C compiler ABI info
    -- Detecting C compiler ABI info - done
    -- Detecting C compile features
    -- Detecting C compile features - done
    -- Conan: Target declared 'uvwasi::uvwasi'
    -- Conan: Target declared 'uv'
    -- Configuring done
    -- Generating done
    -- Build files have been written to: /home/conan/workspace/prod-v1/bsr/cci-86e52200/recipes/uvwasi/all/test_package/build/Debug
    uvwasi/0.0.21 (test package): CMake command: cmake --build "/home/conan/workspace/prod-v1/bsr/cci-86e52200/recipes/uvwasi/all/test_package/build/Debug" '--' '-j3'
    
    ----Running------
    > cmake --build "/home/conan/workspace/prod-v1/bsr/cci-86e52200/recipes/uvwasi/all/test_package/build/Debug" '--' '-j3'
    -----------------
    Scanning dependencies of target test_package
    [ 50%] Building C object CMakeFiles/test_package.dir/test_package.c.o
    [100%] Linking C executable test_package
    [100%] Built target test_package
    uvwasi/0.0.21 (test package): Running test()
    
    ----Running------
    > . "/home/conan/workspace/prod-v1/bsr/cci-86e52200/recipes/uvwasi/all/test_package/build/Debug/generators/conanrun.sh" && ./test_package
    -----------------
    CMake Warning:
      Manually-specified variables were not used by the project:
    
        CMAKE_POLICY_DEFAULT_CMP0091
    
    
    test_package: /home/conan/workspace/prod-v1/bsr/cci-86e52200/recipes/uvwasi/all/test_package/test_package.c:29: main: Assertion `err == UVWASI_ESUCCESS' failed.
    Aborted
    WARN: *** Conan 1 is legacy and on a deprecation path ***
    WARN: *** Please upgrade to Conan 2 ***
    uvwasi/0.0.21 (test package): WARN: Using the new toolchains and generators without specifying a build profile (e.g: -pr:b=default) is discouraged and might cause failures and unexpected behavior
    uvwasi/0.0.21 (test package): WARN: Using the new toolchains and generators without specifying a build profile (e.g: -pr:b=default) is discouraged and might cause failures and unexpected behavior
    ERROR: uvwasi/0.0.21 (test package): Error in test() method, line 26
    	self.run(bin_path, env="conanrun")
    	ConanException: Error 134 while executing ./test_package
    

Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants