Bug 452691
Summary: | Review Request: btrfs-progs - supporting programs for btrfs | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Josef Bacik <josef> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, jbacik, notting |
Target Milestone: | --- | Flags: | j:
fedora-review+
kevin: 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: | 2008-08-11 12:11:30 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: |
Description
Josef Bacik
2008-06-24 14:28:59 UTC
This seems somewhat early, considering there's no kernel support (and the on-disk format isn't even stabilized yet....) How does having this in now help? Just helps me stay ahead of the game, when it does hit mainline I'm going to have a bunch of other crap to do, so I'd like to go ahead and get this into fedora now to cut down on the amount of work I have to do later on. I'll go ahead and review this, but I can't test it at all so I'm asking some of the kernel folks if they'll sign off on it. Full review forthcoming.... The only question I have involves the compiler flags; assuming x86_64, this package uses: -Wall -fno-strict-aliasing -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2 -g -Werror -Os whereas the defaults are -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic I've no particular issues with -Os versus -O2, but my understanding is that -fstack-protector is of some importance, at least. Any reason not to use our default compiler flags, perhaps with s/-O2/-Os/? * source files match upstream: ca261e50a5a66f7169b60d7bb5b9835e6284f4fa81d58e17d942fd3b4934618a btrfs-progs-0.15.tar.bz2 * package meets naming and versioning guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * summary is OK. * description is OK. * dist tag is present. * build root is OK. * license field matches the actual license. * license is open source-compatible. * license text included in package. * latest version is being packaged. * BuildRequires are proper. ? Not sure about compiler flags: -Wall -fno-strict-aliasing -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2 -g -Werror -Os * %clean is present. * package builds in mock (rawhide, x86_64). * package installs properly. * debuginfo package looks complete. * rpmlint is silent. * final provides and requires are sane: btrfs-progs = 0.15-3.fc10 = libuuid.so.1()(64bit) * %check is present; no test suite upstream. I cannot test this package. * no shared libraries are added to the regular linker search paths. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no scriptlets present. * code, not content. * documentation is small, so no -doc subpackage is necessary. * %docs are not necessary for the proper functioning of the package. * no headers. * no pkgconfig files. * no static libraries. * no libtool .la files. i can change the cflags in the spec file for now if you like, the flags that are used are just what chris happened to pick when he wrote the makefile :). I'll probably send something up to add -fstack-protector and -fexceptions. It would probably be best to pass CFLAGS="$RPM_OPT_FLAGS" into the makefile, or to tweak that for -Os if you prefer. That way if we need to mess with the flags for whatever reason the package just needs a rebuild. K updated the package http://people.redhat.com/jwhiter/btrfs-progs.spec http://people.redhat.com/jwhiter/btrfs-progs-0.15-4.fc8.src.rpm This looks fine to me. I guess there's little hope of getting any of the kernel folks to chime in here, and I can't really see any reason to hold this up because I can at least run the tools, I just can't mount any filesystems. APPROVED New Package CVS Request ======================= Package Name: btrfs-progs Short Description: Userspace programs for btrfs Owners: josef Branches: F-9 InitialCC: Cvsextras Commits: yes cvs done. This was built and is in rawhide currently; any reason not to close this ticket? |