Skip to content

[native_assets_builder] Rename build.dart to hook/build.dart #823

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.

Sign up for GitHub

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

Jump to bottom
Closed
6 tasks done
Tracked by #54334
dcharkes opened this issue Nov 23, 2023 · 6 comments
Closed
6 tasks done
Tracked by #54334

[native_assets_builder] Rename build.dart to hook/build.dart #823

dcharkes opened this issue Nov 23, 2023 · 6 comments
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures package:hooks_runner

Comments

@dcharkes
Copy link
Collaborator

dcharkes commented Nov 23, 2023 •
edited
Loading

We have settled on hook/build.dart as the place for the build.dart "script"/"hook".

If we fall back to build.dart for the time being, we have less rolling issues.

The example that's run against the Dart SDK will break for the time being though.

  • Update native_assets_builder to look for hook/build.dart instead of build.dart. For now: fall-back to build.dart if hook/build.dart does not exist, to not immediately break everyone.
  • Update tests.
  • Update "script" terminology to "hook".
  • Roll into Dart.
  • Update examples (requires Dart roll).
  • Roll into Flutter. (currently blocked by [native_assets_cli] DynamicLoadingBundled and dryRun in Flutter #1049)
@dcharkes dcharkes added P1 A high priority bug; for example, a single project is unusable or has many test failures package:hooks_runner labels Nov 23, 2023
@dcharkes
Copy link
Collaborator Author

It will be easier to roll stuff if we first land build_assets.dart to preceed build.dart roll everything, and then remove build.dart.

@dcharkes
Copy link
Collaborator Author

dcharkes commented Dec 4, 2023

Note from offline discussion: We might want to stick with build.dart. So far we have only a single conflicting use: flutter/flutter#138727 (comment).

@dcharkes dcharkes added P3 A lower priority bug or feature request and removed P1 A high priority bug; for example, a single project is unusable or has many test failures labels Dec 4, 2023
@mit-mit
Copy link
Member

mit-mit commented Dec 13, 2023

I still prefer the more general names. We may only know of a single conflict now, but having a hard coded name is a very brute mechanism, so I think it should be more specific.

@dcharkes dcharkes changed the title [native_assets_builder] Rename build.dart to build_assets.dart [native_assets_builder] Rename build.dart to hook/build.dart Jan 24, 2024
This was referenced Jan 24, 2024
Closed
Closed
@dcharkes
Copy link
Collaborator Author

dcharkes commented Feb 26, 2024 •
edited
Loading

(moved list to top)

@dcharkes dcharkes added P1 A high priority bug; for example, a single project is unusable or has many test failures and removed P3 A lower priority bug or feature request labels Feb 26, 2024
@dcharkes dcharkes mentioned this issue Mar 15, 2024
Merged
auto-submit bot pushed a commit that referenced this issue Mar 19, 2024
@dcharkes
[native_assets_builder] build.dart -> hook/build.dart (#1018)
ba431cb 
* #823

Notes:

* Should keep existing toplevel `build.dart` scripts working by trying both `hook/build.dart` and `build.dart`.
* Does not migrate the examples yet, so that we don't have to do `breaking-change` is true in the ci.
   * Will do this after rolling into Dart.
* `native_toolchain_c` had a `this.dartBuildFiles = const ['build.dart'],`. Updating that to `hook/build.dart` would not work for unmigrated scripts, because files listed as dependencies that don't exist will also trigger a rebuild, e.g. it is considered deleting a file. It would also break on any kind of invocation from `hook/link.dart`. So, I've removed the default value for now. It's not pretty though. Ideas @mosuem?
HosseinYousefi pushed a commit that referenced this issue Mar 21, 2024
@dcharkes @HosseinYousefi
[native_assets_builder] build.dart -> hook/build.dart (#1018)
3e5ae0d 
* #823

Notes:

* Should keep existing toplevel `build.dart` scripts working by trying both `hook/build.dart` and `build.dart`.
* Does not migrate the examples yet, so that we don't have to do `breaking-change` is true in the ci.
   * Will do this after rolling into Dart.
* `native_toolchain_c` had a `this.dartBuildFiles = const ['build.dart'],`. Updating that to `hook/build.dart` would not work for unmigrated scripts, because files listed as dependencies that don't exist will also trigger a rebuild, e.g. it is considered deleting a file. It would also break on any kind of invocation from `hook/link.dart`. So, I've removed the default value for now. It's not pretty though. Ideas @mosuem?
@dcharkes dcharkes mentioned this issue Mar 22, 2024
Merged
@dcharkes
Copy link
Collaborator Author

With renaming script to hook. The question is, is the hook/build.dart itself the hook? Or is the thing that invokes the hook/build.dart the hook? It could be argued either way. 🙈

Since build.dart is placed inside hook/ (and not inside hooked/ 😆 ), I'm leaning towards the hook/build.dart being called a hook. And the thing that invokes the hook to be the "invoker", or "to latch on", or something.

So I'm going to call a package's hook/build.dart that packages "build hook".

Ready for another round of bike shedding! cc @mosuem

@dcharkes dcharkes mentioned this issue Mar 22, 2024
Merged
auto-submit bot pushed a commit that referenced this issue Mar 22, 2024
@dcharkes
[native_assets_cli] Examples build.dart -> hook/build.dart (#1041)
c3f1493 
The changes rolled into the Dart SDK, so we can now update the examples.

(The changes have not yet been rolled into Flutter, so the examples will fail in Flutter until we do.)

* #823
@dcharkes
Copy link
Collaborator Author

This has rolled into Flutter in:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures package:hooks_runner
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dcharkes @mit-mit