(In reply to Daniel Espinosa from comment #0)
> Vala makes hard to maintain a C API & ABI.

I don't think that is true. The post doesn't give any examples, the one bug linked to relates to binding to the existing C library and not creating a new API. By marking symbols as `public` and leaving the rest as internal or explicitly marking them as `internal` allows control of the API. The --hide-internal switch with valac needs to be used.

There are choices to be made when designing an API. For making best use of GObject introspection see https://wiki.gnome.org/Projects/GObjectIntrospection/WritingBindingableAPIs
This applies as much to C as it does to Vala, for example: https://github.com/jcupitt/libvips/issues/741

For ABI compatibility there are some very good blog posts by the main author of libgee:

https://blog.piechotka.com.pl/2013/07/30/libraries-in-vala-abi-compatibility-part-i/
https://blog.piechotka.com.pl/2013/12/20/libraries-in-vala-abi-compatibility-part-ii/

There is a beginnings of a tutorial on API/ABI design for libraries written in Vala. Although there need to be more third party examples of developing libraries in Vala to further it. So people blogging about how they developed their API is useful. The tutorial is here:
https://wiki.gnome.org/Projects/Vala/LibraryWritingAPIABI

While I agree with you, as I use Vala for almost all my projects, we need to improve this situation and Wikis are a good point to start on.

We need to copy that posts to Vala's Wiki documentation, so we will not lost them, like:

https://essentials.xebia.com/apis-are-forever/

no longer exists.

On GXml, I'm going to start a process to stabilize its API and ABI, so any documentation is well come.

On interfaces, like the ones at libgee, we need to make easiest, like avoid the CCode attribute property ordering, and make Valac to respect its order as declared in Vala source, documenting this behavior. Interfaces and abstract class should be translated to C in the same order as for Vala source, in order to help ABI stability.

(In reply to Daniel Espinosa from comment #2)
> We need to copy that posts to Vala's Wiki documentation, so we will not lost
> them, like:
> 
> https://essentials.xebia.com/apis-are-forever/
> 
> no longer exists.

It does, but it is only working with http. Maybe they have a configuration problem with their server. Any way I have changed it to http on the wiki. The quote "Public APIs, like diamonds, are forever" is attributed to Joshua Bloch. There are plenty of articles referencing him and his ideas. That is why the link is under Further Reading on the wiki page. A good tutorial should outline the principles with examples rather than copy posts directly into it.

(In reply to Daniel Espinosa from comment #3)
> On GXml, I'm going to start a process to stabilize its API and ABI, so any
> documentation is well come.

One useful thing would be adding an API/ABI checker to the toolchain. There are plenty out there, but they need to be evaluated and an example given of fitting them in to a Vala build system. So a tutorial on that, especially using it with Meson, would be very useful. That is the benefit of tutorials they save time because some one else has done the research. Some links I collected a while ago:

https://lvc.github.io/abi-compliance-checker/
https://blog.remirepo.net/post/2010/06/20/ABI-:-stability-check
https://fedoraproject.org/wiki/How_to_check_for_ABI_changes_with_abi_compliance_checker
https://sourceware.org/glibc/wiki/Testing/ABI_checker
http://ispras.linuxbase.org/index.php/ABI_compliance_checker


> On interfaces, like the ones at libgee, we need to make easiest, like avoid
> the CCode attribute property ordering, and make Valac to respect its order
> as declared in Vala source, documenting this behavior. Interfaces and
> abstract class should be translated to C in the same order as for Vala
> source, in order to help ABI stability.

Maciej (Matthew) Piechotka, the main author of libgee, did some significant work on API/ABI stability. For example the --hide-internal switch patch:
https://bugzilla.gnome.org/show_bug.cgi?id=700157

For interfaces and abstract classes you should take a look at:
https://bugzilla.gnome.org/show_bug.cgi?id=706712
https://blog.piechotka.com.pl/2013/05/18/vala-abi-and-branch-prediction/

The article you are referring to (https://blog.piechotka.com.pl/2013/12/20/libraries-in-vala-abi-compatibility-part-ii/) has some useful advice:

 - "Vala do order the fields more or less in order they appear in source code (there is more to it but I’ll describe it later) which means that changing the order of fields or virtual methods break the ABI (even when API remains the same)" That's pretty much the definition of ABI compatibility

 - "As a rule of thumb you should not have a public, protected or internal fields and rely on private ones (the further problem with internal fields is that Vala does not handle them very well at this moment)" - see https://bugzilla.gnome.org/show_bug.cgi?id=697777

 - "While in most cases the methods are easier to handle (as long as they are not changed into virtuals ... ) there are still a few things to remember...The first problem is the ownership...Second, possibly more obvious, problem is a default value for parameters." This seems sensible advice

 - "The interfaces are easier to handle than objects. The client access them only by pointers so they can be extended without worrying about breaking ABI. However the ordering is still an issue as the methods, signals and properties are sorted as with classes" Maybe this is a problem, but an API with interfaces and virtual methods is fairly sophisticated. With libgee  there is also the use of generics. It would be good to concentrate on a more basic API and how the ABI can be made stable easily before looking more closely at virtual methods and interfaces

All your recommendations are welcome, but what about to write down them to Vala's documentation in a ABI stability chapter for libraries, in order to explain basics and let tutorials to go in dept?

May be Vala from Valadate project, wants to work on adding that chapter?

Is clear for a C programmer how to handle ABI, but is not for Vala ones, because it is translated to C, but no advise on how virtual, abstract methods are handled, in order to avoid common mistakes.

To be fair, control over the API was by far the least important reason to port the library but just a nice addition. I probably should have written that writting the API in C gives me complete control over the target API instead. Actually now I suffer from a looser control over the generated Vala API, but thats less of a problem. :)

To make it more clear, as I want to promote the lib as a C one, having to generate the C API from Vala makes me have to trust valac to always produce the exact same API in for each version of Vala and I have to trust myself to perfectly understand how the Vala API is translated into C. It's not that I don't trust valac, but I *do* have to trust it, contrary to when I write it directly in the target language where the only thing I have to trust is myself minus my ability to understand Vala.

We need a way to document how Vala generated C API can be keep stable across Vala versions, by adding tests in Vala it self and by having a way to compare one API to another.

One option is to use generated C headers and add them to the project GIT history, so you always see how they change after they were generated. This is the easiest way.

Created attachment 366868 [details] [review]
codegen: Improved ABI stability for interfaces and abstract classes

 Now, virtual and abstract methods and properties, as long as fields
 are added to class' C struct in the same order as they are declared
 in Vala source, so in order to keep ABI programmer should take
 this in account.
    
 CCode ordering property just affect interfaces no abstract classes.
 While this is good because will not interfere with actual code
 using that annotation, virtual and abstract methods are added in the
 same order to internal virtuals array, in the same order they apear
 declared in the source code, so this change no effect interface
 declarations using this property.

I've tested locally with following code, C declarations in classes' structs are respect the order as declared in source code:

public interface Test : Object {
	public abstract void foo ();
	public abstract int kfoo ();
	private static string field1;
	public abstract void afoo ();
	public abstract string aab ();
	public abstract string sprop { get; set; }
	public abstract size_t rprop { get; }
	public virtual bool get_field () { return field2; }
	public abstract size_t xprop { get; set; }
	public abstract void bfoo ();
	private static bool field2;
	public abstract int zprop { get; set; }
	public abstract int cfoo ();
}

public abstract class TestClass : Object {
	public abstract void foo ();
	public abstract int kfoo ();
	public string field1;
	public abstract void afoo ();
	public abstract string aab ();
	public abstract string sprop { get; set; }
	public abstract size_t rprop { get; }
	public virtual bool get_field () { return field2; }
	public abstract size_t xprop { get; set; }
	public abstract void bfoo ();
	public bool field2;
	public abstract int zprop { get; set; }
	public abstract int cfoo ();
}

[Compact]
public abstract class TestAClass {
	public abstract void foo ();
	public abstract int kfoo ();
	public string field1;
	public abstract void afoo ();
	public abstract string aab ();
	public abstract string sprop { get; set; }
	public abstract size_t rprop { get; }
	public virtual bool get_field () { return field2; }
	public abstract size_t xprop { get; set; }
	public abstract void bfoo ();
	public bool field2;
	public abstract int zprop { get; set; }
	public abstract int cfoo ();
}

I've added error reports if a field in an interface is not declared as static and private.

Review of attachment 366868 [details] [review]:

As we discussed this should be be conditional and split up into logical parts.
Please watch your code-style/indents and don't add VIM headers.

The symbols which needs to be handled are *all* public/protected abstract/virtual ones (properties, methods and signals), and fields if needed.

CodeContext
	* Add new compiler-flag and boolean setting "--abi-stability"
ObjectTypeSymbol
	* Add support for /*< public >*/ and /*< private >*/ sections in public class-structs
	* Add support of padding, maybe something like
		[CCode (is_padding = true)]
		public void* padding[12];
	* unify internal management of child symbols,
	  add get_symbols() which returns list
Interface/Class
	* Move lists up to ObjectTypeSymbol,
	  keep public API to retrieve symbol-specific lists
GTypeModule/BaseModule/...
	* Switch to new ObjectTypeSymbol API

(Every mark is meant to be one commit, the codegen change are likely more complex and maybe split up)

(In reply to Daniel Espinosa from comment #9)
> I've added error reports if a field in an interface is not declared as
> static and private.

Why is this needed? Afaics those are already handled and rejected by an existing error.

(In reply to Rico Tzschichholz from comment #11)
> (In reply to Daniel Espinosa from comment #9)
> > I've added error reports if a field in an interface is not declared as
> > static and private.
> 
> Why is this needed? Afaics those are already handled and rejected by an
> existing error.

I avoid to take time on this, just I will test it and propose a patch if not implemented.

Created attachment 366897 [details] [review]
vala: Moving methods/fields/properties/signals lists from class and interface to ObjectTypeSymbol

This patch move lists from class and interface vala objects to ObjectTypeSymbol, keeping API no ABI.

Added a new list with all methods/fields/properties/signals of
class and interfaces objects, as a first step to keep declareation
ordering.

No code generator's behavior change is made here.

Created attachment 366898 [details] [review]
vala: Added --abi-stability switch for future ABI stability
 implementation

This patch just add --abi-stability to valac and abi_stability property to CodeContext.

This switch is ignored by default and is added for future implementations.

Review of attachment 366898 [details] [review]:

Don't restrict this to --library, this should always apply if wanted.

There are quite a lot spelling mistakes.

Created attachment 366918 [details] [review]
compiler: Add --abi-stability option for future ABI stability enablement

Created attachment 366919 [details] [review]
vala: Move member lists from Class/Interface up to ObjectTypeSymbol

Members are fields, methods, properties and signals

(In reply to Rico Tzschichholz from comment #16)
> Created attachment 366918 [details] [review] [review]
> compiler: Add --abi-stability option for future ABI stability enablement

There is no padding CCode attribute detail - so the text can be misleading. Amended copy:

'''
This changes the current behaviour to output public members of classes and interfaces in the same order as they appear in Vala source. For libraries it is recommended to use \fB--abi-stability\fR to ensure the maintainability of the resulting Application Binary Interface (ABI).
It is also highly recommended to use this option with the \fBordering\fR CCode attribute detail.

This option is disabled by default for backward compatibility because it can break ABI of existing projects.
'''

This also uses consistent terminology - attribute instead of annotation. Attribute is in the Vala source code and the manual. Removes unusual hyphenation. Defines the abbreviation ABI on first use as is common practise in technical documentation.

I think there also need to be further discussion on how this option becomes the default. Once a working set of patches is available then maybe take the discussion to the mailing list to get feedback from any Vala library writers. libvala doesn't guarantee ABI compatibility. We need to see how this affects libgee. libvaladate is in a state of limbo. libvalum is another library I know of.

One option would be to mark this option as experimental for a release cycle so people can test it and make any adjustments to their projects. Then make it the default and provide and --old-abi-ordering option for those who have not adjusted their projects.

I will try to collect the patches in
https://git.gnome.org/browse/vala/log/?h=wip/abi

Created attachment 366950 [details] [review]
compiler: Add --abi-stability option for future ABI stability enablement

Created attachment 366965 [details] [review]
codegen: Moved struct method generation to private method

This a preparation to introduce a new path for ABI stability

Created attachment 366966 [details] [review]
codegen: Moved class virtual/abstract properties generation to private method

This is the second step in preparation to add ABI stability code path, this patch depend on patch # 366919

Created attachment 366967 [details] [review]
codegen: Moved class field generation to private method

This the third step in order to introduce new ABI stability code path.

This patch depends on patch # 366966.

@daniel: The way you set has_struct_member is not correct anymore. It is 
*only* set by generate_struct_method_declaration()

Created attachment 366970 [details] [review]
codegen: Fixed detection of struct emty structs

A maintenance patch depending on 366967 patch.

Created attachment 366973 [details] [review]
codegen: added ABI stability code path for Class object's struct generation

This is the fifth patch on ABI stability, depending on patch # 366970.

This adds ABI stability path for a Class, standard or virtual/abstract. Supports Compact Classes too.

Created attachment 366979 [details] [review]
vala: Enable ABI stability for Interface if requested

This is the last patch of the series, to implement ABI stability, unless for ordering of methods and properties.

This work is on Interface object. CodeContext is not available in an Interface, so a new constructor is used when --abi-stability is used, so now is possible to populate virtuals lists in the order they are declared in the source code.

virtuals lists, is reordered in case CCode ordering is set, so if used, source code ordering, when using --abi-stability is used, is ignored to follow attributes ordering declaration.

With or Without --abi-stability, CCode ordering attribute have effect in interfaces, no abstract classes.

Without --abi-stability code generator's behavior is to use original ordering.

Review of attachment 366979 [details] [review]:

Try to conditionally use get_members() for interfaces too.
We should not start to carry a CodeContext object around (again).

A test in this form has no value. so this can be dropped.
Making use of the new attributes while comparing the final struct sizes would make sense, but I think it isn't possible currently.

Created attachment 366999 [details] [review]
codegen: Factor-out generators for struct entries of class members

This way they can be reused in the abi-stability paths later.

Created attachment 367000 [details] [review]
codegen: Add abi-stability path for classes

Attachment 366919 [details] pushed as ce9af51 - vala: Move member lists from Class/Interface up to ObjectTypeSymbol
Attachment 366999 [details] pushed as 7700a0d - codegen: Factor-out generators for struct entries of class members

Created attachment 367009 [details] [review]
codegen: Add abi-stability path for interfaces

Review of attachment 367000 [details] [review]:

You have missed authoring information, as for other bugs and discussed on IRC

Review of attachment 367009 [details] [review]:

You have missed authoring information, as for other bugs and discussed on IRC

(In reply to Daniel Espinosa from comment #33)
> Review of attachment 367000 [details] [review] [review]:
> 
> You have missed authoring information, as for other bugs and discussed on IRC

I have taken credit since I already fixed a bunch of things in the two committed patches attributed to you and this also contains changes.

(In reply to Daniel Espinosa from comment #34)
> Review of attachment 367009 [details] [review] [review]:
> 
> You have missed authoring information, as for other bugs and discussed on IRC

How so? This isn't based your patch.

Attachment 366950 [details] pushed as 82bd593 - compiler: Add --abi-stability option for future ABI stability enablement
Attachment 367000 [details] pushed as 04f8058 - codegen: Add abi-stability path for classes
Attachment 367009 [details] pushed as aa2803c - codegen: Add abi-stability path for interfaces

Created attachment 367151 [details] [review]
codegen: Add "padding" CCode attributes for classes/interfaces

New CCode attribute are:
 * "class_padding" and "instance_padding" of integer type
 * "class_padding_ctype" and "instance_padding_ctype" of string type

The *_ctype attributes default to "void *" and for *_padding > 0 a field
will be appended to the respective struct, e.g. "void * _vala_padding[3];".

Created attachment 367152 [details] [review]
compiler: Implicitly enable hide-internal with abi-stability

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/vala/issues/598.