Merge package:ffigen into dart-lang/native #185

liamappelbe · 2023-11-02T03:03:18Z

Do NOT squash merge!

Fixes #437

Following the steps from dart-lang/tools#100

Current step:

Create a pull request and get it reviewed. Do not submit through the github UI, since it does not allow a fast forward merge. If there are changes required, back out and start over (it's OK to force push to the PR branch)

We decided to defer this step until a follow up PR, to make it easier to review:

Replace paths and links in files
1. Badges: https://github.com/dart-lang/ffigen/actions/workflows/test-package.yml -> https://github.com/dart-lang/native/actions/workflows/ffigen.yml
2. Remaining links, manually check if this does the right thing, otherwise insert steps above this step. https://github.com/dart-lang/ffigen -> https://github.com/dart-lang/native/tree/main/pkgs/ffigen

Steps to do:

Disable squash-only in GitHub settings, and merge with a fast forward merge to the main branch, enable squash-only in GitHub settings.
Push tags to github
Follow up with a PR adding links to the top-level readme table.
Add a commit to https://github.com/dart-lang/ffigen/ with it's readme pointing to this repo
Archive https://github.com/dart-lang/ffigen/.

* Fix missing typedef dependencies of func and struct * added tests * update version, changelog

* Added fallback to flutter format if dartfmt is not found * added red color for severe, green for success in log printing * Only catch ProcessException when calling dartfmt * used package:cli_utils for finding dart_sdk and ansi colors * use path.join

…pec.yaml (#88) * removed dart:cli usage, use dylib version from pubspec.lock

* Bool datatype is now mapped to Uint8, Added test in native_test * Added dart-bool to config, bools in function parameters and return type now use dart bool instead of int by default

…#96) * added support for including/excluding/renaming unnamed_enums added faq, update version, changelog, update test * fix typo

…ters (#103)

Fixed code block indentation issue in configuration table.

… macros (#106)

* Fixed issues with macros with value double.infinity and double.NaN

…ypedef (#116)

* Iterate over all typedefs rather than jumping to the base type. * Added typedef-map to config. * Updated version, changelog, and readme.

* Migrated code to pass the null safe syntax. * Generating null safe code.

…125)

* Removed usage of --no-sound-null-safety flag * Update version, changelog * Use flag when running tests on travis

Co-authored-by: J-P Nurmi <[email protected]>

…135, #137) (#164) Co-authored-by: J-P Nurmi <[email protected]>

* WIP static functions * Revert old code gen changes * Refactor function code gen * Arg and return conversions * fix bugs * Fix analysis, add more tests * fmt * Fix autorelease pool test * Handle NS_RETURNS_RETAINED * Daco's comments

dcharkes · 2023-11-02T08:29:41Z

Thanks @liamappelbe!

It looks like the coverage configuration and the changelog need to be updated, see redness on the bots.

mit-mit · 2023-11-02T09:43:22Z

After this lands, please also update the table https://github.com/dart-lang/native/blob/main/README.md#packages

devoncarew · 2023-11-02T16:17:08Z

Do NOT squash merge!

@dcharkes - I believe you have sufficient permissions for this repo to temporarily enable 'Allow rebase merging', merge this PR, and then disable rebase merging. Let me know if this is not the case -

mit-mit · 2023-11-02T16:41:11Z

If not, Daco just ping me and I can help with that.

dcharkes · 2023-11-02T16:47:19Z

Do NOT squash merge!

@dcharkes - I believe you have sufficient permissions for this repo to temporarily enable 'Allow rebase merging', merge this PR, and then disable rebase merging. Let me know if this is not the case -

I do indeed.

(But we'll have to address the redness on coverage and the publishing bot first.)

liamappelbe · 2023-11-02T21:53:54Z

Not sure what to do about this error. Seems like this validator doesn't understand dev versions. Unless someone has a workaround, we'll have to wait to merge the repos until after SDK 3.2 is published.

  Resolving dependencies...
  The current Dart SDK version is 3.1.5.
  
  Because ffigen requires SDK version >=3.2.0-114.0.dev <4.0.0, version solving failed.

dcharkes · 2023-11-03T08:58:21Z

@liamappelbe try setting the channel to beta in .github/workflows/publish.yaml (and update FFIgen to beta instead of dev). docs: https://github.com/dart-lang/ecosystem/blob/main/.github/workflows/publish.yaml#L60-L67

github-actions · 2023-11-08T22:57:37Z

Package publishing

Package	Version	Status	Publish tag (post-merge)
package:ffigen	10.0.0-dev.0	ready to publish	`ffigen-v10.0.0-dev.0`
package:native_toolchain_c	0.3.2	already published at pub.dev
package:native_assets_builder	0.3.0	already published at pub.dev
package:native_assets_cli	0.3.2	already published at pub.dev

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

coveralls · 2023-11-09T00:35:06Z

coverage: 91.903% (-6.1%) from 97.992%
when pulling ff47496 on merge-ffigen-package
into eaea725 on main.

liamappelbe · 2023-11-09T01:35:23Z

@dcharkes Fixed the publish bot. PTAL

dcharkes · 2023-11-09T07:13:12Z

@dcharkes Fixed the publish bot. PTAL

Nice! Both the publisher and code coverage tool is happy!

Regarding CLA bot, the two non-signed CLAs are ex Googlers who contributed during their time at Google, so these should be fine. Please submit an exception request following the link to the bot.

Before I manually merge, as we can only do this once, and commits are not squashed:

Can we land some required changes on the FFIgen repo first? (e.g. the fix Dart formatting+beta SDK, and the fix markdown formatting)
Should we consider also prefixing all commits with [ffigen] ? --commit-callback some examples.
Can we squash the commits that have not been merged into the FFIgen repo before the merge commit and the ones after the merge commit into one commit? (or one commit before and one after)? Also please write detailed commit messages on these, they will be in our history forever.
Smaller nit:
- Can we delete pkgs/ffigen/.github/workflows/test-package.yaml in the same commit as adding ./github/workflows/ffigen.yaml
Merge with the last commit in dart-lang/native

liamappelbe · 2023-11-09T23:49:01Z

@dcharkes Ok, done. Should be good to go now. It's Friday afternoon here, so feel free to merge this during my weekend if it looks good.

dcharkes · 2023-11-10T09:13:19Z

The urls don't look correct: https://github.com/dart-lang/native/tree/main/pkgs/ffigen/blob/main/test/native_test/native_test.dart should be https://github.com/dart-lang/native/tree/main/pkgs/ffigen/test/native_test/native_test.dart.

https://github.com/dart-lang/native/tree/main/pkgs/ffigen/issues/386 we can only rewrite the URLs for issues after we have migrated the issues. Though it might be that the old URLs for issues will still work, so in that case we can just keep the old URLs and let GitHub just forward to the migrated issue.

https://github.com/dart-lang/native/tree/main/pkgs/ffigen/pull/402#issuecomment-1154348670 This should definitely stay the old URL.

Also, can we do the README header rewrite and the SDK to beta bump as a PR on the FFIgen repo first? That means less reviewing on the commits manually.

It's Friday afternoon here, so feel free to merge this during my weekend if it looks good.

I'd rather do it on Monday, who knows we might miss something and need to do some cleanup. 🙄

At least we need to transfer all issues, issue labels, label the issues with package:ffigen etc. (I can't find anything on transfering all issues to another repo at the same time. So we're going to have to hit "transfer issue" 85 times manually. Unless, @devoncarew knows some magic?)

parlough · 2023-11-10T16:51:48Z

At least we need to transfer all issues, issue labels, label the issues with package:ffigen etc. (I can't find anything on transfering all issues to another repo at the same time. So we're going to have to hit "transfer issue" 85 times manually. Unless, devoncarew knows some magic?)

Devon does have some magic prepared. The repo_manage tool has a transfer-issues bulk transfer command that can add a specific label if desired.

devoncarew · 2023-11-10T21:52:52Z

At least we need to transfer all issues, issue labels, label the issues with package:ffigen etc. (I can't find anything on transfering all issues to another repo at the same time. So we're going to have to hit "transfer issue" 85 times manually. Unless, devoncarew knows some magic?)

Devon does have some magic prepared. The repo_manage tool has a transfer-issues bulk transfer command that can add a specific label if desired.

Yup, from memory , there are a few ways to x-fer issues in bulk. Moritz wrote the one in the repo_manage tool. I believe he said that it would occasionally fail after some number of issues, but that that was harmless? You can you capture more of the moves w/ a subsequent run.

You definitely do want to have an additional label to help categorize the issues.

Also: - Delete dependabot.yml - Rename test-package.yml to ffigen.yml, and modify it to use pkgs/ffigen as its working dir

This is needed because the ffigen package depends on the beta version of the SDK.

liamappelbe · 2023-11-13T23:29:35Z

@dcharkes PTAL

Merge package:ffigen into dart-lang/native

mannprerak2 and others added 30 commits August 20, 2020 12:24

[ffigen] Remove structs members for bit field members, Added tests (#85)

e07937e

[ffigen] Fix missing typedef dependencies (#90)

74a808b

* Fix missing typedef dependencies of func and struct * added tests * update version, changelog

[ffigen] Improve logs (#87)

82875d4

* Added fallback to flutter format if dartfmt is not found * added red color for severe, green for success in log printing * Only catch ProcessException when calling dartfmt * used package:cli_utils for finding dart_sdk and ansi colors * use path.join

[ffigen] Removed dart:cli usage, use dylib version from ffigen's pubs…

9566c7e

…pec.yaml (#88) * removed dart:cli usage, use dylib version from pubspec.lock

[ffigen] Added support for c99 bool (#86)

eb3cae2

* Bool datatype is now mapped to Uint8, Added test in native_test * Added dart-bool to config, bools in function parameters and return type now use dart bool instead of int by default

[ffigen] Clarify LLVM error messages (#93)

f760287

[ffigen] Added support for including/excluding/renaming unnamed_enums (…

8e287ce

…#96) * added support for including/excluding/renaming unnamed_enums added faq, update version, changelog, update test * fix typo

[ffigen] Update README.md (#97)

4a17f0d

[ffigen] Update lib/src/README.md (#100)

74a89af

[ffigen] Bump version to 1.0.0, minor fixes (#101)

ad56104

[ffigen] Handle function types in functions/typedefs as function poin…

026ffe1

…ters (#103)

[ffigen] Update readme (#104)

ed2cf4d

Fixed code block indentation issue in configuration table.

[ffigen] Fixed errors due to extended ASCII and control characters in…

46dc624

… macros (#106)

[ffigen] Updated tool to use dart format when sdk >=2.10.0 (#110)

a4923ab

[ffigen] Handled proper conversion of macro doubles. (#113)

13a64f3

* Fixed issues with macros with value double.infinity and double.NaN

[ffigen] Fixed missing typedefs nested under return type of another t…

7a7a823

…ypedef (#116)

[ffigen] Added workaround to map typedef names to a NativeType. (#119)

59708c6

* Iterate over all typedefs rather than jumping to the base type. * Added typedef-map to config. * Updated version, changelog, and readme.

[ffigen] Added support for Dart_Handle to be generated as Handle (#121)

3f86713

[ffigen] Migrate to GitHub Actions (#124)

68e4fa0

[ffigen] Null safety migration (#122)

d98fee1

* Migrated code to pass the null safe syntax. * Generating null safe code.

[ffigen] Remove usage of package:collection. (#130)

2ab4701

[ffigen] Add include path for systems with LLVM 11 (e.g. Debian 11) (#…

fb8ac4e

…125)

[ffigen] Removed usage of --no-sound-null-safety flag. (#136)

4a9672f

* Removed usage of --no-sound-null-safety flag * Update version, changelog * Use flag when running tests on travis

[ffigen] Update README.md (#152)

06131d6

[ffigen] Support for Nested structs (#133) (#161)

7f3948f

Co-authored-by: J-P Nurmi <[email protected]>

[ffigen] Support for passing/returning Structs by value. (#134) (#163)

bee5db2

[ffigen] Remove setup phase and usage of --no-sound-null-safety flag (#…

7ae4d24

…135, #137) (#164) Co-authored-by: J-P Nurmi <[email protected]>

[ffigen] Added support for Global variables (#139) (#165)

bd78139

[ffigen] Use Opaque to represent empty Structs (#142, #143) (#166)

f75b3e8

[ffigen] Skip inline functions (#147) (#167)

3cf6fc0

[ffigen] ObjC static functions (#633)

2491826

* WIP static functions * Revert old code gen changes * Refactor function code gen * Arg and return conversions * fix bugs * Fix analysis, add more tests * fmt * Fix autorelease pool test * Handle NS_RETURNS_RETAINED * Daco's comments

liamappelbe requested a review from dcharkes November 2, 2023 03:03

github-actions bot added the type-infra A repository infrastructure change or enhancement label Nov 2, 2023

mannprerak2 mentioned this pull request Nov 8, 2023

Prevent generating and delete generated file on any [SEVERE] Errors #439

Closed

liamappelbe force-pushed the merge-ffigen-package branch from 9a13216 to 7690b0f Compare November 9, 2023 23:26

liamappelbe added 4 commits November 13, 2023 08:14

[ffigen] Prepare for merge into dart-lang/native (#641)

1920cc9

[ffigen] Change ffigen to use dart-lang/native's directory stucture

85e4257

Also: - Delete dependabot.yml - Rename test-package.yml to ffigen.yml, and modify it to use pkgs/ffigen as its working dir

Merge package:ffigen into dart-lang/native

3634229

Switch publish workflow to use the beta SDK

ff47496

This is needed because the ffigen package depends on the beta version of the SDK.

liamappelbe force-pushed the merge-ffigen-package branch from 2f5cbd9 to ff47496 Compare November 13, 2023 22:42

dcharkes approved these changes Nov 14, 2023

View reviewed changes

dcharkes merged commit 29e423c into main Nov 14, 2023
9 checks passed

dcharkes deleted the merge-ffigen-package branch November 14, 2023 10:40

HosseinYousefi pushed a commit that referenced this pull request Nov 16, 2023

Merge pull request #185 from dart-lang/merge-ffigen-package

9e85b82

Merge package:ffigen into dart-lang/native

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge package:ffigen into dart-lang/native #185

Merge package:ffigen into dart-lang/native #185

liamappelbe commented Nov 2, 2023 •

edited

Loading

dcharkes commented Nov 2, 2023

mit-mit commented Nov 2, 2023

devoncarew commented Nov 2, 2023

mit-mit commented Nov 2, 2023

dcharkes commented Nov 2, 2023

liamappelbe commented Nov 2, 2023

dcharkes commented Nov 3, 2023

github-actions bot commented Nov 8, 2023 •

edited

Loading

coveralls commented Nov 9, 2023 •

edited by github-actions bot

Loading

liamappelbe commented Nov 9, 2023

dcharkes commented Nov 9, 2023

liamappelbe commented Nov 9, 2023

dcharkes commented Nov 10, 2023

parlough commented Nov 10, 2023 •

edited

Loading

devoncarew commented Nov 10, 2023

liamappelbe commented Nov 13, 2023

Merge package:ffigen into dart-lang/native #185

Merge package:ffigen into dart-lang/native #185

Conversation

liamappelbe commented Nov 2, 2023 • edited Loading

Renamed tags

dcharkes commented Nov 2, 2023

mit-mit commented Nov 2, 2023

devoncarew commented Nov 2, 2023

mit-mit commented Nov 2, 2023

dcharkes commented Nov 2, 2023

liamappelbe commented Nov 2, 2023

dcharkes commented Nov 3, 2023

github-actions bot commented Nov 8, 2023 • edited Loading

Package publishing

coveralls commented Nov 9, 2023 • edited by github-actions bot Loading

liamappelbe commented Nov 9, 2023

dcharkes commented Nov 9, 2023

liamappelbe commented Nov 9, 2023

dcharkes commented Nov 10, 2023

parlough commented Nov 10, 2023 • edited Loading

devoncarew commented Nov 10, 2023

liamappelbe commented Nov 13, 2023

liamappelbe commented Nov 2, 2023 •

edited

Loading

github-actions bot commented Nov 8, 2023 •

edited

Loading

coveralls commented Nov 9, 2023 •

edited by github-actions bot

Loading

parlough commented Nov 10, 2023 •

edited

Loading