Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 35878 Remove circular relationships between .gyp files
Starred by 10 users Project Member Reported by mark@chromium.org, Feb 16 2010 Back to list
Status: Archived
Owner: ----
Closed: Sep 8
Cc: phajdan.jr@chromium.org
Components:
OS: Linux, Windows
Pri: 2
Type: Bug


Sign in to add a comment
There aren't any circular relationships between .gyp files on the Mac, but 
there are on other platforms.

Google Chrome XP: gyp.input.CircularException: Some files not reachable, 
cycle in .gyp file dependency graph detected involving some or all of: 
src\chrome\chrome.gyp src\chrome_frame\chrome_frame.gyp 
src\webkit\default_plugin\default_plugin.gyp 
src\chrome\installer\mini_installer.gyp 
src\chrome\app\locales\locales.gyp src\build\all.gyp 
src\chrome\installer\installer.gyp src\webkit\webkit.gyp

Google Chrome Linux: gyp.input.CircularException: Some files not 
reachable, cycle in .gyp file dependency graph detected involving some or 
all of: /b/slave/google-chrome-rel-
linux/build/src/chrome/installer/installer.gyp /b/slave/google-chrome-
rel-linux/build/src/build/all.gyp /b/slave/google-chrome-rel-
linux/build/src/chrome/chrome.gyp

Google Chrome Linux x64: gyp.input.CircularException: Some files not 
reachable, cycle in .gyp file dependency graph detected involving some or 
all of: /b/slave/google-chrome-rel-
linux_64/build/src/chrome/chrome.gyp /b/slave/google-chrome-rel-
linux_64/build/src/build/all.gyp /b/slave/google-chrome-rel-
linux_64/build/src/chrome/installer/installer.gyp

 
Comment 1 by bugdro...@gmail.com, Feb 16 2010
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=39128 

------------------------------------------------------------------------
r39128 | mark@chromium.org | 2010-02-16 12:14:26 -0800 (Tue, 16 Feb 2010) | 13 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/DEPS?r1=39128&r2=39127
   M http://src.chromium.org/viewvc/chrome/trunk/src/build/gyp_chromium?r1=39128&r2=39127

Circular relationships between .gyp files should be errors.  Make them errors,
but currently only on the Mac.

These relationships should be errors on all platforms, but some currently
exist on non-Mac platforms.  See http://crbug.com/35878.  Because the Mac is
the only platform where a circular dependency between .gyp files is known to
cause tangible problems, the portions of Chromium's .gyp files that are used
by Macs have been fixed to remove these relationships, and the check is left
enabled on the Mac to ensure that no new ones are created.

BUG= 35308 
TEST=none
Review URL: http://codereview.chromium.org/600151
------------------------------------------------------------------------

Comment 2 by tony@chromium.org, May 19 2010
Blockedon: 44538
Labels: Area-BuildTools
Comment 4 Deleted
Comment 5 by tony@chromium.org, Mar 22 2011
Blockedon: 77100
Project Member Comment 6 by bugdroid1@chromium.org, Mar 10 2013
Blockedon: -chromium:44538 -chromium:77100 chromium:44538 chromium:77100
Blocking: -chromium:35308 chromium:35308
Labels: -Area-Internals -Internals-Core -Area-Build Cr-Internals Build Cr-Internals-Core
Cc: phajdan.jr@chromium.org
Labels: TaskForce-GreenTree
Project Member Comment 8 by bugdroid1@chromium.org, Aug 19 2013
------------------------------------------------------------------------
r218334 | phajdan.jr@chromium.org | 2013-08-19T22:04:53.112530Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/tools/generate_library_loader/generate_library_loader.py?r1=218334&r2=218333&pathrev=218334
   M http://src.chromium.org/viewvc/chrome/trunk/src/build/linux/system.gyp?r1=218334&r2=218333&pathrev=218334

Linux: untangle circular dependencies between .gyp files.

Not enabling the check yet because ChromeOS still has cycles.

BUG= 35878 
R=mark@chromium.org

Review URL: https://codereview.chromium.org/22825011
------------------------------------------------------------------------
Comment 9 by tapted@chromium.org, Jan 22 2014
Attaching
 - loops that currently exist with use_aura = 1,
 - a python script I hacked together to make sense of things
And, generated with `cat aura_and_views_loops.txt | sed -e 's/ -> /\n/g; s/Cycle: //g' | sort -u` as input)
 - aura.png: graph of loops that exist around aura
 - views.png: graph of loops that exist around views
find_depcycles.py
3.2 KB View Download
aura.png
290 KB View Download
aura_and_views_loops.txt
12.7 KB View Download
views.png
111 KB View Download
Attaching result of splitting aura.gyp into aura.gyp and aura_tests.gyp (https://codereview.chromium.org/131843004/)
after_aura_tests.png
126 KB View Download
Project Member Comment 11 by bugdroid1@chromium.org, Feb 7 2014
------------------------------------------------------------------------
r249568 | tfarina@chromium.org | 2014-02-07T00:57:53.043875Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/ui_unittests.gyp?r1=249568&r2=249567&pathrev=249568
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/shell_dialogs/shell_dialogs.gyp?r1=249568&r2=249567&pathrev=249568

Add unit test target for shell_dialogs module.

This is necessary to avoid a circular dependency that on Mac is forbidden:

ui_unittests -> shell_dialogs -> aura -> ui_unittests

The down side is that momentarily we lose automatic test coverage on the bots for the shell_dialogs.
This can be fixed by teaching the buildbot scripts to run it after this lands.

BUG=331669,  35878 , 299841
TEST=shell_dialogs_unittests
R=ben@chromium.org

Review URL: https://codereview.chromium.org/151253004
------------------------------------------------------------------------
Project Member Comment 12 by bugdroid1@chromium.org, Mar 7 2014
------------------------------------------------------------------------
r255512 | tapted@chromium.org | 2014-03-07T03:41:36.610793Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/strings/ui_strings.gyp?r1=255512&r2=255511&pathrev=255512
   A http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/test/framework-Info.plist?r1=255512&r2=255511&pathrev=255512
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/test/run_all_unittests.cc?r1=255512&r2=255511&pathrev=255512
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/l10n/time_format_unittest.cc?r1=255512&r2=255511&pathrev=255512
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/ui_unittests.gyp?r1=255512&r2=255511&pathrev=255512
   A http://src.chromium.org/viewvc/chrome/trunk/src/ui/ui_unittests_bundle.gypi?r1=255512&r2=255511&pathrev=255512

Introduce a mock ui_unittests Framework for loading resources.

This allows ui_unittests to stop depending on the chrome framework.

On Mac, this creates (e.g.)
- out/ui_unittests Framework.framework/
  +-- Resources -> Versions/A/Resources
  \-- Versions
      \-- A
          \-- Resources
              +-- Info.plist
              +-- am.lproj
              |   \-- locale.pak
              +-- ...
              +-- chrome_100_percent.pak -> ui_test.pak
              +-- ...
              +-- en.lproj
              |   \-- locale.pak
              +-- ...

On other platforms, out/ui_test.pak is loaded directly and
out/ui_unittests_strings/ is set as the locale folder (for tests that
load en-US.pak from there). ui_unittests currently depends on
out/locales/ which is only created when Chrome is built.

Note that ui_unittests does not currently succeed in a clobber build
(crbug.com/347851), so that missing dependency is fixed by this change
as well.

BUG=331669,  35878 ,  347851 
TEST=ui_unittests should build and run after clobbering the build folder

Review URL: https://codereview.chromium.org/152543005
------------------------------------------------------------------------
Project Member Comment 13 by bugdroid1@chromium.org, Mar 7 2014
------------------------------------------------------------------------
r255528 | tapted@chromium.org | 2014-03-07T04:42:24.204488Z

Changed paths:
   D http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/test/framework-Info.plist?r1=255528&r2=255527&pathrev=255528
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/test/run_all_unittests.cc?r1=255528&r2=255527&pathrev=255528
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/l10n/time_format_unittest.cc?r1=255528&r2=255527&pathrev=255528
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/ui_unittests.gyp?r1=255528&r2=255527&pathrev=255528
   D http://src.chromium.org/viewvc/chrome/trunk/src/ui/ui_unittests_bundle.gypi?r1=255528&r2=255527&pathrev=255528
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/strings/ui_strings.gyp?r1=255528&r2=255527&pathrev=255528

Revert 255512 "Introduce a mock ui_unittests Framework for loadi..."

All the chromium.mac bots were happy, but the official builder didn't like it.

> Introduce a mock ui_unittests Framework for loading resources.
> 
> This allows ui_unittests to stop depending on the chrome framework.
> 
> On Mac, this creates (e.g.)
> - out/ui_unittests Framework.framework/
>   +-- Resources -> Versions/A/Resources
>   \-- Versions
>       \-- A
>           \-- Resources
>               +-- Info.plist
>               +-- am.lproj
>               |   \-- locale.pak
>               +-- ...
>               +-- chrome_100_percent.pak -> ui_test.pak
>               +-- ...
>               +-- en.lproj
>               |   \-- locale.pak
>               +-- ...
> 
> On other platforms, out/ui_test.pak is loaded directly and
> out/ui_unittests_strings/ is set as the locale folder (for tests that
> load en-US.pak from there). ui_unittests currently depends on
> out/locales/ which is only created when Chrome is built.
> 
> Note that ui_unittests does not currently succeed in a clobber build
> (crbug.com/347851), so that missing dependency is fixed by this change
> as well.
> 
> BUG=331669,  35878 ,  347851 
> TEST=ui_unittests should build and run after clobbering the build folder
> 
> Review URL: https://codereview.chromium.org/152543005

TBR=tapted@chromium.org

Review URL: https://codereview.chromium.org/190133003
------------------------------------------------------------------------
Project Member Comment 14 by bugdroid1@chromium.org, Mar 12 2014
------------------------------------------------------------------------
r256419 | tapted@chromium.org | 2014-03-12T04:33:56.475665Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/strings/ui_strings.gyp?r1=256419&r2=256418&pathrev=256419
   A http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/test/framework-Info.plist?r1=256419&r2=256418&pathrev=256419
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/test/run_all_unittests.cc?r1=256419&r2=256418&pathrev=256419
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/l10n/time_format_unittest.cc?r1=256419&r2=256418&pathrev=256419
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/ui_unittests.gyp?r1=256419&r2=256418&pathrev=256419
   A http://src.chromium.org/viewvc/chrome/trunk/src/ui/ui_unittests_bundle.gypi?r1=256419&r2=256418&pathrev=256419

Introduce a mock ui_unittests Framework for loading resources.

This allows ui_unittests to stop depending on the chrome framework.

On Mac, this creates (e.g.)
- out/ui_unittests Framework.framework/
  +-- Resources -> Versions/A/Resources
  \-- Versions
      \-- A
          \-- Resources
              +-- Info.plist
              +-- am.lproj
              |   \-- locale.pak
              +-- ...
              +-- chrome_100_percent.pak -> ui_test.pak
              +-- ...
              +-- en.lproj
              |   \-- locale.pak
              +-- ...

On other platforms, out/ui_test.pak is loaded directly and
out/ui_unittests_strings/ is set as the locale folder (for tests that
load en-US.pak from there). ui_unittests currently depends on
out/locales/ which is only created when Chrome is built.

Note that ui_unittests does not currently succeed in a clobber build
(crbug.com/347851), so that missing dependency is fixed by this change
as well.

BUG=331669,  35878 ,  347851 
TEST=ui_unittests should build and run after clobbering the build folder

Previously Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255512

Review URL: https://codereview.chromium.org/152543005
------------------------------------------------------------------------
Project Member Comment 15 by bugdroid1@chromium.org, Mar 23 2014
------------------------------------------------------------------
r258758 | tfarina@chromium.org | 2014-03-22T03:11:55.443910Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/build/all.gyp?r1=258758&r2=258757&pathrev=258758
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/views.gyp?r1=258758&r2=258757&pathrev=258758
   A http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/controls/webview/webview_tests.gyp?r1=258758&r2=258757&pathrev=258758
   M http://src.chromium.org/viewvc/chrome/trunk/src/ash/ash.gyp?r1=258758&r2=258757&pathrev=258758
   A http://src.chromium.org/viewvc/chrome/trunk/src/ui/views/examples/examples.gyp?r1=258758&r2=258757&pathrev=258758

Break cycles between views, content and webview.

When running gyp_chromium with the following diff:

diff --git a/build/gyp_chromium b/build/gyp_chromium
index 63e8671..ca9b6a4 100755
--- a/build/gyp_chromium
+++ b/build/gyp_chromium
@@ -509,8 +509,6 @@ if __name__ == '__main__':
   # option.  http://crbug.com/35878.
   # TODO(tc): Fix circular dependencies in ChromiumOS then add linux2
   # list.
-  if sys.platform not in ('darwin',):
-    args.append('--no-circular-check')

These cycles are found:

gyp: Cycles in .gyp file dependency graph detected:
Cycle: content/content_shell_and_tests.gyp ->
ui/views/controls/webview/webview.gyp -> ui/views/views.gyp ->
content/content_shell_and_tests.gyp
Cycle: ui/views/controls/webview/webview.gyp -> ui/views/views.gyp ->
content/content_shell_and_tests.gyp ->
ui/views/controls/webview/webview.gyp
Cycle: ui/views/views.gyp -> content/content_shell_and_tests.gyp ->
ui/views/controls/webview/webview.gyp -> ui/views/views.gyp
Cycle: ui/views/views.gyp -> content/content_shell_and_tests.gyp ->
ui/views/views.gyp
Cycle: ui/views/controls/webview/webview.gyp -> ui/views/views.gyp ->
ui/views/controls/webview/webview.gyp
Cycle: ui/views/views.gyp -> ui/views/controls/webview/webview.gyp ->
ui/views/views.gyp
Cycle: content/content_shell_and_tests.gyp -> ui/views/views.gyp ->
content/content_shell_and_tests.gyp

By moving '*examples*' targets from views.gyp to examples.gyp we break
most of these cycles.

Then it remains the cycle:
Cycle: content/content_shell_and_tests.gyp -> ui/views/controls/webview/webview.gyp -> content/content_shell_and_tests.gyp

To fix that we introduced a webview_tests.gyp to which we moved the include of
content_shell_and_tests.gyp from webview.gyp, and thus breaking that
cycle and fixing all the circlar dependencies found above.

BUG=331669, 35878 
TEST=run gyp_chromium with the above diff, gyp should not throw any
cycles output.

R=ben@chromium.org, harrym@chromium.org, tapted@chromium.org

Review URL: https://codereview.chromium.org/201093002
-----------------------------------------------------------------
Blockedon: chromium:418455
Project Member Comment 17 by bugdroid1@chromium.org, Oct 2 2014
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426

commit 344272ddd4ca47fb56fd6a1ca6a4f1418df2c426
Author: tapted <tapted@chromium.org>
Date: Thu Oct 02 23:45:52 2014

Move components/native_app_window to extensions/components/native_app_window

There's currently a dependency cycle between gyp files: components.gyp
and extensions.gyp. extensions.gyp depends on a number of components,
and two components depend on exetensions.gyp. These are:
 - components/renderer_context_menu/
 - components/native_app_window/ (more recently)

renderer_context_menu has an "optional" extensions dependency, which can
be skipped when ENABLE_EXTENSIONS is not defined. Still, it is necessary
for renderer_context_menu.gyp to omit its extensions.gyp dependency and
build as a static library.

For native_app_window, since it is not compiled on Mac, the gyp circular
check is not performed on the bots.

This CL fixes the layering by adding a folder,
src/extensions/components. Things here may depend on extensions, but not
on other extensions/components folders in a way that creates a cycle.
This also allows the benefits of a component-architecture to make sense
of interdependencies between the ~1337 files under src/extensions.

This layout also makes it clear that by depending on one of these
components, a target is also depending on src/extensions. Currently,
this is not clear.

BUG= 35878 ,  418455 
TBR=reed@google.com

Review URL: https://codereview.chromium.org/606953002

Cr-Commit-Position: refs/heads/master@{#297952}

[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/apps/BUILD.gn
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/apps/apps.gypi
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/athena/athena.gyp
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/athena/extensions/DEPS
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/athena/extensions/athena_native_app_window_views.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/chrome/browser/BUILD.gn
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/chrome/browser/chromeos/login/DEPS
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/chrome/browser/chromeos/login/kiosk_browsertest.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/chrome/browser/ui/BUILD.gn
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/chrome/browser/ui/views/apps/DEPS
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/chrome/browser/ui/views/apps/chrome_native_app_window_views.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/chrome/chrome_browser.gypi
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/chrome/chrome_browser_ui.gypi
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/components/BUILD.gn
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/components/OWNERS
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/components/components.gyp
[delete] https://chromium.googlesource.com/chromium/src.git/+/c4c7e842b3ba77671b74ac263e5fb7217c3dfbfb/components/native_app_window.gypi
[delete] https://chromium.googlesource.com/chromium/src.git/+/c4c7e842b3ba77671b74ac263e5fb7217c3dfbfb/components/native_app_window/DEPS
[modify] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/extensions/DEPS
[add] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/extensions/components/DEPS
[add] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/extensions/components/README
[add] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/extensions/components/extensions_components.gyp
[add] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/extensions/components/native_app_window.gypi
[rename] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/extensions/components/native_app_window/BUILD.gn
[add] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/extensions/components/native_app_window/DEPS
[add] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/extensions/components/native_app_window/OWNERS
[add] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/extensions/components/native_app_window/README
[rename] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/extensions/components/native_app_window/native_app_window_views.cc
[rename] https://chromium.googlesource.com/chromium/src.git/+/344272ddd4ca47fb56fd6a1ca6a4f1418df2c426/extensions/components/native_app_window/native_app_window_views.h

Project Member Comment 18 by sheriffbot@chromium.org, Jun 26
Labels: Hotlist-OpenBugWithCL
A change has landed for this issue, but it's been open for over 6 months. Please review and close it if applicable. If this issue should remain open, remove the "Hotlist-OpenBugWithCL" label. If no action is taken, it will be archived in 30 days.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-OpenBugWithCL
Since it is blocking and has blcoked on issues , removing Hotlist-OpenBugWithCL label.
Status: Archived
Sign in to add a comment