Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infinite recursion #655

Closed
botbotty opened this issue Apr 28, 2023 · 4 comments
Closed

Infinite recursion #655

botbotty opened this issue Apr 28, 2023 · 4 comments

Comments

@botbotty
Copy link

west/src/west/commands.py

Lines 280 to 289 in 51ad886

def _get_config(self) -> Configuration:
'''Property for the config which was passed to run().
If `do_run` was given a *config* kwarg, it is returned.
Otherwise, a fatal error occurs.
'''
if self._config is None:
self.die(f"can't run west {self.name}; it requires config "
"variables, which were not available.")
return self._config

self.die() checks config color.ui, which will call self._get_config() again, but self._config is still None.

@marc-hb
Copy link
Collaborator

marc-hb commented Apr 28, 2023

Looks like a good catch. For the record can you please share the west version and some basic reproduction steps?

@botbotty
Copy link
Author

west version is v1.0.0

reproduction steps:

west init -m https://github.com/zephyrproject-rtos/example-application reproduction
cd reproduction/
west update

west example-west-command --help # so far so good

cat > patch.diff << EOF
--- a/example-application/scripts/example_west_command.py
+++ b/example-application/scripts/example_west_command.py
@@ -24,6 +24,7 @@ reflowed for you. You can also pass
 formatter_class=argparse.RawDescriptionHelpFormatter when calling
 parser_adder.add_parser() below if you want to keep your line
 endings.''')
+        self.manifest # Try to access manifest

     def do_add_parser(self, parser_adder):
         # This is a bit of boilerplate, which allows you full control over the
EOF
patch -p1 < patch.diff

west example-west-command --help # this will fail

@mbolivar-nordic
Copy link
Contributor

Hmm, I believe we fixed this in 354b0e8. @botbotty the steps to reproduce you've posted are using self.manifest outside of WestCommand.do_run(), which is not legal. From the docstring for this property:

        '''Property for the manifest which was passed to run().

        If `do_run` was given a *manifest* kwarg, it is returned.
        Otherwise, a fatal error occurs.
        '''

Reading this property from the constructor is an attempt to read it before do_run() was called, so an error is not unexpected.

Can you make this happen in another way?

@mbolivar-nordic
Copy link
Contributor

Can you make this happen in another way?

Please reopen with more details if so, thanks!

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

No branches or pull requests

3 participants