Skip to content

Commit 4a0fbd5

Browse files
dcharkesdcharkes
authored
[native_assets_cli] Make CodeAsset os and architecture optional in JSON (#2139)
Bug: #2138 This makes the OS and architecture optional in the syntax and the internals in the semantic API (after #2138 they are no longer available in the semantic API). The validation logic still checks that the OS and architecture is consistent with the `CodeConfig`. (We cannot remove them yet, because link hooks can rely on these fields being present in the input if they were present in the build hook output. The previous PR fixed one such case. So we'll leave that cleanup to later once the removal of the API has rolled everywhere.)
1 parent 627484f commit 4a0fbd5

File tree

6 files changed

+64
-98
lines changed

6 files changed

+64
-98
lines changed

‎pkgs/code_assets/doc/schema/shared/shared_definitions.schema.json

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -340,10 +340,8 @@
340340
}
341341
},
342342
"required": [
343-
"architecture",
344343
"id",
345-
"link_mode",
346-
"os"
344+
"link_mode"
347345
],
348346
"if": {
349347
"properties": {

‎pkgs/code_assets/test/schema/schema_test.dart

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,15 @@ FieldsFunction _codeFields(AllTestData allTestData) {
8787
required Hook hook,
8888
required Party party,
8989
}) {
90-
const requiredCodeAssetFields = [
91-
['architecture'],
92-
['os'],
93-
['id'],
94-
['link_mode'],
95-
['link_mode', 'type'],
96-
];
90+
// (expectFunction, path)
91+
const codeAssetFields =
92+
<(List<Object>, void Function(ValidationResults result))>[
93+
(['architecture'], expectOptionalFieldMissing),
94+
(['os'], expectOptionalFieldMissing),
95+
(['id'], expectRequiredFieldMissing),
96+
(['link_mode'], expectRequiredFieldMissing),
97+
(['link_mode', 'type'], expectRequiredFieldMissing),
98+
];
9799

98100
return <(List<Object>, void Function(ValidationResults result))>[
99101
for (final codeConfigPath in [
@@ -111,20 +113,17 @@ FieldsFunction _codeFields(AllTestData allTestData) {
111113
expectRequiredFieldMissing,
112114
),
113115
if (hook == Hook.link) ...[
114-
for (final field in requiredCodeAssetFields)
116+
for (final (field, expect) in codeAssetFields)
115117
for (final encoding in _encoding)
116-
(
117-
['assets', 0, ...encoding, ...field],
118-
expectRequiredFieldMissing,
119-
),
118+
(['assets', 0, ...encoding, ...field], expect),
120119
],
121120
],
122121
if (inputOrOutput == InputOrOutput.output) ...[
123-
for (final field in requiredCodeAssetFields)
122+
for (final (field, expect) in codeAssetFields)
124123
for (final encoding in _encoding)
125-
(['assets', 0, ...encoding, ...field], expectRequiredFieldMissing),
124+
(['assets', 0, ...encoding, ...field], expect),
126125
if (hook == Hook.build) ...[
127-
for (final field in requiredCodeAssetFields)
126+
for (final (field, expect) in codeAssetFields)
128127
for (final encoding in _encoding)
129128
for (final assetsForLinking in [
130129
'assetsForLinking',
@@ -138,7 +137,7 @@ FieldsFunction _codeFields(AllTestData allTestData) {
138137
...encoding,
139138
...field,
140139
],
141-
expectRequiredFieldMissing,
140+
expect,
142141
),
143142
],
144143
(['assets', staticIndex, 'file'], expectRequiredFieldMissing),

‎pkgs/native_assets_cli/lib/src/code_assets/code_asset.dart

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@ final class CodeAsset {
5555
final String id;
5656

5757
/// The operating system this asset can run on.
58-
final OS _os;
58+
final OS? _os;
5959

6060
/// The architecture this asset can run on.
61-
final Architecture _architecture;
61+
final Architecture? _architecture;
6262

6363
/// The link mode for this native code.
6464
///
@@ -99,9 +99,9 @@ final class CodeAsset {
9999
CodeAsset._({
100100
required this.id,
101101
required this.linkMode,
102-
required OS os,
102+
required OS? os,
103103
required this.file,
104-
required Architecture architecture,
104+
required Architecture? architecture,
105105
}) : _architecture = architecture,
106106
_os = os;
107107

@@ -113,8 +113,14 @@ final class CodeAsset {
113113
);
114114
return CodeAsset._(
115115
id: syntaxNode.id,
116-
os: OSSyntax.fromSyntax(syntaxNode.os),
117-
architecture: ArchitectureSyntax.fromSyntax(syntaxNode.architecture),
116+
os: switch (syntaxNode.os) {
117+
null => null,
118+
final s => OSSyntax.fromSyntax(s),
119+
},
120+
architecture: switch (syntaxNode.architecture) {
121+
null => null,
122+
final s => ArchitectureSyntax.fromSyntax(s),
123+
},
118124
linkMode: LinkModeSyntax.fromSyntax(syntaxNode.linkMode),
119125
file: syntaxNode.file,
120126
);
@@ -151,12 +157,11 @@ final class CodeAsset {
151157

152158
EncodedAsset encode() {
153159
final encoding = syntax.NativeCodeAssetEncoding(
154-
architecture: _architecture.toSyntax(),
160+
architecture: _architecture?.toSyntax(),
155161
file: file,
156162
id: id,
157163
linkMode: linkMode.toSyntax(),
158-
159-
os: _os.toSyntax(),
164+
os: _os?.toSyntax(),
160165
);
161166
return EncodedAsset(CodeAsset.type, encoding.json);
162167
}
@@ -166,9 +171,9 @@ final class CodeAsset {
166171

167172
// These field will be removed in the future, prevent anyone from reading them.
168173
extension ForValidationOnly on CodeAsset {
169-
OS get os => _os;
174+
OS? get os => _os;
170175

171-
Architecture get architecture => _architecture;
176+
Architecture? get architecture => _architecture;
172177
}
173178

174179
extension OSLibraryNaming on OS {

‎pkgs/native_assets_cli/lib/src/code_assets/syntax.g.dart

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -818,12 +818,12 @@ class NativeCodeAsset extends Asset {
818818
NativeCodeAsset.fromJson(super.json, {super.path}) : super._fromJson();
819819

820820
NativeCodeAsset({
821-
required Architecture architecture,
821+
required Architecture? architecture,
822822
required NativeCodeAssetEncoding? encoding,
823823
required Uri? file,
824824
required String id,
825825
required LinkMode linkMode,
826-
required OS os,
826+
required OS? os,
827827
}) : super(type: 'native_code') {
828828
_architecture = architecture;
829829
_encoding = encoding;
@@ -837,12 +837,12 @@ class NativeCodeAsset extends Asset {
837837
/// Setup all fields for [NativeCodeAsset] that are not in
838838
/// [Asset].
839839
void setup({
840-
required Architecture architecture,
840+
required Architecture? architecture,
841841
required NativeCodeAssetEncoding? encoding,
842842
required Uri? file,
843843
required String id,
844844
required LinkMode linkMode,
845-
required OS os,
845+
required OS? os,
846846
}) {
847847
_architecture = architecture;
848848
_encoding = encoding;
@@ -853,17 +853,18 @@ class NativeCodeAsset extends Asset {
853853
json.sortOnKey();
854854
}
855855

856-
Architecture get architecture {
857-
final jsonValue = _reader.get<String>('architecture');
856+
Architecture? get architecture {
857+
final jsonValue = _reader.get<String?>('architecture');
858+
if (jsonValue == null) return null;
858859
return Architecture.fromJson(jsonValue);
859860
}
860861

861-
set _architecture(Architecture value) {
862-
json['architecture'] = value.name;
862+
set _architecture(Architecture? value) {
863+
json.setOrRemove('architecture', value?.name);
863864
}
864865

865866
List<String> _validateArchitecture() =>
866-
_reader.validate<String>('architecture');
867+
_reader.validate<String?>('architecture');
867868

868869
NativeCodeAssetEncoding? get encoding {
869870
final jsonValue = _reader.optionalMap('encoding');
@@ -919,16 +920,17 @@ class NativeCodeAsset extends Asset {
919920
return linkMode.validate();
920921
}
921922

922-
OS get os {
923-
final jsonValue = _reader.get<String>('os');
923+
OS? get os {
924+
final jsonValue = _reader.get<String?>('os');
925+
if (jsonValue == null) return null;
924926
return OS.fromJson(jsonValue);
925927
}
926928

927-
set _os(OS value) {
928-
json['os'] = value.name;
929+
set _os(OS? value) {
930+
json.setOrRemove('os', value?.name);
929931
}
930932

931-
List<String> _validateOs() => _reader.validate<String>('os');
933+
List<String> _validateOs() => _reader.validate<String?>('os');
932934

933935
@override
934936
List<String> validate() => [
@@ -974,11 +976,11 @@ class NativeCodeAssetEncoding {
974976
NativeCodeAssetEncoding.fromJson(this.json, {this.path = const []});
975977

976978
NativeCodeAssetEncoding({
977-
required Architecture architecture,
979+
required Architecture? architecture,
978980
required Uri? file,
979981
required String id,
980982
required LinkMode linkMode,
981-
required OS os,
983+
required OS? os,
982984
}) : json = {},
983985
path = const [] {
984986
_architecture = architecture;
@@ -989,17 +991,18 @@ class NativeCodeAssetEncoding {
989991
json.sortOnKey();
990992
}
991993

992-
Architecture get architecture {
993-
final jsonValue = _reader.get<String>('architecture');
994+
Architecture? get architecture {
995+
final jsonValue = _reader.get<String?>('architecture');
996+
if (jsonValue == null) return null;
994997
return Architecture.fromJson(jsonValue);
995998
}
996999

997-
set _architecture(Architecture value) {
998-
json['architecture'] = value.name;
1000+
set _architecture(Architecture? value) {
1001+
json.setOrRemove('architecture', value?.name);
9991002
}
10001003

10011004
List<String> _validateArchitecture() =>
1002-
_reader.validate<String>('architecture');
1005+
_reader.validate<String?>('architecture');
10031006

10041007
Uri? get file => _reader.optionalPath('file');
10051008

@@ -1034,16 +1037,17 @@ class NativeCodeAssetEncoding {
10341037
return linkMode.validate();
10351038
}
10361039

1037-
OS get os {
1038-
final jsonValue = _reader.get<String>('os');
1040+
OS? get os {
1041+
final jsonValue = _reader.get<String?>('os');
1042+
if (jsonValue == null) return null;
10391043
return OS.fromJson(jsonValue);
10401044
}
10411045

1042-
set _os(OS value) {
1043-
json['os'] = value.name;
1046+
set _os(OS? value) {
1047+
json.setOrRemove('os', value?.name);
10441048
}
10451049

1046-
List<String> _validateOs() => _reader.validate<String>('os');
1050+
List<String> _validateOs() => _reader.validate<String?>('os');
10471051

10481052
List<String> validate() => [
10491053
..._validateArchitecture(),

‎pkgs/native_assets_cli/lib/src/code_assets/validation.dart

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -232,16 +232,15 @@ void _validateCodeAsset(
232232
}
233233

234234
final os = codeAsset.os;
235-
if (codeConfig.targetOS != os) {
235+
if (os != null && codeConfig.targetOS != os) {
236236
final error =
237237
'CodeAsset "$id" has a os "$os", which '
238238
'is not the target os "${codeConfig.targetOS}".';
239239
errors.add(error);
240240
}
241241

242242
final architecture = codeAsset.architecture;
243-
244-
if (architecture != codeConfig.targetArchitecture) {
243+
if (architecture != null && architecture != codeConfig.targetArchitecture) {
245244
errors.add(
246245
'CodeAsset "$id" has an architecture "$architecture", which '
247246
'is not the target architecture "${codeConfig.targetArchitecture}".',

‎pkgs/native_assets_cli/test/code_assets/validation_test.dart

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -158,45 +158,6 @@ void main() {
158158
),
159159
isEmpty,
160160
);
161-
162-
traverseJson<Map<String, Object?>>(outputBuilder.json, [
163-
'assets',
164-
0,
165-
'encoding',
166-
]).remove('architecture');
167-
expect(
168-
await validateCodeAssetBuildOutput(
169-
input,
170-
BuildOutput(outputBuilder.json),
171-
),
172-
contains(
173-
contains(
174-
'No value was provided for \'assets.0.encoding.architecture\'.'
175-
' Expected a String.',
176-
),
177-
),
178-
);
179-
180-
traverseJson<Map<String, Object?>>(outputBuilder.json, [
181-
'assets',
182-
0,
183-
]).remove('encoding');
184-
traverseJson<Map<String, Object?>>(outputBuilder.json, [
185-
'assets',
186-
0,
187-
]).remove('architecture');
188-
expect(
189-
await validateCodeAssetBuildOutput(
190-
input,
191-
BuildOutput(outputBuilder.json),
192-
),
193-
contains(
194-
contains(
195-
'No value was provided for \'assets.0.architecture\'.'
196-
' Expected a String.',
197-
),
198-
),
199-
);
200161
});
201162

202163
test('dynamic_loading_system uri missing', () async {

0 commit comments

Comments
 (0)