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

Macro build order #11634

Draft
wants to merge 2 commits into
base: development
Choose a base branch
from
Draft

Macro build order #11634

wants to merge 2 commits into from

Conversation

kLabz
Copy link
Contributor

@kLabz kLabz commented Apr 10, 2024

As an alternative to #11622, adds support for @:buildOrder(Late) and @:buildOrder(Early) on build macros, letting those specify if what they're doing requires running before or after other build macros for current type.

Added tests pass with 4.3.x but not with current nightlies (compilation error).

Also, windows CI is broken again for some reason... works again this morning 🤷

Closes #11194

@Antriel
Copy link
Contributor

Antriel commented Apr 11, 2024

Could it support explicit number, instead of just two fixed values? E.g. @:buildOrder(-100) and @:buildOrder(10), or even @:buildOrder(10.001)? With @:buildOrder(0) being the implicit default?

@kLabz
Copy link
Contributor Author

kLabz commented Apr 11, 2024

I think it'll already be hard enough to get Simon to accept it without precise values 😅
But yeah I wouldn't mind that myself.

@Simn
Copy link
Member

Simn commented Apr 11, 2024

I don't like this. We're addressing a short-time problem by introducing a long-term one. Having just Early/Late won't provide enough control, and arbitrary numbers run the risk of creating some kind of arms race. Surely there has to be a more robust way of addressing build macro order problems...

@Antriel
Copy link
Contributor

Antriel commented Apr 11, 2024

Some prior art is https://github.com/haxetink/tink_syntaxhub, it uses priority queue to order stuff, so things can specify their order relative to others.

@kLabz
Copy link
Contributor Author

kLabz commented Apr 11, 2024

Compiler can't just guess what macro expects things to be ready when they run and what macro is meant to prepare things. Macros have been built with the previous build order (that was depending on your inheritance level) and now half of them are run in reverse order and things just break in ugly ways.

We can't just break openfl + heaps/domkit/hxbit projects + likely other big libs/frameworks with unusable errors and no proper way to update code 😕

before(path)/after(path) could maybe work, though it's not clear how to apply it to build macros. I see two main places: the @:build(...)/@:autoBuild(...) metadata, and metadata on the build macro itself.

@Simn
Copy link
Member

Simn commented Apr 11, 2024

You keep trying to sell me the problem, and I keep telling you that this is not necessary: I acknowledge the problem, I just don't like this particular solution.

I like the notion of involving paths, that would be more explicit.

@kLabz
Copy link
Contributor Author

kLabz commented Apr 11, 2024

Where do you think makes more sense for it? On @:build side or on the build macro side?

@Simn
Copy link
Member

Simn commented Apr 11, 2024

I can see both being useful. Some macros might by-design require other macros to run first, so for them it would be good to manage this themselves. However, users that combine macros might still want to be able to control their order.

@kLabz
Copy link
Contributor Author

kLabz commented Apr 11, 2024

The problem I see there is if there's some incompatibility in the rules, but I guess that's already a problem with two build macros requiring to run before (or after) one another.. So we'll just have to error properly in such cases I suppose 🤔

@kLabz kLabz marked this pull request as draft April 11, 2024 09:22
@Simn
Copy link
Member

Simn commented Apr 11, 2024

Yes, this is essentially a dependency graph and we'll have to detect and report cycles.

@Antriel
Copy link
Contributor

Antriel commented Apr 11, 2024

Yeah, error makes sense to me. And to further complicate it make it more flexible:

  • It should allow specifying multiple rules/paths, for "before" and "after".
  • It should allow specifying whole modules (the paths are module based right?), or just packages.
  • Packages should be soft rules, i.e. explicit module path should take priority, so before a.b and after a.b.Foo shouldn't be a conflict.

@skial skial mentioned this pull request Apr 16, 2024
1 task
@kLabz kLabz added this to the 5.0 preview 1 milestone Apr 29, 2024
@kLabz kLabz modified the milestones: 5.0 preview 1, Release 5.0 Jul 21, 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.

Possible bug in @:autoBuild meta order
3 participants