[ci] add missing BuildRequires

[tchaikov]

there are couple missing BuildRequires not specified by the .spec recipe. without them, mock is not able to build the rpm package in a clean room environment.

tested on rockylinux-9 using:

$ VERSION=0.70.0 REVISION=1 rpmbuild -bs *.spec
$ mock --verbose --rebuild $HOME/rpmbuild/SRPMS/pipy-0.70.0-1.src.rpm

Signed-off-by: Kefu Chai tchaikov@gmail.com

We thank you for helping improve Pipy. In order to ease the reviewing process, we invite you to read the guidelines and ask you to consider the following points before submitting a PR:

1. We prefer to discuss the underlying issue prior to discussing the code. Therefore, we kindly ask you to refer to an existing issue, or an existing discussion in a public space with members of the Core Team. In few cases, we acknowledge that this might not be necessary, for instance when refactoring code or small bug fixes. In this case, the PR must include the same information an issue would have: a clear explanation of the issue, reproducible code, etc.

2. Focus the PR to the referred issue, and restraint from adding unrelated changes/additions. We do welcome another PR if you fixed another issue.

3. If your change is big, consider breaking it into several smaller PRs. In general, the smaller the change, the quicker we can review it.

[keveinliu]

Hi Kefu Chai,

Thanks for your contribution! However, the default RPM spec recipe is based on CentOS 7/ RHEL 7, considering of most CentOS users. There're differentiations between CentOS 7 and rockylinux-9. Let's see what you've posted:

BuildRequires:  /usr/bin/chrpath                 # Yes, this is needed by default
BuildRequires:  autoconf                             # No, I didn't see what does it for
BuildRequires:  automake                           # No, Same as autoconf
BuildRequires:  clang                                   # No, clang is provided by llvm-toolset-7 packages.
BuildRequires:  cmake3                               # Yes, this is needed by default
BuildRequires:  gcc                                      # Yes, this is needed by default
BuildRequires:  make                                   # Yes, this is needed by default
BuildRequires:  nodejs-packaging              # No, it seems not necessary 
BuildRequires:  perl-interpreter                  # Yes, this is needed by default
BuildRequires:  perl(Module::Load::Conditional), perl(File::Temp)  # No, it seems not necessary 

For those requirements, could you please double check them?

[keveinliu]

By the way, would you please help to make a rpm spec recipe and dockerfile for building pipy on RHEL 9 or Rocky Linux 9? That would be very helpful.

[tchaikov]

Hi Kefu Chai,

Thanks for your contribution! However, the default RPM spec recipe is based on CentOS 7/ RHEL 7, considering of most CentOS users. There're differentiations between CentOS 7 and rockylinux-9. Let's see what you've posted:

BuildRequires:    /usr/bin/chrpath                 # Yes, this is needed by default
BuildRequires:    autoconf                             # No, I didn't see what does it for
BuildRequires:    automake                           # No, Same as autoconf

they are used for building asio. see also https://src.fedoraproject.org/rpms/asio/blob/rawhide/f/asio.spec

BuildRequires: clang # No, clang is provided by llvm-toolset-7 packages.

i cannot find llvm-toolset-7 in this spec recipe. and an rpm recipe is supposed to be self-contained. could you point to me where llvm-toolset is defined in .spec file?

BuildRequires: cmake3 # Yes, this is needed by default BuildRequires: gcc # Yes, this is needed by default BuildRequires: make # Yes, this is needed by default BuildRequires: nodejs-packaging # No, it seems not necessary

it's used by npm install and npm run build commands.

BuildRequires: perl-interpreter # Yes, this is needed by default BuildRequires: perl(Module::Load::Conditional), perl(File::Temp) # No, it seems not necessary

it's used by openssl's Configure script. see https://github.com/openssl/openssl/blob/master/Configure#L16 and https://src.fedoraproject.org/rpms/openssl/blob/8574fb5150572c292010a84fa948a074556fa4a2/f/openssl.spec#_73 .

For those requirements, could you please double check them?

thank you for your review @keveinliu . sure. i did. could you take another look at your convenience?

[tchaikov]

By the way, would you please help to make a rpm spec recipe and dockerfile for building pipy on RHEL 9 or Rocky Linux 9? That would be very helpful.

i am not sure if we need a separate rpm recipe for el9. i'd keep a single one and use something like

%if 0%{?rhel} == 9

to conditionalize the statements if they diverge from el7 or el8.

regarding dockerfile, i don't use docker at this moment. but i'd like to do one thing at a moment. once this patch lands. i think the next thing to do is to cleanup the rpm/Dockerfile a little bit. and then, we might need to work on a Dockerfile file for EL8 and EL9 respectively.

[keveinliu]

Thanks for your update. I've confirmed your comments, but the clang should be llvm-toolset-%{?rhel} instead. Like the following:

BuildRequires:  llvm-toolset-%{?rhel}

Could you please help confirm it? Then we can merge this request. Cheers!

[tchaikov]

Thanks for your update. I've confirmed your comments, but the clang should be llvm-toolset-%{?rhel} instead. Like the following:

BuildRequires:  llvm-toolset-%{?rhel}

this is wrong. the number at the end of llvm-toolset is not the distribution's major release number. it is the major version of LLVM. take llvm-toolset-13 as an example, see https://centos.pkgs.org/9-stream/centos-appstream-aarch64/llvm-toolset-13.0.1-1.el9.aarch64.rpm.html

also, llvm-toolset is an umbrella package which depends on a couple of packages ranging from clang, lld to llvm. IIUC, clang alone would be enough for building pipy. am i right?

Could you please help confirm it? Then we can merge this request. Cheers!

[keveinliu]

Hi Kefu,

Finally I've verified your commit, and will merge it. Since we still need to build rpm package on RHEL/CentOS 7, and no official clang package available on RHEL/CentOS 7 repositories as far as I know(correct me if I'm wrong). So I'll do another commit to make the BuildRequire: clang conditionally. That means I'll still check llvm-toolset-7 in rpm spec. Thanks again for your contribution!

Cheers, Kevein

[tchaikov]

@keveinliu created #132.

please note for the same reason explained above, i think llvm-toolset-7.0-clang would be a better choice than llvm-toolset-7 .