#46 Target support for Makefile parser

open
None
5
2008-03-29
2008-03-23
Enrico Tröger
No

Attached is a simple patch to add detection of Makefile targets to the Makefile parser.

Discussion

1 2 > >> (Page 1 of 2)
  • Enrico Tröger
    Enrico Tröger
    2008-03-23

    Target detection in Makefiles

     
  • Elliott Hughes
    Elliott Hughes
    2008-03-29

    • assigned_to: nobody --> elliotth
     
  • Elliott Hughes
    Elliott Hughes
    2008-03-29

    Logged In: YES
    user_id=1127237
    Originator: NO

    looks okay to me.

    might also be nice to treat special targets differently (probably not outputting a tag for them)? seems silly to have a hundred tags called ".PHONY".

    this patch also gets confused by stuff like:

    DRY_RUNNING := $(findstring n,$(MAKEFLAGS))

    thinking it's seeing a target called DRY_RUNNING.

     
  • Elliott Hughes
    Elliott Hughes
    2008-03-29

    Logged In: YES
    user_id=1127237
    Originator: NO

    how about something more like this:

    --- trunk/make.c (revision 650)
    +++ trunk/make.c (working copy)
    @@ -26,11 +26,12 @@ (unknown)
    * DATA DEFINITIONS
    */
    typedef enum {
    - K_MACRO
    + K_MACRO, K_TARGET
    } shKind;

    static kindOption MakeKinds [] = {
    - { TRUE, 'm', "macro", "macros"}
    + { TRUE, 'm', "macro", "macros"},
    + { TRUE, 't', "target", "targets"}
    };

    /*
    @@ -73,6 +74,21 @@ isIdentifier
    return (boolean)(c != '\0' && (isalnum (c) || strchr (".-_", c) != NULL));
    }

    +static boolean notSpecial (vString *const name)
    +{
    + return (vStringChar (name, 0) != '.');
    +}
    +
    +static void newTarget (vString *const name)
    +{
    + makeSimpleTag (name, MakeKinds, K_TARGET);
    +}
    +
    +static void newMacro (vString *const name)
    +{
    + makeSimpleTag (name, MakeKinds, K_MACRO);
    +}
    +
    static void readIdentifier (const int first, vString *const id)
    {
    int c = first;
    @@ -162,7 +178,7 @@ findMakeTags
    in_define = TRUE;
    c = skipToNonWhite ();
    readIdentifier (c, name);
    - makeSimpleTag (name, MakeKinds, K_MACRO);
    + newMacro (name);
    skipLine ();
    }
    else {
    @@ -171,7 +187,17 @@ findMakeTags
    {
    boolean append = (boolean)(c == '+');
    if (c == ':')
    - in_rule = TRUE;
    + {
    + int c2 = nextChar ();
    + fileUngetc (c2);
    + if (c2 != '=') {
    + in_rule = TRUE;
    + if (notSpecial (name))
    + {
    + newTarget (name);
    + }
    + }
    + }
    c = nextChar ();
    if (c != '=')
    fileUngetc (c);
    @@ -183,7 +209,7 @@ findMakeTags
    }
    if (c == '=')
    {
    - makeSimpleTag (name, MakeKinds, K_MACRO);
    + newMacro (name);
    in_rule = FALSE;
    skipLine ();
    }

     
  • Elliott Hughes
    Elliott Hughes
    2008-03-29

    Logged In: YES
    user_id=1127237
    Originator: NO

    that's not right either. it doesn't cope with a target like .generated/build-version.txt, for example, nor a rule with multiple targets (as in "kbd.o command.o files.o : command.h", say).

    this patch is closer, but it misses lots of variable definitions that used to be tagged as macros. (arguably that was already wrong, but a less contentious fix would tag variable definitions as variable definitions!)

    i notice also that we don't have a single test case for the makefile parser. we could do with at least a simple and a complex test case. maybe the examples from http://www.gnu.org/software/make/manual/make.html would be good.

    --- trunk/make.c (revision 650)
    +++ trunk/make.c (working copy)
    @@ -26,11 +26,12 @@ (unknown)
    * DATA DEFINITIONS
    */
    typedef enum {
    - K_MACRO
    + K_MACRO, K_TARGET
    } shKind;

    static kindOption MakeKinds [] = {
    - { TRUE, 'm', "macro", "macros"}
    + { TRUE, 'm', "macro", "macros"},
    + { TRUE, 't', "target", "targets"}
    };

    /*
    @@ -70,9 +71,41 @@ skipToNonWhite

    static boolean isIdentifier (int c)
    {
    - return (boolean)(c != '\0' && (isalnum (c) || strchr (".-_", c) != NULL));
    + return (boolean)(c != '\0' && (isalnum (c) || strchr (".-_/", c) != NULL));
    }

    +static boolean isSpecialTarget (vString *const name)
    +{
    + size_t i = 0;
    + /* All special targets begin with '.'. */
    + if (vStringChar (name, i++) != '.') {
    + return FALSE;
    + }
    + while (i < vStringLength (name)) {
    + char ch = vStringChar (name, i++);
    + if (ch != '_' && !isupper (ch))
    + {
    + return FALSE;
    + }
    + }
    + return TRUE;
    +}
    +
    +static void newTarget (vString *const name)
    +{
    + /* Ignore GNU Make's "special targets". */
    + if (isSpecialTarget (name))
    + {
    + return;
    + }
    + makeSimpleTag (name, MakeKinds, K_TARGET);
    +}
    +
    +static void newMacro (vString *const name)
    +{
    + makeSimpleTag (name, MakeKinds, K_MACRO);
    +}
    +
    static void readIdentifier (const int first, vString *const id)
    {
    int c = first;
    @@ -162,7 +195,7 @@ findMakeTags
    in_define = TRUE;
    c = skipToNonWhite ();
    readIdentifier (c, name);
    - makeSimpleTag (name, MakeKinds, K_MACRO);
    + newMacro (name);
    skipLine ();
    }
    else {
    @@ -170,23 +203,33 @@ findMakeTags
    if (strchr (":?+", c) != NULL)
    {
    boolean append = (boolean)(c == '+');
    - if (c == ':')
    + boolean was_colon = (c == ':');
    + c = nextChar ();
    + if (was_colon && c != '=')
    + {
    in_rule = TRUE;
    - c = nextChar ();
    - if (c != '=')
    - fileUngetc (c);
    + newTarget (name);
    + }
    else if (append)
    {
    skipLine ();
    continue;
    }
    + else
    + {
    + fileUngetc (c);
    + }
    }
    - if (c == '=')
    + else if (c == '=')
    {
    - makeSimpleTag (name, MakeKinds, K_MACRO);
    + newMacro (name);
    in_rule = FALSE;
    skipLine ();
    }
    + else
    + {
    + fileUngetc (c);
    + }
    }
    }
    else

     
  • Enrico Tröger
    Enrico Tröger
    2008-04-03

    Better handling of rules with multiple targets

     
  • Enrico Tröger
    Enrico Tröger
    2008-04-03

    Logged In: YES
    user_id=1117045
    Originator: YES

    Your version is much better. I didn't take care of special targets like .PHONY but I agree it doesn't make much sense to list them.

    I'll attach a modified version of your last patch which handles rules with multiple targets better ("kbd.o command.o files.o : command.h").

    I'm not sure what you mean with "variable definitions" (I'm not a Makefile guru ;-)).
    File Added: ctags_make_c_target_v2.patch

     
  • Elliott Hughes
    Elliott Hughes
    2008-04-13

    Logged In: YES
    user_id=1127237
    Originator: NO

    if you download the source .tgz from http://software.jessies.org/salma-hayek/ you'll have a nice little collection of modern makefiles. if you specifically look at lib/build/parent.make you'll see that the current make.c finds only the variable "SUBDIRS" (and misses the targets), whereas this patch finds the targets but misses "SUBDIRS".

    notice also JAVAHPP_RULE in lib/build/per-directory.make: it's an explicit "define" that the current code tags, but the patched code misses.

    (i'd be happier if only we had some tests, because tests usually indicate filed bugs, and that gives us some idea of what features, if any, people are actually using. when we get a patch we want to commit, we'll have to make sure that we commit some examples to the Tests/ directory, too, showing what we claim to support.)

     
  • Enrico Tröger
    Enrico Tröger
    2008-04-15

    Fix the mentioned issues.

     
  • Enrico Tröger
    Enrico Tröger
    2008-04-15

    Logged In: YES
    user_id=1117045
    Originator: YES

    Attached is patch v3 which fixes the mentioned problems(missing SUBDIRS macro and the JAVAHPP_RULE).
    I think it should be ok now at least for most cases. There are probably some rare cases where the parser fails but this can IMO be fixed in the future.
    For usual Makefiles it should be enough I think. I guess most users would mostly only read/edit/write simple Makefiles and any auto-generated Makefiles aren't that often read and probably never modified.

    I'll attach also a simple.mak which is just a collection of mentioned macros and targets. No idea whether this is what you like to add in the Tests/ directory as it isn't a real, working Makefile.
    File Added: ctags_make_c_target_v3.patch

     
1 2 > >> (Page 1 of 2)