mergetag support: UnknownCommitExtra error

Bug #963525 reported by Jelmer Vernooij on 2012-03-23
88
This bug affects 13 people
Affects Status Importance Assigned to Milestone
Bazaar Git Plugin
High
Unassigned
Dulwich
Fix Released
Medium
milki
Launchpad itself
Critical
Unassigned
Valentina
Undecided
auto-dismine-1
brz-git
Medium
Unassigned

Bug Description

dulwich should properly support merge tags, which have started appearing in newer versions of git.

E.g.:

gwenhwyvar:/tmp/xorg-gtest% git cat-file -p ee43d9c90720e4da2298c34ef39ddb0f692e8e1f
tree e2e44481ef857c53836a9968d596fa9c33e2ff4b
parent ae84f3022847de851f6fd8f84df7d2fef5bf4408
parent a15d88e39f19efdd6aa20053a7a3ce6ffc8c0d9e
author Chase Douglas <email address hidden> 1330102289 -0800
committer Chase Douglas <email address hidden> 1330102289 -0800
mergetag object a15d88e39f19efdd6aa20053a7a3ce6ffc8c0d9e
 type commit
 tag xorg-gtest-0.1.1
 tagger Chase Douglas <email address hidden> 1330101752 -0800

 Version 0.1.1
 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1.4.11 (GNU/Linux)

 iQIcBAABAgAGBQJPR74BAAoJEI3Z6a9pX7cqH/8P/jfoi9lQt1HzffM82dZsEsUK
 qlo80qhWgP9lnMC6QtmSFwx7GRjV+FO1YODNAWd05eo2J0Wi8yuQEy189M2NcnAc
 x7cIB/ETQ1+uVlyY/cPeP9bey06YOqlR9BB7QUrStFpvwI1tJXzCXb7/ilwichea
 ox8fec5xQHjVRQ4mLIsJ4gMUiaeXChjY2EN06MjAnBBg/FFOUxlg9Hulqoa68FYI
 dWUmT9ljpoMB/MyTKihGRV/9eb2ij0aMbwDr0uV265XLd4vRVvw2t4bwRP+6VtpP
 1XqTeR9kUlMH+UcyL5MKFkJsFfulNlh7kGZrf516PBjQd1H8wuJ9Ij1r9RmizMfu
 zsvpsJZDkz/iNTYhAd9IQoHiDM82F00M9BLOjhcJSiA1vWrAOVLo2oPq5rLDuWZB
 7Ia2+iOsKcmkmdbJ1oaK3km9qrlq29Y8MM0P51BWRfkSRJeWv4jVPIUhur1glCDM
 DRocMsziWH7X/RyZpxOrJq0FA6hTjTTTjd6dsE5P1FletZr3mLjwkX2XlFbWAaK8
 B6hA0bERytH9lKSGggHX/mJBsjKG+9GQvz3TT8SNe+DBWcTaBBRHfu/gLY6vHHYx
 RNHbjObr5hqKtikKEnAGZG824AIcpVX3HtxqtAlGMDq1d08iilqEVZwD0J8FMBvR
 fOsU8Pb/CdKjxDRWZ3LE
 =5gUI
 -----END PGP SIGNATURE-----

Merge tag 'xorg-gtest-0.1.1' into debian-unstable

Version 0.1.1

Jelmer Vernooij (jelmer) wrote :

This is breaking several Launchpad imports.

Changed in dulwich:
status: New → Triaged
importance: Undecided → Medium
Changed in bzr-git:
status: New → Triaged
Changed in launchpad:
status: New → Triaged
importance: Undecided → High
Changed in bzr-git:
importance: Undecided → High
Changed in dulwich:
assignee: nobody → Jelmer Vernooij (jelmer)
Changed in launchpad:
assignee: nobody → Jelmer Vernooij (jelmer)
Changed in bzr-git:
assignee: nobody → Jelmer Vernooij (jelmer)
Jelmer Vernooij (jelmer) on 2012-04-03
Changed in launchpad:
assignee: Jelmer Vernooij (jelmer) → nobody
Jelmer Vernooij (jelmer) on 2012-04-09
Changed in bzr-git:
assignee: Jelmer Vernooij (jelmer) → nobody

Not actually on the Launchpad roadmap at the moment as far as I know.

summary: - signed tag support
+ mergetag support
description: updated
Changed in launchpad:
importance: High → Low

I also tried to clone a branch that contained a commit with a mergetag. To me, this looks as if bzr is surprised to see unexpected metadata, see this error message:

bzr: ERROR: Unknown extra fields in <Commit f45a66d9ea4e1a3043c66e4cb0ee21f56ff7dcfd>: ['mergetag', '', '', '', '', '', '', '', '', '', '', '', ''].

So cant we just teach bzr to ignore the metadata?

Jelmer Vernooij (jelmer) wrote :

Hi Michael,

Just ignoring the metadata wouldn't be an option since that means bzr-git can no longer reconstruct the original git objects, which are required when fetching from a git repository (they can be used as a delta base).

Francis J. Lacoste (flacoste) wrote :

Given that this is breaking Git imports that were working fine previously, this qualify as a Critical bug.

Changed in launchpad:
importance: Low → Critical
tags: added: regression

On Fri, Jul 06, 2012 at 03:25:47PM -0000, Francis J. Lacoste wrote:
> Given that this is breaking Git imports that were working fine
> previously, this qualify as a Critical bug.
This is a new Git feature though that these upstreams have started to
use.

Would this mean that bug 402814 should be considered critical as well?
It also affects various imports that have at some point broken because
upstream started using git submodules.

Cheers,

Jelmer

That's an interesting question. I don't think submodules support is on par here. But it might just be because I don't know Git very well.

In which condition will this mergetag metadata appears? My impression is that it's under normal operation and so it's very easy for an upstream to break the import using that "new feature". Whereas to use submodules, you really need to reorganize your tree, which makes it much more like a separate feature to me.

But if mergetag is a not as transparent as I think, and is more like submodules, I'd downgrade it.

Jelmer Vernooij (jelmer) wrote :

mergetag appears if you're merging signed tags with a newer version of git. That's indeed less of a conscious decision than e.g. using submodules. A lot of git development happens without signed tags though, and bzr-git has never supported signed tags (it just ignores them). There are a bunch of other things that have broken existing imports too - using a filename with a backslash in it, for example, and there are some repositories that introduced oddly formatted file modes written by certain git implementations (bug 581064) .

Most imports are actually created by upstreams that don't care about the Bazaar imports on Launchpad. Both submodules and mergetag are regressions from the point of view of whoever is relying on these imports in Launchpad.
There are at least as much imports on Launchpad that started failing because their upstream started using submodules as imports that fail because of mergetag.

I'm not opposed to bumping this to critical - there seem to be a fair number of users who are affected by it - unlike e.g. bug 581064. I do think we should be consistent though if this is labeled a regression.

Francis J. Lacoste (flacoste) wrote :

I think key difference to qualify this as a regression and not submodules is that Launchpad did support of Git repo with signed tags (even though it ignored them) whereas submodules were never supported.

My current work-around is to only do git merges with version older than 1.7.8. http://code.google.com/p/git-core/downloads/detail?name=git-1.7.6.5.tar.gz for example worked fine for me. Of course that's not sustainable and only works in situations where upstream is aware of this issue.

Jelmer Vernooij (jelmer) on 2012-07-20
Changed in dulwich:
assignee: Jelmer Vernooij (jelmer) → nobody
Curtis Hovey (sinzui) on 2012-10-05
tags: added: code-import
William Grant (wgrant) on 2012-10-22
tags: added: bzr
Curtis Hovey (sinzui) on 2012-10-29
tags: added: git
Curtis Hovey (sinzui) on 2013-01-08
summary: - mergetag support
+ mergetag support: UnknownCommitExtra error
tags: added: lp-code
milki (milki-p) wrote :

I did not have any problems within dulwich itself in terms of cloning, branching, committing to a repo with a mergetag commit. I did encounter the problem michael had in bzr-git in branching a git branch with a mergetag commit.

What exactly is missing for mergetag support in dulwich?

milki (milki-p) wrote :

From discussion in IRC, jelmer said the mergetag needs to be serialized.

The attached patch makes the mergetag deserialize into a Tag object and serialize properly as an extra header.

On Fri, 2013-01-11 at 22:04 +0000, milki wrote:
> >From discussion in IRC, jelmer said the mergetag needs to be serialized.
>
> The attached patch makes the mergetag deserialize into a Tag object and
> serialize properly as an extra header.
>
> ** Patch added: "0001-Add-mergetag-property-to-commit.patch"
> https://bugs.launchpad.net/dulwich/+bug/963525/+attachment/3480543/+files/0001-Add-mergetag-property-to-commit.patch
Thanks, that looks reasonable.

Any chance you can add a test with two mergetags?

Cheers,

Jelmer

milki (milki-p) wrote :

Additional patch for multiple mergetags

Changed in bzr-git:
status: Triaged → Fix Committed
Changed in dulwich:
status: Triaged → Fix Released
Changed in launchpad:
status: Triaged → Fix Released
Jelmer Vernooij (jelmer) wrote :

Hi Artur,

Please stop marking these bugs as fixed - they're not.

(I reverted the Launchpad bug state to New, since I can't set it to Triaged anymore)

Jelmer

Changed in bzr-git:
status: Fix Committed → Triaged
Changed in dulwich:
status: Fix Released → Triaged
Changed in launchpad:
status: Fix Released → New
William Grant (wgrant) on 2013-01-13
Changed in launchpad:
status: New → Triaged
Jelmer Vernooij (jelmer) wrote :

Merged, thanks for the patch!

Changed in dulwich:
status: Triaged → Fix Committed
assignee: nobody → milki (milki-p)
Jelmer Vernooij (jelmer) on 2013-05-31
Changed in dulwich:
milestone: none → 0.9.0
status: Fix Committed → Fix Released

Sorry but how long before having them in launchpad?

Jelmer Vernooij (jelmer) wrote :

This requires a fix in bzr-git to use the new interface in dulwich and to somehow stash the mergetag data in bzr revision properties. I don't think there is anybody working on that at the moment.

The chances of that fix for bzr-git happening any time soon are remote, unfortunately. I am no longer involved with bzr-git or launchpad (just Dulwich) but it looks like the last change to bzr-git was more than 6 months ago.

Mantas Kriaučiūnas (mantas) wrote :

Is the same problem with this import:

https://code.launchpad.net/~gnome-shell-extensions/gnome-shell-extensions/appindicator-support-head

[..]
  File "/srv/importd.launchpad.net/production/launchpad-rev-17045/bzrplugins/git/mapping.py", line 334, in import_commit
    raise UnknownCommitExtra(commit, [item[0] for item in commit.extra])
bzrlib.plugins.git.errors.UnknownCommitExtra: Unknown extra fields in <Commit 801f49df01d3b7d5ff58aeef43a8c9561f1a4b23>: ['gpgsig'].

milki (milki-p) wrote :

Mantas, this is the wrong bug. While the symptom is caused by the same code path, this bug is specific for mergetag support. Please comment on the existing bug 1251682.

Jelmer Vernooij (jelmer) wrote :

On Wed, Jun 18, 2014 at 01:12:19PM -0000, milki wrote:
> Mantas, this is the wrong bug. While the symptom is caused by the same
> code path, this bug is specific for mergetag support. Please comment on
> the existing bug 1251682.

Actually, it is bug 1084403.

Cheers,

Jelmer

The attachment "0001-Add-mergetag-property-to-commit.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

tags: added: patch
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in valentina (Ubuntu):
status: New → Confirmed
rahmadani (rahmadani) on 2017-04-06
Changed in valentina:
status: New → Confirmed
Jelmer Vernooij (jelmer) wrote :

Please don't add other projects to affects; this is not a bug in valentina.

no longer affects: valentina (Ubuntu)
Changed in valentina:
status: Confirmed → Invalid
Jelmer Vernooij (jelmer) wrote :

You might want to switch to git-to-git imports for valentina now that Launchpad supports them.

Roman (dismine) wrote :

Thank you @jelmer, but i resolved this bug for Valentina long time ago. Just recovered git repo.

Jelmer Vernooij (jelmer) on 2017-06-20
Changed in brz-git:
status: New → Triaged
importance: Undecided → Medium
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

  • auto-dismine-1 Edit

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