Skip to content

Commit

Permalink
Use first 7 letters of sha256 for subdirectory part of target id
Browse files Browse the repository at this point in the history
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 mesonbuild#4226.
  • Loading branch information
Aleksey Filippov committed Oct 16, 2018
1 parent ddc15e1 commit 9cfff2f
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 11 deletions.
36 changes: 28 additions & 8 deletions mesonbuild/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import copy, os, re
from collections import OrderedDict
import itertools, pathlib
import hashlib
import pickle
from functools import lru_cache

Expand Down Expand Up @@ -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:
Expand Down
21 changes: 18 additions & 3 deletions run_unittests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
'''
Expand Down Expand Up @@ -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()

Expand Down

0 comments on commit 9cfff2f

Please sign in to comment.