Skip to content

Commit

Permalink
src: fix multi-output-actions bug in make
Browse files Browse the repository at this point in the history
  • Loading branch information
refack committed Mar 11, 2019
1 parent 949e8bb commit be4ecdf
Show file tree
Hide file tree
Showing 8 changed files with 167 additions and 159 deletions.
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

0 comments on commit be4ecdf

Please sign in to comment.