GNOME Bugzilla – Bug 678912
Add [Version] attribute
Last modified: 2016-02-01 22:39:50 UTC
As discussed in IRC, add a [Version] attribute and deprecated [Deprecated]. [Version (since = "0.0.1", experimental = false, experimental_until = "0.1.0", deprecated_since = "0.2.0", replacement = "foo")] experimental could be true until a version is released in which the symbol is not experimental, at which time it should be replaced by experimental_until. Version numbers would refer to the upstream packages, unless prefixed with a package name (e.g., "vala-0.18"). This would allow us to use `pkg-config --modversion` to check a symbol in valac and avoid an undefined function C error. We could even add a generic --target-package argument to override `pkg-config --modversion` to help people make sure they don't accidently bump a dependency version.
*** Bug 669593 has been marked as a duplicate of this bug. ***
Created attachment 242735 [details] [review] [Version] patches
> @@ -413,6 +441,42 @@ public abstract class Vala.Symbol : CodeNode { > return isclass; > } > > + private int cmp_versions (string v1str, string v2str) { > + string[] v1arr = v1str.split ("."); > + string[] v2arr = v2str.split ("."); > + int i = 0; > + > + while (v1arr[i] != null && v2arr[i] != null) { > + int v1num = int.parse (v1arr[i]); > + int v2num = int.parse (v2arr[i]); > + > + if (v1num < 0 || v2num < 0) { > + // invalid format > + return 0; > + } > + > + if (v1num > v2num) { > + return 1; > + } > + > + if (v1num < v2num) { > + return -1; > + } > + > + i++; > + } > + > + if (v1arr[i] != null && v2arr[i] == null) { > + return 1; > + } > + > + if (v1arr[i] == null && v2arr[i] != null) { > + return -1; > + } > + > + return 0; > + } > + > /** > * Check to see if the symbol has been deprecated, and emit a warning > * if it has. Wouldn't it be beneficial to put the comparation outside private method? I will use comparation for codegen in bug #693455 and it's possibly better to keep it dry (possibly just adding a new type of attribute as I've done it but I'm not insisting. It would be good to have this code common anyway).
Instead of always checking against the installed version of packages, it would be nice to provide a way to target a specific package version much as we do with --target-glib. It's likely that people will want to target versions of packages lower than what they have installed. We could add an argument (i.e., --pkg gtk+-3.0 --target-pkg gtk+-3.0:3.8) or (ab)use the existing --pkg argument (--pkg gtk+-3.0:3.8)...
Review of attachment 242735 [details] [review]: I think we can even abuse --pkg. The patch looks fine overall, anyway a few questions arise: 1) Is it possible to merge check_experimental, check_deprecated and check_availability? 2) What about moving all the .deprecated, .experimental, .since stuff in a .version object? ::: vala/valaobjectcreationexpression.vala @@ +259,3 @@ symbol_reference.used = true; symbol_reference.check_deprecated (source_reference); + symbol_reference.check_availability (source_reference); Why don't we check_experimental here? Also, what about merging check_deprecated, check_experimental and check_availability in a unique check_version? ::: vala/valasourcefile.vala @@ +58,3 @@ + + private string? _installed_version = null; + private bool _version_reqeusted = false; Typo here, "_version_requested". ::: vala/valasymbol.vala @@ +134,3 @@ } + public string? experimental_until { What about moving all this experimental, experimental_until, replacement, and so on stuff in a public Version version object?
Created attachment 243246 [details] [review] [Version] patches Note: There is a ongoing discussion about package-targets.
Review of attachment 243246 [details] [review]: Looks fine to me, except I'd use ArgumentType.SINCE. ::: vala/valagirparser.vala @@ +53,3 @@ REPLACEMENT, DEPRECATED_SINCE, + VERSION, Can we use ArgumentType.SINCE instead?
Created attachment 243318 [details] [review] [Version] patches
close?
This work has been merged in master.