After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 788837 - valabind can't build on vala 0.38 because it requires CCodeBaseModule (aka codegen apis)
valabind can't build on vala 0.38 because it requires CCodeBaseModule (aka co...
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: general
0.38.x
Other Mac OS
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2017-10-11 16:22 UTC by pancake
Modified: 2018-01-19 12:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Make some methods in CCodeBaseModule public to support Valabind (131.22 KB, patch)
2017-10-12 18:52 UTC, Al Thomas
none Details | Review
codegen: Install as private library for sharing between internal components (1.33 KB, patch)
2017-10-13 14:16 UTC, Rico Tzschichholz
committed Details | Review
codegen: Factor out public ccode attribute getters (96.97 KB, patch)
2017-10-14 18:27 UTC, Rico Tzschichholz
committed Details | Review

Description pancake 2017-10-11 16:22:25 UTC
Related issue: https://github.com/radare/valabind/issues/43
Comment 1 Al Thomas 2017-10-11 18:44:14 UTC
This is a problem caused by making the ccode and codegen APIs internal. See - https://git.gnome.org/browse/vala/commit/?id=c9aa4716b2645af40080bd6523065e78fceba3b4

This has also caused a problem for Anjuta: https://bugzilla.gnome.org/show_bug.cgi?id=786973

Anjuta was using CodeBaseModule.get_ccode_lower_case_prefix ()

From the Valabind issue it looks like the following are being used:

 - CCodeBaseModule.get_ccode_header_filenames ()
 - CCodeBaseModule.get_ccode_name ()
 - CCodeBaseModule.is_reference_counting ()
 - CCodeBaseModule.get_ccode_unref_function ()
 - CCodeBaseModule.get_ccode_free_function ()
 - CCodeBaseModule.get_ccode_prefix ()

All of these, apart from is_reference_counting (), are simple wrappers around the method:
public static CCodeAttribute get_ccode_attribute (CodeNode node) {}

All of these are static methods. It looks as though they are a way of getting the CCode attribute and relevant detail from the Vala source file, e.g. [CCode (cheader_filename = "test.h")]. I'm not sure if Vala modifies these during the compilation process. If not, then the functions are a way of querying the Vala abstract syntax tree as created by the parser.

There are a couple of options to fix this. Firstly, after a good understanding of the use cases have been gained it might be best to develop a new, higher level, API that allows extracting C names from the Vala AST. This may be useful to IDEs and other tools, such as third party binding generators (e.g. valabind). The current Vala project plans appear to be switching to Meson, including any subsequent restructuring of the filenames and layout, and development of the test infrastructure. Tools using libvala, libvaladoc, like an implementation of a language server protocol for IDEs, are a bit further off. So maybe it is better to put off creating a new API.

The second option is to make these few methods public again. This could be done on a very limited basis by making all the other methods in CCodeBaseModule internal. The main problem is how this fits into the current build system.
Comment 2 pancake 2017-10-11 23:49:09 UTC
another solution would be to provide a libvala-private.so accessible with --pkg vala-private to use those apis, i assume api rewrite will take more time.
Comment 3 Al Thomas 2017-10-12 10:51:49 UTC
(In reply to pancake from comment #2)
> another solution would be to provide a libvala-private.so accessible with
> --pkg vala-private to use those apis, i assume api rewrite will take more
> time.

I don't understand the advantage of that approach. You are suggesting a separate build artefact called libvala-private.so and interface file vala-private.vapi. These would need to be added to distributions' packages. It's essentially an interim API and means valabind would need to have conditional compilation for those blocks and wrap them in #if VALA_0_38 #endif. Then if a new API was developed you would have to add another conditional compilation block on top.

The basic function Valabind is using in CCodeBaseModule is:

public static CCodeAttribute get_ccode_attribute (CodeNode node) {
	var attr = node.get_attribute_cache (ccode_attribute_cache_index);
	if (attr == null) {
		attr = new CCodeAttribute (node);
		node.set_attribute_cache (ccode_attribute_cache_index, attr);
	}
	return (CCodeAttribute) attr;
}

and then the wrapper functions access the properties of CCodeAttribute. For a moment I had thought it might be possible for Valabind to incorporate this into its own code base because CodeNode is currently part of Vala's public API. CCodeAttribute is, however, part of the internal API. See codegen/valaccodeattribute.vala 

CCodeAttribute accesses the CCode attribute details in the source code, but also provides a default value if there is no value given in the source. e.g. the header_filenames property looks for the cheader_filename detail in [CCode] and then calls get_default_header_filenames () if there is none. So it wouldn't be possible to incorporate this code into Valabind directly. A Vala API is needed and probably the simplest way is to re-open the small set of CCodeBaseModule functions being used.
Comment 4 pancake 2017-10-12 11:53:43 UTC
Making those APIs available again would be another good solution for me. At least it's simpler to support multiple versions of Vala, but after breaking them in 0.38 the mess is has been done already. If you think i can "reimplement" all those functions without having access to any of the currently hidden functions maybe having a .vala file i can ship with valabind would work too.
Comment 5 Al Thomas 2017-10-12 12:17:37 UTC
(In reply to pancake from comment #4)
> Making those APIs available again would be another good solution for me. At
> least it's simpler to support multiple versions of Vala, but after breaking
> them in 0.38 the mess is has been done already. 

If they are made public again then it would be seen as a bug in 0.38.0 and fixed in a later point release, like 0.38.3 or 0.38.4. Valabind wouldn't need to change, just point out it needs to be built with a newer version of 0.38. It's not ideal, but is the pragmatic solution.


> If you think i can
> "reimplement" all those functions without having access to any of the
> currently hidden functions maybe having a .vala file i can ship with
> valabind would work too.

That would be the ideal solution - so the codegen and ccode APIs remain fully internal and valabind builds again. The trouble is methods of CCodeAttribute, like get_default_header_filenames () and get_default_name () are private methods within the CCodeAttribute class.
Comment 6 Al Thomas 2017-10-12 18:52:56 UTC
Created attachment 361449 [details] [review]
Make some methods in CCodeBaseModule public to support Valabind

This patch is the first step in allowing Valabind to continue to use some of the CCodeBaseModule methods. There also needs to be:

 - a patch to the build system so codegen is made public again
 - further patches to codegen to mark methods as internal to minimize the public interface

There are a number of methods in CCodeBaseModule, for example get_ccode_ref_function (), that aren't showing in the current bug report from Valabind. If Valabind is using a different way of getting those values then that should be used instead of making these CCodeBaseModule methods public again.
Comment 7 Rico Tzschichholz 2017-10-13 08:02:46 UTC
I would prefer a new wrapper which can be linked into libvala and uses a private library which contains the current ccode/codegen source.
Comment 8 Rico Tzschichholz 2017-10-13 14:16:16 UTC
Created attachment 361528 [details] [review]
codegen: Install as private library for sharing between internal components
Comment 9 Rico Tzschichholz 2017-10-14 18:27:13 UTC
Created attachment 361590 [details] [review]
codegen: Factor out public ccode attribute getters
Comment 10 Rico Tzschichholz 2017-11-19 14:03:05 UTC
Attachment 361528 [details] pushed as a470946 - codegen: Install as private library for sharing between internal components
Comment 11 pancake 2017-12-27 23:43:22 UTC
I have managed to build valabind against vala-0.39.2, but i had to refer to the vala source directory (hardcoded for now), this is the commit: 

https://github.com/radare/valabind/commit/b1611959b372844da3a9008e1f4fe83d67a39b1a

it will be great if vala-0.40 can expose the missing pieces to remove the hacky ifeq in the makefile. for backward compat with CCodeBaseModeule -> Vala i don't care too much and i assume that's soemthing internal.

The codegen and ccode vapis and include files should be installed. that's why i have to refer the source directory
Comment 12 Daniel Espinosa 2017-12-29 06:02:40 UTC
Are there any reason to keep this as a private library?

May is better to create a public library to allow reusable code by other projects, like Anjuta and Valabind.

May take a look to other parts in Vala as candidate to be part of a public library to expose it's features to new projects, like:

a) Vala Language Server (for IEDs and compilers)
b) Documentation provider (for IEDs)
c) Macro to C language generator (for compilers, like the one for Rust)
d) Intermidiate Language Representation (For parsers, compilers and Macro Languages)

All this pieces will be used by valac and for any other effort to take advantage of Vala features, like profiles, without distract Vala advances in its own.
Comment 13 Rico Tzschichholz 2018-01-19 12:19:29 UTC
Closing this one in reference to https://github.com/radare/valabind/issues/43