buildstream-migration / bst-staging

GNU Lesser General Public License v2.1
0 stars 0 forks source link

Buildstream crashes unhelpfully when ujson can't process a cache key #1341

Open Cynical-Optimist opened 4 years ago

Cynical-Optimist commented 4 years ago

See original issue on GitLab In GitLab by [Gitlab user @willsalmon] on Jun 16, 2020, 10:28

Summary

[Edit/update] Buildstream plugins must supply a function called "get_unique_key", which returns an object that can be used to identify an element based on the config defined in its YAML. If the object is in a format that cannot be processed correctly, this can cause buildstream to crash when calculating the key.

There ought to be a clean failure, with a helpful error message. Instead we get a crash, and a traceback that doesn't immediately explain the problem. [End edit/update]

(venvbuildstream) [will[[Gitlab user @wsx280]](https://gitlab.com/wsx280) freedesktop-sdk]$ bst build flatpak-release.bst
[--:--:--][        ][    main:core activity                 ] START   Build
[--:--:--][        ][    main:core activity                 ] START   Loading elements
[00:00:02][        ][    main:core activity                 ] SUCCESS Loading elements
[--:--:--][        ][    main:core activity                 ] START   Resolving elements
[00:00:00][        ][    main:core activity                 ] SUCCESS Resolving elements
[--:--:--][        ][    main:core activity                 ] START   Initializing remote caches
[00:00:00][        ][    main:core activity                 ] SUCCESS Initializing remote caches
[--:--:--][        ][    main:core activity                 ] START   Resolving cached state
[--:--:--][        ][    main:core activity                 ] START   Terminating buildbox-casd
[00:00:01][        ][    main:core activity                 ] SUCCESS Terminating buildbox-casd
[--:--:--][        ][    main:core activity                 ] BUG     <configparser.ConfigParser object at 0x7f5a1077e850> is not JSON serializable

    Traceback (most recent call last):
      File "/home/will/projects/buildstream/venvbuildstream/bin/bst", line 8, in <module>
        sys.exit(cli())
      File "/home/will/projects/buildstream/venvbuildstream/lib64/python3.7/site-packages/click/core.py", line 829, in __call__
        return self.main(*args, **kwargs)
      File "/home/will/projects/buildstream/buildstream/src/buildstream/_frontend/cli.py", line 280, in override_main
        original_main(self, args=args, prog_name=prog_name, complete_var=None, standalone_mode=standalone_mode, **extra)
      File "/home/will/projects/buildstream/venvbuildstream/lib64/python3.7/site-packages/click/core.py", line 782, in main
        rv = self.invoke(ctx)
      File "/home/will/projects/buildstream/venvbuildstream/lib64/python3.7/site-packages/click/core.py", line 1259, in invoke
        return _process_result(sub_ctx.command.invoke(sub_ctx))
      File "/home/will/projects/buildstream/venvbuildstream/lib64/python3.7/site-packages/click/core.py", line 1066, in invoke
        return ctx.invoke(self.callback, **ctx.params)
      File "/home/will/projects/buildstream/venvbuildstream/lib64/python3.7/site-packages/click/core.py", line 610, in invoke
        return callback(*args, **kwargs)
      File "/home/will/projects/buildstream/venvbuildstream/lib64/python3.7/site-packages/click/decorators.py", line 33, in new_func
        return f(get_current_context().obj, *args, **kwargs)
      File "/home/will/projects/buildstream/buildstream/src/buildstream/_frontend/cli.py", line 501, in build
        app.stream.build(elements, selection=deps, ignore_junction_targets=ignore_junction_targets, remote=remote)
      File "/home/will/projects/buildstream/buildstream/src/buildstream/_stream.py", line 282, in build
        dynamic_plan=True,
      File "/home/will/projects/buildstream/buildstream/src/buildstream/_stream.py", line 1266, in _load
        self._pipeline.resolve_elements(self.targets)
      File "/home/will/projects/buildstream/buildstream/src/buildstream/_pipeline.py", line 121, in resolve_elements
        element._initialize_state()
      File "/home/will/projects/buildstream/buildstream/src/buildstream/element.py", line 1184, in _initialize_state
        self.__update_resolved_state()
      File "/home/will/projects/buildstream/buildstream/src/buildstream/element.py", line 2330, in __update_resolved_state
        self.__update_cache_keys()
      File "/home/will/projects/buildstream/buildstream/src/buildstream/element.py", line 3006, in __update_cache_keys
        self.__weak_cache_key = self._calculate_cache_key(dependencies)
      File "/home/will/projects/buildstream/buildstream/src/buildstream/element.py", line 2104, in _calculate_cache_key
        return _cachekey.generate_key(cache_key_dict)
      File "/home/will/projects/buildstream/buildstream/src/buildstream/_cachekey.py", line 65, in generate_key
        ustring = ujson.dumps(value, sort_keys=True, escape_forward_slashes=False).encode("utf-8")
    TypeError: <configparser.ConfigParser object at 0x7f5a1077e850> is not JSON serializable

(venvbuildstream) [will[[Gitlab user @wsx280]](https://gitlab.com/wsx280) freedesktop-sdk]$

Steps to reproduce

Install bst at tag 1.93.4

install bst-plugins-experimental at commit aa3c71568b0cdc537028e6ec389276bb79441f46 (HEAD -> master, origin/master, origin/HEAD)

checkout Freedesktop-sdk at

commit 6eaed03f17ba70428c7beac6b8121f2da62aac7d (HEAD -> willsalmon/bst2-fv, origin/willsalmon/bst2-fv)
Author: William Salmon <will.salmon[[Gitlab user @codethink]](https://gitlab.com/codethink).co.uk>
Date:   Wed May 13 16:07:47 2020 +0100

    Use pip from bst-plugins-experimental

What is the current bug behaviour?

[Gitlab user @BenjaminSchubert] thinks this is a issue with the plugins repo, i dont understand why it works in CI https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/jobs/593127809 with what i belive are the same versions.

But if the bug is in the plugins then bst should handle this failer cleanly

Cynical-Optimist commented 4 years ago

In GitLab by [Gitlab user @willsalmon] on Jun 16, 2020, 10:28

changed the description

Cynical-Optimist commented 4 years ago

In GitLab by [Gitlab user @willsalmon] on Jun 17, 2020, 11:40

A example in CI https://gitlab.com/freedesktop-sdk/freedesktop-sdk/-/jobs/598778674

Cynical-Optimist commented 4 years ago

In GitLab by [Gitlab user @coldtom] on Jun 17, 2020, 12:19

[Gitlab user @willsalmon] This is caused by the flatpak_image (or maybe flatpak_repo) plugin passing a configparser to get_unique_key, which can't be dumped by ujson.

Cynical-Optimist commented 4 years ago

In GitLab by [Gitlab user @DouglasWinship] on Jun 18, 2020, 14:30

In the CI example above, the fault was with the snap_image plugin.

Managed to avoid the issue by editing the configure() function.

-        self.metadata = node.get_node('metadata')
+        self.metadata = node.get_node('metadata').strip_node_info()

But as Will says, the real bug here (for buildstream) is that it doesn't fail cleanly.

Cynical-Optimist commented 4 years ago

In GitLab by [Gitlab user @abderrahimk] on Jun 20, 2020, 07:35

The difference in version is in ujson, ujson 1.x accepts the ConfigParser, but 2.0 doesn't.

I agree that bst should have a more defensive attitude towards things coming from the plugins, and fail cleanly.

Cynical-Optimist commented 4 years ago

In GitLab by [Gitlab user @DouglasWinship] on Jun 22, 2020, 10:33

changed title from {-Crash with trace back-} to {+Buildstream crashes unhelpfully when ujson can't process a cache key+}

Cynical-Optimist commented 4 years ago

In GitLab by [Gitlab user @DouglasWinship] on Jun 22, 2020, 10:33

changed the description