Merge into development-branch : mirout-save-load : Code : Mir

Status:	Merged
Approved by:	Daniel van Vugt on 2017-02-14
Approved revision:	no longer in the source branch.
Merged at revision:	4026
Proposed branch:	lp:~vanvugt/mir/mirout-save-load
Merge into:	lp:mir
Diff against target:	170 lines (+108/-5) 1 file modified src/utils/out.c (+108/-5)
To merge this branch:	bzr merge lp:~vanvugt/mir/mirout-save-load
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Mir CI Bot	continuous-integration		Approve on 2017-02-14
Cemil Azizoglu (community)			Approve on 2017-02-10
Kevin DuBois (community)		2017-02-09	Needs Information on 2017-02-09
Daniel van Vugt			Abstain on 2017-02-09
Review via email: mp+316676@code.launchpad.net

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-02-08:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2952/rebuild

PASSED: Continuous integration, rev:4019
https://mir-jenkins.ubuntu.com/job/mir-ci/2952/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/3912
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3998
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3988
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3988
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/3988
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3939
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3939/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3939
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3939/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3939
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3939/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3939
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3939/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3939
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3939/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3939
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3939/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2952/rebuild

review: Approve (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2017-02-08:

#

Got a better idea...

review: Needs Fixing

Revision history for this message

Daniel van Vugt (vanvugt) on 2017-02-09:

#

review: Abstain

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-02-09:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2960/rebuild

PASSED: Continuous integration, rev:4022
https://mir-jenkins.ubuntu.com/job/mir-ci/2960/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/3928
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4014
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4004
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4004
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4004
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3955
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/3955/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3955
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3955/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3955
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/3955/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3955
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3955/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3955
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3955/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3955
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3955/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/2960/rebuild

review: Approve (continuous-integration)

Revision history for this message

Kevin DuBois (kdub) wrote on 2017-02-09:

#

alright as is, although I've forgotten why we need MirBlob as part of the client api at all. Seems it would be be better for this client to come up with its own data format (csv? xml?) than rely on mirblob.

Suppose, needs discussion on that point, otherwise lgtm

review: Needs Information

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2017-02-10:

#

I wondered that myself. Why does MirBlob exist?...

MirBlob has one benefit in that it is a single object with data and length fields. If MirBlob did not exist then the only real alternative is to pass around two variables instead:
void *data;
size_t size;
Which is basically what MirBlob is.

Your question however is also around data formats. That's a different question and the answer is that MirBlob is not a data format. The data format of the files here is from the Protobuf serialization function:
https://developers.google.com/protocol-buffers/docs/reference/cpp/google.protobuf.message_lite#MessageLite.SerializeWithCachedSizes

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2017-02-10:

#

> alright as is, although I've forgotten why we need MirBlob as part of the
> client api at all. Seems it would be be better for this client to come up with
> its own data format (csv? xml?) than rely on mirblob.
>
> Suppose, needs discussion on that point, otherwise lgtm

The requirement comes from a compromise at a sprint a couple of years ago.

Downstream didn't want to deal with tracking any changes to the display configuration information or backward compatibility for the serialized form. So we had to provided one.

Despite the urgency and shouting I don't think they ever got around to using it. So we could probably drop it for 1.0.

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2017-02-10:

#

Ok by me

review: Approve

Revision history for this message

Kevin DuBois (kdub) wrote on 2017-02-10:

#

> Despite the urgency and shouting I don't think they ever got around to using
> it. So we could probably drop it for 1.0.

Sure, would be fine by me as well.

Revision history for this message

Kevin DuBois (kdub) wrote on 2017-02-10:

#

> I wondered that myself. Why does MirBlob exist?...
>
> MirBlob has one benefit in that it is a single object with data and length
> fields. If MirBlob did not exist then the only real alternative is to pass
> around two variables instead:
> void *data;
> size_t size;
> Which is basically what MirBlob is.
>
> Your question however is also around data formats. That's a different question
> and the answer is that MirBlob is not a data format. The data format of the
> files here is from the Protobuf serialization function:
> https://developers.google.com/protocol-buffers/docs/reference/cpp/google.pr
> otobuf.message_lite#MessageLite.SerializeWithCachedSizes

Well, its more of compatibility of the file this utility produces.

If a user produces a file with this utility against mir 1.0.0
Then we change the internal structure of MirBlob in mir 1.x (as the internal structure isn't mirclient abi, this is something we could do)
Then the user tries to load the file with the mir 1.x utility, seems that this has a chance for breakage.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-02-13:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2017-02-13:

#

^^^
Ongoing network/infrastructure problems today. But also bug 1646375.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2017-02-13:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2017-02-14:

#

^^^
More network failures

Revision history for this message

Mir CI Bot (mir-ci-bot) on 2017-02-14:

#

review: Approve (continuous-integration)

 === modified file 'src/utils/out.c'
 --- src/utils/out.c	2017-01-18 02:29:37 +0000
 +++ src/utils/out.c	2017-02-09 07:47:02 +0000
@@ -17,10 +17,12 @@
   */
  #include "mir_toolkit/mir_client_library.h"
++#include "mir_toolkit/mir_blob.h"
  #include <stdio.h>
  #include <math.h>
  #include <string.h>
  #include <stdbool.h>
++#include <stdlib.h>
  static const char *power_mode_name(MirPowerMode m)
+ {
@@ -88,6 +90,79 @@
                                                          : "out-of-range";
+ }
++static bool save(MirDisplayConfig* conf, char const* path)
++{
++    MirBlob* blob = mir_blob_from_display_config(conf);
++    if (!blob)
++    {
++        fprintf(stderr, "mir_blob_from_display_config failed\n");
++        return false;
++    }
++    bool ret = false;
++    FILE* file = !strcmp(path, "-") ? stdout : fopen(path, "wb");
++    if (!file)
++    {
++        perror("save:fopen");
++    }
++    else
++    {
++        if (1 == fwrite(mir_blob_data(blob), mir_blob_size(blob), 1, file))
++            ret = true;
++        else
++            perror("save:fwrite");
++
++        if (file != stdout)
++            fclose(file);
++    }
++    mir_blob_release(blob);
++    return ret;
++}
++
++static MirDisplayConfig* load(char const* path)
++{
++    MirDisplayConfig* conf = NULL;
++    FILE* file = !strcmp(path, "-") ? stdin : fopen(path, "rb");
++    if (!file)
++    {
++        perror("load:fopen");
++    }
++    else
++    {
++        fseek(file, 0, SEEK_END);
++        long size = ftell(file);
++        fseek(file, 0, SEEK_SET);
++        void* data = malloc(size);
++        if (!data)
++        {
++            perror("load:malloc");
++        }
++        else
++        {
++            if (1 != fread(data, size, 1, file))
++            {
++                perror("load:fread");
++            }
++            else
++            {
++                MirBlob* blob = mir_blob_onto_buffer(data, size);
++                if (!blob)
++                {
++                    fprintf(stderr, "mir_blob_onto_buffer failed with %ld bytes\n", size);
++                }
++                else
++                {
++                    conf = mir_blob_to_display_config(blob);
++                    mir_blob_release(blob);
++                }
++            }
++            free(data);
++        }
++        if (file != stdin)
++            fclose(file);
++    }
++    return conf;
++}
++
  static bool modify(MirDisplayConfig* conf, int actionc, char** actionv)
+ {
      int const max_targets = 256;
@@ -332,6 +407,8 @@
  int main(int argc, char *argv[])
+ {
      char const* server = NULL;
++    char const* infile = NULL;
++    char const* outfile = NULL;
      char** actionv = NULL;
      int actionc = 0;
      bool verbose = false;
@@ -349,13 +426,33 @@
                  case 'v':
                      verbose = true;
                      break;
++                case 'i':
++                    ++a;
++                    if (a >= argc)
++                    {
++                        fprintf(stderr, "%s requires a file path\n", arg);
++                        return 1;
++                    }
++                    infile = argv[a];
++                    break;
++                case 'o':
++                    ++a;
++                    if (a >= argc)
++                    {
++                        fprintf(stderr, "%s requires a file path\n", arg);
++                        return 1;
++                    }
++                    outfile = argv[a];
++                    break;
                  case 'h':
                  default:
                      printf("Usage: %s [OPTIONS] [/path/to/mir/socket] [[output OUTPUTID] ACTION ...]\n"
                             "Options:\n"
--                           "    -h  Show this help information.\n"
--                           "    -v  Show verbose information.\n"
--                           "    --  Ignore the rest of the command line.\n"
++                           "    -h       Show this help information.\n"
++                           "    -i PATH  Load display configuration from a file instead of the server.\n"
++                           "    -o PATH  Save resulting configuration to a file.\n"
++                           "    -v       Show verbose information.\n"
++                           "    --       Ignore the rest of the command line.\n"
                             "Actions:\n"
                             "    off | suspend | standby | on\n"
                             "    disable | enable\n"
@@ -387,12 +484,13 @@
+     }
      int ret = 0;
--    MirDisplayConfig* conf = mir_connection_create_display_configuration(conn);
++    MirDisplayConfig* conf = infile ? load(infile) :
++                             mir_connection_create_display_configuration(conn);
      if (conf == NULL)
+     {
          fprintf(stderr, "Failed to get display configuration (!?)\n");
+     }
--    else if (actionc)
++    else if (actionc || infile)
+     {
          if (modify(conf, actionc, actionv))
+         {
@@ -520,7 +618,12 @@
              if (num_modes)
                  printf("\n");
+         }
++    }
++    if (conf)
++    {
++        if (outfile && !ret)
++            ret = save(conf, outfile) ? 0 : 1;
          mir_display_config_release(conf);
+     }

Mir

Merge lp:~vanvugt/mir/mirout-save-load into lp:mir

Commit message

Description of the change

Preview Diff

Subscribers