From f2abfba8ee1d947f60c8482dbc6aeb5727b82919 Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Fri, 9 Aug 2024 11:49:17 +0100 Subject: [PATCH 1/3] tools/batch-rebase: Fix content of .batch-rebase-state when a conflict occured FIX Ensure the state is properly updated with the information from the conflict before being dumped to disk. --- tools/batch-rebase/src/batch_rebase/main.py | 30 ++++++++++++--------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/tools/batch-rebase/src/batch_rebase/main.py b/tools/batch-rebase/src/batch_rebase/main.py index 3eaa6c007..71b07ffc1 100755 --- a/tools/batch-rebase/src/batch_rebase/main.py +++ b/tools/batch-rebase/src/batch_rebase/main.py @@ -193,6 +193,8 @@ def empty_staging_area(repo): return True def do_create(conf_folder, repo, temp_repo, new_branch, conf, persistent_tags, tags_suffix): + conf = conf.copy() + # Use --shared to avoid copying the git objects, so we only pay for a # checkout call_git(['clone', '--shared', '--', repo, temp_repo]) @@ -252,14 +254,15 @@ def do_resume(temp_repo, conf): return (False, True) def do_cherry_pick(repo, temp_repo, conf, persistent_tags, tags_suffix, branch, rr_cache): - has_conflict, persistent_refs = _do_cherry_pick(temp_repo, conf, persistent_tags, tags_suffix) + has_conflict, conf, persistent_refs = _do_cherry_pick(temp_repo, conf, persistent_tags, tags_suffix) if has_conflict: - conf['resume'] = { + conf = conf.copy() + conf['resume'].update({ 'repo': str(repo), 'rr-cache': str(rr_cache) if rr_cache else rr_cache, 'branch': branch, - } + }) # Save the augmented manifest for later resumption dump_conf(conf, temp_repo/RESUME_MANIFEST_NAME) else: @@ -334,7 +337,7 @@ def _do_cherry_pick(repo, conf, persistent_tags, tags_suffix): git(['diff-index', '--quiet', 'HEAD', '--']) except subprocess.CalledProcessError: info('Please commit all files before running batch-rebase resume') - return (True, persistent_refs) + return (True, conf, persistent_refs) # Finish cherry picking the topic with the conflict. # If the commit was the last in the topic, this will fail as the `git @@ -406,12 +409,15 @@ def _do_cherry_pick(repo, conf, persistent_tags, tags_suffix): if not cherry_pick_ref(repo, range_sha1s): # Save the current state for later resumption - conf['resume'] = { - 'conflict-topic': name, - 'persistent-refs': sorted(persistent_refs), - 'tags': { - 'persistent': persistent_tags, - 'suffix': tags_suffix, + conf = { + **conf, + 'resume': { + 'conflict-topic': name, + 'persistent-refs': sorted(persistent_refs), + 'tags': { + 'persistent': persistent_tags, + 'suffix': tags_suffix, + } } } info(textwrap.dedent(""" @@ -430,7 +436,7 @@ def _do_cherry_pick(repo, conf, persistent_tags, tags_suffix): repo=repo, range_ref=range_ref, )).strip()) - return (True, persistent_refs) + return (True, conf, persistent_refs) tag_name = add_tag(name) if persistent_tags: @@ -439,7 +445,7 @@ def _do_cherry_pick(repo, conf, persistent_tags, tags_suffix): else: raise ValueError('Unknown action: {}'.format(action)) - return (False, persistent_refs) + return (False, conf, persistent_refs) def cherry_pick_ref(repo, refs): -- GitLab From be8f3db63bd4ff32da1e7d3b2dda34ca20ebc4e5 Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Fri, 9 Aug 2024 13:18:21 +0100 Subject: [PATCH 2/3] tools/batch-rebase: Improve error message when parsing configuration FIX Since multiple formats are tried, show what format was being assumed when the error was raised. --- tools/batch-rebase/src/batch_rebase/main.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/batch-rebase/src/batch_rebase/main.py b/tools/batch-rebase/src/batch_rebase/main.py index 71b07ffc1..97b434af4 100755 --- a/tools/batch-rebase/src/batch_rebase/main.py +++ b/tools/batch-rebase/src/batch_rebase/main.py @@ -72,18 +72,19 @@ def load_conf(path): return json.load(f) loaders = [ - load_json, - *([load_yaml] if try_yaml else []) + ('JSON', load_json), + *([('YAML', load_yaml)] if try_yaml else []) ] last_excep = ValueError('No loader selected') - for loader in loaders: + for fmt, loader in loaders: try: conf = loader(path) except Exception as e: last_excep = e - error(e) + error(f'Error while trying to load the configuration as {fmt}: {e}') else: + info(f'Configuration successfully loaded as {fmt}') return postprocess(conf) raise last_excep -- GitLab From 04772ad43ad327dfff2e8f8d248f8c794f55b2c9 Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Fri, 9 Aug 2024 13:21:23 +0100 Subject: [PATCH 3/3] tools/batch-rebase: Always try YAML load and display an error if ruamel.yaml is not installed FIX Ensure that a user expecting YAML to be attempted will get an actionable error message. --- tools/batch-rebase/src/batch_rebase/main.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/tools/batch-rebase/src/batch_rebase/main.py b/tools/batch-rebase/src/batch_rebase/main.py index 97b434af4..b9f395a8d 100755 --- a/tools/batch-rebase/src/batch_rebase/main.py +++ b/tools/batch-rebase/src/batch_rebase/main.py @@ -55,17 +55,15 @@ def load_conf(path): def postprocess(conf): return conf['rebase-conf'] - try: - from ruamel.yaml import YAML - except ImportError: - try_yaml = False - else: - try_yaml = True - def load_yaml(path): - yaml = YAML(typ='safe') - with open(path) as f: - return yaml.load(f) + try: + from ruamel.yaml import YAML + except ImportError: + raise RuntimeError('ruamel.yaml is not installed, YAML support disabled') + else: + yaml = YAML(typ='safe') + with open(path) as f: + return yaml.load(f) def load_json(path): with open(path) as f: @@ -73,7 +71,7 @@ def load_conf(path): loaders = [ ('JSON', load_json), - *([('YAML', load_yaml)] if try_yaml else []) + ('YAML', load_yaml), ] last_excep = ValueError('No loader selected') -- GitLab