Skip to content

Commit

Permalink
submit: Fix outdated PR templates with lower-case names (#371)
Browse files Browse the repository at this point in the history
We use the contents of possible PR template sites
as the hash key for caching the PR templates we get from the remote.
This was only looking at `PULL_REQUEST_TEMPLATE.md` names,
even though `pull_request_template.md` is valid and accepted.

This change ensures that lower-cased versions are also considered
when calculating the hash key.

To do this, we consider upper-case and lower-case variants
of configured Forge template paths when calculating the cache key.
Case-insensitivity of this path is considered part of the Forge contract
because GitHub and GitLab both treat these as case-insensitive-ish.
We can adjust this assumption if we add a forge that doesn't.

Resolves #369
  • Loading branch information
abhinav authored Aug 30, 2024
1 parent 8bdfcaa commit bccac00
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 4 deletions.
3 changes: 3 additions & 0 deletions .changes/unreleased/Fixed-20240829-204353.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
kind: Fixed
body: 'github: Fix outdated PR templates being used when templates used lower-cased file names.'
time: 2024-08-29T20:43:53.225092-07:00
9 changes: 8 additions & 1 deletion internal/forge/shamhub/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,15 @@ func (sh *ShamHub) handleChangeTemplate(w http.ResponseWriter, r *http.Request)
logw, flush := ioutil.LogWriter(sh.log, log.DebugLevel)
defer flush()

templatePaths := make(map[string]struct{})
for _, p := range _changeTemplatePaths {
templatePaths[p] = struct{}{}
templatePaths[strings.ToLower(p)] = struct{}{}
templatePaths[strings.ToUpper(p)] = struct{}{}
}

var res changeTemplateResponse
for _, path := range _changeTemplatePaths {
for path := range templatePaths {
cmd := exec.Command(sh.gitExe, "cat-file", "-p", "HEAD:"+path)
cmd.Dir = sh.repoDir(owner, repo)
cmd.Stderr = logw
Expand Down
18 changes: 15 additions & 3 deletions internal/spice/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import (
"crypto/sha256"
"errors"
"fmt"
"sort"
"maps"
"path"
"slices"
"strings"

"go.abhg.dev/gs/internal/forge"
"go.abhg.dev/gs/internal/git"
Expand All @@ -20,8 +23,17 @@ func (s *Service) ListChangeTemplates(
fr forge.Repository,
) ([]*forge.ChangeTemplate, error) {
// TODO: Should Repo be injected?
templatePaths := fr.Forge().ChangeTemplatePaths()
sort.Strings(templatePaths)
pathSet := make(map[string]struct{})
for _, p := range fr.Forge().ChangeTemplatePaths() {
pathSet[p] = struct{}{}

// Template paths are case-insensitive,
// so we'll also want to check other variants:
dir, file := path.Split(p)
pathSet[path.Join(dir, strings.ToLower(file))] = struct{}{}
pathSet[path.Join(dir, strings.ToUpper(file))] = struct{}{}
}
templatePaths := slices.Sorted(maps.Keys(pathSet))

// Cache key is a SHA256 hash of the following, in order:
// - Number of allowed template paths
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# 'branch submit' invalidates template cache when a template is changed,
# and it has a lower-cased name.
#
# https://github.com/abhinav/git-spice/issues/369

as 'Test <[email protected]>'
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

# Push a new template
gs trunk
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

# 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": "2036762c8e15b28b80f94f5085db6d4d1f2678e8"
},
"head": {
"ref": "feature1",
"sha": "56594727085e14b4c394420451042c322eaf9441"
}
},
{
"number": 2,
"html_url": "$SHAMHUB_URL/alice/example/change/2",
"state": "open",
"title": "feature2",
"body": "\n\nThis is the second template.\n",
"base": {
"ref": "main",
"sha": "2036762c8e15b28b80f94f5085db6d4d1f2678e8"
},
"head": {
"ref": "feature2",
"sha": "91537b46b0c34e9df135cebe1e9270d5d7627952"
}
}
]

0 comments on commit bccac00

Please sign in to comment.