Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: fix multi-output-actions bug in make #32

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
235 changes: 119 additions & 116 deletions pylib/gyp/MakefileWriter.py

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions pylib/gyp/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ def RelativePath(path, relative_to, follow_path_symlink=True):
return path

rel = os.path.relpath(path, relative_to)
if rel == '.':
rel = ''
return rel


Expand Down
29 changes: 15 additions & 14 deletions pylib/gyp/generator/make.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from __future__ import print_function

import os
import posixpath as path
import re
import subprocess
import gyp
Expand Down Expand Up @@ -221,7 +222,7 @@ def WriteAutoRegenerationRule(params, root_makefile, makefile_name, build_files,

gyp_binary = gyp.common.FixIfRelativePath(params['gyp_binary'], options.toplevel_dir)
if not gyp_binary.startswith(os.sep):
gyp_binary = os.path.join('.', gyp_binary)
gyp_binary = path.join('.', gyp_binary)

root_makefile.write(
"quiet_cmd_regen_makefile = ACTION Regenerating $@\n"
Expand Down Expand Up @@ -261,12 +262,12 @@ def CalculateMakefilePath(build_file_arg, base_name):
# paths relative to the source root for the master makefile. Grab
# the path of the .gyp file as the base to relativize against.
# E.g. "foo/bar" when we're constructing targets for "foo/bar/baz.gyp".
base_makefile_path = gyp.common.RelativePath(os.path.dirname(build_file_arg), options.depth)
base_makefile_path = gyp.common.RelativePath(path.dirname(build_file_arg), options.depth)
# We write the file in the base_makefile_path directory.
output_makefile = os.path.join(options.depth, base_makefile_path, base_name)
output_makefile = path.join(options.depth, base_makefile_path, base_name)
if options.generator_output:
output_makefile = os.path.join(options.depth, options.generator_output, base_makefile_path, base_name)
base_makefile_path = gyp.common.RelativePath(os.path.dirname(build_file_arg), options.toplevel_dir)
output_makefile = path.join(options.depth, options.generator_output, base_makefile_path, base_name)
base_makefile_path = gyp.common.RelativePath(path.dirname(build_file_arg), options.toplevel_dir)
return base_makefile_path, output_makefile

# TODO: search for the first non-'Default' target. This can go
Expand All @@ -284,9 +285,9 @@ def CalculateMakefilePath(build_file_arg, base_name):

srcdir = '.'
makefile_name = 'Makefile' + options.suffix
makefile_path = os.path.join(options.toplevel_dir, makefile_name)
makefile_path = path.join(options.toplevel_dir, makefile_name)
if options.generator_output:
makefile_path = os.path.join(options.toplevel_dir, options.generator_output, makefile_name)
makefile_path = path.join(options.toplevel_dir, options.generator_output, makefile_name)
srcdir = gyp.common.RelativePath(srcdir, options.generator_output)
Sourceify.srcdir_prefix = '$(srcdir)/'

Expand Down Expand Up @@ -407,7 +408,7 @@ def CalculateMakefilePath(build_file_arg, base_name):
WriteRootHeaderSuffixRules(root_makefile)

# Put build-time support tools next to the root Makefile.
dest_path = os.path.dirname(makefile_path)
dest_path = path.dirname(makefile_path)
gyp.common.CopyTool(flavor, dest_path)

# Find the list of targets that derive from the gyp file(s) being built.
Expand Down Expand Up @@ -437,7 +438,7 @@ def CalculateMakefilePath(build_file_arg, base_name):
gyp.common.UnrelativePath(included_file, build_file),
options.toplevel_dir
)
abs_include_file = os.path.abspath(relative_include_file)
abs_include_file = path.abspath(relative_include_file)
# If the include file is from the ~/.gyp dir, we should use absolute path
# so that relocating the src dir doesn't break the path.
if params['home_dot_gyp'] and abs_include_file.startswith(params['home_dot_gyp']):
Expand All @@ -458,7 +459,7 @@ def CalculateMakefilePath(build_file_arg, base_name):

# Our root_makefile lives at the source root. Compute the relative path
# from there to the output_file for including.
mkfile_rel_path = gyp.common.RelativePath(output_file, os.path.dirname(makefile_path))
mkfile_rel_path = gyp.common.RelativePath(output_file, path.dirname(makefile_path))
include_list.add(mkfile_rel_path)

assert writer
Expand All @@ -468,14 +469,14 @@ def CalculateMakefilePath(build_file_arg, base_name):
# The paths in build_files were relativized above, so undo that before
# testing against the non-relativized items in target_list and before
# calculating the Makefile path.
build_file_path = os.path.join(depth_rel_path, build_file)
build_file_path = path.join(depth_rel_path, build_file)
related_gyp_targets = [t for t in target_list if t.startswith(build_file) and t in needed_targets]
# Only generate Makefiles for gyp files with targets.
if not related_gyp_targets:
continue
build_file_name = "%s.Makefile" % os.path.splitext(os.path.basename(build_file))[0]
build_file_name = "%s.Makefile" % path.splitext(path.basename(build_file))[0]
_, submake_output_file = CalculateMakefilePath(build_file_path, build_file_name)
makefile_rel_path = gyp.common.RelativePath(os.path.dirname(makefile_path), os.path.dirname(submake_output_file))
makefile_rel_path = gyp.common.RelativePath(path.dirname(makefile_path), path.dirname(submake_output_file))
gyp_targets_names = [target_dicts[t]['target_name'] for t in related_gyp_targets]
writer.WriteSubMake(submake_output_file, makefile_rel_path, gyp_targets_names, builddir_name)

Expand Down Expand Up @@ -547,7 +548,7 @@ def CalculateGeneratorInputInfo(params):
output_dir = params['options'].generator_output or \
params['options'].toplevel_dir
builddir_name = generator_flags.get('output_dir', 'out')
qualified_out_dir = os.path.normpath(os.path.join(
qualified_out_dir = path.normpath(path.join(
output_dir, builddir_name, 'gypfiles'))

global generator_filelist_paths
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
# found in the LICENSE file.

"""
Verifies actions with multiple outputs & dependncies will correctly rebuild.
Verifies actions with multiple outputs & dependencies will correctly rebuild.

This is a regression test for crrev.com/1177163002.
"""
Expand All @@ -17,10 +17,6 @@
import sys
import time

if sys.platform in ('darwin', 'win32'):
print("This test is currently disabled: https://crbug.com/483696.")
sys.exit(0)

test = TestGyp.TestGyp()

TESTDIR='relocate/src'
Expand All @@ -30,6 +26,7 @@
def build_and_check(content):
test.write(TESTDIR + '/input.txt', content)
test.build('action.gyp', 'upper', chdir=TESTDIR)
test.up_to_date('action.gyp', 'upper', chdir=TESTDIR)
test.built_file_must_match('result.txt', content, chdir=TESTDIR)

build_and_check('Content for first build.')
Expand Down
20 changes: 14 additions & 6 deletions test/actions-multiple-outputs-with-dependencies/src/action.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,20 @@
{
'target_name': 'lower',
'type': 'none',
'actions': [{
'action_name': 'lower_action',
'inputs': ['input.txt'],
'outputs': ['<(PRODUCT_DIR)/out1.txt', '<(PRODUCT_DIR)/out2.txt'],
'action': ['python', 'rcopy.py', '<@(_inputs)', '<@(_outputs)'],
}],
'actions': [
{
'action_name': 'lower_action',
'external': 'true',
'inputs': ['input.txt'],
'outputs': ['<(PRODUCT_DIR)/out1.txt', '<(PRODUCT_DIR)/out2.txt'],
'action': ['python', 'rcopy.py', '<@(_inputs)', '<@(_outputs)'],
},
{
'action_name': 'lower_action2',
'inputs': ['input.txt'],
'outputs': ['<(PRODUCT_DIR)/out2.1.txt'],
'action': ['python', 'rcopy.py', '<@(_inputs)', '<@(_outputs)'],
}],
},
],
}
18 changes: 3 additions & 15 deletions test/actions-multiple-outputs/gyptest-multiple-outputs.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,9 @@
#!/usr/bin/env python

# Copyright (c) 2015 Google Inc. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

"""
Verifies actions with multiple outputs will correctly rebuild.
"""

from __future__ import print_function

import TestGyp
import os
import sys

if sys.platform == 'win32':
print("This test is currently disabled: https://crbug.com/483696.")
sys.exit(0)

test = TestGyp.TestGyp()

Expand All @@ -28,6 +15,7 @@
def build_and_check():
# Build + check that both outputs exist.
test.build('multiple-outputs.gyp', chdir=chdir)
test.up_to_date('action.gyp', chdir=chdir)
test.built_file_must_exist('out1.txt', chdir=chdir)
test.built_file_must_exist('out2.txt', chdir=chdir)

Expand All @@ -36,10 +24,10 @@ def build_and_check():

# Remove either + rebuild. Both should exist (again).
os.remove(test.built_file_path('out1.txt', chdir=chdir))
build_and_check();
build_and_check()

# Remove the other + rebuild. Both should exist (again).
os.remove(test.built_file_path('out2.txt', chdir=chdir))
build_and_check();
build_and_check()

test.pass_test()
1 change: 1 addition & 0 deletions test/actions-multiple-outputs/src/multiple-outputs.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
'actions': [
{
'action_name': 'action1',
'external': 'true',
'inputs': [],
'outputs': [
'<(PRODUCT_DIR)/out1.txt',
Expand Down
14 changes: 11 additions & 3 deletions test/lib/TestGyp.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,22 @@ def match_modulo_line_numbers(contents_a, contents_b):
return TestCommon.match_exact(contents_a, contents_b)


def mk_temp_dir(workdir):
def mk_temp_dir(workdir, hint=''):
# Put test output in out/testworkarea by default.
# Use temporary names so there are no collisions.
workdir = workdir or 'testworkarea'
workdir = os.path.join('out', workdir)
# Create work area if it doesn't already exist.
if not os.path.isdir(workdir):
os.makedirs(workdir)
return tempfile.mktemp(prefix='testgyp.', dir=workdir)
if hint:
slug = re.sub(r'[/.\\]', '_', hint)
dir_name = os.path.join(workdir, slug)
if os.path.exists(dir_name):
shutil.rmtree(dir_name)
return dir_name
else:
return tempfile.mktemp(prefix='testgyp.', dir=workdir)


@contextmanager
Expand Down Expand Up @@ -131,7 +138,8 @@ def __init__(self, gyp=None, **kw):
if 'description' not in kw:
bt = [t[0] for t in traceback.extract_stack() if 'gyptest' in t[0]]
kw['description'] = bt and bt.pop()
kw['workdir'] = mk_temp_dir(kw.get('workdir'))
kw_workdir = kw.get('workdir')
kw['workdir'] = mk_temp_dir(kw_workdir, kw['description'])
kw_formats = kw.pop('formats', [])

if not gyp:
Expand Down