Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

option precedence needs to be reviewed #3

Open
dagolden opened this issue Feb 7, 2012 · 6 comments
Open

option precedence needs to be reviewed #3

dagolden opened this issue Feb 7, 2012 · 6 comments

Comments

@dagolden
Copy link

dagolden commented Feb 7, 2012

c.f. discussion in 886abd6

tl;dr: precedent of options set by .moduledefaultrc, PERL_MB_OPT and the command line should be reviewed. It should most likely be as follows:

# during Build.PL
.modulebuildrc supplied info
PERL_MB_OPT supplied info
things I passed when calling Build.PL

# during ./Build action_name
merged results of calling Build.PL
.modulebuildrc supplied info for action_name
things I passed when calling ./Build action_name
@Leont
Copy link
Member

Leont commented Feb 7, 2012

  1. I'm not sure we should specify in that much detail when things happen, only that they have to happen in a certain prescribed order.
  2. I am not quite sure what you mean exactly with that first «.modulebuildrc supplied info». Build_PL and *?
  3. I am not so sure the «.modulebuildrc supplied info for action_name» should happen after «things I passed when calling Build.PL». It sounds rather counter-intuitive, and suffering from the same problem mst already described.

I would suggest this variant on mst's order:

  • .modulebuildrc supplied info
      • arguments
    • action specific arguments
  • PERL_MB_OPT supplied info
  • things I passed when calling Build.PL
  • things I passed when calling ./Build action_name

@dagolden
Copy link
Author

dagolden commented Feb 8, 2012

Two big problems with all of this:

  • Both PERL_MB_OPT and .modulebuildrc could be different between the Buld.PL run and the ./Build action run.
  • Both * arguments and PERL_MB_OPT (currently at least) apply to all actions. To me, that implies a very high level of precedence. They are not "default" options they are "always do this" options.

Thus, I think it's cleaner to think of two phases: configuration (Build.PL) and actions; within each we look to what options are least-specific to most-specific.

For configuration: .modulebuildrc (both * and Build_PL, probably in that order from least to most specific) and PERL_MB_OPT and the command line provide inputs that determine a set of configuration outputs that are persisted to disk.

For actions: the configuration outputs are the least-specific starting point, modified by action-specific options in .modulebuildrc and the action command line arguments.

Taking PERL_MB_OPT out of the action phase avoids having its high-precedence intended for the configuration phase blead into the action phase where configuration results have lowest precedence.

@Leont
Copy link
Member

Leont commented Feb 10, 2012

Both PERL_MB_OPT and .modulebuildrc could be different between the Buld.PL run and the ./Build action run.

This is an issue, though I'm tempted to call both EBKAC.

Both * arguments and PERL_MB_OPT (currently at least) apply to all actions. To me, that implies a very high level of precedence. They are not "default" options they are "always do this" options.

Same can be said of Build.PL arguments. This thing should never have become as complicated as it is now.

Thus, I think it's cleaner to think of two phases: configuration (Build.PL) and actions; within each we look to what options are least-specific to most-specific.

For configuration: .modulebuildrc (both * and Build_PL, probably in that order from least to most specific) and PERL_MB_OPT and the command line provide inputs that determine a set of configuration outputs that are persisted to disk.

For actions: the configuration outputs are the least-specific starting point, modified by action-specific options in .modulebuildrc and the action command line arguments.

Taking PERL_MB_OPT out of the action phase avoids having its high-precedence intended for the configuration phase blead into the action phase where configuration results have lowest precedence.

I think you're thinking about this too much in terms of Module::Build's implementation details. In particular, M::B is currently not separating reading the input from parsing it (see a pattern?), which makes the two-stage process a mess. This approach doesn't seem cleaner to think of at all to me.

@dagolden
Copy link
Author

On Thu, Feb 9, 2012 at 7:14 PM, Leon Timmermans
<reply+i-3129272-8742c57bde961e338d19cec52acba3726b96080c-

I think you're thinking about this too much in terms of Module::Build's implementation details. In particular, M::B is currently not separating reading the input from parsing it (see a pattern?), which would makes the two-stage process a mess. This approach doesn't seem cleaner to think off at all to me.

You may be right. However, it's the defacto standard. I think what I
described is how it worked before PERL_MB_OPT was added. It wouldn't
take a lot of code to fix.

I'm open to something better, or even, potentially, fixing the spec if
we can find a way to do so that doesn't break backwards compatibility.
Possibly horrible thought: .modulebuld.conf with a new format that
doesn't have "*" and deprecate old config file name.

That shouldn't break modern local::lib's that use PERL_MB_OPT as long
as we get that to apply at the right place.

All this sounds like fodder for Paris and copious alcohol.

-- David

@Leont
Copy link
Member

Leont commented Feb 16, 2012

Thinking about it, we're missing item in the precedence list: the Module::Build constructor. Fortunately, this is the lowest precedence of them all, which is the only sensible option IMHO.

@Leont
Copy link
Member

Leont commented Jul 17, 2012

Possibly horrible thought: .modulebuld.conf with a new format that doesn't have "*" and deprecate old config file name.

Actually, I wouldn't mind deprecating .modulebuildrc at all. It's a lot of head-acke though no-one really uses it anymore. I don't think it really needs a replacement either, an external config file doesn't fit in well with how the rest of the toolchain works IMHO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant