conda-forge / perl-feedstock

A conda-smithy repository for perl.
BSD 3-Clause "New" or "Revised" License
3 stars 32 forks source link

extra (empty) outputs for core perl packages #25

Open djsutherland opened 6 years ago

djsutherland commented 6 years ago

Things like ExtUtils::MakeMaker are included with our perl package but listed as requirements by conda build skeleton create. Either skeleton should be taught not to do that, or we could add empty outputs to this recipe for each of them.

https://github.com/conda-forge/staged-recipes/pull/5149

xileF1337 commented 6 years ago

Since @bgruening invited me to help transferring general-purpose Perl packages from bioconda to conda-forge, this question is also quite interesting to me.

If I understand the source of conda skeleton 3.12.1 correctly, some efforts have already been made to identify core packages and comment them out in the requirement list. However, this does not seem to work: e.g. conda skeleton cpan Unix::Processors creates a skeleton which excludes perl-warnings, but not perl-test, which is a core module. skeleton seems to be using the MetaCPAN API to perform its core module check, but I don't really get the rationale of it. I wondered whether MetaCPAN actually knows whether a module is a core module or not. After all, what is a core module and what not can change from Perl version to Perl version. For example, Module::Build was a core module in Perl 5.20, but was removed from the core as of 5.21(cf. perl5220delta). In Perl, the (core ;-)) module Module::CoreList may be used to determine whether a module is a core module in a given version of Perl.

Given the changing nature of the core module list, I propose not to remove core module dependencies from each and every Perl module recipe, as this would mean one has to touch all recipes depending on a module removed from the core every time the Perl version pinning is updated.

The better choice would probably be to have "empty" recipes, as you proposed. However, in my opinion, these should also be fully functional recipes which include an unconditional build/skip key plus a comment stating that this is a core module. Then, should updating the Perl pinning make it necessary, the recipe can be "activated" easily. What do you think?

mingwandroid commented 6 years ago

Absolutely not. That's my opinion.

On Wed, Aug 15, 2018, 8:52 AM Felix Kühnl notifications@github.com wrote:

Since @bgruening https://github.com/bgruening invited me to help transferring general-purpose Perl packages from bioconda to conda-forge, this question is also quite interesting to me.

If I understand the source of conda skeleton 3.12.1 correctly, some efforts have already been made to identify core packages and comment them out in the requirement list. However, this does not seem to work: e.g. conda skeleton cpan Unix::Processors creates a skeleton which excludes perl-warnings, but not perl-test, which is a core module. skeleton seems to be using the MetaCPAN API to perform its core module check, but I don't really get the rationale of it. I wondered whether MetaCPAN actually knows whether a module is a core module or not. After all, what is a core module and what not can change from Perl version to Perl version. For example, Module::Build was a core module in Perl 5.20, but was removed from the core as of 5.21(cf. perl5220delta https://perldoc.perl.org/perl5220delta.html#Removed-Modules-and-Pragmata). In Perl, the (core ;-)) module Module::CoreList may be used to determine whether a module is a core module in a given version of Perl.

Given the changing nature of the core module list, I propose not to remove core module dependencies from each and every Perl module recipe, as this would mean one has to touch all recipes depending on a module removed from the core every time the Perl version pinning is updated.

The better choice would probably be to have "empty" recipes, as you proposed. However, in my opinion, these should also be fully functional recipes which include an unconditional build/skip key plus a comment stating that this is a core module. Then, should updating the Perl pinning make it necessary, the recipe can be "activated" easily. What do you think?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/conda-forge/perl-feedstock/issues/25#issuecomment-413121911, or mute the thread https://github.com/notifications/unsubscribe-auth/AA_pdBuE3Nw8_VUgZJ_iIGhNdsjL0cDKks5uQ9NIgaJpZM4VTH6U .

bgruening commented 6 years ago

Absolutely not. That's my opinion.

Is this about having empty pkgs? So you don't want to have this?

@dougalsutherland I think it is not that bad to add new module as soon as we pin to a new perl version. Version bumps and re-rendering of pkgs happens with a bot in conda-forge and this bot could easily add new modules.

xileF1337 commented 6 years ago

@mingwandroid Could you please explain what exactly you mean? Which part of my comment don't you agree with and why?

mingwandroid commented 6 years ago

I don't want to have empty packages that pretend to be something they are not, I also do not want sloppy dependency tracking causing unnecessary installation.

I also don't believe that this is a big deal to maintain 'normally' either, how frequently do modules move from core? How frequently do we update perl packages?

xileF1337 commented 6 years ago

... how frequently do modules move from core?

Let's see:

##### All modules REMOVED from one version to next
perl_core_mods_removed() { 
    perl -wle '
        use strict;
        use Module::CoreList; 
        my ($lastver, $nxtver) = @ARGV;
        my %nxtver_mods             # lookup table for core mods of nxt ver
            = map {$_ => 1} Module::CoreList->find_modules(qr/.*/, $nxtver+0);
        print join "\n", 
                   grep {not $nxtver_mods{$_}} 
                        Module::CoreList->find_modules(qr/.*/, $lastver+0);   
    ' $1 $2; 
}

lastver=5.008; 
for nxtver in 5.0{10,12,14,16,18,20,22,24,26}; 
    do echo -e "\n### $lastver => $nxtver ###"; 
    perl_core_mods_removed $lastver $nxtver; 
    lastver=$nxtver; 
done

Output:

### 5.008 => 5.010 ###
B::Asmdata
B::Assembler
B::Bblock
B::Bytecode
B::C
B::CC
B::Disassembler
B::Stackobj
B::Stash
ByteLoader
JNI
JPL::AutoLoader
JPL::Class
JPL::Compile
Thread::Signal
Thread::Specific

### 5.010 => 5.012 ###
CPAN::API::HOWTO
CPAN::DeferedCode                                                                                   
CPANPLUS::inc                                                                                       
ExtUtils::MakeMaker::bytes                                                                          
ExtUtils::MakeMaker::vmsish                                                                         
Test::Harness::Assert                                                                               
Test::Harness::Iterator                                                                             
Test::Harness::Point                                                                                
Test::Harness::Results                                                                              
Test::Harness::Straps                                                                               
Test::Harness::Util                                                                                 
attrs                                                                                               

### 5.012 => 5.014 ###                                                                              
Class::ISA                                                                                          
Pod::Plainer                                                                                        
Switch                                                                                              
TAP::Parser::Source::Perl                                                                           
XS::APItest::KeywordRPN                                                                             

### 5.014 => 5.016 ###                                                                              
Devel::DProf                                                                                        
ExtUtils::MakeMaker::YAML
Locale::Constants
Shell
Sys::Syslog::win32::Win32

### 5.016 => 5.018 ###
List::Util::PP
Scalar::Util::PP
Version::Requirements

### 5.018 => 5.020 ###
Archive::Extract
B::Lint
B::Lint::Debug
CPANPLUS
CPANPLUS::Backend
CPANPLUS::Backend::RV
CPANPLUS::Config
CPANPLUS::Config::HomeEnv
CPANPLUS::Configure
CPANPLUS::Configure::Setup
CPANPLUS::Dist
CPANPLUS::Dist::Autobundle
CPANPLUS::Dist::Base
CPANPLUS::Dist::Build
CPANPLUS::Dist::Build::Constants
CPANPLUS::Dist::MM
CPANPLUS::Dist::Sample
CPANPLUS::Error
CPANPLUS::Internals
CPANPLUS::Internals::Constants
CPANPLUS::Internals::Constants::Report
CPANPLUS::Internals::Extract
CPANPLUS::Internals::Fetch
CPANPLUS::Internals::Report
CPANPLUS::Internals::Search
CPANPLUS::Internals::Source
CPANPLUS::Internals::Source::Memory
CPANPLUS::Internals::Source::SQLite
CPANPLUS::Internals::Source::SQLite::Tie
CPANPLUS::Internals::Utils
CPANPLUS::Internals::Utils::Autoflush
CPANPLUS::Module
CPANPLUS::Module::Author
CPANPLUS::Module::Author::Fake
CPANPLUS::Module::Checksums
CPANPLUS::Module::Fake
CPANPLUS::Module::Signature
CPANPLUS::Selfupdate
CPANPLUS::Shell
CPANPLUS::Shell::Classic
CPANPLUS::Shell::Default
CPANPLUS::Shell::Default::Plugins::CustomSource
CPANPLUS::Shell::Default::Plugins::Remote
CPANPLUS::Shell::Default::Plugins::Source
Devel::InnerPackage
File::CheckTree
Log::Message
Log::Message::Config
Log::Message::Handlers
Log::Message::Item
Log::Message::Simple
Module::Build::Platform::Amiga
Module::Build::Platform::EBCDIC
Module::Build::Platform::MPEiX
Module::Build::Platform::RiscOS
Module::Pluggable
Module::Pluggable::Object
Object::Accessor
Pod::LaTeX
TAP::Parser::Utils
Term::UI
Term::UI::History
Text::Soundex

### 5.020 => 5.022 ###
CGI
CGI::Apache
CGI::Carp
CGI::Cookie
CGI::Fast
CGI::Pretty
CGI::Push
CGI::Switch
CGI::Util
Module::Build
Module::Build::Base
Module::Build::Compat
Module::Build::Config
Module::Build::ConfigData
Module::Build::Cookbook
Module::Build::Dumper
Module::Build::ModuleInfo
Module::Build::Notes
Module::Build::PPMMaker
Module::Build::Platform::Default
Module::Build::Platform::MacOS
Module::Build::Platform::Unix
Module::Build::Platform::VMS
Module::Build::Platform::VOS
Module::Build::Platform::Windows
Module::Build::Platform::aix
Module::Build::Platform::cygwin
Module::Build::Platform::darwin
Module::Build::Platform::os2
Module::Build::PodParser
Module::Build::Version
Module::Build::YAML
Package::Constants
inc::latest

### 5.022 => 5.024 ###
ExtUtils::MakeMaker::version::vpp
Win32API::File::ExtUtils::Myconst2perl
autodie::ScopeUtil
version::vpp

### 5.024 => 5.026 ###

The list is quite lengthy because it contains all sub-packages of each removed module, and it also contains sub-packages removed despite their containing module still being present. Still, you can see that core package removal has happened quite often in the past. Maybe the core module list is more stable now, maybe not.

So what's the alternative to empty packages? Update conda skeleton cpan, maybe to include a hard-coded list of Perl core modules for each Perl version?

mingwandroid commented 6 years ago

So what's the alternative to empty packages? Update conda skeleton cpan, maybe to include a hard-coded list of Perl core modules for each Perl version?

We could scan the source to see what's required? I do this for CRAN stuff (detecting if compilers and autotools etc are needed) and it works well. Not sure if its easy to do this for Perl though?

xileF1337 commented 6 years ago

We know what modules are required already, it can be retrieved from MetaCPAN. But we need to know what's a core module and what not. Scanning the source won't help as, AFAIK, a module doesn't know whether it's a core module or not. In the end, "core module" only means "shipped with a standard Perl distribution". Everyone shipping a Perl distribution may choose to exclude or include different modules, cf. this comment from brian d foy.

Scanning the source would, however, be useful for the other points you mentioned, as they affect Perl as well (we need Make for MakeMaker-modules, compilers + Make for XS modules or Inline'd code, etc).

djsutherland commented 6 years ago

@xileF1337 Aren't those versions extremely old? 5.10 came out in 2007. Is it possible to run it for a more recent set of perl versions?

xileF1337 commented 6 years ago

Version 5.010 is actually version v5.10.0, version 5.022 is version v5.22.0. Perl, for historical reasons, supports two types of formats for version specifications: either as float value or using the v-notation; support for the latter was added later. Example from perldoc -f use:

use v5.6.1;     # compile time version check
use 5.6.1;      # ditto
use 5.006_001;  # ditto; preferred for backwards compatibility

(The underscore in the last line, btw, is just an optional thousands separator which is ignored by the parser and can be omitted. ) So the data shown goes until Perl v5.26.0.

djsutherland commented 6 years ago

Ah, confusing, just like most things about Perl!

In that case I think that empty package outputs, pinned to the particular build, seems to make a lot of sense to me. I wonder whether it's possible to automate that in the recipe jinja, though, instead of having to maintain the list of core packages and checking with each new build....

mingwandroid commented 6 years ago

What is the problem being solved here? I posit that there is none.

Given the changing nature of the core module list, I propose not to remove core module dependencies from each and every Perl module recipe, as this would mean one has to touch all recipes depending on a module removed from the core every time the Perl version pinning is updated.

This is exactly what should be done, you make conda skeleton handle everything for you. For R we routinely do this and it is far from problematic.

djsutherland commented 6 years ago

@mingwandroid The problem is that whether a recipe needs to separately depend on perl-module-build depends on whether it's built against perl 5.20 or 5.22....

There have been few enough recent changes that maybe it's not worth the engineering work for that and skeleton should just be fixed, but conceptually it seems like empty pinned packages are the right solution.

mingwandroid commented 6 years ago

Which is no different from R.

Conceptually empty packages are garbage.

djsutherland commented 6 years ago

With empty packages:

Without empty packages:

World 1 feels like it's a lot less work for recipe maintainers, and the centralized work is done here rather than in conda skeleton which then needs to be released for the updates to take effect rather than happening immediately on the channel. That feels like a better world to me, with the only cost some extra clutter for the solver.

mingwandroid commented 6 years ago

Consumers do not need to do anything. Conda skeleton does.

djsutherland commented 6 years ago

Except when things change from version to version, unless you're calling skeleton again....

I don't think it's a huge deal since it seems like the core modules are relatively stable, and fixing skeleton is probably also reasonable. But it seems to me that empty packages are the conceptually safer solution.

mingwandroid commented 6 years ago

Yes that's precisely what you should do!

Whenever you update perl you update all the things that depend upon it and rebuild them.

xileF1337 commented 6 years ago

To add to the confusion, I just noticed that e.g. conda skeleton cpan Test::More just returns:

We found core module perl-test-more. Skipping recipe creation.

But despite conda skeleton refusing to create a recipe, there is a recipe for perl-test-more in Bioconda and the package is available in the channel.

For the core module Test, however, conda skeleton does create a recipe.

So my conclusions are that: 1) The current policy for conda skeleton is to not create a recipe for Perl core modules, and consequently, not to have any packages for Perl core modules. 2) The detection mechanism for core modules in conda skeleton is broken and does not recognize all core modules properly.

Do you agree so far?

As a consequence from my conclusions, the following actions should be performed: a) conda skeleton needs to be fixed to properly recognize all core modules (e.g. using a hard-coded list of core modules generated by Perls Module::CoreList module). It will then comment them out these dependencies during recipe creation. b) The core module packages and their recipes should be removed from the channels to avoid that contributors unaware of this rule add dependencies to core modules manually, which would cause unnecessary installations (of packages already part of Perl). c) The packages should be blacklisted to prevent that they are re-added by contributors unaware of this rule.

This way we avoid empty packages and can regenerate (most) Perl module recipes by just running conda skeleton should the core module list change. We avoid unnecessary installation of modules and ensure the core module versions match those shipped with the standard Perl distribution. @mingwandroid, does this comply with your stance?

xileF1337 commented 6 years ago

I just found https://github.com/conda/conda-build/pull/2156#issuecomment-313188886 which explains what the code is actually doing. The definition of "core module" in the cpanm skeleton is "is a Perl core module AND does not have a dedicated CPAN release/distribution" (a code comment would have been helpful!). The intent was that if a core module gets updated before a new Perl release, that update can be easily made available via conda. So it seems that conda skeleton is, in fact, working as intended.