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

Fix LFS Locks over SSH #6999

Merged
merged 8 commits into from
May 28, 2019
Merged

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented May 20, 2019

Whilst looking at adding more tests for #6993 I've noticed that LFS locking was not-preauthenticating correctly over SSH.

This PR fixes this.

Fixes #3084

@zeripath zeripath added this to the 1.9.0 milestone May 20, 2019
@lunny
Copy link
Member

lunny commented May 20, 2019

It seems there is an issue reported that.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 20, 2019
@zeripath
Copy link
Contributor Author

@lunny do you know which issue it is because I can't seem to easily find it.

@lunny
Copy link
Member

lunny commented May 20, 2019

Maybe it is #5478

@zeripath
Copy link
Contributor Author

It's possible that this is part of the fix for that, but I think #6993 is probably more likely to fix that.

I'm going to incorporate this in #6993 in any case.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 20, 2019
@codecov-io
Copy link

codecov-io commented May 20, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@27b271d). Click here to learn what that means.
The diff coverage is 54.32%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #6999   +/-   ##
=========================================
  Coverage          ?   41.43%           
=========================================
  Files             ?      442           
  Lines             ?    59693           
  Branches          ?        0           
=========================================
  Hits              ?    24731           
  Misses            ?    31725           
  Partials          ?     3237
Impacted Files Coverage Δ
routers/routes/routes.go 82.68% <100%> (ø)
modules/lfs/locks.go 48.59% <53.75%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27b271d...82f8938. Read the comment docs.

Copy link
Member

@sapk sapk left a comment

Choose a reason for hiding this comment

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

You need to use the Skip method in place of simple return in skipped tests. Overall looks good but I haven't tested yet.

integrations/git_test.go Show resolved Hide resolved
@zeripath
Copy link
Contributor Author

@sapk have you been able to test this yet? git_test.go does test the locking now btw.

@lafriks lafriks requested a review from sapk May 24, 2019 17:40
integrations/git_test.go Show resolved Hide resolved
routers/routes/routes.go Show resolved Hide resolved
@zeripath zeripath requested a review from lunny May 27, 2019 06:52
@sapk
Copy link
Member

sapk commented May 28, 2019

@zeripath not much time to do so lately. So if someone other validate it should be good as my requested changes are done.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 28, 2019
@zeripath zeripath merged commit 31557b1 into go-gitea:master May 28, 2019
@zeripath zeripath deleted the fix-lfs-locks-over-ssh branch May 28, 2019 10:32
@lunny
Copy link
Member

lunny commented Jun 16, 2019

@zeripath I think this should resolve #7219, could you send back port to v1.8 .

zeripath added a commit to zeripath/gitea that referenced this pull request Jun 16, 2019
* Fix LFS Locks over SSH
* Mark test as skipped
zeripath added a commit that referenced this pull request Jun 17, 2019
* Fix LFS Locks over SSH
* Mark test as skipped
jeffliu27 pushed a commit to jeffliu27/gitea that referenced this pull request Jul 18, 2019
* Fix LFS Locks over SSH
* Mark test as skipped
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Git LFS Lock: credential issue when using SSH
7 participants