mergetag support: UnknownCommitExtra error

[Jelmer Vernooij]

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)]

This is breaking several Launchpad imports.

[Jelmer Vernooij (jelmer)]

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

[Michael Hasselmann (michael-hasselmann)]

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)]

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)]

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

[Jelmer Vernooij (jelmer)]

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

[Francis J. Lacoste (flacoste)]

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)]

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)]

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.

[Michael Hasselmann (michael-hasselmann)]

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.

[milki (milki-p)]

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)]

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.

[Jelmer Vernooij (jelmer)]

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)]

Additional patch for multiple mergetags

[Jelmer Vernooij (jelmer)]

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

[Jelmer Vernooij (jelmer)]

Merged, thanks for the patch!

[Gianfranco Costamagna (costamagnagianfranco)]

Sorry but how long before having them in launchpad?

[Jelmer Vernooij (jelmer)]

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)]

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)]

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)]

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

[Ubuntu Foundations Team Bug Bot (crichton)]

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.]

[Launchpad Janitor (janitor)]

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

[Jelmer Vernooij (jelmer)]

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

[Jelmer Vernooij (jelmer)]

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

[Roman (dismine)]

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

[deaunapaul (waxmigs2902)]

cheers

On Sun, Apr 1, 2018, 9:34 PM Jelmer Vernooij, <email address hidden>
wrote:

> ** Changed in: brz-git
> Status: Fix Released => Triaged
>
> --
> You received this bug notification because you are subscribed to
> Launchpad itself.
> Matching subscriptions: Launchpad, Launchpad, postgres, ppa me
> https://bugs.launchpad.net/bugs/963525
>
> Title:
> mergetag support: UnknownCommitExtra error
>
> Status in brz-git:
> Triaged
> Status in Bazaar Git Plugin:
> Triaged
> Status in Dulwich:
> Fix Released
> Status in Launchpad itself:
> Triaged
> Status in Valentina:
> Invalid
>
> 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
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/brz-git/+bug/963525/+subscriptions
>