Bug 1104746 - Review Request: soscleaner - sosreport data obfuscation
Review Request: soscleaner - sosreport data obfuscation
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Tom "spot" Callaway
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2014-06-04 11:12 EDT by Jamie Duncan
Modified: 2014-07-22 23:01 EDT (History)
5 users (show)

See Also:
Fixed In Version: soscleaner-0.2-1.fc20
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-07-22 14:08:46 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
jduncan: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)
f-e review.txt (7.41 KB, text/plain)
2014-06-04 19:05 EDT, Jamie Duncan
no flags Details

  None (edit)
Description Jamie Duncan 2014-06-04 11:12:31 EDT
Spec URL: http://people.redhahttp://people.redhat.com/jduncan/soscleaner/soscleaner.spec
SRPM URL: http://people.redhat.com/jduncan/soscleaner/soscleaner-0.1-11.src.rpm
Description: 

SOSCleaner helps users in environments that have data export restrictions clean up an sosreport so it can be safely uploaded to a support group for analysis. IT IS NOT ALL that should be done for this process, but it does help with the most common and repetitive items.

This is going to be incorporated into the redhat-support-tool soon, and there is strong desire to have it as a standalone tool as well. This will, I hope, begin the process of creating the upstream for the project.

This is my first package submission to Fedora.

Fedora Account System Username: jduncan

Thanks,

Jamie Duncan
Comment 1 Jamie Duncan 2014-06-04 11:31:58 EDT
EL6 Candidate Koji Build:
https://bugzilla.redhat.com/show_bug.cgi?id=1104746

F20 Koji Build:
https://bugzilla.redhat.com/show_bug.cgi?id=1104746
Comment 2 Jamie Duncan 2014-06-04 11:33:13 EDT
don't know what happened there...

EL6:
http://koji.fedoraproject.org/koji/taskinfo?taskID=6924175

F20:
http://koji.fedoraproject.org/koji/taskinfo?taskID=6924156
Comment 3 Michael Schwendt 2014-06-04 15:49:38 EDT
> Spec URL: http://people.redhahttp://people.redhat.com/jduncan/soscleaner/soscleaner.spec

Spec URL: http://people.redhat.com/jduncan/soscleaner/soscleaner.spec

During review, keep the "SRPM URL:" and "Spec URL:" lines up-to-date when you modify the package, so the fedora-review tool can be used. Consider running "fedora-review -b 1104746" to let that tool perform many helpful checks.

The spec file is full of mistakes and pitfalls.


> %define name soscleaner
> %define version 0.1
> %define release 11

%{name}, %{version} and %{release} are implicitly defined by the "Name:", "Version:" and "Release:" tags, so it's very poor form to first define these macros only to have the tags redefine the macros afterwards.

If you want these values at the top of the spec file, move the tags at the top. Easy as that.


> %define unmangled_version 0.1

https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define



> Source0: %{name}-%{unmangled_version}.tar.gz

That's not an URL:
https://fedoraproject.org/wiki/Packaging:SourceURL


> Group: Applications

Not a group listed in /usr/share/doc/rpm/GROUPS and the Group tag is optional nowadays. In case of doubt, remove the tag from the spec file:
https://fedoraproject.org/wiki/Packaging:Guidelines#Group_tag


> BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-buildroot

https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag


> Prefix: %{_prefix}

https://fedoraproject.org/wiki/Packaging:Guidelines#Relocatable_packages


> Vendor: Jamie Duncan <jduncan@redhat.com>
> Packager: Jamie Duncan <jduncan@redhat.com>

https://fedoraproject.org/wiki/Packaging:Guidelines#Tags


> %clean
> rm -rf $RPM_BUILD_ROOT

https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean


> %files -f INSTALLED_FILES

As smart as you may find this technique, it hides too much under the carpet. What about directory ownership, for example? Prefer listing files and directories in the %files section directly. Use wildcards where helpful.


> %defattr(-,root,root)

https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions
Comment 4 Jamie Duncan 2014-06-04 16:21:37 EDT
Michael,

Thanks for the feedback. The spec file is *almost* 100% how it came from distutils.  

I will make the above changes and also take advantage of the tools you mention.

Thanks again,

Jamie Duncan
Comment 5 Jamie Duncan 2014-06-04 17:23:23 EDT
Michael and everyone else.

It's amazing how much more sense the Packaging Guide makes when you have a little push in the right direction and a goal.

I've refactored the spec file to be (I think) in line with the guidelines.  I've also confirmed it builds of course both on my local system and in Koji.

http://koji.fedoraproject.org/koji/taskinfo?taskID=6925123

I bumped up the release number to 12, with artifacts available at:

SRPM URL: http://people.redhat.com/jduncan/soscleaner/soscleaner-0.1-12.src.rpm
Spec URL: http://people.redhat.com/jduncan/soscleaner/soscleaner.spec

Thanks again for helping to straighten me out.

Jamie Duncan
Comment 6 Jamie Duncan 2014-06-04 19:05:20 EDT
Created attachment 902362 [details]
f-e review.txt
Comment 8 Jamie Duncan 2014-06-04 21:53:22 EDT
have gone through fedora-review a few times and it looks solid to me at this point. Definitely a learning experience.
Comment 9 Christopher Meng 2014-06-05 03:06:11 EDT
1. Why was the release bumped to 12 suddenly?

2. %doc /usr/share/doc/%{name}-%{version}/LICENSE
%doc /usr/share/man/man8/%{name}.8.gz

/usr/share --> %{_datadir}
/usr/share/doc --> %{_docdir}
/usr/share/man/ --> %{_mandir}

For manpages, it's better to include them as:

%{_mandir}/man8/%{name}.8*, as the "*" is helpful if the manpages are not compressed(rpm will do this)

And dont mark these as %doc, they will be marked automatically by RPM.
Comment 10 Michael Schwendt 2014-06-05 04:50:22 EDT
Christopher, that's

  https://fedoraproject.org/wiki/Packaging:Guidelines#Macros

with a few explanations. Nowadays it is not mandatory anymore to use path macros.

For paths which don't change often (or which have been pretty much constant over many years), there is no benefit in using macros. Especially not if the packaged software hardcodes its installation paths somewhere.  However, if parts of the build framework (such as a "configure" script) accept definitions from within the spec file (such as with the %configure macro or options to "make"), it can be beneficial to reuse the same path macros inside the %files list(s).


> And dont mark these as %doc, they will be marked automatically by RPM.

Which means it's not a strict "don't", but nice to know.

$ rpm -E %__docdir_path
/usr/share/doc:/usr/share/man:/usr/share/info:/usr/share/gtk-doc/html::/usr/share/man:/usr/share/info:/usr/share/javadoc:/usr/doc:/usr/man:/usr/info:/usr/X11R6/man


> fedora-review

[ ]: Package requires other packages for directories it uses.
     Note: No known owner of /usr/share/doc/soscleaner-0.1
[ ]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/doc/soscleaner-0.1

https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership
https://fedoraproject.org/wiki/Packaging:UnownedDirectories

It could be fixed with an added %dir entry for /usr/share/doc/%{name}-%{version} or by including the entire directory instead only the LICENSE file.


[ ]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "GPL (v2 or later)". Detailed output of licensecheck in
     /home/jduncan/1104746-soscleaner/licensecheck.txt

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#.22or_later_version.22_licenses


> Rpmlint

soscleaner.src:17: W: setup-not-quiet

That refers to using "%setup -q …" and really is not an issue. Non-quiet %setup output can be helpful in a build.log.


soscleaner.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/SOSCleaner.py 0644L /usr/bin/env

That's a strange error message:

rpmlint -i … would tell '''This text file contains a shebang or is located in a path dedicated for executables, but lacks the executable bits and cannot thus be executed.  If the file is meant to be an executable script, add the executable bits, otherwise remove the shebang or move the file elsewhere.'''

Since it's the Python Modules' path and not a path for executables, the shebang is harmless ... but useless. Probably that's why rpmlint points it out.
Comment 11 Jamie Duncan 2014-06-05 11:16:13 EDT
Michael, Christopher, et al,

Thanks again for the continued feedback. I've made the above changes and things continue to look cleaner. 

koji builds cleanly, and fedora-review now shows cleaner as well.

Cheers,

Jamie Duncan
Comment 13 Jamie Duncan 2014-06-25 20:05:12 EDT
reason for needinfo 
I've pinged into #fedora-devel and asked around to those I know. How do I help this get some traction?
Comment 14 Tom "spot" Callaway 2014-07-07 13:25:30 EDT
Package Review
===============

rpmlint reports:

soscleaner.src: W: spelling-error Summary(en_US) sosreport -> misreport, presort
soscleaner.src: W: file-size-mismatch soscleaner-0.1.tar.gz = 14660, http://people.redhat.com/jduncan/soscleaner/soscleaner-0.1.tar.gz = 15244
soscleaner.noarch: W: spelling-error Summary(en_US) sosreport -> misreport, presort

The spelling-errors are false positives, but you should see why the tarballs do not match.

- package meets naming guidelines
- package meets packaging guidelines
- license (GPLv2) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (noarch)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file

Simple package, easy review, APPROVED. Please do check that the tarballs match up before committing the bits to Fedora.
Comment 17 Tom "spot" Callaway 2014-07-16 17:09:56 EDT
You're now sponsored. Lifting FE-NEEDSPONSOR.
Comment 18 Jamie Duncan 2014-07-16 17:18:51 EDT
New Package SCM Request
=======================
Package Name: soscleaner
Short Description: analyse and scrub sensitive sosreport data
Upstream URL: https://github.com/jduncan-rva/soscleaner
Owners: jduncan
Branches: el6 epel7 f20 f21
InitialCC: jduncan
Comment 19 Paul W. Frields 2014-07-16 17:24:44 EDT
Spot, I was just talking to Jamie in IRC, apparently while you were working on this.  I'm happy to {co-,}sponsor him for this package.  I had set up a watch on his email and hadn't got to the step of noting in this bug that I'd take the sponsorship.
Comment 20 Petr Pisar 2014-07-17 02:19:58 EDT
Guys, please follow the Fedora review processes <https://fedoraproject.org/wiki/Package_Review_Process>. This review is neither in ASSIGNED state, neither has fedora-review+. So there is no point in requesting fedora‑cvs? (And I do not comment reassigning the review to cvs component.)
Comment 21 Jon Ciesla 2014-07-17 08:04:53 EDT
Review flag not set.
Comment 22 Jamie Duncan 2014-07-17 08:37:20 EDT
setting flags back to what they're supposed to be. 
fedora-review was ack'd in c#14.

resetting fedora-cvs flag to ?
Comment 23 Jon Ciesla 2014-07-17 11:46:19 EDT
Git done (by process-git-requests).
Comment 24 Fedora Update System 2014-07-17 16:29:12 EDT
soscleaner-0.2-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/soscleaner-0.2-1.fc20
Comment 25 Fedora Update System 2014-07-17 16:47:42 EDT
soscleaner-0.2-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/soscleaner-0.2-1.el6
Comment 26 Fedora Update System 2014-07-19 02:01:22 EDT
soscleaner-0.2-1.fc20 has been pushed to the Fedora 20 testing repository.
Comment 27 Fedora Update System 2014-07-22 14:08:46 EDT
soscleaner-0.2-1.el6 has been pushed to the Fedora EPEL 6 stable repository.
Comment 28 Fedora Update System 2014-07-22 23:01:12 EDT
soscleaner-0.2-1.fc20 has been pushed to the Fedora 20 stable repository.

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