Hello,


There are several issues:

 - You install a *.so in libdir: you must run ldconfig in %post and %postun
   See https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries

%post
/sbin/ldconfig

%postun
/sbin/ldconfig

 - You install a systemd user unit dir: these must be handled by systemd scriplets too. See https://fedoraproject.org/wiki/Packaging:Scriptlets?rd=Packaging:ScriptletSnippets#Systemd

  First you need to modify your systemd BR by adding a macro before it:

%{?systemd_requires}
BuildRequires: systemd

  Then you add the scriplets in %post, %preun, %postun:

%post
%systemd_post libcloudproviders.service

%preun
%systemd_preun libcloudproviders.service

%postun
%systemd_postun_with_restart libcloudproviders.service

 - The Release tag should starts at 1, not 0 (0.1 if it's a development snapshot).

 - Why do you split %files? Just reunite the two sections:

%files
%doc CHANGELOG README.md
%license LICENSE
%{_libdir}/libcloudproviders.so.*
%{_libexecdir}/cloudproviderd
%{_datadir}/cloud-providers/org.freedesktop.CloudProviderServerExample.ini
%{_datadir}/dbus-1/services/org.freedesktop.CloudProviderServerExample.service
%{_userunitdir}/libcloudproviders.service

 - Your changelog is empty! It must contain the date, name, version and so on:

* Sun Sep  3 2017 Carlos Soriano <csoriano> - 0.1.1-1
- Initial RPM release


Once all of this is fixed, I'll continue the review.

Rpmlint complains about other things:

 - Your description line is too long, split it to stay below 80 characters per line.

 - libcloudproviders.x86_64: W: shared-lib-calls-exit /usr/lib64/libcloudproviders.so.0.1.1 exit.5

This library package calls exit() or _exit(), probably in a non-fork()
context. Doing so from a library is strongly discouraged - when a library
function calls exit(), it prevents the calling program from handling the
error, reporting it to the user, closing files properly, and cleaning up any
state that the program has. It is preferred for the library to return an
actual error code and let the calling program decide how to handle the
situation.

Since you're on the upstream team, you probably could fix that.

Thanks Robert for the review, it's my first package so a little noobie here. I'll fix the spec and the upstream stuff tomorrow.

Here is the updated spec. It was basically what you mentioned in the comment, except for changing .1 to 0.1 since it's a preview version.
https://gitlab.gnome.org/snippets/17

The link you provided ask me to login, don't you have a direct link for it please?

If I do login, the file is 404ing.

Sorry about that, must be our new spam countermeasures in GNOME GitLab (I'll look into it). I was aware one rightful rule for spec review is to not require logins, so thanks for trying anyway.

I put it now in a fpaste: https://paste.fedoraproject.org/paste/7d7QkODJgPWql14nWzGX1A

All okay, package accepted.

Thanks Robert!

Not sure how package process works, but don't take it to production yet. We still need to fix upstream exit() call and in the final spec we will have 0.2.0 version for the library. I wanted to make sure is buildable by Fedora before bumping to that version upstream

Well it just works. Import it into Fedora when you're ready. Be aware that a Review request validity is 60 days, if you try to create your repo with fedrepo-req after 60 days, it will throw you an error.

Ah I see.
Just to be in the same page, that means the review is fine, I can change few details and it's still fine, and I can import the package to Fedora as long as it is under 60 days?

Yes the review is fine. You just need to *create the repo* with fedrepo-req within 60 days. You can import your SRPM any time later.

(In reply to Robert-André Mauchin (afk until Mon 11) from comment #12)
> Yes the review is fine. You just need to *create the repo* with fedrepo-req
> within 60 days. You can import your SRPM any time later.

Perfect, thanks!

(fedrepo-req-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/libcloudproviders

Package has been imported, closing ticket.