Bug 4532

Hi,

Your spec file has a major issue, you need to use fedora build flags


%build
cmake \
  -DCMAKE_INSTALL_PREFIX=%{_libdir}/kodi/addons/



Use the %cmake macro, you can see what is does by running '$ rpm -E %cmake'
also use the %make_build macro instead of 

make %{?_smp_mflags}


Change it to

%build
%cmake .
%make_build

The release version is wrong for a git snapshot, replace 1 with 0.1

The debug package has spurious-executable-perm errors that could be fixed by add this to %prep section

# fix spurious-executable-perm on debug package
find . -name '*.h' -or -name '*.cpp' | xargs chmod a-x

(In reply to Dominic Robinson from comment #0)

> The output rpmlint gives on both the source and binary packages. Explain for
> each message why you've chosen to ignore it.
> No errors, no warnings.
> 

You have no errors because your debug package is broken (review failure) due to your cmake command

kodi-inputstream.adaptive-debuginfo.x86_64: E: debuginfo-without-sources


Once the debug package is fixed you have plenty of errors

$ rpmlint /home/leigh/development/rpmbuild/RPMS/x86_64/kodi-inputstream.adaptive-debuginfo-17.1-1.20161215gitcd26d5d.fc26.x86_64.rpm
1 packages and 0 specfiles checked; 224 errors, 246 warnings.

Thanks for the feedback...

Release number has been amended and I am now using the cmake macro. I have also added your snippet to correct the permission issues within the debuginfo package (which now build) along with some permission issues post install.

rpmlint now reporting 222 errors and 2 warnings- the warnings are in relation to en_US dictionary checks merely inputstream instead of "input stream" ; that can be ignored for obvious reasons.

The 222 errors are all "incorrect-fsf-address" which are all contained within a third party library upstream. This is largely out of my control, but as suggested here https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address I have reported the issue upstream; see https://github.com/peak3d/inputstream.adaptive/issues/27 .

^

SPEC:
https://raw.githubusercontent.com/dcrdev/rpm-kodi-inputstream.adaptive/kodi-inputstream.adaptive-17.1-0.1.20161215gitcd26d5d/kodi-inputstream.adaptive.spec

Source RPM:
https://github.com/dcrdev/rpm-kodi-inputstream.adaptive/releases/download/kodi-inputstream.adaptive-17.1-0.1.20161215gitcd26d5d/kodi-inputstream.adaptive-17.1-0.1.20161215gitcd26d5d.fc25.src.rpm

Debuginfo:
https://github.com/dcrdev/rpm-kodi-inputstream.adaptive/releases/download/kodi-inputstream.adaptive-17.1-0.1.20161215gitcd26d5d/kodi-inputstream.adaptive-debuginfo-17.1-0.1.20161215gitcd26d5d.fc25.x86_64.rpm

You should replace dots in the package name by dashes, as required in the packaging guidelines:
https://fedoraproject.org/wiki/Packaging:Naming?rd=Packaging:NamingGuidelines#Separators

As a result, your package should be named kodi-inputstream-adaptive.

Done

SPEC:
https://raw.githubusercontent.com/dcrdev/rpm-kodi-inputstream-adaptive/kodi-inputstream-adaptive-17.1-0.1.20161215gitcd26d5d/kodi-inputstream-adaptive.spec

Source RPM:
https://github.com/dcrdev/rpm-kodi-inputstream-adaptive/releases/download/kodi-inputstream-adaptive-17.1-0.1.20161215gitcd26d5d/kodi-inputstream-adaptive-17.1-0.1.20161215gitcd26d5d.fc25.src.rpm

Debuginfo:
https://github.com/dcrdev/rpm-kodi-inputstream-adaptive/releases/download/kodi-inputstream-adaptive-17.1-0.1.20161215gitcd26d5d/kodi-inputstream-adaptive-debuginfo-17.1-0.1.20161215gitcd26d5d.fc25.x86_64.rpm

Thanks

Sorry to bump this - but I'd be really interested in getting this into RPMFusion.

(In reply to Dominic Robinson from comment #7)
> Sorry to bump this - but I'd be really interested in getting this into
> RPMFusion.
@Dominic,
I'm okay to sponsor you if you can find a reviewer for your package.
Please consider reviewing other packages until then.
Probably the best candidate for the review is Mohamed, (pikachu.2014).

Sure, I will review this package :)

(In reply to Dominic Robinson from comment #7)
> Sorry to bump this - but I'd be really interested in getting this into
> RPMFusion.

Hopefully Mohamed will be able to review.

One little issue. Currently Kodi only build for i686 and x86_64 arches. So you need to use that in your spec:
---
ExclusiveArch: i686 x86_64
---

Huh I thought it built under arm as well - in any case I've made the changes:

https://repo.dcrdev.com/fc25/specfiles/kodi-inputstream-adaptive.spec

https://repo.dcrdev.com/fc25/SRPMS/kodi-inputstream-adaptive-17.1-0.1.20161215gitcd26d5d.fc25.src.rpm

https://repo.dcrdev.com/fc25/x86_64/debug/kodi-inputstream-adaptive-debuginfo-17.1-0.1.20161215gitcd26d5d.fc25.x86_64.rpm

(In reply to Dominic Robinson from comment #11)
> Huh I thought it built under arm as well - in any case I've made the changes:
Yes, but the way it is compiled is specific to a given SOC, so there is a need to patch kodi to support armv7hl/aarch64 in an agnostic way.

Sorry for the delay, here is a first review.

1) Package version is not 17.1. You should refer to the inputstream.adaptive/addon.xml.in file to get the right version corresponding to the snapshot you're packaging. The snapshot you're packaging corresponds to a 1.0.6 version (see https://github.com/peak3d/inputstream.adaptive/commit/cd26d5d7896e402be180e252b155c932b163b181).
You should ask upstream to create release tags to make things easier for you... Even if it quite rare from Kodi addon developers :(.

2) As a result, you should set up a dedicated for the minimum required Kodi version. By the way, since Kodi addon API is stable between major release, requiring kodi >= 17.0 may be enough:

%global kodi_version 17.0

[...]

BuildRequires:  cmake
BuildRequires:  gcc-c++
BuildRequires:  kodi-devel >= %{kodi_version}
BuildRequires:  kodi-platform-devel >= %{kodi_version}
BuildRequires:  expat-devel

Requires:       kodi >= %{kodi_version}

3) If you plan to package more binary Kodi addons and use this .spec as a model, I suggest you to define your package name from the upstream name, and not the reverse (in case some Kodi addons may use dashes as separators). Replace :

Name:           kodi-inputstream-adaptive
[...]
%global aname %(n=%{name}; n=${n//-/.}; echo ${n:5})

by

%global aname inputstream.adaptive
[...]
Name:           kodi-%(n=%{name}; n=${n//./-})

4) Binary addons are dlopened at Kodi execution. They're not shared libraries. As a result, you don't need to call ldconfig at post-install. Remove %post/%postun targets.

5) This addon bundled libbento4. If you don't plan to unbundle this third-party library as allowed by Fedora guidelines, you must add a virtual Provided on this library:

Provides: bundled(bento4)

(unfortunately I'm unable to determine which version of bento4 is bundled).

You're also missing the license text in the %files section. Please ask upstream to ship a copy of the GPL as "COPYING" or "LICENSE" and then you can add a %license COPYING line.

Ok thanks all for the feedback - I've made some adjustments:
https://repo.dcrdev.com/fc25/specfiles/kodi-inputstream-adaptive.spec
https://repo.dcrdev.com/fc25/SRPMS/kodi-inputstream-adaptive-1.0.6-0.1.20161215gitcd26d5d.fc25.src.rpm
https://repo.dcrdev.com/fc25/x86_64/debug/kodi-inputstream-adaptive-debuginfo-1.0.6-0.1.20161215gitcd26d5d.fc25.x86_64.rpm
https://repo.dcrdev.com/fc25/x86_64/kodi-inputstream-adaptive-1.0.6-0.1.20161215gitcd26d5d.fc25.x86_64.rpm

@Mohamed - I take your point about the naming convention, but unfortunately rpmbuild complains about recursion when trying to resolve the name directive dynamically.

In regards to the bundled bento4 - I'm not certain which version it is either; I did some comparisons between it and the bento4 git repo and was unable come up with anything mainly due to the infrequency in which it is committed to. I have added the virtual provides for now, but the lack of version obviously throws a warning in rpmlint.

@Michael
I have requested they ship a copy of the gpl upstream.

Further to my previous comment I was able to get the dynamic Name directive working.

https://repo.dcrdev.com/fc25/specfiles/kodi-inputstream-adaptive.spec
https://repo.dcrdev.com/fc25/SRPMS/kodi-inputstream-adaptive-1.0.6-0.1.20161215gitcd26d5d.fc25.src.rpm
https://repo.dcrdev.com/fc25/x86_64/debug/kodi-inputstream-adaptive-debuginfo-1.0.6-0.1.20161215gitcd26d5d.fc25.x86_64.rpm
https://repo.dcrdev.com/fc25/x86_64/kodi-inputstream-adaptive-1.0.6-0.1.20161215gitcd26d5d.fc25.x86_64.rpm

Any update on this? Thanks

(In reply to Dominic Robinson from comment #17)
> Any update on this? Thanks

F26 release is probably a major factor for the delay, reviews tend to be lower priority during this time.

Sorry for the late answer. A lot of work at my job unfortunately.

Your package seems quite good now. Some notes:

- Fix BR condition on kodi-devel, which should match the one on kodi in Requires:
    BuildRequires:  kodi-devel >= %{kodi_version}

- Since upstream doesn't publish release tags, I'd consider that any commit containing an update for the plugin version in inputstream.adaptive/addon.xml.in should be considered to be a stable release. As a result, I'd remove all the snapshot stuff in Release:
    Version:        1.0.6
    Release:        1%{?dist}
This is a simple suggestion anyway.

Done -
https://repo.dcrdev.com/fc26/specfiles/kodi-inputstream-adaptive.spec
https://repo.dcrdev.com/fc26/SRPMS/kodi-inputstream-adaptive-1.0.6-0.1.20161215gitcd26d5d.fc26.src.rpm
https://repo.dcrdev.com/fc26/x86_64/kodi-inputstream-adaptive-1.0.6-0.1.20161215gitcd26d5d.fc26.x86_64.rpm
https://repo.dcrdev.com/fc26/x86_64/debug/kodi-inputstream-adaptive-debuginfo-1.0.6-0.1.20161215gitcd26d5d.fc26.x86_64.rpm

Regarding the git snapshot - upstream keep the version in the xml static across commits, therefore to me I think that would make it difficult to differentiate between package versions. Additionally I'm picking the commit based one what Kodi are shipping as part of Kodiubuntu.

(In reply to Mohamed El Morabity from comment #19)
> Sorry for the late answer. A lot of work at my job unfortunately.
> 
> Your package seems quite good now. Some notes:
> 
> - Fix BR condition on kodi-devel, which should match the one on kodi in
> Requires:
>     BuildRequires:  kodi-devel >= %{kodi_version}
It would means the project itslef requires at least this kodi version whereas it might be possible to build for several kodi version (say 16 and 17 or later).
What would be needed instead if to enforce a build to be at least >= than the version of kodi it was built against.
So it means it would be easier if koji_version RPM macro would be defined in the kodi-devel package so dependent package could use it.

Reviewing the package.

Ping ?

I don't see any problem with that per say , but I know for a fact that this can only be built against kodi >= 17; for future versions sure.

So to clear up my limited understanding of koji, how would this work exactly? If a koji_version macro were to be defined in the kodi-devel package, how would that get passed to this package?

Is it a case of:
- Dependent packages are being built in a single chroot a la mock chain.
- Therefore once koji_version is set, it is exported before all subsequent builds within that chroot.

(In reply to Dominic Robinson from comment #23)
> I don't see any problem with that per say , but I know for a fact that this
> can only be built against kodi >= 17; for future versions sure.
> 
> So to clear up my limited understanding of koji, how would this work
> exactly? If a koji_version macro were to be defined in the kodi-devel
> package, how would that get passed to this package?
Sorry, I meant a %kodi_version macro. 

It would be defined in the kodi-devel package (
echo %kodi_version %{version} > %{buildroot}%{rpmmacrodir}/macros.kodi
)

Then when plugins would build, they would have:
Requires: kodi >= %{kodi_version}


Anyway, it seems a minor issue and not particular to this plugins. this can be fixed and coordinated later. 

Do you plan to fix the name with modified macro ? This looks really scary.

I hadn't planned on changing it -

I mean might be a problem if I reuse this spec for other addons, but I don't see an issue in the context of this addon and makes things neater.

Also bit confused - is this package approved, I note the fedora-review + flag and also the repository having been created here: https://admin.rpmfusion.org/pkgdb/package/free/kodi-inputstream-adaptive/ .

Wasn't I suppose to be the one who requests the package, as it stands I'm not the maintainer of that repository?

(In reply to Dominic Robinson from comment #25)
> I hadn't planned on changing it -
> 
> I mean might be a problem if I reuse this spec for other addons, but I don't
> see an issue in the context of this addon and makes things neater.

Well, this is not the case from a RPM Fusion rel-eng perspective.
If ever the aname is changed, which might happen if the package get fixed , the package name will be automatically changed. This is not possible with the infra where koji expect the spec file to be named according to the git module name.
As a consequence, the build will fails, but it may requires additional steps to cure this issue.

For example, upstream may consider to change inputstream.adaptive to inputstream_adaptive. So the point of using a variable in this case doesn't hold.

(In reply to Dominic Robinson from comment #26)
> Also bit confused - is this package approved, I note the fedora-review +
> flag and also the repository having been created here:
Yep, seems like this package was mass imported along the other ones from Mohamed.
I will transfer the ownership to you once this package got formally approved
can you remind your RPM Fusion FAS account name ?

(In reply to Nicolas Chauvet from comment #28)
> (In reply to Dominic Robinson from comment #26)
> > Also bit confused - is this package approved, I note the fedora-review +
> > flag and also the repository having been created here:
> Yep, seems like this package was mass imported along the other ones from
> Mohamed.
> I will transfer the ownership to you once this package got formally approved
> can you remind your RPM Fusion FAS account name ?

Yes it's "dcrdev" .

(In reply to Nicolas Chauvet from comment #27)
> (In reply to Dominic Robinson from comment #25)
> > I hadn't planned on changing it -
> > 
> > I mean might be a problem if I reuse this spec for other addons, but I don't
> > see an issue in the context of this addon and makes things neater.
> 
> Well, this is not the case from a RPM Fusion rel-eng perspective.
> If ever the aname is changed, which might happen if the package get fixed ,
> the package name will be automatically changed. This is not possible with
> the infra where koji expect the spec file to be named according to the git
> module name.
> As a consequence, the build will fails, but it may requires additional steps
> to cure this issue.
> 
> For example, upstream may consider to change inputstream.adaptive to
> inputstream_adaptive. So the point of using a variable in this case doesn't
> hold.

OK makes sense - I'll hardcode it to "kodi-inputstream-adaptive" . I'll kick off a build this evening, as I don't have access to my build system right now.

okay, so this package is formally APPROVED.

I will give the ownership to you.

It doesn't look like the git server has my public key key authorised? Does this take a while?

debug1: Host 'pkgs.rpmfusion.org' is known and matches the RSA host key.
debug1: Found key in /home/dcrdev/.ssh/known_hosts:1
debug3: send packet: type 21
debug2: set_newkeys: mode 1
debug1: rekey after 4294967296 blocks
debug1: SSH2_MSG_NEWKEYS sent
debug1: expecting SSH2_MSG_NEWKEYS
debug3: receive packet: type 21
debug1: SSH2_MSG_NEWKEYS received
debug2: set_newkeys: mode 0
debug1: rekey after 4294967296 blocks
debug2: key: /home/dcrdev/.ssh/id_rsa (0x55e5989f52b0), agent
debug2: key: /home/dcrdev/.ssh/id_dsa ((nil))
debug2: key: /home/dcrdev/.ssh/id_ecdsa ((nil))
debug2: key: /home/dcrdev/.ssh/id_ed25519 ((nil))
debug3: send packet: type 5
debug3: receive packet: type 6
debug2: service_accept: ssh-userauth
debug1: SSH2_MSG_SERVICE_ACCEPT received
debug3: send packet: type 50
debug3: receive packet: type 51
debug1: Authentications that can continue: publickey
debug3: start over, passed a different list publickey
debug3: preferred gssapi-keyex,gssapi-with-mic,publickey,keyboard-interactive,password
debug3: authmethod_lookup publickey
debug3: remaining preferred: keyboard-interactive,password
debug3: authmethod_is_enabled publickey
debug1: Next authentication method: publickey
debug1: Offering RSA public key: /home/dcrdev/.ssh/id_rsa
debug3: send_pubkey_test
debug3: send packet: type 50
debug2: we sent a publickey packet, wait for reply
debug3: receive packet: type 51
debug1: Authentications that can continue: publickey
debug1: Trying private key: /home/dcrdev/.ssh/id_dsa
debug3: no such identity: /home/dcrdev/.ssh/id_dsa: No such file or directory
debug1: Trying private key: /home/dcrdev/.ssh/id_ecdsa
debug3: no such identity: /home/dcrdev/.ssh/id_ecdsa: No such file or directory
debug1: Trying private key: /home/dcrdev/.ssh/id_ed25519
debug3: no such identity: /home/dcrdev/.ssh/id_ed25519: No such file or directory
debug2: we did not send a packet, disable method
debug1: No more authentication methods to try.
Permission denied (publickey).

(In reply to Dominic Robinson from comment #31)
> It doesn't look like the git server has my public key key authorised? Does
> this take a while?
Fixed by forcing genacls.sh script

Sadly still not liking it:

debug1: Offering RSA public key: /home/dcrdev/.ssh/id_rsa
debug3: send_pubkey_test
debug3: send packet: type 50
debug2: we sent a publickey packet, wait for reply
debug3: receive packet: type 51
debug1: Authentications that can continue: publickey
debug1: Trying private key: /home/dcrdev/.ssh/id_dsa
debug3: no such identity: /home/dcrdev/.ssh/id_dsa: No such file or directory
debug1: Trying private key: /home/dcrdev/.ssh/id_ecdsa
debug3: no such identity: /home/dcrdev/.ssh/id_ecdsa: No such file or directory
debug1: Trying private key: /home/dcrdev/.ssh/id_ed25519
debug3: no such identity: /home/dcrdev/.ssh/id_ed25519: No such file or directory
debug2: we did not send a packet, disable method
debug1: No more authentication methods to try.
Permission denied (publickey).

Okay, this time it was the Fas sync script that didn't worked.
fixed.

Ok thanks - working now.

So I've pushed to all branches, requested builds for those branches and those builds have succeeded:

http://koji.rpmfusion.org/koji/packageinfo?packageID=508

Is that all I need to do? I note they are not listed here, as suggested by the documentation:
http://koji.rpmfusion.org/mash/

(In reply to Dominic Robinson from comment #35)
...
> Is that all I need to do? I note they are not listed here, as suggested by
> the documentation:
I try to push packages 2/3 times a week.
First time the package goes into testing, the second time into stable.
If there is anything broken, please warn.