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

[tests] Compilation server vs defineModule/defineType #11159

Merged
merged 8 commits into from
Jan 3, 2024

Conversation

kLabz
Copy link
Contributor

@kLabz kLabz commented Apr 18, 2023

Goal

  • Find a proper solution for a compilation server safe way of adding types with defineModule/defineType
  • Have a test in Haxe repo to make sure we don't break that with compilation server changes
  • Add a cookbook entry showing how to implement it
  • Point library authors to that cookbook entry if they have "x is redefined from x" / missing types in generated code issues

Provided implementation

Feel free to suggest changes to the implementation I provided, as I plan to put in on the cookbook.
I did not manage to break it, but if you find a way please tell me and I'll update the tests and try to address the issue.

I also looked into implementing CompilationServer.isAlive(t:Type):Bool but it wasn't working as well as the custom implementation I'm using here in the end...

Note: name mangling is very minimal there and won't be enough in some (many) cases; will add something about it in cookbook to tell people to use something that will generate truly unique names for their use case (any suggestion on a particular haxe.macro.Type name mangling implementation?)

@kLabz kLabz marked this pull request as draft April 18, 2023 08:33
@skial skial mentioned this pull request Apr 18, 2023
1 task
@kLabz
Copy link
Contributor Author

kLabz commented Jan 2, 2024

... I think current failures might actually be false negatives.

I managed to reproduce a few failures locally, and this is the debug data:

src/cases/CsSafeTypeBuilding.hx:45: Fail: doesn't contain "[runtime] Hello from Foo__Bar__Bar"
src/cases/CsSafeTypeBuilding.hx:46: Fail: doesn't contain "[runtime] Hello from Foo__Baz__Baz"
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: Building Baz.
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: Building Main.
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: Building Bar.
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: [Bar] Generating type for Foo__Bar__Bar.
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: [Main] Generating type for Foo__Main__Main.
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: [Baz] Generating type for Foo__Baz__Baz.
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: [Baz] Reusing previously generated type for Foo__Bar__Bar.
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: [runtime] Hello from Bar
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: 
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: [runtime] Hello from Baz
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: 
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: [runtime] Hello from Foo__Bar__Bar[runtime] Hello from Foo__Baz__Baz
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: [runtime] Hello from Foo__Main__Main
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: 
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: [runtime] Hello from Main
src/cases/CsSafeTypeBuilding.hx:51: Haxe print: 

The prints that were not found were actually there, but some have been merged into a single "message"...
Checking if CI failures are the same.

@Simn
Copy link
Member

Simn commented Jan 2, 2024

Mac-specific JS failures? Huh...

@kLabz
Copy link
Contributor Author

kLabz commented Jan 3, 2024

Nope it's not mac specific failures, can happen on linux too.

@kLabz
Copy link
Contributor Author

kLabz commented Jan 3, 2024

OK, I got it into control

Haxe server prints are passed as 0x01 + message + newline
haxeserver parses prints as such, but sometimes we get 0x01 + message1 + 0x01 + message2 + newline

Fixed in #11459

@kLabz kLabz marked this pull request as ready for review January 3, 2024 07:40
@kLabz
Copy link
Contributor Author

kLabz commented Jan 3, 2024

Works on 4.3.x too but requires #11153 to be added to 4.3_bugfix

@Simn Simn merged commit f8d1caf into HaxeFoundation:development Jan 3, 2024
61 checks passed
0b1kn00b pushed a commit to 0b1kn00b/haxe that referenced this pull request Jan 25, 2024
…#11159)

* [tests] server tests - basic support for @:variant

* [tests] add server test for compilation server safe type building

* [tests] server tests - error handling for @:variant

* [tests] add debug info

* [tests] temp change of build params

* [tests] add more debug data

* [tests] better debug data on failures

* [tests] reenable other server tests
kLabz added a commit that referenced this pull request Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants