From 8bdfcaa165f6cd7a19cb4a2664e6b1ded02f005d Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Thu, 29 Aug 2024 20:29:04 -0700 Subject: [PATCH] submit: Invalidate template cache if origin/main is updated (#370) We calculate the template cache key based on the hashes of the files inside the locations in the repository where tempates are stored, but we check their state at `$trunk` instead of `$remote/$trunk`. This results in the cache not being invalidated if the remote has been updated and fetched, but the local ref is behind. Ref #369 --- .../unreleased/Fixed-20240829-202317.yaml | 3 + branch_submit.go | 4 +- internal/spice/template.go | 8 +- ...submit_pr_template_cache_remote_update.txt | 94 +++++++++++++++++++ 4 files changed, 106 insertions(+), 3 deletions(-) create mode 100644 .changes/unreleased/Fixed-20240829-202317.yaml create mode 100644 testdata/script/issue369_branch_submit_pr_template_cache_remote_update.txt diff --git a/.changes/unreleased/Fixed-20240829-202317.yaml b/.changes/unreleased/Fixed-20240829-202317.yaml new file mode 100644 index 00000000..c7694637 --- /dev/null +++ b/.changes/unreleased/Fixed-20240829-202317.yaml @@ -0,0 +1,3 @@ +kind: Fixed +body: 'submit: Fix outdated PR template being used when trunk is behind its remote ref.' +time: 2024-08-29T20:23:17.238952-07:00 diff --git a/branch_submit.go b/branch_submit.go index c0f66222..cd05892f 100644 --- a/branch_submit.go +++ b/branch_submit.go @@ -295,6 +295,7 @@ func (cmd *branchSubmitCmd) run( svc, store, repo, + remote, remoteRepo, branch.Base, ) @@ -558,6 +559,7 @@ func (cmd *branchSubmitCmd) preparePublish( svc *spice.Service, store *state.Store, repo *git.Repository, + remoteName string, remoteRepo forge.Repository, baseBranch string, ) (*preparedBranch, error) { @@ -569,7 +571,7 @@ func (cmd *branchSubmitCmd) preparePublish( ctx, cancel := context.WithTimeout(ctx, time.Second) defer cancel() - templates, err := svc.ListChangeTemplates(ctx, remoteRepo) + templates, err := svc.ListChangeTemplates(ctx, remoteName, remoteRepo) if err != nil { log.Warn("Could not list change templates", "error", err) templates = nil diff --git a/internal/spice/template.go b/internal/spice/template.go index e7312ff7..d1207b29 100644 --- a/internal/spice/template.go +++ b/internal/spice/template.go @@ -14,7 +14,11 @@ import ( // ListChangeTemplates returns the Change templates defined in the repository. // For GitHub, these are PR templates. -func (s *Service) ListChangeTemplates(ctx context.Context, fr forge.Repository) ([]*forge.ChangeTemplate, error) { +func (s *Service) ListChangeTemplates( + ctx context.Context, + remoteName string, + fr forge.Repository, +) ([]*forge.ChangeTemplate, error) { // TODO: Should Repo be injected? templatePaths := fr.Forge().ChangeTemplatePaths() sort.Strings(templatePaths) @@ -26,7 +30,7 @@ func (s *Service) ListChangeTemplates(ctx context.Context, fr forge.Repository) keyHash := sha256.New() _, _ = fmt.Fprintf(keyHash, "%d\n", len(templatePaths)) for _, path := range templatePaths { - h, err := s.repo.HashAt(ctx, s.store.Trunk(), path) + h, err := s.repo.HashAt(ctx, remoteName+"/"+s.store.Trunk(), path) if err != nil { if errors.Is(err, git.ErrNotExist) { _, _ = fmt.Fprintf(keyHash, "0\n") diff --git a/testdata/script/issue369_branch_submit_pr_template_cache_remote_update.txt b/testdata/script/issue369_branch_submit_pr_template_cache_remote_update.txt new file mode 100644 index 00000000..47f36675 --- /dev/null +++ b/testdata/script/issue369_branch_submit_pr_template_cache_remote_update.txt @@ -0,0 +1,94 @@ +# 'branch submit' invalidates template cache +# if remote has an update even if the local branch is behind. +# +# https://github.com/abhinav/git-spice/issues/369 + +as 'Test ' +at '2024-08-29T20:21:22Z' + +# setup +cd repo +git init +git add .shamhub +git commit -m 'Initial commit' + +# set up a fake remote +shamhub init +shamhub new origin alice/example.git +shamhub register alice +git push origin main + +env SHAMHUB_USERNAME=alice +gs auth login + +# Submit a PR with the first template. +git add feature1.txt +gs bc -m feature1 +gs branch submit --fill + +# Update the template remotely +cd $WORK +shamhub clone alice/example.git fork +cd fork +mv $WORK/extra/CHANGE_TEMPLATE.md .shamhub/CHANGE_TEMPLATE.md +git add .shamhub/CHANGE_TEMPLATE.md +git commit -m 'Change the template' +git push origin main + +# Update origin/main but not main +cd $WORK/repo +git fetch + +# Create a new PR with the new template. +git add feature2.txt +gs bc -m feature2 +gs branch submit --fill + +shamhub dump changes +cmpenv stdout $WORK/golden/pulls.json + +-- repo/.shamhub/CHANGE_TEMPLATE.md -- +This is the first template. + +-- extra/CHANGE_TEMPLATE.md -- +This is the second template. + +-- repo/feature1.txt -- +feature 1 + +-- repo/feature2.txt -- +feature 2 + +-- golden/pulls.json -- +[ + { + "number": 1, + "html_url": "$SHAMHUB_URL/alice/example/change/1", + "state": "open", + "title": "feature1", + "body": "\n\nThis is the first template.\n", + "base": { + "ref": "main", + "sha": "a272771860a18e5e7ee7ef971e749c27574826fd" + }, + "head": { + "ref": "feature1", + "sha": "68b4a07edfb5682221a0207c31de4a99c7dd0d3d" + } + }, + { + "number": 2, + "html_url": "$SHAMHUB_URL/alice/example/change/2", + "state": "open", + "title": "feature2", + "body": "\n\nThis is the second template.\n", + "base": { + "ref": "feature1", + "sha": "68b4a07edfb5682221a0207c31de4a99c7dd0d3d" + }, + "head": { + "ref": "feature2", + "sha": "bfe1a896f408b265d43552a2d450618a181e8f8b" + } + } +]