Sharing shared definitions

Consider the following scenario:

header1 - with a struct
header2 which imports header1 - having that struct in a signature
header3 which imports header1 - having also that struct in the signature

Now we run the generator on header 2 and 3: they both generate the same struct.
These two structs will be Dart classes with the same name but incompatible.
You can work around that with Pointer.cast<>(), but that's not safe.
Also, this increases code size.

It would be better to be able to run the generator on all 3 headers files and let the generated files for header 2 and 3 import the generated file for header 1.

We need to think about a good design for this, especially if headers 2 and 3 are in different dart packages.

(I am not sure how common this scenario is. For example libclang and sqlite3 stand very much on themselves. But I can imagine structs defined in OS APIs fall into this category, so it would be nice to address this.)

@dcharkes

Notes from today's discussion (@dcharkes)

We need a way to specify declarations dependencies that are already satisfied. (Maybe something like passing the config.yaml file of a binding as it is to some other package)
Specifying imports needed for generated bindings.
An option to disable automatically adding struct (Windows dart ci #23) (May not be needed if it's already part of the dependency)

@mannprerak2

Notes from today's discussion (@mannprerak2)

We could have a serializable intermediate format, to avoid reparsing the dependencies.
- This would change our architecture from parsing -> AST -> generating (resolves conflicting names) to parsing -> AST -> resolving conflicts -> AST2 (serializable) -> generating.

Some high level design questions/remarks.

We have two main use cases:

Having reusable library bindings in a separate package (For example standard library bindings.)
Having multiple generation steps in a single library. (For example the bindings for a native library and the bindings of some utility C code that the package author likes to keep separate from the native library. For example in package:webcrypto).

These two use cases mean we might both have import urls to other packages as well imports to relative paths.

Then we need to consider the users of reusable libraries, what is the minimal information they need to specify in their ffigen.yaml?

The absolute minimum is the path to the symbols.yaml (possibly in another package).

shared-bindings:
  base_h: # we don't actually need this name 
    package: 'foo' # optional, if omitted its a path in the package itself
    path: 'generated/base_symbols.yaml'

This means the generated file would need to contain the file paths.

We could refer to the ffigen.yaml instead, and fish the output path from there. (This would assume the ffigen invocation and the config are always in the same folder.)

@dcharkes

@dcharkes how are we planning to serve the symbol.yaml file to the user? Does pub allow packaging these?

And perhaps another wild idea could be to use annotations here somehow? Wdyt?

how are we planning to serve the symbol.yaml file to the user? Does pub allow packaging these?

Yes, anything that is not in .gitignore is packaged with a package on pub.

We can find a packageRoot of dependent packages by parsing .dart_tool/package_config.json with package:package_config, and then find the yaml within the package.

And perhaps another wild idea could be to use annotations here somehow? Wdyt?

That's a neat idea!

We could generate an annotation with the data in Dart maps/lists/sets/strings instead of putting it in a yaml. We could put it on every individual definition instead of building a map. I don't have so much experience with annotation processing to know how easy it is to find it on all definitions. Alternatively we could put an annotation on the top-class with the full mapping.

One potential downside of annotations is annotation parsing speed vs yaml parsing. We'd need to parse the whole Dart file. I'm not sure if this is a concern, we should check the speed for that.

Another potential downside of annotations is that we can't use a re-export mechanism. For example, I expect to generate bindings in lib/src/.../*.dart which should not be imported by other packages. And then re-export those bindings in lib/<my-package>.dart.

@mannprerak2

Reply to design in dart-archive/ffigen#468.

A library can now export the symbols by specifying

symbol-file: 
  output: '../generated/base_symbols.yaml'
  import-path: '../generated/base_gen.dart' # Warns if not using a package: URI.

lgtm

We could make import-path optional if we generate into lib/**.dart and not lib/src/**.dart, but then ffigen has to detect where the packageRoot is. If we're being run by dart run ffigen ... it should be fairly easy to detect the package root.

Exported symbol.yaml file looks like

import-path: ../generated/base_gen.dart
symbols:
  "c:@E@BaseEnum":
    name: BaseEnum
  "c:@F@base_func1":
    name: base_func1
  "c:@S@BaseStruct1":

Would we be ever in a situation in which we would want to have multiple import paths in a single mapping file? If so, it would be better to nest the symbols under the file path.

"package:foo/foo.dart" :
  symbols:
    "c:@E@BaseEnum":
      name: BaseEnum
    "c:@F@base_func1":
      name: base_func1

Maybe we should keep the option open that we might generate multiple files out of a single ffigen run in the future. (If we come up with some reason that our generated files are too big and we want to split them based on C source file structure for example.)
Also, maybe someone would like to combine multiple symbol.yamls into a single file for some reason.

This way we don't have to worry about a breaking change to the format later.

Thinking about breaking changes to this format, should we encode a format version in the file? Since packages can have different major versions of ffigen in their dev dependencies we can't rely on ffigen package versioning.

format_version: 1.0.0
"../generated/base_gen.dart" :
  symbols:
    "c:@E@BaseEnum":
      name: BaseEnum
    "c:@F@base_func1":
      name: base_func1

Now there is a question if we just care about the format itself, or also about the structure of the generated files. If we're also dependent on the structure of the generated files, it should probably encode the ffigen version instead. (At the downside that people might have to ping their dependencies to upgrade their ffigen and regenerate the file.)

ffigen_version: 7.0.0
symbols:
  "../generated/base_gen.dart" :
    "c:@E@BaseEnum":
      name: BaseEnum
    "c:@F@base_func1":
      name: base_func1

Or even:

"../generated/base_gen.dart" :
  ffigen_version: 7.0.0 # Enables combining 7.0.0 and 7.0.1 etc in a single file. But now we don't have a version at the top, so maybe a bad idea.
  symbols:
    "c:@E@BaseEnum":
      name: BaseEnum
    "c:@F@base_func1":
      name: base_func1

Also, we have config options that influence how the output is generated. For example the @FfiNatives put the symbols in the top level, so those config options need to be reflected in the yaml as well. For these options its probably best to just mirror the config for ffigen itself.

format_version: 1.0.0
"../generated/base_gen.dart" :
  used_config:
    ffi-native: # present or not
  symbols:
    "c:@E@BaseEnum":
      name: BaseEnum
    "c:@F@base_func1":
      name: base_func1

To import symbols, library-imports are used -

library-imports:
  old_format: 'old/format/still/works.dart'
  base_h:
    symbol-file: '../generated/base_symbols.yaml'
    import-path: '../generated/base_gen.dart' # Optional, can be used to override the value in symbol-file.

lgtm

Points to decide:

Do we want to version with ffigen or do we want to version the format by itself.
- With ffigen can lead to spurious version mismatches that would work in practise, but is easy.
- It's own versioning we could forget to bump the symbols.yaml major version even though we do a breaking change to generated code.
Do we want to support multi-file: I say yes. Even though right now we don't.

Points to fix:

Add some kind of versioning.
Add the config items that influence the output file. (ffi-native:)

wdyt @mannprerak2 (fyi @mahesh-hegde)

This seems to be a good enough symbol file format to me

format_version: 1.0.0
"../generated/base_gen.dart" :
  used_config:
    ffi-native: true # or false
  symbols:
    "c:@E@BaseEnum":
      name: BaseEnum
    "c:@F@base_func1":
      name: base_func1

Do we want to version with ffigen or do we want to version the format by itself

How about putting the version of the least supported ffigen version in the generated symbols file? Each ffigen release will have to ensure that if there's a breaking change in symbols file or the generated bindings, we increment the least supported version.

Do we want to support multi-file: I say yes. Even though right now we don't.

The above symbol file lgtm. We can always increment the format version later if need be.

For example the @FfiNatives put the symbols in the top level,

How are we planning to use this information?

For libraries with different ffi-native flags (true/false)

Type sharing is only possible for NoLookupBindings so the imports will still work
However since we are also not generating the shared LookupBindings (since they are part of the shared bindings) libraries which have different ffi-native: flag are not compatible. The library author needs to either generate 2 bindings, (for true/false) and 2 symbol files each. Or in case the ffi-native flags are incompatible, we generate all the NoLookupBindings instead of excluding them.

How about putting the version of the least supported ffigen version in the generated symbols file? Each ffigen release will have to ensure that if there's a breaking change in symbols file or the generated bindings, we increment the least supported version.

We could do that, at the cost of newer versions of ffigen having to be able to keep parsing older format versions. If we need to update that number manually, I think it makes more sense to have a separate version for the format. We could also add a test to ensure we don't forget to bump the number of we break something by:

creating a temp folder
write a pubspec.yaml with a dev dependency on an older version of ffigen (we pin it in the test)
do dart pub get
write a symbols.yaml and ffigen.yaml
do dart run ffigen
Then the test should fail if that older ffigen fails to run on the yaml.

Type sharing is only possible for NoLookupBindings so the imports will still work

Ah, you're correct. We're not actually invoking any of the functions bound to. 🙃

2. However since we are also not generating the shared LookupBindings (since they are part of the shared bindings) libraries which have different ffi-native: flag are not compatible.

I think mixing and matching FfiNatives with lookup is not going to work from a bundling libraries with a Dart of Flutter application point of view. Maybe the lookup ones could depend on the FfiNative ones, e.g. the bundling question should be solved by the Dart infrastructure later for FfiNatives, so layering lookup with which bundling/finding the dynamic library is manual could work.

How are they not compatible in the generated code? Can you paste a snippet with an example?

Maybe it would be slightly cleaner to put the map with files under its own key.

format_version: 1.0.0
files:
  "../generated/base_gen.dart" :
    used_config:
      ffi-native: true # or false
    symbols:
      "c:@E@BaseEnum":
        name: BaseEnum
      "c:@F@base_func1":
        name: base_func1

Then we don't have to disambiguate format_version and any other top level keys we add from file paths.

How are they not compatible in the generated code? Can you paste a snippet with an example?

But say there's a Base library that is exported by some other library with ffi-native: true. Someone now wants to share the base library bindings but want to use dynamic linking. If they use the shared symbol file, we'll end up not generating the functions in base library. So the user is forced to just generate the bindings by themselves now.

library-imports:
  old_format: 'old/format/still/works.dart'
  base_h:
    symbol-file: '../generated/base_symbols.yaml'
    import-path: '../generated/base_gen.dart' # Optional, can be used to override the value in symbol-file.

If a symbol-file has multiple paths, how do we update this config? since a library import is mapped to a single import statement (import something.dart as lib_name)?

I can't really see any drawbacks of symbol file with only 1 import. The shared library can always have/generate multiple symbol files, otherwise a single file exporting multiple files is also an option.

I can't really see any drawbacks of symbol file with only 1 import. The shared library can always have/generate multiple symbol files, otherwise a single file exporting multiple files is also an option.

It would be rather inconvenient if we would generate mappings into multiple files in the future. For example if we a huge C lib with multiple header files and we'd like the Dart bindings to be split among different files the same way. Now, if some other ffigen invocation wants to use all those generated bindings, but we generate a single symbol file per generated Dart bindings file, the other library has to list N symbol files to import. I'd much prefer the other library only listing 1 file to import. Or did you mean to ask something else?

If a symbol-file has multiple paths, how do we update this config? since a library import is mapped to a single import statement (import something.dart as lib_name)?

Why do we need a lib_name? Can we invent one? The import name used is just something internal to the generated file, correct?

Why do we need a lib_name? Can we invent one? The import name used is just something internal to the generated file, correct?

Yeah, this is used as the import name.

I guess we can use this as the import prefix when there are multiple files involved.

library-imports:
  old_format: 'old/format/still/works.dart'
  base_h:
    symbol-file: '../generated/base_symbols.yaml'
    import-path: '../generated/base_gen.dart' # Optional, can be used to override the value in symbol-file.

If the default new use case is not not overwriting the import-path, and if the lib_name can be invented, writing it out as

library-imports:
  base_h:
    symbol-file: '../generated/base_symbols.yaml'
  foo_h:
    symbol-file: '../generated/foo_symbols.yaml'

feels verbose.

If we want to remove lib_name and invent it. We need to make it a list rather than a map with lib_name as the key.
And if it's a list, it needs to be under a key.
And, if we have only the symbol-file when not specifying import-path overrides, we could omit the symbol-file key.

library-imports:
  symbol-files:
    - '../generated/base_symbols.yaml'
    - '../generated/foo_symbols.yaml'

And then if you want to overwrite the import-path:

library-imports:
  symbol-files:
    - '../generated/base_symbols.yaml'
    -
      symbol-file: '../generated/foo_symbols.yaml'
      import-path: '../generated/foo_gen.dart'

One drawback of inventing lib_name is that the user can no longer use a symbol-file import in our type-map configuration manually.

Or maybe we should keep symbol-file a separate config unrelated to library-imports

One drawback of inventing lib_name is that the user can no longer use a symbol-file import in our type-map configuration manually.

Ah I see, I hadn't realized we also used that identifier in our type-map.

I wonder how common that use case is or not. Do we use the type-map often?

Or maybe we should keep symbol-file a separate config unrelated to library-imports

That is an option. (That would also not support using it in the type-map.)

In some sense the symbol files are like automated library-imports while the library imports and type map are like manual imports. So with that reasoning it does make sense to keep them together, correct?

I would like to understand our current use cases of the type-map to understand if we should support it.

The only use case in our whole code base is currently:

  # These can be used to map any typedef to a specific native type.
  type-map:
    typedefs:
      'time_t':
        lib: 'ffi'
        c-type: 'Int64'
        dart-type: 'int'
    structs:
      'CXCursorSetImpl':
        lib: 'custom_import'
        c-type: 'CXCursorSetImpl'
        dart-type: 'CXCursorSetImpl'

Which is the example that two type maps. But our actual libclang bindings don't have the type map.

Is the type-map a workaround that we used to use because we were not supporting AbiSpecificIntegers in the past? But now that we have AbiSpecificIntegers, the type-map is a feature that we should deprecate?

I would like to understand our current use cases of the type-map to understand if we should support it.

It's basically manually using types from another library, perhaps one that doesn't have a symbols file in the future. Another use case could be using custom structs that are not yet supported by ffigen but can somehow be written.

I think both features (type-map and symbol-file) can coexist without interfering and there's no overhead of keeping type-map so maybe we should defer deprecating it later (if need be).

It's basically manually using types from another library, perhaps one that doesn't have a symbols file in the future.

Generating a symbols file should be trivial. So making a PR for such package should be trivial.

Another use case could be using custom structs that are not yet supported by ffigen but can somehow be written.

Right, but that would then also be solved later.

So I think in some beautiful future, users would only be using symbol files and not writing any library imports or type maps by hand.

I do think it would still make sense to nest it under library-imports as:

library-imports:
  symbol-files:
    - '../generated/base_symbols.yaml'
    - '../generated/foo_symbols.yaml'

library-imports:
  symbol-files:
    - '../generated/base_symbols.yaml'
    -
      symbol-file: '../generated/foo_symbols.yaml'
      import-path: '../generated/foo_gen.dart'

We'd also need to specify symbol-file as output.

In JNIgen I proposed the following output structure: #738

Should we also have an output structure in the yaml for FFIgen?

I do think it would still make sense to nest it under library-imports

library-imports currently expects arbitrary key-value pairs, key: library name, value: path
Nesting it under is doable but not very clean (both implementation and documentation wise)

And since type-map can no longer use a symbol-file import, I'd still prefer an independent config like

symbol-file-import:
    - '../generated/base_symbols.yaml'
    -
      symbol-file: '../generated/foo_symbols.yaml'
      import-path: '../generated/foo_gen.dart'

We'd also need to specify symbol-file as output.

We can follow suit to the your proposed structure and unify it with the output config while keeping it backwards compatible

output:
    bindings: path/to/bindings
    symbol-file: path/to/symbol/file

Now the question of how import-path and bindings out will change for multiple import paths in the symbol-file still remains?
Since in the future we may want to add support for multiple files being listed in the symbol file

And since type-map can no longer use a symbol-file import, I'd still prefer an independent config like
symbol-file-import:
    - '../generated/base_symbols.yaml'
    -
      symbol-file: '../generated/foo_symbols.yaml'
      import-path: '../generated/foo_gen.dart' 
    

Can we call it import? (Is there anything else we would ever like to import?)

Or

import:
  symbol-files:
    - '../generated/base_symbols.yaml'
    -
      symbol-file: '../generated/foo_symbols.yaml'
      import-path: '../generated/foo_gen.dart'

I'm just kind of brainstorming here. This would make import->symbol-files and output->symbol-file each others' dual.

output:
  bindings: path/to/bindings
  symbol-file: path/to/symbol/file

sgtm

Now the question of how import-path and bindings out will change for multiple import paths in the symbol-file still remains?
Since in the future we may want to add support for multiple files being listed in the symbol file

Maybe we leave that for when we get to it then. Or do you have any suggestions.

Can we call it import?
sgtm

Maybe we leave that for when we get to it then.

Cool

@dcharkes

@dcharkes I've pushed changes according to above discussion (#468).
Some more discussion points -

import:
  symbol-files:
    -
      symbol-file: '../generated/foo_symbols.yaml'
      import-path: '../generated/foo_gen.dart' 

output:
  bindings: path/to/bindings
  symbol-file: path/to/symbol/file

The import-path is not really making sense here, since the symbol file is designed to be able to declare more than a single file. Maybe we remove it? And we only allow package Uris in generated symbol file yaml?
output->symbol-file here cannot just be a string, since the import path (ideally a package import) is also needed for the symbol file.
We can let it be a string, and use output->bindings as the import path. We can also mandate output->bindings to be a package uri.

The import-path is not really making sense here, since the symbol file is designed to be able to declare more than a single file. Maybe we remove it?

sgtm

And we only allow package Uris in generated symbol file yaml?

That might preclude using relative imports if we do multiple ffigens in a single package. (I'm not sure how common that use case is.) Take for example https://github.com/google/webcrypto.dart/blob/9a0af9d3596fd50b70087375827c2242d6f96db2/tool/update_bindings.dart which has two invocations. I think in general relative uris within a package are preferred.

output->symbol-file here cannot just be a string, since the import path (ideally a package import) is also needed for the symbol file.

We can add an import-path here.

We can also mandate output->bindings to be a package uri.

I'd like to support two use cases here:

FFIgen generates into lib/<non-src>abc/xyz.dart, so the generated file itself is immediately available under my_package:abc/xyz.dart. (This we could probably infer.)
FFIgen generates into lib/src/abc/xyz.dart, so there is no valid package url directly, but we have a lib/<non-src>foo.dart that exports lib/src/abc/xyz.dart.

I'm not entirely sure how to make that the most ergonomic.

Maybe (2) is not that common. We could support (1) ergonomically by forcing output->bindings to be a package uri indeed.

We could keep output->bindings a path, but detect if it is accessible under a package uri, and then users can omit output->import-path.

I think in general relative uris within a package are preferred.

Perhaps we can just print a warning to inform users that the symbol file generated cannot be used by external packages since the import-path is not a package uri.

We could keep output->bindings a path, but detect if it is accessible under a package uri, and then users can omit output->import-path

I think it may be simpler to just have a mandatory import-path key.

sgtm

Navigation Menu

Repository navigation

Sharing shared definitions #539

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Sharing shared definitions #539

Description

Activity

mannprerak2 commented on Jul 7, 2020

dcharkes commented on Jul 16, 2020

dcharkes commented on Oct 5, 2022

mannprerak2 commented on Oct 5, 2022

dcharkes commented on Oct 5, 2022

dcharkes commented on Oct 6, 2022

mannprerak2 commented on Oct 6, 2022

dcharkes commented on Oct 7, 2022

dcharkes commented on Oct 7, 2022

mannprerak2 commented on Oct 7, 2022

mannprerak2 commented on Oct 7, 2022

dcharkes commented on Oct 10, 2022

mannprerak2 commented on Oct 10, 2022

dcharkes commented on Oct 12, 2022

mannprerak2 commented on Oct 12, 2022

dcharkes commented on Oct 12, 2022

mannprerak2 commented on Oct 12, 2022

dcharkes commented on Oct 12, 2022

mannprerak2 commented on Oct 12, 2022

dcharkes commented on Oct 12, 2022

mannprerak2 commented on Oct 13, 2022

mannprerak2 commented on Oct 16, 2022

dcharkes commented on Oct 17, 2022

mannprerak2 commented on Oct 18, 2022

dcharkes commented on Oct 18, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions