Setting repository HEAD using the tutorial does not work

Bug #610550 reported by rtjmay
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Dulwich
Fix Released
Low
Unassigned

Bug Description

When setting HEAD after the first commit to a new repository, the sample code in the tutorial no longer works.

Trying the code from the tutorial:

>>> repo.refs['refs/heads/master'] = commit.id
>>> repo.refs['HEAD'] = 'ref: refs/heads/master'
>>> repo['HEAD']

Results in a KeyError and no refs being set. The commands must be reversed:

>>> repo.refs['HEAD'] = 'ref: refs/heads/master'
>>> repo.refs['refs/heads/master'] = commit.id
>>> repo['HEAD']

This correctly returns the commit id.

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

I can confirm that this is a bug, though we should fix the code rather than the tutorial.

Changed in dulwich:
status: New → Triaged
importance: Undecided → Low
milestone: none → 0.6.1
milestone: 0.6.1 → 0.6.2
Revision history for this message
yvesf (yvesf) wrote :

i can confirm too,
i think its because of this lines in dulwich/repo.py
  1067 except KeyError:
  1068 c.parents = []
  1069 self.object_store.add_object(c)
  1070 ok = self.refs.add_if_new("HEAD", c.id)

so i think setting HEAD can be omitted

there is a another issue in the first tutorial belonging to parse_timezone,
it returns a tuple:

    :return: Tuple with timezone as seconds difference to UTC
        and a boolean indicating whether this was a UTC timezone
        prefixed with a negative sign (-0000).

- >>> tz = parse_timezone('-0200')
+ >>> tz = parse_timezone('-0200')[0]

Revision history for this message
Dave Borowitz (dborowitz) wrote :

That try/except block in repo.py is confusing but I think it is correct. Note that RefsContainer.__getitem__ and RefsContainer.set_if_equals follow all symbolic refs. By definition, committing a tree means advancing HEAD, whether or not it is a symbolic ref.

The "try" block first tries an atomic compare-and-swap of HEAD with the new commit. If that fails due to a KeyError, it means there is either no ref named HEAD, or HEAD is a broken symref (which is the default state in a new repo created). Then we do an atomic compare-and-add of HEAD. If HEAD didn't exist, this creates HEAD as a non-symref. If HEAD was a dangling symref, this writes through the symref, e.g. to refs/heads/master.

Following the tutorial as of a383e450, it seems to work. However, there is still a bug: the tutorial tells you to set "repo.refs['HEAD'] = 'ref: refs/heads/master'", which is wrong for two reasons:
1. HEAD is already set in a new repo, cf. Repo._init_maybe_bare.
2. Symbolic refs should be set with set_symbolic_ref; the 'ref: foo' syntax should be considered an implementation detail.
(3. There is no atomic compare-and-swap version of set_symbolic_ref, but we'll ignore that for this bug.)

I'll mail a patch for the tutorial that clarifies the setting of HEAD.

Jelmer Vernooij (jelmer)
Changed in dulwich:
status: Triaged → Fix Committed
Jelmer Vernooij (jelmer)
Changed in dulwich:
milestone: 0.6.2 → 0.7.0
Jelmer Vernooij (jelmer)
Changed in dulwich:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.