Bug 221175
Summary: | Review Request: libisofs - A library to create ISO 9660 disk images | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Jesse Keating <jkeating> |
Component: | Package Review | Assignee: | Ville Skyttä <ville.skytta> |
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | dcantrell, lemenkov, redhat-bugzilla |
Target Milestone: | --- | Flags: | gwync:
fedora-cvs+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-01-15 15:44:39 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Jesse Keating
2007-01-02 18:58:35 UTC
- All build dependencies are spurious - "-n libisofs-devel" could be replaced by just "devel" in devel's %package and %description, and libisofs by %{name} in -devel main pkg dependency - Summary: s/A library/Library/ - Unowned dir /usr/include/libisofs - Add CONTRIBUTORS to %doc - Quite a few executable permissions in -debuginfo source files - Dubious Cflags in *.pc, should be -I${includedir}/libisofs? - Is the libburn dependency really needed in the main package? - Bad grammar in %description: "libisofs the library to ...", dunno if the last sentence of it is needed, it's subject to bitrot (In reply to comment #1) > - All build dependencies are spurious Sorry, libburn-devel is not, but others apparently are. (In reply to comment #1) > - All build dependencies are spurious Ah, probably carryover from when the package was built in libburn. > - "-n libisofs-devel" could be replaced by just "devel" in devel's %package and > %description, and libisofs by %{name} in -devel main pkg dependency Whoops, more carry over from previous package. > - Summary: s/A library/Library/ Fixed. > - Unowned dir /usr/include/libisofs Hrm, I assumed that it would pick up ownership of that since it was creating the dir, my bad. > - Add CONTRIBUTORS to %doc Hurray for new files. > - Quite a few executable permissions in -debuginfo source files This is a complaint with upstream that I've asked them to fix. Is this really a problem that I should chmod the source? > - Dubious Cflags in *.pc, should be -I${includedir}/libisofs? Upstream issue, I'll ask them to fix it for the next release. > - Is the libburn dependency really needed in the main package? I was told yes, but I'll clarify. > - Bad grammar in %description: "libisofs the library to ...", dunno if the last > sentence of it is needed, it's subject to bitrot Yeah, this was their upstream content. http://people.redhat.com/jkeating/extras/libisofs-0.2.3-2.src.rpm Spec url is the same. (In reply to comment #3) > > - Quite a few executable permissions in -debuginfo source files > This is a complaint with upstream that I've asked them to fix. Is this > really a problem that I should chmod the source? Well, it took you many more keystrokes to ask that question than fixing it would have taken... :). Why not? Don't we want proper permissions for all installed files? > > - Dubious Cflags in *.pc, should be -I${includedir}/libisofs? > Upstream issue, I'll ask them to fix it for the next release. Unless I'm missing something, I think it should be patched in this package. Unless of course you prefer waiting for the next upstream release before continuing the review. > > - Is the libburn dependency really needed in the main package? > I was told yes, but I'll clarify. Looks spurious to me. Nothing is linked against libburn, nor it is dlopened, and I don't see anything else in libburn that would be a candidate for explaining the dependency. Caveat: I haven't tried actually using this package for anything. (In reply to comment #4) > (In reply to comment #3) > > > > - Quite a few executable permissions in -debuginfo source files > > This is a complaint with upstream that I've asked them to fix. Is this > > really a problem that I should chmod the source? > > Well, it took you many more keystrokes to ask that question than fixing it would > have taken... :). Why not? Don't we want proper permissions for all installed > files? I'd rather see it fixed upstream. I'll "fix" it if needbe, but this really doesn't hurt anything. > > > - Dubious Cflags in *.pc, should be -I${includedir}/libisofs? > > Upstream issue, I'll ask them to fix it for the next release. > > Unless I'm missing something, I think it should be patched in this package. > Unless of course you prefer waiting for the next upstream release before > continuing the review. As I prefer it to be fixed upstream, I'll either get them to do another release, or at least generate the patch based on an upstream source fix. > > > - Is the libburn dependency really needed in the main package? > > I was told yes, but I'll clarify. > > Looks spurious to me. Nothing is linked against libburn, nor it is dlopened, > and I don't see anything else in libburn that would be a candidate for > explaining the dependency. Caveat: I haven't tried actually using this package > for anything. I should have something more for you tomorrow after discussing with upstream. I'm just playing package monkey here. http://people.redhat.com/jkeating/extras/libisofs-0.2.4-1.src.rpm New upstream tarball (emailed to me, they haven't posted it on their website yet, I"ll wait for that before importing/building). This should resolve the remaining issues. Upstream 0.2.4 tarball is available, but it's not quite the same as the copy included in 0.2.4-1 SRPM, lib micro version has changed. Upstream URLs changed: libburn -> libburnia, libburn-download -> libburnia-download I think the doxygen docs belong to -devel rather than the main package. The dependency on libburn is still there. Yeah, I noticed he changed a few things in the tarball, this srpm uses the now upstream one. Dep on libburn is because libisofs can actually make a call to libburn I'm told. Moving the docs to devel. http://people.redhat.com/jkeating/extras/libisofs-0.2.4-2.src.rpm (In reply to comment #8) > Dep on libburn is because libisofs can actually make a call to libburn I'm told. I've been told that a few times here too, but I'm still too thick to understand how does that happen. Anyway, I won't block the package on that, but suggest finding out the real details and fixing in future package revisions if applicable. Approved. Built, asking for FC-6 branch. Added to owners.list too. Please add 'denis' as a co-owner of this module. Please provide the bugzilla name. denis Package Change Request ====================== Package Name: libisofs New Branches: EL-4 EL-5 Owners: robert cvs done. Package Change Request ====================== Package Name: libisofs New Branches: epel7 Owners: robert Git done (by process-git-requests). |