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

AssertionError when combining --configure and --dry-run #136

Open
zimmerrol opened this issue Jun 13, 2023 · 3 comments
Open

AssertionError when combining --configure and --dry-run #136

zimmerrol opened this issue Jun 13, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@zimmerrol
Copy link

zimmerrol commented Jun 13, 2023

When calling the tool via git-delete-merged-branches --configure --effort=3 --dry-run --debug I get the following error message after selecting completing the configuration steps:

Traceback (most recent call last):
  File "...\miniconda3\lib\site-packages\git_delete_merged_branches\_cli.py", line 163, in _inner_main
    _innermost_main(config, messenger, colorize)
  File "...\miniconda3\lib\site-packages\git_delete_merged_branches\_cli.py", line 138, in _innermost_main
    git_config = dmb.ensure_configured(config.force_reconfiguration)
  File "...\miniconda3\lib\site-packages\git_delete_merged_branches\_engine.py", line 154, in ensure_configured
    assert self._is_configured(git_config)
AssertionError

Am I doing something wrong or is this a bug?
I'm not sure which additional information will be helpful for debugging this, so please let me know if you need more information. I'm using Windows 11 and powershell core as my terminal.

@hartwork
Copy link
Owner

Hi @zimmerrol,

thanks for bringing this here. During live configuration git-delete-merged-branches adds a key delete-merged-branches.configured to <gitdir>/config by means of the git config command. Its value is supposed to be 5.0.0+ as can be seen here:

# git config --get delete-merged-branches.configured
5.0.0+

For some reason writing and re-reading that config key and value did not work. Could you run git config --get delete-merged-branches.configured on this repository and also git --version and share their output? (I have Git 2.41.0 on Linux here at the moment and no easy access to Windows.)

Thanks!

@zimmerrol
Copy link
Author

zimmerrol commented Jun 13, 2023

Hi @hartwork,
Thanks for getting back to me that quickly.
It seems like this key has not been set. Is it possible that the --dry-run flag disables writing this flag?

>> git config --get delete-merged-branches.configured
>> git --version
git version 2.29.2.windows.3

Edit: Updating git to 2.41.0.windows.1 did not resolve the issue.

@hartwork
Copy link
Owner

hartwork commented Jun 13, 2023

Hi @zimmerrol I overlooked the --dry-run somehow, that's it, thanks for bringing it to my attention.

I think there are at least these theoretical options:

  • a) Make --dry-run and live configuration mutually exclusive officially and technically; would need to cover explicit --configure and implicit need
  • b) Drop/adjust the assertion (after double-checking it's fully safe to)
  • c) Pass a cached copy of git config state around at/to more places and update that cache also for --dry-run
  • d) Exclude config changes from --dry-run and major bump the version to accompany that change

So far I don't really like any of these options. I'll have to sleep over this. Let me know if you can think of alternatives that I might be missing or how you feel about the mentioned options, if you feel like it.

@hartwork hartwork changed the title Engine runs into AssertionError AssertionError when combining --configure and --dry-run Jun 18, 2023
@hartwork hartwork added the bug Something isn't working label Jun 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants
@hartwork @zimmerrol and others