Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1853888 - Review Request: libLTK - Ladspa v3 ToolKit
Summary: Review Request: libLTK - Ladspa v3 ToolKit
Keywords:
Status: NEW
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-NEEDSPONSOR
TreeView+ depends on / blocked
 
Reported: 2020-07-05 09:04 UTC by Lewis
Modified: 2020-07-25 02:35 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Lewis 2020-07-05 09:04:02 UTC
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

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.

Comment 1 Lyes Saadi 2020-07-05 18:40:25 UTC
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> - 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.

Comment 2 Lewis 2020-07-06 21:56:27 UTC
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.

Comment 3 Lyes Saadi 2020-07-06 23:08:43 UTC
> 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!

Comment 4 Robert-André Mauchin 🐧 2020-07-07 18:55:08 UTC
 - 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.

Comment 5 Lewis 2020-07-07 22:40:53 UTC
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...

Comment 6 Robert-André Mauchin 🐧 2020-07-08 12:48:13 UTC
> 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

Comment 7 Lewis 2020-07-08 22:58:10 UTC
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.

Comment 8 Robert-André Mauchin 🐧 2020-07-09 14:20:05 UTC
> 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.

Comment 9 Lewis 2020-07-15 22:03:08 UTC
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?
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)

Comment 10 Robert-André Mauchin 🐧 2020-07-16 14:40:16 UTC
(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.

Comment 11 Lewis 2020-07-16 21:12:30 UTC
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.

Comment 12 Robert-André Mauchin 🐧 2020-07-17 10:51:46 UTC
(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.

Comment 13 Robert-André Mauchin 🐧 2020-07-17 11:04:13 UTC
 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> - 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.

Comment 14 Lewis 2020-07-24 21:40:37 UTC
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.

Comment 15 Robert-André Mauchin 🐧 2020-07-25 02:35:46 UTC
Mock takes a srpm as input, not a spec file.
You can generate a srpm with: fedpkg --release f33 srpm


Note You need to log in before you can comment on or make changes to this bug.