Bug 435276 (kBuild-review) - Review Request: kBuild - A cross-platform build enviroment
Summary: Review Request: kBuild - A cross-platform build enviroment
Keywords:
Status: CLOSED NEXTRELEASE
Alias: kBuild-review
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Xavier Lamien
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-02-28 14:23 UTC by Jay Turner
Modified: 2015-01-08 00:16 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-12-25 14:59:48 UTC
Type: ---
Embargoed:
lxtnow: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Lubomir Kundrak 2008-02-28 14:23:01 UTC
SPEC: http://people.redhat.com/lkundrak/SPECS/kBuild.spec
SRPM:
http://people.redhat.com/lkundrak/SRPMS/kBuild-0.1.0-0.4.20070627svn.fc8.src.rpm

A cross-platform build enviroment.
This is used by VirtualBox.

Comment 1 Parag AN(पराग) 2008-02-29 03:43:26 UTC
why this package have
ExclusiveArch:  %{ix86} x86_64 ?

Comment 2 Lubomir Kundrak 2008-02-29 05:25:30 UTC
Parag: see where do we set BUILD_TARGET_ARCH. kBuild uses architecture name as
part of file names and it seems to be only aware of "x86" and "amd64". That
makes perfect sense -- it is only used to build VirtualBox and VirtualBox runs
nowhere else.

Comment 3 Parag AN(पराग) 2008-02-29 06:28:23 UTC
Thanks for reply. Reason to ask this is that I see some reviewers asking same
thing as review should make sure new package builds on all archs. But if its not
doing that then better to know reason and add that in SPEC.


Comment 4 Thorsten Leemhuis 2008-03-01 12:57:36 UTC
(In reply to comment #2)
> [...] it is only used to build VirtualBox and VirtualBox runs nowhere else.

Just my 2 cent: I normally strongly prefer to "just call a package exactly like
upstream calls its software". But for this package I'm starting to wonder if it
might be better to call it VirtualBox-kbuild or something, to avoid confusion
with the linux-kernel-internal kbuild infra.






Comment 5 Lubomir Kundrak 2008-03-01 13:16:45 UTC
I tend to agree with Thorsten and the name he suggested. Anyone has a different
opinion?

Comment 6 Till Maas 2008-03-01 14:22:49 UTC
In theory, kBuild can be used to build anything and not only virtualbox,
therefore I do not like the name very much. Also debian use only kBuild as a
name for this package. In case there is a strong need to rename it, I would
prefer something like "kBuild-makefile-framework", which describes it better.

Btw. one issue with the current kBuild spec is, that RPM_OPT_FLAGS are not used
to compile it. But I do not currently know how to change this.

Comment 7 Lubomir Kundrak 2008-03-01 14:29:37 UTC
(In reply to comment #6)
> In theory, kBuild can be used to build anything and not only virtualbox,
> therefore I do not like the name very much. Also debian use only kBuild as a
> name for this package. In case there is a strong need to rename it, I would
> prefer something like "kBuild-makefile-framework", which describes it better.

That also makes sense. I'm do not dare how many users would confuse kBuild with
kernel's build system, but I certainly wouldn't. In fact I can't imagine a
scenario where that could occur.

> Btw. one issue with the current kBuild spec is, that RPM_OPT_FLAGS are not used
> to compile it. But I do not currently know how to change this.

Thanks for telling that, I'm glad I avoided scop noticing that :)) I'll take
care of that.

Comment 8 Lubomir Kundrak 2008-03-01 21:29:48 UTC
Actually; kBuild is used to use something else than kBuild -- itself :)

I changed it to obey rpmflags.

Moreover binaries are installed to %{_bindir} where they belong.
(For VirtualBox packagers: this probably won't build current package. Let us
adjust it just once this is in Fedora repository)

Another change was to depend on itself, so that it does not have to be
bootstrapped. It can, though, by definig bootstrap to 1.

Last change was to enable ppc32 and ppc64 builds. Seems like upstream counts on
them.

http://people.redhat.com/lkundrak/SPECS/kBuild.spec
http://people.redhat.com/lkundrak/SRPMS/kBuild-0.1.0-0.5.20070627svn.fc8.src.rpm

Comment 9 Parag AN(पराग) 2008-03-03 04:07:13 UTC
build failed http://koji.fedoraproject.org/koji/taskinfo?taskID=485864

Comment 11 Till Maas 2008-03-07 13:29:35 UTC
I forgot to mention this the last time:

The libexecdir does not need to created here, because nothing is installed there
anymore:
mkdir -p $RPM_BUILD_ROOT%{_libexecdir}/kBuild

But this can be changed before it is imported to cvs or when there is major
change   made before this.

Comment 12 Lubomir Kundrak 2008-03-07 13:35:22 UTC
Till; thanks for noticing that.
I'll wait for if Parag has any comments so I don't roll a new package just for this.

Comment 13 Till Maas 2008-03-08 11:16:57 UTC
Another issue is to install kBuild with "kmk PATH_INS=${RPM_BUILD_ROOT}
NIX_INSTALL_DIR=%{_prefix} install", then also the *.kmk scripts will be
installed. This are the contents of the debian pacakage:
http://packages.debian.org/sid/i386/kbuild/filelist

The problem is, that kmk install does not work with this version. I tried to
build it with a snapshot from date 20071215 (this should be the release that
debian uses), but then kmk install complained about expection kBuild version
0.1.2 but getting 0.1.0.

Comment 14 Parag AN(पराग) 2008-03-13 04:16:19 UTC
Lubomir,
  Can you remove following line from SPEC?
mkdir -p $RPM_BUILD_ROOT%{_libexecdir}/kBuild

any reason for not using latest svn checkout?

Comment 15 Lubomir Kundrak 2008-03-13 16:07:21 UTC
(In reply to comment #13)
> Another issue is to install kBuild with "kmk PATH_INS=${RPM_BUILD_ROOT}
> NIX_INSTALL_DIR=%{_prefix} install", then also the *.kmk scripts will be
> installed. This are the contents of the debian pacakage:
> http://packages.debian.org/sid/i386/kbuild/filelist

What are they good for? They are mostly useless even in context of building
virtualbox itself (setting compiler flags to unwanted ones, etc.) and all we do
during the build of kBuild is to struggle with them for influence on the flags.

(In reply to comment #14)
> Lubomir,
>   Can you remove following line from SPEC?
> mkdir -p $RPM_BUILD_ROOT%{_libexecdir}/kBuild

I'll do.

> any reason for not using latest svn checkout?

Today's svn snapshot won't be latest tomorrow. This one is tried and works well
for our purpose.

Comment 16 Till Maas 2008-03-13 17:01:38 UTC
(In reply to comment #15)
> (In reply to comment #13)
> > Another issue is to install kBuild with "kmk PATH_INS=${RPM_BUILD_ROOT}
> > NIX_INSTALL_DIR=%{_prefix} install", then also the *.kmk scripts will be
> > installed. This are the contents of the debian pacakage:
> > http://packages.debian.org/sid/i386/kbuild/filelist
> 
> What are they good for? They are mostly useless even in context of building
> virtualbox itself (setting compiler flags to unwanted ones, etc.) and all we do
> during the build of kBuild is to struggle with them for influence on the flags.

Afaik, they are the same files that are in the kBuild directory in the
virtualbox tarball and without them, the binaries within the package are
useless, i.e. it does not really make sense to install the binaries in /usr/bin,
because it could still not be used by projects that do not ship a local copy of
kBuild.

Comment 17 Parag AN(पराग) 2008-04-04 03:40:00 UTC
I better move away from this review. Till, feel free to review this package.

Comment 18 Till Maas 2008-04-04 09:26:30 UTC
(In reply to comment #17)
> I better move away from this review. Till, feel free to review this package.

I do not currently have the time to review this, also I doubt that it is ok with
the spirit of the review process that I review this, because I wrote the initial
spec of this packages.

Btw. you need to clear the fedora-review flag, when you step down as a reviewer,
I did this now.

Comment 20 Jason Tibbitts 2008-09-08 14:24:11 UTC
Is the package from comment #10 the one which should be reviewed?  Because those links are no longer valid.

Comment 21 Lubomir Rintel 2008-09-08 16:04:09 UTC
I've done some more work on this, I'll try to get in reviewable state and submit the new package shortly.

Comment 22 Jason Tibbitts 2008-11-05 16:07:32 UTC
Any updates?

Comment 23 Jason Tibbitts 2008-11-16 15:24:20 UTC
Please clear the whiteboard when you are ready.

Comment 24 Xavier Lamien 2008-12-13 11:01:52 UTC
ping ?

Comment 25 Lubomir Rintel 2008-12-14 20:14:37 UTC
Boo!

SPEC: http://netbsd.sk/~lkundrak/SPECS/kBuild.spec
SRPM: http://netbsd.sk/~lkundrak/SRPMS/kBuild-0.1.4-1.el5.src.rpm

* Builds in fedora-devel-i386
* RPMlint silent
* If you type "google" into google, you'll break the internets!

Comment 26 Xavier Lamien 2008-12-17 19:48:17 UTC
taking review.

Comment 27 Xavier Lamien 2008-12-21 03:27:24 UTC
Well, 
Everything looks good.
Kbuild flags are properly set and built on all expected arches.
According to packaging guideline, this package can be added.

Approved.

Comment 28 Xavier Lamien 2008-12-22 10:49:59 UTC
btw, do you mind to add me as co-owner ?

Comment 29 Lubomir Rintel 2008-12-22 13:44:26 UTC
(In reply to comment #28)
> btw, do you mind to add me as co-owner ?

I'd be glad to. By the way, I noticed that you're planning to add VirtualBox into RPM Fusion, which I'm happy to hear, and which was the reason I added this review as well.

I don't know whether you package is finished, or whether you based your package on Till's one as well, but you may want to have a short look at my package, just to cross-check [1].

[1] http://elvn.getrpm.net/rpm/el5/src/

Comment 30 Xavier Lamien 2008-12-23 16:30:50 UTC
Thanks, I'll have a look and merge if so.
Good to know you're involve in.

Package Request
==================
Package names: kBuild
Description: A cross-platform build environment
Owners: lkundrak, laxathom
Branches: F-8 F-9 F-10
InitialCC:

Comment 31 Lubomir Rintel 2008-12-23 20:49:05 UTC
Package Request
==================
Package names: kBuild
Description: A cross-platform build environment
Owners: lkundrak, laxathom
Branches: EL-5 F-9 F-10

(Removing EOL-ed F-8, adding EPEL-5)

Comment 32 Kevin Fenzi 2008-12-25 00:35:39 UTC
cvs done.

Comment 33 Lubomir Rintel 2008-12-25 14:59:48 UTC
Imported and built. Thanks to Parag, Till, Xavier and Kevin.

Xavier: Please add me to Cc when you'll add a review request for VirtualBox in RPM Fusion. I'd eventually gladly review it, so you can assign it to me right away.


Note You need to log in before you can comment on or make changes to this bug.