[Notes] [Git][BuildStream/buildstream][mandatory_suffix] 7 commits: _context.py Move `warn()` here from plugin.py



Title: GitLab

Phillip Smyth pushed to branch mandatory_suffix at BuildStream / buildstream

Commits:

12 changed files:

Changes:

  • NEWS
    ... ... @@ -2,6 +2,10 @@
    2 2
     buildstream 1.3.1
    
    3 3
     =================
    
    4 4
     
    
    5
    +  o All elements must now be suffixed with `.bst`
    
    6
    +    Attempting to use an element that does not have the `.bst` extension,
    
    7
    +    will result in a warning.
    
    8
    +
    
    5 9
       o BREAKING CHANGE: The 'manual' element lost its default 'MAKEFLAGS' and 'V'
    
    6 10
         environment variables. There is already a 'make' element with the same
    
    7 11
         variables. Note that this is a breaking change, it will require users to
    

  • buildstream/_context.py
    ... ... @@ -27,7 +27,7 @@ from . import _cachekey
    27 27
     from . import _signals
    
    28 28
     from . import _site
    
    29 29
     from . import _yaml
    
    30
    -from ._exceptions import LoadError, LoadErrorReason, BstError
    
    30
    +from ._exceptions import LoadError, LoadErrorReason, BstError, PluginError
    
    31 31
     from ._message import Message, MessageType
    
    32 32
     from ._profile import Topics, profile_start, profile_end
    
    33 33
     from ._artifactcache import ArtifactCache
    
    ... ... @@ -142,6 +142,28 @@ class Context():
    142 142
             self._log_handle = None
    
    143 143
             self._log_filename = None
    
    144 144
     
    
    145
    +    # Print a warning message, checks warning_token against project configuration
    
    146
    +    #
    
    147
    +    # Args:
    
    148
    +    #     project (Project): The related project
    
    149
    +    #     brief (str): The brief message
    
    150
    +    #     detail (str): An optional detailed message, can be multiline output
    
    151
    +    #     warning_token (str): An optional configurable warning assosciated with this warning,
    
    152
    +    #                          this will cause PluginError to be raised if this warning is configured as fatal.
    
    153
    +    #                          (*Since 1.4*)
    
    154
    +    #
    
    155
    +    # Raises:
    
    156
    +    #     (:class:`.PluginError`): When warning_token is considered fatal by the project configuration
    
    157
    +    #
    
    158
    +    def warn(self, project, unique_id, brief, *, detail=None, warning_token=None):
    
    159
    +        if warning_token:
    
    160
    +            if project._warning_is_fatal(warning_token):
    
    161
    +                detail = detail if detail else ""
    
    162
    +                raise PluginError(message="{}\n{}".format(brief, detail), reason=warning_token)
    
    163
    +
    
    164
    +        message = Message(unique_id, MessageType.WARN, brief, detail=detail)
    
    165
    +        self.message(message)
    
    166
    +
    
    145 167
         # load()
    
    146 168
         #
    
    147 169
         # Loads the configuration files
    

  • buildstream/_frontend/cli.py
    ... ... @@ -109,7 +109,11 @@ def complete_target(args, incomplete):
    109 109
         if element_directory:
    
    110 110
             base_directory = os.path.join(base_directory, element_directory)
    
    111 111
     
    
    112
    -    return complete_path("File", incomplete, base_directory=base_directory)
    
    112
    +    complete_list = []
    
    113
    +    for p in complete_path("File", incomplete, base_directory=base_directory):
    
    114
    +        if p.endswith(".bst ") or p.endswith("/") or p.endswith(".conf "):
    
    115
    +            complete_list.append(p)
    
    116
    +    return complete_list
    
    113 117
     
    
    114 118
     
    
    115 119
     def override_completions(cmd, cmd_param, args, incomplete):
    

  • buildstream/_loader/loader.py
    ... ... @@ -97,6 +97,7 @@ class Loader():
    97 97
         # Returns: The toplevel LoadElement
    
    98 98
         def load(self, targets, rewritable=False, ticker=None, fetch_subprojects=False):
    
    99 99
     
    
    100
    +        invalid_elements = []
    
    100 101
             for filename in targets:
    
    101 102
                 if os.path.isabs(filename):
    
    102 103
                     # XXX Should this just be an assertion ?
    
    ... ... @@ -106,6 +107,18 @@ class Loader():
    106 107
                                     "path to the base project directory: {}"
    
    107 108
                                     .format(filename, self._basedir))
    
    108 109
     
    
    110
    +            if not filename.endswith(".bst") and not filename.endswith("/") and not filename.endswith(".conf"):
    
    111
    +                invalid_elements.append(filename)
    
    112
    +
    
    113
    +        if invalid_elements:
    
    114
    +            # TODO CoreWarnings should probably move somewhere else
    
    115
    +            from ..plugin import CoreWarnings
    
    116
    +
    
    117
    +            self._context.warn(self.project, None,
    
    118
    +                               "Target elements '{}' do not have expected file extension `.bst` "
    
    119
    +                               "Improperly named elements will not be discoverable by commands"
    
    120
    +                               .format(invalid_elements),
    
    121
    +                               warning_token=CoreWarnings.BAD_ELEMENT_SUFFIX)
    
    109 122
             # First pass, recursively load files and populate our table of LoadElements
    
    110 123
             #
    
    111 124
             deps = []
    
    ... ... @@ -269,7 +282,12 @@ class Loader():
    269 282
             self._elements[filename] = element
    
    270 283
     
    
    271 284
             # Load all dependency files for the new LoadElement
    
    285
    +        invalid_elements = []
    
    272 286
             for dep in element.deps:
    
    287
    +            if not dep.name.endswith(".bst") and not dep.name.endswith("/") and not dep.name.endswith(".conf"):
    
    288
    +                invalid_elements.append(dep.name)
    
    289
    +                continue
    
    290
    +
    
    273 291
                 if dep.junction:
    
    274 292
                     self._load_file(dep.junction, rewritable, ticker, fetch_subprojects, yaml_cache)
    
    275 293
                     loader = self._get_loader(dep.junction, rewritable=rewritable, ticker=ticker,
    
    ... ... @@ -278,12 +296,19 @@ class Loader():
    278 296
                     loader = self
    
    279 297
     
    
    280 298
                 dep_element = loader._load_file(dep.name, rewritable, ticker, fetch_subprojects, yaml_cache)
    
    281
    -
    
    282 299
                 if _yaml.node_get(dep_element.node, str, Symbol.KIND) == 'junction':
    
    283 300
                     raise LoadError(LoadErrorReason.INVALID_DATA,
    
    284 301
                                     "{}: Cannot depend on junction"
    
    285 302
                                     .format(dep.provenance))
    
    286
    -
    
    303
    +        if invalid_elements:
    
    304
    +            # TODO CoreWarnings should probably move somewhere else
    
    305
    +            from ..plugin import CoreWarnings
    
    306
    +
    
    307
    +            self._context.warn(self.project, None,
    
    308
    +                               "Target elements '{}' do not have expected file extension `.bst` "
    
    309
    +                               "Improperly named elements will not be discoverable by commands"
    
    310
    +                               .format(invalid_elements),
    
    311
    +                               warning_token=CoreWarnings.BAD_ELEMENT_SUFFIX)
    
    287 312
             return element
    
    288 313
     
    
    289 314
         # _check_circular_deps():
    

  • buildstream/_project.py
    ... ... @@ -445,6 +445,9 @@ class Project():
    445 445
             self.config.options = OptionPool(self.element_path)
    
    446 446
             self.first_pass_config.options = OptionPool(self.element_path)
    
    447 447
     
    
    448
    +        # Early initialise fatal warnings
    
    449
    +        self._fatal_warnings = _yaml.node_get(pre_config_node, list, 'fatal-warnings', default_value=[])
    
    450
    +
    
    448 451
             self.loader = Loader(self._context, self,
    
    449 452
                                  parent=parent_loader,
    
    450 453
                                  tempdir=tempdir)
    

  • buildstream/plugin.py
    ... ... @@ -492,28 +492,25 @@ class Plugin():
    492 492
             self.__message(MessageType.INFO, brief, detail=detail)
    
    493 493
     
    
    494 494
         def warn(self, brief, *, detail=None, warning_token=None):
    
    495
    -        """Print a warning message, checks warning_token against project configuration
    
    496
    -
    
    497
    -        Args:
    
    498
    -           brief (str): The brief message
    
    499
    -           detail (str): An optional detailed message, can be multiline output
    
    500
    -           warning_token (str): An optional configurable warning assosciated with this warning,
    
    501
    -                                this will cause PluginError to be raised if this warning is configured as fatal.
    
    502
    -                                (*Since 1.4*)
    
    503
    -
    
    504
    -        Raises:
    
    505
    -           (:class:`.PluginError`): When warning_token is considered fatal by the project configuration
    
    495
    +        """ Print a warning message, checks warning_token against project configuration
    
    496
    +
    
    497
    +         Args:
    
    498
    +             project (Project): The related project
    
    499
    +             brief (str): The brief message
    
    500
    +             detail (str): An optional detailed message, can be multiline output
    
    501
    +             warning_token (str): An optional configurable warning assosciated with this warning,
    
    502
    +                                  this will cause PluginError to be raised if this warning is configured as fatal.
    
    503
    +                                  (*Since 1.4*)
    
    504
    +
    
    505
    +         Raises:
    
    506
    +             (:class:`.PluginError`): When warning_token is considered fatal by the project configuration
    
    506 507
             """
    
    507 508
             if warning_token:
    
    508
    -            warning_token = _prefix_warning(self, warning_token)
    
    509
    +            warning_token = _prefix_warning(plugin, warning_token)
    
    509 510
                 brief = "[{}]: {}".format(warning_token, brief)
    
    510
    -            project = self._get_project()
    
    511
    -
    
    512
    -            if project._warning_is_fatal(warning_token):
    
    513
    -                detail = detail if detail else ""
    
    514
    -                raise PluginError(message="{}\n{}".format(brief, detail), reason=warning_token)
    
    515 511
     
    
    516
    -        self.__message(MessageType.WARN, brief=brief, detail=detail)
    
    512
    +        context = self._get_context()
    
    513
    +        context.warn(self._get_project(), self._get_unique_id(), brief, detail=detail, warning_token=warning_token)
    
    517 514
     
    
    518 515
         def log(self, brief, *, detail=None):
    
    519 516
             """Log a message into the plugin's log file
    
    ... ... @@ -784,6 +781,12 @@ class CoreWarnings():
    784 781
         which is found to be invalid based on the configured track
    
    785 782
         """
    
    786 783
     
    
    784
    +    BAD_ELEMENT_SUFFIX = "bad-element-suffix"
    
    785
    +    """
    
    786
    +    This warning will be produced when an element whose name doesn not end in .bst
    
    787
    +    is referenced either on the command line or by another element
    
    788
    +    """
    
    789
    +
    
    787 790
     
    
    788 791
     __CORE_WARNINGS = [
    
    789 792
         value
    

  • tests/completions/completions.py
    ... ... @@ -66,6 +66,13 @@ PROJECT_ELEMENTS = [
    66 66
         "target.bst"
    
    67 67
     ]
    
    68 68
     
    
    69
    +INVALID_ELEMENTS = [
    
    70
    +    "target.foo"
    
    71
    +    "target.bst.bar"
    
    72
    +]
    
    73
    +
    
    74
    +MIXED_ELEMENTS = PROJECT_ELEMENTS + INVALID_ELEMENTS
    
    75
    +
    
    69 76
     
    
    70 77
     def assert_completion(cli, cmd, word_idx, expected, cwd=None):
    
    71 78
         result = cli.run(cwd=cwd, env={
    
    ... ... @@ -85,6 +92,24 @@ def assert_completion(cli, cmd, word_idx, expected, cwd=None):
    85 92
         assert words == expected
    
    86 93
     
    
    87 94
     
    
    95
    +def assert_completion_failed(cli, cmd, word_idx, expected, cwd=None):
    
    96
    +    result = cli.run(cwd=cwd, env={
    
    97
    +        '_BST_COMPLETION': 'complete',
    
    98
    +        'COMP_WORDS': cmd,
    
    99
    +        'COMP_CWORD': str(word_idx)
    
    100
    +    })
    
    101
    +    words = []
    
    102
    +    if result.output:
    
    103
    +        words = result.output.splitlines()
    
    104
    +
    
    105
    +    # The order is meaningless, bash will
    
    106
    +    # take the results and order it by its
    
    107
    +    # own little heuristics
    
    108
    +    words = sorted(words)
    
    109
    +    expected = sorted(expected)
    
    110
    +    assert words != expected
    
    111
    +
    
    112
    +
    
    88 113
     @pytest.mark.parametrize("cmd,word_idx,expected", [
    
    89 114
         ('bst', 0, []),
    
    90 115
         ('bst ', 1, MAIN_COMMANDS),
    
    ... ... @@ -226,6 +251,19 @@ def test_argument_element(datafiles, cli, project, cmd, word_idx, expected, subd
    226 251
         assert_completion(cli, cmd, word_idx, expected, cwd=cwd)
    
    227 252
     
    
    228 253
     
    
    254
    +@pytest.mark.datafiles(DATA_DIR)
    
    255
    +@pytest.mark.parametrize("project,cmd,word_idx,expected,subdir", [
    
    256
    +
    
    257
    +    # When element has invalid suffix
    
    258
    +    ('project', 'bst --directory ../ show ', 4, [e + ' ' for e in MIXED_ELEMENTS], 'files')
    
    259
    +])
    
    260
    +def test_argument_element_invalid(datafiles, cli, project, cmd, word_idx, expected, subdir):
    
    261
    +    cwd = os.path.join(str(datafiles), project)
    
    262
    +    if subdir:
    
    263
    +        cwd = os.path.join(cwd, subdir)
    
    264
    +    assert_completion_failed(cli, cmd, word_idx, expected, cwd=cwd)
    
    265
    +
    
    266
    +
    
    229 267
     @pytest.mark.parametrize("cmd,word_idx,expected", [
    
    230 268
         ('bst he', 1, ['help ']),
    
    231 269
         ('bst help ', 2, MAIN_COMMANDS),
    

  • tests/frontend/buildcheckout.py
    ... ... @@ -60,9 +60,34 @@ def test_build_checkout(datafiles, cli, strict, hardlinks):
    60 60
         assert os.path.exists(filename)
    
    61 61
     
    
    62 62
     
    
    63
    +@pytest.mark.datafiles(DATA_DIR)
    
    64
    +@pytest.mark.parametrize("strict,hardlinks", [
    
    65
    +    ("non-strict", "hardlinks"),
    
    66
    +])
    
    67
    +def test_build_invalid_suffix(datafiles, cli, strict, hardlinks):
    
    68
    +    project = os.path.join(datafiles.dirname, datafiles.basename)
    
    69
    +    checkout = os.path.join(cli.directory, 'checkout')
    
    70
    +
    
    71
    +    result = cli.run(project=project, args=strict_args(['build', 'target.foo'], strict))
    
    72
    +    result.assert_main_error(ErrorDomain.PLUGIN, "bad-element-suffix")
    
    73
    +
    
    74
    +
    
    75
    +@pytest.mark.datafiles(DATA_DIR)
    
    76
    +@pytest.mark.parametrize("strict,hardlinks", [
    
    77
    +    ("non-strict", "hardlinks"),
    
    78
    +])
    
    79
    +def test_build_invalid_suffix_dep(datafiles, cli, strict, hardlinks):
    
    80
    +    project = os.path.join(datafiles.dirname, datafiles.basename)
    
    81
    +    checkout = os.path.join(cli.directory, 'checkout')
    
    82
    +
    
    83
    +    # target2.bst depends on an element called target.foo
    
    84
    +    result = cli.run(project=project, args=strict_args(['build', 'target2.bst'], strict))
    
    85
    +    result.assert_main_error(ErrorDomain.PLUGIN, "bad-element-suffix")
    
    86
    +
    
    87
    +
    
    63 88
     @pytest.mark.datafiles(DATA_DIR)
    
    64 89
     @pytest.mark.parametrize("deps", [("run"), ("none")])
    
    65
    -def test_build_checkout_deps(datafiles, cli, deps):
    
    90
    +def test_build_checkoue_deps(datafiles, cli, deps):
    
    66 91
         project = os.path.join(datafiles.dirname, datafiles.basename)
    
    67 92
         checkout = os.path.join(cli.directory, 'checkout')
    
    68 93
     
    

  • tests/frontend/project/elements/target.foo
    1
    +kind: stack
    
    2
    +description: |
    
    3
    +
    
    4
    +  Main stack target for the bst build test

  • tests/frontend/project/elements/target2.bst
    1
    +kind: stack
    
    2
    +description: |
    
    3
    +
    
    4
    +  Main stack target for the bst build test
    
    5
    +
    
    6
    +depends:
    
    7
    +- target.foo

  • tests/frontend/project/project.conf
    ... ... @@ -2,3 +2,6 @@
    2 2
     name: test
    
    3 3
     
    
    4 4
     element-path: elements
    
    5
    +
    
    6
    +fatal-warnings:
    
    7
    +- bad-element-suffix

  • tests/integration/source-determinism.py
    ... ... @@ -33,7 +33,7 @@ def create_test_directory(*path, mode=0o644):
    33 33
     @pytest.mark.skipif(IS_LINUX and not HAVE_BWRAP, reason='Only available with bubblewrap on Linux')
    
    34 34
     def test_deterministic_source_umask(cli, tmpdir, datafiles, kind, integration_cache):
    
    35 35
         project = str(datafiles)
    
    36
    -    element_name = 'list'
    
    36
    +    element_name = 'list.bst'
    
    37 37
         element_path = os.path.join(project, 'elements', element_name)
    
    38 38
         repodir = os.path.join(str(tmpdir), 'repo')
    
    39 39
         sourcedir = os.path.join(project, 'source')
    
    ... ... @@ -108,7 +108,7 @@ def test_deterministic_source_local(cli, tmpdir, datafiles, integration_cache):
    108 108
         """Only user rights should be considered for local source.
    
    109 109
         """
    
    110 110
         project = str(datafiles)
    
    111
    -    element_name = 'test'
    
    111
    +    element_name = 'test.bst'
    
    112 112
         element_path = os.path.join(project, 'elements', element_name)
    
    113 113
         sourcedir = os.path.join(project, 'source')
    
    114 114
     
    



  • [Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]