Conversation
adamjstewart
left a comment
There was a problem hiding this comment.
This is awesome! I've been avoiding adding perl as a build dependency to a lot of packages because Spack-built Perl was missing some Perl modules that were present on my system copy. Can you add an XML::Parser package as well? That's the most commonly required one I've been seeing.
| Buildx() | ||
| if self.run_tests: | ||
| Buildx('test') | ||
| Buildx('install') |
There was a problem hiding this comment.
You'll want to split this into multiple phases. It looks like each package has a build and install phase. See basically any package class aside from RPackage to see what I mean.
There was a problem hiding this comment.
Will do ... after lunch!
| straightforward way than with MakeMaker. It also does not require a make on | ||
| your system - most of the Module::Build code is pure-perl and written in a | ||
| very cross-platform way. | ||
| """ |
There was a problem hiding this comment.
You should indent this normally like other packages.
There was a problem hiding this comment.
Fixed. But I didn't see a warning about that in the flake8 tests.
|
Hi @adamjstewart - I have an XML::Parser package ready to go, but it depends on the extendable perl feature. |
tgamblin
left a comment
There was a problem hiding this comment.
@mjwoods: this is great! Thanks for all the effort putting this together. I have basically the same comments as @adamjstewart but otherwise this LGTM!
| body = """\ | ||
| # FIXME: Select appropriate perl build method: | ||
| # build_method = 'Makefile.PL' # Default | ||
| # build_method = 'Build.PL' # Needed by some packages |
There was a problem hiding this comment.
Do any packages provide both? And if so, is there any advantage to using one or the other? If not, let's just see which file exists and use that. No need to require the user to specify this.
There was a problem hiding this comment.
There are a lot of perl packages, so it is difficult to be sure what works in general. I think auto-detection will work, and we can provide extra overrides if/when we find they are needed.
| makex = inspect.getmodule(self).make | ||
| makex() | ||
| if self.run_tests: | ||
| makex('test') |
There was a problem hiding this comment.
Let's move the test step to a separate function. See AutotoolsPackage for how this is done. The benefit of this is that packages that don't provide a test target can skip this. Also, if there are any additional/different tests to run, users can override this by adding a test(self) function to their package.
There was a problem hiding this comment.
Done. A separate method is more elegant - I didn't know it was available before.
|
Hi @adamjstewart, I think my recent changes should take care of your requests. I still have some errors from the automated code coverage tests. Are there any tests for the other build systems (e.g. autotools, python) that I could adapt for perl? |
|
Nope, not a lot of tests for specific build systems (yet). The coverage tests don't need to pass for this to be merged, but it is nice. |
adamjstewart
left a comment
There was a problem hiding this comment.
This looks good to me. @alalazo is this missing anything?
* perl: make extendable and add Module::Build package * perl: allow 'spack create' to identify perl packages from their contents * perl-module-build: fix indenting of package docstring * perl: split install() method for extensions into phases * perl: auto-detect build method (Makefile.PL vs Build.PL) and define a 'check' method * PerlPackage: use import statements similar to those in AutotoolsPackage * PerlModule: fix detection of Build.PL * PerlPackageTemplate: remove extraneous lines to avoid flake8 warnings * PerlPackageTemplate: split into separate templates for Makefile.PL and Build.PL * PerlPackage: add cross-references to docstrings * AutotoolsPackage: fix ambiguous cross-references to avoid errors in doc tests * PerlbuildPackageTemplate: depend on perl-module-build if Build.PL exists
* perl: make extendable and add Module::Build package * perl: allow 'spack create' to identify perl packages from their contents * perl-module-build: fix indenting of package docstring * perl: split install() method for extensions into phases * perl: auto-detect build method (Makefile.PL vs Build.PL) and define a 'check' method * PerlPackage: use import statements similar to those in AutotoolsPackage * PerlModule: fix detection of Build.PL * PerlPackageTemplate: remove extraneous lines to avoid flake8 warnings * PerlPackageTemplate: split into separate templates for Makefile.PL and Build.PL * PerlPackage: add cross-references to docstrings * AutotoolsPackage: fix ambiguous cross-references to avoid errors in doc tests * PerlbuildPackageTemplate: depend on perl-module-build if Build.PL exists
* perl: make extendable and add Module::Build package * perl: allow 'spack create' to identify perl packages from their contents * perl-module-build: fix indenting of package docstring * perl: split install() method for extensions into phases * perl: auto-detect build method (Makefile.PL vs Build.PL) and define a 'check' method * PerlPackage: use import statements similar to those in AutotoolsPackage * PerlModule: fix detection of Build.PL * PerlPackageTemplate: remove extraneous lines to avoid flake8 warnings * PerlPackageTemplate: split into separate templates for Makefile.PL and Build.PL * PerlPackage: add cross-references to docstrings * AutotoolsPackage: fix ambiguous cross-references to avoid errors in doc tests * PerlbuildPackageTemplate: depend on perl-module-build if Build.PL exists
* perl: make extendable and add Module::Build package * perl: allow 'spack create' to identify perl packages from their contents * perl-module-build: fix indenting of package docstring * perl: split install() method for extensions into phases * perl: auto-detect build method (Makefile.PL vs Build.PL) and define a 'check' method * PerlPackage: use import statements similar to those in AutotoolsPackage * PerlModule: fix detection of Build.PL * PerlPackageTemplate: remove extraneous lines to avoid flake8 warnings * PerlPackageTemplate: split into separate templates for Makefile.PL and Build.PL * PerlPackage: add cross-references to docstrings * AutotoolsPackage: fix ambiguous cross-references to avoid errors in doc tests * PerlbuildPackageTemplate: depend on perl-module-build if Build.PL exists
Resolves #1817.
Issue #1817 raises a scenario where installing a package is difficult without making perl extendable. This feature is listed as being up-for-grabs.
I have added support for perl extensions, using R extensions as a guide.
Perl packages use a variety of build methods, some of which are not provided by the core perl distribution. As a test of my perl extension code, I have included an extension that provides one of the common "unofficial" build methods: perl-module-build.