-
Notifications
You must be signed in to change notification settings - Fork 132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use NQP prefix for finding profiler template #608
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already generate nqp-config.nqp
where NQP prefix
is specified. It should work, to my understanding. Besides, nqp_home
from config is the key to be used, I think.
@@ -93,6 +93,14 @@ sub configure_misc { | |||
|
|||
$config->{moar_stage0} = $self->nfp( "src/vm/moar/stage0", no_quote => 1 ); | |||
$config->{jvm_stage0} = $self->nfp( "src/vm/jvm/stage0", no_quote => 1 ); | |||
|
|||
# TODO: Use the template infrastructure from nqp-configure? | |||
open my $configfile, '>', 'src/core/Config.nqp'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We must not autogenerate anything in src. It actually belongs to gen/
.
It seems to me that the correct code in my $temppath := nqp::getcomp("nqp").config()<libdir> ~ '/profiler/template.html'; |
@vrurg I gave that a try. It fails with: X::AdHoc: Cannot call method 'config' on a null object. Not really sure where to continue from here. I tried a few things but it seems the nqp compiler is not always available. |
49a878d
to
a7806d5
Compare
Ok, I see. I didn't know that rakudo uses profiling code from nqp. nqp::getcomp("Raku").config<nqp-home> ~ 'lib/profiler/template.html' So, the problem would be to know which compiler to use. @lizmat do you have an idea? |
Try both, and see which one yields something to work with? |
@lizmat the first thought. But what if somebody writes another nqp-based compiler? Actually, as I look into it, the place where template.html gets installed is incorrect because the file is moar-backend specific and as such should be installed under some kind of backend subdir. |
So far, it looks like the proper solution would be to add compiler parameter to Then, there is minor discrepancy to be taken care of somehow. NQP config has |
If |
No, it is purely our implementation detail thing. It is only affected by Configure.pl command line. If there is Changing it to |
This seems overly complicated. When configuring NQP, we already know where it is going to be installed into, so we should be able to just get the path in there. What about making hll-config, which is the function generated by gen-version.pl, part of the core setting? Then we can just call that and extract nqp-home or even libdir. |
Nevermind, this has to be done because nqp-home can be overridden with the NQP_HOME environment variable when invoking Rakudo. I’ll see if passing the compiler config around works. |
And if the environment variable is called NQP_HOME, I think it should be called "nqp_home" internally as well. |
@lizmat I disagree. By this logic we should use uppercase too. As well as Snake case would make it inconsistent with other keys in the config. It's even more confusing, than the difference from the environment. Whereas if one already knows what's the rule of thomb he'd expect the key to be kebab-lower-cased. |
Fixes #567.
This fixes the problem but I am not sure what would be the best way/place to generate the file with the nqpconfig subroutine definition in it. Also missing .gitignore entry.