Fix version() on repositories that tag with a leading "v" (r(170)) - #30
Open
jpazvd wants to merge 1 commit into
Open
Fix version() on repositories that tag with a leading "v" (r(170))#30jpazvd wants to merge 1 commit into
jpazvd wants to merge 1 commit into
Conversation
…rsion> github install ... version(v1.2.3) fails with r(170) on any repository whose tags carry a leading "v". The archive downloads fine, but GitHub names the folder inside it after the tag with the "v" removed, so v18.7.0 unpacks into <package>-18.7.0 while the code cds to <package>-v18.7.0 and stops. The same mismatch affects the -force- path on repositories that have renamed their default branch: archive/master.zip is served for them, but it unpacks into <package>-main rather than <package>-master. Rather than guess, try the plausible names in order. The expected name is tried first, so archives that already resolved are unaffected -- verified against haghish/markdoc 5.0.2, which still resolves on the first candidate. Directory discovery was considered and rejected: this command unpacks into the user's working directory and leaves the folder behind, so a second install in the same directory leaves several <package>-* folders and makes discovery ambiguous. An earlier draft used it and failed exactly that way in testing. Verified in Stata 17, in a directory holding four unpacked archives at once: yaml v2.0.1 -> yaml-2.0.1 (v stripped) yaml master -> yaml-main (default branch renamed) markdoc 5.0.2 -> markdoc-5.0.2 (unchanged, first candidate) wbopendata v18.7.0 -> wbopendata-18.7.0 (v stripped) yaml v9.9.9 -> clean error listing what was tried Note for anyone testing this: -confirm file "dir/."- is not a usable directory test here; it returns 601 whether or not the directory exists. capture cd is used instead, which the surrounding code already relies on.
There was a problem hiding this comment.
Pull request overview
This PR fixes github install <repo>, version(<tag>) failures when GitHub’s ZIP archive unpacks into a directory name that doesn’t exactly match <package>-<version> (notably when tags are v-prefixed or when master.zip unpacks to -main).
Changes:
- Tries multiple plausible unpacked directory names instead of assuming
<package>-<version>. - Adds handling for
v-prefixed tags (try the stripped-vdirectory name). - Adds handling for
master/maindefault-branch mismatches by trying both suffixes.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if !`found' { | ||
| di as err "cannot locate the folder unpacked from `packagename'-`version'.zip" | ||
| di as txt "looked for: `candidates'" | ||
| exit 601 |
Comment on lines
+556
to
+557
| local found 1 | ||
| local dir "`c'" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github install <repo>, version(v1.2.3)fails with r(170) on any repository whose tags carry a leadingv.Cause
The archive downloads correctly, but GitHub names the folder inside it after the tag with the
vremoved. So the code unpacks<package>-v18.7.0.zipinto<package>-18.7.0, then doescd <package>-v18.7.0and stops.Measured directly:
cdtargethaghish/markdoc5.0.2markdoc-5.0.2markdoc-5.0.2✓jpazvd/yamlv2.0.1yaml-2.0.1yaml-v2.0.1✗jpazvd/wbopendatav18.7.0wbopendata-18.7.0wbopendata-v18.7.0✗Your own packages tag without the prefix, which is why this has not surfaced — but
v-prefixed tags are common enough thatversion()is unusable for those repositories today.A second case, same cause
The
forcepath requestsarchive/master.zipand setsversion master. GitHub still serves that archive for repositories that have renamed their default branch, but it unpacks into<package>-main. Socd <package>-masterfails there too.The change
Try the plausible names in order rather than assuming one. The expected name is tried first, so archives that already resolved are untouched.
I considered listing the directory instead and rejected it: the command unpacks into the user's working directory and leaves the folder behind, so a second install in the same directory leaves several
<package>-*folders and makes discovery ambiguous. An earlier draft did exactly that and failed that way in testing.Verified
Stata 17, in a directory deliberately holding four unpacked archives at once:
The
markdocrow is the no-regression check: it still resolves on the first candidate, exactly as before.One note for anyone testing this —
confirm file "dir/."is not a usable directory test here, returning 601 whether or not the directory exists. The patch usescapture cd, which the surrounding code already relies on.Happy to adjust the style or the error wording if you'd prefer something different.