Spec URL: http://dl.dropbox.com/u/41369/fedora/SPECS/vboot-utils.spec SRPM URL: http://dl.dropbox.com/u/41369/fedora/SRPMS/vboot-utils-20130109gite8cfa31-1.fc18.src.rpm Description: Verified Boot utility from Chromium OS. This collection of tools will help Fedora install onto the 1012 Sansung Chromebook computer. The collection contains tools to format a GTP disklabel, pack & cryptography sign the kernel, among other useful things. Fedora Account System Username: parasense
typo: s/1012/2012/ Please note that I've tested this to work in a mock environment, so I have good faith that the package would build in koji too.
Hi, a few notes : - the patches are not documented, and should mention how they were sent upstream ( if done ) - there is no comment on how the tarball got generated - why does it requires this : BuildRequires: glibc-static - the package do not seems to use the %optflag macro, and so not use the regular build option.
Also, it doesn't build on mock on my f18 cc -DCHROMEOS_ENVIRONMENT -Wall -Werror -DNDEBUG -I/builddir/build/BUILD/vboot-utils-20130109gite8cfa31/firmware/include -I/builddir/build/BUILD/vboot-utils-20130109gite8cfa31/firmware/stub/include -I./include -I/builddir/build/BUILD/vboot-utils-20130109gite8cfa31/firmware/lib/include -I/builddir/build/BUILD/vboot-utils-20130109gite8cfa31/firmware/lib/cgptlib/include -I/builddir/build/BUILD/vboot-utils-20130109gite8cfa31/firmware/lib/cryptolib/include -I/builddir/build/BUILD/vboot-utils-20130109gite8cfa31/firmware/lib/tpm_lite/include -I/builddir/build/BUILD/vboot-utils-20130109gite8cfa31/host/include -MMD -MF /builddir/build/BUILD/vboot-utils-20130109gite8cfa31/build/utility/dump_kernel_config.d dump_kernel_config_main.c -o /builddir/build/BUILD/vboot-utils-20130109gite8cfa31/build/utility/dump_kernel_config \ /builddir/build/BUILD/vboot-utils-20130109gite8cfa31/build/vboot_host.a /builddir/build/BUILD/vboot-utils-20130109gite8cfa31/build/libdump_kernel_config.a -lcrypto bmpblk_utility.cc: In member function 'void vboot_reference::BmpBlockUtil::load_yaml_config(const char*)': bmpblk_utility.cc:102:61: error: format '%u' expects argument of type 'unsigned int', but argument 2 has type 'std::vector<std::basic_string<char> >::size_type {aka long unsigned int}' [-Werror=format] bmpblk_utility.cc:106:59: error: format '%u' expects argument of type 'unsigned int', but argument 2 has type 'std::map<std::basic_string<char>, vboot_reference::ImageConfig>::size_type {aka long unsigned int}' [-Werror=format] bmpblk_utility.cc:117:61: error: format '%u' expects argument of type 'unsigned int', but argument 2 has type 'std::map<std::basic_string<char>, vboot_reference::ScreenConfig>::size_type {aka long unsigned int}' [-Werror=format] bmpblk_utility.cc: In member function 'void vboot_reference::BmpBlockUtil::pack_bmpblock()': bmpblk_utility.cc:595:70: error: format '%u' expects argument of type 'unsigned int', but argument 3 has type 'std::basic_string<char>::size_type {aka long unsigned int}' [-Werror=format] bmpblk_utility.cc:607:59: error: format '%u' expects argument of type 'unsigned int', but argument 3 has type 'std::basic_string<char>::size_type {aka long unsigned int}' [-Werror=format] cc1plus: all warnings being treated as errors make[1]: Leaving directory `/builddir/build/BUILD/vboot-utils-20130109gite8cfa31/utility' make[1]: *** [/builddir/build/BUILD/vboot-utils-20130109gite8cfa31/build/utility/bmpblk_utility.o] Error 1 make: *** [all] Error 2 erreur : Mauvais statut de sortie pour /var/tmp/rpm-tmp.KWRVaz (%build) Erreur de construction de RPM :
Fixed the problem with patch1 that caused Michael Scherer to have build failure. Should build on all architectures now. Sent the printf patch upstream to chromium-os developers. http://code.google.com/p/chromium-os/issues/detail?id=37804 Once that have incorporated the fix I'll disable the patch in the source rpm. Will also add comments in the SPEC file describing the patches. I will poke at the build requires, ensure glibc-static is a legitimate requirement. My hope is that after disablinng -static build flags that the static lib versions will not be needed. Will investigate the %optflag macro. hope to update the ticket latere with new url to the improved package.
I have updated the spec file and source RPM for your evaluation: SPEC FILE: http://dl.dropbox.com/u/41369/fedora/SPECS/vboot-utils.spec SOURCE RPM: http://dl.dropbox.com/u/41369/fedora/SRPMS/vboot-utils-20130129git68f54d4-1.fc18.src.rpm Please let me know if the SPEC file can improve. Thanks in advance!
Created attachment 690051 [details] Remove a couple minor rpmlint warnings This patch fixes a couple a warning about mixing spaces and tabs and the incoherent changelog entry.
The package plays with CFLAGS. It would be nice if that it just used the Fedora supplied CFLAGS. In the rpmlint results see that it isn't using the '-g' and there isn't any debuginfo information to strip out: vboot-utils-debuginfo.armv7hl: E: debuginfo-without-sources
Tweaked the vboot-utils.spec to exclude some variations of s390 and ppc. Probably don't want to build on those. Also included the patch in comment#6. The source rpm is available from: http://people.redhat.com/wcohen/chromebook/vboot-utils-20130129git68f54d4-2.fc17.src.rpm Both the f18 sratch builds are failing on f18. Arm scratch build for f18: http://arm.koji.fedoraproject.org/koji/taskinfo?taskID=1419159 The i386/x86-64 scratch builds for f18 at: http://koji.fedoraproject.org/koji/taskinfo?taskID=4927856
The srpm built locally on the arm, so it looks like there is some difference between the local and koji arm build.
I'll try and get an initial review done this week.
There appears to be a dependency problem in the vboot-utils make file, so parallel builds have a problem. The following srpm turns off the parallel builds: http://people.redhat.com/wcohen/chromebook/vboot-utils-20130129git68f54d4-3.fc17.src.rpm It built successfully on arm: http://arm.koji.fedoraproject.org/koji/taskinfo?taskID=1421410 It built successfully on x86_64, but still has some other problem on i686): http://koji.fedoraproject.org/koji/taskinfo?taskID=4930021 On i686 looks like it has a problem figuring out how to build crosssystem_arch.o: cc -DCHROMEOS_ENVIRONMENT -Wall -Werror -DNDEBUG -MMD -MF /builddir/build/BUILD/vboot-utils-20130129git68f54d4/build/firmware/linktest/main.o.d -Ifirmware/include -Ifirmware/lib/include -Ifirmware/lib/cgptlib/include -Ifirmware/lib/cryptolib/include -Ifirmware/lib/tpm_lite/include -Ifirmware/stub/include -c -o /builddir/build/BUILD/vboot-utils-20130129git68f54d4/build/firmware/linktest/main.o firmware/linktest/main.c make: *** No rule to make target `/builddir/build/BUILD/vboot-utils-20130129git68f54d4/build/host/arch/%{ARCH}/lib/crossystem_arch.o', needed by `/builddir/build/BUILD/vboot-utils-20130129git68f54d4/build/vboot_host.a'. Stop. RPM build errors:
(In reply to comment #11) > On i686 looks like it has a problem figuring out how to build > crosssystem_arch.o: Try %ifarch %ix86 instead of %ifarch i386. May-be, using ARCH=%{_arch} also is an option (works for x86_64, ix86, but dunno if this works for the arm). > cc -DCHROMEOS_ENVIRONMENT -Wall -Werror -DNDEBUG -MMD -MF > /builddir/build/BUILD/vboot-utils-20130129git68f54d4/build/firmware/linktest/ => Package does not honor optflags/RPM_OPT_FLAGS.
Yes, figured out that test for i686 was wrong this morning. Have srpm that fixes that: http://people.redhat.com/wcohen/chromebook/vboot-utils-20130129git68f54d4-4.fc17.src.rpm
Pulled down the latest upstream git. Removed printf formating patch (fixed upstream). Fixed patch to disable static build. Got rid of BuildRequires for C++ & libstdc++. use XZ instead of BZIP2 in the source archive for smaller SRPM size. Enabled %check tests, but ignore test failures for the sake of mock. Still need to spend time with package to get it to honor opt flags, use the right triplet. However, it seems to works just fine. For the sake of release early and often here is the latest: http://dl.dropbox.com/u/41369/fedora/SPECS/vboot-utils.spec http://dl.dropbox.com/u/41369/fedora/SRPMS/vboot-utils-20130222gite6cf2c2-1.fc18.src.rpm
You don't need %check for Fedora. FYI
(In reply to comment #15) > You don't need %check for Fedora. FYI You don't need it but if the package supports it you should use it and I've found it picks up a lot of legitimate bugs
So does rpmlint. I'm more worried about the licensing. LICENSE file isn't clear, and the f18 scratch build failed: https://koji.fedoraproject.org/koji/taskinfo?taskID=5046640
Jon, Please host this on your fedora people account instead of dropox for the time being.
Will make sure to test x86 going forward. The LICENSE file is a standard 3-clause BSD license. That is why the SPEC files has "BSD" for the license type. http://opensource.org/licenses/BSD-3-Clause Change log: - put back the patch for c++ printf formating error. - put back gcc-c++ and libstdc++ - fixed i686 build, and tested. Here are the new files: http://dl.dropbox.com/u/41369/fedora/SRPMS/vboot-utils-20130222gite6cf2c2-2.fc18.src.rpm http://dl.dropbox.com/u/41369/fedora/SPECS/vboot-utils.spec Thanks!
few notes: - drop the unneeded BRs (glibc, gcc, ...) - see https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Exceptions_2 - if supported arches are arm* and x86* only, then using "ExclusiveArch: %{arm} %{ix86} x86_64" will be preferred - add steps required to obtain the source archive - see https://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control
One of the things that bothered me about vboot-utils was it didn't use the CFLAGS passed in by rpmbuild. Turning that on uncovered a couple complaints. One was with misuse of strncat that should be fixed in the upstream version. Also found the arm version wanting to link with libpthread. However, there is probably a better way to fix that than just adding a "-lpthread". The following url has the suggestions from comment #20: http://people.redhat.com/wcohen/chromebook/vboot-utils-20130222gite6cf2c2-3.fc17.src.rpm Did some scratch builds to make sure it builds: arm: http://arm.koji.fedoraproject.org/koji/taskinfo?taskID=1546691 x86: http://koji.fedoraproject.org/koji/taskinfo?taskID=5093195
Some initial feedback: Because it's only ARM and x86 the ExcludeArch should be: ExclusiveArch: %{ix86} x86_64 %{arm} You also need to put as comments the exact commands used to generate the source tar file and record either the tag or the git ID used for the build as per these guidelines: http://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control It also needs a %description field. For patch lines I tend to do the following primarily for readability: %patch0 -p0 -b .nostatic %patch1 -p0 -b .fixprintf Rest of the review in progress.
Drop: BuildRequires: glibc gcc As per http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires Exceptions
There's a few minor bits to be cleaned up but we're mostly there. ? rpmlint output: The output below is mostly OK, there's a few things like spaces vs tabs need cleanup. rpmlint vboot-utils.spec ../SRPMS/vboot-utils-20130222gite6cf2c2-2.fc18.src.rpm vboot-utils.spec:18: W: mixed-use-of-spaces-and-tabs (spaces: line 18, tab: line 1) vboot-utils.spec: W: invalid-url Source0: vboot-utils-20130222gite6cf2c2.tar.xz vboot-utils.src: W: spelling-error %description -l en_US chromebook -> chrome book, chrome-book, chromosome vboot-utils.src: W: spelling-error %description -l en_US gpt -> got, gt, pt vboot-utils.src:18: W: mixed-use-of-spaces-and-tabs (spaces: line 18, tab: line 1) vboot-utils.src: W: invalid-url Source0: vboot-utils-20130222gite6cf2c2.tar.xz 1 packages and 1 specfiles checked; 0 errors, 6 warnings. rpmlint vboot-utils-20130222gite6cf2c2-2.fc20.x86_64.rpm vboot-utils.x86_64: W: spelling-error %description -l en_US chromebook -> chrome book, chrome-book, chromosome vboot-utils.x86_64: W: spelling-error %description -l en_US gpt -> got, gt, pt vboot-utils.x86_64: W: no-manual-page-for-binary dump_kernel_config vboot-utils.x86_64: W: no-manual-page-for-binary dev_make_keypair vboot-utils.x86_64: W: no-manual-page-for-binary dev_debug_vboot vboot-utils.x86_64: W: no-manual-page-for-binary bmpblk_font vboot-utils.x86_64: W: no-manual-page-for-binary bmpblk_utility vboot-utils.x86_64: W: no-manual-page-for-binary tpmc vboot-utils.x86_64: W: no-manual-page-for-binary tpm_init_temp_fix vboot-utils.x86_64: W: no-manual-page-for-binary enable_dev_usb_boot vboot-utils.x86_64: W: no-manual-page-for-binary dumpRSAPublicKey vboot-utils.x86_64: W: no-manual-page-for-binary vbutil_kernel vboot-utils.x86_64: W: no-manual-page-for-binary futility vboot-utils.x86_64: W: no-manual-page-for-binary vbutil_firmware vboot-utils.x86_64: W: no-manual-page-for-binary eficompress vboot-utils.x86_64: W: no-manual-page-for-binary pad_digest_utility vboot-utils.x86_64: W: no-manual-page-for-binary signature_digest_utility vboot-utils.x86_64: W: no-manual-page-for-binary vbutil_keyblock vboot-utils.x86_64: W: no-manual-page-for-binary vbutil_key vboot-utils.x86_64: W: no-manual-page-for-binary efidecompress vboot-utils.x86_64: W: no-manual-page-for-binary vbutil_what_keys vboot-utils.x86_64: W: no-manual-page-for-binary dev_sign_file vboot-utils.x86_64: W: no-manual-page-for-binary dump_fmap vboot-utils.x86_64: W: no-manual-page-for-binary gbb_utility vboot-utils.x86_64: W: no-manual-page-for-binary load_kernel_test vboot-utils.x86_64: W: no-manual-page-for-binary verify_data vboot-utils.x86_64: W: no-manual-page-for-binary crossystem vboot-utils.x86_64: W: no-manual-page-for-binary cgpt 1 packages and 0 specfiles checked; 0 errors, 28 warnings. + package name satisfies the packaging naming guidelines + specfile name matches the package base name + package should satisfy packaging guidelines + license meets guidelines and is acceptable to Fedora + license matches the actual package license + latest version packaged + %doc includes license file + spec file written in American English + spec file is legible ? upstream sources match sources in the srpm + package successfully builds on at least one architecture tested using koji scratch build on both x86 and ARM + BuildRequires list all build dependencies There's a few redundent extras n/a %find_lang instead of %{_datadir}/locale/* n/a binary RPM with shared library files must call ldconfig in %post and %postun+ does not use Prefix: /usr n/a package owns all directories it creates n/a no duplicate files in %files + Package perserves timestamps on install Permissions on files must be set properly + consistent use of macros + package must contain code or permissible content n/a large documentation files should go in -doc subpackage + files marked %doc should not affect package runtime n/a header files should be in -devel n/a static libraries should be in -static n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig' n/a libfoo.so must go in -devel n/a devel must require the fully versioned base + packages should not contain libtool .la files n/a packages containing GUI apps must include %{name}.desktop file + packages must not own files or directories owned by other packages + filenames must be valid UTF-8 Optional: + if there is no license file, packager should query upstream to include it n/a translations of description and summary for non-English languages, if available + reviewer should build the package in mock/koji + the package should build into binary RPMs on all supported architectures n/a review should test the package functions as described + scriptlets should be sane n/a non -devel packages should require fully versioned base n/a pkgconfig files should go in -devel + shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or /usr/sbin n/a Package should have man files. It would nice to have some more docs but not a blocker
I have uploaded the required fixes. SPEC FILE http://dl.dropbox.com/u/41369/fedora/SPECS/vboot-utils.spec SRPM http://dl.dropbox.com/u/41369/fedora/SRPMS/vboot-utils-20130222gite6cf2c2-3.fc18.src.rpm
Looks good. APPROVED.
New Package GIT Request ======================= Package Name: vboot-utils Short Description: Verified boot utils from Chromium-OS Owners: parasense Branches: F-19 F-18 InitialCC:
Git done (by process-git-requests).