← Back to team overview

dulwich-users team mailing list archive

Re: Deleting a remote branch via Dulwich?

 

Awesome, this fixed my problem completely. My code is now working. Thank
you so much.

Does this help? Attached are patches for the two changes discussed in this
thread:

- send_pack should pass determine_wants a copy of the refs dict, not the
refs dict, so that the comparison shortly after can't get confused
- Make usage of 'determine_wants' callbacks explicit in inline documentation

Things to note: I claim in the documentation that determine_wants changes
remote heads even on fetch (because this appears to be the case from
reading the code?); and I do not improve the usage documentation around the
"generate_pack_contents" callbacks, because I do not understand how they
are used.

Also, for future reference, do you prefer proposed patches as patches on
the mailing list, or pull requests on github, or what?

On Wed, Jul 11, 2012 at 5:26 PM, Jelmer Vernooij <jelmer@xxxxxxxxxx> wrote:

> On Sun, Jul 08, 2012 at 01:41:18PM -0700, Andi McClure wrote:
> > Here is a standalone script which reproduces the problem, it's just a
> > connect, the push and some prints. My expected behavior is that on the
> > second run it will print "target is already deleted" because the first
> run
> > will have deleted it. What I see is that on every run it gets an accurate
> > list of heads from the server but then has no effect when it tries to
> > change them. Changing the ZERO_SHA to None has no effect either.
> >
> > Since this connects directly to one of my github repos you will probably
> > get a permissions error if you run it as written, maybe you could clone
> > https://github.com/mcclure/backup-Polycode and then change the "path"
> line
> > to point to your own github account?
> >
> > ######## Start
> >
> > import dulwich.client
> > from dulwich.protocol import ZERO_SHA
> >
> > client = dulwich.client.SSHGitClient("git@xxxxxxxxxx")
> > path = "/mcclure/backup-Polycode"
> > delete_this_head = "refs/heads/bullet279"
> >
> > print("client, path is: %s,%s\ntarget is %s\n" %
> > (client,repr(path),repr(delete_this_head)))
> >
> > def changed(refs):
> > print("determine_wants argument:\n%s\n" % (refs))
> > if delete_this_head in refs:
> > refs[delete_this_head] = ZERO_SHA
> > print("determine_wants returning:\n%s\n" % (refs))
> > else:
> > print("target is already deleted!\n")
> > return refs
>
> Unfortunately your mailer seems to have stripped the whitespace from
> these lines. It looks though like you're trying to modify the existing
> refs dictionary rather than returning your own.
>
> dulwich compares the dictionary it passes in and yours to see if it
> should make any changes to the remote server. A simple fix for this
> should be to add "refs = dict(refs)" as the first instruction in the
> changed() function.
>
> We should probably also improve dulwich by making it harder to shoot
> yourself in the foot like this, and having it take create a copy to
> pass to you.
>
> Cheers,
>
> Jelmer
>
> >
> > On Sun, Jul 8, 2012 at 6:27 AM, Jelmer Vernooij <jelmer@xxxxxxxxx>
> wrote:
> >
> > > On Sat, Jul 07, 2012 at 08:07:51PM -0700, Andi McClure wrote:
> > > > "What do you mean exactly when you say it doesn't work?"
> > >
> > > > When I say it doesn't work, what I mean is it appears to do nothing.
> The
> > > > remote repository is afterward unchanged, looking at github.com.
> > >
> > > > If it's okay to paste a big block, here's exactly what is happening.
> I'm
> > > > running the following code with the argument "key" = "bullet279" and
> the
> > > > referenced value "null_revision_hex" equal to "00"*20.
> > > Can you condense it down into just the bit that reproduces the issue?
> > > This should just do the connect and the call to .send_pack().
> > >
> > > Actually, come to think of it - you might have to set the value to
> > > None rather than the ZERO_SHA1 - have you tried that?
> > >
> > > Cheers,
> > >
> > > Jelmer
> > >
> > > >             client, path =
> > > > > self.git_handler.get_transport_and_path(self.path)
> > > > >             print("client, path is:")
> > > > >             print(client)
> > > > >             print(path)
> > > > >             def changed(refs):
> > > > >                 desired = gitrepo.head_prefix + key
> > > > >                 print("determine_wants argument:")
> > > > >                 print(refs)
> > > > >                 if desired in refs:
> > > > >                     refs[desired] = GitHandler.null_revision_hex
> > > > >                     print("determine_wants return:")
> > > > >                     print(refs)
> > > > >                 return refs
> > > > >             def adebugger(*args):
> > > > >                 print("generate_pack_contents arguments:")
> > > > >                 print(args)
> > > > >                 import pdb; pdb.set_trace()
> > > > >                 return args
> > > > >             genpack = adebugger
> > > > >             try:
> > > > >                 self.git_handler.ui.status(_("creating and sending
> data
> > > > > for remote bookmark deletion\n"))
> > > > >                 changed_refs = client.send_pack(path, changed,
> genpack)
> > > > >                 print("sent_pack returned:")
> > > > >                 print(changed_refs)
> > > > >                 return True
> > > > >             except (HangupException, GitProtocolError), e:
> > > > >                 raise hgutil.Abort(_("git remote error: ") +
> str(e))
> > >
> > >
> > > > This is resulting in the following printed:
> > >
> > > > client, path is:
> > > > > <dulwich.client.SSHGitClient object at 0x118bed0>
> > > > > /mcclure/backup-Polycode
> > > > > creating and sending data for remote bookmark deletion
> > > > > ["git-receive-pack '/mcclure/backup-Polycode'"]
> > > > > determine_wants argument:
> > > > > {'refs/heads/screeninfo':
> '60660e76486583708684ae10aa5df4751f948570',
> > > > > 'refs/heads/createlua': '54995beed753f0af4d7f1c4f79b520e18fc6d22a',
> > > > > 'refs/heads/xibfix': 'd69b878d38632f3af8fd07127f05ce9c9e368e23',
> > > > > 'refs/heads/scene': '1a686b0d0ad3d9344c9ca463dac196be61dfd49e',
> > > > > 'refs/heads/zlibfix': '79762dc444f66c30710af2d8534956eab27aef7d',
> > > > > 'refs/heads/sound': 'b7e414ee212e3cc2fa93ed93053eeff71abfbd0a',
> > > > > 'refs/heads/dashg': '51030fa9ab337006ed2c9db05151b275cc4341f5',
> > > > > 'refs/heads/add_dependencies':
> > > '300bf6ac818221882505b9c8976830c2777a6964',
> > > > > 'refs/heads/bullet279': 'b13f974ce94790d53bdf166ecc68b499fbf0b629',
> > > > > 'refs/heads/cmake': '8a024fc8760e93f37229a8ef3f8c71961e1c4bf8',
> > > > > 'refs/heads/mingw': 'a573d046fe2cb9daad806a4a4c0853e4abf3b7c3',
> > > > > 'refs/heads/windowsquit':
> '94f728a23c8ddb93c6d1426eea25ada165af8bd9',
> > > > > 'refs/heads/zlib127': '67e474d6020f3ed3c20bde1d905f9fc53273416f',
> > > > > 'refs/heads/clearMesh': '6be76b1b21216485557d5cc1356be321ac761864',
> > > > > 'refs/heads/safestring':
> '9de8332077b1b13acb6c40c52479dde51f6d88df',
> > > > > 'refs/heads/master': '8355b00050696f4ddaaf13bf8ec721b3cf8b08e2',
> > > > > 'refs/heads/shaderfixes':
> 'f64acadc1c0ef4b6cfe6ff1e43c61b22bc216673'}
> > > > > determine_wants return:
> > > > > {'refs/heads/screeninfo':
> '60660e76486583708684ae10aa5df4751f948570',
> > > > > 'refs/heads/createlua': '54995beed753f0af4d7f1c4f79b520e18fc6d22a',
> > > > > 'refs/heads/xibfix': 'd69b878d38632f3af8fd07127f05ce9c9e368e23',
> > > > > 'refs/heads/scene': '1a686b0d0ad3d9344c9ca463dac196be61dfd49e',
> > > > > 'refs/heads/zlibfix': '79762dc444f66c30710af2d8534956eab27aef7d',
> > > > > 'refs/heads/sound': 'b7e414ee212e3cc2fa93ed93053eeff71abfbd0a',
> > > > > 'refs/heads/dashg': '51030fa9ab337006ed2c9db05151b275cc4341f5',
> > > > > 'refs/heads/add_dependencies':
> > > '300bf6ac818221882505b9c8976830c2777a6964',
> > > > > 'refs/heads/bullet279': '0000000000000000000000000000000000000000',
> > > > > 'refs/heads/cmake': '8a024fc8760e93f37229a8ef3f8c71961e1c4bf8',
> > > > > 'refs/heads/mingw': 'a573d046fe2cb9daad806a4a4c0853e4abf3b7c3',
> > > > > 'refs/heads/windowsquit':
> '94f728a23c8ddb93c6d1426eea25ada165af8bd9',
> > > > > 'refs/heads/zlib127': '67e474d6020f3ed3c20bde1d905f9fc53273416f',
> > > > > 'refs/heads/clearMesh': '6be76b1b21216485557d5cc1356be321ac761864',
> > > > > 'refs/heads/safestring':
> '9de8332077b1b13acb6c40c52479dde51f6d88df',
> > > > > 'refs/heads/master': '8355b00050696f4ddaaf13bf8ec721b3cf8b08e2',
> > > > > 'refs/heads/shaderfixes':
> 'f64acadc1c0ef4b6cfe6ff1e43c61b22bc216673'}
> > > > > sent_pack returned:
> > > > > {'refs/heads/screeninfo':
> '60660e76486583708684ae10aa5df4751f948570',
> > > > > 'refs/heads/createlua': '54995beed753f0af4d7f1c4f79b520e18fc6d22a',
> > > > > 'refs/heads/xibfix': 'd69b878d38632f3af8fd07127f05ce9c9e368e23',
> > > > > 'refs/heads/scene': '1a686b0d0ad3d9344c9ca463dac196be61dfd49e',
> > > > > 'refs/heads/zlibfix': '79762dc444f66c30710af2d8534956eab27aef7d',
> > > > > 'refs/heads/sound': 'b7e414ee212e3cc2fa93ed93053eeff71abfbd0a',
> > > > > 'refs/heads/dashg': '51030fa9ab337006ed2c9db05151b275cc4341f5',
> > > > > 'refs/heads/add_dependencies':
> > > '300bf6ac818221882505b9c8976830c2777a6964',
> > > > > 'refs/heads/bullet279': '0000000000000000000000000000000000000000',
> > > > > 'refs/heads/cmake': '8a024fc8760e93f37229a8ef3f8c71961e1c4bf8',
> > > > > 'refs/heads/mingw': 'a573d046fe2cb9daad806a4a4c0853e4abf3b7c3',
> > > > > 'refs/heads/windowsquit':
> '94f728a23c8ddb93c6d1426eea25ada165af8bd9',
> > > > > 'refs/heads/zlib127': '67e474d6020f3ed3c20bde1d905f9fc53273416f',
> > > > > 'refs/heads/clearMesh': '6be76b1b21216485557d5cc1356be321ac761864',
> > > > > 'refs/heads/safestring':
> '9de8332077b1b13acb6c40c52479dde51f6d88df',
> > > > > 'refs/heads/master': '8355b00050696f4ddaaf13bf8ec721b3cf8b08e2',
> > > > > 'refs/heads/shaderfixes':
> 'f64acadc1c0ef4b6cfe6ff1e43c61b22bc216673'}
> > >
> > >
> > > > So some things I notice:
> > > > - my generate_pack_contents function is never called.
> > > > - refs/heads/bullet279 is set to the value listed as ZERO_SHA1 in the
> > > > dulwich source in the return of my "determine_wants" function.
> > > > - The value returned by send_pack seems to be equal to the value
> returned
> > > > by my determine_wants function, indicating I succeeded.
> > >
> > > > However after running I check my test repository
> > > > https://github.com/mcclure/backup-Polycode in a web browser, and in
> the
> > > web
> > > > browser it says bullet279 is still present and pointing to b13f9.
> > >
> > > > Have I done anything wrong here?
> > >
> > > > "determine_wants() receives a dictionary of refs (refs as keys, the
> SHA1
> > > > they references as value) and should return a list of heads that you
> > > would
> > > > like to see fetched."
> > >
> > > > In the case of send_pack, does this change to "a list of heads that
> you
> > > > would like to see written"?
> > >
> > > > Thanks.
> > >
> > > > (Jelmer, I'm sorry, I originally hit "reply" instead of "reply-all",
> so
> > > you
> > > > will get two copies of this message.)
> > >
> > > > On Sat, Jul 7, 2012 at 4:31 AM, Jelmer Vernooij <jelmer@xxxxxxxxx>
> > > wrote:
> > >
> > > > > On Thu, Jul 05, 2012 at 12:51:09AM -0700, Andi McClure wrote:
> > > > > > Thanks! What I'm personally trying to do is interface with
> Github. I
> > > > > don't
> > > > > > know what extensions they do or don't support.
> > > > > They support delete-refs.
> > >
> > > > > > So, I'm not sure I understand what to do here. ZERO_SHA1 is just
> 160
> > > bits
> > > > > > of 0x00, right? I tried this and it didn't seem to work. What I'm
> > > doing
> > > > > is
> > > > > > again, the "changed" function which is my second argument to
> > > send_pack,
> > > > > > when this function is called it gets in a mapping of all git
> > > repository
> > > > > > heads to their corresponding git shas, I change the value
> associated
> > > with
> > > > > > (for example) "refs/heads/bullet279" in this dictionary to
> ("00"*20)
> > > and
> > > > > > return the modified dictionary. As far as I can tell (?)
> returning
> > > this
> > > > > > dictionary rewrites what all the ref names point to during the
> > > pack-- the
> > > > > > only point of the generate_pack_contents on the other hand seems
> (?)
> > > to
> > > > > be
> > > > > > to return object files given a sha (the shas returned by
> > > > > > determine_wants??). However when I set one of these to the
> > > > > 20-bytes-of-zero
> > > > > > hash and return it, it appears to have no effect on the Github
> side.
> > > I
> > > > > > don't get an error or anything, just... nothing happens. Any idea
> > > what I
> > > > > > might be doing wrong, or how to debug this further? I'm using
> Dulwich
> > > > > 0.8.2.
> > > > > What do you mean exactly when you say it doesn't work? The ref
> doesn't
> > > > > go away, or it simply gets its value set to the ZERO_SHA1?
> > >
> > > > > > By the way, there might be room for improvement in the
> documentation
> > > > > here.
> > > > > > If you look at the GitClient documentation:
> > >
> > > > >
> > >
> http://www.samba.org/~jelmer/dulwich/apidocs/dulwich.client.GitClient.html#send_pack
> > > > > > send_pack does not have its "determine_wants" argument
> documented at
> > > all.
> > > > > > The fetch() method documents an argument by that name with
> "Optional
> > > > > > function to determine what refs to fetch", but this doesn't seem
> to
> > > > > answer
> > > > > > questions like, what are the arguments to this optional function
> and
> > > what
> > > > > > does it return? The description of the generate_pack_contents
> > > argument of
> > > > > > send_pack is similarly vague in terms of what this function
> takes as
> > > an
> > > > > > argument and what it returns (I'm afraid I'm misunderstanding
> what it
> > > > > does
> > > > > > completely...).
> > > > > determine_wants() receives a dictionary of refs (refs as keys, the
> > > > > SHA1 they references as value) and should return a list of heads
> that
> > > > > you would like to see fetched.
> > >
> > > > > Patches to improve the documentation are very much welcome.
> > >
> > > > > Cheers,
> > >
> > > > > Jelmer
> > >
> > >
> > > > > > On Wed, Jul 4, 2012 at 3:34 PM, Jelmer Vernooij <
> jelmer@xxxxxxxxx>
> > > > > wrote:
> > >
> > > > > > > On Tue, Jul 03, 2012 at 11:04:30PM -0700, Andi McClure wrote:
> > > > > > > > Hello, I am trying to patch the "hg-git" project, which is
> based
> > > on
> > > > > > > > Dulwich. I do not understand very much about what I am doing
> and
> > > am
> > > > > > > mostly
> > > > > > > > working by studying the surrounding code. What I am trying
> to do
> > > is
> > > > > add a
> > > > > > > > feature where I delete a remote branch by name.
> > >
> > > > > > > > As far as I can tell, the way I'm meant to communicate with
> the
> > > > > server is
> > > > > > > > to call
> > > > > > > > client.send_pack(path, changed, genpack)
> > > > > > > > where client is a GitClient object, path is the appropriate
> git
> > > path,
> > > > > > > > "changed" is a method taking a dictionary of branch-name
> > > > > > > > ->branch-points-to-rev and returning a new dictionary
> describing
> > > > > same,
> > > > > > > and
> > > > > > > > "genpack" is
> > > > > dulwich.object_store.BaseObjectStore.generate_pack_contents.
> > > > > > > > The existing hg-git code uses this interface to upload
> objects
> > > and
> > > > > assign
> > > > > > > > refs.
> > >
> > > > > > > > How can I use the GitClient object to *delete* a ref, though?
> > > > > > > You should be able to do this by setting a particular ref to
> the
> > > > > > > ZERO_SHA1.
> > >
> > > > > > > Note that the server has to support the 'delete-refs'
> capability
> > > for
> > > > > > > this to work.
> > >
> > > > > > > Cheers,
> > >
> > > > > > > Jelmer
> > >
> > >
> > >
> > > --
> > >
>

Attachment: a683eb919b74.determinewantsfix.p
Description: Binary data

Attachment: 7284e1171c71.documentationfix.p
Description: Binary data


Follow ups

References