automake
[Top][All Lists]
Advanced

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

Re: Redefined extension caused problems


From: Alexandre Duret-Lutz
Subject: Re: Redefined extension caused problems
Date: 03 Jan 2002 23:24:03 +0100
User-agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7

 adl> IMHO, Automake should be changed to work on a line by line basis
 adl> (i.e.  not paragraph) and let file_contents_internal accumulate
 adl> the lines for a rule.  Other opinions?

 Tom> That's what we used to do.  Akim changed how it worked.  He'd really
 Tom> have to address this.

I've discussed this with Akim this morning.  He said the
paragraph stuff was there because it's not always possible to
work on a per-line basis (he mentioned a rule that had so many
targets it would span on several lines using backslashes, thus
making the $RULE_PATTERN regex unusable in a line context) and
recommended to track the rules' paragraphs pretty much like
rules' lines are tracked.

The patch below is my attempt at this.  It fixes the error
observed by Dmitry (extraneous $(F77COMPILE) call in
distclean-compile), but not in the most clever way: I use a flag
to keep track of the rule context from one paragraph to the
other.  The paragraphs are still output as they are proceeded;
which is bogus when the rule has to be output several times
(e.g. for several conditions).

The "right" fix might be to accumulate all the paragraphs for a
rule, and spit the resulting rule only after it has been fully
swallowed.  It's not clear how easy it is.

I've used Dmitry pet-project for testing but haven't turned it
into an Automake testcase: testing whether $(F77COMPILE) appears
in distclean-compile seems quite useless to me (the resulting
bug depends on the ordering into which the am/*.am files are
included and the content of these files, two things that can
vary and make the testcase a noop).  If there was a way to
require Automake to "file_content_internal" a specific file for
this testcase it would be easy to exercise this function..

Index: ChangeLog
--- ChangeLog
+++ ChangeLog
@@ -1,1 +1,9 @@
+2002-01-03  Alexandre Duret-Lutz  <address@hidden>
+
+       * automake.in (file_contents_internal): Introduce two variables,
+       $is_rule and $discard_rules to track rules spanning across multiple
+       paragraphs.  This fixes a very nasty bug reported by Dmitry Mikhin
+       where only the first paragraph of such a multi-paragraph rule was
+       discarded, but leaves many latent bugs (see the FIXMEs).
+

Index: automake.in
===================================================================
RCS file: /home/adl/CVSROOT/automake-20020103-2043/automake.in,v
retrieving revision 1.1
diff -u -r1.1 automake.in
--- automake.in 3 Jan 2002 19:43:26 -0000 1.1
+++ automake.in 3 Jan 2002 21:28:42 -0000
@@ -7232,6 +7232,11 @@
     my $comment = '';
     my $spacing = '';
 
+    # The following flags are used to track rules spanning across
+    # multiple paragraphs.
+    my $is_rule = 0;           # 1 if we are processing a rule.
+    my $discard_rule = 0;      # 1 if the current rule should not be output.
+
     # We save the conditional stack on entry, and then check to make
     # sure it is the same on exit.  This lets us conditonally include
     # other files.
@@ -7248,11 +7253,13 @@
 
        if (/^$/)
        {
+           $is_rule = 0;
            # Stick empty line before the incoming macro or rule.
            $spacing = "\n";
        }
        elsif (/$COMMENT_PATTERN/mso)
        {
+           $is_rule = 0;
            # Stick comments before the incoming macro or rule.
            $comment = "$_\n";
        }
@@ -7289,6 +7296,8 @@
         # Handling rules.
        elsif (/$RULE_PATTERN/mso)
        {
+         $is_rule = 1;
+         $discard_rule = 0;
          # Separate relationship from optional actions: the first
          # `new-line tab" not preceded by backslash (continuation
          # line).
@@ -7309,7 +7318,16 @@
          foreach (split (' ' , $targets))
            {
              # FIXME: We are not robust to people defining several targets
-             # at once, only some of them being in %dependencies.
+             # at once, only some of them being in %dependencies.  The
+             # actions from the targets in %dependencies are usually generated
+             # from the content of %actions, but if some targets in $targets
+             # are not in %dependencies the ELSE branch will output
+             # a rule for all $targets (i.e. the targets which are both
+             # in %dependencies and $targets will have two rules).
+
+             # FIXME: The logic here is not able to output a
+             # multi-paragraph rule several time (e.g. for each conditional
+             # it is defined for) because it only knows the first paragraph.
 
              # Output only if not in FALSE.
              if (defined $dependencies{$_}
@@ -7347,14 +7365,29 @@
 
                  if ($cond ne 'FALSE')
                    {
-                     my $undefined_cond;
-                     for $undefined_cond (@undefined_conds)
+                     for my $undefined_cond (@undefined_conds)
                      {
                          my $condparagraph = $paragraph;
                          $condparagraph =~ s/^/make_condition (@cond_stack, 
$undefined_cond)/gme;
-                         $result_rules .= "$spacing$comment$condparagraph\n"
-                             if rule_define ($targets, $is_am,
-                                             "$cond $undefined_cond", $file);
+                         if (rule_define ($targets, $is_am,
+                                         "$cond $undefined_cond", $file))
+                         {
+                             $result_rules .=
+                                 "$spacing$comment$condparagraph\n"
+                         }
+                         else
+                         {
+                             # Remember to discard next paragraphs
+                             # if they belong to this rule.
+                             $discard_rule = 1;
+                         }
+                     }
+                     if ($#undefined_conds == -1)
+                     {
+                         # This target has already been defined, the rule
+                         # has not been defined. Remember to discard next
+                         # paragraphs if they belong to this rule.
+                         $discard_rule = 1;
                      }
                    }
                  $comment = $spacing = '';
@@ -7369,6 +7402,8 @@
            file_error ($file, "macro `$var' with trailing backslash")
              if /\\$/;
 
+           $is_rule = 0;
+
            # Accumulating variables must not be output.
            append_comments $var, $spacing, $comment;
            macro_define ($var, $is_am, $type, $cond, $val, $file)
@@ -7388,7 +7423,7 @@
            # This isn't an error; it is probably some tokens which
            # configure is supposed to replace, such as address@hidden@',
            # or some part of a rule cut by an if/endif.
-           if ($cond ne 'FALSE')
+           if ($cond ne 'FALSE' && ! ($is_rule && $discard_rule))
              {
                s/^/make_condition (@cond_stack)/gme;
                $result_rules .= "$spacing$comment$_\n";

-- 
Alexandre Duret-Lutz



reply via email to

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