Skip to content

[native_assets_cli] Add user_defines #2165

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

Merged
merged 3 commits into from
Apr 4, 2025
Merged

[native_assets_cli] Add user_defines #2165

merged 3 commits into from
Apr 4, 2025

Conversation

dcharkes
Copy link
Collaborator

@dcharkes dcharkes commented Apr 3, 2025

Bug: #39

Note passing in the user-defines should be done by the Dart and Flutter SDK, they need to decide where to take the command-line arguments, and how to read from the pubspec.yaml.

The NativeAssetsBuildRunner provides a suggestion about where to put user-defines in the pubspec.yaml:

hooks:
  user_defines:
    my_package:
      user_define_key: user_define_value
      user_define_key2:
        foo: bar
    some_other_package:
      user_define_key3: user_define_value3

Moreover, it provides a helper function that can be reused in SDKs after the pubspec yaml is parsed.

Design choices

(According to the discussion on the bug.)

  • User-defines are name-spaced per package, we don't want new user-defines to invalidate the cache for other packages.
  • User-defines can be provided for any package in the dependency graph.
  • If user-defines need to be shared across packages, the common dependency package can export it as metadata.

Test

This change is tested by invoking the build with user-defines, and in the hook failing if the defines are not available.

Copy link

github-actions bot commented Apr 3, 2025

PR Health

Breaking changes ✔️
Package Change Current Version New Version Needed Version Looking good?
Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package Leaked API symbols
License Headers ✔️
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
Files
no missing headers

All source files should start with a license header.

Unrelated files missing license headers
Files
pkgs/jni/lib/src/third_party/generated_bindings.dart
pkgs/objective_c/lib/src/ns_input_stream.dart

@github-actions github-actions bot added the type-infra A repository infrastructure change or enhancement label Apr 3, 2025
@dcharkes dcharkes marked this pull request as ready for review April 3, 2025 13:35
@dcharkes dcharkes force-pushed the metadata-refactor branch from 5514561 to 6041be6 Compare April 4, 2025 07:51
@dcharkes dcharkes force-pushed the metadata-refactor branch from 6041be6 to fff25a7 Compare April 4, 2025 08:04
@dcharkes dcharkes force-pushed the metadata-refactor branch from fff25a7 to 3525c01 Compare April 4, 2025 08:18
@dcharkes dcharkes force-pushed the metadata-refactor branch from 3525c01 to 28009f3 Compare April 4, 2025 08:35
@dcharkes dcharkes force-pushed the metadata-refactor branch from 803df09 to 22b0a86 Compare April 4, 2025 13:47
Base automatically changed from metadata-refactor to main April 4, 2025 14:24
@coveralls
Copy link

coveralls commented Apr 4, 2025

Coverage Status

coverage: 84.832% (-0.09%) from 84.923%
when pulling df5af88 on user-defines
into 8a1f836 on main.

@auto-submit auto-submit bot merged commit 3fbbf07 into main Apr 4, 2025
46 checks passed
@auto-submit auto-submit bot deleted the user-defines branch April 4, 2025 15:11
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Apr 10, 2025
Updating the dart-lang/native dependencies to the ones published
yesterday.

Incorporates a fix for:

* dart-lang/native#2149
* Note that a bunch of tests were relying on ignoring build assets. The
tests have been fixed.

And the new `AssetRouting` syntax in the build hook for sending assets
to the link hook:

* dart-lang/native#2164

Does not include support for user-defines in the pubspec yet. Will do
this in a follow up PR.

* dart-lang/native#2165
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit package:hooks_runner package:hooks type-infra A repository infrastructure change or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants