From 9cfff2fd43f6d51973b149be00ff7217be7e351b Mon Sep 17 00:00:00 2001 From: Aleksey Filippov Date: Tue, 25 Sep 2018 18:07:09 +0100 Subject: [PATCH] Use first 7 letters of sha256 for subdirectory part of target id Fixed-size hash makes paths shorter and prevents doubling of path length because of subdir usage in target id: "subdir/id" would generate "subdir/{subdir-without-slashes}@@id" target otherwise. Export construct_id_from_path() to aid tests. Add a separate unit test for this function to make sure it is not broken unexpectedly. Closes #4226. --- mesonbuild/build.py | 36 ++++++++++++++++++++++++++++-------- run_unittests.py | 21 ++++++++++++++++++--- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/mesonbuild/build.py b/mesonbuild/build.py index eb0e2946bb10..414ecb7dcdbe 100644 --- a/mesonbuild/build.py +++ b/mesonbuild/build.py @@ -15,6 +15,7 @@ import copy, os, re from collections import OrderedDict import itertools, pathlib +import hashlib import pickle from functools import lru_cache @@ -330,19 +331,38 @@ def get_basename(self): def get_subdir(self): return self.subdir - def get_id(self): + @staticmethod + def _get_id_hash(target_id): + # We don't really need cryptographic security here. + # Small-digest hash function with unlikely collision is good enough. + h = hashlib.sha256() + h.update(target_id.encode(encoding='utf-8', errors='replace')) + # This ID should be case-insensitive and should work in Visual Studio, + # e.g. it should not start with leading '-'. + return h.hexdigest()[:7] + + @staticmethod + def construct_id_from_path(subdir, name, type_suffix): + """Construct target ID from subdir, name and type suffix. + + This helper function is made public mostly for tests.""" # This ID must also be a valid file name on all OSs. # It should also avoid shell metacharacters for obvious # reasons. '@' is not used as often as '_' in source code names. # In case of collisions consider using checksums. # FIXME replace with assert when slash in names is prohibited - name_part = self.name.replace('/', '@').replace('\\', '@') - assert not has_path_sep(self.type_suffix()) - myid = name_part + self.type_suffix() - if self.subdir: - subdir_part = self.subdir.replace('/', '@').replace('\\', '@') - myid = subdir_part + '@@' + myid - return myid + name_part = name.replace('/', '@').replace('\\', '@') + assert not has_path_sep(type_suffix) + my_id = name_part + type_suffix + if subdir: + subdir_part = Target._get_id_hash(subdir) + # preserve myid for better debuggability + return subdir_part + '@@' + my_id + return my_id + + def get_id(self): + return self.construct_id_from_path( + self.subdir, self.name, self.type_suffix()) def process_kwargs(self, kwargs): if 'build_by_default' in kwargs: diff --git a/run_unittests.py b/run_unittests.py index 8fe1c11650c3..df0f137c7e6f 100755 --- a/run_unittests.py +++ b/run_unittests.py @@ -44,6 +44,7 @@ from mesonbuild.environment import detect_ninja from mesonbuild.mesonlib import MesonException, EnvironmentException from mesonbuild.dependencies import PkgConfigDependency, ExternalProgram +from mesonbuild.build import Target import mesonbuild.modules.pkgconfig from run_tests import exe_suffix, get_fake_env, get_meson_script @@ -1551,7 +1552,8 @@ def test_internal_include_order(self): incs = [a for a in shlex.split(execmd) if a.startswith("-I")] self.assertEqual(len(incs), 9) # target private dir - self.assertPathEqual(incs[0], "-Isub4/sub4@@someexe@exe") + someexe_id = Target.construct_id_from_path("sub4", "someexe", "@exe") + self.assertPathEqual(incs[0], "-I" + os.path.join("sub4", someexe_id)) # target build subdir self.assertPathEqual(incs[1], "-Isub4") # target source subdir @@ -2744,6 +2746,17 @@ def test_buildtype_setting(self): self.assertEqual(opts['debug'], True) self.assertEqual(opts['optimization'], '0') + def test_target_construct_id_from_path(self): + # This id is stable but not guessable. + # The test is supposed to prevent unintentional + # changes of target ID generation. + target_id = Target.construct_id_from_path('some/obscure/subdir', + 'target-id', '@suffix') + self.assertEqual('5e002d3@@target-id@suffix', target_id) + target_id = Target.construct_id_from_path('subproject/foo/subdir/bar', + 'target2-id', '@other') + self.assertEqual('81d46d1@@target2-id@other', target_id) + class FailureTests(BasePlatformTests): ''' @@ -3521,8 +3534,10 @@ def test_compiler_cpp_stds(self): def test_unity_subproj(self): testdir = os.path.join(self.common_test_dir, '46 subproject') self.init(testdir, extra_args='--unity=subprojects') - self.assertPathExists(os.path.join(self.builddir, 'subprojects/sublib/subprojects@sublib@@simpletest@exe/simpletest-unity.c')) - self.assertPathExists(os.path.join(self.builddir, 'subprojects/sublib/subprojects@sublib@@sublib@sha/sublib-unity.c')) + simpletest_id = Target.construct_id_from_path('subprojects/sublib', 'simpletest', '@exe') + self.assertPathExists(os.path.join(self.builddir, 'subprojects/sublib', simpletest_id, 'simpletest-unity.c')) + sublib_id = Target.construct_id_from_path('subprojects/sublib', 'sublib', '@sha') + self.assertPathExists(os.path.join(self.builddir, 'subprojects/sublib', sublib_id, 'sublib-unity.c')) self.assertPathDoesNotExist(os.path.join(self.builddir, 'user@exe/user-unity.c')) self.build()