Spec URL: https://github.com/muayyad-alsadi/dumb-init/blob/add-rpm-spec/dumb-init.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/alsadi/dumb-init/fedora-24-x86_64/00441467-dumb-init/dumb-init-1.1.3-1.fc24.src.rpm Description: used as entrypoint in docker but it can handle signals and process zombies Fedora Account System Username: alsadi
https://copr-be.cloud.fedoraproject.org/results/alsadi/dumb-init/fedora-24-x86_64/00441499-dumb-init/dumb-init-1.1.3-2.fc24.src.rpm
Upstream ticket https://github.com/Yelp/dumb-init/pull/111 They are asking for a static version (maybe as a sub-package) Does this make sense?
%install rm -rf $RPM_BUILD_ROOT Please, remove mkdir -p "${RPM_BUILD_ROOT}/%{_bindir}" "${RPM_BUILD_ROOT}/%{_mandir}/man1/" Pòease, use cp -p or install -pm 755 for the binary and install -pm 644 for the man1 cp %{name} "${RPM_BUILD_ROOT}/%{_bindir}/" cp %{name}.1.gz "${RPM_BUILD_ROOT}/%{_mandir}/man1/" %files %{_bindir}/%{name} %license LICENSE %doc README.md %doc %{_mandir}/man1/%{name}.1.gz Please, remove the %doc macro
I apologize for the question. but it is your first package for Fedora?
Created attachment 1191403 [details] updated spec
Created attachment 1191404 [details] updated src.rpm
> I apologize for the question. but it is your first package for Fedora? I'm an old fedora contributor, but never maintained a package before. I have submitted patches (to packagekit, yumex, and many other) that got accepted. I took part in packaging some packages like luajit
my tickets can be found by alsadi
install -pm 755 %{name}.1.gz "${RPM_BUILD_ROOT}/%{_mandir}/man1/" why must be executable? when normally should have read permissions ...
Welcome back Alsadi , Happy to continue with your first review .
First look : 1- For updated SPEC SRPM you must use new URLs in last comment , not in attachment part. 2- Spec URL must be in plain mode (in your case https://raw.githubusercontent.com/muayyad-alsadi/dumb-init/add-rpm-spec/dumb-init.spec) 3- Upstream has make , you may use it .
> install -pm 755 %{name}.1.gz "${RPM_BUILD_ROOT}/%{_mandir}/man1/" that's a typo. I'll fix it. > 1- For updated SPEC SRPM you must use new URLs in last comment , not in attachment part. sure, > 3- Upstream has make , you may use it . no, this is not an option, their make do not respect env like CFLAGS="%{optflags}" and it produce statically linked binary. BTW: what group should I use System Environment/Base System Environment/Daemons System Environment/Shells Busybox uses "System Environment/Shells" but maybe "System Environment/Daemons" might be better.
here are the url of latest spec and src.rpm https://raw.githubusercontent.com/muayyad-alsadi/dumb-init/add-rpm-spec/dumb-init.spec https://copr-be.cloud.fedoraproject.org/results/alsadi/dumb-init/fedora-24-x86_64/00442106-dumb-init/dumb-init-1.1.3-4.fc24.src.rpm
* missing BuildRequires: gcc > Source0: https://github.com/Yelp/dumb-init/archive/v%{version}.tar.gz Source0: %{url}/archive/v%{version}/%{name}-%{version}.tar.gz > help2man --no-discard-stderr --include debian/help2man --no-info --name '%{summary}' ./%{name} | gzip -9 > %{name}.1.gz help2man --no-discard-stderr --include debian/help2man --no-info --name '%{summary}' ./%{name} > %{name}.1 > mkdir -p "${RPM_BUILD_ROOT}/%{_bindir}" "${RPM_BUILD_ROOT}/%{_mandir}/man1/" > install -pm 755 %{name} "${RPM_BUILD_ROOT}/%{_bindir}/" > install -pm 644 %{name}.1.gz "${RPM_BUILD_ROOT}/%{_mandir}/man1/" install -Dpm0755 %{name} %{buildroot}%{_bindir}/%{name} install -Dpm0644 %{name}.1 %{buildroot}%{_mandir}/man1/%{name}.1 > %{_mandir}/man1/%{name}.1.gz %doc %{_mandir}/man/%{name}.1*
> * missing BuildRequires: gcc I remember that the docs says you should assume this (to confirm go to any package in fedora which was built using GCC and gcc would always be implied not explicit) BTW: I have validated it with mock on my laptop and on copr > Source0: %{url}/archive/v%{version}/%{name}-%{version}.tar.gz sure > help2man --no-discard-stderr --include debian/help2man --no-info --name '%{summary}' ./%{name} > %{name}.1 why you removed the gzip? I have checked all manpages in fedora are compressed. > %doc %{_mandir}/man/%{name}.1* wait, @cattaneo just told me to remove %doc on the man page
(In reply to Muayyad Alsadi from comment #15) > > * missing BuildRequires: gcc > > I remember that the docs says you should assume this (to confirm go to any > package in fedora which was built using GCC and gcc would always be implied > not explicit) > > BTW: I have validated it with mock on my laptop and on copr Guidelines recently have been changed (almost half year ago I suppose) and it requires listing of all BRs. > > > Source0: %{url}/archive/v%{version}/%{name}-%{version}.tar.gz > > sure > > > help2man --no-discard-stderr --include debian/help2man --no-info --name '%{summary}' ./%{name} > %{name}.1 > > why you removed the gzip? I have checked all manpages in fedora are > compressed. Because now it's gzip, in next year it might be xz. Compression is done automatically by RPM. > > > %doc %{_mandir}/man/%{name}.1* > > wait, @cattaneo just told me to remove %doc on the man page Then he's actually wrong. Man pages are documentation actually.
>> mkdir -p "${RPM_BUILD_ROOT}/%{_bindir}" "${RPM_BUILD_ROOT}/%{_mandir}/man1/" >> install -pm 755 %{name} "${RPM_BUILD_ROOT}/%{_bindir}/" >> install -pm 644 %{name}.1.gz "${RPM_BUILD_ROOT}/%{_mandir}/man1/" >install -Dpm0755 %{name} %{buildroot}%{_bindir}/%{name} >install -Dpm0644 %{name}.1 %{buildroot}%{_mandir}/man1/%{name}.1 https://fedoraproject.org/wiki/Packaging:Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS > There is very little value in choosing one style over the other, but I like your style so. so I'll do that. I'll fix all those nodes.
notes* here are the latest spec https://raw.githubusercontent.com/muayyad-alsadi/dumb-init/add-rpm-spec/dumb-init.spec or actually the exact commit https://raw.githubusercontent.com/muayyad-alsadi/dumb-init/512f31bc57a8d093e7ccd83519188be6efa19eb1/dumb-init.spec and .src.rpm https://copr-be.cloud.fedoraproject.org/results/alsadi/dumb-init/fedora-24-x86_64/00442118-dumb-init/dumb-init-1.1.3-6.fc24.src.rpm
(In reply to Igor Gnatenko from comment #16) > > > > > %doc %{_mandir}/man/%{name}.1* > > > > wait, @cattaneo just told me to remove %doc on the man page > Then he's actually wrong. Man pages are documentation actually. The documentation flag is added automatically by RPM. https://fedoraproject.org/wiki/Packaging:Guidelines#Manpages
actually yes, it was automatically marked $ rpm -qdp /home/alsadi/rpmbuild/RPMS/x86_64/dumb-init-1.1.3-7.fc24.x86_64.rpm /usr/share/doc/dumb-init/README.md /usr/share/man/man1/dumb-init.1.gz so here is the latest spec https://github.com/muayyad-alsadi/dumb-init/blob/663474f99fa16106a8ba0f5ee261df8636b48fda/dumb-init.spec
both spec and rpm https://github.com/muayyad-alsadi/dumb-init/blob/663474f99fa16106a8ba0f5ee261df8636b48fda/dumb-init.spec https://copr-be.cloud.fedoraproject.org/results/alsadi/dumb-init/fedora-24-x86_64/00442135-dumb-init/dumb-init-1.1.3-7.fc24.src.rpm
(In reply to Muayyad Alsadi from comment #12) > BTW: what group should I use > > System Environment/Base > System Environment/Daemons > System Environment/Shells > > Busybox uses "System Environment/Shells" but maybe "System > Environment/Daemons" might be better. The Group: tag is unnecessary. https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections
so we are ready?
(In reply to Muayyad Alsadi from comment #15) > > * missing BuildRequires: gcc > > I remember that the docs says you should assume this (to confirm go to any > package in fedora which was built using GCC and gcc would always be implied > not explicit) It seems like it have been updated: https://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B#BuildRequires_and_Requires
@Mosaab Alzoubi, it's your turn now
- Package could be pass by this way but I suggest that fixing make mechanism and using them because upstream has tests , so try to include tests in %%check unit .. - Don't forget to clean spec from unneeded comments . - Also never mind for group tag :) ------- > no, this is not an option, their make do not respect env like CFLAGS="%{optflags}" and it produce statically linked binary. You can fix make file as you want :) by sed or any way. ------- * Name: ... OK * Rpmlint: o errors ... OK * License: MIT ... OK * Binaries: None ... OK - Spec: see notes above * Content: Clean ... OK - %check test: see notes above * Upstream CHECKSUM(MD5) this package: cc910402302810b750858e5bb2a5433e CHECKSUM(MD5) upstream package: cc910402302810b750858e5bb2a5433e ... OK ------- I'm waking for late hour of this night , just mention me any time to mark this as approved after processing the notes.
> - Package could be pass by this way but I suggest that fixing make mechanism and using them because upstream has tests , so try to include tests in %%check unit I'll work with upstream with this regard. But this is outside the scope of this ticket. Because they use it as a statically linked file in their docker images (that's why they want to override default flags to make it statically linked regardless of your flags) > because upstream has tests , so try to include tests in %%check unit .. upstream tests require a running docker daemon. If compared to python-pymongo package, it has two sets of test, one that require a running mongo daemon and one does not. Fedora python-pymongo package only tests the second type. Regarding this packages, I guess there is no headless tests. > - Don't forget to clean spec from unneeded comments . I kept two comments, for the upsteam to be able to make packages for non-released versions. > You can fix make file as you want :) by sed or any way. hmmm, I can't see the value, this is a very simple single file c code, it does not need any kind of build systems like `make`. I guess it would be overkill to use their makefile Most of their makefile is not used to build it, but to help them maintain their github releases (generate tarballs, test it on their ubuntu specific docker images)
here https://github.com/Yelp/dumb-init/pull/111#issuecomment-241953813 the upstream indicated that we can't use "make test" because it uses pip which needs external network. the upstream suggested adding the following build requirements python, python2-mock, python2-pytest and then run PATH=.:$PATH py.test tests/ but this gave me 38 failed, 135 passed in 34.31 seconds maybe tests are debian specific @mosaab what is the next step?
when I used mock it gave me 4 test errors, so I believe we should stop here, and leave it to the upstream to fix his tests. (I'll open a ticket for him) https://paste.fedoraproject.org/414308/22152031/
the test assert than when sh is killed by a signal it should return status code = 128 + signal_code does fedora sh have this behavior or is it debian specific?
here is the upstream issue for fixing the tests https://github.com/Yelp/dumb-init/issues/115 how should we continue?
Please place spec file in a place anyone could wget it.
the tests can be done on both python2 or python3 for python2 making BR python-mock and python-test would work for both fedora and EPEL but for python3 in fedora they are called python3-test, python3-mock while on EPEL they are called python34-test, python34-mock I'll play it safe and go with python2
@dmitrij here is the latest spec url (which you can wget/curl) curl -L https://raw.githubusercontent.com/muayyad-alsadi/dumb-init/663474f99fa16106a8ba0f5ee261df8636b48fda/dumb-init.spec
I was able to produce an rpm with %check that validates the build 1. after applying the merged upstream patch (not included in 1.1.3 release) 2. adding the following BR (to make it work for epel too) %if 0%{?rhel} BuildRequires: python34, python34-pytest python34-mock %else BuildRequires: python3, python3-pytest python3-mock %endif I've built it on copr so it's your call now 1. we can go with the above spec which does not validate the build with %check 2. apply the merged upstream patch that fix the tests and ship here is the new .src.rpm with tests included https://copr-be.cloud.fedoraproject.org/results/alsadi/dumb-init/fedora-24-x86_64/00448255-dumb-init/dumb-init-1.1.3-9.fc24.src.rpm the upstream patch https://gist.githubusercontent.com/muayyad-alsadi/ca4153efc636f761e26b76a186832077/raw/a3de08469e3070a079ee2db0e8c26d4406d94740/dumb-init.fix-test.patch the updated spec that includes the patch and the https://gist.githubusercontent.com/muayyad-alsadi/ca4153efc636f761e26b76a186832077/raw/a3de08469e3070a079ee2db0e8c26d4406d94740/dumb-init.spec
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/dumb-init
Now it's perfect , thank you Muayyad for continuing to this point :)
(In reply to Muayyad Alsadi from comment #35) > I was able to produce an rpm with %check that validates the build > > 1. after applying the merged upstream patch (not included in 1.1.3 release) > 2. adding the following BR (to make it work for epel too) > > %if 0%{?rhel} > BuildRequires: python34, python34-pytest python34-mock > %else > BuildRequires: python3, python3-pytest python3-mock > %endif Just use: BuildRequires: python%{python3_pkgversion} etc. Basically, replace python3 with python%{python3_pkgversion} everywhere except in macro names.
the tests failed on armv7hl https://kojipkgs.fedoraproject.org//work/tasks/2590/15452590/build.log I notified upstream and he does not have access to arm
since the tests works on python2 on all architectures. I'll stick to python2.
*** Bug 1440289 has been marked as a duplicate of this bug. ***