Spec URL: https://siddharths.fedorapeople.org/SPECS/sqlcipher.spec SRPM URL: https://siddharths.fedorapeople.org/SRPMS/sqlcipher-3.3.1-1.fc23.src.rpm Description: SQLCipher is an SQLite extension that provides transparent 256-bit AES encryption of database files. Pages are encrypted before being written to disk and are decrypted when read back. Due to the small footprint and great performance itβs ideal for protecting embedded application databases and is well suited for mobile development. Fedora Account System Username: siddharths
Offhand, in -devel subpkg I see: 1. %{_includedir}/%{name}/sqlite3ext.h %{_includedir}/%{name}/sqlite3.h which implies that %{_includedir}/%{name} dir is not owned, imo, easier to replace those 2 lines with just: %{_includedir}/sqlcipher/ (personal preference of mine to not use %name macro here, so that it'll continue to work regardless of the actual package name) 2. %{_libdir}/libsqlcipher-3.8.10.2.so.0 %{_libdir}/libsqlcipher-3.8.10.2.so.0.8.6 these libraries should be in the main pkg (not -devel), and include ldconfig scriptlets 3. %{_libdir}/libsqlcipher.la libtool archives should not be included in packaging, this file should be deleted or otherwise excluded. 4. %package -devel MUST have a tighter dependency on the main pkg, instead of Requires: %{name} = %{version} use Requires: %{name}%{?_isa} = %{version}-%{release} 5. SHOULD document why these compiler flags are needed in a .spec comment CFLAGS="-DSQLITE_HAS_CODEC -DSQLITE_TEMP_STORE=2" LDFLAGS="-lcrypto" 6. While you're at it, MUST respect distro compiler flags, using (something like) CFLAGS="%{optflags} -DSQLITE_HAS_CODEC -DSQLITE_TEMP_STORE=2" LDFLAGS="%{?__global_ldflags} -lcrypto"
Hi, is there any progress with this package? I could do the official review.
(In reply to Raphael Groner from comment #2) > Hi, > is there any progress with this package? I could do the official review. Thanks I will fix it today.
(In reply to Raphael Groner from comment #2) > Hi, > is there any progress with this package? I could do the official review. new SPEC and SRPM SPEC: https://siddharths.fedorapeople.org/SPECS/sqlcipher.spec SRPM: https://siddharths.fedorapeople.org/SRPMS/sqlcipher-3.3.1-2.fc23.src.rpm
It would also be good to look at this bug https://bugzilla.redhat.com/show_bug.cgi?id=996813 while reviewing new specs in https://bugzilla.redhat.com/show_bug.cgi?id=1310294#c4
*** Bug 996813 has been marked as a duplicate of this bug. ***
Why do you set the fedora-review flag for yourself? That's not in conjunction with the common process, instead a reviewer have to do that (besides also assigning the bug). https://fedoraproject.org/wiki/Package_Review_Process#Reviewer
I anyone looking to do fedora-review
Is anyone looking to do fedora-review
What's your FAS account? Please use the same e-mail address and name for your Bugzilla account. Otherwise, people have trouble to identify you.
(In reply to Siddharth Sharma from comment #4) > (In reply to Raphael Groner from comment #2) > > Hi, > > is there any progress with this package? I could do the official review. > > new SPEC and SRPM > > SPEC: https://siddharths.fedorapeople.org/SPECS/sqlcipher.spec > SRPM: > https://siddharths.fedorapeople.org/SRPMS/sqlcipher-3.3.1-2.fc23.src.rpm Review swap with bug #1343733?
(In reply to Raphael Groner from comment #10) > What's your FAS account? Please use the same e-mail address and name for > your Bugzilla account. Otherwise, people have trouble to identify you. FAS: siddharths
Are you interested in a review swap? See comment #11.
Are you interested in some review for mono packages to do a review swap?
I can review today
Looks good, all the blocker items mentioned previously were addressed. Remaining items to consider: 5. SHOULD document why these compiler flags are needed in a .spec comment CFLAGS="-DSQLITE_HAS_CODEC -DSQLITE_TEMP_STORE=2" LDFLAGS="-lcrypto" ... I'd suggest a comment: # recommended in README.md ## Compiling section 7a. MUST use %%license tag, instead of: %doc LICENSE README.md use %doc README.md %license LICENSE 7b. SHOULD, since main pkg already includes README.md and LICENSE, no need to include in -devel and -tcl subpkg's too, please remove them. 8. (since you're already using %{make_install}), SHOULD use convenience macro %{make_build} instead of make %{?_smp_mflags} use %{make_build} naming: ok sources: ok 26be3c23220192fb42e1d60f8c90ac69 v3.3.1.tar.gz macros: ok scriptlets: ok licensing: NOT ok 9. MUST use License: BSD to match recommendation on https://fedoraproject.org/wiki/Licensing 10a. SHOULD consider using build option: --disable-tcl and avoid the hassle of the -tcl subpkg, unless you specifically are willing to support this. 10b. MUST: If you choose to keep -tcl subpkg, then must add a versioned dependency to main pkg: Requires: %{name}%{?_isa} = %{version}-%{release} 11. MUST remove %define _unpackaged_files_terminate_build 0 there are better ways of fixing unpackaged files than this hack, for example, adding this after %{make_install}: rm -fv %{buildroot}%{_libdir}/lib*.la 12. MUST fix rpath issues, doing a local build, I get: ERROR 0001: file '/usr/share/tcl8.6/sqlite3/libtclsqlite3.so' contains a standard rpath '/usr/lib64' in [/usr/lib64] ERROR 0001: file '/usr/bin/sqlcipher' contains a standard rpath '/usr/lib64' in [/usr/lib64] per https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Removing_Rpath adding after %configure, this helps: 13. SHOULD remove deprecated items/tags from .spec, including: (from %install section): rm -rf $RPM_BUILD_ROOT Please fix all MUST (blocker) items, and I will approve this package review. Consider addressing SHOULD (optional) items too, and you'll be extra awesome. If it helps, here's a copy of https://rdieter.fedorapeople.org/rpms/sqlcipher.spec that implements all the suggested fixes (I think).
re-reading, looks like my item 12 was missing the actual spec suggestion: ... this helps: # fix/workaround hard-coded rpaths sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool
(In reply to Rex Dieter from comment #17) > re-reading, looks like my item 12 was missing the actual spec suggestion: > ... this helps: > # fix/workaround hard-coded rpaths > sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' > libtool > sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool Thanks a lot Rex :) Will fix it asap.
(In reply to Rex Dieter from comment #17) > re-reading, looks like my item 12 was missing the actual spec suggestion: > ... this helps: > # fix/workaround hard-coded rpaths > sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' > libtool > sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool Updated for Review SPECS: https://siddharths.fedorapeople.org/SPECS/sqlcipher.spec SRPMS: https://siddharths.fedorapeople.org/SRPMS/sqlcipher-3.3.1-4.fc24.src.rpm I removed tcl from packaging and openssl-devel was missing from build requires.
Looks good, APPROVED
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/sqlcipher
builds submitted http://koji.fedoraproject.org/koji/taskinfo?taskID=15039542 http://koji.fedoraproject.org/koji/taskinfo?taskID=15039547 http://koji.fedoraproject.org/koji/taskinfo?taskID=15039559 http://koji.fedoraproject.org/koji/taskinfo?taskID=15039582
Are you planning on submitting updates for f24/f23 to bodhi? (I don't see any yet)
sqlcipher-3.3.1-4.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-068d457ba0
sqlcipher-3.3.1-4.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-5ff49b651b
sqlcipher-3.3.1-4.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-5ff49b651b
sqlcipher-3.3.1-4.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-068d457ba0
Can this be closed as sqlcipher is already available now.
Yes, closing