From e67c961905792cd63950c6f6efc33275ad243c49 Mon Sep 17 00:00:00 2001 From: philwo Date: Mon, 6 May 2019 04:28:48 -0700 Subject: [PATCH] Fix a non-determinism in create_embedded_tools.py. PiperOrigin-RevId: 246804128 --- scripts/bootstrap/compile.sh | 2 +- src/BUILD | 12 +++---- src/create_embedded_tools.py | 33 +++++++++++-------- tools/platforms/BUILD | 2 +- .../{platforms.BUILD => BUILD.tools} | 0 5 files changed, 27 insertions(+), 22 deletions(-) rename tools/platforms/{platforms.BUILD => BUILD.tools} (100%) diff --git a/scripts/bootstrap/compile.sh b/scripts/bootstrap/compile.sh index bfc0d6e3367d24..069ee91836b01d 100755 --- a/scripts/bootstrap/compile.sh +++ b/scripts/bootstrap/compile.sh @@ -272,7 +272,7 @@ EOF # Set up @bazel_tools//platforms properly mkdir -p ${BAZEL_TOOLS_REPO}/platforms - cp tools/platforms/platforms.BUILD ${BAZEL_TOOLS_REPO}/platforms/BUILD + cp tools/platforms/BUILD.tools ${BAZEL_TOOLS_REPO}/platforms/BUILD # Overwrite tools.WORKSPACE, this is only for the bootstrap binary chmod u+w "${OUTPUT_DIR}/classes/com/google/devtools/build/lib/bazel/rules/tools.WORKSPACE" diff --git a/src/BUILD b/src/BUILD index 30116d7e209bda..79d852b1ede609 100644 --- a/src/BUILD +++ b/src/BUILD @@ -61,9 +61,9 @@ genquery( # Create dummy tools so we can do select to prevent building iOS target on # Linux. OSX_DUMMY_TARGETS = [ - "src/tools/xcode/actoolwrapper/actoolwrapper", - "src/tools/xcode/ibtoolwrapper/ibtoolwrapper", - "src/tools/xcode/momcwrapper/momcwrapper", + "src/tools/xcode/actoolwrapper/actoolwrapper.sh", + "src/tools/xcode/ibtoolwrapper/ibtoolwrapper.sh", + "src/tools/xcode/momcwrapper/momcwrapper.sh", "src/objc_tools/bundlemerge/bundlemerge_deploy.jar", "src/objc_tools/plmerge/plmerge_deploy.jar", "src/tools/xcode/realpath/realpath", @@ -83,9 +83,9 @@ OSX_DUMMY_TARGETS = [ filegroup( name = "darwin_tools", srcs = [ - "//src/tools/xcode/actoolwrapper:actoolwrapper", - "//src/tools/xcode/ibtoolwrapper:ibtoolwrapper", - "//src/tools/xcode/momcwrapper:momcwrapper", + "//src/tools/xcode/actoolwrapper:actoolwrapper.sh", + "//src/tools/xcode/ibtoolwrapper:ibtoolwrapper.sh", + "//src/tools/xcode/momcwrapper:momcwrapper.sh", "//src/objc_tools/bundlemerge:bundlemerge_deploy.jar", "//src/objc_tools/plmerge:plmerge_deploy.jar", "//src/tools/xcode/realpath:realpath", diff --git a/src/create_embedded_tools.py b/src/create_embedded_tools.py index 92eec34f392fa0..52adf63c5c682c 100644 --- a/src/create_embedded_tools.py +++ b/src/create_embedded_tools.py @@ -31,7 +31,7 @@ ('*tools/jdk/BUILD', lambda x: 'tools/jdk/BUILD'), ('*tools/build_defs/repo/BUILD.repo', lambda x: 'tools/build_defs/repo/BUILD'), - ('*tools/platforms/platforms.BUILD', lambda x: 'platforms/BUILD'), + ('*tools/platforms/BUILD.tools', lambda x: 'platforms/BUILD'), ('*tools/platforms/*', lambda x: 'platforms/' + os.path.basename(x)), ('*tools/cpp/runfiles/generated_*', lambda x: 'tools/cpp/runfiles/' + os.path.basename(x)[len('generated_'):]), @@ -51,9 +51,7 @@ lambda x: 'tools/objc/make_hashed_objlist.py'), ('*xcode*realpath', lambda x: 'tools/objc/realpath'), ('*xcode*xcode-locator', lambda x: 'tools/objc/xcode-locator'), - ('*src/tools/xcode/*.sh', lambda x: 'tools/objc/' + os.path.basename(x)), - ('*src/tools/xcode/*', - lambda x: 'tools/objc/' + os.path.basename(x) + '.sh'), + ('*src/tools/xcode/*', lambda x: 'tools/objc/' + os.path.basename(x)), ('*external/openjdk_*/file/*.tar.gz', lambda x: 'jdk.tar.gz'), ('*external/openjdk_*/file/*.zip', lambda x: 'jdk.zip'), ('*src/minimal_jdk.tar.gz', lambda x: 'jdk.tar.gz'), @@ -70,15 +68,18 @@ def get_output_path(path): def get_input_files(argsfile): - """Returns a sorted list of tuples (archive_file, input_file). + """Returns a dict of archive_file to input_file. This describes the files that should be put into the generated archive. Args: argsfile: The file containing the list of input files. + + Raises: + ValueError: When two input files map to the same output file. """ with open(argsfile, 'r') as f: - input_files = set(x.strip() for x in f.readlines()) + input_files = sorted(set(x.strip() for x in f.readlines())) result = {} for input_file in input_files: @@ -87,14 +88,16 @@ def get_input_files(argsfile): input_file + '.tools' in input_files): continue - # This gives us the same behavior as the older bash version of this - # tool: If two input files map to the same output files, the one that - # comes last in the list of input files overrides all earlier ones. - result[get_output_path(input_file)] = input_file + # It's an error to have two files map to the same output file, because the + # result is hard to predict and can easily be wrong. + output_path = get_output_path(input_file) + if output_path in result: + raise ValueError( + 'Duplicate output file: Both {} and {} map to {}'.format( + result[output_path], input_file, output_path)) + result[output_path] = input_file - # By sorting the file list, the resulting ZIP file will not be reproducible - # and deterministic. - return sorted(result.items()) + return result def copy_jdk_into_archive(output_zip, archive_file, input_file): @@ -124,7 +127,9 @@ def main(): zipinfo.external_attr = 0o644 << 16 output_zip.writestr(zipinfo, 'workspace(name = "bazel_tools")\n') - for archive_file, input_file in input_files: + # By sorting the file list, the resulting ZIP file will be reproducible and + # deterministic. + for archive_file, input_file in sorted(input_files.items()): if os.path.basename(archive_file) in ('jdk.tar.gz', 'jdk.zip'): copy_jdk_into_archive(output_zip, archive_file, input_file) else: diff --git a/tools/platforms/BUILD b/tools/platforms/BUILD index da6f476adb8565..f0dc074f3ead4e 100644 --- a/tools/platforms/BUILD +++ b/tools/platforms/BUILD @@ -7,7 +7,7 @@ package( filegroup( name = "package-srcs", srcs = [ - "platforms.BUILD", + "BUILD.tools", ], ) diff --git a/tools/platforms/platforms.BUILD b/tools/platforms/BUILD.tools similarity index 100% rename from tools/platforms/platforms.BUILD rename to tools/platforms/BUILD.tools