New issue
Advanced search Search tips

Issue 827221 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Nov 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 806788



Sign in to add a comment

Clean up cryptohome's GYP files

Project Member Reported by emaxx@chromium.org, Mar 29 2018

Issue description

Notably:
* Dependencies, deps and link libraries should be specified directly for the libraries that need them, instead of listing them in the "leaf" targets (executables / shared libraries).
* Deps should be correctly propagated to dependent targets (e.g., via all_dependent_settings).
* Source files shouldn't be duplicated across multiple targets.
 

Comment 1 by emaxx@chromium.org, Mar 29 2018

Blocking: 806788
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/85c87e131969d5d691cabb5ef347b2f97ee919a2

commit 85c87e131969d5d691cabb5ef347b2f97ee919a2
Author: Maksim Ivanov <emaxx@google.com>
Date: Thu Apr 12 14:59:49 2018

cryptohome: Add GYP deps for code-generated targets

Now the targets that create code-generated files provide the "deps"
specified for all dependencies of the output files. These "deps"
are set up to be propagated to the dependent targets via
"all_dependent_settings".

In particular, this change is for these targets:
* Protobuf bindings generation.
  For these, 'protobuf' is in their own deps because the symbols
  from it are used in the compiled object files.
* D-Bus bindings and adaptors generation.
  As these only generate headers and don't produce any object
  files, the libraries included from these headers are specified
  only as deps for the dependent settings, but not for the targets
  themselves.

Note that the removal of unnecessary "deps" for dependent targets
will be done in a follow-up CL.

BUG= chromium:827221 
TEST=none

Change-Id: Iffeab526a66b42ec62f6b7a23b6f59657164869f
Reviewed-on: https://chromium-review.googlesource.com/986436
Commit-Ready: Maksim Ivanov <emaxx@chromium.org>
Tested-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/85c87e131969d5d691cabb5ef347b2f97ee919a2/cryptohome/cryptohome-libs.gypi
[modify] https://crrev.com/85c87e131969d5d691cabb5ef347b2f97ee919a2/cryptohome/cryptohome.gyp

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/67e8fad7003ae7fa9b6256a1aa91f2faad0d30e7

commit 67e8fad7003ae7fa9b6256a1aa91f2faad0d30e7
Author: Maksim Ivanov <emaxx@google.com>
Date: Fri Apr 13 20:25:25 2018

cryptohome: Remove unused includes

* crypto.cc doesn't use ecryptfs and keyutils (this was likely
  moved to platform.cc);
* cryptolib.cc doesn't use scrypt (likely moved to crypto.cc).

BUG= chromium:827221 
TEST=existing tests

Change-Id: I4a4b90b6e2416e3348d107978070a720ba71c53c
Reviewed-on: https://chromium-review.googlesource.com/982616
Commit-Ready: Maksim Ivanov <emaxx@chromium.org>
Tested-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Gwendal Grignou <gwendal@google.com>

[modify] https://crrev.com/67e8fad7003ae7fa9b6256a1aa91f2faad0d30e7/cryptohome/cryptolib.cc
[modify] https://crrev.com/67e8fad7003ae7fa9b6256a1aa91f2faad0d30e7/cryptohome/crypto.cc

Project Member

Comment 4 by bugdroid1@chromium.org, May 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/1b084ca754caf27e78a4b8d5562a75b70a8a7188

commit 1b084ca754caf27e78a4b8d5562a75b70a8a7188
Author: Maksim Ivanov <emaxx@google.com>
Date: Fri May 11 02:40:57 2018

cryptohome: GYP cleanup for cryptohome-libs.gypi

* Added some missing deps and link libraries;
* Made deps propagated to "leaf" targets via "all_dependent_settings".

BUG= chromium:827221 
TEST=existing tests

Change-Id: Idb9bef2a16e2b880d9c35251fb53dd96332f38a4
Reviewed-on: https://chromium-review.googlesource.com/986576
Commit-Ready: Maksim Ivanov <emaxx@chromium.org>
Tested-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>

[modify] https://crrev.com/1b084ca754caf27e78a4b8d5562a75b70a8a7188/cryptohome/cryptohome-libs.gypi

Components: OS>Systems>Security
Status: WontFix (was: Started)
This got obsolete after cryptohome was migrated to GN (http://crrev.com/c/1256343).

Sign in to add a comment