Closed Bug 995209 Opened 6 years ago Closed 6 years ago

Create a Build.PL script using Module::Build for testing/installing/packaging of Bugzilla code

Categories

(Bugzilla :: Installation & Upgrading, enhancement)

4.5.2
enhancement
Not set

Tracking

()

RESOLVED FIXED
Bugzilla 4.0

People

(Reporter: dkl, Assigned: dkl)

Details

Attachments

(2 files, 3 obsolete files)

I am creating this bug as a holding place for future work I would like to do on creating a proper Build.PL script for performing operations on the Bugzilla code. These can include packaging for distribution, running the test suites, clean up, etc. Also as a bonus, if the Build.PL script is present in the github repo, Travis CI will automatically see it and use it to run the test suite when a code change is made.

dkl
I would recommend Dist::Zilla for this -- the name being awesome aside, it has many advantages to a module author. http://dzil.org/. It does take a little more effort to get Travis CI working with it, however.
Attached file cpanfile
Actually, Travis CI can use cpanfiles (like a gemfile for perl). We can *also* use the cpanfile from Dist::Zilla and eventually have Bugzilla be a well-behaved perl application. 

We'll need to generate cpanfiles for extensions too -- which we can almost certainly automate.
Attached patch 995209_1.patch (obsolete) — Splinter Review
You can install the dependencies using:

cpanm --installdeps --quiet --with-recommends .
Assignee: installation → dkl
Status: NEW → ASSIGNED
Attachment #8414515 - Flags: review?(glob)
Comment on attachment 8414515 [details] [diff] [review]
995209_1.patch

>=== added file 'Build.PL'

>+#!/usr/bin/perl -w

Please don't use the -w  flag, see bug 996893.
Attached patch 995209_2.patch (obsolete) — Splinter Review
Attachment #8414515 - Attachment is obsolete: true
Attachment #8414515 - Flags: review?(glob)
Attachment #8414696 - Flags: review?(glob)
Comment on attachment 8414696 [details] [diff] [review]
995209_2.patch

Review of attachment 8414696 [details] [diff] [review]:
-----------------------------------------------------------------

::: Build.PL
@@ +1,4 @@
> +#!/usr/bin/perl
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.

this license is different from bugzilla's normal license (it doesn't include "incompatible with secondary license").
i don't see any reason why this file should be licensed differently.

@@ +1,5 @@
> +#!/usr/bin/perl
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +

this file would benefit from a short description describing why it exists/what it does.

@@ +10,5 @@
> +use File::Basename;
> +use Module::Build;
> +
> +BEGIN { chdir dirname($0); }
> +use lib qw(. lib);

it would be simpler to use FindBin

use FindBin qw($RealBin);
use lib ($RealBin, "$RealBin/lib");

@@ +13,5 @@
> +BEGIN { chdir dirname($0); }
> +use lib qw(. lib);
> +use Bugzilla::Install::Requirements qw(REQUIRED_MODULES OPTIONAL_MODULES);
> +use Bugzilla::Constants qw(BUGZILLA_VERSION);
> +sub requires {

nit: add a blank line before this sub

@@ +20,5 @@
> +    foreach my $module (@$requirements) {
> +        $hrequires->{$module->{module}} = $module->{version};
> +    }
> +    return $hrequires;
> +};

s/};/}/

@@ +22,5 @@
> +    }
> +    return $hrequires;
> +};
> +
> +sub build_requires { return requires(); }

don't collapse this sub onto a single line

@@ +39,5 @@
> +my $build = Module::Build->new(
> +    module_name        => 'Bugzilla',
> +    dist_abstract      => <<END,
> +Bugzilla is a free bug-tracking system that is developed by an active
> +community of volunteers in the Mozilla community. You can install and

bugzilla's community is much more than just "the mozilla community".
remove "in the mozilla community" from that sentence.

@@ +40,5 @@
> +    module_name        => 'Bugzilla',
> +    dist_abstract      => <<END,
> +Bugzilla is a free bug-tracking system that is developed by an active
> +community of volunteers in the Mozilla community. You can install and
> +use it without having to pay any license fee

nit: add a period to the end of that sentence

@@ +46,5 @@
> +    dist_version_from  => 'Bugzilla/Constants.pm',
> +    dist_version       => BUGZILLA_VERSION,
> +    requires           => requires(),
> +    recommends         => recommends(),
> +    license            => 'mozilla',

'mozilla' is for mpl-1 or mpl-1.1, which isn't the correct license for bugzilla.
you want 'Mozilla_2_0'

@@ +53,5 @@
> +);
> +
> +$build->create_build_script;
> +
> +package MY;

is this empty package required?
Attachment #8414696 - Flags: review?(glob) → review-
Attached patch 995209_3.patch (obsolete) — Splinter Review
A more recent version of Module::Build and Software::License needs to be installed than what is included with Centos6 for this to work.

dkl
Attachment #8414696 - Attachment is obsolete: true
Attachment #8415517 - Flags: review?(glob)
Comment on attachment 8415517 [details] [diff] [review]
995209_3.patch

Review of attachment 8415517 [details] [diff] [review]:
-----------------------------------------------------------------

r=glob with the changes below to be fixed on commit

::: Build.PL
@@ +12,5 @@
> +
> +use FindBin qw($RealBin);
> +use lib ($RealBin, "$RealBin/lib");
> +
> +use File::Basename;

you don't appear to be using File::Basename anymore.

@@ +13,5 @@
> +use FindBin qw($RealBin);
> +use lib ($RealBin, "$RealBin/lib");
> +
> +use File::Basename;
> +use Module::Build;

since we require 0.36_14 or higher, make this:
use Module::Build 0.36_14;
Attachment #8415517 - Flags: review?(glob) → review+
Flags: approval?
Target Milestone: --- → Bugzilla 5.0
Attached patch 995209_4.patchSplinter Review
Fixed patch with reviewer comments. Moved r+ also and awaiting a+.
Attachment #8415517 - Attachment is obsolete: true
Attachment #8415932 - Flags: review+
Flags: approval? → approval+
Thanks justdave. Actually I am going to need this for each of the branches we support so that we can use Travis CI for testing (it uses Build.PL to install the modules). So asking for approval for those branches as well.
Flags: approval4.4?
Flags: approval4.2?
Flags: approval4.0?
For the build!
Flags: approval4.4?
Flags: approval4.4+
Flags: approval4.2?
Flags: approval4.2+
Flags: approval4.0?
Flags: approval4.0+
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   7505025..190006d  master -> master

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   14208d5..3b4ddd1  4.4 -> 4.4

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   39412f0..afa7005  4.2 -> 4.2

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   0654abc..2b92f27  4.0 -> 4.0
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: Bugzilla 5.0 → Bugzilla 4.0
You need to log in before you can comment on or make changes to this bug.