[DRAFT-only-for-discussion] QRMI packaging#68
[DRAFT-only-for-discussion] QRMI packaging#68cclaudio wants to merge 6 commits intoqiskit-community:mainfrom
Conversation
Signed-off-by: Claudio Carvalho <cclaudio@ibm.com>
For usage run: make help With a Makefile we can have single source of truth on how to build, package, lint and format check QRMI. Both users and CI workflows could leverage that. Signed-off-by: Claudio Carvalho <cclaudio@ibm.com>
The caller must provide some inputs, such as the distro and distro version. Currently, this is building both the Rust and Python codes. We probably want to focus on the Rust code only for now since it has higher priority. Signed-off-by: Claudio Carvalho <cclaudio@ibm.com>
Signed-off-by: Claudio Carvalho <cclaudio@ibm.com>
This proposes to release the source and the vendor tarballs. Users can use the latter to build QRMI in a disconnected environment. This also packages QRMI for some rpm-based and deb-based distros. We can add more distros or even postpone this if we want. Signed-off-by: Claudio Carvalho <cclaudio@ibm.com>
Signed-off-by: Claudio Carvalho <cclaudio@ibm.com>
|
@cclaudio I haven't yet viewed all of the files though, but I have a couple of questions as starting point.
|
Currently, the PR does not include any RHEL. I can check if the ubi8 and ubi9 provides all the packages we need to build the libqrmi |
| Description: Quantum Resource Management Interface (QRMI) | ||
| QRMI is a set of libraries that provide a unified interface to control | ||
| the state of quantum resources. It exposes a C API (libqrmi.so / qrmi.h) | ||
| and a Python API built on top of the same Rust core. |
There was a problem hiding this comment.
Question: What do you mean by "same Rust core"?
The libqrmi.so built with cargo build --release and the one built with maturin build --release are different. The latter includes src/pyext.rs, references Python’s shared library symbols, and attempts to link to them at runtime.
There was a problem hiding this comment.
I used AI to assist me with some specific tasks. Description was AI generated
| g++, | ||
| make, | ||
| cmake, | ||
| libssl-dev, |
There was a problem hiding this comment.
libssl-dev is no longer required. This dependency was removed by #67 . Also next zlib1g-dev is no longer needed.
| Section: libdevel | ||
| Depends: libqrmi (= ${binary:Version}), ${misc:Depends} | ||
| Description: Development files for libqrmi | ||
| Header file (qrmi.h) and static library (libqrmi.a) for building |
There was a problem hiding this comment.
Why does this package contain only static library and header ?
I think libqrmi.so should be also included in this package for development.
There was a problem hiding this comment.
That description is outdated. The package is not including the static library (IIUC, the static library is linked to glibc and if so the QRMI code would be contaminated with GPL)
| push: | ||
| tags: | ||
| - "v*.*.*" | ||
| jobs: |
There was a problem hiding this comment.
package-rhel8, package-rhel9 and package-rhel10 are missing. At least, we need to provide packages for rhel8 and 9 because most of HPC data center uses.
There was a problem hiding this comment.
How do we build libqrmi.so with RHEL8 compiler/linker ?
There was a problem hiding this comment.
How do we build libqrmi.so with RHEL8 compiler/linker ?
That depends:
- for the CI we just need to add one more job in the
release.yamlfor each RHEL version we want - for users via cmdline, they would need to run
make buildinside a container. We could create a Dockerfile (docker native) or Containerfile (podman native)
There was a problem hiding this comment.
package-rhel8, package-rhel9 and package-rhel10 are missing. At least, we need to provide packages for rhel8 and 9 because most of HPC data center uses.
OK, I will add RHEL 8 9 and 10
| distro: almalinux | ||
| distro-version: "9" | ||
| container-image: almalinux:9 | ||
|
|
There was a problem hiding this comment.
almalinux10 is not supported ?
| distro: rockylinux | ||
| distro-version: "9" | ||
| container-image: rockylinux:9 | ||
|
|
There was a problem hiding this comment.
rockylinux 10 is not supported ?
There was a problem hiding this comment.
I can add rockylinux 10
| # | ||
| # Packages produced: | ||
| # qrmi - shared library (libqrmi.so) and C header (qrmi.h) | ||
| # qrmi-devel - development files (static library + header) |
There was a problem hiding this comment.
I'm confused with above. Why do we need to create 2 separate packages ? - Both will be used by developers for development because each contains header file. I thought it is enough to create 1 package that contains libqrmi.so, libqrmi.a and qrmi.h.
| # Build requirements: | ||
| # - Rust toolchain >= 1.91 (install via rustup; not available as an RPM on EL8/EL9) | ||
| # - cargo-vendor (bundled with cargo) | ||
| # - gcc, make, cmake, openssl-devel, zlib-devel, pkg-config |
There was a problem hiding this comment.
please remove openssl-devel and zlib-devel. They are no longer required by recent change #67.
| BuildRequires: gcc-c++ | ||
| BuildRequires: make | ||
| BuildRequires: cmake | ||
| BuildRequires: openssl-devel |
There was a problem hiding this comment.
openssl-devel is no longer needed.
| BuildRequires: make | ||
| BuildRequires: cmake | ||
| BuildRequires: openssl-devel | ||
| BuildRequires: zlib-devel |
There was a problem hiding this comment.
zlib-devel is no longer needed.
|
Personally, I think we can treat RHEL interchangeability with rocky and alma.
I would like it if this is available as a tarball though, personally. I fear we're complicating things here |
@cclaudio Are you considering providing RPM/Deb packages instead of the tarball distribution that you and Alex discussed and agreed on earlier? There are pitfalls to this approach. Considering those backgrounds, I proposed to create a tarball, instead of RPM/deb. |
| %{_libdir}/libqrmi.so | ||
| %{_includedir}/qrmi.h | ||
|
|
||
| # --------------------------------------------------------------------------- |
There was a problem hiding this comment.
on file in general: OK after looking and thinking more I understand more about why you want to have the rpm specs. in principle ok
| # Format check libqrmi, dependencies, examples and binaries | ||
| fmt: check-rustfmt | ||
| cargo fmt --all -- --check --verbose | ||
|
|
||
| lint: check-clippy | ||
| cargo clippy --all-targets -- -D warnings |
There was a problem hiding this comment.
should also lint and format python code
| lint-bins: check-clippy | ||
| cargo clippy --bin task_runner --features="build-binary" -- -D warnings | ||
| cargo clippy --bin stubgen --features="pyo3" -- -D warnings | ||
|
|
||
| lint-rust: lint lint-bins |
There was a problem hiding this comment.
I don't understand this, why do we want to lint bins? what does that mean
| maturin build --release | ||
|
|
||
| fmt-wheels: check-venv | ||
| .venv/bin/black --check ./python |
There was a problem hiding this comment.
should not be hardcoded the way you setup your python venv imo
| @@ -0,0 +1,125 @@ | |||
| --- | |||
| # SPDX-License-Identifier: Apache-2.0 | |||
There was a problem hiding this comment.
Suppose multiple developers share the same system — for example, the login nodes or pre/post nodes on an HPC cluster. If Developer-A needs a specific version of libqrmi.so/qrmi.h for his/her development, while Developer-B needs a different version, I think RPM/Deb‑based distribution will not allow installing multiple versions simultaneously.
In addition, wouldn’t installation require admin privileges?
There was a problem hiding this comment.
Could you please consider to support a simple tarball distribution, which contains libqrmi.so, libqrmi.a and qrmi.h ?
|
I've reviewed and thought some more. (Cross-postedo on Slack) I think I would advise to drop the current MR, and just put out the part building on RHEL 8 - based code, and uploading that artifact. Then we can discuss more deeply what we need in terms of the RPM builds and Makefiles and everything. ]Simply because I don't see us finalizing the current PR at a level we're all happy with and agree with, and on my side I have some urgency on the libqrmi.so+qrmi.h availability. |
|
I think this PR fullfiled its initial purpose which is discussing direction. Thank you all for your quick feedback! I am keeping this PR opened so that we don't loose the discussion and we can come back to it later. For the sake of time, I am creating another PR where we can laser focus on:
|
|
Are we going to deliver our libqrmi.so/qrmi.h in form of RPM/deb, not a simple tarball ? Then, I have some concerns about distributing our library and header files using RPM/deb packages, especially in the context of HPC environments. Concerns about distributing our library via RPM/deb packages
Please ignore if you've already addressed above my concerns. |
|
Thank you @ohtanim. We have not addressed all the points you mentioned. Currently, these are the libraries that the libqrmi.so is depending on. Since all of them are already installed in the UBI8, I think it should be fine to publish a tarball like I will incorporate that in the new PR |
|
I agree tarballs are important for people who want more flexibility, but, at the same time, I think RPM/deb packages are still very useful. I believe, in many practical cases for hpc-admins, one installation is enough, and package-manager install is just the simplest option. So I’d see packages as a useful extra, not something that replaces tarballs. |
Our feedback from Julich on slack was that they prefer to install from source build. with Spack / easybuild or similar I think. |
Fixes: #65
Some discussion topics:
make rpmandmake debcan be used by users and the CI. If we want to publish rpm and deb files, we should define what distros (and distro versions) has higher priority. That can save some space