Regex matching for _find

[kriegaex]

Hi!

I am using your nice script to visualise makefiles, but sometimes want to match targets in a pattern-based way for "exclude" or "end_with". So I did this little, useful change. I would be happy if you included it upstream:

--- /usr/local/share/perl/5.12.4/Makefile/GraphViz.pm   2011-08-17 18:29:38.000000000 +0200
+++ /usr/local/share/perl/5.12.4/Makefile/GraphViz.pm   2011-10-22 07:25:26.000000000 +0200
@@ -68,7 +68,7 @@
 sub _find ($@) {
     my $elem = shift;
     foreach (@_) {
-        return 1 if $elem eq $_;
+        return 1 if $elem =~ m/$_/;
     }
     return undef;
 }

[agentzh]

This patch breaks the test suite because the _find method is used to look up exact match by design. Maybe we should explicitly pass a Perl regex object to claim that we need a fuzzy match here?

[kriegaex]

Sounds good to me. What would the code look like then? And how would I use it? (Attention, I do not speak Perl, I am just calling your script!) Currently I am doing something like this with the patched version above:

# Helper script for shell script visualise_make
#
# Author: Alexander Kriegisch (http://scrum-master.de), 2008-02-10

use Makefile::GraphViz;

# Parse makefile
$parser = Makefile::GraphViz->new;
$parser->parse($ARGV[0]);

# Let GraphViz generate an image layout and make sure to exclude "|" as a
# target (will be falsely regarded as one sometimes) and do not display
# dependencies of ".config" (lots of "Config.in" files)
$gv = $parser->plot(
    $ARGV[1],
    trim_mode => true,
    init_args => { rankdir => true,  },
    exclude => [ "^\\|\$", "/[.](unpack|configur)ed\$" ],
    end_with => [ "^[.]config\$", "^Config[.]in[.]cache\$", "^dl\$", "^download-toolchain\$", "^glib2-precompiled\$" ]
);

# Save as PNG bitmap
$gv->as_png($ARGV[2]);

The Perl script is called by a shell script which prepares and cleans up a big makefile with recursive includes like this, because your script cannot deal with what I need correctly:

#!/bin/bash

# Visualise makefile dependencies using Perl module Makefile::GraphViz, see
# http://search.cpan.org/~agent/Makefile-GraphViz/lib/Makefile/GraphViz.pm
#
# If not installed, you will probably see an error like this:
#     Creating dependency graph image 'foo.png' for target 'foo' ...
#     Can't locate GraphViz.pm in @INC (@INC contains: ...)
#     at tools/visualise_make.pl line 5.
#
# So you need to install the module. But before you do, make sure you install
# GraphViz (Debian package "graphviz") first, otherwise the Perl module's
# installation will fail later because of this missing dependency.
#
# You might want to install the module and its dependencies using
# Debian/Ubuntu package "cpanminus" and then issuing the shell command
#     sudo cpanm Makefile::GraphViz
#
# If tests fail for Makefile::GraphViz, retry and force installation after
# failed tests when prompted. In my case, the module worked flawlessly anyway:
#     sudo cpanm Makefile::GraphViz --prompt
#
# Author: Alexander Kriegisch (http://scrum-master.de), 2008-02-10

CMD=$(basename $0)

usage()
{
    cat << EOF

Usage:     $CMD [options] target [log-target]
More help: $CMD -?

EOF
}

help()
{
    cat << EOF

$CMD - create PNG image visualising a Make target's dependency graph

Usage: $CMD [options] target [log-target]
  -?          print this usage help
  -a [lfica]  perform specified actions (default: lfi)
     l        create log via "make -p"
     f        filter log as a preparation for visualisation
     i        create dependency graph image for specified target
     c        clean up: delete log and filtered file before exiting
     a        all: equal to "lfic"
  -l <file>   log file name (default: $CMD.log); contains complete
              output of: LC_ALL=C make -j1 -pns log-target
  -f <file>   filtered log file name (default: $CMD.flt); contains only the
              main makefile's targets (no sub-makefile contents in case of 
              recursive make calls) without variables, implicit rules,
              actions, comments etc., i.e. just the data needed for the
              dependency graph
  -i <file>   alternative image file name (default: <target>.png, but dots,
              spaces and slashes in target names will be replaced by
              underscores, condensing consecutive underscores to one, i.e.
              target "../src/foo_/bar/zot.h" becomes image file name
              "_src_foo_bar_zot_h.png")
  target      target to be visualised, resulting in <target>.png (mandatory)
  log-target  target for log file creation (optional); default: none, i.e.
              makefile's default target (usually "all"); specify only if you
              wish to create the log-file from a target other than the
              default one.

Examples:
  $CMD my_target
      create + filter log file, possibly overwriting existing *.log, *.flt
      files; then create my_target.png containing visual representation of
      my_target's dependency graph; do not delete *.log, *.flt so they can be
      re-used with another target
  $CMD -a i my_other_target
      create my_other_target.png, re-using existing $CMD.flt
  $CMD -a fi -l my_dir/saved_make.log -i graph-03.png my_3rd_target
      filter existing make log into $CMD.flt, then create
      graph-03.png for specified target
  $CMD -a a -l make.log -f make.flt my_target big_target
      create + filter log file, using custom file names make.log, make.flt,
      but do not use default make target for log file creation, but big_target;
      then create visual representation of my_target's dependency graph;
      finally, clean up make.log, make.flt
  $CMD -a c
      delete $CMD.log and $CMD.flt, do nothing else
  $CMD -?
      print usage help

Hints for creating custom log files:
  - You may create your own make logs via "make -p".
  - If so, take care to use "-j1" so as to get a clean log file with
    sequential entries. Multi-threaded make may create garbled output.
    If your makefile contains -j2 or greater, change it there temporarily,
    because the command line switch might not override the makefile setting.
  - If you just want to simulate a make run in order to create a log file,
    but do not want to actually build files, use "make -n" (dry-run).
    Combined with log output generation this would be "make -pn".
  - This tool can only filter English make logs, so please use
    "LC_ALL=C make ..." so as to avoid locale-specific output.
  - This tool only works well for top-level makefiles, so if you have a
    build system using recursive make calls, just modify the desired
    sub-build's "make" call so as to create a log file of its own,
    subsequently filtering it by
        $CMD -a fi -l custom_make.log my_target
    The reason it does not work for recursive make is that target names can
    repeat themselves (e.g. often-used standard targets like all, install,
    clean, menuconfig, distclean). To avoid problems, the filter action
    always eliminates sub-make output when creating *.flt from *.log.

EOF
}

clean_up()
{
    rm -rf "$LOG_FILE"
    rm -rf "$FILTER_FILE"
}

# No args -> short usage help
if [ $# -eq 0 ]; then
    usage >&2
    exit 1
fi

# First arg "-?" -> verbose help
if [ "$1" == "-?" ]; then
    help
    exit 0
fi

# Set defaults
DO_LOG=y
DO_FILTER=y
DO_IMAGE=y
unset DO_CLEAN
LOG_FILE=$CMD.log
FILTER_FILE=$CMD.flt

while getopts "a:l:f:i:" opt; do
    case "$opt" in
        a)
            ACTIONS="$OPTARG"
            # Actions explicitly specified -> only use these, reset others
            unset DO_LOG DO_FILTER DO_IMAGE DO_CLEAN
            [ "${ACTIONS##*l*}" ] || DO_LOG=y
            [ "${ACTIONS##*f*}" ] || DO_FILTER=y
            [ "${ACTIONS##*i*}" ] || DO_IMAGE=y
            [ "${ACTIONS##*c*}" ] || DO_CLEAN=y
            [ "${ACTIONS##*a*}" ] || { DO_LOG=y; DO_FILTER=y; DO_IMAGE=y; DO_CLEAN=y; }
            ;;
        l)
            LOG_FILE="$OPTARG"
            ;;
        f)
            FILTER_FILE="$OPTARG"
            ;;
        i)
            IMAGE_FILE=$(echo "$OPTARG"  | sed -r 's#[_/. \t]+#_#g').png
            ;;
        *)
            usage >&2
            exit 1
            ;;
    esac
done
shift $(($OPTIND - 1))

# Special case: clean up is the only action, no target specified
if [ $# -eq 0 -a "$DO_CLEAN" -a ! "$DO_LOG" -a ! "$DO_FILTER" -a ! "$DO_IMAGE" ]; then
    clean_up
    exit 0
fi

# Now there should be 1 or 2 positional parameters left (target, log-target)
if [ $# -ne 1 -a $# -ne 2 ]; then
    usage >&2
    exit 1
fi

TARGET="$1"
LOG_TARGET="$2"
IMAGE_FILE=$(echo "$TARGET"  | sed -r 's#[_/. \t]+#_#g').png

# Step 1: generate log file
if [ "$DO_LOG" ]; then
    echo -n "Creating make log '$LOG_FILE' from log target '$LOG_TARGET' ... "
    LC_ALL=C make -j1 -pnsq $LOG_TARGET > "$LOG_FILE" 2> /dev/null
    echo "done."
fi

# Step 2: filter log file
if [ "$DO_FILTER" ]; then
    echo -n "Filtering make log '$LOG_FILE' into '$FILTER_FILE'... "
    # Filter make log, removing stuff unnecessary for visualisation
    cat "$LOG_FILE" |
        # Remove everything above files section
        sed -r '/^# GNU Make/,/^# Files$/d' |
        # Remove comments
        sed -r '/^#.*/d' |
        # Remove indented lines (recipes)
        sed -r '/^[[:space:]]+.*/d' |
        # Remove everything else not containing a ":" (target definitions)
        sed -r '/^[^:]*$/d' |
        # Remove pipe ('|') characters used in order-only prerequisites,
        # because the Makefile::GraphViz library would mistake the pipe for a
        # target and show it in graphs
        tr -d '|' \
        > "$FILTER_FILE"
    echo "done."
fi

# Step 3: visualise dependency graph as PNG image
if [ "$DO_IMAGE" ]; then
    echo -n "Creating dependency graph image '$IMAGE_FILE' for target '$TARGET' ... "
    perl $(dirname $0)/$CMD.pl "$FILTER_FILE" $TARGET "$IMAGE_FILE" #> /dev/null 2>&1
    #perl $(dirname $0)/$CMD.pl Makefile.makesimple $TARGET "$IMAGE_FILE" #> /dev/null 2>&1
    [ $? -ne 0 ] && echo "FAILED" && exit 1
    echo "done."
fi

# Step 4: clean up log + filter files
if [ "$DO_CLEAN" ]; then
    echo -n "Cleaning up log/filter files ... "
    clean_up > /dev/null 2>&1
    echo "done."
fi

exit 0

Semi off-topic: It is not just that I do not speak Perl, I also do not speak Dot/GraphViz. Having said that, do you happen to know if after I have created a big Dot file once, can I render just parts of the graph (starting from a specific target or regmatched sets of targets) somehow? After a quick search I did not find any options for the GraphViz command line tools permitting any kind of filtering. It would be nice not to have to run make everytime I want to document any of my many, many subtargets.

[kriegaex]

Ah, talking about the tests you mentioned: They fail upon installation via cpanminus all the time, so I have to skip them to install the tool anyway.

[agentzh]

On Thu, Mar 15, 2012 at 2:55 PM, Alexander Kriegisch < reply@reply.github.com

wrote:

$gv = $parser->plot( $ARGV[1], trim_mode => true, init_args => { rankdir => true, }, exclude => [ "^|\$", "/.ed\$" ], end_with => [ "^[.]config\$", "^Config[.]in[.]cache\$", "^dl\$", "^download-toolchain\$", "^glib2-precompiled\$" ] );

Just pass regex objects to the "exclude" and "end_with" options, like

   exclude => [ qr{^\\|\$}, qr{/[.](unpack|configur)ed\$} ],
   end_with => [ qr{^[.]config\$}, qr{^Config[.]in[.]cache\$},

qr{^dl\$}, qr{^download-toolchain\$}, qr{^glib2-precompiled\$} ]

And then in the _find method, we should check if the pattern is a plain string or a regex object:

if (ref $_) {
    return 1 if $elem =~ $_;
}

return 1 if $elem eq $_;

Regards, -agentzh

[agentzh]

On Thu, Mar 15, 2012 at 2:56 PM, Alexander Kriegisch < reply@reply.github.com

wrote:

Ah, talking about the tests you mentioned: They fail upon installation via cpanminus all the time, so I have to skip them to install the tool anyway.

What errors are you getting while running the test suite on your side?

-agentzh

[agentzh]

On Thu, Mar 15, 2012 at 3:06 PM, agentzh agentzh@gmail.com wrote:

And then in the _find method, we should check if the pattern is a plain string or a regex object:

if (ref $_) {
    return 1 if $elem =~ $_;

}

return 1 if $elem eq $_;

I've already committed this patch to the git master branch.

Could you check if it works for you?

Thanks! -agentzh

[kriegaex]

Actually, you forgot to unescape the regexes which formerly were strings, so it rather looks like this:

    exclude => [ qr{^\|$}, qr{/[.](unpack|configur)ed$} ],
    end_with => [ qr{^[.]config$}, qr{^Config[.]in[.]cache$}, qr{^dl$}, qr{^download-toolchain$}, qr{^glib2-precompiled$} ]

This works with the change you suggested (I did it manually, because I have not checked out your repo).

By the way, while you are at it, could you please make sure that the "|" separator between normal and order-only targets is not considered a target, consequently not creating a graph node for it anymore? You would save me the filtering (see scripts above, currently I am filtering it via regex and also in the shell script so as for you to see the two ways it can be done).

As for the failing tests, I will be busy now for a few hours. I need to un-/re-install the module to see the error messages again, I guess. I will let you know ASAP.

[agentzh]

On Thu, Mar 15, 2012 at 3:56 PM, Alexander Kriegisch < reply@reply.github.com

wrote:

Actually, you forgot to unescape the regexes which formerly were strings, so it rather looks like this:

       exclude => [ qr{^\|$}, qr{/[.](unpack|configur)ed$} ],
       end_with => [ qr{^[.]config$}, qr{^Config[.]in[.]cache$}, qr{^dl$},
qr{^download-toolchain$}, qr{^glib2-precompiled$} ]

Right.

This works with the change you suggested (I did it manually, because I have not checked out your repo).

Cool :)

By the way, while you are at it, could you please make sure that the "|" separator between normal and order-only targets is not considered a target, consequently not creating a graph node for it anymore? You would save me the filtering (see scripts above, currently I am filtering it via regex and also in the shell script so as for you to see the two ways it can be done).

Have you tried the makesimple script shipped with the Makefile::Parser project? See http://search.cpan.org/dist/Makefile-Parser/

It's recommended to use makesimple to simplify your Makefiles and then do gvmake on that makefile output :)

Regards, -agentzh

[kriegaex]

I tried to uninstall the module in cpanp and reinstall it, but no more test errors. On two machines the tests only failed upon initial installation (which also happened to be the first CPAN module installations on those machines at all). Any idea how I can reproduce a fresh install?

I am going to try makesimple later. As you can see in my shell script above (makefile name "Makefile.makesimple" in commented line), I experimented with it a while ago, but got interrupted and forgot about it. I will let you know if it helps.

What about the order-only thing ("|" separator)? Will makesimple take care of it or are you planning to do something about it in this module? Would be nice and probably easy for you, knowing your own code.

[agentzh]

On Fri, Mar 16, 2012 at 1:57 AM, Alexander Kriegisch reply@reply.github.com wrote:

I tried to uninstall the module in cpanp and reinstall it, but no more test errors. On two machines the tests only failed upon initial installation (which also happened to be the first CPAN module installations on those machines at all). Any idea how I can reproduce a fresh install?

Installing perl from source?

I am going to try makesimple later. As you can see in my shell script above (makefile name "Makefile.makesimple" in commented line), I experimented with it a while ago, but got interrupted and forgot about it. I will let you know if it helps.

Cool :)

What about the order-only thing ("|" separator)? Will makesimple take care of it or are you planning to do something about it in this module? Would be nice and probably easy for you, knowing your own code.

To be honest, I cannot remember the exact details because this piece of code was written many years ago :P It may help here or may not.

Best, -agentzh

[kriegaex]

I tried to uninstall the module in cpanp and reinstall it, but no more test errors. On two machines the tests only failed upon initial installation (which also happened to be the first CPAN module installations on those machines at all). Any idea how I can reproduce a fresh install?

Installing perl from source?

I guess I will rather not.

I am going to try makesimple later. As you can see in my shell script above (makefile name "Makefile.makesimple" in commented line), I experimented with it a while ago, but got interrupted and forgot about it. I will let you know if it helps.

Cool :)

As I just wrote in the other ticket, it runs for 20 minutes which is way too slow for my purposes.

What about the order-only thing ("|" separator)? Will makesimple take care of it or are you planning to do something about it in this module? Would be nice and probably easy for you, knowing your own code.

To be honest, I cannot remember the exact details because this piece of code was written many years ago :P It may help here or may not.

I just checked: makesimple also creates files with "|" in the output, which is correct. But I guess the right place to filter it would be Makefile::GraphViz anyway because it is only during visualisation that the "|" should not be displayed (at least not as a target node).

Another question. It is OT because it is a GraphViz question, but maybe you know it anyway. I did not find anything in GV's documentation: Is here a way to render nodes for long targets in a way which does not cut the path names? Just as an option. How would I configure Makefile::GraphViz to do that?

[agentzh]

On Fri, Mar 16, 2012 at 5:30 PM, Alexander Kriegisch reply@reply.github.com wrote:

I just checked: makesimple also creates files with "|" in the output, which is correct. But I guess the right place to filter it would be Makefile::GraphViz anyway because it is only during visualisation that the "|" should not be displayed (at least not as a target node).

Right, Makefile::GraphViz should handle this.

Another question. It is OT because it is a GraphViz question, but maybe you know it anyway. I did not find anything in GV's documentation: Is here a way to render nodes for long targets in a way which does not cut the path names? Just as an option. How would I configure Makefile::GraphViz to do that?

Makefile::GraphViz cuts those long target names. I think we should expose a configure option to disable that.

Regards, -agentzh

[kriegaex]

I just checked: makesimple also creates files with "|" in the output, which is correct. But I guess the right place to filter it would be Makefile::GraphViz anyway because it is only during visualisation that the "|" should not be displayed (at least not as a target node).

Right, Makefile::GraphViz should handle this.

Cool :)

Another question. It is OT because it is a GraphViz question, but maybe you know it anyway. I did not find anything in GV's documentation: Is here a way to render nodes for long targets in a way which does not cut the path names? Just as an option. How would I configure Makefile::GraphViz to do that?

Makefile::GraphViz cuts those long target names. I think we should expose a configure option to disable that.

Double cool :)

Another question: I often have graphs for targets like "mc-precompiled" which depend on other targets ".-precompiled". So I use stuff like qr{^glib2-precompiled$} in my script. Because I must edit this for each and every target, I would prefer to use `qr{^.-precompiled$}`, but this also makes the script end with my target. It would be nice if your script would either automatically not end with the target(s) given or if there was another category "no_end_with" or in which to specify exceptions (also optionally as regexes). I would also need an exception list for "exclude", maybe something called "no_exclude".

[agentzh]

On Fri, Mar 16, 2012 at 6:15 PM, Alexander Kriegisch reply@reply.github.com wrote:

Another question: I often have graphs for targets like "mc-precompiled" which depend on other targets ".-precompiled". So I use stuff like qr{^glib2-precompiled$} in my script. Because I must edit this for each and every target, I would prefer to use `qr{^.-precompiled$}`, but this also makes the script end with my target. It would be nice if your script would either automatically not end with the target(s) given or if there was another category "no_end_with" or in which to specify exceptions (also optionally as regexes). I would also need an exception list for "exclude", maybe something called "no_exclude".

These two options make sense. Would you provide patches?

Thanks! -agentzh

[kriegaex]

The following patch contains both changes discussed here (regmatch for _find plus new options "no_exclude" and "no_end_with". I am not sure if the first use of "no_exclude" is correct, though. You better double-check. Please note that I have not updated the help text. I suggest you update it the way you want. You have your own style of writing.

--- /usr/local/share/perl/5.12.4/Makefile/GraphViz.pm   2012-03-16 11:38:12.027600173 +0100
+++ /usr/local/share/perl/5.12.4/Makefile/GraphViz.pm   2012-03-16 11:39:44.543601642 +0100
@@ -68,6 +68,9 @@
 sub _find ($@) {
     my $elem = shift;
     foreach (@_) {
+        if (ref $_) {
+            return 1 if $elem =~ $_;
+        }
         return 1 if $elem eq $_;
     }
     return undef;
@@ -124,11 +127,19 @@
     $val = $opts{end_with};
     my @end_with = ($val and ref $val) ? @$val : ();

+    # process the ``no_end_with'' option:
+    $val = $opts{no_end_with};
+    my @no_end_with = ($val and ref $val) ? @$val : ();
+
     # process the ``exclude'' option:
     $val = $opts{exclude};
     my @exclude = ($val and ref $val) ? @$val : ();

-    return $gv if _find($root_name, @exclude);
+    # process the ``no_exclude'' option:
+    $val = $opts{no_exclude};
+    my @no_exclude = ($val and ref $val) ? @$val : ();
+
+    return $gv if _find($root_name, @exclude) and !_find($root_name, @no_exclude);

     if (!$gv) {
         $gv = GraphViz->new(%init_args);
@@ -152,7 +163,7 @@
         $is_virtual = 1;
     }

-    if (!@roots or _find($root_name, @end_with)) {
+    if (!@roots or _find($root_name, @end_with) and !_find($root_name, @no_end_with)) {
         $gv->add_node(
             $root_name,
             label => $short_name,
@@ -194,7 +205,7 @@
         my @prereqs = $root->prereqs;
         foreach (@prereqs) {
             #warn "$_\n";
-            next if _find($_, @exclude);
+            next if _find($_, @exclude) and !_find($_, @no_exclude);
             $gv->add_edge(
                 $_ => $lower_node,
                 $is_virtual ? (style => 'dashed') : ());

Now I can call Makefile::GraphViz like this:

# Helper script for shell script visualise_make
#
# Author: Alexander Kriegisch (http://scrum-master.de), 2008-02-10

use Makefile::GraphViz;

# Parse makefile
$parser = Makefile::GraphViz->new;
$parser->parse($ARGV[0]);

# Let GraphViz generate an image layout and make sure to exclude "|" as a
# target (will be falsely regarded as one sometimes) and do not display
# dependencies of ".config" (lots of "Config.in" files)
$gv = $parser->plot(
    $ARGV[1],
    trim_mode => true,
    init_args => { rankdir => true,  },
    exclude => [ qr{^\|$}, qr{/[.](unpack|configur)ed$} ],
    end_with => [ qr{^[.]config$}, qr{^Config[.]in[.]cache$}, qr{^dl$}, qr{^download-toolchain$}, qr{^.*-precompiled$} ],
    no_end_with => [ $ARGV[1] ]
);

# Save as PNG bitmap
$gv->as_png($ARGV[2]);

As you can see, I am now using qr{^.*-precompiled$} in "end_with" plus $ARGV[1] in "no_end_with". This does what I want. :)

[kriegaex]

BTW: It would be nice if you would push out a new release to CPAN after you have updated the documentation and tests for those two changes. This way I could use the script out of the box in my project without having to instruct all users to patch Makefile::GraphViz. I do not know how busy you are (probably pretty much), so whenever you get around to doing that, it will be appreciated.

[kriegaex]

P.S.: I forgot about the "|" node issue and about the hard-wired target name shortening. I will be busy for a while, but maybe later I can provide patches for those two things as well. But beware! I still do not speak Perl, the code will probably be horrible.

[kriegaex]

Okay, I just had a minute to fix the "|" issue:

@@ -194,7 +207,7 @@
         my @prereqs = $root->prereqs;
         foreach (@prereqs) {
             #warn "$_\n";
-            next if _find($_, @exclude);
+            next if $_ eq "|" or (_find($_, @exclude) and !_find($_, @no_exclude));
             $gv->add_edge(
                 $_ => $lower_node,
                 $is_virtual ? (style => 'dashed') : ());

[agentzh]

On Fri, Mar 16, 2012 at 6:57 PM, Alexander Kriegisch < reply@reply.github.com

wrote:

BTW: It would be nice if you would push out a new release to CPAN after you have updated the documentation and tests for those two changes. This way I could use the script out of the box in my project without having to instruct all users to patch Makefile::GraphViz. I do not know how busy you are (probably pretty much), so whenever you get around to doing that, it will be appreciated.

Thank you very much! I'll apply all your patches and make a new CPAN release after you confirm everything is straight :)

Thanks! -agentzh

[kriegaex]

Sorry to bother you, but I am just trying to do something like this:

my @NodeTrimCmds = [
    qr{s/.+(.{5}[\\\/].*)$/...$1/o},
    qr{s/\\/\\\\/g}
];

I am getting the error: Use of uninitialized value $1 in regexp compilation at /usr/local/share/perl/5.12.4/Makefile/GraphViz.pm line 46.

I really have no idea about how to use regexes in Perl even though I know quite a lot about regexes in general. Can you please tell me how to put regexes containing a search-replace op into an array without having them precompiled in a way which raises this exception? And how to use them later in a loop? I searched a while, but probably not for the right thing. I found a lot of infor about perlre, but not exactly what I was looking for. And I do not want to spend three days learning everything there is to learn about perlre.

[kriegaex]

Well, I thought I asked a simple question, but if I look at this, it seems to be a bit more complicated: http://stackoverflow.com/questions/125171/passing-a-regex-substitution-as-a-variable-in-perl

I guess to modify the behaviour of _trim_cmd and _trim_path we can just as well have the user pass a complete function instead of an array or quoted or cut-in-two replacement expressions. Working on it later, I am needed elsewhere now.

[kriegaex]

Okay, here is yet another iteration of my code extension. Now it is also possible to set node_trim_fct and cmd_trim_fct if a user wants to take care of trimming target paths (node) and commands (cmd) by herself. It took me a while to find out how code references work in Perl. I hope I have done it right. At least my test case seems to work.

+++ /usr/local/share/perl/5.12.4/Makefile/GraphViz.pm   2012-03-17 09:30:02.673576095 +0100
+++ /usr/local/share/perl/5.12.4/Makefile/GraphViz.pm   2012-03-17 09:30:02.673576095 +0100
@@ -68,6 +68,9 @@
 sub _find ($@) {
     my $elem = shift;
     foreach (@_) {
+        if (ref $_) {
+            return 1 if $elem =~ $_;
+        }
         return 1 if $elem eq $_;
     }
     return undef;
@@ -120,15 +123,31 @@
     my $trim_mode = $opts{trim_mode};
     #warn "TRIM MODE: $trim_mode\n";

+    # process the ``node_trim_fct'' option:
+    $val = $opts{node_trim_fct};
+    my $node_trim_fct = ($val and ref $val) ? $val : \&_trim_path;
+
+    # process the ``cmd_trim_fct'' option:
+    my $cmd_trim_fct = ($val and ref $val) ? $val : \&_trim_cmd;
+    $val = $opts{cmd_trim_fct};
+
     # process the ``end_with'' option:
     $val = $opts{end_with};
     my @end_with = ($val and ref $val) ? @$val : ();

+    # process the ``no_end_with'' option:
+    $val = $opts{no_end_with};
+    my @no_end_with = ($val and ref $val) ? @$val : ();
+
     # process the ``exclude'' option:
     $val = $opts{exclude};
     my @exclude = ($val and ref $val) ? @$val : ();

-    return $gv if _find($root_name, @exclude);
+    # process the ``no_exclude'' option:
+    $val = $opts{no_exclude};
+    my @no_exclude = ($val and ref $val) ? @$val : ();
+
+    return $gv if _find($root_name, @exclude) and !_find($root_name, @no_exclude);

     if (!$gv) {
         $gv = GraphViz->new(%init_args);
@@ -145,17 +164,17 @@
     my @roots = ($root_name and ref $root_name) ?
         $root_name : ($self->target($root_name));

-    my $short_name = _trim_path($root_name);
+    my $short_name = $node_trim_fct->($root_name);
     if ($normal_nodes{$root_name}) {
         $is_virtual = 0;
     } elsif ($vir_nodes{$root_name} or @roots and !$roots[0]->commands) {
         $is_virtual = 1;
     }

-    if (!@roots or _find($root_name, @end_with)) {
+    if (!@roots or _find($root_name, @end_with) and !_find($root_name, @no_end_with)) {
         $gv->add_node(
             $root_name,
-            label => $short_name,
+            label => "[" . $short_name . "]",
             $is_virtual ? %vir_node_style : ()
         );
         return $gv;
@@ -181,7 +200,7 @@
         my @cmds = $root->commands;
         if (!$trim_mode and @cmds) {
             $lower_node = _gen_id();
-            my $cmds = join("\n", map { _trim_cmd($_); } @cmds);
+            my $cmds = join("\n", map { $cmd_trim_fct->($_); } @cmds);
             $gv->add_node($lower_node, label => $cmds, %cmd_style);
             $gv->add_edge(
                 $lower_node => $root_name,
@@ -194,7 +213,7 @@
         my @prereqs = $root->prereqs;
         foreach (@prereqs) {
             #warn "$_\n";
-            next if _find($_, @exclude);
+            next if $_ eq "|" or (_find($_, @exclude) and !_find($_, @no_exclude));
             $gv->add_edge(
                 $_ => $lower_node,
                 $is_virtual ? (style => 'dashed') : ());

I was also thinking about whether it might be a good idea to have a special node style for end_with nodes/targets to visualise that the node is actually a subtree, not a leave. But maybe this is not so important because exclude nodes are also (by definition) not visible and there is not indicator that they exist at all.

Update: I decided to print labels for end_with nodes like this if label equals "foobar": [foobar]. I do not know if you like this, but I think it is simple, yet clear, if documented properly.

[agentzh]

On Sat, Mar 17, 2012 at 3:07 PM, Alexander Kriegisch reply@reply.github.com wrote:

Okay, here is yet another iteration of my code extension.

Thank you for your contribution! I'll find some time to merge your patches :)

Thanks! -agentzh

[kriegaex]

To do for documentation update:

• Regex allowed for exclude and end_with, also mixed with simple strings
• New options no_exclude and no_end_with: arrays of exceptions (string, regex) for exclude and end_with
• New options node_trim_fct and cmd_trim_fct: optional subroutines to replace the normal length trimming behaviour
• Labels for end_with nodes are now surrounded by square brackets: [foobar]
• Bugfix: order-only prerequisite separator "|" is no longer considered a target and not shown in the graph as such anymore

All new/changed features also need code examples.

Update: end_with nodes printed as [ foobar ] (see above).

[agentzh]

On Sat, Mar 17, 2012 at 3:18 PM, Alexander Kriegisch reply@reply.github.com wrote:

To do for documentation update:

[...]

All new/changed features also need code examples.

Do you mind providing some POD documentation patches for these? ;)

Best, -agentzh

[kriegaex]

1. What is POD documentation? That inline stuff you have in your code?
2. By the way, I am just trying to change the layout for code nodes. I guess the ellipse shape is not good for code, so I changed it to a box style with light gray background. I also tried to make it left-justified because code usually is not centered. A few questions about that:
   • Other than adding "\\l" in the join command instead of "\n" and once more at the end of the joined string, i.e. once per line, is there a better way to declare left-justified layout for %CmdStyle globally?
   • Is there a way to select a mono-spaced font for the code shape?

3. Until lately, I have always preprocessed my makefile, removing all recipes (indented code) or replacing it by a dummy command like "@echo", so as not to run into problems with your script. I also do not intend to use trim_mode => 0 very often, but at the moment I am trying to do so, so now I do not remove the recipes anymore. But now I am getting errors like these:

   Bareword found where operator expected at (eval 557) line 1, near "s/^(\S*)(\S*)$/"${1}$(NCURSES_TARGET_DIR)/lib"
   Bareword found where operator expected at (eval 604) line 1, near "s/^(\S*)(\S*)$/"${1}$(NTFS_DEST_DIR)/usr"
   String found where operator expected at (eval 604) line 1, at end of line
   (Missing semicolon on previous line?)
   (...)
   I dunno how to do with it: sane-backends-dirclean--int:
   (...)

Basically there are three types of errors:

• Bareword found where operator expected
• String found where operator expected
• I dunno how to do with it:

[kriegaex]

Okay, I found out why the error messages were appearing:

• "Bareword/string found" happened because in make recipes there are "$" characters which are interpreted as meta characters when you evaluate your regex stuff in Parser.pm. I worked around it in my shell script by replacing each "$" by "$$". Probably it would be better to fix this in one of your modules. I did not find the place where to do this right away.
• "I dunno how..." was caused by some places in my makefiles where reciped end with "\" but there is no line continuation. Probably your parser should tolerate this, just like GNU make, if the following line is not indented (i.e. end of recipe). BTW, I replaced "dunno how to do with X" by "dunno what to do with X". The "how" is wrong there.

Can you tell me where to fix these two issues?

[kriegaex]

Okay, here is the next version with note-style, left-justified code blocks (not nicely done, as I said, so if you know a better way, let me know), a spelling fix in Parser.pm and still no documentation update. Sorry for my inflationary posting, but I want you to know what I am doing and how I am making progress and/or finding problems.

+++ /usr/local/share/perl/5.12.4/Makefile/GraphViz.pm   2012-03-17 10:37:54.865679438 +0100
+++ /usr/local/share/perl/5.12.4/Makefile/GraphViz.pm   2012-03-17 10:37:54.865679438 +0100
@@ -33,9 +33,9 @@

 my %CmdStyle =
 (
-    shape => 'ellipse',
+    shape => 'note',
     style => 'filled',
-    fillcolor => '#c7f77c',
+    fillcolor => '#dddddd',
 );

 my %InitArgs = (
@@ -68,6 +68,9 @@
 sub _find ($@) {
     my $elem = shift;
     foreach (@_) {
+        if (ref $_) {
+            return 1 if $elem =~ $_;
+        }
         return 1 if $elem eq $_;
     }
     return undef;
@@ -120,15 +123,31 @@
     my $trim_mode = $opts{trim_mode};
     #warn "TRIM MODE: $trim_mode\n";

+    # process the ``node_trim_fct'' option:
+    $val = $opts{node_trim_fct};
+    my $node_trim_fct = ($val and ref $val) ? $val : \&_trim_path;
+
+    # process the ``cmd_trim_fct'' option:
+    $val = $opts{cmd_trim_fct};
+    my $cmd_trim_fct = ($val and ref $val) ? $val : \&_trim_cmd;
+
     # process the ``end_with'' option:
     $val = $opts{end_with};
     my @end_with = ($val and ref $val) ? @$val : ();

+    # process the ``no_end_with'' option:
+    $val = $opts{no_end_with};
+    my @no_end_with = ($val and ref $val) ? @$val : ();
+
     # process the ``exclude'' option:
     $val = $opts{exclude};
     my @exclude = ($val and ref $val) ? @$val : ();

-    return $gv if _find($root_name, @exclude);
+    # process the ``no_exclude'' option:
+    $val = $opts{no_exclude};
+    my @no_exclude = ($val and ref $val) ? @$val : ();
+
+    return $gv if _find($root_name, @exclude) and !_find($root_name, @no_exclude);

     if (!$gv) {
         $gv = GraphViz->new(%init_args);
@@ -145,17 +164,17 @@
     my @roots = ($root_name and ref $root_name) ?
         $root_name : ($self->target($root_name));

-    my $short_name = _trim_path($root_name);
+    my $short_name = $node_trim_fct->($root_name);
     if ($normal_nodes{$root_name}) {
         $is_virtual = 0;
     } elsif ($vir_nodes{$root_name} or @roots and !$roots[0]->commands) {
         $is_virtual = 1;
     }

-    if (!@roots or _find($root_name, @end_with)) {
+    if (!@roots or _find($root_name, @end_with) and !_find($root_name, @no_end_with)) {
         $gv->add_node(
             $root_name,
-            label => $short_name,
+            label => "[" . $short_name . "]",
             $is_virtual ? %vir_node_style : ()
         );
         return $gv;
@@ -181,8 +200,8 @@
         my @cmds = $root->commands;
         if (!$trim_mode and @cmds) {
             $lower_node = _gen_id();
-            my $cmds = join("\n", map { _trim_cmd($_); } @cmds);
-            $gv->add_node($lower_node, label => $cmds, %cmd_style);
+            my $cmds = join("\\l", map { $cmd_trim_fct->($_); } @cmds);
+            $gv->add_node($lower_node, label => $cmds . "\\l", %cmd_style);
             $gv->add_edge(
                 $lower_node => $root_name,
                 $is_virtual ? (style => 'dashed') : ()
@@ -194,7 +213,7 @@
         my @prereqs = $root->prereqs;
         foreach (@prereqs) {
             #warn "$_\n";
-            next if _find($_, @exclude);
+            next if $_ eq "|" or (_find($_, @exclude) and !_find($_, @no_exclude));
             $gv->add_edge(
                 $_ => $lower_node,
                 $is_virtual ? (style => 'dashed') : ());
+++ /usr/local/share/perl/5.12.4/Makefile/Parser.pm 2012-03-17 12:36:47.337857384 +0100
+++ /usr/local/share/perl/5.12.4/Makefile/Parser.pm 2012-03-17 12:36:47.337857384 +0100
@@ -201,7 +201,7 @@
             $Error = "syntax error: line $.: $_\n";
             return undef;
         } else {
-            warn "I dunno how to do with it: $_\n";
+            warn "I dunno what to do with it: $_\n";
         }
     }
     $self->{_tars} = \%tars;

[kriegaex]

The next new idea: Would you like a vertically separated record shape for trim_mode => 0 instead of a separate node? I think it would be more intuitive. See http://www.graphviz.org/doc/info/shapes.html#record for more info about record shapes. I imagine the target name in yellow and centered on top and below a separator the (optional) code in gray and left-justified, possibly in a monospace font if I ever find out how to do that. All in all it would look like a box with two vertical compartments.

[kriegaex]

Now that you made me a committer, I guess we can safely close this issue. Our discussion went way beyong the original inquiry anyway. I have pushed some changes related to our discussion upstream, so the ticket is closed.