From d1b1dcc82796e8a04ce6c8d2efd9d031261ee4a5 Mon Sep 17 00:00:00 2001 From: Pete Emerson Date: Thu, 12 Nov 2020 08:50:24 -0800 Subject: [PATCH 01/10] Intial plan for implementing git refs --- README.md | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ad81be5..388c007 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ authentication. Fetch makes it possible to handle all of these cases with a one- #### Features -- Download from a specific git tag, branch, or commit SHA. +- Download from a generic git reference or specific git tag, branch, or commit SHA. - Download a single file, a subset of files, or all files from the repo. - Download one or more binary assets from a specific release that match a regular expression. - Verify the SHA256 or SHA512 checksum of a binary asset. @@ -79,6 +79,7 @@ The supported options are: - `--branch` (**Optional**): The git branch from which to download; the latest commit in the branch will be used. If specified, will override `--tag`. - `--commit` (**Optional**): The SHA of a git commit to download. If specified, will override `--branch` and `--tag`. +- `--ref` (**Optional**): The git reference to download. If specified, will override `--commit`, `--branch`, and `--tag`. - `--source-path` (**Optional**): The source path to download from the repo (e.g. `--source-path=/folder` will download the `/folder` path and all files below it). By default, all files are downloaded from the repo unless `--source-path` or `--release-asset` is specified. This option can be specified more than once. @@ -163,7 +164,7 @@ fetch --repo="https://github.com/foo/bar" --branch="sample-branch" /tmp/josh1 Download all files from the git commit `f32a08313e30f116a1f5617b8b68c11f1c1dbb61`, and save them to `/tmp`: ``` -fetch --repo="https://github.com/foo/bar" --commit="f32a08313e30f116a1f5617b8b68c11f1c1dbb61" /tmp/josh1 +fetch --repo="https://github.com/foo/bar" --commit="f32a08313e30f116a1f5617b8b68c11f1c1dbb61" /tmp ``` #### Usage Example 6 @@ -182,6 +183,14 @@ Download the release asset `foo.exe` from a GitHub release hosted on a GitHub En fetch --repo="https://ghe.mycompany.com/foo/bar" --tag="0.1.5" --release-asset="foo.exe" /tmp ``` +#### Usage Example 8 + +Download all files from the latest commit on the `sample-branch` branch by specifying the branch as a git reference, and save them to `/tmp`: + +``` +fetch --repo="https://github.com/foo/bar" --ref="sample-branch" /tmp +``` + ## License This code is released under the MIT License. See [LICENSE.txt](/LICENSE.txt). @@ -191,3 +200,12 @@ This code is released under the MIT License. See [LICENSE.txt](/LICENSE.txt). - Introduce code verification using something like GPG signatures or published checksums - Explicitly test for exotic repo and org names - Apply stricter parsing for repo-filter command-line arg + +## Issue #73 + +- Modify CLI to accept --ref +- --ref takes precidence over --commit, --branch, --tag (in validateOptions) +- downloadSourcePaths() adds gitRef parameter +- Modify GitHubCommit to add a `GitRef` of type string +- For testing in `file_test.go`, all commit, branch, and tag tests should be repeated with gitref specified instead of the specific type. + From 67fa66168a4ad9bfd15d6492ca1193586a060c49 Mon Sep 17 00:00:00 2001 From: Pete Emerson Date: Thu, 12 Nov 2020 09:01:53 -0800 Subject: [PATCH 02/10] Make examples use --ref except for tag filtering --- README.md | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 388c007..528af60 100644 --- a/README.md +++ b/README.md @@ -138,7 +138,7 @@ fetch --repo="https://github.com/foo/bar" --tag="~>0.1.5" --source-path="/module Download all files in `/modules/foo` from a GitHub release where the tag is exactly `0.1.5`, and save them to `/tmp`: ``` -fetch --repo="https://github.com/foo/bar" --tag="0.1.5" --source-path="/modules/foo" /tmp +fetch --repo="https://github.com/foo/bar" --ref="0.1.5" --source-path="/modules/foo" /tmp ``` #### Usage Example 3 @@ -148,7 +148,7 @@ Download all files from a private GitHub repo using the GitHUb oAuth Token `123` ``` GITHUB_OAUTH_TOKEN=123 -fetch --repo="https://github.com/foo/bar" --tag="0.1.5" /tmp +fetch --repo="https://github.com/foo/bar" --ref="0.1.5" /tmp ``` #### Usage Example 4 @@ -156,7 +156,7 @@ fetch --repo="https://github.com/foo/bar" --tag="0.1.5" /tmp Download all files from the latest commit on the `sample-branch` branch, and save them to `/tmp`: ``` -fetch --repo="https://github.com/foo/bar" --branch="sample-branch" /tmp/josh1 +fetch --repo="https://github.com/foo/bar" --ref="sample-branch" /tmp/josh1 ``` #### Usage Example 5 @@ -164,7 +164,7 @@ fetch --repo="https://github.com/foo/bar" --branch="sample-branch" /tmp/josh1 Download all files from the git commit `f32a08313e30f116a1f5617b8b68c11f1c1dbb61`, and save them to `/tmp`: ``` -fetch --repo="https://github.com/foo/bar" --commit="f32a08313e30f116a1f5617b8b68c11f1c1dbb61" /tmp +fetch --repo="https://github.com/foo/bar" --ref="f32a08313e30f116a1f5617b8b68c11f1c1dbb61" /tmp ``` #### Usage Example 6 @@ -172,7 +172,7 @@ fetch --repo="https://github.com/foo/bar" --commit="f32a08313e30f116a1f5617b8b68 Download the release asset `foo.exe` from a GitHub release where the tag is exactly `0.1.5`, and save it to `/tmp`: ``` -fetch --repo="https://github.com/foo/bar" --tag="0.1.5" --release-asset="foo.exe" /tmp +fetch --repo="https://github.com/foo/bar" --ref="0.1.5" --release-asset="foo.exe" /tmp ``` #### Usage Example 7 @@ -180,15 +180,7 @@ fetch --repo="https://github.com/foo/bar" --tag="0.1.5" --release-asset="foo.exe Download the release asset `foo.exe` from a GitHub release hosted on a GitHub Enterprise instance running at `ghe.mycompany.com` where the tag is exactly `0.1.5`, and save it to `/tmp`: ``` -fetch --repo="https://ghe.mycompany.com/foo/bar" --tag="0.1.5" --release-asset="foo.exe" /tmp -``` - -#### Usage Example 8 - -Download all files from the latest commit on the `sample-branch` branch by specifying the branch as a git reference, and save them to `/tmp`: - -``` -fetch --repo="https://github.com/foo/bar" --ref="sample-branch" /tmp +fetch --repo="https://ghe.mycompany.com/foo/bar" --ref="0.1.5" --release-asset="foo.exe" /tmp ``` ## License From 059502dcb21b4b72b5fe9a42ca3dabc915faa3e7 Mon Sep 17 00:00:00 2001 From: Pete Emerson Date: Thu, 12 Nov 2020 09:02:48 -0800 Subject: [PATCH 03/10] Put --ref first in the README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 528af60..2e1a24d 100644 --- a/README.md +++ b/README.md @@ -74,12 +74,12 @@ fetch [OPTIONS] The supported options are: - `--repo` (**Required**): The fully qualified URL of the GitHub repo to download from (e.g. https://github.com/foo/bar). +- `--ref` (**Optional**): The git reference to download. If specified, will override `--commit`, `--branch`, and `--tag`. - `--tag` (**Optional**): The git tag to download. Can be a specific tag or a [Tag Constraint Expression](#tag-constraint-expressions). - `--branch` (**Optional**): The git branch from which to download; the latest commit in the branch will be used. If specified, will override `--tag`. - `--commit` (**Optional**): The SHA of a git commit to download. If specified, will override `--branch` and `--tag`. -- `--ref` (**Optional**): The git reference to download. If specified, will override `--commit`, `--branch`, and `--tag`. - `--source-path` (**Optional**): The source path to download from the repo (e.g. `--source-path=/folder` will download the `/folder` path and all files below it). By default, all files are downloaded from the repo unless `--source-path` or `--release-asset` is specified. This option can be specified more than once. From 5186bd5ee0f793d8806ca193631b1ba83239ae7c Mon Sep 17 00:00:00 2001 From: Pete Emerson Date: Thu, 12 Nov 2020 09:04:27 -0800 Subject: [PATCH 04/10] Update README.md Co-authored-by: Yevgeniy Brikman --- README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/README.md b/README.md index 2e1a24d..b9d6829 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ authentication. Fetch makes it possible to handle all of these cases with a one- #### Features -- Download from a generic git reference or specific git tag, branch, or commit SHA. +- Download from any git reference, such as a specific git tag, branch, or commit SHA. - Download a single file, a subset of files, or all files from the repo. - Download one or more binary assets from a specific release that match a regular expression. - Verify the SHA256 or SHA512 checksum of a binary asset. @@ -200,4 +200,3 @@ This code is released under the MIT License. See [LICENSE.txt](/LICENSE.txt). - downloadSourcePaths() adds gitRef parameter - Modify GitHubCommit to add a `GitRef` of type string - For testing in `file_test.go`, all commit, branch, and tag tests should be repeated with gitref specified instead of the specific type. - From b72cedd08ecc39aa11e818fc1595cc158eaff885 Mon Sep 17 00:00:00 2001 From: Pete Emerson Date: Thu, 12 Nov 2020 09:09:16 -0800 Subject: [PATCH 05/10] Add thought about detecting tag constraints --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 2e1a24d..b8de2bb 100644 --- a/README.md +++ b/README.md @@ -200,4 +200,5 @@ This code is released under the MIT License. See [LICENSE.txt](/LICENSE.txt). - downloadSourcePaths() adds gitRef parameter - Modify GitHubCommit to add a `GitRef` of type string - For testing in `file_test.go`, all commit, branch, and tag tests should be repeated with gitref specified instead of the specific type. +- Needs more thought by Pete: can `--ref` detect a tag constraint and do the right thing? That'd be great. Let's see how time permits. From 70562d519b4f511182967e69f07da9bbb8746773 Mon Sep 17 00:00:00 2001 From: Pete Emerson Date: Thu, 12 Nov 2020 09:24:28 -0800 Subject: [PATCH 06/10] First cut at working code ``` $ go build . && ./fetch --repo=https://github.com/pete0emerson/demo --ref=banana demo Downloading git reference "banana" of https://github.com/pete0emerson/demo ... Extracting files from / to demo ... 2 files extracted Download and file extraction complete. ``` Note that this doesn't do any testing yet (booo). It also doesn't consider the parsing of the ref for a potential tag constraint. --- README.md | 8 ++++---- file.go | 4 +++- github.go | 1 + main.go | 20 +++++++++++++++----- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index b923652..3056014 100644 --- a/README.md +++ b/README.md @@ -195,10 +195,10 @@ This code is released under the MIT License. See [LICENSE.txt](/LICENSE.txt). ## Issue #73 -- Modify CLI to accept --ref -- --ref takes precidence over --commit, --branch, --tag (in validateOptions) -- downloadSourcePaths() adds gitRef parameter -- Modify GitHubCommit to add a `GitRef` of type string +- ~~Modify CLI to accept --ref~~ +- ~~--ref takes precidence over --commit, --branch, --tag (in validateOptions)~~ +- ~~downloadSourcePaths() adds gitRef parameter~~ +- ~~Modify GitHubCommit to add a `GitRef` of type string~~ - For testing in `file_test.go`, all commit, branch, and tag tests should be repeated with gitref specified instead of the specific type. - Needs more thought by Pete: can `--ref` detect a tag constraint and do the right thing? That'd be great. Let's see how time permits. diff --git a/file.go b/file.go index fce4072..73f2fb0 100644 --- a/file.go +++ b/file.go @@ -151,7 +151,9 @@ func MakeGitHubZipFileRequest(gitHubCommit GitHubCommit, gitHubToken string, ins // This represents either a commit, branch, or git tag var gitRef string - if gitHubCommit.CommitSha != "" { + if gitHubCommit.GitRef != "" { + gitRef = gitHubCommit.GitRef + } else if gitHubCommit.CommitSha != "" { gitRef = gitHubCommit.CommitSha } else if gitHubCommit.BranchName != "" { gitRef = gitHubCommit.BranchName diff --git a/github.go b/github.go index 0669639..ba5f09c 100644 --- a/github.go +++ b/github.go @@ -37,6 +37,7 @@ type GitHubInstance struct { // - Example: BranchName alone is specified; use BranchName type GitHubCommit struct { Repo GitHubRepo // The GitHub repo where this release lives + GitRef string // The git reference GitTag string // The specific git tag for this release BranchName string // If specified, indicates that this commit should be the latest commit on the given branch CommitSha string // If specified, indicates that this commit should be exactly this Git Commit SHA. diff --git a/main.go b/main.go index bf399a8..1f967e1 100644 --- a/main.go +++ b/main.go @@ -18,6 +18,7 @@ var VERSION string type FetchOptions struct { RepoUrl string + GitRef string CommitSha string BranchName string TagConstraint string @@ -37,6 +38,7 @@ type AssetDownloadResult struct { } const optionRepo = "repo" +const optionRef = "ref" const optionCommit = "commit" const optionBranch = "branch" const optionTag = "tag" @@ -62,6 +64,10 @@ func main() { Name: optionRepo, Usage: "Required. Fully qualified URL of the GitHub repo.", }, + cli.StringFlag{ + Name: optionRef, + Usage: "The git reference to download. If specified, will override --commit, --branch, and --tag.", + }, cli.StringFlag{ Name: optionCommit, Usage: "The specific git commit SHA to download. If specified, will override --branch and --tag.", @@ -171,7 +177,7 @@ func runFetch(c *cli.Context) error { } // Download any requested source files - if err := downloadSourcePaths(options.SourcePaths, options.LocalDownloadPath, repo, desiredTag, options.BranchName, options.CommitSha, instance); err != nil { + if err := downloadSourcePaths(options.SourcePaths, options.LocalDownloadPath, repo, options.GitRef, desiredTag, options.BranchName, options.CommitSha, instance); err != nil { return err } @@ -214,6 +220,7 @@ func parseOptions(c *cli.Context) FetchOptions { return FetchOptions{ RepoUrl: c.String(optionRepo), + GitRef: c.String(optionRef), CommitSha: c.String(optionCommit), BranchName: c.String(optionBranch), TagConstraint: c.String(optionTag), @@ -237,8 +244,8 @@ func validateOptions(options FetchOptions) error { return fmt.Errorf("Missing required arguments specifying the local download path. Run \"fetch --help\" for full usage info.") } - if options.TagConstraint == "" && options.CommitSha == "" && options.BranchName == "" { - return fmt.Errorf("You must specify exactly one of --%s, --%s, or --%s. Run \"fetch --help\" for full usage info.", optionTag, optionCommit, optionBranch) + if options.GitRef == "" && options.TagConstraint == "" && options.CommitSha == "" && options.BranchName == "" { + return fmt.Errorf("You must specify exactly one of --%s, --%s, --%s, or --%s. Run \"fetch --help\" for full usage info.", optionRef, optionTag, optionCommit, optionBranch) } if options.ReleaseAsset != "" && options.TagConstraint == "" { @@ -253,7 +260,7 @@ func validateOptions(options FetchOptions) error { } // Download the specified source files from the given repo -func downloadSourcePaths(sourcePaths []string, destPath string, githubRepo GitHubRepo, latestTag string, branchName string, commitSha string, instance GitHubInstance) error { +func downloadSourcePaths(sourcePaths []string, destPath string, githubRepo GitHubRepo, gitRef string, latestTag string, branchName string, commitSha string, instance GitHubInstance) error { if len(sourcePaths) == 0 { return nil } @@ -264,13 +271,16 @@ func downloadSourcePaths(sourcePaths []string, destPath string, githubRepo GitHu // So we can guarantee (at least logically) that this struct instance is in a valid state right now. gitHubCommit := GitHubCommit{ Repo: githubRepo, + GitRef: gitRef, GitTag: latestTag, BranchName: branchName, CommitSha: commitSha, } // Download that release as a .zip file - if gitHubCommit.CommitSha != "" { + if gitHubCommit.GitRef != "" { + fmt.Printf("Downloading git reference \"%s\" of %s ...\n", gitHubCommit.GitRef, githubRepo.Url) + } else if gitHubCommit.CommitSha != "" { fmt.Printf("Downloading git commit \"%s\" of %s ...\n", gitHubCommit.CommitSha, githubRepo.Url) } else if gitHubCommit.BranchName != "" { fmt.Printf("Downloading latest commit from branch \"%s\" of %s ...\n", gitHubCommit.BranchName, githubRepo.Url) From ee34bc76a4021a38a946511b313b88187180c5c2 Mon Sep 17 00:00:00 2001 From: Pete Emerson Date: Thu, 12 Nov 2020 09:47:35 -0800 Subject: [PATCH 07/10] Test GitRef for each type of test --- file_test.go | 228 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 146 insertions(+), 82 deletions(-) diff --git a/file_test.go b/file_test.go index 40e5f78..c04e7b6 100644 --- a/file_test.go +++ b/file_test.go @@ -44,44 +44,56 @@ func TestDownloadGitTagZipFile(t *testing.T) { } for _, tc := range cases { - gitHubCommit := GitHubCommit{ - Repo: GitHubRepo{ - Owner: tc.repoOwner, - Name: tc.repoName, + gitHubCommits := []GitHubCommit{ + // Test as a GitTag + GitHubCommit{ + Repo: GitHubRepo{ + Owner: tc.repoOwner, + Name: tc.repoName, + }, + GitTag: tc.gitTag, + }, + // Test as a GitRef + GitHubCommit{ + Repo: GitHubRepo{ + Owner: tc.repoOwner, + Name: tc.repoName, + }, + GitRef: tc.gitTag, }, - GitTag: tc.gitTag, } + for _, gitHubCommit := range gitHubCommits { + zipFilePath, err := downloadGithubZipFile(gitHubCommit, tc.githubToken, tc.instance) - zipFilePath, err := downloadGithubZipFile(gitHubCommit, tc.githubToken, tc.instance) - - defer os.RemoveAll(zipFilePath) + defer os.RemoveAll(zipFilePath) - // We don't have a running instance of GitHub Enterprise against which to validate tests as we do for GitHub public, - // so this test will only validate that fetch attempted to download from the expected URL. The download itself - // will fail. + // We don't have a running instance of GitHub Enterprise against which to validate tests as we do for GitHub public, + // so this test will only validate that fetch attempted to download from the expected URL. The download itself + // will fail. - githubEnterpriseDownloadUrl := fmt.Sprintf("https://%s/repos/%s/%s/zipball/%s", tc.instance.ApiUrl, tc.repoOwner, tc.repoName, tc.gitTag) + githubEnterpriseDownloadUrl := fmt.Sprintf("https://%s/repos/%s/%s/zipball/%s", tc.instance.ApiUrl, tc.repoOwner, tc.repoName, tc.gitTag) - // TODO: The awkwardness of this test makes it clear that a better structure for this program would be to refactor - // the downloadGithubZipFile() function to a function called downloadGithubFile() that would accept a URL as a - // param. We could then test explicitly that the URL is as expected, which would make GitHub Enterprise test cases - // simpler to handle. + // TODO: The awkwardness of this test makes it clear that a better structure for this program would be to refactor + // the downloadGithubZipFile() function to a function called downloadGithubFile() that would accept a URL as a + // param. We could then test explicitly that the URL is as expected, which would make GitHub Enterprise test cases + // simpler to handle. - if err != nil && strings.Contains(err.Error(), "no such host") { - if strings.Contains(err.Error(), githubEnterpriseDownloadUrl) { - t.Logf("Found expected download URL %s. Download itself failed as expected because no GitHub Enterprise instance exists at the given URL.", githubEnterpriseDownloadUrl) - return - } else { - t.Fatalf("Attempted to download from URL other than the expected download URL of %s. Full error: %s", githubEnterpriseDownloadUrl, err.Error()) + if err != nil && strings.Contains(err.Error(), "no such host") { + if strings.Contains(err.Error(), githubEnterpriseDownloadUrl) { + t.Logf("Found expected download URL %s. Download itself failed as expected because no GitHub Enterprise instance exists at the given URL.", githubEnterpriseDownloadUrl) + return + } else { + t.Fatalf("Attempted to download from URL other than the expected download URL of %s. Full error: %s", githubEnterpriseDownloadUrl, err.Error()) + } } - } - if err != nil { - t.Fatalf("Failed to download file: %s", err) - } + if err != nil { + t.Fatalf("Failed to download file: %s", err) + } - if _, err := os.Stat(zipFilePath); os.IsNotExist(err) { - t.Fatalf("Downloaded file doesn't exist at the expected path of %s", zipFilePath) + if _, err := os.Stat(zipFilePath); os.IsNotExist(err) { + t.Fatalf("Downloaded file doesn't exist at the expected path of %s", zipFilePath) + } } } } @@ -106,22 +118,32 @@ func TestDownloadGitBranchZipFile(t *testing.T) { } for _, tc := range cases { - gitHubCommit := GitHubCommit{ - Repo: GitHubRepo{ - Owner: tc.repoOwner, - Name: tc.repoName, + gitHubCommits := []GitHubCommit{ + GitHubCommit{ + Repo: GitHubRepo{ + Owner: tc.repoOwner, + Name: tc.repoName, + }, + BranchName: tc.branchName, + }, + GitHubCommit{ + Repo: GitHubRepo{ + Owner: tc.repoOwner, + Name: tc.repoName, + }, + GitRef: tc.branchName, }, - BranchName: tc.branchName, - } - - zipFilePath, err := downloadGithubZipFile(gitHubCommit, tc.githubToken, tc.instance) - defer os.RemoveAll(zipFilePath) - if err != nil { - t.Fatalf("Failed to download file: %s", err) } + for _, gitHubCommit := range gitHubCommits { + zipFilePath, err := downloadGithubZipFile(gitHubCommit, tc.githubToken, tc.instance) + defer os.RemoveAll(zipFilePath) + if err != nil { + t.Fatalf("Failed to download file: %s", err) + } - if _, err := os.Stat(zipFilePath); os.IsNotExist(err) { - t.Fatalf("Downloaded file doesn't exist at the expected path of %s", zipFilePath) + if _, err := os.Stat(zipFilePath); os.IsNotExist(err) { + t.Fatalf("Downloaded file doesn't exist at the expected path of %s", zipFilePath) + } } } } @@ -145,18 +167,28 @@ func TestDownloadBadGitBranchZipFile(t *testing.T) { } for _, tc := range cases { - gitHubCommit := GitHubCommit{ - Repo: GitHubRepo{ - Owner: tc.repoOwner, - Name: tc.repoName, + gitHubCommits := []GitHubCommit{ + GitHubCommit{ + Repo: GitHubRepo{ + Owner: tc.repoOwner, + Name: tc.repoName, + }, + BranchName: tc.branchName, + }, + GitHubCommit{ + Repo: GitHubRepo{ + Owner: tc.repoOwner, + Name: tc.repoName, + }, + GitRef: tc.branchName, }, - BranchName: tc.branchName, } - - zipFilePath, err := downloadGithubZipFile(gitHubCommit, tc.githubToken, tc.instance) - defer os.RemoveAll(zipFilePath) - if err == nil { - t.Fatalf("Expected that attempt to download repo %s/%s for branch \"%s\" would fail, but received no error.", tc.repoOwner, tc.repoName, tc.branchName) + for _, gitHubCommit := range gitHubCommits { + zipFilePath, err := downloadGithubZipFile(gitHubCommit, tc.githubToken, tc.instance) + defer os.RemoveAll(zipFilePath) + if err == nil { + t.Fatalf("Expected that attempt to download repo %s/%s for branch \"%s\" would fail, but received no error.", tc.repoOwner, tc.repoName, tc.branchName) + } } } } @@ -183,22 +215,32 @@ func TestDownloadGitCommitFile(t *testing.T) { } for _, tc := range cases { - gitHubCommit := GitHubCommit{ - Repo: GitHubRepo{ - Owner: tc.repoOwner, - Name: tc.repoName, + GitHubCommits := []GitHubCommit{ + GitHubCommit{ + Repo: GitHubRepo{ + Owner: tc.repoOwner, + Name: tc.repoName, + }, + CommitSha: tc.commitSha, + }, + GitHubCommit{ + Repo: GitHubRepo{ + Owner: tc.repoOwner, + Name: tc.repoName, + }, + GitRef: tc.commitSha, }, - CommitSha: tc.commitSha, - } - - zipFilePath, err := downloadGithubZipFile(gitHubCommit, tc.githubToken, tc.instance) - defer os.RemoveAll(zipFilePath) - if err != nil { - t.Fatalf("Failed to download file: %s", err) } + for _, gitHubCommit := range GitHubCommits { + zipFilePath, err := downloadGithubZipFile(gitHubCommit, tc.githubToken, tc.instance) + defer os.RemoveAll(zipFilePath) + if err != nil { + t.Fatalf("Failed to download file: %s", err) + } - if _, err := os.Stat(zipFilePath); os.IsNotExist(err) { - t.Fatalf("Downloaded file doesn't exist at the expected path of %s", zipFilePath) + if _, err := os.Stat(zipFilePath); os.IsNotExist(err) { + t.Fatalf("Downloaded file doesn't exist at the expected path of %s", zipFilePath) + } } } } @@ -227,18 +269,29 @@ func TestDownloadBadGitCommitFile(t *testing.T) { } for _, tc := range cases { - gitHubCommit := GitHubCommit{ - Repo: GitHubRepo{ - Owner: tc.repoOwner, - Name: tc.repoName, + + gitHubCommits := []GitHubCommit{ + GitHubCommit{ + Repo: GitHubRepo{ + Owner: tc.repoOwner, + Name: tc.repoName, + }, + CommitSha: tc.commitSha, + }, + GitHubCommit{ + Repo: GitHubRepo{ + Owner: tc.repoOwner, + Name: tc.repoName, + }, + GitRef: tc.commitSha, }, - CommitSha: tc.commitSha, } - - zipFilePath, err := downloadGithubZipFile(gitHubCommit, tc.githubToken, tc.instance) - defer os.RemoveAll(zipFilePath) - if err == nil { - t.Fatalf("Expected that attempt to download repo %s/%s at commmit sha \"%s\" would fail, but received no error.", tc.repoOwner, tc.repoName, tc.commitSha) + for _, gitHubCommit := range gitHubCommits { + zipFilePath, err := downloadGithubZipFile(gitHubCommit, tc.githubToken, tc.instance) + defer os.RemoveAll(zipFilePath) + if err == nil { + t.Fatalf("Expected that attempt to download repo %s/%s at commmit sha \"%s\" would fail, but received no error.", tc.repoOwner, tc.repoName, tc.commitSha) + } } } } @@ -262,17 +315,28 @@ func TestDownloadZipFileWithBadRepoValues(t *testing.T) { } for _, tc := range cases { - gitHubCommit := GitHubCommit{ - Repo: GitHubRepo{ - Owner: tc.repoOwner, - Name: tc.repoName, + gitHubCommits := []GitHubCommit{ + GitHubCommit{ + Repo: GitHubRepo{ + Owner: tc.repoOwner, + Name: tc.repoName, + }, + GitTag: tc.gitTag, + }, + GitHubCommit{ + Repo: GitHubRepo{ + Owner: tc.repoOwner, + Name: tc.repoName, + }, + GitRef: tc.gitTag, }, - GitTag: tc.gitTag, } + for _, gitHubCommit := range gitHubCommits { - _, err := downloadGithubZipFile(gitHubCommit, tc.githubToken, tc.instance) - if err == nil && err.errorCode != 500 { - t.Fatalf("Expected error for bad repo values: %s/%s:%s", tc.repoOwner, tc.repoName, tc.gitTag) + _, err := downloadGithubZipFile(gitHubCommit, tc.githubToken, tc.instance) + if err == nil && err.errorCode != 500 { + t.Fatalf("Expected error for bad repo values: %s/%s:%s", tc.repoOwner, tc.repoName, tc.gitTag) + } } } } From 10f4cca028bf73e75dcde720e6714043e69e634e Mon Sep 17 00:00:00 2001 From: Pete Emerson Date: Thu, 12 Nov 2020 10:25:58 -0800 Subject: [PATCH 08/10] Make tag constraints work with --ref --- main.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/main.go b/main.go index 1f967e1..e332c5d 100644 --- a/main.go +++ b/main.go @@ -151,10 +151,21 @@ func runFetch(c *cli.Context) error { } } - specific, desiredTag := isTagConstraintSpecificTag(options.TagConstraint) + var specific bool + var desiredTag string + var tagConstraint string + + if options.GitRef != "" { + specific, desiredTag = isTagConstraintSpecificTag(options.GitRef) + tagConstraint = options.GitRef + } else { + specific, desiredTag = isTagConstraintSpecificTag(options.TagConstraint) + tagConstraint = options.TagConstraint + } + if !specific { // Find the specific release that matches the latest version constraint - latestTag, err := getLatestAcceptableTag(options.TagConstraint, tags) + latestTag, err := getLatestAcceptableTag(tagConstraint, tags) if err != nil { if err.errorCode == invalidTagConstraintExpression { return errors.New(getErrorMessage(invalidTagConstraintExpression, err.details)) @@ -271,13 +282,14 @@ func downloadSourcePaths(sourcePaths []string, destPath string, githubRepo GitHu // So we can guarantee (at least logically) that this struct instance is in a valid state right now. gitHubCommit := GitHubCommit{ Repo: githubRepo, - GitRef: gitRef, + GitRef: latestTag, GitTag: latestTag, BranchName: branchName, CommitSha: commitSha, } // Download that release as a .zip file + if gitHubCommit.GitRef != "" { fmt.Printf("Downloading git reference \"%s\" of %s ...\n", gitHubCommit.GitRef, githubRepo.Url) } else if gitHubCommit.CommitSha != "" { From d789832a858b84b2ea6886772b3eeade43a6b349 Mon Sep 17 00:00:00 2001 From: Pete Emerson Date: Thu, 12 Nov 2020 10:29:45 -0800 Subject: [PATCH 09/10] Unnecessary parameter to downloadSourcePaths --- main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.go b/main.go index e332c5d..1835213 100644 --- a/main.go +++ b/main.go @@ -188,7 +188,7 @@ func runFetch(c *cli.Context) error { } // Download any requested source files - if err := downloadSourcePaths(options.SourcePaths, options.LocalDownloadPath, repo, options.GitRef, desiredTag, options.BranchName, options.CommitSha, instance); err != nil { + if err := downloadSourcePaths(options.SourcePaths, options.LocalDownloadPath, repo, desiredTag, options.BranchName, options.CommitSha, instance); err != nil { return err } @@ -271,7 +271,7 @@ func validateOptions(options FetchOptions) error { } // Download the specified source files from the given repo -func downloadSourcePaths(sourcePaths []string, destPath string, githubRepo GitHubRepo, gitRef string, latestTag string, branchName string, commitSha string, instance GitHubInstance) error { +func downloadSourcePaths(sourcePaths []string, destPath string, githubRepo GitHubRepo, latestTag string, branchName string, commitSha string, instance GitHubInstance) error { if len(sourcePaths) == 0 { return nil } From e924fe7c92d2ccd5ed56d2f2f18150f15bb2c9a3 Mon Sep 17 00:00:00 2001 From: Pete Emerson Date: Thu, 12 Nov 2020 10:34:25 -0800 Subject: [PATCH 10/10] Remove implementation comments --- README.md | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/README.md b/README.md index 3056014..b18e1ed 100644 --- a/README.md +++ b/README.md @@ -191,14 +191,4 @@ This code is released under the MIT License. See [LICENSE.txt](/LICENSE.txt). - Introduce code verification using something like GPG signatures or published checksums - Explicitly test for exotic repo and org names -- Apply stricter parsing for repo-filter command-line arg - -## Issue #73 - -- ~~Modify CLI to accept --ref~~ -- ~~--ref takes precidence over --commit, --branch, --tag (in validateOptions)~~ -- ~~downloadSourcePaths() adds gitRef parameter~~ -- ~~Modify GitHubCommit to add a `GitRef` of type string~~ -- For testing in `file_test.go`, all commit, branch, and tag tests should be repeated with gitref specified instead of the specific type. -- Needs more thought by Pete: can `--ref` detect a tag constraint and do the right thing? That'd be great. Let's see how time permits. - +- Apply stricter parsing for repo-filter command-line arg \ No newline at end of file