1. Einstein Toolkit Avatar Einstein Toolkit
  2. Einstein Toolkit
  3. tickets
  4. Issues

piraha breaks ThornDoc building

Issue #2013 closed
Roland Haas created an issue

Doing make ThornDocHTML I get an error message in eg in./repos/flesh/doc/HTML/ThornDoc/CactusConnect/Socket/LOG_SCHEDLATEX_MSGS of

Undefined subroutine &piraha::parse_peg_file called at /home/rhaas/postdoc/gr/cactus/ET_trunk/lib/sbin/ScheduleParser.pl line 83.

this seems to be caused by the Piraha perl module not being loaded in those files.

Keyword:

Comments (17)

  2. Roland Haas reporter
    • removed comment

    This fixes it:

    diff --git a/lib/sbin/InterLatex.pl b/lib/sbin/InterLatex.pl
index e4b0a460..898b3ed4 100755
--- a/lib/sbin/InterLatex.pl
+++ b/lib/sbin/InterLatex.pl
@@ -56,6 +56,7 @@ my $sbin_dir = "${cctk_home}lib/sbin";
 require "$sbin_dir/ThornUtils.pm";

 # for use of create_parametere_database
+require "$sbin_dir/Piraha.pm";
 require "$sbin_dir/interface_parser.pl";
 require "$sbin_dir/CSTUtils.pl";

diff --git a/lib/sbin/ParamLatex.pl b/lib/sbin/ParamLatex.pl
index 06f1b011..b1810fdd 100644
--- a/lib/sbin/ParamLatex.pl
+++ b/lib/sbin/ParamLatex.pl
@@ -62,6 +62,7 @@ my $sbin_dir = "${cctk_home}lib/sbin";
 require "$sbin_dir/ThornUtils.pm";

 # for use of create_parametere_database
+require "$sbin_dir/Piraha.pm";
 require "$sbin_dir/parameter_parser.pl";
 require "$sbin_dir/CSTUtils.pl";

diff --git a/lib/sbin/SchedLatex.pl b/lib/sbin/SchedLatex.pl
index f8b52700..4e1db1f9 100755
--- a/lib/sbin/SchedLatex.pl
+++ b/lib/sbin/SchedLatex.pl
@@ -53,6 +53,7 @@ if ($h || $help) {
 $cctk_home .= '/' if (($cctk_home !~ /\/$/) && defined $cctk_home);

 my $sbin_dir = "${cctk_home}lib/sbin";
+require "$sbin_dir/Piraha.pm";
 require "$sbin_dir/ScheduleParser.pl";
 require "$sbin_dir/CSTUtils.pl";

diff --git a/lib/sbin/parameter_parser.pl b/lib/sbin/parameter_parser.pl
index cb39d013..3cdbf9fc 100644
--- a/lib/sbin/parameter_parser.pl
+++ b/lib/sbin/parameter_parser.pl
@@ -16,6 +16,13 @@ use strict;
 use Carp;
 my $ccl_file;

+sub trim_quotes
+{
+  my $str = shift;
+  $str =~ s/^(["'])(.*)\1$/$2/s;
+  return $str;
+}
+
 #/*@@
 #  @routine    create_parameter_database
 #  @date       Wed Sep 16 11:45:18 1998

    however it does not seem to be the "right" solution. Shouldn't require piraha be in the actual parsers? Similarly parameter parser uses a function (trim_spaces) that was originally only defined in interface parser and which has no GRdoc text whatsoever.

  5. Steven R. Brandt
    • removed comment

    So I think including Piraha the way you have it is correct, if you think of Piraha as being "like" CSTUtils, a utility that all the parsers need.

    The trim_quotes() function should probabaly move to CSTUtils (and get GRDoc comments). Do you agree?

  6. Roland Haas reporter
    • removed comment

    Hello Steve,

    well I think I am not including it properly. Right now in both CST and the LaTeX parsers, the parsers require that their caller already provides piraha for them. This seems wrong since the caller usually does not even user piraha and also violates encapsulation of internals since the caller should not know how the parsers internally parse the files.

    Moving trim_quotes() into a common library package is of course fine with me, as long as all packages that use trim_quotes() actually require the library package. I should also receive a GRdoc header if possible.

  7. Steven R. Brandt
    • removed comment

    Well, I could include Piraha in each of the parsers. It would have to look like this:

    and rely on the calling routine to set $main::sbin_dir correctly. Alternatively, we could put "use lib $sbin_dir" in the CST and "use Piraha;" in each of the parsers. Or we could include Piraha in the CSTUtils? Which do you prefer? I put Piraha in the CST because it felt more like the style of what had been done to me.
  8. Roland Haas reporter
    • removed comment

    My feeling would be that the modules (ie the parsers) should encapsulate information that CST or their other callers do not need to know about (such as whether Piraha or another parser is used) and that as little information as possible should be passed from callers to the modules using global variables. So with that in mind I would move both the require "$main::sbin_dir/Piraha.pm"; and the code that computes sbin_dir to the parser packages or (alternatively and probably better) have main adjust the perl search path to include the Cactus perl module directory where Piraha lives.

  9. Steven R. Brandt
    • removed comment

    OK, so how about we put "use lib $sbin_dir;" in CST, and "use Piraha;" in the parsers?

  11. Steven R. Brandt
    • removed comment

    Yes. I thought this was something you were asking me to do. :) Does that mean we should close this ticket?

  12. Roland Haas reporter
    • removed comment

    If you review the pull request and give me a "Please apply" I will be happy to do so. Note that while I did indeed read the Camel book on perl this was many years in the past (say in 2001 or so) and by now most of my perl knowledge is obtained from the Internet so I think I got this all working and the methods seem to be ok, if there is a simpler (rather than just "more than one") way to achieve that I'd be happy to implement that as well. Naturally if this is "fine" as is, then I am also very happy not to spend more time on this than needed.

  15. Roland Haas reporter
    • changed status to open
    • removed comment

    Nope this still happens. In spite of what I claimed, 5dd70e1e does not fix this (in fact checking out 5dd70e1e shows that even that commit does not work). This is ultimately an issue about executing perl code at compile time to provide the "use lib" statement with a path.

  18. Log in to comment
Assignee
Type
bug
Priority
minor
Status
closed
Component
Cactus
Milestone
–
Version
development version
Votes
0
Watchers
0
Jira Software: the preferred issue tracker for Bitbucket. Join the team!