Skip to content

Conversation

@andir
Copy link
Contributor

@andir andir commented Jul 12, 2021

This addresses a bunch of security fixes that PyYAML has addresses since
the last vendored version of the package.

Fixes #708

Follow up to #796

This addresses a bunch of security fixes that PyYAML has addresses since
the last vendored version of the package.
@anthonyskipper
Copy link

This pull would be super helpful as it would fix a bunch of downstream breakages like the azure-cli

@andir
Copy link
Contributor Author

andir commented Sep 14, 2021

Sorry for the ping @bitprophet but could you perhaps have a look on this? If this isn't the right approach please tell me what would be acceptable.

@minchinweb
Copy link

I think this is keeping invoke from running on Python 3.10...

@bitprophet bitprophet mentioned this pull request Nov 9, 2021
@bitprophet
Copy link
Member

@andir (following up from #796):

From looking at recent PyYAML versions it looked like all of them required native extensions for them to work. I am not sure how good it will be to basically maintain a parallel build system from the original project.

Not having looked upstream yet, do you mean they added dependencies on C extensions? That'd be less than ideal. This changeset looks to be limited to the vendored Python files though - does it install cleanly?

Have you considered hiding the YAML feature behind and "extra" flag? For example in my work we use invoke but don't make use of any of the YAML features (knowingly).

If PyYAML 5.x does have a non-pure-Python install flow, then I would take a harder look at whether to continue vendoring it. This would almost certainly be in a 3.x backwards incompat release but dropping Python 2 support would likely necessitate that anyways - so this could be rolled up into that work.


@minchinweb do you have a sample of the failure for 3.10? Invoke is next on my list for ending up on CircleCI (after a Paramiko bugfix release this week & an oncall shift next week) and I assume by the time I get to that, they'll have 3.10 available if they don't already. Which would give me a handy way to shake that sort of problem out of the tree.

@bitprophet bitprophet added this to the Priority milestone Nov 9, 2021
@andir
Copy link
Contributor Author

andir commented Nov 9, 2021

@andir (following up from #796):

From looking at recent PyYAML versions it looked like all of them required native extensions for them to work. I am not sure how good it will be to basically maintain a parallel build system from the original project.

Not having looked upstream yet, do you mean they added dependencies on C extensions? That'd be less than ideal. This changeset looks to be limited to the vendored Python files though - does it install cleanly?

It does work for me and I didn't notice any issues with the PR as it stands. Perhaps the C extensions are properly treated as an optional "optimization".

Have you considered hiding the YAML feature behind and "extra" flag? For example in my work we use invoke but don't make use of any of the YAML features (knowingly).

If PyYAML 5.x does have a non-pure-Python install flow, then I would take a harder look at whether to continue vendoring it. This would almost certainly be in a 3.x backwards incompat release but dropping Python 2 support would likely necessitate that anyways - so this could be rolled up into that work.

Given that we seem to get away without native extensions this probably remains a topic for a later point in time (if and when they will require native code)?

@minchinweb
Copy link

@bitprophet I believe it's the combination of having an invoke.yaml file in the project root and some rearragnments made in the standard library in Python 3.10. The actual error is AttributeError: module 'collections' has no attribute 'Hashable'.

Without the invoke.yaml file, invoke works. I guess this was just the first project I tried to use with Python 3.10.

To reproduct my setup, I'm working with the code at https://github.com/MinchinWeb/minchin.releaser and Python 3.10 on Windows 10 (although I don't think anything in Windows specific). Clone the repo, create and activate a virtual environment, and install the project (pip install .) (which will install invoke), and then run any invoke command, including invoke --list. Here is the traceback:

(.venv-310) C:\Code\minchin.releaser [master ≡]
> invoke --list
Traceback (most recent call last):
  File "C:\Python310\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Python310\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Code\minchin.releaser\.venv-310\Scripts\invoke.exe\__main__.py", line 7, in <module>
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\program.py", line 373, in run
    self.parse_collection()
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\program.py", line 465, in parse_collection
    self.load_collection()
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\program.py", line 702, in load_collection
    self.config.load_project()
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\config.py", line 750, in load_project
    self._load_file(prefix="project", merge=merge)
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\config.py", line 889, in _load_file
    self._set(data, loader(filepath))
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\config.py", line 909, in _load_yaml
    return yaml.load(fd)
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\vendor\yaml3\__init__.py", line 72, in load
    return loader.get_single_data()
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\vendor\yaml3\constructor.py", line 37, in get_single_data
    return self.construct_document(node)
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\vendor\yaml3\constructor.py", line 46, in construct_document
    for dummy in generator:
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\vendor\yaml3\constructor.py", line 398, in construct_yaml_map
    value = self.construct_mapping(node)
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\vendor\yaml3\constructor.py", line 204, in construct_mapping
    return super().construct_mapping(node, deep=deep)
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\vendor\yaml3\constructor.py", line 126, in construct_mapping
    if not isinstance(key, collections.Hashable):
AttributeError: module 'collections' has no attribute 'Hashable'

@davidalo
Copy link

@bitprophet I believe it's the combination of having an invoke.yaml file in the project root and some rearragnments made in the standard library in Python 3.10. The actual error is AttributeError: module 'collections' has no attribute 'Hashable'.

Without the invoke.yaml file, invoke works. I guess this was just the first project I tried to use with Python 3.10.

To reproduct my setup, I'm working with the code at https://github.com/MinchinWeb/minchin.releaser and Python 3.10 on Windows 10 (although I don't think anything in Windows specific). Clone the repo, create and activate a virtual environment, and install the project (pip install .) (which will install invoke), and then run any invoke command, including invoke --list. Here is the traceback:

(.venv-310) C:\Code\minchin.releaser [master ≡]
> invoke --list
Traceback (most recent call last):
  File "C:\Python310\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Python310\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Code\minchin.releaser\.venv-310\Scripts\invoke.exe\__main__.py", line 7, in <module>
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\program.py", line 373, in run
    self.parse_collection()
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\program.py", line 465, in parse_collection
    self.load_collection()
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\program.py", line 702, in load_collection
    self.config.load_project()
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\config.py", line 750, in load_project
    self._load_file(prefix="project", merge=merge)
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\config.py", line 889, in _load_file
    self._set(data, loader(filepath))
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\config.py", line 909, in _load_yaml
    return yaml.load(fd)
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\vendor\yaml3\__init__.py", line 72, in load
    return loader.get_single_data()
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\vendor\yaml3\constructor.py", line 37, in get_single_data
    return self.construct_document(node)
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\vendor\yaml3\constructor.py", line 46, in construct_document
    for dummy in generator:
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\vendor\yaml3\constructor.py", line 398, in construct_yaml_map
    value = self.construct_mapping(node)
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\vendor\yaml3\constructor.py", line 204, in construct_mapping
    return super().construct_mapping(node, deep=deep)
  File "C:\Code\minchin.releaser\.venv-310\lib\site-packages\invoke\vendor\yaml3\constructor.py", line 126, in construct_mapping
    if not isinstance(key, collections.Hashable):
AttributeError: module 'collections' has no attribute 'Hashable'

I am having the same issue, any updates on this?

@exhuma
Copy link

exhuma commented Dec 19, 2021

I can also confirm this with fabric. I just recently upgrade to Python 3.10 and am now running into this issue. Removing my fabric.yaml is a possible workaround for me (for now).

@bitprophet bitprophet merged commit 4fd787f into pyinvoke:main Mar 18, 2022
@bitprophet
Copy link
Member

bitprophet commented Mar 18, 2022

Just merged this for version 1.7. Also switched our call for loading config files to be safe_load 😅

@minchinweb
Copy link

This seems to have fixed the issue on my end. So excited to see the new release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update vendored yaml

6 participants