Skip to content
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

Merge signal generation and other features #10

Merged
merged 18 commits into from
Jan 8, 2021
Merged

Merge signal generation and other features #10

merged 18 commits into from
Jan 8, 2021

Conversation

mikolasstuchlik
Copy link
Contributor

@mikolasstuchlik mikolasstuchlik commented Dec 25, 2020

This PR is WIP due to unserolved issues:

  • Add inline documentation
  • Add macOS to the CI and fix broken support for macOS in general

Issues addressed by this PR:

  • Improvements to the Build experience and LSP Pitch: Possible improvements to the build experience. SwiftGtk#34
  • Fix issues with LLDB Issues with LLDB - LLDB fails upon setting a breakpoing and crashes when executing any Swift code SwiftGtk#39
  • Controversial: Implicitly marks all declarations named "priv" as if they had attribute private=1
  • Prevents all "Private" records from generating unless generated in their instance record (added option -a to the main.swift file which requires all records to be genrated)
  • Introduces CI
  • For Class metadata types no longer generates class wrappers. Ref structs now contain static method which returnes the GType of the class and instance of the Class metatype wrapped in the Ref struct.
  • Adds final class GWeak where T could be any Ref struct of a type which supports ARC. This class is a property wrapper which contains weak reference to any instance of T. This is especially beneficial for capture lists.
  • Adds support for weak observation.
  • Constructors and factories of GObjectInitiallyUnowned classes now consume floating reference upon initialization as adviced by the GObject documentation

Partially implemented:

This PR is ment for initial "quick look" into my implementation. I took the liberty to separate gir2swift files and add some directories. I also used as of yet unreleased returnBuilders.

After the PR is accepted for a review:

  • update README documentation

mikolasstuchlik and others added 18 commits January 3, 2021 22:08
This commit splits long files to smaller ones, introduces subdirectories
and removes some superfluous extensions
Generates supporting code for GWeak
Generate code to automatically consume floating references upon init
Adds support for ommiting private types
Ommits class generation for metatypes
* [Build/CI] Fix build until 5.4; add CI for macOS

* Attempt: fix libffi missing in macOS build

* Remove libffi brew; fix driver path

* Add echo in order to debug

* Fix the possible issue

* Remove incorrect argument

* Attempt to solve the issue by removing all priv memebers

Co-authored-by: Mikoláš Stuchlík <mikolas.stuchlik@futured.app>
@mikolasstuchlik
Copy link
Contributor Author

Compatibility with macOS was restored. Working demo could be found here https://github.com/mikolasstuchlik/Matika alongside for build instructions.

There was an issue with macOS build which was resolved in a controversial manner - the issue is described here https://github.com/mikolasstuchlik/gir2swift/pull/7 in more detail.

CI passed for both lates LTS Ubuntu and macOS.

Since the issue of macOS build was addressed, the PR is no longer WIP and is officially proposed.

@mikolasstuchlik mikolasstuchlik changed the title [WIP] Merge signal generation and other features Merge signal generation and other features Jan 4, 2021
@rhx
Copy link
Owner

rhx commented Jan 7, 2021

Any obstacles left for reviewing (and potentially merging) this?

@mikolasstuchlik
Copy link
Contributor Author

mikolasstuchlik commented Jan 8, 2021

Any obstacles left for reviewing (and potentially merging) this?

No, at this moment I don't plan on changing this PR in a significant way.

I look forward to a discussion about any aspect of the PR, since I have made a lot of (controversial) changes.

I plan on updating READMEs and related projects (like HelloWorld) after that (before merge).

I would like to ask you to set-up the Github Actions - the PR contains configuration file, but I apperently lack the authority to do so. The configuration in this PR requests build only at latest macOS and Ubuntu 20.04 with latest Swift and latest Gtk 3. I can add configuration for any other desired platforms.

@rhx
Copy link
Owner

rhx commented Jan 8, 2021

Here are some initial observations:

  • overall this seems to work well
  • I did a quick test on macOS Catalina (as well as Big Sur) and it works fine (haven't tried different versions of Linux, but I don't expect problems as long as they run a recent version of Swift)
    • I would suggest setting up testing for at least macOS Big Sur and Catalina, the latest two Ubuntu LTS versions (and potentially other Linux distributions such as Debian, Fedora, and Arch Linux)

Some minor issues (in addition to the ones you mentioned earlier):

  • The new scripts don't support spaces in directories (e.g. sources located on /Volumes/Google Drive).
    • This should be fixable by properly quoting "$PWD" and similar parameters
  • Performance degradation.
    • Some operations take a very long time.
    • For example with the old config script time ./config.sh takes 0.7s on my system, while time ./run-gir2swift.sh flags -noUpdate takes 22.6 seconds.
  • Extra build dependencies (jq and oniguruma)
  • Some functionality missing (e.g. app-bundle.sh).

None of these are show stoppers. I'll probably merge this into a new development branch first. This gives us some more time to iron things out before unleashing everything to the greater public.

@rhx rhx merged commit 2230b94 into rhx:master Jan 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants