no support for gpgsig tags

[Chris Halse Rogers]

I'm trying to branch the xserver with bzr-git, and it's failing:

#: bzr branch git://anongit.freedesktop.org/git/xorg/xserver xserver-bzr
bzr: ERROR: Unknown extra fields in <Commit afc153a5b4fc58ae70dc214f61a71b1a8c855f06>: ['gpgsig', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', ''].

#: git show --show-signature afc153a5b4fc58ae70dc2
commit afc153a5b4fc58ae70dc214f61a71b1a8c855f06
gpgkeys: key 23C6CDF7272D9D43 not found on keyserver
gpg: Signature made Wed 09 May 2012 08:05:49 EST using RSA key ID 272D9D43
gpg: requesting key 272D9D43 from hkp server keyserver.ubuntu.com
gpg: no valid OpenPGP data found.
gpg: Total number processed: 0
gpg: Can't check signature: public key not found
Author: James Cloos <email address hidden>
Date: Tue May 8 17:55:10 2012 -0400

    Fix RANDR’s gamma_to_ramp().

    In order to generate a 256-entry ramp in [0,65535] which covers the full
    range, one must mupliply eight-bit values not by 256 but rather by 257.

    Many years back – well before the RANDR extension was written, and
    before xorg@fdo – a similar bug fix was made to the DIX for converting
    client-supplied eight-bit color values into sixteen-bit values.

    Noticed by: Elle Stone and Graeme Gill.

    Signed-off-by: James Cloos <email address hidden>

diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
index a773c34..aca0734 100644
--- a/hw/xfree86/modes/xf86RandR12.c
+++ b/hw/xfree86/modes/xf86RandR12.c
@@ -1679,11 +1679,11 @@ gamma_to_ramp(float gamma, CARD16 *ramp, int size)

     for (i = 0; i < size; i++) {
         if (gamma == 1.0)
- ramp[i] = i << 8;
+ ramp[i] = i | i << 8;
         else
             ramp[i] =
                 (CARD16) (pow((double) i / (double) (size - 1), 1. / gamma)
- * (double) (size - 1) * 256);
+ * (double) (size - 1) * 257);
     }
 }

ProblemType: Bug
DistroRelease: Ubuntu 13.04
Package: bzr-git 0.6.9-1
ProcVersionSignature: Ubuntu 3.7.0-3.9-generic 3.7.0-rc6
Uname: Linux 3.7.0-3-generic x86_64
ApportVersion: 2.6.2-0ubuntu5
Architecture: amd64
Date: Thu Nov 29 16:39:30 2012
EcryptfsInUse: Yes
InstallationDate: Installed on 2012-06-16 (165 days ago)
InstallationMedia: Ubuntu 12.10 "Quantal Quetzal" - Alpha amd64 (20120521)
MarkForUpload: True
PackageArchitecture: all
SourcePackage: bzr-git
UpgradeStatus: No upgrade log present (probably fresh install)

[Chris Halse Rogers (raof)]

Do you have an estimate of the complexity of fixing this? It's semi-blocking something that I need to work on, so if it's likely to be doable by someone with no prior bzr-git experience I'd be able to hack on it.

[Jelmer Vernooij (jelmer)]

Basically what needs to happen is:

 * in dulwich, extend Commit to serialize and deserialize gpgsig data (in dulwich/objects.py)
 * in bzr-git:
  - have all old mappings raise an exception if a gpgsigs are encountered (mapping.py)
  - have new mapping convert gpg data into a bzr revision property and back

I think it'd probably a day or two if you're familiar with the git and bzr internals.

[Chris Halse Rogers (raof)]

Hm. Given I'm familiar with neither the bzr nor the git internals, that
might be a bit longer for me then.

Don't let me block you from working on it ☺.

[Jelmer Vernooij (jelmer)]

> Don't let me block you from working on it ☺.
FWIW I don't think bzr-git is actively maintained at the moment.

[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

[Harald Sitter (apachelogger)]

Making this non-fatal (on launchpad) would be rather helpful since as of a couple of days ago this is blocking KDE Frameworks 5 building [1]. Upstream is less than thrilled.

[1] https://code.launchpad.net/~neon/kde-workspace/master

[Jelmer Vernooij (jelmer)]

On Sun, Nov 03, 2013 at 10:07:36AM -0000, Harald Sitter wrote:
> Making this non-fatal (on launchpad) would be rather helpful since as of
> a couple of days ago this is blocking KDE Frameworks 5 building [1].
> Upstream is less than thrilled.
>
> [1] https://code.launchpad.net/~neon/kde-workspace/master
>
> ** Attachment added: "raw commit data of affected commit in kde-workspace"
> https://bugs.launchpad.net/ubuntu/+source/bzr-git/+bug/1084403/+attachment/3899038/+files/commit
Unfortunately just ignoring these tags is not possible without
fundamentally breaking various assumptions in bzr-git.

bzr-git does a lossless conversion from git to bzr, so it can later
regenerate the exact same git objects. This is necessary since objects
sent by the server can be sent as deltas against fulltexts that the
server knows the client has.

If these tags are ignored, then bzr-git will be unable to reproduce
git commit objects with the exact same contents, and break.

Jelmer

[Jelmer Vernooij (jelmer)]

More practically speaking, I think the most achievable solution is probably to run a manual fast-export/fast-import import.

[Aron Xu (happyaron)]

Is there a way that can work around this bug and set up auto-import for a git repository that has some historical commits contain gpgsig? It seems that I'm not able to push to the branch set up by vcs-import, so fast-export/fast-import does not work for this use case.

[Jelmer Vernooij (jelmer)]

On Tue, Nov 12, 2013 at 06:55:17PM -0000, Aron Xu wrote:
> Is there a way that can work around this bug and set up auto-import for
> a git repository that has some historical commits contain gpgsig? It
> seems that I'm not able to push to the branch set up by vcs-import, so
> fast-export/fast-import does not work for this use case.
Unfortunately that's not possible.

You should be able to sue fast-export/fast-import with a new (non-vcs-imports)
branch.

Cheers,

Jelmer

[Thomas Karl Pietrowski (thopiekar)]

same problem here: https://code.launchpad.net/~thopiekar/emgd/x11proto-core

[Jelmer Vernooij (jelmer)]

See bug 1251682 for the relevant dulwich bug.

[John Vrbanac (john.vrbanac)]

Just ran into this issue a few days ago when a project I contribute to failed to sync because of a gpg signed commit:

You can find the log from launchpad in the attachment as well as in the following pastebin:
http://paste.ubuntu.com/6788623/

[Thomas Kluyver (takluyver)]

This is now preventing me from importing the matplotlib codebase to do nightly builds:

https://code.launchpad.net/~takluyver/matplotlib/trunk

[Peter Sabaini (peter-sabaini)]

I'm unable to import openstack/tempest because of this

[Jelmer Vernooij (jelmer)]

Thanks for the replies.

At this point the issue is well documented; please don't add more examples of broken repositories.

[LocutusOfBorg (costamagnagianfranco)]

I do not want to add another example, because there are already too many.

Just FYI, Debian is discussing right now the defaulting of gpg signed commits, so I guess this bug will be more and more important from now.

[est31 (mtest31)]

Major git hosting service GitHub has just announced support for verifying signed git commits and tags [1].
Just noting, as this will most probably increase the usage of signed git commits.

[1] : https://github.com/blog/2144-gpg-signature-verification

[Qwerty Chouskie (asdfghrbljzmkd)]

Just found this patched version: https://code.launchpad.net/~simon-marchi/bzr-git/devel
Could this get merged please? This bug breaks SuperTuxKart daily builds, see https://github.com/supertuxkart/stk-code/issues/1415 towards end.

[Jelmer Vernooij (jelmer)]

That fix is incomplete and possibly corrupts future pulls. See the bug report for an explanation.

On 13 June 2016 03:45:52 BST, Qwerty Chouskie <email address hidden> wrote:
>Just found this patched version:
>https://code.launchpad.net/~simon-marchi/bzr-git/devel
>Could this get merged please? This bug breaks SuperTuxKart daily
>builds, see https://github.com/supertuxkart/stk-code/issues/1415
>towards end.

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[Qwerty Chouskie (asdfghrbljzmkd)]

I saw comment #8, but it does not seem like ignoring gpgsig tags should break deltas, as the code changes in the commits are the same...

[Qwerty Chouskie (asdfghrbljzmkd)]

Also, the code change will defiantly not make anything worse, and might make things better, so it would make sense to merge it as a temporary fix... And, I tested the first section of the diff (https://i264933589.restricted.launchpadlibrarian.net/264933589/8e33a154-3111-11e6-9da8-002481e91f22.txt?token=17TdjWddN9v0cBJKTTTsGgMmsb8NjTg8) and it worked for branching SuperTuxKart (https://github.com/supertuxkart/stk-code.git).

[Colin Watson (cjwatson)]

Our direction for this in Launchpad is to get git-to-git imports working (hopefully soonish), and then you can just use git-based recipes instead. This will be much simpler and more robust against all kinds of strangenesses in git repositories; for example, a fix for gpgsig is no help for the also-common problem that bzr-git doesn't handle submodules.

[Jelmer Vernooij (jelmer)]

It will make things worse. People will get confusing errors (sha1 checksum or delta invalid exceptions), and bzr-git loses the ability to properly support the gpgsig tag.

On 13 June 2016 22:16:19 BST, Qwerty Chouskie <email address hidden> wrote:
>Also, the code change will defiantly not make anything worse, and might
>make things better, so it would make sense to merge it as a temporary
>fix... And, I tested the first section of the diff
>(https://i264933589.restricted.launchpadlibrarian.net/264933589/8e33a154-3111-11e6-9da8-002481e91f22.txt?token=17TdjWddN9v0cBJKTTTsGgMmsb8NjTg8)
>and it worked for branching SuperTuxKart
>(https://github.com/supertuxkart/stk-code.git).

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[Qwerty Chouskie (asdfghrbljzmkd)]

I tried:

=== modified file 'mapping.py'
--- mapping.py 2013-09-02 00:54:58 +0000
+++ mapping.py 2016-06-13 04:07:10 +0000
@@ -333,7 +333,11 @@
         if commit is None:
             raise AssertionError("Commit object can't be None")
         if commit.extra:
- raise UnknownCommitExtra(commit, [item[0] for item in commit.extra])
+ for (extra_key, extra_value) in commit.extra:
+ if extra_key == "gpgsig":
+ pass
+ sys.stderr.write("WARNING: GPGSIG TAGS ARE NOT SUPPOTED AND MAY CAUSE ISSUES!!!!!!!\n")
+ else:
+ raise UnknownCommitExtra(commit, [item[0] for item in commit.extra])
         rev = ForeignRevision(commit.id, self,
                 self.revision_id_foreign_to_bzr(commit.id))
         rev.git_metadata = None

But there was no output on the terminal, what am I doing wrong?

[Jelmer Vernooij (jelmer)]

It will break deltas, as bzr-git won't be able to create the original git object anymore from the bzr data. The object it creates will lack the gpgsig tag and thus result in a checksum mismatch.

On 13 June 2016 21:29:38 BST, Qwerty Chouskie <email address hidden> wrote:
>I saw comment #8, but it does not seem like ignoring gpgsig tags should
>break deltas, as the code changes in the commits are the same...

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[Qwerty Chouskie (asdfghrbljzmkd)]

I made this patch: http://jacobspctuneup.tk/bzr-git%20patches/patchtry1.txt
It looks good, but nothing actually appears in the terminal or "~/.bzr.log". Maybe it only shows up if there is an error?

[Jelmer Vernooij (jelmer)]

Are you fetching into a new repository and have you removed the bzr-git cache?

On 15 June 2016 01:24:10 BST, Qwerty Chouskie <email address hidden> wrote:
>I made this patch:
>http://jacobspctuneup.tk/bzr-git%20patches/patchtry1.txt
>It looks good, but nothing actually appears in the terminal or
>"~/.bzr.log". Maybe it only shows up if there is an error?

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[Qwerty Chouskie (asdfghrbljzmkd)]

Where is the cache?

[Jelmer Vernooij (jelmer)]

It depends on your system. Usually $XDG_CACHE_HOME/bazaar/git I think.

On 16 June 2016 17:50:44 BST, Qwerty Chouskie <email address hidden> wrote:
>Where is the cache?

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[Qwerty Chouskie (asdfghrbljzmkd)]

I checked ~/.cache, but there was nothing bzr-related there. I also checked ~/.bazzar/, but there was nothing cache-related there.

[Jelmer Vernooij (jelmer)]

The cache could also be under the bzr repository somewhere.

On 16 June 2016 19:06:43 BST, Qwerty Chouskie <email address hidden> wrote:
>I checked ~/.cache, but there was nothing bzr-related there. I also
>checked ~/.bazzar/, but there was nothing cache-related there.

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[Qwerty Chouskie (asdfghrbljzmkd)]

I deleted the whole repo before re-import, so that should not be the problem.

[William Grant (wgrant)]

That patch won't do anything because you included the new check inside the "if commit is not None" block, which is an assertion that is meant to never fail.

Even if it was in the right place, however, it would try to raise None as an exception and crash.

If you fixed that, it would do as Jelmer says: create a corrupt branch that is unable to reproduce the commits that it says it can, because it won't store the gpgsig field that is needed for reconstitution.

This is fundamentally a very difficult dilemma, because you can't ignore the field: you either have to store it verbatim in a revision property, giving up the ability to implement it properly in future; or implement it properly, which it isn't at all clear how to do. Due to the way that git and bzr-git operate you can't just ignore it and hope it will work.

[Jelmer Vernooij (jelmer)]

This is not a "trivial bug", which papercuts seems to be about.

[Rob Loach (robloach)]

This also has affected libretro-snes9x:
http://launchpadlibrarian.net/272633750/libretro-libretro-snes9x-libretro.log

bzrlib.plugins.git.errors.UnknownCommitExtra: Unknown extra fields in <Commit 03c8bad484ff5210c9a374d687d26126b6c1f71c>: ['gpgsig'].

https://github.com/libretro/snes9x/issues/22

[Rob Loach (robloach)]

Built ontop of wgrant's feedback, a new patch:
http://files.robloach.net/bzr-git--gpg.diff

While I understand the best way around this would be to implement the GPG signatures in bzr, but this is affecting many projects brought into Launchpad through bzr-git. Ignoring it may be the best option until we can implement it correctly in the bzr space.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "bzr-git--gpg.diff" 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.]

[Jelmer Vernooij (jelmer)]

Ignoring it will cause confusing import errors in incremental pulls for Launchpad imports as well.

Incremental pulls rely on deltas against full texts recreated from bzr data including PGP signatures. If even one byte in the full text is incorrect, that causes incremental pulls to fail.

On 19 July 2016 02:08:31 GMT+02:00, Rob Loach <email address hidden> wrote:
>Built ontop of wgrant's feedback, a new patch:
>http://files.robloach.net/bzr-git--gpg.diff
>
>While I understand the best way around this would be to implement the
>GPG signatures in bzr, but this is affecting many projects brought into
>Launchpad through bzr-git. Ignoring it may be the best option until we
>can implement it correctly in the bzr space.

[Rob Loach (robloach)]

Thanks for taking a look, Jelmer. Do you have an idea of a way forward with it? William mentioned he was unsure of how to do it properly.

[Jelmer Vernooij (jelmer)]

On Tue, Jul 19, 2016 at 05:56:28PM -0000, Rob Loach wrote:
> Thanks for taking a look, Jelmer. Do you have an idea of a way forward
> with it? William mentioned he was unsure of how to do it properly.
They are the equivalent of testaments, so ideally they should be
converted to those. I don't recall enough about testaments to know
whether that would actually work though.

Alternatively, they could just be stowed in bzr revision properties
and then reimported whenever bzr revisions are converted to git
commits.

Jelmer

[John Pye (jdpipe)]

Adding a note to mention that this bug makes is quite difficult to use the PPA with Github repositories. I wanted to try to build my own .deb for Jabref, so I started by trying to import the code from the Jabref github pages. Here is where I got to:
https://code.launchpad.net/~jdpipe/jabref/debian

However, when importing from Github, there is an error from Launchpad, specifically from "/srv/importd.launchpad.net/production/launchpad-rev-18169/bzrplugins/git/mapping.py" line 334, where a 'gpgsig' commit field seems to have triggered some kind of error.

A little surprised by this... surely other people are importing code from Github?

Cheers
JP

2016-08-03 02:07:49 INFO Starting job.
2016-08-03 02:07:49 INFO Getting exising bzr branch from central store.
2016-08-03 02:07:50 INFO [chan bzr SocketAsChannelAdapter] Opened sftp connection (server version 3)
2016-08-03 02:07:50 INFO [chan bzr SocketAsChannelAdapter] Opened sftp connection (server version 3)
2016-08-03 02:07:50 INFO 285 bytes transferred
2016-08-03 02:07:51 INFO Importing branch.
2016-08-03 02:07:53 INFO Counting objects: 118432, done. 0
2016-08-03 02:08:53 INFO 68240614 bytes transferred | Compressing objects 3/3
2016-08-03 02:09:05 INFO finding revisions to fetch:generating index 0/118432
2016-08-03 02:09:26 INFO finding revisions to fetch:generating index 0/118432
2016-08-03 02:09:38 INFO finding revisions to fetch 1/3
2016-08-03 02:09:38 INFO
Traceback (most recent call last):
  File "/srv/importd.launchpad.net/production/launchpad-rev-18169/scripts/code-import-worker.py", line 96, in <module>
    sys.exit(script.main())
  File "/srv/importd.launchpad.net/production/launchpad-rev-18169/scripts/code-import-worker.py", line 91, in main
    return import_worker.run()
  File "/srv/importd.launchpad.net/production/launchpad-rev-18169/lib/lp/codehosting/codeimport/worker.py", line 583, in run
    return self._doImport()
  File "/srv/importd.launchpad.net/production/launchpad-rev-18169/lib/lp/codehosting/codeimport/worker.py", line 737, in _doImport
    inter_branch.fetch(limit=revision_limit)
  File "/srv/importd.launchpad.net/production/launchpad-rev-18169/bzrplugins/git/branch.py", line 722, in fetch
    self.fetch_objects(stop_revision, fetch_tags=fetch_tags, limit=limit)
  File "/srv/importd.launchpad.net/production/launchpad-rev-18169/bzrplugins/git/branch.py", line 745, in fetch_objects
    determine_wants, self.source.mapping, limit=limit)
  File "/srv/importd.launchpad.net/production/launchpad-rev-18169/bzrplugins/git/fetch.py", line 718, in fetch_objects
    limit)
  File "/srv/importd.launchpad.net/production/launchpad-rev-18169/bzrplugins/git/fetch.py", line 484, in import_git_objects
    mapping.revision_id_foreign_to_bzr)
  File "/srv/importd.launchpad.net/production/launchpad-rev-18169/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 e0380b795f649afaf6cd708abc5bf2680c8a01c0>: ['gpgsig'].
Import failed:
Traceback (most recent call last):
Failure: twisted.internet.error.ProcessTerm...

[Colin Watson (cjwatson)]

John, it has nothing to do with whether you're importing from GitHub; it depends on whether the repository's history contains commits with gpgsig tags.

Our plan for this remains as I outlined in comment 24, although some other urgent bits of work have jumped the queue and delayed us implementing it. A workaround for the time being might be to occasionally push the git repository to Launchpad's own git hosting manually.

[Robert Bruce Park (robru)]

lp:emacs is bit by this bug since April 2016. I'm eagerly awaiting those git-to-git imports ;-)

[Colin Watson (cjwatson)]

Launchpad git-to-git imports are available now: http://blog.launchpad.net/code/git-to-git-imports

[Thomas Karl Pietrowski (thopiekar)]

Great news! Is there also a issue report here on Launchpad for the nest-part implementation for git recipes? Would like to get notified when it's there.

[Colin Watson (cjwatson)]

Thomas: I implemented that a while back (bug 1537579), but just forgot to update the documentation. I've done the latter as well now.

[Thomas Karl Pietrowski (thopiekar)]

Awesome! Many thanks, Colin!