test -fuck hides HDRRULE's bug 🐞

[paolopas]

Forgive me for the name of the issue but it will soon become clear, another heading could have been "how not to test the build system".

I was looking for an example of HDRRULE and the only thing I found was test/core_dependencies.py. I figured out how it worked, but when I tried to extract a Jamfile from the test, I only got errors.
And if you want to experiment for yourself, just use the following Jamroot (which is a simplified version)

DEPENDS all : c ;

HDRSCAN on c = (.*) ;
HDRRULE on c = hdrrule ;

rule hdrrule
{
    ECHO $(<) $(>) ;
}

write anything in a "c" file and then

# echo hello > c
# b2
(no frame):ERROR: rule "hdrrule" unknown in root module.

But how is it possible then that the test works?

Well, in short, here's what the test does (you can do it too):

# b2 -fJamroot
c hello

...found 2 targets...

Just use the -f option, and the script magically works!

Test code in core_dependencies.py use the interpreter with the -f- argument to pass a script to the process's standard input, but it is not uncommon to use the -f option with any file argument in tests.
This issue heading is now clear, but what happens is not.
For that, look at the implementation of the guarded_main(int argc, char * argv[]) function in jam.cpp:

    ...
    /* Load build system. */
    if (args_data.files_to_parse.empty())
    {
            FRAME frame;
            frame_init(&frame);
            /* Initialize the native API bindings. */
            b2::jam::bind_jam(&frame);

            /* Launch the bootstrap to load up the build system. */
            status = b2::startup::bootstrap(&frame) ? 0 : 13;
    }

    /* Parse ruleset. */
    if (!args_data.files_to_parse.empty())
    {
            FRAME frame;
            frame_init(&frame);
            for (const std::string & s : args_data.files_to_parse)
            {
                    b2::value_ref filename(s);
                    parse_file(filename, &frame);
            }
    }

And now it should be clear why the test using -f is not running the build system!
Tests often give a false sense of security, but insecurity is guaranteed when the context in which a test is executed is very different from the context of real use.

Now there are 2 things left to do:

1. Fix this HDRRULE bug
2. Use the -f option with caution in testing

[grafikrobot]

Note sure I understand the problems. The -f option is definitely something to use with caution. As, yes, it's supposed to bypass most loading of the build system in favor of directly loading the given file. I.e. the implementation looks correct to me. So the problem appears to be not handling stdin indicated by the "-" file name. That was probably my fault when rewriting jam.cpp. :-(

[paolopas]

No René the problem is not the stdin (if you look at my code snippet I wrote b2 -fJamroot, not -f-), that's works fine, neither the guarded_main implementation is wrong. And to be clear, I have nothing against the -f option, in fact it's great that the interpreter has several operating modes. The problem is that HDRRULE has a bug that prevent it to work with the build system and the test doesn't show it.

Now let's forget about the -f option and focus on the HDRRULE problem.
I recently found that is a known issue (as stated in Built-in Variables, even if the oldest documentation is also the least reliable) a serious bug IMO, but anyway there is no reason to abandon HDRRULE.

In a nutshell what happens is that when the jamfile is loaded directly with the parse_file() (-f option) the parsing happens in the root module (aka global), but if the build system loads it (no -f option given) a separate module is created for each project in project.jam (actually, there are 2 modules created for each project), to simplify, let's say that the parsing happens in a module called something like Jamfile</some/parent/dir> that is different from the global one.

When the headers function in headers.cpp is called (by make0 in make.cpp) to process the HDRRULE, it is passed only the target, and to process it, creates a new frame and then looks for the hdrrule rule in the root module (which is the one associated with the new frame). But the rule is elsewhere.

In practice, the information about the module associated with the jamfile in which the target is defined is lost. And when the rule is searched for, and not found, a fake new empty (no procedure) rule is made by bindurule in rules.cpp:

rule_ptr bindrule( b2::value_ptr rulename, module_ptr m )
{
    rule_ptr result = find_rule( rulename, m );
    /* We have only one caller, 'evaluate_rule', which will complain about
     * calling an undefined rule. We could issue the error here, but we do not
     * have the necessary information, such as frame.
     */
    if ( !result )
        result = enter_rule( rulename, m );
    return result;
}

which then causes the error

(no frame):ERROR: rule "hdrrule" unknown in root module.

I've done some tests by passing the jam file in addition to the usual

HDRSCAN on ...
HDRRULE on ...

also

__MODULE__ on ... = $(__name__) ;

and this would allow me to solve the problem, obviously with the necessary changes in the headers function. Basically, I'm adding the module name to the target's SETTINGS to pass it to the headers later.

How can we get the same effect without having to manually set the variable on the target?

Well, there is a way. Modify the code that creates a target in bindtarget (from rules.cpp) during the initial jamfiles scan so that it uses the name of the loading module passed to the load (from mod_jam_modules.cpp.) During the scan the process uses only the main thread and the interface for exchanging the module name does not present any particular problems.

On the newly created target you can install a synthetic variable using addsettings (from rules.cpp); of course you only need to do this on targets that are in user jamfiles.

So I can finally say that I have a solution to the HDRRULE bug.

[grafikrobot]

It should not be needed to pass anything other than the rule name in HDRRULE. As the rule name itself needs to be qualified with the module like scanner.hdrrule (that's from scanner.jam). That name should be an alias for the module scoped rule that is shadowed to the root module. And the way the -f parsing work it should be putting declarations into the root module (as you said).

That leads me to think that what is happening is that the core_dependencies test is defining hdrrule in the core-depdency-helpers modules (as that's the file it creates). But as it doesn't qualify the HDRRULE on .. = hdrrule ; it doesn't resolve to the fully qualified rule.

Now the problem may actually be in the include rule. As, IIRC, it doesn't create a module for a parsed file. It just starts parsing as if it was copy-pasted. But maybe we changed that? The one that is supposed to create a module for the included file is the import rule.

Hope that makes sense?

[grafikrobot]

PS. I'll experiment in the weekend to see if I can find out anything more.

[grafikrobot]

PPS. After some sleep..

I think in your example above instead of HDRRULE on c = hdrrule ; you should have HDRRULE on c = $(__name__).hdrrule ;. For the case where it's in a Jamroot project module. And if you need it to work in both modes it would need to be: if $(__name__) { HDRRULE on c = $(__name__).hdrrule ; } else { HDRRULE on c = hdrrule ; }. Not particularly intuitive. But this is rather low level functionality we are dealing with.

[paolopas]

Function load in mod_jam_modules does the include from a target module called <module@>Jamroot (not root module for obvious reasons of name clashing) in which it then defines the alias of the rule with the name you suggested; the load also creates a target with the same name to set the BINDRULE and SEARCH variables on (and this is probably the rationale for the aliases in the target module.)

But the problem is that the evaluate_rule will always be called from the root module (not being able to know what the module name was during parsing).

If you try your solution here is the result

(no frame):ERROR: rule "Jamfile</some/parent/dir>.hdrrule" unknown in root module.

Personally, I find the preamble (no frame): annoying (and will remove it if I can).

Of course, if the rule alias were in the root module, it might work. You'd need to modify the load function to verify this.
Maybe we'll eventually find out that the bug is also in the load function (not only in the HDRRULE machinery), but I don't think so.

Surely HDRRULE should work as it is now described, with no conditions in its name, and the problem of getting the module name into the headers remains.

So I still think there's a big bug out there (and I'm surprised I still don't see the red label on this issue.) But as I wrote to you, I already have an elegant solution that works.

[paolopas]

A convenient place in the code where you can get module information, without disturbing the load in mod_jam_modules.cpp, is in the function_run in function.cpp, specifically in the INSTR_SET_ON case.
This involves intercepting the execution of the compiled script in the part that sets the HDRRULE/HDRSCAN variables on the target, simultaneously decorating it with the module information.

This solution is even cleaner and more efficient than the one involving load, which requires a stack to keep track of reentrant calls. Furthermore, no modification of the bindtarget is required. Of course, I've already written it, and it works perfectly.

[paolopas]

Note

If you see a 🐞 in the issue heading it's because my browser doesn't support label management. René please put the label you think is appropriate.