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

Rewrite to C - enforce standards #501

Closed
ikeydoherty opened this issue Jul 17, 2016 · 21 comments

Comments

Projects
None yet
4 participants
@ikeydoherty
Copy link
Member

commented Jul 17, 2016

The codebase has begun to grow out of control..

With Vala we find many odd runtime issues, constant issues between new vapi files, an excessively heavy approach to the heap, poor debugging, poor maintainability, and frankly utterly shite C code that is emitted.

Even with those items out of the way, things to consider:

  • We use a seriously limited set of CFLAGS to build all Vala elements in Budgie Desktop. Given that it generates C code, this code is not actually subject to strict compiler flags, and poses security risks.
  • We cannot enforce PIE/RELRO properly, again because the resulting code buckles (Solus employs full relro (bind now) by default)
  • There are over 400 bugs encountered by static analysis in the generated C code, some of them having the potential to grow into critical bugs
  • I'm good at C, I don't need a translator...
  • Stable ABI: Completely SOL with Vala as we cannot access GCC attributes, so must rely only on linker scripts, without the ability to correctly track ABI changes.
  • The code is horrifically inefficient.
  • Vala is a complete heap-whore, ignoring stack space and copying All The Things, and explicitly copying things living in .text.
  • No proper tooling around Vala for any form of real world continuous integration or code formatting (clang-format..)
  • In many, many situations, we've been completely roadblocked because Vala isn't advanced enough to speak the level of C we need, i.e. dealing with long transfers via X atoms (bitwise shift isn't a strength here.).
  • At the end of the day, it's not a real programming language. It's translated into C and then the compiler does the real work, if it was even interpreted or contained its own toolchain based on LLVM then we wouldn't have half of these issues, as it stands, it's a translator.

It's been spoken about for a while to do the C rewrite, now lets schedule this as a base requirement for the next major Budgie release, and rip all the shit out of the codebase

Right now we're busting at the seams with non-interoptable crap:

Totals grouped by language (dominant language first):
vala:         10183 (53.34%)
ansic:         8502 (44.54%)
xml:            308 (1.61%)
sh:              96 (0.50%)




Total Physical Source Lines of Code (SLOC)                = 19,089
Development Effort Estimate, Person-Years (Person-Months) = 4.42 (53.09)
 (Basic COCOMO model, Person-Months = 2.4 * (KSLOC**1.05))
Schedule Estimate, Years (Months)                         = 0.94 (11.31)
 (Basic COCOMO model, Months = 2.5 * (person-months**0.38))
Estimated Average Number of Developers (Effort/Schedule)  = 4.69
Total Estimated Cost to Develop                           = $ 597,673
 (average salary = $56,286/year, overhead = 2.40).
SLOCCount, Copyright (C) 2001-2004 David A. Wheeler

Requirements going forward:

  • Travis CI for all PRs
  • Require all C code to comply with our .clang-format configuration, with absolutely no exceptions.
  • Enforce heavier CFLAG requirements and respect distributionCFLAGS fully.
  • Drop ALL Vala code from the core of Budgie - no more Vala additions will be accepted now.
  • Bump the ABI to become incompatible with the current 10.2 branch of Budgie (intentionally)

@ikeydoherty ikeydoherty added this to the 10.3 milestone Jul 17, 2016

@ikeydoherty ikeydoherty self-assigned this Jul 17, 2016

christiankaindl referenced this issue Aug 5, 2016

Convert plugin API to pure C implementation
Signed-off-by: Ikey Doherty <ikey@solus-project.com>
@rilian-la-te

This comment has been minimized.

Copy link

commented Aug 5, 2016

I understand the reason to moving to C, but coding in C is inconvenient, AFAIK, because of GObject boilerplate. GObject boilerplate is simply not good to write:( Why not simplier native lang, like Go or Rust?

@ikeydoherty

This comment has been minimized.

Copy link
Member Author

commented Aug 5, 2016

C isn't the issue really, GObject/GLib is the nastiness.We need a C ABI to allow native extensions/plugins for Budgie. Moving to another language wouldn't enable this.

Doing it in Rust would make it virtually impossible to package Budgie Desktop anywhere, and Solus is one of the few distributions with a working native Rust toolchain.

Go also represents distribution issues, and frankly I'm just not sure how well they'd interact with the eccentric nature of GLib (refcounting vs Go's garbage collection)

@rilian-la-te

This comment has been minimized.

Copy link

commented Aug 5, 2016

GObject/GLib is the nastiness. - You are telling the truth:)
I like Budgie anyways, but writing C boilerplate makes me boring. Is there any tools to automate its writing? And Vala gives to us many convenient functions like DBus annotation (whole interface without XML), GtkTemplate annotation (very convenient template automation) and some smaller.
Rewriting it in C is painful, very painful. But benefical.

@ikeydoherty

This comment has been minimized.

Copy link
Member Author

commented Aug 5, 2016

So I mean what we could do is add some very simple tooling to generate the boilerplate shite for us, including all the properties accessors, etc.

@rilian-la-te

This comment has been minimized.

Copy link

commented Aug 5, 2016

It is definetely worth:) Especially when its tools will be available for plugin developers.
What I like in Vala, I wrote above. And code simplicity.

@AGhost-7

This comment has been minimized.

Copy link

commented Aug 20, 2016

Doing it in Rust would make it virtually impossible to package Budgie Desktop anywhere, and Solus is one of the few distributions with a working native Rust toolchain.

Why is that? Just wondering. Only thing that comes to mind is dynamic linking, but Rust does seem to support it fine (TBH I never tried dynamic linking in Rust so I'd be interested in hearing about that).

@ikeydoherty

This comment has been minimized.

Copy link
Member Author

commented Aug 21, 2016

Because Rust applications cannot be packaged properly, and require internet access to do any builds. cargo basically fetches everything from github. That cannot be permitted for distro packages

@AGhost-7

This comment has been minimized.

Copy link

commented Aug 21, 2016

Looks like you can use overrides to pull stuff from other places.

Was added to cargo fairly recently.

I'd just be really interested in a DE which uses Rust 😄 . I'm not good at C but if its in Rust maybe I can do a few contributions here and there.

@ikeydoherty

This comment has been minimized.

Copy link
Member Author

commented Aug 23, 2016

Right but we'd have to do this for every single package we used for Rust - they need a proper working solution before we ever consider it.

@rilian-la-te

This comment has been minimized.

Copy link

commented Sep 5, 2016

@ikeydoherty do you allow to use g_autoptr in Budgie?

@ikeydoherty

This comment has been minimized.

Copy link
Member Author

commented Sep 5, 2016

@rilian-la-te I would far rather use the autofree mechanism from libnica and cve-check-tool, its very robust, extensible, and isn't dependent on the new ptr adds in subsequent glib releases. Plus the style isn't misleading (you still explicitly use *)

@rilian-la-te

This comment has been minimized.

Copy link

commented Sep 5, 2016

But libnica DEF_AUTOFREE is same as g_autoptr or g_auto or g_autofree. Style only diff, I see, but libnica require linking with it, which is not good. Do you want to release a macro only library with C helpers?

@ikeydoherty

This comment has been minimized.

Copy link
Member Author

commented Sep 5, 2016

Style only diff, I see, but libnica require linking with it, which is not good.

Or yknow, copying the macros into a header file. Far more portable.

Do you want to release a macro only library with C helpers?

Eh. Nica is a shared library and isn't macro-only.

@ikeydoherty

This comment has been minimized.

Copy link
Member Author

commented Sep 5, 2016

So lets explore this now over @ https://github.com/ikeydoherty/budgie-panel :)

@rilian-la-te

This comment has been minimized.

Copy link

commented Sep 8, 2016

What minimal version of GLib is for budgie contributing?

@ikeydoherty

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2016

Atm 2.44

@ikeydoherty ikeydoherty added the P1 label Sep 13, 2016

@ikeydoherty

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2016

This is now a P1 bug. It holds precedence over all other issues, and is a public blocker for the 10.3 target.

@ikeydoherty ikeydoherty removed the bug label Sep 13, 2016

@ikeydoherty

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2016

Deleted comments not pertaining to the discussion (i.e. the netsplit thing..)

@pksadiq

This comment has been minimized.

Copy link

commented Oct 13, 2016

Hi. I just found[0] that several code written in C here, are still using the old long boring boilerplate code.
Glib has recently (since 2.44 release) included a much simple way to do this. Please see the gtk+ examples[1].

Thanks

[0] https://github.com/solus-project/budgie-desktop/blob/master/plugin/plugin.h
[1] https://git.gnome.org/browse/gtk+/commit/?id=22ae9d0884fffb61ee826ec450e8d62ad1cdb957

@ikeydoherty

This comment has been minimized.

Copy link
Member Author

commented Oct 13, 2016

It's written using the old boilerplate for backwards compatibility where the macros don't exist. I'm also not going to rely completely on macros to define my API and not know what it truly looks like.

@solus-project solus-project locked and limited conversation to collaborators Oct 13, 2016

@ikeydoherty

This comment has been minimized.

Copy link
Member Author

commented Jan 25, 2017

This issue has now been been deprecated by https://budgie-desktop.org/2017/01/25/kicking-off-budgie-11/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.