1174695 - The cpanfile created by checksetup.pl defines the same feature multiple times, breaking cpanm

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, defect)

Description

[Ed Morley [:emorley]]

Bug 1143005 added a '--cpanfile' option to checksetup.pl which exports a cpanfile for use with utilities like cpanm.

The current exported file looks like:

"""
requires 'CGI', '3.51';
requires 'Digest::SHA';
requires 'Date::Format', '2.21';
...
feature 'graphical_reports', 'GD' => sub {
  recommends 'GD', '1.20';
};
feature 'new_charts', 'GD' => sub {
  recommends 'GD', '1.20';
};
feature 'old_charts', 'GD' => sub {
  recommends 'GD', '1.20';
};
feature 'new_charts', 'Chart' => sub {
  recommends 'Chart::Lines', '2.1.0';
};
...
"""

Unfortunately the duplicate features (ie: the two instances of feature 'new_charts' above; plus many more in the full file) confuse cpanm, causing it to only install the dependencies for the last definition of a feature.

This is why the docker project has to manually install a bunch of deps:


We should combine all of a particular feature's dependencies, and output them as one entry, eg:

feature 'new_charts', 'GD' => sub {
  recommends 'GD', '1.20';
  recommends 'Chart::Lines', '2.1.0';
};

Comment 1

[Ed Morley [:emorley]]

Attachment: checksetup: Only define each feature once in the cpanfile

cpanm gets confused if a feature is defined more than once in the
cpanfile, so we now consolidate all of the modules associated with each
feature, rather than outputting each separately.

In addition, we no longer output a description for each feature, since
(a) using the package name doesn't work when there is more than one
module listed for a feature,
(b) according to the spec, the description field is supposed to describe
the feature, not the modules it requires.
For now we're better off omitting it entirely, since it's optional.

This patch also adds some comments to break up the different sections of
the cpanfile, as well as sorts the features/their deps by name, to make
them easier to skim read.

Comment 2

[Ed Morley [:emorley]]

Example cpanfile created with this patch applied on the BMO repo:
https://emorley.pastebin.mozilla.org/8836840

Once this lands upstream, we'll want to backport to BMO too - to help with the Docker project :-)

Comment 3

[David Lawrence [:dkl]]

Comment on attachment 8622463 [details] [diff] [review]
checksetup: Only define each feature once in the cpanfile

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

Looks good. r=dkl

Comment 4

[David Lawrence [:dkl]]

To ssh://[e-mail]/bugzilla/bugzilla.git
   f53defb..e85aa1b  master -> master