https://bugzilla.redhat.com/show_bug.cgi?id=1872427
Bug ID: 1872427 Summary: Review Request: ec2-hibinit-agent - support for hibernation for Amazon ec2 Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: davdunc@amazon.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://davdunc.fedorapeople.org/ec2-hibinit-agent.spec SRPM URL: https://davdunc.fedorapeople.org/ec2-hibinit-agent-1.0.2-3.amzn2.src.rpm Description: Hibernation setup utility for Amazon EC2 Fedora Account System Username: davdunc
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
David Duncan davdunc@amazon.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |cxiaoyi@amazon.com, | |laurenrx@amazon.com, | |mabouba@amazon.com, | |tumeya@redhat.com Doc Type|--- |If docs needed, set a value
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
Artur Iwicki fedora@svgames.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora@svgames.pl
--- Comment #1 from Artur Iwicki fedora@svgames.pl ---
Group: System Environment/Daemons [...] %clean
Not used in Fedora. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_section...
License: Apache 2.0
Fedora uses the "ASL 2.0" short name for this licence. https://fedoraproject.org/wiki/Licensing:Main#Good_Licenses
Source0: ec2-hibinit-agent-%{version}.tar.gz
Ideally, this should be a downloadable URL. In case that's not possible, a comment explaining how the tarball was created is highly recommended, for the sake of reproducibility. https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
BuildRequires: python2 python2-devel
Python2 is considered deprecated, to bring any new software depending on Python2 into Fedora you'll need an exception from the Engineering Steering Committee. https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_python_ve...
%{__python2} setup.py install --prefix=usr -O1 --skip-build --root $RPM_BUILD_ROOT
Why not just specify %{buildroot} as the --root, instead of installing to $RPM_BUILD_ROOT and then manually copying files?
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
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 --- - This won't be approved because Py 2 but still some remarks:
%{?systemd_requires} → BuildRequires: systemd-rpm-macros
- %{__python2} setup.py build → %py2_build
- %{__python2} setup.py install --prefix=usr -O1 --skip-build --root $RPM_BUILD_ROOT → %py2_install
- In any case don't mix $RPM_BUILD_ROOT and %{buildroot}
- Not needed
%defattr(-,root,root)
%clean rm -rf $RPM_BUILD_ROOT
- It is forbidden to glob the entire %{python2_sitelib}/ directory, be more specific instead.
- You must install the LICENSE.txt with %license in %files
- Source0: must either be a URL or you need to specify how to build the Source
- Add your own changelog entry
Maybe try targetinng EPEL7 (I don't know the status of Python 2 in EPEL8)
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #3 from Mohamed Aboubakr mabouba@amazon.com --- Created attachment 1715005 --> https://bugzilla.redhat.com/attachment.cgi?id=1715005&action=edit Spec file that include changes for Fedora and Redhat.
I resolved the comments that were given in previous RPM. Also I added extra changes to support REDHAT and Fedora because the previous spec file was supporting Amazon-linux only.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
Carl George 🤠 carl@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |carl@redhat.com
--- Comment #4 from Carl George 🤠 carl@redhat.com --- Mohamed, are you taking over this package review submission from David? If so, please follow the same format as comment 0. fedora-review doesn't operate on attachments, and we'll need your FAS username when/if the package is approved. The information required is described in more detail in the wiki [0].
I manually took a look at your attachment, and there are still unresolved issues. The Group tag and %clean section must be removed [1]. The License identifier should be "ASL 2.0" [2]. All the files in `%{python3_sitelib}/ec2_hibinit_agent-%{version}-py3.*.egg-info` are owned, but that directory is not [3]. Since this requires acpid, there are several directories you don't need to own in %files (compare and remove the ones already owned by acpid).
There are several improvements that could be made in the name of legibility [4].
- The %amzn macro is not allowed. - Between sections, this spec has double space, single space, and no space at all. Pick either double or single spacing and use it consistently. - The %setup macro by default expects %{name}-%{version}, so the -n flag is unnecessary. - Since you are setting the permissions on sleepconf and sleep.sh with install during %install, you don't need to set them again with %attr in %files.
[0] https://fedoraproject.org/wiki/Package_Review_Process [1] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_section... [2] https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuideline... [3] https://docs.fedoraproject.org/en-US/packaging-guidelines/UnownedDirectories... [4] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_spec_legibility
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
David Duncan davdunc@amazon.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(davdunc@amazon.co | |m)
--- Comment #5 from David Duncan davdunc@amazon.com --- (In reply to Carl George 🤠 from comment #4)
Mohamed, are you taking over this package review submission from David? If so, please follow the same format as comment 0. fedora-review doesn't operate on attachments, and we'll need your FAS username when/if the package is approved. The information required is described in more detail in the wiki [0].
I am still working on this Carl. Mohamed is working on the upstream integration and getting us through the internal Amazon reviews for the code. As you can imagine, he is eager to get this working to meet his team goals so he is submitting changes ahead of my validations. Thanks for the review.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
David Duncan davdunc@amazon.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(davdunc@amazon.co |needinfo?(package-review@li |m) |sts.fedoraproject.org)
--- Comment #6 from David Duncan davdunc@amazon.com --- Updated
Spec URL: https://davdunc.fedorapeople.org/ec2-hibinit-agent.spec SRPM URL: https://davdunc.fedorapeople.org/ec2-hibinit-agent-1.0.3-2.fc33.src.rpm
Updated with a comment for the source0. Waiting on final approval for the source uri to be valid.
Product: Fedora Version: rawhide Component: Package Review
David Duncan davdunc@amazon.com has asked Package Review package-review@lists.fedoraproject.org for needinfo: Bug 1872427: Review Request: ec2-hibinit-agent - support for hibernation for Amazon ec2 https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #6 from David Duncan davdunc@amazon.com --- Updated
Spec URL: https://davdunc.fedorapeople.org/ec2-hibinit-agent.spec SRPM URL: https://davdunc.fedorapeople.org/ec2-hibinit-agent-1.0.3-2.fc33.src.rpm
Updated with a comment for the source0. Waiting on final approval for the source uri to be valid.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #7 from Robert-André Mauchin 🐧 zebob.m@gmail.com --- %global debug_package %{nil}
Suspicious, it needs to be justified' why can't you generate debuginfo
- 2 times systemd-rpm-macros which should be a BuildRequires not a Requires:
BuildRequires: systemd-rpm-macros BuildRequires: python3-devel Requires: acpid Requires: grubby Requires: tuned
- I don't particully mind %if 0%{?amzn}, I asked once about suse directive and I was said it was ok.
- Have a look at the SELINUX macros at /usr/lib/rpm/macros.d/macros.selinux-policy There are some macros you should use:
%selinux_requires
# %selinux_modules_install [-s <policytype>] [-p <modulepriority>] module [module]... %selinux_modules_install("s:p:")
# %selinux_modules_uninstall [-s <policytype>] [-p <modulepriority>] module [module]... %selinux_modules_uninstall("s:p:") \
# %selinux_relabel_pre [-s <policytype>] %selinux_relabel_pre("s:") \
# %selinux_relabel_post [-s <policytype>] %selinux_relabel_post("s:") \
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #8 from Robert-André Mauchin 🐧 zebob.m@gmail.com ---
%global debug_package %{nil}
Suspicious, it needs to be justified' why can't you generate debuginfo
As this is a Python package, you may simply specify
BuildArch: noarch
since it is not an arched package. No need for the debuginfo then.
%{python3_sitelib}/ec2_hibinit_agent-*.egg-info/
I find it weird that you provides egg-info without providing the library supposedly installed in %{python3_sitelib}/
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #9 from Carl George 🤠 carl@redhat.com --- The latest spec file still has conditionals for another distribution, which is explicitly prohibited by policy [0].
To help facilitate legibility, only macros and conditionals for Fedora and EPEL are allowed to be used in Fedora Packages.
Once the source location has been "approved" (I'm assuming this means a git tag in that GitHub repo), make sure to use the recommended format for GitHub git tags [1].
The %setup invocation is wrong. %{name}-%{version} is the argument for the -n flag that I mentioned should be removed. If the top level directory in the source tarball matches %{name}-%{version}, the invocation should be just `%setup -q`.
[0] https://docs.fedoraproject.org/en-US/packaging-guidelines/#_spec_legibility [1] https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#_git_ta...
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
leigh scott leigh123linux@googlemail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(package-review@li | |sts.fedoraproject.org) |
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
David Duncan davdunc@amazon.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |nobody@fedoraproject.org Flags| |needinfo?(nobody@fedoraproj | |ect.org)
--- Comment #10 from David Duncan davdunc@amazon.com --- Okay. finally have some updates for this package with modifications in place. Please have another look and see if we are close to moving forward. https://davdunc.fedorapeople.org/ec2-hibinit-agent-1.0.3-3.fc33.src.rpm https://davdunc.fedorapeople.org/ec2-hibinit-agent.spec
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |ngompa13@gmail.com Assignee|nobody@fedoraproject.org |ngompa13@gmail.com Flags|needinfo?(nobody@fedoraproj |fedora-review? |ect.org) |
--- Comment #11 from Neal Gompa ngompa13@gmail.com --- I will take this formal review, since nobody has so far.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #12 from Neal Gompa ngompa13@gmail.com ---
%global release_number 3
Please use "baserelease" here, so that Release Engineering automation will be able to correctly auto-bump the Release field. Alternatively, decouple the upstream "release" field from the Release number being used in the "Release:" tag.
Recommendation for upstream: Don't tag versions with release numbers, just bump the patch version.
%py3_build_egg %py3_install_egg
Why are we building eggs instead of just doing the normal "%py3_build" and "%py3_install" macros? Egg installations mean that everything is installed as a zip bundle rather than as files on disk. This is not the recommended way to build Python stuff as RPMs.
Requires: python3 Requires: python3-setuptools
This should no longer be needed, as they will be autogenerated correctly if we install Python code the normal way. If you're also trying to build this for EL8 without EPEL, you will also want to add the following at the top of your spec:
# Enable Python dependency generation %{?python_enable_dependency_generator}
%selinux_requires
This requires "BuildRequires: selinux-policy, selinux-policy-devel". Also, please do "%{?selinux_requires}" so that SRPM builds don't fail.
# Disable THP by switching to nothp_profile profile
Is there a reason we're doing this?
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #13 from Neal Gompa ngompa13@gmail.com ---
%global release_number 3
Actually, to more concretely word this correctly: Do not use upstream release versions in the "Release:" tag. Please decouple them so things like mass rebuilds don't break.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #14 from Mohamed Aboubakr mabouba@amazon.com --- Question related to
%py3_build_egg %py3_install_egg
I receive this error
error: Installed (but unpackaged) file(s) found: /usr/config/hibinit-config.cfg
/usr/lib/python3.6/site-packages/ec2_hibinit_agent-1.0.2-py3.6.egg-info/PKG-INFO
/usr/lib/python3.6/site-packages/ec2_hibinit_agent-1.0.2-py3.6.egg-info/SOURCES.txt
/usr/lib/python3.6/site-packages/ec2_hibinit_agent-1.0.2-py3.6.egg-info/dependency_links.txt
/usr/lib/python3.6/site-packages/ec2_hibinit_agent-1.0.2-py3.6.egg-info/top_level.txt
I did this fix under %file section
%{python3_sitelib}/ec2_hibinit_agent-%{version}-py3.*.egg-info/* %{python3_sitelib}/ec2_hibinit_agent-%{version}-py3.*.egg-info/._SOURCES.txt %{python3_sitelib}/ec2_hibinit_agent-%{version}-py3.*.egg-info/._dependency_links.txt
The feedback that I received All the files in `%{python3_sitelib}/ec2_hibinit_agent-%{version}-py3.*.egg-info` are owned, but that directory is not [3].
[3] https://docs.fedoraproject.org/en-US/packaging-guidelines/UnownedDirectories...
I confused little here, what should I do ?
If I shouldn't use py3_build_egg and use py3_build, what should I do in the above error ?
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
David Duncan davdunc@amazon.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(davdunc@amazon.co | |m)
--- Comment #15 from David Duncan davdunc@amazon.com --- Hi Neal,
%global release_number 3
Please use "baserelease" here, so that Release Engineering automation will be able to correctly auto-bump the Release field. Alternatively, decouple the upstream "release" field from the Release number being used in the "Release:" tag.
Did not realize that was going on there. Will fix.
Recommendation for upstream: Don't tag versions with release numbers, just bump the patch version.
Ack. Will modify to ensure that we are not putting that at risk.
%py3_build_egg %py3_install_egg
Why are we building eggs instead of just doing the normal "%py3_build" and "%py3_install" macros? Egg installations mean that everything is installed as a zip bundle rather than as files on disk. This is not the recommended way to build Python stuff as RPMs.
Requires: python3 Requires: python3-setuptools
This should no longer be needed, as they will be autogenerated correctly if we install Python code the normal way. If you're also trying to build this for EL8 without EPEL, you will also want to add the following at the top of your spec:
# Enable Python dependency generation %{?python_enable_dependency_generator}
Ack, will add that in. That's important, thanks.
%selinux_requires
This requires "BuildRequires: selinux-policy, selinux-policy-devel". Also, please do "%{?selinux_requires}" so that SRPM builds don't fail.
got it. Thanks.
# Disable THP by switching to nothp_profile profile
Is there a reason we're doing this?
This enables workloads that don't include the following patch: https://lore.kernel.org/linux-mm/20200820045323.7809-1-hsiangkao@redhat.com/
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #16 from Neal Gompa ngompa13@gmail.com --- (In reply to Mohamed Aboubakr from comment #14)
Question related to
%py3_build_egg %py3_install_egg
I receive this error
error: Installed (but unpackaged) file(s) found: /usr/config/hibinit-config.cfg
You should probably fix this in the code, because this is a legitimate error. You have made a setup.py that installs a config file in the wrong place. Either don't have the setup.py install the file, or adjust it to install it to the correct location.
/usr/lib/python3.6/site-packages/ec2_hibinit_agent-1.0.2-py3.6.egg-info/PKG- INFO
/usr/lib/python3.6/site-packages/ec2_hibinit_agent-1.0.2-py3.6.egg-info/ SOURCES.txt
/usr/lib/python3.6/site-packages/ec2_hibinit_agent-1.0.2-py3.6.egg-info/ dependency_links.txt
/usr/lib/python3.6/site-packages/ec2_hibinit_agent-1.0.2-py3.6.egg-info/ top_level.txt
I did this fix under %file section
%{python3_sitelib}/ec2_hibinit_agent-%{version}-py3.*.egg-info/* %{python3_sitelib}/ec2_hibinit_agent-%{version}-py3.*.egg-info/._SOURCES.txt %{python3_sitelib}/ec2_hibinit_agent-%{version}-py3.*.egg-info/. _dependency_links.txt
The feedback that I received All the files in `%{python3_sitelib}/ec2_hibinit_agent-%{version}-py3.*.egg-info` are owned, but that directory is not [3].
[3] https://docs.fedoraproject.org/en-US/packaging-guidelines/UnownedDirectories... #_wildcarding_files_inside_a_created_directory
I confused little here, what should I do ?
If I shouldn't use py3_build_egg and use py3_build, what should I do in the above error ?
You can just do the following for the Python metadata files:
%{python3_sitelib}/ec2_hibinit_agent-%{version}-*.egg-info/
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
David Duncan davdunc@amazon.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(davdunc@amazon.co | |m) |
--- Comment #17 from David Duncan davdunc@amazon.com --- (In reply to Neal Gompa from comment #16)
(In reply to Mohamed Aboubakr from comment #14)
@nea I worked on this with Mohamed and I think we have a better submission this time, but looking forward to the next review. This was a super helpful review and I think this is more inline with the expectations.
[...]
error: Installed (but unpackaged) file(s) found: /usr/config/hibinit-config.cfg
You should probably fix this in the code, because this is a legitimate error. You have made a setup.py that installs a config file in the wrong place. Either don't have the setup.py install the file, or adjust it to install it to the correct location.
Mohamed is pushing the update to the github repo for setup.py so that the cfg file lands somewhere sane, not ```/usr/config```
[...]
If I shouldn't use py3_build_egg and use py3_build, what should I do in the above error ?
You can just do the following for the Python metadata files:
%{python3_sitelib}/ec2_hibinit_agent-%{version}-*.egg-info/
A spec file is up for review with the release changes and in anticipation of modify in setup.py to move back to /etc
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #18 from Mohamed Aboubakr mabouba@amazon.com --- @Nea FYI : the spec file is updated https://davdunc.fedorapeople.org/ec2-hibinit-agent.spec
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #19 from Neal Gompa ngompa13@gmail.com ---
%global gittag %{version}
This is effectively no longer needed, so you can replace all usage of "%{gittag}" with "%{version}".
%{_sysconfdir}/hibinit-config.cfg
This needs to be marked as "%config(noreplace)"
%dir %{_localstatedir}/lib/hibinit-agent %ghost %attr(0600,root,root) %{_localstatedir}/lib/hibinit-agent/hibernation-enabled
"%{_localstatedir}/lib" should be replaced with "%{_sharedstatedir}"
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
David Duncan davdunc@amazon.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(ngompa13@gmail.co | |m)
--- Comment #20 from David Duncan davdunc@amazon.com --- bumping to highlight that there are new updates https://davdunc.fedorapeople.org/ec2-hibinit-agent.spec
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(ngompa13@gmail.co | |m) |
--- Comment #21 from Neal Gompa ngompa13@gmail.com --- The "URL" is missing.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #22 from Neal Gompa ngompa13@gmail.com --- Blech, I mean that the "URL:" field is missing in the spec.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
David Duncan davdunc@amazon.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(ngompa13@gmail.co | |m)
--- Comment #23 from David Duncan davdunc@amazon.com --- Added the URL: back.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(ngompa13@gmail.co |needinfo?(davdunc@amazon.co |m) |m)
--- Comment #24 from Neal Gompa ngompa13@gmail.com --- Can you please regenerate the SRPM? fedora-review is failing to run because the SRPM is out of sync with the spec and Mock is failing.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
David Duncan davdunc@amazon.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(davdunc@amazon.co | |m) |
--- Comment #25 from David Duncan davdunc@amazon.com --- Spec URL: https://davdunc.fedorapeople.org/ec2-hibinit-agent.spec SRPM URL: https://davdunc.fedorapeople.org/ec2-hibinit-agent-1.0.3-3.fc33.src.rpm
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
David Duncan davdunc@amazon.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(ngompa13@gmail.co | |m)
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #26 from Mohamed Aboubakr mabouba@amazon.com --- Any updates on this ?
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(ngompa13@gmail.co | |m) |
--- Comment #27 from Neal Gompa ngompa13@gmail.com --- This fails a mock build:
Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.nBkcOv + umask 022 + cd /builddir/build/BUILD + cd /builddir/build/BUILD + rm -rf amazon-ec2-hibinit-agent-1.0.3 + /usr/bin/gzip -dc /builddir/build/SOURCES/ec2-hibinit-agent-1.0.3.tar.gz + /usr/bin/tar -xof - + STATUS=0 + '[' 0 -ne 0 ']' + cd amazon-ec2-hibinit-agent-1.0.3 /var/tmp/rpm-tmp.nBkcOv: line 39: cd: amazon-ec2-hibinit-agent-1.0.3: No such file or directory
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #28 from Mohamed Aboubakr mabouba@amazon.com --- Do you have in SOURCE folder this amazon-ec2-hibinit-agent-1.0.3 ?
cd ~/rpmbuild/SOURCES wget https://github.com/aws/amazon-ec2-hibinit-agent/archive/v1.0.3/ec2-hibinit-a... tar -xf ec2-hibinit-agent-1.0.3.tar.gz
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #29 from Neal Gompa ngompa13@gmail.com --- I built from the SRPM, there's *no reason* I should need to fetch anything.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #30 from David Duncan davdunc@amazon.com --- (In reply to Neal Gompa from comment #29)
I built from the SRPM, there's *no reason* I should need to fetch anything.
Absolutely agreed!
I have the current SRPM built for f33 here: https://davdunc.fedorapeople.org/ec2-hibinit-agent-1.0.3-3.fc33.src.rpm I apologize for the previous one. This one is verified to function as expected.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #31 from Neal Gompa ngompa13@gmail.com --- Thanks for the fixed SRPM. fedora-review identified a serious issue I hadn't noticed earlier:
%config(noreplace) %{_prefix}/lib/systemd/system-sleep/sleep-handler.sh %config(noreplace) %{_prefix}/lib/systemd/logind.conf.d/00-hibinit-agent.conf
These *must not* be marked as config files, as %config markers are only allowed for files in /etc.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
David Duncan davdunc@amazon.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(ngompa13@gmail.co | |m)
--- Comment #32 from David Duncan davdunc@amazon.com --- @ngompa13@gmail.com https://davdunc.fedorapeople.org/ec2-hibinit-agent-1.0.3-3.fc33.src.rpm updated with the content modifications for sleep-handler.sh and 00-hibinitagent.conf
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(ngompa13@gmail.co | |m) |
--- Comment #33 from Neal Gompa ngompa13@gmail.com --- I need both the spec file and the SRPM updated, otherwise fedora-review will choke on it.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
David Duncan davdunc@amazon.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(ngompa13@gmail.co | |m)
--- Comment #34 from David Duncan davdunc@amazon.com --- @ngompa13@gmail.com
https://davdunc.fedorapeople.org/ec2-hibinit-agent-1.0.3-3.fc33.src.rpm Updated Spec file: https://davdunc.fedorapeople.org/ec2-hibinit-agent.spec
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #35 from Mohamed Aboubakr mabouba@amazon.com --- Hello Neal, Do you have any updates ?
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(ngompa13@gmail.co | |m) |
--- Comment #36 from Neal Gompa ngompa13@gmail.com --- (In reply to Mohamed Aboubakr from comment #35)
Hello Neal, Do you have any updates ?
My feedback in comment 31 is still not addressed.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #37 from David Duncan davdunc@amazon.com --- (In reply to Neal Gompa from comment #36)
(In reply to Mohamed Aboubakr from comment #35)
Hello Neal, Do you have any updates ?
My feedback in comment 31 is still not addressed.
That looks like I uploaded the wrong spec file on the issue and clobbered the intended changes. Updated the SRPM and SPEC files
https://davdunc.fedorapeople.org/ec2-hibinit-agent-1.0.3-3.fc33.src.rpm https://davdunc.fedorapeople.org/ec2-hibinit-agent.spec
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #38 from Neal Gompa ngompa13@gmail.com --- (In reply to David Duncan from comment #37)
(In reply to Neal Gompa from comment #36)
(In reply to Mohamed Aboubakr from comment #35)
Hello Neal, Do you have any updates ?
My feedback in comment 31 is still not addressed.
That looks like I uploaded the wrong spec file on the issue and clobbered the intended changes. Updated the SRPM and SPEC files
It's still there.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #39 from Mohamed Aboubakr mabouba@amazon.com --- Hello Neal,
I clicked on the David link. We have removed %config(noreplace) from the following lines
%{_prefix}/lib/systemd/system-sleep/sleep-handler.sh %{_prefix}/lib/systemd/logind.conf.d/00-hibinit-agent.conf
Could you please confirm that you have opened the right link ? ---------
%files %doc README.md %license LICENSE.txt
%config(noreplace) %{_sysconfdir}/hibinit-config.cfg %{_unitdir}/hibinit-agent.service %{_bindir}/hibinit-agent %config(noreplace) %{_sysconfdir}/acpi/events/sleepconf %config(noreplace) %{_sysconfdir}/acpi/actions/sleep.sh %{python3_sitelib}/ec2_hibinit_agent-*.egg-info/ %dir %{_sharedstatedir}/hibinit-agent %ghost %attr(0600,root,root) %{_sharedstatedir}/hibinit-agent/hibernation-enabled
%dir %{_prefix}/lib/systemd/logind.conf.d %dir %{_prefix}/lib/systemd/system-sleep
%dir %{_sysconfdir}/tuned/nothp_profile %config(noreplace) %{_sysconfdir}/tuned/nothp_profile/tuned.conf
%{_prefix}/lib/systemd/system-sleep/sleep-handler.sh %{_prefix}/lib/systemd/logind.conf.d/00-hibinit-agent.conf %attr(0644,root,root) %{_datadir}/selinux/packages/*.pp.bz2
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #40 from Neal Gompa ngompa13@gmail.com --- Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
===== MUST items =====
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* Apache License 2.0", "Unknown or generated", "Apache License 2.0". 16 files have unknown license. Detailed output of licensecheck in /home/ngompa/1872427-ec2-hibinit- agent/licensecheck.txt [x]: Package does not own files or directories owned by other packages. Note: Dirs in package are owned also by: /usr/lib/systemd/system- sleep(tlp, systemd-udev) [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. [-]: 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]: Package is not known to require an ExcludeArch tag. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 10240 bytes in 1 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 must own all directories that it creates. [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 must not depend on deprecated() packages. [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]: systemd_post is invoked in %post, systemd_preun in %preun, and systemd_postun in %postun for Systemd service files. Note: Systemd service file(s) in ec2-hibinit-agent [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. [x]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: 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. [?]: 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]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified.
===== EXTRA items =====
Generic: [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Spec file according to URL is the same as in SRPM.
Rpmlint ------- Checking: ec2-hibinit-agent-1.0.3-3.fc34.noarch.rpm ec2-hibinit-agent-1.0.3-3.fc34.src.rpm ec2-hibinit-agent.noarch: E: executable-marked-as-config-file /etc/acpi/actions/sleep.sh ec2-hibinit-agent.noarch: E: non-readable /var/lib/hibinit-agent/hibernation-enabled 600 ec2-hibinit-agent.noarch: W: no-manual-page-for-binary hibinit-agent ec2-hibinit-agent.noarch: W: dangerous-command-in-%pre cp ec2-hibinit-agent.noarch: W: dangerous-command-in-%posttrans rm 2 packages and 0 specfiles checked; 2 errors, 3 warnings.
Rpmlint (installed packages) ---------------------------- ec2-hibinit-agent.noarch: E: executable-marked-as-config-file /etc/acpi/actions/sleep.sh ec2-hibinit-agent.noarch: E: non-readable /var/lib/hibinit-agent/hibernation-enabled 600 ec2-hibinit-agent.noarch: W: no-manual-page-for-binary hibinit-agent ec2-hibinit-agent.noarch: W: dangerous-command-in-%pre cp ec2-hibinit-agent.noarch: W: dangerous-command-in-%posttrans rm 1 packages and 0 specfiles checked; 2 errors, 3 warnings.
Source checksums ---------------- https://github.com/aws/amazon-ec2-hibinit-agent/archive/v1.0.3/ec2-hibinit-a... : CHECKSUM(SHA256) this package : 6ac264c6c67381e2458c1af534aea34b482e69f266ea00aa5d678f2817dadb09 CHECKSUM(SHA256) upstream package : 6ac264c6c67381e2458c1af534aea34b482e69f266ea00aa5d678f2817dadb09
Requires -------- ec2-hibinit-agent (rpmlib, GLIBC filtered): /bin/sh /usr/bin/bash /usr/bin/python3 /usr/bin/sh acpid config(ec2-hibinit-agent) grubby libselinux-utils policycoreutils policycoreutils-python-utils python(abi) selinux-policy selinux-policy-base systemd tuned
Provides -------- ec2-hibinit-agent: config(ec2-hibinit-agent) ec2-hibinit-agent python3.9dist(ec2-hibinit-agent) python3dist(ec2-hibinit-agent)
Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10 Command line :/usr/bin/fedora-review -b 1872427 -m fedora-rawhide-x86_64 Buildroot used: fedora-rawhide-x86_64 Active plugins: Shell-api, Generic Disabled plugins: R, PHP, Ocaml, Perl, Haskell, fonts, Python, SugarActivity, Java, C/C++ Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
Neal Gompa ngompa13@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #41 from Neal Gompa ngompa13@gmail.com --- At this point, I think I'm satisfied with the state of things.
PACKAGE APPROVED.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #42 from Gwyn Ciesla gwync@protonmail.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/ec2-hibinit-agent
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
David Duncan davdunc@amazon.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |NEXTRELEASE Last Closed| |2021-01-21 16:35:37
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #43 from Fedora Update System updates@fedoraproject.org --- FEDORA-2021-58b86608cb has been submitted as an update to Fedora 33. https://bodhi.fedoraproject.org/updates/FEDORA-2021-58b86608cb
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #44 from Fedora Update System updates@fedoraproject.org --- FEDORA-2021-58b86608cb has been pushed to the Fedora 33 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2021-58b86608cb *` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-58b86608cb
See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|NEXTRELEASE |ERRATA
--- Comment #45 from Fedora Update System updates@fedoraproject.org --- FEDORA-2021-58b86608cb has been pushed to the Fedora 33 stable repository. If problem still persists, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #46 from Fedora Update System updates@fedoraproject.org --- FEDORA-EPEL-2021-a5d5e1ef36 has been submitted as an update to Fedora EPEL 8. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-a5d5e1ef36
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #47 from Fedora Update System updates@fedoraproject.org --- FEDORA-EPEL-2021-a5d5e1ef36 has been pushed to the Fedora EPEL 8 testing repository.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-a5d5e1ef36
See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #48 from Fedora Update System updates@fedoraproject.org --- FEDORA-EPEL-2021-a5d5e1ef36 has been pushed to the Fedora EPEL 8 stable repository. If problem still persists, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #49 from Fedora Update System updates@fedoraproject.org --- FEDORA-EPEL-2023-236199f4d4 has been submitted as an update to Fedora EPEL 8. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-236199f4d4
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #50 from Fedora Update System updates@fedoraproject.org --- FEDORA-EPEL-2023-236199f4d4 has been pushed to the Fedora EPEL 8 testing repository.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-236199f4d4
See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #51 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-642382d570 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2023-642382d570
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #52 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-4edd939f66 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-4edd939f66
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #53 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-4edd939f66 has been pushed to the Fedora 38 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-4edd939f66` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-4edd939f66
See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #54 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-642382d570 has been pushed to the Fedora 37 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-642382d570` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-642382d570
See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #55 from Fedora Update System updates@fedoraproject.org --- FEDORA-EPEL-2023-236199f4d4 has been pushed to the Fedora EPEL 8 stable repository. If problem still persists, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #56 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-642382d570 has been pushed to the Fedora 37 stable repository. If problem still persists, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1872427
--- Comment #57 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-4edd939f66 has been pushed to the Fedora 38 stable repository. If problem still persists, please make note of it in this bug report.
package-review@lists.fedoraproject.org