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

Update dependencies described in docs #1960

Merged
merged 3 commits into from
May 24, 2023

Conversation

l0rem1psum
Copy link

While setting up the environment, I encountered a few dependency issues that I hope to solve in this pull request.

  1. Building wasmedge-sys requires wget, which is not found in the fedora/33-cloud-base VirtualBox image. See: https://github.com/WasmEdge/WasmEdge/blob/master/bindings/rust/wasmedge-sys/build.rs#L408
  2. Building openssl-sys requires the host system to have the OpenSSL library, which is also not found in fedora/33-cloud-base.

@YJDoc2
Copy link
Collaborator

YJDoc2 commented May 24, 2023

Hey @l0rem1psum Thanks for this PR! Can you please sign your commit for DCO?
Also is there a need for specifically installing ssl library in ubuntu based distros as well?
Apart from that, can you also update the same in the docs section? Thank you.

@YJDoc2 YJDoc2 self-requested a review May 24, 2023 05:09
@codecov-commenter
Copy link

codecov-commenter commented May 24, 2023

Codecov Report

Merging #1960 (9e6dd54) into main (50294dd) will increase coverage by 0.00%.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1960   +/-   ##
=======================================
  Coverage   65.01%   65.02%           
=======================================
  Files         129      129           
  Lines       14731    14731           
=======================================
+ Hits         9578     9579    +1     
+ Misses       5153     5152    -1     

@l0rem1psum
Copy link
Author

l0rem1psum commented May 24, 2023

@YJDoc2 Thanks! The previous commit was already signed off.

Let me set up a Ubuntu-based distro to verify it before I make further commits. BTW, do you have any recommended VirtualBox images in mind for me to test? Or I can just use any one?

@YJDoc2
Copy link
Collaborator

YJDoc2 commented May 24, 2023

@l0rem1psum The docs states in general 'Debian and Ubuntu based distro' ; so any recent version of those should be fine.

@l0rem1psum
Copy link
Author

Tested with ubuntu/lunar64 box. The same OpenSSL error still occurs:

error: failed to run custom build command for `openssl-sys v0.9.87`

Caused by:
  process didn't exit successfully: `/home/vagrant/youki/target/debug/build/openssl-sys-698a11f884b7d1ec/build-script-main` (exit status: 101)
  --- stdout
  cargo:rerun-if-env-changed=X86_64_UNKNOWN_LINUX_GNU_OPENSSL_LIB_DIR
  X86_64_UNKNOWN_LINUX_GNU_OPENSSL_LIB_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_LIB_DIR
  OPENSSL_LIB_DIR unset
  cargo:rerun-if-env-changed=X86_64_UNKNOWN_LINUX_GNU_OPENSSL_INCLUDE_DIR
  X86_64_UNKNOWN_LINUX_GNU_OPENSSL_INCLUDE_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_INCLUDE_DIR
  OPENSSL_INCLUDE_DIR unset
  cargo:rerun-if-env-changed=X86_64_UNKNOWN_LINUX_GNU_OPENSSL_DIR
  X86_64_UNKNOWN_LINUX_GNU_OPENSSL_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_DIR
  OPENSSL_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_NO_PKG_CONFIG
  cargo:rerun-if-env-changed=PKG_CONFIG_x86_64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_x86_64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG
  cargo:rerun-if-env-changed=PKG_CONFIG
  cargo:rerun-if-env-changed=OPENSSL_STATIC
  cargo:rerun-if-env-changed=OPENSSL_DYNAMIC
  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_STATIC
  cargo:rerun-if-env-changed=PKG_CONFIG_ALL_DYNAMIC
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_x86_64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH_x86_64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_PATH
  cargo:rerun-if-env-changed=PKG_CONFIG_PATH
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_x86_64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR_x86_64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_LIBDIR
  cargo:rerun-if-env-changed=PKG_CONFIG_LIBDIR
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_x86_64-unknown-linux-gnu
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR_x86_64_unknown_linux_gnu
  cargo:rerun-if-env-changed=HOST_PKG_CONFIG_SYSROOT_DIR
  cargo:rerun-if-env-changed=PKG_CONFIG_SYSROOT_DIR
  run pkg_config fail: `PKG_CONFIG_ALLOW_SYSTEM_CFLAGS="1" "pkg-config" "--libs" "--cflags" "openssl"` did not exit successfully: exit status: 1
  error: could not find system library 'openssl' required by the 'openssl-sys' crate

  --- stderr
  Package openssl was not found in the pkg-config search path.
  Perhaps you should add the directory containing `openssl.pc'
  to the PKG_CONFIG_PATH environment variable
  Package 'openssl', required by 'virtual:world', not found


  --- stderr
  thread 'main' panicked at '

  Could not find directory of OpenSSL installation, and this `-sys` crate cannot
  proceed without this knowledge. If OpenSSL is installed and this crate had
  trouble finding it,  you can set the `OPENSSL_DIR` environment variable for the
  compilation process.

  Make sure you also have the development packages of openssl installed.
  For example, `libssl-dev` on Ubuntu or `openssl-devel` on Fedora.

  If you're in a situation where you think the directory *should* be found
  automatically, please open a bug at https://github.com/sfackler/rust-openssl
  and include information about your system as well as this message.

  $HOST = x86_64-unknown-linux-gnu
  $TARGET = x86_64-unknown-linux-gnu
  openssl-sys = 0.9.87

  ', /home/vagrant/.cargo/registry/src/github.com-1ecc6299db9ec823/openssl-sys-0.9.87/build/find_normal.rs:190:5
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...
make: *** [Makefile:38: unittest] Error 101

OS Release:

vagrant@ubuntu-lunar:~/youki$ cat /etc/os-release 
PRETTY_NAME="Ubuntu 23.04"
NAME="Ubuntu"
VERSION_ID="23.04"
VERSION="23.04 (Lunar Lobster)"
VERSION_CODENAME=lunar
ID=ubuntu
ID_LIKE=debian
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
UBUNTU_CODENAME=lunar
LOGO=ubuntu-logo

I will update and tidy up the docs accordingly.

l0rem1psum added 2 commits May 24, 2023 16:23
Signed-off-by: l0rem1psum <iswenxuan@gmail.com>
…changes

Signed-off-by: l0rem1psum <iswenxuan@gmail.com>
@l0rem1psum
Copy link
Author

l0rem1psum commented May 24, 2023

Docs and README are all updated. In the first commit, I wrongly put libssl-devel. It should be libssl-dev.

Also, looks like the fedora/33-cloud-base box (released 2 years) is a little outdated and the latest one is fedora/38-cloud-base. Any plans to upgrade it? I can help with that.

Signed-off-by: l0rem1psum <iswenxuan@gmail.com>
@YJDoc2
Copy link
Collaborator

YJDoc2 commented May 24, 2023

Hey, why are these added in vagrantfile pkg-config elfutils-libelf-devel clang-devel ?

@l0rem1psum
Copy link
Author

Without them, it's not possible to run unit tests. Also, it was mentioned in the docs but not in the Vagrantfile. Thought it would make more sense to make things consistent.

@YJDoc2
Copy link
Collaborator

YJDoc2 commented May 24, 2023

Also, it was mentioned in the docs but not in the Vagrantfile.

Oh yes, we must have missed while updating 😓 Thanks a lot for updation. Looks good to me, I'll merge this once CI is done.

Copy link
Collaborator

@YJDoc2 YJDoc2 left a comment

Choose a reason for hiding this comment

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

lgtm 👍
Thanks a lot for your contribution!

@YJDoc2 YJDoc2 merged commit dc5d021 into containers:main May 24, 2023
10 checks passed
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.

None yet

3 participants