Re: [waffle] [PATCH 06/12] wflinfo: add option for JSON output

[Chad Versace]

On Thu 21 Apr 2016, Frank Henigman wrote:
> On Fri, Apr 8, 2016 at 8:14 PM, Chad Versace  wrote:
> > On 01/06/2016 11:56 AM, Frank Henigman wrote:

> I really hope we can keep this simple.
> 
> I mentioned in the cover letter to this series that the next step is
> to gut wflinfo and translate json to the old format.  We can insert
> that after this patch if you like.  Option 1 is the nicest by far, if
> the translator is written in python.  It's just a few lines of code to
> read the json, plus a table that lists the correspondence between old
> format lines and json keys.  So we have some really simple C code to
> call the waffle api and get the json, and a really simple python
> script to translate to old format.  I've got this much roughed out.
> For maximum backward compatibility these two bits would be tied
> together by a script that accepts all the options currently accepted
> by wflinfo, i.e. we replace wflinfo with a script.  Which for
> portability is also written in python.  Yes this introduces a
> dependency on python to run wflinfo, but I doubt that's much of a
> problem.

I'm open to exploring that path. I just took a serious look at
wflinfo.c, and it's needless complex for the little job that it does. If
you could port it to Python, greatly reducing and simplifying the code,
I'd like to see that.

I've never used Python's C interface, so I look forward to learning
about it from your patches.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH 06/12] wflinfo: add option for JSON output

[Frank Henigman]

On Fri, Apr 8, 2016 at 8:14 PM, Chad Versace  wrote:
> On 01/06/2016 11:56 AM, Frank Henigman wrote:
>> With "-f json" wflinfo will now print the result of
>> waffle_display_info_json().
>>
>> Signed-off-by: Frank Henigman 
>> ---
>>  src/utils/wflinfo.c | 40 
>>  1 file changed, 36 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/utils/wflinfo.c b/src/utils/wflinfo.c
>> index 268d4b8..58f5688 100644
>> --- a/src/utils/wflinfo.c
>> +++ b/src/utils/wflinfo.c
>
>
>> @@ -1119,9 +1137,23 @@ main(int argc, char **argv)
>>  glGetStringi = waffle_get_proc_address("glGetStringi");
>>  }
>>
>> -ok = print_wflinfo();
>> -if (!ok)
>> -error_waffle();
>> +char *info;
>> +switch (opts.format) {
>> +case FORMAT_ORIGINAL:
>> +ok = print_wflinfo();
>> +if (!ok)
>> +error_waffle();
>> +break;
>> +
>> +case FORMAT_JSON:
>> +info = waffle_display_info_json(dpy, false);
>> +if (info) {
>> +printf("%s\n", info);
>> +free(info);
>> +} else
>> +error_waffle();
>> +break;
>> +}
>
>
> I agree with the patch series's goals: teach wflinfo to print 
> platform-specific
> info (such as EGL info); extract that info inside libwaffle and expose it
> through public API; that public API should provide JSON, and perhaps 
> additional
> formats.
>
> But this patch 6/12 poses a problem. Post-patch, wflinfo can print its info in
> the old format and in a json format. But, depending on the output format,
> wflinfo uses completely separate code paths to obtain the information.
>
> The actual information that wflinfo provides, and the method by which it
> obtains that info, should be largely independent of the output format. When 
> the
> user chooses an output format, that choice should affect the *presentation* 
> but
> not fundamentally affect the *content*.
>
> So...
>
> For wflinfo's sake, we need a public waffle_get_my_info() function that 
> returns
> data that can be translated into the old format and into the json format, or 
> perhaps
> returns the old format and json directly.
>
> To move forward with this patch series, I see the following options:
>
> 1. waffle_get_my_info() returns only json
>
>   I don't like this. This would require that wflinfo decode the json in
>   order to provide the old format output. I would like to avoid decoding
>   json in Waffle though, because that would require either (a) Waffle
>   contain a json decoder implementation or (b) Waffle rely on some json
>   library. As for (a), I don't want to maintain a json-decoder (a json
>   *encoder*, though, I don't object to). As for (b), Waffle is such
>   a small, focused project that it feels wrong to pull in a json-decoder
>   library as a dependency.
>
>   BUT, maybe this option could work and I'm overestimating the maintenance
>   overhead of decoding json.
>
> 2. waffle_get_my_info() returns only some-other-well-defined-format-FOO
>
>I don't like this either. Just like option 1, it would require that
>wflinfo decode the FOO format in order to provide json output.
>
> 3. waffle_get_my_info() can return a json string or a string in the old
>wflinfo format
>
>If option 3 can be implemented cleanly, I think it's the best choice.
>This option eliminates the need for decoding any special format.
>
>Perhaps we could implement this by defining a private struct...
>
> struct wcore_display_info {
> struct {
> char *platform;
> char *api;
> } waffle;
>
> struct {
> char *vendor;
> char *renderer;
> ...
> char **extensions;
> } opengl;
>
> struct {
> char *version;
> char *vendor;
> char *client_apis;
> ...
> } egl;
>
> ...
> };
>
> ...that waffle_get_my_info() populates and then encodes into the
> respected format: JSON, the old wflinfo format, a format that
> mimics glxinfo, or whatever.
>
>
> Frank, what do you think?

I really hope we can keep this simple.

I mentioned in the cover letter to this series that the next step is
to gut wflinfo and translate json to the old format.  We can insert
that after this patch if you like.  Option 1 is the nicest by far, if
the translator is written in python.  It's just a few lines of code to
read the json, plus a table that lists the correspondence between old
format lines and json keys.  So we have some really simple C code to
call the waffle api and get the json, and a really simple 

Re: [waffle] [PATCH 06/12] wflinfo: add option for JSON output

[Chad Versace]

On 01/06/2016 11:56 AM, Frank Henigman wrote:
> With "-f json" wflinfo will now print the result of
> waffle_display_info_json().
> 
> Signed-off-by: Frank Henigman 
> ---
>  src/utils/wflinfo.c | 40 
>  1 file changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/src/utils/wflinfo.c b/src/utils/wflinfo.c
> index 268d4b8..58f5688 100644
> --- a/src/utils/wflinfo.c
> +++ b/src/utils/wflinfo.c


> @@ -1119,9 +1137,23 @@ main(int argc, char **argv)
>  glGetStringi = waffle_get_proc_address("glGetStringi");
>  }
>  
> -ok = print_wflinfo();
> -if (!ok)
> -error_waffle();
> +char *info;
> +switch (opts.format) {
> +case FORMAT_ORIGINAL:
> +ok = print_wflinfo();
> +if (!ok)
> +error_waffle();
> +break;
> +
> +case FORMAT_JSON:
> +info = waffle_display_info_json(dpy, false);
> +if (info) {
> +printf("%s\n", info);
> +free(info);
> +} else
> +error_waffle();
> +break;
> +}


I agree with the patch series's goals: teach wflinfo to print platform-specific
info (such as EGL info); extract that info inside libwaffle and expose it
through public API; that public API should provide JSON, and perhaps additional
formats.

But this patch 6/12 poses a problem. Post-patch, wflinfo can print its info in
the old format and in a json format. But, depending on the output format,
wflinfo uses completely separate code paths to obtain the information.

The actual information that wflinfo provides, and the method by which it
obtains that info, should be largely independent of the output format. When the
user chooses an output format, that choice should affect the *presentation* but
not fundamentally affect the *content*.

So...

For wflinfo's sake, we need a public waffle_get_my_info() function that returns
data that can be translated into the old format and into the json format, or 
perhaps
returns the old format and json directly.

To move forward with this patch series, I see the following options:

1. waffle_get_my_info() returns only json

  I don't like this. This would require that wflinfo decode the json in
  order to provide the old format output. I would like to avoid decoding
  json in Waffle though, because that would require either (a) Waffle
  contain a json decoder implementation or (b) Waffle rely on some json
  library. As for (a), I don't want to maintain a json-decoder (a json
  *encoder*, though, I don't object to). As for (b), Waffle is such
  a small, focused project that it feels wrong to pull in a json-decoder
  library as a dependency.

  BUT, maybe this option could work and I'm overestimating the maintenance
  overhead of decoding json.

2. waffle_get_my_info() returns only some-other-well-defined-format-FOO

   I don't like this either. Just like option 1, it would require that
   wflinfo decode the FOO format in order to provide json output.

3. waffle_get_my_info() can return a json string or a string in the old
   wflinfo format

   If option 3 can be implemented cleanly, I think it's the best choice.
   This option eliminates the need for decoding any special format.

   Perhaps we could implement this by defining a private struct...

struct wcore_display_info {
struct {
char *platform;
char *api;
} waffle;

struct {
char *vendor;
char *renderer;
...
char **extensions;
} opengl;

struct {
char *version;
char *vendor;
char *client_apis;
...
} egl;

...
};

...that waffle_get_my_info() populates and then encodes into the
respected format: JSON, the old wflinfo format, a format that
mimics glxinfo, or whatever.


Frank, what do you think?
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle