1143005 - Add parameter to checksetup.pl that generates a cpanfile usable by utilities such as cpanm for installing Perl dependencies

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, enhancement)

Description

[David Lawrence [:dkl]]

This could replace or the current Build.PL script that is used now. Useful for continuous integration testing as well such as Travis CI and TaskCluster.

Thoughts?

dkl

Comment 1

[David Lawrence [:dkl]]

Attachment: 1143005_1.patch

Comment 2

[Frédéric Buclin]

Comment on attachment 8577266 [details] [diff] [review]
1143005_1.patch

>+++ b/Bugzilla/Install/Requirements.pm

>+sub export_cpanfile {

>+        print $requires;

It looks weird to me to print the rules to STDOUT. IMO, nobody expects this behavior. You should print the output to some given (hardcoded) filename.
Moreover, is it intentional to omit all DBD:: modules?

Comment 3

[David Lawrence [:dkl]]

(In reply to Frédéric Buclin from comment #2)
> Comment on attachment 8577266 [details] [diff] [review]
> 1143005_1.patch
> 
> >+++ b/Bugzilla/Install/Requirements.pm
> 
> >+sub export_cpanfile {
> 
> >+        print $requires;
> 
> It looks weird to me to print the rules to STDOUT. IMO, nobody expects this
> behavior. You should print the output to some given (hardcoded) filename.

My original intention was to have it where the admin could redirect it where needed if not the default location. As the majority of people will put it in the root directory of the code base, I will change the patch to just put it there and they can move it if desired.

> Moreover, is it intentional to omit all DBD:: modules?

I did think about that but since we support several DBs and I did not want cpanfile to include all by default, I assumed it would be better for them to install one manually once checksetup.pl is ran and complains. 

Thoughts on best behavior for the last issue?

dkl

Comment 4

[David Lawrence [:dkl]]

Attachment: 1143005_2.patch

Changes: 

1. Added cpanfile to .htaccess to disallow reading
2. Now writes out to the cpanfile file name in the document root by default.

dkl

Comment 5

[Frédéric Buclin]

(In reply to David Lawrence [:dkl] from comment #3)
> I did think about that but since we support several DBs and I did not want
> cpanfile to include all by default, I assumed it would be better for them to
> install one manually once checksetup.pl is ran and complains. 

They should at least be recommended, IMO, except Oracle which is a pain to install.

About your patch, you forgot to update POD in checksetup.pl, see http://www.bugzilla.org/docs/tip/en/html/api/checksetup.html.

Comment 6

[David Lawrence [:dkl]]

Attachment: 1143005_3.patch

Thanks for the comments LpSolit. New patch with changes:

1. Updated POD for checksetup.pl
2. Added DBD modules as 'features' that can be enabled. For example the following command line will install all normal features but only the 'mysql'
database:

# cpanm --installdeps --with-recommends --with-all-features --without-feature oracle --without-feature sqlite --without-feature pg .

dkl

Comment 7

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8579641 [details] [diff] [review]
1143005_3.patch

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

r- patch doesn't cleanly apply.

Think this patch should remove the Build.PL?

Comment 8

[David Lawrence [:dkl]]

Attachment: 1143005_4.patch

Comment 9

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8585508 [details] [diff] [review]
1143005_4.patch

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

r=dylan

I used this to install deps for testing the REST versioning patch, it seems pretty complete.

Comment 10

[David Lawrence [:dkl]]

To ssh://[e-mail]/bugzilla/bugzilla.git
   1fa1aa5..a29f798  master -> master

Comment 11

[Frédéric Buclin]

I added cpanfile to .gitignore so that git leaves this file alone.

To ssh://[e-mail]/bugzilla/bugzilla.git
   7adedfc..b37a1c6  master -> master