automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: User extensions


From: Ralf Wildenhues
Subject: Re: User extensions
Date: Mon, 1 Nov 2010 21:54:29 +0100
User-agent: Mutt/1.5.20 (2010-08-04)

* Valentin David wrote on Mon, Nov 01, 2010 at 09:41:47PM CET:
> On Mon, Nov 1, 2010 at 8:18 PM, Ralf Wildenhues wrote:
> > Also, I like the approach of calling the whole feature experimental and
> > at the same time asking users to tell us which features from automake.in
> > they need so we can document them and add testsuite coverage for them so
> > we can be sure to not break them in the future.
> 
> The problem is that the important variables in automake.in are all
> local (my). So even if you set the module to package Automake, you
> still cannot access to them without modifying automake.in. We should
> have accessors to most of them. For example, in the patch I sent,
> there is a bug in the test case, as it tries to add something
> %clean_files but actually does not.

Good points.  Accessors sound like a good idea; more generally, it will
probably end up being cleanest to do some restructuring so that a more
stable API can be provided.

The testsuite additions should ensure that such things as cleaning files
actually work.

> >> @@ -1876,9 +1887,9 @@ sub handle_single_transform ($$$$$%)
> >>           $linker = $lang->linker;
> >>
> >>           my $this_obj_ext;
> >> -         if (defined $source_extension)
> >> +         if ($output_extension ne '.$(OBJEXT)')
> >
> > Why this?  This looks wrong, but I cannot judge because I don't
> > understand why your patch would need this.
> 
> lang_..._rewrite sometimes returned a pair where the rhs to be
> $source_extension, which correspond to the extension of the generated
> file. However, there is already another entry that does this. When I
> rewrote the lang_..._rewrite, I just dropped this variable. Now it
> uses $output_extension which is always defined ($source_extension was
> also not really a wise name given it is the generated file, not really
> a "source"). Then to check if it is derived source (which means needs
> another compiler after), the test is to check the extension needed of
> the output.

OK.

> >>           {
> >> -             $this_obj_ext = $source_extension;
> >> +             $this_obj_ext = $output_extension;
> >>               $derived_source = 1;
> >>           }
> >>           elsif ($lang->ansi)
> >> @@ -2056,10 +2067,19 @@ sub handle_single_transform ($$$$$%)
> >>           $lang->target_hook ($aggregate, $object, $full, %transform);
> >>       }
> >>
> >> +     # Transform .o or $o file into .P file (for automatic
> >> +     # dependency code).
> >> +     if ($lang && $lang->autodep ne 'no')
> >> +     {
> >> +         my $depfile = $object;
> >> +         $depfile =~ s/\.([^.]*)$/.P$1/;
> >> +         $depfile =~ s/\$\(OBJEXT\)$/o/;
> >> +         $dep_files{dirname ($depfile) . '/$(DEPDIR)/'
> >> +                      . basename ($depfile)} = 1;
> >> +     }
> >> +
> >>       if ($derived_source)
> >>         {
> >> -         prog_error ($lang->name . " has automatic dependency tracking")
> >> -           if $lang->autodep ne 'no';
> >
> > This removal (without replacement) strikes me as weird.  What did you
> > need it for?  We should have a testsuite addition that exposes the
> > error, and see that a fix of yours keeps the old failure working.
> 
> It is wrong to assume that a compiler that generates source files
> cannot have automatic dependencies.

Sure.  What I'm not sure is whether the rest of the Automake code
doesn't provide the required functionality only for such languages.
Hmm, this was introduced in Release-1-4f-43-g081f2d5, which is really
long ago; it might still be valid but then again maybe not.

How about we remove it at the point where we add testsuite coverage for
a language that has this feature (so we can be fairly sure that Automake
is prepared)?

> Take Stratego, this compiler takes
> .str files and generate .c files. But several .str files can be
> dependency to one .c file. And the compiler actually generate a
> dependency file. The current way is now to have a "-include $(wildcard
> *.dep)" in each file. And this is not really good thing.
> 
> See http://strategoxt.org/

Thanks for the explanations.

> Secondly this error message can appear only on a bug in Automake, and
> cannot be triggered from the files of the user. So you cannot really
> make a test that fails here. And now that the autodep section (moved
> right before) can handle derived source, this assertion is not useful
> anymore.

> >> +register_language (# Short name of the language (c, f77...).
> >> +                'name' => "foo",
> >> +                # Nice name of the language (C, Fortran 77...).
> >> +                'Name' => "Foo",
> >> +                # List of configure variables which must be defined.
> >> +                'config_vars' => ['FOO'],
> >> +                'autodep' => 'FOO',
> >> +
> >> +                # Name of the compiling variable (COMPILE).
> >> +                'compiler'  => "FOOCOMPILE",
> >> +                # Content of the compiling variable.
> >> +                'compile'  => '$(FOO) $(FOOFLAGS)',
> >> +                # Flag to require compilation without linking (-c).
> >> +                'extensions' => ['.foo'],
> >> +                # A subroutine to compute a list of possible extensions of
> >> +                # the product given the input extensions.
> >> +                # (defaults to a subroutine which returns ('.$(OBJEXT)', 
> >> '.lo'))
> >> +                'output_extensions' => sub { return ('.c',); },
> >> +                # A list of flag variables used in 'compile'.
> >> +                # (defaults to [])
> >> +                'flags' => ["FOOFLAGS"],
> >> +
> >> +                # The file to use when generating rules for this language.
> >> +                # The default is 'depend2'.
> >> +                'rule_file' => "foo",
> >> +
> >> +                # Name of the compiler variable (CC).
> >> +                'ccer' => "FOO",
> >> +
> >> +                '_finish' => sub {},
> >> +
> >> +                # This is a subroutine which is called whenever we finally
> >> +                # determine the context in which a source file will be
> >> +                # compiled.
> >> +
> >> +                '_target_hook' => sub {
> >> +                  my ($self, $aggregate, $output, $input, %transform) = 
> >> @_;
> >> +                  $clean_files{$output} = MAINTAINER_CLEAN;
> >> +                },
> >> +
> >> +                # If TRUE, nodist_ sources will be compiled using 
> >> specific rules
> >> +                # (i.e. not inference rules).  The default is FALSE.
> >> +                'nodist_specific' => 1);
> >
> > All of these struct elements would need to be documented eventually, I
> > guess.  Pretty ugly, with things like _target_hook and the like...
> 
> I tried not to modify too much yet. So the ugly things in automake.in
> are emerging now. But probably we need to do something about it, sure.
> But I have no idea what names to give. We can change that when we
> decide to document an API.

ACK.

> I hope I will be able to prepare the new patches pretty soon.

Cool.  Whatever you send, several small changes (each being a logical
entity) is always easier to review than one large one.

Thanks,
Ralf



reply via email to

[Prev in Thread] Current Thread [Next in Thread]