Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

Add a Force flag to Fetch and Clone, mimicking the command-line client #497

Closed
wants to merge 1 commit into from
Closed

Add a Force flag to Fetch and Clone, mimicking the command-line client #497

wants to merge 1 commit into from

Conversation

lupine
Copy link

@lupine lupine commented Jul 22, 2017

See #490

@lupine
Copy link
Author

lupine commented Jul 22, 2017

@mcuadros this is currently lacking tests but you get the basic idea. WDYT?

@codecov
Copy link

codecov bot commented Jul 22, 2017

Codecov Report

Merging #497 into master will decrease coverage by 0.71%.
The diff coverage is 40%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #497      +/-   ##
==========================================
- Coverage   78.08%   77.36%   -0.72%     
==========================================
  Files         129      127       -2     
  Lines        9693     9403     -290     
==========================================
- Hits         7569     7275     -294     
- Misses       1298     1329      +31     
+ Partials      826      799      -27
Impacted Files Coverage Δ
options.go 84.48% <ø> (ø) ⬆️
worktree.go 66.35% <100%> (+1.23%) ⬆️
remote.go 69.72% <35.71%> (-3.79%) ⬇️
plumbing/transport/ssh/common.go 2.81% <0%> (-47.89%) ⬇️
plumbing/transport/ssh/auth_method.go 33.33% <0%> (-24.77%) ⬇️
utils/ioutil/common.go 60% <0%> (-24.62%) ⬇️
plumbing/transport/http/upload_pack.go 61.81% <0%> (-7.28%) ⬇️
plumbing/transport/http/receive_pack.go 76.19% <0%> (-4.77%) ⬇️
plumbing/transport/server/server.go 61.97% <0%> (-3.77%) ⬇️
submodule.go 60.13% <0%> (-3.57%) ⬇️
... and 26 more

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 11a403e...c004dd3. Read the comment docs.

@mcuadros
Copy link
Contributor

Looks good!

@lupine
Copy link
Author

lupine commented Jul 25, 2017

I'm having a little trouble with the tests, actually. Consider:

func (s *RemoteSuite) TestFetchNoForce(c *C) {
       r := newRemote(memory.NewStorage(), &config.RemoteConfig{
               URL: s.GetBasicLocalRepositoryURL(),
       })

       // Can be fast-forwarded
       r.s.SetReference(plumbing.NewReferenceFromStrings(
               "refs/heads/master", "918c48b83bd081e863dbe1b80f8998f058cd8294",
       ))

       s.testFetch(c, r, &FetchOptions{
               RefSpecs: []config.RefSpec{
                       config.RefSpec("+refs/heads/master:refs/remotes/origin/master"),
               },
       }, []*plumbing.Reference{
               plumbing.NewReferenceFromStrings("refs/heads/master", "f7b877701fbf855b44c0a9e86f3fdce2c298b07f"),
               plumbing.NewReferenceFromStrings("refs/remotes/origin/master", "f7b877701fbf855b44c0a9e86f3fdce2c298b07f"),
       })
}

I get the error:

remote_test.go:193:
    c.Assert(exp.String(), Equals, r.String())
... obtained string = "f7b877701fbf855b44c0a9e86f3fdce2c298b07f refs/heads/master"
... expected string = "918c48b83bd081e863dbe1b80f8998f058cd8294 refs/heads/master"

I want to set up the local ref "master" such that the fetch has to update it. I can then set the ref to commits that are (or are not) ancestors and exercise the behaviour that way, but I've not managed to get it working just yet.

I'll look into it again on Thursday, but in the meantime, any tips appreciated!

@mcuadros
Copy link
Contributor

You were lucky? Can you elaborate a little more, I don't understand the goal.

@mcuadros
Copy link
Contributor

mcuadros commented Sep 5, 2017

@lupine any update? If not I will try to take a look to it, since we want to release the v4, and this should be included.

@lupine
Copy link
Author

lupine commented Sep 5, 2017

Sorry @mcuadros, the tests and the safe locking behaviour both prevented me from finishing this, and in the end we went for a "wrap the SSH client" approach over at GitLab.

There's also the question of respecting "+" in refspecs.

If you can finish this off, I'd be super-grateful!

@mcuadros
Copy link
Contributor

mcuadros commented Sep 5, 2017 via email

@lupine
Copy link
Author

lupine commented Sep 5, 2017

That's right :/ we're doing https://gitlab.com/gitlab-org/gitlab-shell/blob/master/lib/gitlab_projects.rb#L432 instead.

I'd love to get it cleaned up to be more like https://gitlab.com/gitlab-org/gitlab-shell/merge_requests/147 but couldn't quite manage it.

@mcuadros
Copy link
Contributor

mcuadros commented Sep 5, 2017

#490 (comment)

@mcuadros mcuadros closed this Sep 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants