https://bugzilla.redhat.com/show_bug.cgi?id=1853888
Bug ID: 1853888 Summary: Review Request: libLTK - Ladspa v3 ToolKit Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Assignee: nobody@fedoraproject.org Reporter: anesa.lewis@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec and SRPM url : https://filebin.net/72pfbe4j3alvw5c8 (expires in one week) Package tested on COPR : https://copr.fedorainfracloud.org/coprs/lewisanesa/CodeColla/build/1517990/ Code hosted on my git server : git clone git://codecolla.com/libltk Description: Ladspa V3 ToolKit is a general purpose toolkit enabling object oriented programming in c. It is aimed to be simple, without memory leaks, and enabling a runtime without any alloc or free system call.
More software based on LTK are yet to come, and more info on man LTK once installed.
Fedora Account System Username: lewisanesa
Email: anesa.lewis@gmail.com
Please consider review my package for fedora official repositories inclusion.
My web server isn't ready to host files but if filebin expires, feel free to send me an email.
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
Lyes Saadi fedora@lyes.eu changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora@lyes.eu Blocks| |177841 (FE-NEEDSPONSOR) Doc Type|--- |If docs needed, set a value
--- Comment #1 from Lyes Saadi fedora@lyes.eu --- Hello :)!
You seem to be new here! You should consider reading thoroughly this page: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers
I am not a sponsor, so I won't do a proper review of your package. But, I've got some advices :P.
--- 1. Store your project online, on a permanent place, preferably on a forge.
Mainly because the Source tag have to be a URL (except if you have to clean the package before of some prohibited content or you use revision control, but you at least need to provide the URL from where you got the code in a comment), see https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/.
A forge is an ideal choice for that, I would recommend GitHub, GitLab, Pagure, SourceHut, Bitbucket... etc.
Also, fix your url. For me at least, `codecolla.com:10734` is unreachable.
--- 2. Please fix your indentation and separate different sections.
Your indentation is huge and inconsistent. Maybe you use 2-tab indentation? Use spaces instead, it will render more nicely and will be more readable.
Also, we tend to separate these sections with 1 or more blank lines: Package Information (Name-Version-Release-Summary), Upstream Information (License-URL-Source[-Architecture]), BuildRequires, Requires, Description, Prep, Build, Install, Files, Changelog.
Example: ``` Name: LTK Version: 1.5.3 Release: 11%{?dist} Summary: Ladspa v3 ToolKit
License: GPLv3 URL: codecolla.com:10734 Source: %{name}-%{version}.tar.gz
BuildRequires: gcc BuildRequires: glibc-devel BuildRequires: make BuildRequires: libunwind-devel
Requires: libunwind
%description Ladspa V3 ToolKit is a general purpose toolkit enabling object oriented programming in c.
%prep %setup -q -c
%build make NAME=%{name} VERSION=%{version}
%install [...] ```
See how much nicer and cleaner it looks ;)?
--- 3. Use `%make_build NAME=%{name} VERSION=%{version}`. This will allow for parallel build and is recommended for more consistency between packages ;).
Even better, you could just add NAME := libLTK and VERSION := 1.5.3 to the Makefile, so you'd only have to call `%make_build`.
--- 4. Add the installation process to the Makefile. In an `install` target, and replace all that with `%make_install`. This will make it way more readable and easier to maintain.
--- 5. Add the compiler flags! By adding `%set_build_flags` in a separate line before `%make_build` or, by adding `%{optflags}` after `%make_build` like that: `%make_build %{optflags}`. See https://docs.fedoraproject.org/en-US/packaging-guidelines/#_compiler_flags.
--- 6. Add a license to the package by including it in the tarball. And add in the `%files` section `%license LICENSE`. GPLv3 requires that the license be distributed with the program.
--- 7. Remove the photos from the tarball. You don't install them, and they add an unnecessary weight. And if they are not Licensed in GPLv3, it could be a Licensing violation.
--- 8. Remove the `%postun`. ``` %postun rm -f %{_libdir}/lib%{name}.so rm -f %{_includedir}/%{name} ``` It is done automatically.
--- 9. Add a devel subpackage for the includes and unversioned.so. If you don't know how to do this, look at packages like libhandy for example: https://src.fedoraproject.org/rpms/libhandy/blob/master/f/libhandy.spec
But generally, just add this after the `%description` section: ``` %package devel Summary: Development files for %{name} Requires: %{name}%{?_isa} = %{version}-%{release}
%description devel The %{name}-devel package contains libraries and header files for developing applications that use %{name}. ```
And this after the `%files` section: ``` %files devel %{_libdir}/lib%{name}.so %{_includedir}/%{name}.%{version} %{_includedir}/%{name} %{_includedir}/%{name}.%{version}/instance.h %{_includedir}/%{name}.%{version}/utils.h ```
And remove those lines from the other files section.
---- 10. Version your changelogs. You don't need to do that for older changelogs, but, at least, the last one: ``` * Sat Jul 04 2020 Lewis ANESA lan@codecolla.com - 1.5.3-11 - Merge branch 'logs' ```
---- 11. Rename the spec or the package. The package and the spec should have the exact same name. I would recommend renaming the package `libLTK` -> `ltk`, unless you really want your package to be uppercase (RPM is case-sensitive, it will be harder to install), also the lib suffix isn't necessary in Fedora.
I hope I've covered them all :P.
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #2 from Lewis anesa.lewis@gmail.com --- Lyes, thanks for your review and yes I'm new, and I'm also a newbie in packaging.
Here is the new version of the project : https://filebin.net/9txmvp0wqo1jn6tq You'll notice that work is still in progress.
Back to things you pointed : 2. 6. 7. 8. 9. 11. Treated but surely need review.
1. Project was hosted on github a long time ago : https://github.com/Pixelec/OSPOOC. But since github moved in the hands of microsoft, I don't find this place trust worthy anymore. codecolla.com is meant to become a tiny forge hosting project and their developpment. It actually hosts a git server but no http server yet.
I'm not comfortable with the idea of add a project to a forge to remove it some months later. Furthermore, it looks like fedora community hosts a git repo, with bugzilla, what a place of choice for project that target only this distribution... May I set the URL to a git URL, looks like no, only web site expected. But anyway, tell me if it's mandatory and I'll move it to nongnu savannah.
3. 4. 5. I autogenerate the spec file from project's content and git logs. I would keep the makefile without the install target. And keep infos of installation process in the generated spec file. Can I do so or what you pointed is mandatory?
The compiler flags, it sounds great to me, I'm looking for doc on this subject. Once I'll know exactly what macro defines inside the makefile, I'll use it. I hope by the end of the week.
10. I may keep only the last changelog, but it implies that lots infos might been lost. That cost me no time and no energy since it is autogenerated from git logs. But once again, let me know if shorten logs is mandatory, and I'll work on that.
12. May I add one point... I'm French and my english might be bad. Can someone check manpages? In my opinion, they are at least as important as the lib itself.
Thanks a lot and see you soon.
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #3 from Lyes Saadi fedora@lyes.eu ---
Here is the new version of the project : https://filebin.net/9txmvp0wqo1jn6tq
I can see that it is already much, much better! Good job!
Project was hosted on github a long time ago : https://github.com/Pixelec/OSPOOC. But since github moved in the hands of microsoft, I don't find this place trust worthy anymore.
This is why I personally use GitLab :P.
codecolla.com is meant to become a tiny forge hosting project and their developpment. It actually hosts a git server but no http server yet.
`URL` tag is for HTTP, you have to point to a website (or the website of a Forge)!
I'm not comfortable with the idea of add a project to a forge to remove it some months later. Furthermore, it looks like fedora community hosts a git repo, with bugzilla, what a place of choice for project that target only this distribution...
Yep, Pagure is awesome (and open for all projects!), and is open source and pretty lightweight!
There's also Gitea and many other great choices that I would recommend (over savannah)!
May I set the URL to a git URL, looks like no, only web site expected. But anyway, tell me if it's mandatory and I'll move it to nongnu savannah.
The `Source` tag (not the `URL` tag as explained above) HAVE TO (It is 1000% mandatory, unless, as I said, it fallen under two exceptions specified by the Guidelines) point directly (without Cloudflare/Anti-bot protection) to the source .tar.gz archive.
I autogenerate the spec file from project's content and git logs. I would keep the makefile without the install target. And keep infos of installation process in the generated spec file. Can I do so or what you pointed is mandatory?
That is fine, I guess. I'm a bit worried about the `ln` though, I never did that in a spec file, and you might need to add %{buildroot} to the first argument! I am not sure, though!
But you should really replace `make NAME=%{name} VERSION=%{version}` by `%make_build NAME=%{name} VERSION=%{version}`. This will only add parallelism to speed up building, and shouldn't cause errors!
The compiler flags, it sounds great to me, I'm looking for doc on this subject. Once I'll know exactly what macro defines inside the makefile, I'll use it. I hope by the end of the week.
`rpm -E "%set_build_flags"`!
This is mandatory, you can override some parameters, but you shouldn't, and it generally points to a bigger problem within the code.
May I add one point... I'm French and my english might be bad. Can someone check manpages? In my opinion, they are at least as important as the lib itself.
Oui, j'avais vu ça en vérifiant si vous étiez Packager ou pas :P. Je penserais à y jetter un coup d'œuil ;)!
But let's keep the rest of the conversation in English :).
Hope a sponsor sees this ticket, so he can proceed to a formal review :P (it shouldn't take too long)! Basically, people not already in the Packager group need to be "approved" by a sponsor. And this generally is done with their first Review Request!
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
Robert-André Mauchin 🐧 zebob.m@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |zebob.m@gmail.com
--- Comment #4 from Robert-André Mauchin 🐧 zebob.m@gmail.com --- - Source must point to the upstream archive URL
Source: %{name}-%{version}.tar.gz
- https://codecolla.com:10734/ doesn't resolve for me. Where to get the source code *officially*?
- Do not gzip the man pages yourself, it is handled by RPM
- Please remove the gz extension for man page in %files and use a glob * instead, as compression may change in the future.
%{_mandir}/man3/LTK.3.gz %{_mandir}/man3/LTKAdd.3.gz %{_mandir}/man3/LTKArray.3.gz %{_mandir}/man3/LTKBacktrace.3.gz %{_mandir}/man3/LTKCtd.3.gz %{_mandir}/man3/LTKCtl.3.gz %{_mandir}/man3/LTKExec.3.gz %{_mandir}/man3/LTKHash.3.gz %{_mandir}/man3/LTKLog.3.gz %{_mandir}/man3/LTKNum.3.gz %{_mandir}/man3/LTKRand.3.gz %{_mandir}/man3/LTKRun.3.gz %{_mandir}/man3/LTKTrg.3.gz
→
%{_mandir}/man3/LTK*.3.*
- includes go to the devel package:
%files devel %{_includedir}/%{name}.%{version} %{_includedir}/%{name}
- The unversioned library goes to devel subpackage too:
%{_libdir}/lib%{name}.so
- Why do you repeat: mkdir -p %{buildroot}%{_mandir}/man3 several times? It only needs to be created once. Try simplify the copy code too
mkdir -p %{buildroot}%{_mandir}/man3 cp -a man/LTK*.3 %{buildroot}%{_mandir}/man3/
- This is not good:
%build make NAME=%{name} VERSION=%{version}
You need to make sure that Fedora build flags are respected while compiling:
We have %set_build_flags to define them but you also need to make sure that the Makefile will respect these flags (CFLAGS, LDFLAGS). I checked the Makefile it seems good so you just need to do:
%build %set_build_flags %make_build NAME=%{name} VERSION=%{version}
If possible use: %make_build to do // compilation:
%make_build NAME=%{name} VERSION=%{version}
- install -m 755 -D bin/lib%{name}.so %{buildroot}%{_libdir}/lib%{name}.so.%{version}
This is not sufficient to set a SONAME, it's just create a link. Setting a soname is mandatory in Fedora, generally ask upstream to do it, or if they refuse, add it to your Fedora package. Should be something like:
gcc -shared -fPIC -Wl,-soname,libfoo.so.1 -o libfoo.so.1.0.0 foo.c
when building your package. Patch the Makefile to add your SONAME if upstream is not responsive.
- Use install -p to keep timestamps.
- Feel free to link to COPR build for your SPEC and SRPM (just don't delete them)
- Feel free to converse in French if needed.
- Separate your changelog entries with a newline
- Add version-release number to your changelog entries
- Main package should be named libLTK. The same name to be used for the SPEC filename, the bug report name and the Name: of the SPEC.
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #5 from Lewis anesa.lewis@gmail.com --- Hi Robert-André Mauchin, really glad to see you there!
"- Feel free to converse in French if needed." ok, everything I'm not sure to say the right way will be written in french.
- Source must point to the upstream archive URL
Source: %{name}-%{version}.tar.gz
The line you paste there, what do you really expect? (same url as in "URL:"?)
- https://codecolla.com:10734/ doesn't resolve for me. Where to get the source code *officially*?
Vraiement officiellement? c'est hébergé sur une raspberry pi chez moi et accessible en git uniquement pour le moment : git clone git://codecolla.com/libltk Je compte à termes y mettre un serveur http pour suivre l'évolution des projets, deployer, tester, démontrer, etc... En attendant je pourrais fournir https://copr.fedorainfracloud.org/coprs/lewisanesa/CodeColla/ comme URL non?
- Please remove the gz extension for man page in %files and use a glob * instead, as compression may change in the future.
J'enlève .gz dans les fichiers, les commandes gzip, mais ne dois-je pas les remplacer par d'autres commandes? Sinon, comment rpm build sait où trouver les man pages dans mes sources?
- Why do you repeat: mkdir -p %{buildroot}%{_mandir}/man3 several times? It only needs to be created once. Try simplify the copy code too
Le spec file est auto généré avec un script, il me faut encore travailler dessus. Vous le trouverez à la racine du projet git au nom de make.sh .
- includes go to the devel package
- The unversioned library goes to devel subpackage too
- %build %set_build_flags %make_build NAME=%{name} VERSION=%{version}
Done.
- install -m 755 -D bin/lib%{name}.so %{buildroot}%{_libdir}/lib%{name}.so.%{version}
This is not sufficient to set a SONAME, it's just create a link. Setting a soname is mandatory in Fedora, generally ask upstream to do it, or if they refuse, add it to your Fedora package.
Le upstream ne risque pas de refuser, puisque c'est moi. Je suis seul et unique a ravailler sur ce projet et je veux coller aux specs de fedora. Je vais faire ça : gcc -shared -fPIC -Wl,-soname,libfoo.so.1 -o libfoo.so.1.0.0 foo.c Mais mettre des virgules dans une commande de compilation me semble étrange.
For non french speaking people, I'll recap changes made to fit fedora requirements, may this help someone one day...
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #6 from Robert-André Mauchin 🐧 zebob.m@gmail.com ---
The line you paste there, what do you really expect? (same url as in "URL:"?)
Un lien pour télécharger l'archive "officiel". Ou à défault, ajoute des commentaires sur comment créer l'archive:
# git clone git://codecolla.com/libltk # tar zxvf blahblah Source0: %{name}-%{version}.tar.gz
Vraiement officiellement? c'est hébergé sur une raspberry pi chez moi et accessible en git uniquement pour le moment :
git clone git://codecolla.com/libltk Je compte à termes y mettre un serveur http pour suivre l'évolution des projets, deployer, tester, démontrer, etc... En attendant je pourrais fournir https://copr.fedorainfracloud.org/coprs/lewisanesa/CodeColla/ comme URL non?
Ne peux-tu pas créer in miroir officiel sur une forge? Gitea, Gitlab, FramaGIT: https://framagit.org/public/projects, Pagure
Tu peux aussi ajouter: VCS: git://codecolla.com/libltk
Mais URL: est aussi obligatoire, oui à la rigueur le lien COPR.
J'enlève .gz dans les fichiers, les commandes gzip, mais ne dois-je pas les remplacer par d'autres commandes?
Sinon, comment rpm build sait où trouver les man pages dans mes sources?
RPM will gzip any man page in %{buildroot}%{_mandir}/manX So copy your man pages uncompressed in the right directory and RPM will take care of zipping.
Le upstream ne risque pas de refuser, puisque c'est moi. Je suis seul et unique a ravailler sur ce projet et je veux coller aux specs de fedora.
Je vais faire ça : gcc -shared -fPIC -Wl,-soname,libfoo.so.1 -o libfoo.so.1.0.0 foo.c Mais mettre des virgules dans une commande de compilation me semble étrange.
http://tldp.org/HOWTO/Program-Library-HOWTO/shared-libraries.html#AEN95
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #7 from Lewis anesa.lewis@gmail.com --- Ok, I think I covered it all.
The problem is now that rpm build don't work anymore.
You can check it out on git clone git://codecolla.com/libltk cd libltk . make.sh
This don't work anymore since I added -Wl,-soname,lib$(NAME).so to the gcc command for final so target.
rpm build says : https://termbin.com/z2ye
Regards.
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #8 from Robert-André Mauchin 🐧 zebob.m@gmail.com ---
gcc -shared -Wl,-soname,libLTK.so -o bin/libLTK.so bin/instance.o bin/utils.o -lunwind
You didn't include the soname version here: ie -Wl,-soname,libLTK.so.X.Y.Z
gcc -Wall -c -fPIC -pedantic -o bin/utils.o src/utils.c -D_XOPEN_SOURCE=700 -DLTKVER="1.6.2" -Iinclude
36 gcc -Wall -c -fPIC -pedantic -o bin/instance.o src/instance.c -D_XOPEN_SOURCE=700 -DLTKVER="1.6.2" -Iinclude
these commands should respect the Fedora builds flags, i.e. use the previously defined CFLAGS
I don't understand anything about your build script, it's over engineered, why don't you write your SPEC file by hand, nothing change that much.
Also why do you use rpmbuild, it's not reproducible. Use a mock chroot for testing:
1. Generate SRPM:
fedpkg --release f33 srpm
2. Test in a mockbuild:
fedpkg --release f33 mockbuild --mock-config fedora-rawhide-x86_64 ---no-clean --no-cleanup-after
Also the Makefile needs to be fixed. ============================================================================= CC := gcc IFLAGS := -Iinclude CFLAGS += -Wall -c -fPIC -pedantic AFLAGS := -shared -Wl,-soname,lib$(NAME).so.$(MAJOR) LFLAGS := -lunwind DFLAGS := -D_XOPEN_SOURCE=700 -D$(NAME)VER="$(VERSION)"
all : bin bin/lib$(NAME).so
bin/lib$(NAME).so : $(patsubst src/%.c,bin/%.o, $(shell ls src/*.c)) $(CC) $(CFLAGS) $(AFLAGS) -o $@.$(MAJOR) $^ $(LFLAGS)
bin/%.o : src/%.c $(CC) $(CFLAGS) -o $@ $< $(DFLAGS) $(IFLAGS)
bin : mkdir -p $@
clean : rm -rf bin
=============================================================================== %global major 1
Name: LTK Version: 1.6.1 Release: 1%{?dist} Summary: Ladspa v3 ToolKit
License: GPLv3 URL: https://copr.fedorainfracloud.org/coprs/lewisanesa/CodeColla # git clone git://codecolla.com/libltk # cd libtlk/PROJECT # git archive --format tar.gz --prefix LTK-1.6.1/ v1.6.1 > LTK-1.6.1.tar.gz Source0: %{name}-%{version}.tar.gz
BuildRequires: gcc BuildRequires: make BuildRequires: glibc-devel BuildRequires: libunwind-devel
%description Ladspa V3 ToolKit is a general purpose toolkit enabling object oriented programming in c.
%package devel Summary: Development files for %{name} Requires: %{name}%{?_isa} = %{version}-%{release}
%description devel The %{name}-devel package contains libraries and header files for developing applications that use %{name}.
%prep %autosetup
%build %set_build_flags %make_build NAME=%{name} VERSION=%{version} MAJOR=%{major}
%install install -pm 0755 -D bin/lib%{name}.so.%{major} %{buildroot}%{_libdir}/lib%{name}.so.%{major} ln -s %{_libdir}/lib%{name}.so.%{version} %{buildroot}%{_libdir}/lib%{name}.so.%{major} ln -s %{_libdir}/lib%{name}.so %{buildroot}%{_libdir}/lib%{name}.so.%{major} mkdir -p %{buildroot}%{_includedir}/%{name} install -pm 0644 -D include/* %{buildroot}%{_includedir}/%{name}/ mkdir -p %{buildroot}%{_mandir}/man3 install -pm 0644 -D man/*.3 %{buildroot}%{_mandir}/man3/
%files %license LICENSE %{_libdir}/lib%{name}.so.%{major}*
%files devel %{_libdir}/lib%{name}.so %{_includedir}/%{name} %{_mandir}/man3/LTK*.3.*
%changelog
===============================================================================
Not tested if it works. At least the build fails due to a Fedora security flag:
gcc -O2 -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -Wall -c -fPIC -pedantic -o bin/utils.o src/utils.c -D_XOPEN_SOURCE=700 -DLTKVER="1.6.1" -Iinclude src/utils.c: In function 'LTKBacktrace': src/utils.c:208:2: error: format not a string literal and no format arguments [-Werror=format-security] 208 | if(before_str) ptr += sprintf(ptr, before_str); | ^~ src/utils.c:215:5: error: format not a string literal and no format arguments [-Werror=format-security] 215 | if(indent_str) ptr += sprintf(ptr, indent_str); | ^~ src/utils.c:218:5: error: format not a string literal and no format arguments [-Werror=format-security] 218 | if(end_str) ptr += sprintf(ptr, end_str); | ^~ src/utils.c:219:5: error: format not a string literal and no format arguments [-Werror=format-security] 219 | else if(indent_str) ptr += sprintf(ptr, indent_str); | ^~~~ src/utils.c:234:2: error: format not a string literal and no format arguments [-Werror=format-security] 234 | if(after_str) ptr += sprintf(ptr, after_str); | ^~ cc1: some warnings being treated as errors make: *** [Makefile:14: bin/utils.o] Error 1
Please fix this so it can be compiled with Fedora's build flags.
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #9 from Lewis anesa.lewis@gmail.com --- Hi,
I took Robert-André Mauchin's advices in account.
Here is the new srpm and spec files : https://copr-be.cloud.fedoraproject.org/results/lewisanesa/CodeColla/fedora-... https://copr-be.cloud.fedoraproject.org/results/lewisanesa/CodeColla/fedora-...
Now it generates libLTK.so.1 and libLTK.so -> libLTK.so.1 libLTK.so.1.6.3 -> libLTK.so.1
But I still have three questions : Why not do libLTK.so -> libLTK.so.1 -> libLTK.so.1.6.3 links? Shouldn't my package be dependent of libunwind? what if libunwind evolves? How can I change the line "rpmbuild -D "_topdir $(pwd)" -ba $SPEC_FILE" in order to use fedpkg or mock or whatever I have to use? (This line is in the over engineered auto script make.sh at the root of git://codecolla.com/libltk)
I said :
For non french speaking people, I'll recap changes made to fit fedora requirements, may this help someone one day...
- Added a MAJOR global definition in the specfile - Commented and corrected Source0 - Added empty line management in dependencies - Switched %prep to %autosetup - Added intermediate shared object file with only major in name - Added it to soname on gcc final output - Gathered together install files and managed multi section manpages - Added link layer to proper tarball creation - Added fedora's compil flags - Removed libunwind dependency (libunwind-devel build dep is enough) - Added constant strings to sprintf calls (fedora flags compatibility)
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #10 from Robert-André Mauchin 🐧 zebob.m@gmail.com --- (In reply to Lewis from comment #9)
Hi,
I took Robert-André Mauchin's advices in account.
Here is the new srpm and spec files : https://copr-be.cloud.fedoraproject.org/results/lewisanesa/CodeColla/fedora- 32-x86_64/01554976-LTK/LTK-1.6.3-14.fc32.src.rpm https://copr-be.cloud.fedoraproject.org/results/lewisanesa/CodeColla/fedora- 32-x86_64/01554976-LTK/LTK-1.6.3-14.spec
Now it generates libLTK.so.1 and libLTK.so -> libLTK.so.1 libLTK.so.1.6.3 -> libLTK.so.1
But I still have three questions : Why not do libLTK.so -> libLTK.so.1 -> libLTK.so.1.6.3 links?
That's the same thing. The unversioned is a link which goes to the -devel subpackage though. See https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages
Shouldn't my package be dependent of libunwind? what if libunwind evolves?
Libraries deps are generated automatically. If libunwind is updated with a SONAME bump, your package will need to be recompiled. SONAME bump should be announced by the maintainer one week in advance through the devel mailing list.
How can I change the line "rpmbuild -D "_topdir $(pwd)" -ba $SPEC_FILE" in order to use fedpkg or mock or whatever I have to use?
Take a look at "man mock". You got examples at the bottom:
mock -r fedora-rawhide-x86_64 --resultdir=./my-results /path/to/your.src.rpm
(This line is in the over engineered auto script make.sh at the root of git://codecolla.com/libltk)
I said :
For non french speaking people, I'll recap changes made to fit fedora requirements, may this help someone one day...
- Added a MAJOR global definition in the specfile
- Commented and corrected Source0
- Added empty line management in dependencies
- Switched %prep to %autosetup
- Added intermediate shared object file with only major in name
- Added it to soname on gcc final output
- Gathered together install files and managed multi section manpages
- Added link layer to proper tarball creation
- Added fedora's compil flags
- Removed libunwind dependency (libunwind-devel build dep is enough)
- Added constant strings to sprintf calls (fedora flags compatibility)
Source0: SOURCES/%{name}-%{version}.tar.gz
The sources should be in the same directory as your SPEC:
Source0: %{name}-%{version}.tar.gz
I'm doing some computation right now, I'll continue with fedora-review when I have more free CPU cycles.
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #11 from Lewis anesa.lewis@gmail.com --- Hi,
That's the same thing. The unversioned is a link which goes to the -devel subpackage though. See https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages
Ok, I'll keep it as is.
Libraries deps are generated automatically. If libunwind is updated with a SONAME bump, your package will need to be recompiled. SONAME bump should be announced by the maintainer one week in advance through the devel mailing list.
Great to hear!!! Admit that one day, in the future, other dev use this package. Each time my package changes it's SOMANE (e.g. MAJOR part of the version), I'll have to mail the devel mailing list at least one week before this change? I don't know this mailing list but anyway, I don't plan to change this at all.
mock -r fedora-rawhide-x86_64 --resultdir=./my-results /path/to/your.src.rpm
I'll try it out but that might not change my package, isn't it? I promise I'll integrate it, I installed mock and added myself to mock group already.
The sources should be in the same directory as your SPEC:
LTK Spec file is in the SPECS folder. LTK Tarball containig sources is in SOURCES folder. As said in section 4 of https://doc.fedora-fr.org/wiki/RPM_:_environnement_de_construction , $HOME/rpmbuild/SOURCES (dossier contenant les sources : archives, patches...) $HOME/rpmbuild/SPECS (dossier contenant les fichiers .spec contenant les instructions de construction) Do you still want me to gather them in the same folder? Is that mandatory, especially in order to find a sponsor?
And last question, if all is ok, am I ready to be sponsorized?
Regards, Lewis ANESA.
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #12 from Robert-André Mauchin 🐧 zebob.m@gmail.com --- (In reply to Lewis from comment #11)
Hi,
That's the same thing. The unversioned is a link which goes to the -devel subpackage though. See https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages
Ok, I'll keep it as is.
Libraries deps are generated automatically. If libunwind is updated with a SONAME bump, your package will need to be recompiled. SONAME bump should be announced by the maintainer one week in advance through the devel mailing list.
Great to hear!!! Admit that one day, in the future, other dev use this package.
I'll have to mail the devel mailing list at least one week before this change? Each time my package changes it's SOMANE (e.g. MAJOR part of the version),
Yes, you look for package that depends on yours, warn the devel mailing-list and the dependent package owner.
I don't know this mailing list but anyway, I don't plan to change this at all.
That ML is recommended but not mandatory. There's a lot of bikeshedding going on.
mock -r fedora-rawhide-x86_64 --resultdir=./my-results /path/to/your.src.rpm
I'll try it out but that might not change my package, isn't it? I promise I'll integrate it, I installed mock and added myself to mock group already.
It doesn't change any thing, it rebuilds your package in a chroot isolated from your main system and from a minimal installation. Like Koji but on your computer.
The sources should be in the same directory as your SPEC:
LTK Spec file is in the SPECS folder. LTK Tarball containig sources is in SOURCES folder. As said in section 4 of https://doc.fedora-fr.org/wiki/RPM_:_environnement_de_construction , $HOME/rpmbuild/SOURCES (dossier contenant les sources : archives, patches...) $HOME/rpmbuild/SPECS (dossier contenant les fichiers .spec contenant les instructions de construction)
TBH I was using this at the beginning but moved on to all fedpkg now. Doing a local build (with fedpkg local or rpmbuild) can be misleading because it will need all deps to be installed in your system, and it can also use a dependency you have installed but forgot to add to the SPEC. I'm not sure I'm clear, but your packages could locally build because you have a local dependency installed, but not work in Mock/Koji because you have forgotten to add it to your SPEC. Mock will start building from a bare system so any missing dep will be detected immediately.
Do you still want me to gather them in the same folder? Is that mandatory, especially in order to find a sponsor?
You can organise your SPEC building however you want, but that directory shouldn't appear in the SPEC. When building from the Fedora GIT repo (dist-git), all the SPEC, SOURCES and patches are at the base of the repo. Random repo: https://src.fedoraproject.org/rpms/dav1d/tree/master SPEC and sources file are at the base of the repo.
And last question, if all is ok, am I ready to be sponsorized?
I'm just reviewing your package for now, being spnsored is a separate process. See https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group
Regards, Lewis ANESA.
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #13 from Robert-André Mauchin 🐧 zebob.m@gmail.com --- I believe your spec filename and spec Name field should be named libltk, and that the devel libs requires it.
- Escape the macros in changelog by doubling the %%, for example
- Switched %%prep to %%autosetup
- The changelog entries should contain the Version-Release info:
%changelog * Wed Jul 15 2020 Lewis ANESA lan@codecolla.com - 1.6.3-14 - Corrected links and compil flags
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
===== MUST items =====
C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [x]: ldconfig not called in %post and %postun for Fedora 28 and later. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. [x]: Development (unversioned) .so files in -devel subpackage, if present.
Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated". 19 files have unknown license. Detailed output of licensecheck in /home/bob/packaging/review/LTK/review- LTK/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [!]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 0 bytes in 0 files. [x]: Packages must not store files under /srv, /opt or /usr/local
===== SHOULD items =====
Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Scriptlets must be sane, if used. [x]: SourceX tarball generation or download is documented. Note: Package contains tarball without URL, check comments [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Fully versioned dependency in subpackages if applicable. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified.
===== EXTRA items =====
Generic: [x]: Rpmlint is run on debuginfo package(s). Note: There are rpmlint messages (see attachment). [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM.
Rpmlint ------- Checking: LTK-1.6.3-14.fc33.x86_64.rpm LTK-devel-1.6.3-14.fc33.x86_64.rpm LTK-debuginfo-1.6.3-14.fc33.x86_64.rpm LTK-debugsource-1.6.3-14.fc33.x86_64.rpm LTK-1.6.3-14.fc33.src.rpm LTK.x86_64: W: spelling-error Summary(en_US) Ladspa -> Lad spa, Lad-spa, Lads pa LTK.x86_64: W: spelling-error %description -l en_US Ladspa -> Lad spa, Lad-spa, Lads pa LTK.x86_64: W: no-version-in-last-changelog LTK.x86_64: W: no-documentation LTK-devel.x86_64: W: no-version-in-last-changelog LTK-debuginfo.x86_64: W: no-version-in-last-changelog LTK-debugsource.x86_64: W: no-version-in-last-changelog LTK.src: W: spelling-error Summary(en_US) Ladspa -> Lad spa, Lad-spa, Lads pa LTK.src: W: spelling-error %description -l en_US Ladspa -> Lad spa, Lad-spa, Lads pa LTK.src: W: no-version-in-last-changelog LTK.src:68: W: macro-in-%changelog %prep LTK.src:68: W: macro-in-%changelog %autosetup LTK.src:92: W: macro-in-%changelog %postun LTK.src: W: invalid-url Source0: SOURCES/LTK-1.6.3.tar.gz 5 packages and 0 specfiles checked; 0 errors, 14 warnings.
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #14 from Lewis anesa.lewis@gmail.com --- Resolved double percent.
Ladspa is a name for a standard for virtual music instrument and is not misspelled.
The line mock -r fedora-rawhide-x86_64 --resultdir=$(pwd) $SPEC_FILE which turn into mock -r fedora-rawhide-x86_64 --resultdir=/home/lewis/Documents/libltk SPECS/LTK-1.6.3-14.spec does not work : INFO: mock.py version 2.3 starting (python version = 3.8.3)... Start(bootstrap): init plugins INFO: selinux enabled Finish(bootstrap): init plugins Start: init plugins INFO: selinux enabled Finish: init plugins INFO: Signal handler active Start: run ERROR: Cannot find/open srpm: SPECS/LTK-1.6.3-14.spec. Error: error reading package header however ll SPECS/LTK-1.6.3-14.spec says : -rw-rw-r--. 1 lewis lewis 7426 Jul 24 23:33 SPECS/LTK-1.6.3-14.spec I must have done something wrong
The changelog entries should contain the Version-Release info
Really hard to do it, I'm working on it.
Regards, Lewis ANESA.
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #15 from Robert-André Mauchin 🐧 zebob.m@gmail.com --- Mock takes a srpm as input, not a spec file. You can generate a srpm with: fedpkg --release f33 srpm
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
Lewis anesa.lewis@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(anesa.lewis@gmail | |.com) |
--- Comment #17 from Lewis anesa.lewis@gmail.com --- Project in pseudo stand by because I moved to a just built house, had a baby and a new work, all together.
But I'll come back in some month, I promise.
Product: Fedora Version: rawhide Component: Package Review
Lewis anesa.lewis@gmail.com has canceled Package Review package-review@lists.fedoraproject.org's request for Lewis anesa.lewis@gmail.com's needinfo: Bug 1853888: Review Request: libLTK - Ladspa v3 ToolKit https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #17 from Lewis anesa.lewis@gmail.com --- Project in pseudo stand by because I moved to a just built house, had a baby and a new work, all together.
But I'll come back in some month, I promise.
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
Lewis anesa.lewis@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(anesa.lewis@gmail | |.com) |
--- Comment #19 from Lewis anesa.lewis@gmail.com --- Project in pseudo stand by because house built, had a second baby, work fine.
Project active, need time for Fedora integration.
But I'll come back in some month/years, I promise.
Product: Fedora Version: rawhide Component: Package Review
Lewis anesa.lewis@gmail.com has canceled Package Review package-review@lists.fedoraproject.org's request for Lewis anesa.lewis@gmail.com's needinfo: Bug 1853888: Review Request: libLTK - Ladspa v3 ToolKit https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #19 from Lewis anesa.lewis@gmail.com --- Project in pseudo stand by because house built, had a second baby, work fine.
Project active, need time for Fedora integration.
But I'll come back in some month/years, I promise.
Product: Fedora Version: rawhide Component: Package Review
Lewis anesa.lewis@gmail.com has canceled Package Review package-review@lists.fedoraproject.org's request for Lewis anesa.lewis@gmail.com's needinfo: Bug 1853888: Review Request: libLTK - Ladspa v3 ToolKit https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #21 from Lewis anesa.lewis@gmail.com --- Project in pseudo stand by because lots of other work.
Project still active, need time for Fedora integration.
But I'll come back in some months (think end of 2023, start of 2024), I promise.
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
Lewis anesa.lewis@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(anesa.lewis@gmail | |.com) |
--- Comment #21 from Lewis anesa.lewis@gmail.com --- Project in pseudo stand by because lots of other work.
Project still active, need time for Fedora integration.
But I'll come back in some months (think end of 2023, start of 2024), I promise.
Product: Fedora Version: rawhide Component: Package Review
Lewis anesa.lewis@gmail.com has canceled Package Review package-review@lists.fedoraproject.org's request for Lewis anesa.lewis@gmail.com's needinfo: Bug 1853888: Review Request: libLTK - Ladspa v3 ToolKit https://bugzilla.redhat.com/show_bug.cgi?id=1853888
--- Comment #23 from Lewis anesa.lewis@gmail.com --- I got back to the project since March.
I'm building a website for hosting libLTK and everything that comes from it.
Last review raised : Package is named according to the Package Naming Guidelines.
Package named libLTK as it mainly installs libLTK.so .
I take for example https://src.fedoraproject.org/rpms/libcbor which is named libcbor and provides libcbor.so .
Should I rename package? lower case? Should I share some API naming policy to be sure having best practise?
In past 4 years libLTK did not changed a lot and serves as a base for extensions libraries.
Best regards, Lewis ANESA.
https://bugzilla.redhat.com/show_bug.cgi?id=1853888
Lewis anesa.lewis@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(anesa.lewis@gmail | |.com) |
--- Comment #23 from Lewis anesa.lewis@gmail.com --- I got back to the project since March.
I'm building a website for hosting libLTK and everything that comes from it.
Last review raised : Package is named according to the Package Naming Guidelines.
Package named libLTK as it mainly installs libLTK.so .
I take for example https://src.fedoraproject.org/rpms/libcbor which is named libcbor and provides libcbor.so .
Should I rename package? lower case? Should I share some API naming policy to be sure having best practise?
In past 4 years libLTK did not changed a lot and serves as a base for extensions libraries.
Best regards, Lewis ANESA.
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com