Bug 228255

I have not performed many reviews, so I may miss things.  It appears that you
need a sponsor - I will be unable to sponsor you.  If you haven't yet, look here
for more info:
http://fedoraproject.org/wiki/Extras/HowToGetSponsored


*Source0 should contain the full URL

*Is there a reason for the duplicate Patch1 and Patch2 and the commented lines
in the %prep section?

*rpmlint output:
W: grub2 summary-ended-with-dot GRUB 2 - the Grub Unified Boot Loader.
W: grub2 non-standard-group System Enviroment/Base
W: grub2 setup-not-quiet
W: grub2 patch-not-applied Patch2: grub2-program-name-tranform-i386-pc.patch

*add "%{?dist}" to Version if this is going in more than one branch.

*can you use "make DESTDIR=%{buildroot} install" or "make
DESTDIR=$RPM_BUILD_ROOT install" instead of %makeinstall?
(see:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002)

Building in mock hasn't completed yet - I'll post the result in the morning.

Created attachment 148896 [details]
mock build log (fedora-6-i386-core)

Failed to build in mock.

Your build system is missing "autoreconf" which I believe is in the autoconf
package. I'll get changes to the grub spec file out shortly.

I've updated the spec file to reflect a lot of your suggestions.

Added
======
 * Source0 now points to a full url
 * fixed up issues from rpmlint
 * fixed duplicate patch, grub2-program-name-tranform-i386-pc.patch should have
been grub2-program-name-tranform-powerpc.patch
*can you use "make DESTDIR=%{buildroot}

Couldn't Add
============
 *chaning %makeinstall. Actually this will not work with the grub 2 build system.

Do I need to make autoconf a depenency?

(In reply to comment #4)
> Do I need to make autoconf a depenency?

If you use autoreconf during build, yes, the build dependency is needed. 
Depending on what stuff autoreconf itself needs to run, you may actually need to
pull in automake or libtool instead of autoconf.  By the way, running autoreconf
(if it's needed) would be better done in %prep, not %build.

Potential reviewers will also find it easier to track progress if you bump the
release tag *always* when making changes, even during the review.

Thanks for the comments guys. I've added autoconf as a dependency as well as
moved the use of autoreconf to %prep. New rpms and specs can be found below:

http://jgotech.net/jerone/grub-1.95/grub2.spec
http://jgotech.net/jerone/grub-1.95/grub2-1.95-3.src.rpm

note:
Have not had a chance to test that new rpms build on powerpc just yet. Though it
should.

Sorry, I steered you wrong!

*%{?dist} should be appended to Release, not Version.

My fault!

It's no problem. Another update to reflect this change.

http://jgotech.net/jerone/grub-1.95/grub2-1.95-4.src.rpm
http://jgotech.net/jerone/grub-1.95/grub2.spec

- It builds cleanly in mock fc6 i386.

- It passes rpmlint

- Spec file looks OK

Building it in mock devel now - as long as that goes as expected, I see no
blockers.  I have no x86_64 or ppc devel environment to build on though.

Created attachment 149014 [details]
log for failed build for rawhide/x86_64

just a note: make should use smp_flags, if possible (or add a comment if it is not).
build on x86_64/devel failed in mock, I have attached the build log.

I'm looking into the the _start being missing now (not sure exactly what this
is, or why it's now gone in x86-64 fc7 gcc). I've update the spec though to add
smp to building. It appears to work fine (tried up to -j4 and it worked).

http://jgotech.net/jerone/grub-1.95/grub2.spec

http://jgotech.net/jerone/grub-1.95/grub2-1.95-5.src.rpm

I'm going to see how grub cvs works under x86 and x86-64 fc7. Though currently
grub 2 cvs needs a little kicking into shape. I'll see what I can do about it
this week.

Any progress here?

build breaks with smpflags on a multiprocessor box.

#5: why is it better to run autoreconf in %prep? this is definitely the first
time i hear and even see it placed there at all. The usual place to do every
thing build related (processing/parsing files and generate outputfiles) is done
in %build

Spent some time tonight and figured out the issue with x86-64 compiling under
Fedora. The package glibc-devel.i386 needs to be made a dependency. I'll prep a
new set of rpms based on cvs build (which has all the patches that I had in the rpm)

Thanks for poking me on this one.

My guess with autoreconf not being apart of build is because you really are not
building the source when you do an autoreconf. You are rebuilding the config
files to build the source. So being in %prep makes since from this stand point.

Ok I rebased on latest CVS. 

http://jerone.googlepages.com/grub2-1.95-1.20070610cvs.fc7.src.rpm
http://jerone.googlepages.com/grub2.spec

http://jerone.googlepages.com/fedoragrub2packages

One problem though .. I can't make "glibc-devel.i386" a build requirement on
x86-64. I could only put "glibc-devel". It seems to install both when doing a
yum install. Anyone have any idea how to specify an architecture package (that
isn't the primary) as a Build Requirement? 

like i said, parallel building is broken (package doesent build with smpflags on
a dual core box), but works fine without:


gcc -Ikern -I./kern -I. -Iinclude -I./include -Wall -W  -Wall -W -Wshadow
-Wpointer-arith -Wmissing-prototypes                  -Wundef
-Wstrict-prototypes -g -Os -falign-jumps=1 -falign-loops=1 -falign-functions=1
-fno-stack-protector -fno-builtin -mrtd -mregparm=3 -m32 -MD -c -o
kernel_img-kern_device.o kern/device.c
util/grub-emu.c:41:31: error: grub_modules_init.h: No such file or directory
util/grub-emu.c: In function 'main':
util/grub-emu.c:188: warning: implicit declaration of function 'grub_init_all'
util/grub-emu.c:211: warning: implicit declaration of function 'grub_fini_all'

I added grub-emu back in and that does work with smpflags. Thanks! I've got new
build up now removing the smp flags while compiling which should solve that problem.

http://jerone.googlepages.com/grub2-1.95-2.20070610cvs.fc7.src.rpm
http://jerone.googlepages.com/grub2.spec

- GPL is not a valid license tag anymore:
http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#head-f21ae23bf2f278444e2c385463cfa74a502396b8

- instead of /etc you should use %{_sysconfdir}
- maybe the file in /etc should be marked with %config, is it intended, that
they are not?
- Why is this update-grub_lib in %{_libdir}/grub and not in %{_libdir}/grub2?
Also in case it is intentional, the directory %{_libdir}/grub/ is not owned by
any package afaics.

- I guess %{_libdir}/grub2/* should be just %{_libdir}/grub2/ because otherwise
the directory %{_libdir}/grub2/ is not owned by any package.
- /etc/grub.d/ is also not owned by the package but created
- /boot/grub2 should be /boot/grub2/ imho to make it more obvious that it is an
directory
- the conditionals in %files can be simplified because there is a
"ExclusiveArch: i386 x86_64 ppc ppc64" tag and everything in the package for 
ppc(64), is also in the package for i386 and x86_64, so this should produce the
same rpms:

/sbin/grub2-emu
/sbin/grub2-install
/bin/grub2-mkimage
/boot/grub2/
%ifarch i386 x86_64
/sbin/grub2-mkdevicemap
/sbin/grub2-probe
/sbin/grub2-setup
%endif

- What do you mean with this comment: #All apart of update-grub2 tool?

Is this still being worked on?

Sadly no. This is no longer being worked on. At the time I was trying to get it
in. But grub2 development is still slowly moving along. I also have little time
anymore to maintain the package and rpm spec file so someone else can refrence
it if they would like.

Work in progress still, gladly accepting patches. Just so that this won't rot on
my hdd:

http://people.redhat.com/lkundrak/SPECS/grub2.spec
http://people.redhat.com/lkundrak/SRPMS/grub2-1.96-1.fc8.src.rpm

for kicks i tried building in mock on rawhide

checking if C symbols get an underscore after compilation...
no
checking if start is defined by the compiler...
no
checking if _start is defined by the compiler...
no
configure: error: neither start nor _start is defined

Seems horribly broken

(In reply to comment #23)
> for kicks i tried building in mock on rawhide
> 
> checking if C symbols get an underscore after compilation...
> no
> checking if start is defined by the compiler...
> no
> checking if _start is defined by the compiler...
> no
> configure: error: neither start nor _start is defined

I am not reproduce on Fedora 9/i386, nor Fedora 8/x86_64.
Did you build on something else? Have complete build.log and root.log somewhere?
 
> Seems horribly broken

What does "horribly" stand here for?

Oh, this is that on x86_64, 32 bit glibc devel that provides crt1.o is not
buildrequired. Add this BuildRequires: /usr/lib/crt1.o to fix.

http://people.redhat.com/lkundrak/SPECS/grub2.spec
http://people.redhat.com/lkundrak/mock-results/grub2-1.96-2.fc8.x86_64/

Doh. I am wondering whether anyone has a copy of the source package or SPEC file?

If there's no package to review and the submitter has indicated that they no
longer want to submit this, there's no reason to keep this ticket open.  If
someone later wants to submit a grub2 package for review, please open a fresh
ticket.

*** This bug has been marked as a duplicate of bug 458472 ***