Skip to content
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

Compilation server cause minject to fail #4892

Closed
kevinresol opened this issue Mar 3, 2016 · 16 comments
Closed

Compilation server cause minject to fail #4892

kevinresol opened this issue Mar 3, 2016 · 16 comments

Comments

@kevinresol
Copy link
Contributor

When built with compilation server, injected variables become null.

Edit: the first build with compilation server may not fail, try modify the source and compile again

import minject.Injector;

class Main
{

    static function main()
    {
        var injector = new Injector();
        injector.map(String).toValue('test');
        var test = injector.instantiate(Test);
        trace(test.s); // null, if compiled by compilation server. 'test' otherwise.
    }
}

class Test
{
    @inject
    public var s:String;
    public function new(){}
}

hxml:

--connect 6112
-lib minject
-main Main
-x bin/test.n
@Simn
Copy link
Member

Simn commented Mar 3, 2016

I can't even compile that, "src/Main.hx:10: characters 8-20 : minject.Injector has no field map".

@kevinresol
Copy link
Contributor Author

requires minject version 2,0,0-rc,1

@Simn
Copy link
Member

Simn commented Mar 3, 2016

Eh okay, why does haxelib not install that automatically?

Anyway, I can reproduce the problem.

@kevinresol
Copy link
Contributor Author

i think haxelib ignores non-release versions? (i.e. "-rc.1")

@Simn
Copy link
Member

Simn commented Mar 3, 2016

Is this a regression?

@Simn
Copy link
Member

Simn commented Mar 3, 2016

Just tested with 3.2.1, same problem. This may or may not be about the compilation server keeping the static state of variables.

I've set the milestone to 3.3 but I can't promise that I'll be looking into it for that. Maybe Nicolas can immediately tell us why this happens, otherwise someone is gonna have to dig into minject.

@kevinresol
Copy link
Contributor Author

minject 2.0 is largely macro-based. And from haxe manual:

The compilation server can have some side effects on macro execution.

Not sure what that means indeed.

@ncannasse
Copy link
Member

@Simn try to disable the macro context cache if you want to check if it's statics related.

@Simn
Copy link
Member

Simn commented Mar 4, 2016

How does one do that?

@ncannasse
Copy link
Member

Check in typer.ml, we have a macro context ref. You need to reset it to None before starting a new fresh compilation in compilation server loop (which is in main.ml)

@Simn
Copy link
Member

Simn commented Mar 16, 2016

I can't figure out how to do that, all I get are stack overflows and or lots of "Cached 0 modules" output. I'll set this to 3.4 because it's not a regression.

@Simn Simn modified the milestones: 3.4, 3.3.0-rc1 Mar 16, 2016
@kevinresol
Copy link
Contributor Author

This is frustrating, several ufront users has been trapped by this bug...
@dpeek, may be you can give us 2 cents about what is happening inside minject which might cause this problem?

@kevinresol
Copy link
Contributor Author

kevinresol commented Jun 3, 2016

I revisited this issue today, and I got:

/haxelib/minject/2,0,0-rc,1/src/lib/minject/point/PropertyInjectionPoint.hx:33: characters 2-46 : Local variable tmp used without being initialized

on a second compilation (with compilation server).
[Note that the original report showed that compilations are fine, but the code failed at runtime]

But there isn't a tmp variable anywhere in PropertyInjectionPoint.hx so I reckon something is happening inside the compiler.

Edit: Hm... sorry for the confusion, this new error happens in another rather complex code base. The original reported behavior stayed the same. I will try to isolate it.

@kevinresol
Copy link
Contributor Author

kevinresol commented Jun 3, 2016

But there isn't a tmp variable anywhere in PropertyInjectionPoint.hx so I reckon something is happening inside the compiler.

Ok, I am building for js and this error comes from Reflect.setProperty

Edit: I believe this one is another issue. Opened a new issue at #5320

@kevinresol
Copy link
Contributor Author

kevinresol commented Jun 22, 2016

You may already know this, but this may serve for record purpose:

If compile to js and diff the two outputs:

616c616
< Test.__meta__ = { obj : { rtti : [["s","String",""]]}};
---
> Test.__meta__ = { fields : { s : { inject : null}}};

The meta with rtti should be the legitimate one.

So by investigating minject's source code, I found that the Injector's build macro uses Context.onGenerate to register a callback to process all the types. But I guess that because Injector has not been modified so it's build macro is not run again in the second compilation, causing the onGenerate not to run and thus the modified file doesn't get processed.

I don't know what to do with this. Maybe the macro guy can give me some hints? @back2dos

@kevinresol
Copy link
Contributor Author

Well, turns out that this is trivial.
Just move the macro from a build macro to a --macro fixed it.

Since this should be fixed in minject, I am closing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants