Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225667
Summary: Merge Review: cryptsetup-luks Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: nobody@fedoraproject.org QAContact: fedora-package-review@redhat.com CC: pjones@redhat.com
Fedora Merge Review: cryptsetup-luks
http://cvs.fedora.redhat.com/viewcvs/devel/cryptsetup-luks/ Initial Owner: pjones@redhat.com
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cryptsetup-luks
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225667
gauret@free.fr changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |gauret@free.fr
------- Additional Comments From gauret@free.fr 2007-02-28 09:20 EST ------- Just a few things I noticed : in the packaged %doc, there's a generic autotools INSTALL file, and two empty files (NEWS and README). Those three files should not be included in %doc, they're useless.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cryptsetup-luks
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225667
kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |NEEDINFO AssignedTo|nobody@fedoraproject.org |kevin@tummy.com Flag| |fedora-review?, | |needinfo?(pjones@redhat.com)
------- Additional Comments From kevin@tummy.com 2007-03-06 00:15 EST -------
OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. OK - License (GPL) OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: e134b82b4706a28ba1d73b9176d5ad0c cryptsetup-luks-1.0.3.tar.bz2 e134b82b4706a28ba1d73b9176d5ad0c cryptsetup-luks-1.0.3.tar.bz2.1 OK - BuildRequires correct OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Package has correct buildroot OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime.
OK - Headers/static libs in -devel subpackage. OK - Spec has needed ldconfig in post and postun OK - .so files in -devel subpackage. See below - -devel package Requires: %{name} = %{version}-%{release} OK - .la files are removed.
OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. See below - No rpmlint output. OK - final provides and requires are sane
SHOULD Items:
OK - Should build in mock. OK - Should build on all supported archs OK - Should function as described. See below - Should have subpackages require base package with fully versioned depend. OK - Should have dist tag See below - Should package latest version 8 outstanding bugs - check for outstanding bugs on package.
Issues:
1. Might have the devel subpackage Requires: %{name} = %{version}-%{release} instead of just Requires: %{name} = %{version}
2. Ditto the docs comments from comment #2.
3. rpmlint says: E: cryptsetup-luks statically-linked-binary /sbin/cryptsetup
Needs to be static per the bug mentioned in the changelog.
E: cryptsetup-luks zero-length /usr/share/doc/cryptsetup-luks-1.0.3/NEWS E: cryptsetup-luks zero-length /usr/share/doc/cryptsetup-luks-1.0.3/README
Should be removed, per issue 2.
W: cryptsetup-luks-devel no-documentation
Can be ignored.
4. 1.0.4 is out, perhaps upgrade to that?
5. Should the static lib be shipped in the devel package here? Is there anything that uses it?
6. 8 outstanding bugs. Might look at those. I think at least one should be solved by upgrading to 1.0.4, possibly more.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cryptsetup-luks
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225667
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |medium Priority|normal |medium
mgarski@post.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mgarski@post.pl
------- Additional Comments From mgarski@post.pl 2007-06-09 15:48 EST ------- Now we have 1.0.5 release, where cryptsetup-luks becomes cryptsetup.
If you upgrade to this release bug #215199 and bug #217983 can be resolved.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cryptsetup-luks
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225667
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Product|Fedora Extras |Fedora
opensource@till.name changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEEDINFO |ASSIGNED CC| |opensource@till.name Flag|needinfo?(pjones@redhat.com)|
------- Additional Comments From opensource@till.name 2007-08-17 08:34 EST ------- (In reply to comment #2)
- Might have the devel subpackage
Requires: %{name} = %{version}-%{release} instead of just Requires: %{name} = %{version}
committed in devel
- Ditto the docs comments from comment #2.
commited in devel
- rpmlint says:
E: cryptsetup-luks statically-linked-binary /sbin/cryptsetup
Needs to be static per the bug mentioned in the changelog.
not anymore, it is build dynamically in devel
E: cryptsetup-luks zero-length /usr/share/doc/cryptsetup-luks-1.0.3/NEWS E: cryptsetup-luks zero-length /usr/share/doc/cryptsetup-luks-1.0.3/README
Should be removed, per issue 2
commited in devel
Can be ignored.
- 1.0.4 is out, perhaps upgrade to that?
Updated to 1.0.5 in devel
- Should the static lib be shipped in the devel package here?
Is there anything that uses it?
Static stuff has been removed in devel.
- 8 outstanding bugs. Might look at those. I think at least one should
be solved by upgrading to 1.0.4, possibly more.
done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cryptsetup-luks
https://bugzilla.redhat.com/show_bug.cgi?id=225667
opensource@till.name changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEEDINFO Flag| |needinfo?(kevin@tummy.com)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cryptsetup-luks
https://bugzilla.redhat.com/show_bug.cgi?id=225667
kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEEDINFO |ASSIGNED Flag|needinfo?(kevin@tummy.com) |
------- Additional Comments From kevin@tummy.com 2007-08-28 19:25 EST ------- Sorry for the delay, I've been swamped of late.
I promise to try and finish this up this week...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cryptsetup-luks
https://bugzilla.redhat.com/show_bug.cgi?id=225667
------- Additional Comments From mgarski@post.pl 2007-08-29 17:31 EST ------- FYI: 1. URL has changed to http://luks.endorphin.org/ 2. License is GPLv2 3. ChangeLog needs to be recoded to UTF-8
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cryptsetup-luks
https://bugzilla.redhat.com/show_bug.cgi?id=225667
------- Additional Comments From opensource@till.name 2007-08-29 17:57 EST ------- (In reply to comment #6)
- URL has changed to http://luks.endorphin.org/
- License is GPLv2
- ChangeLog needs to be recoded to UTF-8
changed in cvs for devel
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cryptsetup-luks
https://bugzilla.redhat.com/show_bug.cgi?id=225667
kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
------- Additional Comments From kevin@tummy.com 2007-08-29 22:11 EST ------- Sorry again for the delay.
Minor nitpick: any reason for not using %{?_smp_mflags} on your make ?
Do you plan to rename this over to cryptsetup?
I see no further blockers, so this package is APPROVED. Feel free to close this rawhide.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cryptsetup-luks
https://bugzilla.redhat.com/show_bug.cgi?id=225667
opensource@till.name changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |RAWHIDE
------- Additional Comments From opensource@till.name 2007-08-30 07:11 EST ------- (In reply to comment #8)
Sorry again for the delay.
No problem, thank you for your review.
Minor nitpick: any reason for not using %{?_smp_mflags} on your make ?
I added it to cvs for devel and a scratch build worked fine, so I will keep it.
Do you plan to rename this over to cryptsetup?
Yes, I already asked how to do this but got no satisfactory reply: https://www.redhat.com/archives/fedora-maintainers/2007-August/msg00419.html The main issues are that there is already a cryptsetup Bugzilla component and that I do not know, whether someone will really take care, that the cryptsetup-luks and cryptsetup component in Bugzilla are merged. Also there seems no way to move the acls in the package db yet. For me it is no a high priority task to rename the package, therefore I want to wait until Fedora's infrastructure really support renaming packages.
I see no further blockers, so this package is APPROVED. Feel free to close this rawhide.
Thank you again. :-)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cryptsetup-luks
https://bugzilla.redhat.com/show_bug.cgi?id=225667
------- Additional Comments From kevin@tummy.com 2007-08-30 12:59 EST ------- Well, at least for now, we have decided to do package renames as follows:
- Create the new package name. - Follow the end of life package guidelines on the old package in the branches where it is being replaced. - Maintainer imports the package into the new name (with Obsoletes/Provides if needed).
The cvs history is maintained in the old package's cvs.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cryptsetup-luks
https://bugzilla.redhat.com/show_bug.cgi?id=225667
------- Additional Comments From opensource@till.name 2007-08-30 13:18 EST ------- (In reply to comment #10)
Well, at least for now, we have decided to do package renames as follows:
- Create the new package name.
How is this done? Just rebuild cryptsetup-luks with name cryptsetup and a cryptsetup.spec?
And are you sure that this will not break anything because there is already a cryptsetup component in Bugzilla? (There was a cryptsetup packege in FC 2 and 3) according to http://cvs.fedora.redhat.com/viewcvs/rpms/cryptsetup/?root=core
And do I have to manually copy the acls from PackageDB into a fedora-cvs request template?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cryptsetup-luks
https://bugzilla.redhat.com/show_bug.cgi?id=225667
------- Additional Comments From kevin@tummy.com 2007-08-31 23:10 EST -------
How is this done? Just rebuild cryptsetup-luks with name cryptsetup and a cryptsetup.spec?
Well, you need to do a cvsrequest to make the new module, but then yes, you just import the new name and spec.
And are you sure that this will not break anything because there is already a cryptsetup component in Bugzilla? (There was a cryptsetup packege in FC 2 and 3) according to http://cvs.fedora.redhat.com/viewcvs/rpms/cryptsetup/?root=core
Well, I don't think so since they are so old... are there any open bugs on that name? You might have to deal with them...
And do I have to manually copy the acls from PackageDB into a fedora-cvs request template?
Yeah, at least as far as I know. I can ask if perhaps someone could copy it over in the database or something if it's difficult to request.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: cryptsetup-luks
https://bugzilla.redhat.com/show_bug.cgi?id=225667
------- Additional Comments From opensource@till.name 2007-09-01 04:29 EST ------- (In reply to comment #12)
Well, I don't think so since they are so old... are there any open bugs on that name? You might have to deal with them...
There was one open bug which was meant to be filed against cryptsetup-luks and I guess the reporter was confused with the two components. I only noticed it by coincidence. This seems not to be well handled, two. E.g. there seems no way to become initialCC of cryptsetup now, because it is not in the PackageDB. I guess the same problem will occur the other way round when cryptsetup-luks will be renamed.
package-review@lists.fedoraproject.org