[Bug 1475228] New: Review Request: tpm2-abrmd - TPM2 access broker and resource manager daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
Bug ID: 1475228 Summary: Review Request: tpm2-abrmd - TPM2 access broker and resource manager daemon Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: yunying.sun@intel.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://raw.githubusercontent.com/yunyings/share/master/tpm2-abrmd.spec SRPM URL: https://github.com/yunyings/share/raw/master/tpm2-abrmd-1.1.0-1.el7.src.rpm
Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=20749893
Description: <description here> tpm2-abrmd is a system daemon implementing the TPM2 access broker (TAB) & Resource Manager (RM) spec from the TCG.
Fedora Account System Username: yunyings
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #1 from Yunying Sun yunying.sun@intel.com --- Upstream repository: https://github.com/01org/tpm2-abrmd
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
Robert-André Mauchin zebob.m@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |zebob.m@gmail.com
--- Comment #2 from Robert-André Mauchin zebob.m@gmail.com --- Hello,
A couple of points first:
- Systemd service files should be installed in %{_unitdir}. See https://fedoraproject.org/wiki/Packaging:Systemd#Filesystem_locations
- Systemd service files also need specific scriptlets. See https://fedoraproject.org/wiki/Packaging:Scriptlets?rd=Packaging:ScriptletSn...
- GCC is not needed as a BuildRequire. See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2
- why do you define %global pkg_prefix tpm2-abrmd instead of using %{name} ?
- you can replace ./bootstrap with "autoreconf -vif" in %build
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #3 from Yunying Sun yunying.sun@intel.com --- Thanks Robert-Andre for the first review.
With current upstream code base, tpm2-abrmd.service is installed at /usr/lib64/systemd/system/tpm2-abrmd.service, not /lib/systemd/system.
I created a ticket for upstream: https://github.com/01org/tpm2-abrmd/issues/115, and will update SPEC once this issue is fixed or clarified.
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #4 from Yunying Sun yunying.sun@intel.com --- (In reply to Robert-André Mauchin from comment #2)
Hello,
A couple of points first:
- Systemd service files should be installed in %{_unitdir}. See
https://fedoraproject.org/wiki/Packaging:Systemd#Filesystem_locations
Hi Robert-Andre, one question here, without upstream change, is it possible to set a different systemd service file installation path in spec file, to override the upstream default(/usr/lib64/systemd/system/)?
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #5 from Robert-André Mauchin zebob.m@gmail.com --- Yes, in %install just move from the file from %{buildroot}/%{_libdir}/systemd/system/tpm2-abrmd.service to %{buildroot}/%{_unitdir}
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #6 from Yunying Sun yunying.sun@intel.com --- Thanks Robert. I've updated SPEC & SRPMS to address the issues in comment 2: Spec URL: https://raw.githubusercontent.com/yunyings/share/master/tpm2-abrmd.spec SRPM URL: https://github.com/yunyings/share/raw/master/tpm2-abrmd-1.1.0-2.el7.src.rpm
Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=20917523
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #7 from Yunying Sun yunying.sun@intel.com --- %changelog * Tue Aug 1 2017 Sun Yunying yunying.sun@intel.com - 1.1.0-3 - Use config option with-systemdsystemunitdir to set systemd unit file location
Spec URL: https://raw.githubusercontent.com/yunyings/share/master/tpm2-abrmd.spec SRPM URL: https://github.com/yunyings/share/raw/master/tpm2-abrmd-1.1.0-2.el7.src.rpm Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=20938040
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #8 from Yunying Sun yunying.sun@intel.com --- SRPM URL should be: https://github.com/yunyings/share/raw/master/tpm2-abrmd-1.1.0-3.el7.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #9 from Robert-André Mauchin zebob.m@gmail.com --- It looks good to me, it just need to be officially reviewed now.
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #10 from Robert-André Mauchin zebob.m@gmail.com --- There's nothing after your %systemd_postun, didn't you forget to add the service file there? %systemd_postun tpm2-abrmd.service
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
Robert-André Mauchin zebob.m@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |zebob.m@gmail.com Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #11 from Yunying Sun yunying.sun@intel.com --- * Tue Aug 15 2017 Sun Yunying yunying.sun@intel.com - 1.1.0-4 - Rename and relocate udev rules file to _udevrulesdir - Update scriptlet to add service name after systemd_postrun
Spec URL: https://raw.githubusercontent.com/yunyings/share/master/tpm2-abrmd.spec SRPM URL: https://github.com/yunyings/share/raw/master/tpm2-abrmd-1.1.0-4.el7.src.rpm Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=21239218
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
Robert-André Mauchin (afk until next Thu) zebob.m@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |POST Flags|fedora-review? |fedora-review+
--- Comment #12 from Robert-André Mauchin (afk until next Thu) zebob.m@gmail.com --- One minor thing: - Source0 should be modified to https://github.com/01org/tpm2-abrmd/archive/%%7Bversion%7D/%%7Bname%7D-%%7Bv...
Otherwise, package accepted.
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]: Header files in -devel subpackage, if present. [x]: ldconfig called in %post and %postun if required. [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: "*No copyright* BSD (2 clause)", "BSD (2 clause)", "Unknown or generated". 26 files have unknown license. Detailed output of licensecheck in /home/bob/packaging/review/tpm2-abrmd/review- tpm2-abrmd/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: Package must own all directories that it creates. Note: Directories without known owners: /usr/lib/udev, /usr/lib/udev/rules.d [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). [x]: 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. [x]: Package contains systemd file(s) if in need. [x]: 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 10240 bytes in 2 files. [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]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [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]: %config files are marked noreplace or the reason is justified. [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]: No %config files under /usr. [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: [-]: 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). [x]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in tpm2 -abrmd-debuginfo , tpm2-abrmd-devel [?]: Package functions as described. [x]: Latest version is packaged. [x]: 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. [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]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: The placement of pkgconfig(.pc) files are correct. [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Package should compile and build into binary rpms on all supported architectures. [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]: Package should not use obsolete m4 macros [x]: Spec file according to URL is the same as in SRPM.
Rpmlint ------- Checking: tpm2-abrmd-1.1.0-4.fc27.x86_64.rpm tpm2-abrmd-debuginfo-1.1.0-4.fc27.x86_64.rpm tpm2-abrmd-devel-1.1.0-4.fc27.x86_64.rpm tpm2-abrmd-1.1.0-4.fc27.src.rpm tpm2-abrmd.x86_64: W: shared-lib-calls-exit /usr/lib64/libtcti-tabrmd.so.0.0.0 exit@GLIBC_2.2.5 tpm2-abrmd.x86_64: W: hidden-file-or-dir /usr/lib/.build-id tpm2-abrmd.x86_64: W: hidden-file-or-dir /usr/lib/.build-id tpm2-abrmd.x86_64: W: one-line-command-in-%postun /sbin/ldconfig tpm2-abrmd-devel.x86_64: W: spelling-error Summary(en_US) config -> con fig, con-fig, configure tpm2-abrmd-devel.x86_64: W: spelling-error %description -l en_US config -> con fig, con-fig, configure tpm2-abrmd-devel.x86_64: W: only-non-binary-in-usr-lib tpm2-abrmd-devel.x86_64: W: no-documentation 4 packages and 0 specfiles checked; 0 errors, 8 warnings.
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #13 from Yunying Sun yunying.sun@intel.com --- Thanks Robert-Andre for the review and approve.
I updated SPEC with the source0 url in your comment. * Wed Aug 16 2017 Sun Yunying yunying.sun@intel.com - 1.1.0-5 - Updated source0 URL to fix rpmlint warnings
Spec URL: https://raw.githubusercontent.com/yunyings/share/master/tpm2-abrmd.spec SRPM URL: https://github.com/yunyings/share/raw/master/tpm2-abrmd-1.1.0-5.el7.src.rpm Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=21253304
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #15 from Gwyn Ciesla limburgher@gmail.com --- (fedrepo-req-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/tpm2-abrmd
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #14 from Yunying Sun yunying.sun@intel.com --- New package request created: https://pagure.io/releng/fedora-scm-requests/issue/61
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #16 from Gwyn Ciesla limburgher@gmail.com --- (fedrepo-req-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/tpm2-abrmd
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #17 from Yunying Sun yunying.sun@intel.com --- $fedpkg clone rpms/tpm2-abrmd /usr/lib/python2.7/site-packages/fedora/client/bodhi.py:48: DeprecationWarning: fedora.client.bodhi has been deprecated. Please use bodhi.client.bindings instead. DeprecationWarning) Cloning into 'tpm2-abrmd'... remote: Counting objects: 3, done. remote: Total 3 (delta 0), reused 0 (delta 0) Receiving objects: 100% (3/3), done.
$cd tpm2-abrmd/ $fedpkg import ~/rpmbuild/SRPMS/tpm2-abrmd-1.1.0-5.el7.src.rpm $git add . $git commit -m "Initial import (#1475228)"
$fedpkg push /usr/lib/python2.7/site-packages/fedora/client/bodhi.py:48: DeprecationWarning: fedora.client.bodhi has been deprecated. Please use bodhi.client.bindings instead. DeprecationWarning) FATAL: W any rpms/tpm2-abrmd yunyings DENIED by fallthru (or you mis-spelled the reponame) fatal: Could not read from remote repository.
Please make sure you have the correct access rights and the repository exists. Could not execute push: Command '['git', 'push']' returned non-zero exit status 128
Seeing at https://src.fedoraproject.org/rpms/tpm2-abrmd/settings, I'm the admin of this repo: yunyings (main admin)
But koji list-pkgs shows Owner of this package is "releng": [test@SKLS02 tpm2-abrmd]$ koji list-pkgs --package=tpm2-abrmd --show-blocked Package Tag Extra Arches Owner ----------------------- ----------------------- ---------------- --------------- tpm2-abrmd f27 releng
Not sure if that's problem or not. Does anyone knows how to fix this push error?
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #18 from Robert-André Mauchin zebob.m@gmail.com --- Could you retry now to see if it's fixed?
Otherwise manifest yourself on this bug report https://pagure.io/fedora-infrastructure/issue/6236
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #19 from Yunying Sun yunying.sun@intel.com --- Just tried. Still failed, but with different error.
[test@SKLS02 tpm2-abrmd]$ fedpkg push /usr/lib/python2.7/site-packages/fedora/client/bodhi.py:48: DeprecationWarning: fedora.client.bodhi has been deprecated. Please use bodhi.client.bindings instead. DeprecationWarning) Counting objects: 3, done. Writing objects: 100% (2/2), 210 bytes | 0 bytes/s, done. Total 2 (delta 0), reused 0 (delta 0) remote: Traceback (most recent call last): remote: File "./hooks/post-receive.default", line 19, in <module> remote: import pagure # noqa: E402 remote: File "/usr/lib/python2.7/site-packages/pagure/__init__.py", line 62, in <module> remote: APP.config.from_envvar('PAGURE_CONFIG') remote: File "/usr/lib/python2.7/site-packages/flask/config.py", line 108, in from_envvar remote: return self.from_pyfile(rv, silent=silent) remote: File "/usr/lib/python2.7/site-packages/flask/config.py", line 128, in from_pyfile remote: with open(filename) as config_file: remote: IOError: [Errno 13] Unable to load configuration file (Permission denied): '/etc/pagure/pagure.cfg' remote: Hook ./hooks/post-receive.default failed with error code 1 To ssh://yunyings@pkgs.fedoraproject.org/rpms/tpm2-abrmd 4665bdc..5a800a2 master -> master
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #20 from Robert-André Mauchin zebob.m@gmail.com --- I don't think this error is important, if it says it pushed master, it ushed master.
However I see a commit here https://src.fedoraproject.org/rpms/tpm2-abrmd/c/5a800a2346ea47b553fb94ccac1d... But it doesn't contain the sources and spec files. Are you sure you didn't forget to do "fedpkg import PATH_TO_SRPM first"?
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #21 from Yunying Sun yunying.sun@intel.com --- Thanks Robert-Andre. You are right, I made an empty commit by mistake and pushed. Just reverted the commit, and pushed a new one with valid files.
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #22 from Yunying Sun yunying.sun@intel.com --- But "fedpkg build"(https://koji.fedoraproject.org/koji/taskinfo?taskID=21277310) failed with: FAILED: BuildError: package tpm2-abrmd not in list for tag f28-pending
Is there something I should do to fix this?
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #23 from Robert-André Mauchin zebob.m@gmail.com --- As far as I know:
« The script [that picks up new packages] does currently only run once per hour and takes about 30min, so you could have a 1.5 hour gap there between a new package being added and it getting synced to koji. »
So you should wait a bit 1:30/2h after pushing your new package. If the problem still persists, please mention it on https://pagure.io/fedora-infrastructure/issue/6236
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #24 from Yunying Sun yunying.sun@intel.com --- Understood. I will try koji build tomorrow. Thanks.
BTW, according to https://pagure.io/fedora-infrastructure/issue/6235, seems the "IOError: [Errno 13] Unable to load configuration file (Permission denied)" is an infrastructure issue which not yet fixed.
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #25 from Yunying Sun yunying.sun@intel.com --- Koji rawhide build completed for tpm2-abrmd-1.1.0-5: https://koji.fedoraproject.org/koji/taskinfo?taskID=21292902
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
--- Comment #26 from Yunying Sun yunying.sun@intel.com --- f27 has tpm2-tss 1.1.0, which does not provide resource manager daemon anymore. tpm2-abrmd is the replacing daemon, which is required when using many tpm2 userspace tools like those tpm2_*** provided by tpm2-tools. Therefore tpm2-abrmd is needed for f27 as well.
I did a tpm2-abrmd-1.1.0-5 koji build for f27: https://koji.fedoraproject.org/koji/taskinfo?taskID=21293091
But "fedpkg update" failed for f27 with error: [test@SKLS02 tpm2-abrmd]$ git branch * f27 master [test@SKLS02 tpm2-abrmd]$ fedpkg update Invalid tag: tpm2-abrmd-1.1.0-5.fc27 not tagged with any of the following tags [u'f22-updates-candidate', u'dist-6E-epel-testing-candidate', u'f21-updates-candidate', u'f24-updates-candidate', u'epel7-testing-candidate', u'f25-updates-candidate', u'dist-5E-epel-testing-candidate', u'f23-updates-candidate', u'f26-updates-candidate'] Unable to determine release from build: tpm2-abrmd-1.1.0-5.fc27
Seems f27 is at early branched state before Bodhi activation point: https://fedoraproject.org/wiki/Releases/27/Schedule
According to https://fedoraproject.org/wiki/Package_update_HOWTO#Rawhide and early Branched , suppose no "fedpkg update" needed and tpm2-abrmd-1.1.0-5 will be included into f27 automatically.
https://bugzilla.redhat.com/show_bug.cgi?id=1475228
Javier Martinez Canillas fmartine@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |CLOSED CC| |fmartine@redhat.com Resolution|--- |CURRENTRELEASE Last Closed| |2018-03-01 09:59:36
--- Comment #27 from Javier Martinez Canillas fmartine@redhat.com --- Yunying,
We can close this bug now that the tpm2-abrmd is already included in Fedora 27.
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com