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 1284527 - Review Request: opal-prd - OPAL Processor Recovery Diagnostics daemon
Summary: Review Request: opal-prd - OPAL Processor Recovery Diagnostics daemon
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: ppc64le
OS: Linux
unspecified
high
Target Milestone: ---
Assignee: Dan Horák
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: PPCTracker F-ExcludeArch-ppc64le, PPC64LETracker 1312018
TreeView+ depends on / blocked
 
Reported: 2015-11-23 14:22 UTC by Vasant Hegde
Modified: 2016-03-17 09:56 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-03-17 09:56:09 UTC
Type: Bug
Embargoed:
dan: fedora-review+


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
IBM Linux Technology Center 133831 0 None None None Never

Description Vasant Hegde 2015-11-23 14:22:54 UTC
Description of problem:
 We have new package to provides diagnostics/recovery on OpenPower system. We want to include these packages in Fedora ppc64le.

opal-prd : 
 is a daemon that listens for hardware diagnostic events (by listening on the /dev/opal-prd device), and executes firmware-provided executable code to handle these events.


opal-utils:
 contains user commands related to diagnostic. For the moment, only the gard command but we might add more.

Upstream tree:
 All these code is part of skiboot project.. which contains FW code for OpenPower system
 https://github.com/open-power/skiboot/tree/master/external/opal-prd.

Comment 1 Vasant Hegde 2015-11-23 14:24:32 UTC
I will build fedora package and post link here later this week.

-Vasant

Comment 2 Dan Horák 2015-11-23 14:48:51 UTC
adding to Fedora/ppc tracker

Comment 4 Vasant Hegde 2015-11-23 17:04:19 UTC
rpmlint output :

rpmlint -v opal-prd-5.1.11-1.fc22.src.rpm
opal-prd.src: I: checking
opal-prd.src: I: checking-url http://github.com/open-power/skiboot (timeout 10 seconds)
opal-prd.src: I: checking-url https://github.com/open-power/skiboot/archive/skiboot-5.1.11.tar.gz (timeout 10 seconds)
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 5 Dan Horák 2015-11-23 17:37:54 UTC
%define kversion `uname -r`
won't work in the buildsystem (or even just in mock), the kernel running is different from the installed kernel-devel package, I think
KERNEL_DIR=/usr/src/kernels/*
will work, but looking for a better solution.

Comment 6 Vasant Hegde 2015-11-23 17:47:49 UTC
Dan,

(In reply to Dan Horák from comment #5)
> %define kversion `uname -r`
> won't work in the buildsystem (or even just in mock), the kernel running is
> different from the installed kernel-devel package, I think
> KERNEL_DIR=/usr/src/kernels/*
> will work, but looking for a better solution.

Thanks for the quick review.. I will fix it.

Also I found couple of other minor issues.. Will fix them and post new version tomorrow.

-Vasant

Comment 7 Vasant Hegde 2015-11-24 05:00:09 UTC
Dan,

(In reply to Dan Horák from comment #5)
> %define kversion `uname -r`
> won't work in the buildsystem (or even just in mock), the kernel running is
> different from the installed kernel-devel package, I think
> KERNEL_DIR=/usr/src/kernels/*
> will work, but looking for a better solution.

We use kernel provided "opal-prd.h" header file while building opal-prd [1]. Hence we need kernel version in build environment. And we don't need this while running binary. 

Hence I'm keeping above define as is. Let me know if there is better way to do it.

[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0d7cd8550d30906c7461ced654306da30f1590e2

-Vasant

Comment 8 Vasant Hegde 2015-11-24 05:03:12 UTC
v2:
 In v1, I missed to add systemd as Build Requirement. Hence build failed in mock environment. I've fixed that issue and now I'm able to build rpm in mock build environment.


Spec URL: https://hegdevasant.fedorapeople.org/opal-prd/v2/opal-prd.spec
SRPM: https://hegdevasant.fedorapeople.org/opal-prd/v2/opal-prd-5.1.11-1.fc22.src.rpm

-Vasant

Comment 9 Vasant Hegde 2015-11-24 05:28:21 UTC
Koji build : http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=2930041

-Vasant

Comment 11 Vasant Hegde 2015-11-24 06:49:18 UTC
I ran fedora-review tool on this bugzilla and here is the output :

cat rpmlint.txt 
Checking: opal-prd-5.1.11-2.fc22.ppc64.rpm
          opal-utils-5.1.11-2.fc22.ppc64.rpm
          opal-prd-5.1.11-2.fc22.src.rpm
opal-utils.ppc64: W: spelling-error %description -l en_US gard -> grad, hard, gars
3 packages and 0 specfiles checked; 0 errors, 1 warnings.

gard is the tool name. Hence I think we can ignore this warning.

-Vasant

Comment 12 Vasant Hegde 2015-11-24 08:09:30 UTC
Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- Package installs properly.
  Note: Installation errors (see attachment)
  See: https://fedoraproject.org/wiki/Packaging:Guidelines
- Package uses either %{buildroot} or $RPM_BUILD_ROOT
  Note: Using both %{buildroot} and $RPM_BUILD_ROOT
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#macros
- 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.
  Note: License file LICENCE is marked as %doc instead of %license
  See:
  http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text


===== MUST items =====

C/C++:
[ ]: Package does not contain kernel modules.
[ ]: Package contains no static executables.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[ ]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "Apache (v2.0)", "BSD (2 clause) GPL (v2 or later)", "Unknown
     or generated". 181 files have unknown license. Detailed output of
     licensecheck in /home/mock/1284527-opal-prd/licensecheck.txt
[ ]: License file installed when any subpackage combination is installed.
[ ]: %build honors applicable compiler flags or justifies otherwise.
[ ]: Package contains no bundled libraries without FPC exception.
[ ]: Changelog in prescribed format.
[ ]: Sources contain only permissible code or content.
[ ]: Each %files section contains %defattr if rpm < 4.4
     Note: %defattr present but not needed
[ ]: Package contains desktop file if it is a GUI application.
[ ]: Development files must be in a -devel package
[ ]: Package uses nothing in %doc for runtime.
[ ]: Package consistently uses macros (instead of hard-coded directory
     names).
[ ]: Package is named according to the Package Naming Guidelines.
[ ]: Package does not generate any conflict.
[ ]: Package obeys FHS, except libexecdir and /usr/target.
[ ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: Requires correct, justified where necessary.
[ ]: Spec file is legible and written in American English.
[ ]: Package contains systemd file(s) if in need.
[ ]: Useful -debuginfo package or justification otherwise.
[ ]: Package is not known to require an ExcludeArch tag.
[ ]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 40960 bytes in 4 files.
[ ]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[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]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[!]: Uses parallel make %{?_smp_mflags} macro.
[ ]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[ ]: Final provides and requires are sane (see attachments).
[ ]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in opal-
     utils
[ ]: Package functions as described.
[ ]: Latest version is packaged.
[ ]: Package does not include license text files separate from upstream.
[ ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: %check is present and all tests pass.
[ ]: Packages should try to preserve timestamps of original installed
     files.
[ ]: Spec use %global instead of %define unless justified.
     Note: %define requiring justification: %define kversion `uname -r`
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[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]: SourceX is a working URL.
[x]: Package should compile and build into binary rpms on all supported
     architectures.

===== EXTRA items =====

Generic:
[!]: Rpmlint is run on all installed packages.
     Note: Mock build failed
     See: http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint
[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.


Installation errors
-------------------
INFO: mock.py version 1.2.8 starting (python version = 3.4.2)...
Start: init plugins
INFO: selinux enabled
Finish: init plugins
Start: run
Start: chroot init
INFO: calling preinit hooks
INFO: enabled root cache
INFO: enabled yum cache
Start: cleaning yum metadata
Finish: cleaning yum metadata
INFO: enabled ccache
Mock Version: 1.2.8
INFO: Mock Version: 1.2.8
Finish: chroot init
INFO: installing package(s): /home/mock/1284527-opal-prd/results/opal-prd-5.1.11-2.fc22.ppc64.rpm /home/mock/1284527-opal-prd/results/opal-utils-5.1.11-2.fc22.ppc64.rpm /home/mock/1284527-opal-prd/results/opal-prd-debuginfo-5.1.11-2.fc22.ppc64.rpm
ERROR: Command failed. See logs for output.
 # /usr/bin/yum-deprecated --installroot /var/lib/mock/fedora-22-ppc64/root/ --releasever 22 install /home/mock/1284527-opal-prd/results/opal-prd-5.1.11-2.fc22.ppc64.rpm /home/mock/1284527-opal-prd/results/opal-utils-5.1.11-2.fc22.ppc64.rpm /home/mock/1284527-opal-prd/results/opal-prd-debuginfo-5.1.11-2.fc22.ppc64.rpm --setopt=tsflags=nocontexts


Rpmlint
-------
Checking: opal-prd-5.1.11-2.fc22.ppc64.rpm
          opal-utils-5.1.11-2.fc22.ppc64.rpm
          opal-prd-5.1.11-2.fc22.src.rpm
opal-utils.ppc64: W: spelling-error %description -l en_US gard -> grad, hard, gars
3 packages and 0 specfiles checked; 0 errors, 1 warnings.




Requires
--------
opal-utils (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    rtld(GNU_HASH)

opal-prd (rpmlib, GLIBC filtered):
    /bin/sh
    kernel
    libc.so.6()(64bit)
    rtld(GNU_HASH)
    systemd



Provides
--------
opal-utils:
    opal-utils
    opal-utils(ppc-64)

opal-prd:
    opal-prd
    opal-prd(ppc-64)



Source checksums
----------------
https://github.com/open-power/skiboot/archive/skiboot-5.1.11.tar.gz :
  CHECKSUM(SHA256) this package     : 65017a81a2f2cb225bdf3d6081117ba4cbc1688cec2d572586dc349c53e48fbb
  CHECKSUM(SHA256) upstream package : 65017a81a2f2cb225bdf3d6081117ba4cbc1688cec2d572586dc349c53e48fbb


Generated by fedora-review 0.5.3 (bcf15e3) last change: 2015-05-04
Command line :/bin/fedora-review -b 1284527
Buildroot used: fedora-22-ppc64
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

Comment 13 Vasant Hegde 2015-11-24 08:15:29 UTC
> Issues:
> =======
> - Package installs properly.
>   Note: Installation errors (see attachment)
>   See: https://fedoraproject.org/wiki/Packaging:Guidelines

I'm using Fedora 22 and for some reason dnf failed with below error. Hence I got installation failure. I've verified it and installation works fine.

Failed to synchronize cache for repo '_local' from 'file:///var/lib/dnf/plugins/local': Cannot download repomd.xml: Cannot download repodata/repomd.xml: All mirrors were tried, disabling.


> - Package uses either %{buildroot} or $RPM_BUILD_ROOT
>   Note: Using both %{buildroot} and $RPM_BUILD_ROOT
>   See: http://fedoraproject.org/wiki/Packaging/Guidelines#macros

Yes. I've used mixed style. I will fix this is next version.

> - 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.
>   Note: License file LICENCE is marked as %doc instead of %license
>   See:
>   http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text

Yep :-( I will move license under %license in next version.


IMO this package passed both "MUST" and "SHOUD" items.  Let me know if I missed anything.

> 
> Installation errors
> -------------------
> INFO: mock.py version 1.2.8 starting (python version = 3.4.2)...
> Start: init plugins
> INFO: selinux enabled
> Finish: init plugins
> Start: run
> Start: chroot init
> INFO: calling preinit hooks
> INFO: enabled root cache
> INFO: enabled yum cache
> Start: cleaning yum metadata
> Finish: cleaning yum metadata
> INFO: enabled ccache
> Mock Version: 1.2.8
> INFO: Mock Version: 1.2.8
> Finish: chroot init
> INFO: installing package(s):
> /home/mock/1284527-opal-prd/results/opal-prd-5.1.11-2.fc22.ppc64.rpm
> /home/mock/1284527-opal-prd/results/opal-utils-5.1.11-2.fc22.ppc64.rpm
> /home/mock/1284527-opal-prd/results/opal-prd-debuginfo-5.1.11-2.fc22.ppc64.
> rpm
> ERROR: Command failed. See logs for output.
>  # /usr/bin/yum-deprecated --installroot /var/lib/mock/fedora-22-ppc64/root/
> --releasever 22 install
> /home/mock/1284527-opal-prd/results/opal-prd-5.1.11-2.fc22.ppc64.rpm
> /home/mock/1284527-opal-prd/results/opal-utils-5.1.11-2.fc22.ppc64.rpm
> /home/mock/1284527-opal-prd/results/opal-prd-debuginfo-5.1.11-2.fc22.ppc64.
> rpm --setopt=tsflags=nocontexts

As mentioned earlier this is my environment issue.  I don't think it will fail in real system.

> 
> 
> Rpmlint
> -------
> Checking: opal-prd-5.1.11-2.fc22.ppc64.rpm
>           opal-utils-5.1.11-2.fc22.ppc64.rpm
>           opal-prd-5.1.11-2.fc22.src.rpm
> opal-utils.ppc64: W: spelling-error %description -l en_US gard -> grad,
> hard, gars
> 3 packages and 0 specfiles checked; 0 errors, 1 warnings.
> 

I think we can ignore this warning.


-Vasant

Comment 16 Dan Horák 2015-11-24 10:59:25 UTC
- distro-wide CFLAGS are not honoured in the build - https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
- make the build of opal-prd verbose so the full command lines are visible (add V=1)
- you should drop the "$RPM_BUILD_DIR/skiboot-skiboot-%version/" string from the make commands, it's the default dir for rpm builds (see build.log)
- I think you don't need kernel-devel and playing with the KERNEL_DIR at all, the asm/opal-prd.h file is part of the kernel-headers package which is installed together with glibc-headers as it is a public API (http://ppc.koji.fedoraproject.org/koji/fileinfo?rpmID=2562590&filename=/usr/include/asm/opal-prd.h)

Comment 17 Dan Horák 2015-11-24 11:12:15 UTC
one more question - is the separation between opal-prd and opal-utils necessary? Won't be these 2 packages always both installed? Or is the daemon accessible remotely?

Comment 18 Vasant Hegde 2015-11-25 07:04:25 UTC
Dan,

Thanks for the review.

(In reply to Dan Horák from comment #16)
> - distro-wide CFLAGS are not honoured in the build -
> https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

Added "%{?_smp_mflags}"

> - make the build of opal-prd verbose so the full command lines are visible
> (add V=1)

Fixed.

> - you should drop the "$RPM_BUILD_DIR/skiboot-skiboot-%version/" string from
> the make commands, it's the default dir for rpm builds (see build.log)

Fixed.

> - I think you don't need kernel-devel and playing with the KERNEL_DIR at
> all, the asm/opal-prd.h file is part of the kernel-headers package which is
> installed together with glibc-headers as it is a public API
> (http://ppc.koji.fedoraproject.org/koji/fileinfo?rpmID=2562590&filename=/usr/
> include/asm/opal-prd.h)

You are right. I will fix this. Also I've removed `kernel` from "Requires" tag as its installed by default.


-Vasant

Comment 19 Vasant Hegde 2015-11-25 08:41:16 UTC
(In reply to Dan Horák from comment #17)
> one more question - is the separation between opal-prd and opal-utils
> necessary? 

opal-prd contains recovery and diagnostics daemon.. Needs to be installed by default

opal-utils contains utilities required for debugging purpose.. Mostly used by developers. We don't want this to be installed by default.

Hence we have create two separate packages.

> Won't be these 2 packages always both installed? 

As mentioned above opal-prd is installed always , opal-utils is optional.

> Or is the daemon  accessible remotely?

No.. its per host daemon.. 

-Vasant

Comment 20 Vasant Hegde 2015-11-25 11:23:25 UTC
V4:
 Addressed review comments from Dan.

Spec URL : https://hegdevasant.fedorapeople.org/opal-prd/v4/opal-prd.spec
SRPM : https://hegdevasant.fedorapeople.org/opal-prd/v4/opal-prd-5.1.11-4.fc22.src.rpm

-Vasant

Comment 21 Dan Horák 2016-01-18 10:06:22 UTC
sorry for the delay, but formal review is here, see the notes explaining OK* and BAD statuses below:

OK      source files match upstream:
            080a0992dc4241ac0c8c2b7e556a20bbb45d068d  skiboot-5.1.11.tar.gz
OK      package meets naming and versioning guidelines.
OK      specfile is properly named, is cleanly written and uses macros consistently.
OK      dist tag is present.
OK      license field matches the actual license.
OK      license is open source-compatible (ASL-2.0). License text included in package.
OK*     latest version is being packaged.
OK      BuildRequires are proper.
BAD     compiler flags are appropriate.
OK      package builds in mock (Rawhide/ppc64 + ppc64le).
OK      debuginfo package looks complete.
OK      rpmlint is silent.
OK      final provides and requires look sane.
N/A     %check is present and all tests pass.
OK      no shared libraries are added to the regular linker search paths.
OK      owns the directories it creates.
OK      doesn't own any directories it shouldn't.
OK      no duplicates in %files.
OK      file permissions are appropriate.
BAD     correct systemd scriptlets present.
OK      code, not content.
OK      documentation is small, so no -docs subpackage is necessary.
OK      %docs are not necessary for the proper functioning of the package.
OK      no headers.
OK      no pkgconfig files.
OK      no libtool .la droppings.
OK      not a GUI app.

- was up-to-date when updating the review ticket, please update to 5.1.12 for the next iteration
- distro-wide CFLAGS are not used, try setting CFLAGS="%{optflags}" for the make call in %build, see https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
- my scratch build = http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3077491
- please update the scriptlets and Requires to the current style, see https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd

Comment 22 IBM Bug Proxy 2016-01-22 10:11:30 UTC
------- Comment From hegdevasant.com 2016-01-22 05:02 EDT-------
(In reply to comment #5)
> sorry for the delay, but formal review is here, see the notes explaining OK*
> and BAD statuses below:
> OK      source files match upstream:
> 080a0992dc4241ac0c8c2b7e556a20bbb45d068d  skiboot-5.1.11.tar.gz
> OK      package meets naming and versioning guidelines.
> OK      specfile is properly named, is cleanly written and uses macros
> consistently.
> OK      dist tag is present.
> OK      license field matches the actual license.
> OK      license is open source-compatible (ASL-2.0). License text included
> in package.
> OK*     latest version is being packaged.
> OK      BuildRequires are proper.
> BAD     compiler flags are appropriate.
> OK      package builds in mock (Rawhide/ppc64 + ppc64le).
> OK      debuginfo package looks complete.
> OK      rpmlint is silent.
> OK      final provides and requires look sane.
> N/A     %check is present and all tests pass.
> OK      no shared libraries are added to the regular linker search paths.
> OK      owns the directories it creates.
> OK      doesn't own any directories it shouldn't.
> OK      no duplicates in %files.
> OK      file permissions are appropriate.
> BAD     correct systemd scriptlets present.
> OK      code, not content.
> OK      documentation is small, so no -docs subpackage is necessary.
> OK      %docs are not necessary for the proper functioning of the package.
> OK      no headers.
> OK      no pkgconfig files.
> OK      no libtool .la droppings.
> OK      not a GUI app.
> - was up-to-date when updating the review ticket, please update to 5.1.12
> for the next iteration
> - distro-wide CFLAGS are not used, try setting CFLAGS="%{optflags}" for the
> make call in %build, see
> - my scratch build =
> http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3077491
> - please update the scriptlets and Requires to the current style, see
> https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd

Thanks for the review.. I will fix them and roll out new version next week.

Comment 23 Vasant Hegde 2016-02-09 11:15:03 UTC
V5:
  - Rebased to latest upstream version (v5.1.13)
  - Updated specfile to include pflash and scom tools in opal-utils package
  - Updated specfile to build opal-firmware package
  

opal-firmware:
  Upstream decided to create 3 packages out of OPAL source code. Hence I've added this package now. Its a noarch package contains our firmware code in big endian mode. We can use this code in qemu [1] 

https://www.flamingspork.com/blog/2015/08/28/running-opal-in-qemu-the-powernv-platform/

> - was up-to-date when updating the review ticket, please update to 5.1.12
> for the next iteration
> - distro-wide CFLAGS are not used, try setting CFLAGS="%{optflags}" for the
> make call in %build, see

I've added above flag except one place where we always want to build big endian firmware.

> - my scratch build =
> http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3077491
> - please update the scriptlets and Requires to the current style, see
> https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd

I've added. But looks like its not starting daemon automatically. Is there a way to start daemon as soon as we install it?
(systemd_post opal-prd.service -> calls systemctl preset)


Spec URL : https://hegdevasant.fedorapeople.org/opal-prd/v5/opal-prd.spec
SRPM : https://hegdevasant.fedorapeople.org/opal-prd/v5/opal-prd-5.1.13-1.fc22.src.rpm

koji build : http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3124995


-Vasant

Comment 24 Dan Horák 2016-02-19 13:31:07 UTC
(In reply to Vasant Hegde from comment #23)
> V5:
>   - Rebased to latest upstream version (v5.1.13)
>   - Updated specfile to include pflash and scom tools in opal-utils package
>   - Updated specfile to build opal-firmware package
>   
> 
> opal-firmware:
>   Upstream decided to create 3 packages out of OPAL source code. Hence I've
> added this package now. Its a noarch package contains our firmware code in
> big endian mode. We can use this code in qemu [1] 

OK
 
> https://www.flamingspork.com/blog/2015/08/28/running-opal-in-qemu-the-
> powernv-platform/
> 
> > - was up-to-date when updating the review ticket, please update to 5.1.12
> > for the next iteration
> > - distro-wide CFLAGS are not used, try setting CFLAGS="%{optflags}" for the
> > make call in %build, see
> 
> I've added above flag except one place where we always want to build big
> endian firmware.

no problem, makes sense, the distro wide flags are meant for regular binaries/libs, firmwares can have different requirements

> > - my scratch build =
> > http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3077491
> > - please update the scriptlets and Requires to the current style, see
> > https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd
> 
> I've added. But looks like its not starting daemon automatically. Is there a
> way to start daemon as soon as we install it?
> (systemd_post opal-prd.service -> calls systemctl preset)

Still missing are the Requires(post|preun|postun) tags mentioned in the guideline, they are required for the scriptlets to work.

The systemd guideline links to https://fedoraproject.org/wiki/Packaging:DefaultServices?rd=Starting_services_by_default for how to handle default state of services. But we can solve the default behaviour after including the package in the distro.

Comment 25 Vasant Hegde 2016-02-19 14:25:47 UTC
Dan,

> > > - my scratch build =
> > > http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3077491
> > > - please update the scriptlets and Requires to the current style, see
> > > https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd
> > 
> > I've added. But looks like its not starting daemon automatically. Is there a
> > way to start daemon as soon as we install it?
> > (systemd_post opal-prd.service -> calls systemctl preset)
> 
> Still missing are the Requires(post|preun|postun) tags mentioned in the
> guideline, they are required for the scriptlets to work.

Do you mean below tags ?

Requires(post): systemd
Requires(preun): systemd
Requires(postun): systemd


> 
> The systemd guideline links to
> https://fedoraproject.org/wiki/Packaging:
> DefaultServices?rd=Starting_services_by_default for how to handle default
> state of services. But we can solve the default behaviour after including
> the package in the distro.

Ok. Thanks. I will look into above link .

-Vasant

Comment 27 Vasant Hegde 2016-02-22 09:59:50 UTC
Dan,

> 
> The systemd guideline links to
> https://fedoraproject.org/wiki/Packaging:
> DefaultServices?rd=Starting_services_by_default for how to handle default
> state of services. But we can solve the default behaviour after including
> the package in the distro.

Looks like we have to update 90-default.preset script in systemd. Do I need to raise separate bug after including this package?

-Vasant

Comment 28 Dan Horák 2016-02-22 10:26:59 UTC
(In reply to Vasant Hegde from comment #27)
> Dan,
> 
> > 
> > The systemd guideline links to
> > https://fedoraproject.org/wiki/Packaging:
> > DefaultServices?rd=Starting_services_by_default for how to handle default
> > state of services. But we can solve the default behaviour after including
> > the package in the distro.
> 
> Looks like we have to update 90-default.preset script in systemd. Do I need
> to raise separate bug after including this package?

I would say YES

Comment 29 Dan Horák 2016-02-22 10:30:19 UTC
(In reply to Vasant Hegde from comment #26)
> V6:
> 
> Chnages:
> Added "Requires(post|preun|postun) tags"
> 
> Spec URL : https://hegdevasant.fedorapeople.org/opal-prd/v6/opal-prd.spec
> SRPM :
> https://hegdevasant.fedorapeople.org/opal-prd/v6/opal-prd-5.1.13-2.fc22.src.
> rpm
> 
> 
> Koji build : http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3159601

looks good, but I have found that opal-prd is being recompiled in the %install section

from the build.log
...
Executing(%install): /bin/sh -e /var/tmp/rpm-tmp.d4xSif
+ umask 022
+ cd /builddir/build/BUILD
+ '[' /builddir/build/BUILDROOT/opal-prd-5.1.13-2.fc23.ppc64 '!=' / ']'
+ rm -rf /builddir/build/BUILDROOT/opal-prd-5.1.13-2.fc23.ppc64
++ dirname /builddir/build/BUILDROOT/opal-prd-5.1.13-2.fc23.ppc64
+ mkdir -p /builddir/build/BUILDROOT
+ mkdir /builddir/build/BUILDROOT/opal-prd-5.1.13-2.fc23.ppc64
+ cd skiboot-skiboot-5.1.13
+ make -C external/opal-prd install DESTDIR=/builddir/build/BUILDROOT/opal-prd-5.1.13-2.fc23.ppc64 prefix=/usr
make: Entering directory '/builddir/build/BUILD/skiboot-skiboot-5.1.13/external/opal-prd'
    CC  opal-prd.o
    CC  thunk.o
    CC  pnor.o
    CC  i2c.o
    CC  module.o
    CC  version.o
    CC  libflash-blocklevel.o
    CC  libflash-libffs.o
    CC  libflash-libflash.o
    CC  libflash-ecc.o
    CC  libflash-file.o
ld -r common-arch_flash_common.o common-arch_flash_powerpc.o -o common-arch_flash.o
  LINK  opal-prd
install -D opal-prd /builddir/build/BUILDROOT/opal-prd-5.1.13-2.fc23.ppc64/usr/sbin/opal-prd
...


It can happen when some variables set for make for %build are not copied to make in %install (or vice versa) ...

Comment 30 Vasant Hegde 2016-02-23 14:59:14 UTC
(In reply to Dan Horák from comment #28)
> (In reply to Vasant Hegde from comment #27)
> > Dan,
> > 
> > > 
> > > The systemd guideline links to
> > > https://fedoraproject.org/wiki/Packaging:
> > > DefaultServices?rd=Starting_services_by_default for how to handle default
> > > state of services. But we can solve the default behaviour after including
> > > the package in the distro.
> > 
> > Looks like we have to update 90-default.preset script in systemd. Do I need
> > to raise separate bug after including this package?
> 
> I would say YES

Ok Thanks. I will create separate bug once we integrate opal-prd package.

-Vasant

Comment 31 Vasant Hegde 2016-02-23 15:00:19 UTC
Dan,

> looks good, but I have found that opal-prd is being recompiled in the
> %install section
> 
> from the build.log
> ...
> Executing(%install): /bin/sh -e /var/tmp/rpm-tmp.d4xSif
> + umask 022
> + cd /builddir/build/BUILD
> + '[' /builddir/build/BUILDROOT/opal-prd-5.1.13-2.fc23.ppc64 '!=' / ']'
> + rm -rf /builddir/build/BUILDROOT/opal-prd-5.1.13-2.fc23.ppc64
> ++ dirname /builddir/build/BUILDROOT/opal-prd-5.1.13-2.fc23.ppc64
> + mkdir -p /builddir/build/BUILDROOT
> + mkdir /builddir/build/BUILDROOT/opal-prd-5.1.13-2.fc23.ppc64
> + cd skiboot-skiboot-5.1.13
> + make -C external/opal-prd install
> DESTDIR=/builddir/build/BUILDROOT/opal-prd-5.1.13-2.fc23.ppc64 prefix=/usr
> make: Entering directory
> '/builddir/build/BUILD/skiboot-skiboot-5.1.13/external/opal-prd'
>     CC  opal-prd.o
>     CC  thunk.o
>     CC  pnor.o
>     CC  i2c.o
>     CC  module.o
>     CC  version.o
>     CC  libflash-blocklevel.o
>     CC  libflash-libffs.o
>     CC  libflash-libflash.o
>     CC  libflash-ecc.o
>     CC  libflash-file.o
> ld -r common-arch_flash_common.o common-arch_flash_powerpc.o -o
> common-arch_flash.o
>   LINK  opal-prd
> install -D opal-prd
> /builddir/build/BUILDROOT/opal-prd-5.1.13-2.fc23.ppc64/usr/sbin/opal-prd
> ...
> 
> 
> It can happen when some variables set for make for %build are not copied to
> make in %install (or vice versa) ...

This is because of minor issue in our Makefile. I will fix and post v7 soon.

-Vasant

Comment 32 Vasant Hegde 2016-02-23 15:04:18 UTC
V7:

Changes in v7:
  Fix opal-prd recompilation issse during install

Spec URL : https://hegdevasant.fedorapeople.org/opal-prd/v7/opal-prd.spec
SRPM : https://hegdevasant.fedorapeople.org/opal-prd/v7/opal-prd-5.1.13-7.fc22.src.rpm


Koji build : http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3165099

-Vasant

Comment 33 Dan Horák 2016-02-25 11:43:37 UTC
thanks, the package is APPROVED

Comment 34 Vasant Hegde 2016-02-25 14:20:54 UTC
(In reply to Dan Horák from comment #33)
> thanks, the package is APPROVED

Thanks Dan. I will raise "SCM admin request" to include this package.

-Vasant

Comment 35 Gwyn Ciesla 2016-02-25 19:45:51 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/opal-prd

Comment 36 Vasant Hegde 2016-02-26 07:52:03 UTC
(In reply to Jon Ciesla from comment #35)
> Package request has been approved:
> https://admin.fedoraproject.org/pkgdb/package/opal-prd

Thanks!

I'm able to import initial package and build it.

-Vasant

Comment 37 Vasant Hegde 2016-02-26 07:52:47 UTC
Dan,

I've imported package and built it for F24 branch [1]. Can you please tag this to fedora 24?

http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3172026

-Vasant

Comment 38 Vasant Hegde 2016-02-26 08:38:53 UTC
> Dan,
> 
> I've imported package and built it for F24 branch [1]. Can you please tag
> this to fedora 24?
> 
> http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3172026

I did scratch build :-(

Did a fresh build again  [1] and I have tagged package to f24 branch.

[hegdevasant@hegdevasant opal-prd]$ koji tag-pkg f24-build  opal-prd-5.1.13-4.fc24
Warning: the pkgurl option is obsolete
Created task 3172161
Watching tasks (this may be safely interrupted)...
3172161 tagBuild (noarch): free
3172161 tagBuild (noarch): free -> closed
  0 free  0 open  1 done  0 failed

3172161 tagBuild (noarch) completed successfully


http://ppc.koji.fedoraproject.org/koji/taskinfo?taskID=3172101

 -Vasant

Comment 39 Dan Horák 2016-02-26 08:51:02 UTC
Any tagging is not necessary, the build in ppc koji correctly landed in f24 tag, because bodhi is not in action yet for f24.

Comment 40 Dan Horák 2016-03-17 09:56:09 UTC
Package built, closing.


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