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

packp/capability: Skip argument validations for unknown capabilities #626

Merged
merged 1 commit into from Oct 29, 2017

Conversation

Projects
None yet
4 participants
@orirawlings
Copy link
Collaborator

orirawlings commented Oct 23, 2017

Fixes #623

@orirawlings orirawlings requested a review from mcuadros Oct 23, 2017

@codecov

This comment has been minimized.

Copy link

codecov bot commented Oct 23, 2017

Codecov Report

Merging #626 into master will decrease coverage by 0.56%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #626      +/-   ##
==========================================
- Coverage   78.26%   77.69%   -0.57%     
==========================================
  Files         130      130              
  Lines       10197    10201       +4     
==========================================
- Hits         7981     7926      -55     
- Misses       1356     1428      +72     
+ Partials      860      847      -13
Impacted Files Coverage Δ
plumbing/protocol/packp/capability/capability.go 100% <ø> (ø) ⬆️
plumbing/protocol/packp/capability/list.go 97.53% <100%> (+0.12%) ⬆️
plumbing/transport/ssh/common.go 20.54% <0%> (-45.21%) ⬇️
plumbing/transport/ssh/auth_method.go 31.57% <0%> (-22.81%) ⬇️

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 b345ed7...4b2b049. Read the comment docs.

@dsvensson

This comment has been minimized.

Copy link

dsvensson commented Oct 23, 2017

Issue creator here... this is the output before vs after this bug fix (tl;dr fix works as expected):

$ cat foo.go
package main

import (
    "fmt"
    git "gopkg.in/src-d/go-git.v4"
)

func main() {
    r, err := git.PlainClone("output", false, &git.CloneOptions{
        URL:               "https://gopkg.in/yaml.v2",
        RecurseSubmodules: git.DefaultSubmoduleRecursionDepth,
    })
    fmt.Println(err)
    fmt.Println(r)
}
go run foo.go
pkt-line 3: invalid capabilities: arguments not allowed (symref=HEAD:refs/heads/v2 multi_ack thin-pack side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed no-done oldref=HEAD:refs/heads/v2 agent=git/github-g09a9ed6d3a)
&{0xc4200683c0 map[] 0xc420121620}
$ ls output
$ find output
output
output/.git
output/.git/config
output/.git/HEAD
output/.git/objects
output/.git/objects/info
output/.git/objects/pack
output/.git/refs
output/.git/refs/heads
output/.git/refs/tags
$ cat output/.git/config
[core]
bare = false
[remote "origin"]
url = https://gopkg.in/yaml.v2
fetch = +refs/heads/*:refs/remotes/origin/*

$ mv vendor/gopkg.in/src-d/go-git.v4 vendor/gopkg.in/src-d/go-git.v4.orig
$ ln -sf ../../../go-git-fixed vendor/gopkg.in/src-d/go-git.v4
$ rm -rf output
$ go run foo.go
<nil>
&{0xc4200683c0 map[] 0xc42012b640}
$ find output
output
output/.git
output/.git/config
output/.git/HEAD
output/.git/index
output/.git/objects
output/.git/objects/info
output/.git/objects/pack
output/.git/objects/pack/pack-fc51ec5c6df85d7b5bc63a332f75e089ecccbff7.idx
output/.git/objects/pack/pack-fc51ec5c6df85d7b5bc63a332f75e089ecccbff7.pack
output/.git/refs
output/.git/refs/heads
output/.git/refs/heads/v2
output/.git/refs/remotes
output/.git/refs/remotes/origin
output/.git/refs/remotes/origin/master
output/.git/refs/remotes/origin/rename-imports
output/.git/refs/remotes/origin/v1
output/.git/refs/remotes/origin/v2
output/.git/refs/tags
output/.travis.yml
output/apic.go
output/decode.go
output/decode_test.go
output/emitterc.go
output/encode.go
output/encode_test.go
output/example_embedded_test.go
output/LICENSE
output/LICENSE.libyaml
output/parserc.go
output/readerc.go
output/README.md
output/resolve.go
output/scannerc.go
output/sorter.go
output/suite_test.go
output/writerc.go
output/yaml.go
output/yamlh.go
output/yamlprivateh.go

@orirawlings orirawlings requested review from ajnavarro and erizocosmico Oct 25, 2017

@mcuadros mcuadros merged commit 18132ca into src-d:master Oct 29, 2017

4 checks passed

codecov/patch 100% of diff hit (target 78.26%)
Details
codecov/project Absolute coverage decreased by -0.56% but relative coverage increased by +21.73% compared to b345ed7
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.