New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 757584 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Last visit 15 days ago
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Task



Sign in to add a comment

Remove dbus-c++ and glib dependencies from mtpd

Project Member Reported by ejcaruso@chromium.org, Aug 21 2017

Issue description

In the interest of getting rid of dbus-c++ and making this code more future-proof, we should serve requests via DBusServiceDaemon and libchrome's dbus bindings.
 
Labels: -Type-Bug Type-Task
Cc: thestig@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/be91a8d1ff56b98045abb63258762aa8e5954a9d

commit be91a8d1ff56b98045abb63258762aa8e5954a9d
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Thu Aug 24 22:15:02 2017

mtpd: build using gyp

CQ-DEPEND=CL:624859
BUG= chromium:757584 
TEST=emerge

Change-Id: I6f63eff3720e86e1202a67f6fa6db1ab789c39b7
Reviewed-on: https://chromium-review.googlesource.com/624910
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/be91a8d1ff56b98045abb63258762aa8e5954a9d/chromeos-base/mtpd/mtpd-9999.ebuild

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mtpd/+/34eeec5b0008e651886a79b5b8bbd5f44f765602

commit 34eeec5b0008e651886a79b5b8bbd5f44f765602
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Thu Aug 24 22:15:02 2017

mtpd: add gyp file and move files to canonical locations

The gyp file is easier to understand than the makefile and will
make it easier to refactor. Also, move D-Bus configuration to
separate directories much like it is in other services that use
D-Bus and remove the old makefile since we're not using it
anymore.

CQ-DEPEND=CL:624910
BUG= chromium:757584 
TEST=emerge-cyan mtpd; FEATURES=test emerge-cyan mtpd

Change-Id: I56121be11221aea5718f0a0daf012e29abc88a0c
Reviewed-on: https://chromium-review.googlesource.com/624859
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[rename] https://crrev.com/34eeec5b0008e651886a79b5b8bbd5f44f765602/dbus_bindings/org.chromium.Mtpd.xml
[delete] https://crrev.com/17cc088dc198f626513d4154644d57553b69fefd/mtpd_testrunner.cc
[modify] https://crrev.com/34eeec5b0008e651886a79b5b8bbd5f44f765602/mtpd_server_impl.h
[rename] https://crrev.com/34eeec5b0008e651886a79b5b8bbd5f44f765602/dbus/org.chromium.Mtpd.conf
[delete] https://crrev.com/17cc088dc198f626513d4154644d57553b69fefd/Makefile
[add] https://crrev.com/34eeec5b0008e651886a79b5b8bbd5f44f765602/PRESUBMIT.cfg
[modify] https://crrev.com/34eeec5b0008e651886a79b5b8bbd5f44f765602/assert_matching_file_types.cc
[add] https://crrev.com/34eeec5b0008e651886a79b5b8bbd5f44f765602/mtpd.gyp
[modify] https://crrev.com/34eeec5b0008e651886a79b5b8bbd5f44f765602/file_entry.h
[modify] https://crrev.com/34eeec5b0008e651886a79b5b8bbd5f44f765602/storage_info.cc
[delete] https://crrev.com/17cc088dc198f626513d4154644d57553b69fefd/common.mk

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mtpd/+/e7cd2685731a9d60b8330028e01051f529fab4c0

commit e7cd2685731a9d60b8330028e01051f529fab4c0
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Thu Aug 24 22:15:03 2017

mtpd: prepend mtpd/ to all local includes

Since we're pulling in headers from the gen directory and also
from other packages (system_api), it makes sense to be explicit
about the fact that the other headers are also coming from the
mtpd directory instead of relying on the fact that they are in
the current directory during compilation.

This also allows us to alphabetize the includes better.

BUG= chromium:757584 
TEST=emerge-cyan mtpd; FEATURES=test emerge-cyan mtpd

Change-Id: I3837f40961ca1bf6f6e1c6531a5f12188e62cf10
Reviewed-on: https://chromium-review.googlesource.com/625054
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/e7cd2685731a9d60b8330028e01051f529fab4c0/string_helpers.cc
[modify] https://crrev.com/e7cd2685731a9d60b8330028e01051f529fab4c0/mtpd_server_impl.cc
[modify] https://crrev.com/e7cd2685731a9d60b8330028e01051f529fab4c0/daemon.cc
[modify] https://crrev.com/e7cd2685731a9d60b8330028e01051f529fab4c0/daemon.h
[modify] https://crrev.com/e7cd2685731a9d60b8330028e01051f529fab4c0/mtpd_server_impl.h
[modify] https://crrev.com/e7cd2685731a9d60b8330028e01051f529fab4c0/storage_info.cc
[modify] https://crrev.com/e7cd2685731a9d60b8330028e01051f529fab4c0/main.cc
[modify] https://crrev.com/e7cd2685731a9d60b8330028e01051f529fab4c0/device_manager_unittest.cc
[modify] https://crrev.com/e7cd2685731a9d60b8330028e01051f529fab4c0/device_manager.h
[modify] https://crrev.com/e7cd2685731a9d60b8330028e01051f529fab4c0/device_manager.cc
[modify] https://crrev.com/e7cd2685731a9d60b8330028e01051f529fab4c0/file_entry.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/mtpd/+/2e3c3ca3b3a6add246f8e4bd314be5a070a85603

commit 2e3c3ca3b3a6add246f8e4bd314be5a070a85603
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Mon Aug 28 22:24:10 2017

mtpd: remove glib and dbus-c++ dependency

Refactors a few things to remove dependencies:
* Move our D-Bus bindings from dbus-c++ to libbrillo
* Move our D-Bus binding generator from xml2cpp to
  chromeos-dbus-bindings
* Move all remaining uses of glib and its message loops
  to libchrome message loops.

BUG= chromium:757584 
TEST=emerge, unit tests, connect MTP device to DUT and verify
  that the files app opens up and I can get images

Change-Id: I5483b1da74975daea07d443f7b5c97da67b7b764
Reviewed-on: https://chromium-review.googlesource.com/627261
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/2e3c3ca3b3a6add246f8e4bd314be5a070a85603/dbus_bindings/org.chromium.Mtpd.xml
[modify] https://crrev.com/2e3c3ca3b3a6add246f8e4bd314be5a070a85603/mtpd_server_impl.cc
[delete] https://crrev.com/e7cd2685731a9d60b8330028e01051f529fab4c0/daemon.cc
[delete] https://crrev.com/e7cd2685731a9d60b8330028e01051f529fab4c0/daemon.h
[modify] https://crrev.com/2e3c3ca3b3a6add246f8e4bd314be5a070a85603/mtpd_server_impl.h
[add] https://crrev.com/2e3c3ca3b3a6add246f8e4bd314be5a070a85603/dbus_bindings/dbus-service-config.json
[modify] https://crrev.com/2e3c3ca3b3a6add246f8e4bd314be5a070a85603/main.cc
[modify] https://crrev.com/2e3c3ca3b3a6add246f8e4bd314be5a070a85603/device_manager.h
[modify] https://crrev.com/2e3c3ca3b3a6add246f8e4bd314be5a070a85603/mtpd.gyp
[modify] https://crrev.com/2e3c3ca3b3a6add246f8e4bd314be5a070a85603/device_manager.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/fb2236b026bf317d09030477bc8d17423c271439

commit fb2236b026bf317d09030477bc8d17423c271439
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Mon Aug 28 22:24:11 2017

mtpd: remove dbus-c++ and glib dependencies

CQ-DEPEND=CL:627261
BUG= chromium:757584 
TEST=emerge

Change-Id: I664dc2cefc846e40ce0ee98e7b62de642d84f4fa
Reviewed-on: https://chromium-review.googlesource.com/634272
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/fb2236b026bf317d09030477bc8d17423c271439/chromeos-base/mtpd/mtpd-9999.ebuild

Status: Fixed (was: Assigned)

Comment 9 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment