ambs / Config-AutoConf

Config::AutoConf Perl Module
Other
2 stars 7 forks source link

API harmonization / consolidation #5

Closed rehsack closed 9 years ago

rehsack commented 10 years ago

Today I set down with @ribasushi and hacked down a sane way to detect whether a distribution should produce an XS build (with extensive checks) or not as discussed via mail some time ago with e.g. @dagolden and @Leont via ML.

However - during that check_produce_xs_build developing we detected that the code isn't Perlish as it could be. For the next stage we thought about harmozing two behaviorals:

  1. action-if-true/action-if-false for the *_if_else() functions - the plan behind is we replace all positional parameters for the optional action-if-true/action-if-false arguments an optional hash-ref containing the both callbacks. This allows easier distingish between specifiying only one of both callbacks or have multiple callbacks in a method and be clear about the action-if-true/action-if-false callbacks.
  2. where reasonable, add an _or_die methods (compile_or_die, check_valid_perlapi_or_die, ...) which is based on above described optional action-if-false callback.

If anyone (http://grep.cpan.me/ mentions @ambs, @rehsack, @autarch, @maxmind, @jeffreykegler, @mongodb, @rra, Paweł Murias, @ingydotnet) is using any of above mentioned _if_else methods, please cry out load and I will submit patches for the distribution and a migration path.

jeffreykegler commented 10 years ago

Where would I look? Is the question about Config::AutoConf functions? (That's my guess but I want to be sure.)

rehsack commented 10 years ago

http://grep.cpan.me/?q=Config%3A%3AAutoConf tells me Marpa-R2-2.092000/Build.PL ;)

jeffreykegler commented 10 years ago

Actually Marpa::R2's build code is distributed over several files. I'll look and get back to you.

rra commented 10 years ago

Jens Rehsack notifications@github.com writes:

However - during that check_produce_xs_build developing we detected that the code isn't Perlish as it could be. For the next stage we thought about harmozing two behaviorals:

  1. action-if-true/action-if-false for the *_if_else() functions - the plan behind is we replace all positional parameters for the optional action-if-true/action-if-false arguments an optional hash-ref containing the both callbacks. This allows easier distingish between specifiying only one of both callbacks or have multiple callbacks in a method and be clear about the action-if-true/action-if-false callbacks.

I'm using _if_else for the following function (which I kept meaning to contribute back, since I think it would be good to have in Config::AutoConf itself, but also needs the same transformation):

# Check whether it's possible to link a program that uses a particular
# function.  This is written like a Config::AutoConf method and should ideally
# be incorporated into that module.  This macro caches its result in the
# ac_cv_func_FUNCTION variable.
#
# $self         - The Config::AutoConf state object
# $function     - The function to check for
# $found_ref    - Code reference to call if the function was found
# $notfound_ref - Code reference to call if the function wasn't found
#
# Returns: True if the function was found, false otherwise
sub check_func {
    my ($self, $function, $found_ref, $notfound_ref) = @_;
    $self = $self->_get_instance();

    # Build the name of the cache variable.
    my $cache_name = $self->_cache_name('func', $function);

    # Wrap the actual check in a closure so that we can use check_cached.
    my $check_sub = sub {
        my $have_func = $self->link_if_else($self->lang_call(q{}, $function));
        if ($have_func) {
            if (defined($found_ref) && ref($found_ref) eq 'CODE') {
                $found_ref->();
            }
        } else {
            if (defined($notfound_ref) && ref($notfound_ref) eq 'CODE') {
                $notfound_ref->();
            }
        }
        return $have_func;
    };

    # Run the check and cache the results.
    return $self->check_cached($cache_name, "for $function", $check_sub);
}

This is currently in AFS::PAG plus another module I maintain that's still experimental and isn't on CPAN yet.

rehsack commented 10 years ago

Beside that I would be happy to integrate that check_func method before doing the refactoring - how about search_lib as done in Unix::Statgrab (https://github.com/i-scream/Unix-Statgrab/blob/master/inc/Config/AutoConf/SG.pm)?

jeffreykegler commented 10 years ago

Ack'ing for _if_else will find everything, right? I did that and have some usage of compile_if_else in code contributed by someone else.

rehsack commented 10 years ago

I'm not sure whether it will find everything - but gives hints.

When you tell me where to check (I might not be able to verify at all) I submit you patches when I'm done in API consolidation.

jeffreykegler commented 10 years ago

@rehsack All calls to Config::AutoConf methods will be in text files under than cpan directory in Marpa::R2. If you just give me locations of problematic method calls, I'll go back to the writer. We may very likely be able to save you the trouble of patching, by eliminating the call, etc., etc. Our install stuff is frankly a little ugly.

jeffreykegler commented 10 years ago

@rehsack Or else just give me a list of the method names to grep/ack for.

rehsack commented 10 years ago

@jeffreykegler I can do that after I finished the consolidation. There will be no upload without warning and even a bunch of dev-releases before.

jeffreykegler commented 10 years ago

Great! I look forward to hearing from you.

On 09/17/2014 12:35 PM, Jens Rehsack wrote:

@jeffreykegler https://github.com/jeffreykegler I can do that after I finished the consolidation. There will be no upload without warning and even a bunch of dev-releases before.

— Reply to this email directly or view it on GitHub https://github.com/ambs/Config-AutoConf/issues/5#issuecomment-55947707.

rra commented 10 years ago

Jens Rehsack notifications@github.com writes:

Beside that I would be happy to integrate that check_func method before doing the refactoring - how about search_lib as done in Unix::Statgrab (https://github.com/i-scream/Unix-Statgrab/blob/master/inc/Config/AutoConf/SG.pm)?

Ah, sorry, I gave you incomplete information. check_func is part of the implementation of check_funcs, and I specifically need those semantics rather than the semantics of search_libs, since I'm not checking for a function in a set of libraries. Rather, I've already decided on a library and I need to probe its capabilities.

Here's the calling function. The two together should match the semantics of AC_CHECK_FUNC and AC_CHECK_FUNCS in Autoconf, for similar purposes. Happy to contribute both of them to Config::AutoConf. I had been intending to do that and had just never gotten around to putting them into a reasonable diff and putting them in RT.

# The same as check_func, but takes a list of functions to look for and checks
# for each in turn.  Define HAVE_FUNCTION for each function that was found,
# and also run the $found_ref code each time a function was found.  Run the
# $notfound_ref code each time a function wasn't found.  Both code references
# are passed the name of the function that was found.
#
# $self          - The Config::AutoConf state object
# $functions_ref - Reference to an array of functions to check for
# $found_ref     - Code reference to call if a function was found
# $notfound_ref  - Code reference to call if a function wasn't found
#
# Returns: True if all functions were found, false otherwise.
sub check_funcs {
    my ($self, $functions_ref, $user_found_ref, $user_notfound_ref) = @_;
    $self = $self->_get_instance();

    # Build the code reference to run when a function was found.  This defines
    # a HAVE_FUNCTION symbol, plus runs the current $found_ref if there is
    # one.
    my $func_found_ref = sub {
        my ($function) = @_;

        # Generate the name of the symbol we'll define.
        my $have_func_name = 'HAVE_' . uc($function);
        $have_func_name =~ tr/_A-Za-z0-9/_/c;

        # Define the symbol.
        $self->define_var($have_func_name, 1,
            "Defined when $function is available");

        # Run the user-provided hook, if there is one.
        if (defined($user_found_ref) && ref($user_found_ref) eq 'CODE') {
            $user_found_ref->($function);
        }
    };

    # Go through the list of functions and call check_func for each one.  We
    # generate new closures for the found and not-found functions that pass in
    # the relevant function name.
    my $return = 1;
    for my $function (@{$functions_ref}) {
        my $found_ref    = sub { $func_found_ref->($function) };
        my $notfound_ref;
        if (defined($user_notfound_ref)) {
            $notfound_ref = sub { $user_notfound_ref->($function) };
        }
        $return &= check_func($self, $function, $found_ref, $notfound_ref);
    }
    return $return;
}
rehsack commented 10 years ago

Today I managed consolidating the basic methods of Config::AutoConf.

@rra - I merged your check_func(s) - I also prefer to try casting tested function to void - but this might cause trouble in c++ environments with function overloading. However - the original code in Autoconf which tries to computes the address of the function heavily fails with GCC builtin's.

I have a patch for the Autoconf macros - maybe it's worth to port it to the perl test.

Anyway - if someone wants to test this state of C::AC, please use $ perl -Mblib=//Config-AutoConf Makefile.PL | Build.PL

Anyone who wants C::AC becomes more perlish - please check consolidated API and provide patches / comments (well reasoned patches preferred).

rra commented 10 years ago

Jens Rehsack notifications@github.com writes:

Today I managed consolidating the basic methods of Config::AutoConf.

@rra - I merged your check_func(s)

Thanks! And thank you for all your work on the module!

I also prefer to try casting tested function to void - but this might cause trouble in c++ environments with function overloading. However - the original code in Autoconf which tries to computes the address of the function heavily fails with GCC builtin's.

I have a patch for the Autoconf macros - maybe it's worth to port it to the perl test.

Ah, I had never attempted to use them with GCC's built-ins. That does sound like a good thing to run past Autoconf upstream and possibly add to the Perl module.

Russ Allbery (eagle@eyrie.org) http://www.eyrie.org/~eagle/

rehsack commented 10 years ago

There are a lot of weird comments regarding parts of the patches upstream ... Compiler builtins are in every compiler - but with GCC one can't distingiush a function or builtin at compile time. When I have C::AC and LMU out, remind me nagging upstream ;)

@tdb had the same job, but his Alzheimer is in advanced stage :P

Leont commented 10 years ago

I'm not fond of that interface either, but as someone who isn't using it myself I don't have firm opinions on how it should be.

rehsack commented 9 years ago

HI folks,

this is an update of refactoring job. I think it's done. Would be cool if one or two review the changes and try them out in their own projects to avoid errors on end-users.

Cheers, Jens

ambs commented 9 years ago

Is it in the main branch? Thanks

rehsack commented 9 years ago

Yes, it's in the main branch ... I plan to "fix" a loading issue allowing C::AC can be loaded without EU::CB to allow check_prog_cc to fail without EU::CB and open for @Leont EU::B in later times ... That's the content for next (dev-) release unless someone has a strong wish/opinion (reasonable!).

rehsack commented 9 years ago

I uploaded 0.306_001 which is supposed to "fix" this issue. Refactoring started with a382b86b4a1d7d119ed3e519eb2df76fde7affdc - so a complete review should be done by "git diff a382b86b4a1d7d119ed3e519eb2df76fde7affdc 0.306_001".

rehsack commented 9 years ago

This one can be closed now - no issues for some month ...