Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove /refs/tags or /refs/heads from Gogs remote File() when using build.Ref #1889

Merged
merged 3 commits into from
Dec 26, 2016

Conversation

mjwwit
Copy link

@mjwwit mjwwit commented Dec 22, 2016

Drone incorrectly used the full Git ref to retrieve a file using the Gogs API.
This is now fixed by removing the path from the ref (when a ref is used to retrieve a file). Behavior when using a Sha is not changed.

This PR fixes issue #1870.

@CLAassistant
Copy link

CLAassistant commented Dec 22, 2016

CLA assistant check
All committers have signed the CLA.

@tboerger
Copy link

You need to call go fmt.

@mjwwit
Copy link
Author

mjwwit commented Dec 22, 2016

Right, forgot about that... Done.

@@ -177,6 +178,12 @@ func (c *client) File(u *model.User, r *model.Repo, b *model.Build, f string) ([
buildRef := b.Commit
if buildRef == "" {
buildRef = b.Ref

// Remove refs/tags or refs/heads, Gogs needs a short ref
refPath := strings.SplitAfterN(b.Ref, "/", 3)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about the following code for high performance?

	refPath := strings.TrimPrefix(
		strings.TrimPrefix(
			b.Ref,
			"refs/heads/",
		),
		"refs/tags/",
	)

https://github.com/drone/drone/blob/master/remote/bitbucketserver/convert.go#L105-L111

@mjwwit
Copy link
Author

mjwwit commented Dec 22, 2016

@appleboy I agree; that implementation is better. I adopted it.

@MehSha
Copy link

MehSha commented Dec 25, 2016

I have tested this with gitea v 1.0 and works very nice.
thank you mjwwit

@bradrydzewski bradrydzewski merged commit 43fbbe4 into harness:master Dec 26, 2016
@jhasse
Copy link

jhasse commented Jan 25, 2017

Missing Features on http://readme.drone.io/0.5/install/setup/gogs/ should probably be updated.

@appleboy
Copy link

@jhasse New document is here: http://readme.drone.io/admin/setup-gogs/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants