Bug 221175 - Review Request: libisofs - A library to create ISO 9660 disk images
Summary: Review Request: libisofs - A library to create ISO 9660 disk images
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
(Show other bugs)
Version: rawhide
Hardware: All Linux
medium
medium
Target Milestone: ---
Assignee: Ville Skyttä
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Keywords:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2007-01-02 18:58 UTC by Jesse Keating
Modified: 2015-05-20 11:00 UTC (History)
3 users (show)

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: ---
limburgher: fedora-cvs+


Attachments (Terms of Use)

Description Jesse Keating 2007-01-02 18:58:35 UTC
Spec URL: http://people.redhat.com/jkeating/extras/libisofs.spec
SRPM URL: http://people.redhat.com/jkeating/extras/libisofs-0.2.3-1.src.rpm
Description: 
libisofs the library to pack up hard disk files and directories into a
ISO 9660 disk image. This may then be brought to CD via libburn.
libisofs is to be the foundation of our upcoming mkisofs emulation.

This software used to be a part of the libburn source package but has been split out into its own tarball, and thus its own srpm.

Comment 1 Ville Skyttä 2007-01-02 20:43:44 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


Comment 2 Ville Skyttä 2007-01-02 20:45:41 UTC
(In reply to comment #1)
> - All build dependencies are spurious

Sorry, libburn-devel is not, but others apparently are.

Comment 3 Jesse Keating 2007-01-02 21:22:31 UTC
(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.


Comment 4 Ville Skyttä 2007-01-02 22:13:20 UTC
(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.

Comment 5 Jesse Keating 2007-01-03 03:23:48 UTC
(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.

Comment 6 Jesse Keating 2007-01-03 20:26:23 UTC
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.

Comment 7 Ville Skyttä 2007-01-07 11:26:10 UTC
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.

Comment 8 Jesse Keating 2007-01-08 22:30:44 UTC
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

Comment 9 Ville Skyttä 2007-01-09 19:11:51 UTC
(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.

Comment 10 Jesse Keating 2007-01-15 15:44:39 UTC
Built, asking for FC-6 branch.  Added to owners.list too.

Comment 11 Jesse Keating 2007-03-13 17:07:09 UTC
Please add 'denis' as a co-owner of this module.

Comment 12 Warren Togami 2007-03-13 22:04:01 UTC
Please provide the bugzilla name.

Comment 13 Jesse Keating 2007-03-14 00:59:29 UTC
denis@poolshark.org

Comment 14 Robert Scheck 2010-02-16 00:02:33 UTC
Package Change Request
======================
Package Name: libisofs
New Branches: EL-4 EL-5
Owners: robert

Comment 15 Kevin Fenzi 2010-02-16 03:40:32 UTC
cvs done.

Comment 17 Robert Scheck 2015-05-19 23:03:13 UTC
Package Change Request
======================
Package Name: libisofs
New Branches: epel7
Owners: robert

Comment 18 Gwyn Ciesla 2015-05-20 11:00:22 UTC
Git done (by process-git-requests).


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