Debian Bug report logs - #700411
git-buildpackage: git-import-orig should filter the upstream debian directory

version graph

Package: git-buildpackage; Maintainer for git-buildpackage is Guido Günther <agx@sigxcpu.org>; Source for git-buildpackage is src:git-buildpackage.

Reported by: Raphaël Hertzog <hertzog@debian.org>

Date: Tue, 12 Feb 2013 13:39:01 UTC

Severity: normal

Tags: patch

Found in version git-buildpackage/0.6.0~git20121124

Reply or subscribe to this bug.

Toggle useless messages

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to debian-bugs-dist@lists.debian.org, Guido Günther <agx@sigxcpu.org>:
Bug#700411; Package git-buildpackage. (Tue, 12 Feb 2013 13:39:04 GMT) Full text and rfc822 format available.

Acknowledgement sent to Raphaël Hertzog <hertzog@debian.org>:
New Bug report received and forwarded. Copy sent to Guido Günther <agx@sigxcpu.org>. (Tue, 12 Feb 2013 13:39:04 GMT) Full text and rfc822 format available.

Message #5 received at submit@bugs.debian.org (full text, mbox):

From: Raphaël Hertzog <hertzog@debian.org>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: git-buildpackage: git-import-orig should filter the upstream debian directory
Date: Tue, 12 Feb 2013 14:37:54 +0100
Package: git-buildpackage
Version: 0.6.0~git20121124
Severity: normal

git-import-orig will happily import upstream tarballs that contain a
debian directory and when the upstream debian dir changes, git-import-orig
will try to merge those changes in the real debian packaging available on
the master branch.

In the best case, it fails, in the worst case it leads to indesirable
changes.

I tried to work-around this problem with --filter='debian/*' but there's
no safe pattern to use... because this one will also strip
"src/plugins/debian" from the upstream tarball and we don't really want
this by default.

So I would suggest that git-import-orig should automatically remove the
top-level "debian" directory when it does its work.

I'm not sure how git-import-dsc behaves in the same situation but it
should probably adjust its behaviour depending on the source format.
With 3.0 (quilt), you want this behaviour since the debian directory
is always coming from a separate tarball and thus both debian directories
are entirely unrelated. With "3.0 (native)" you want to keep the debian
directory because it only exists in the upstream tarball. With "1.0" it
depends...

-- System Information:
Debian Release: 7.0
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'testing'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 3.7-trunk-amd64 (SMP w/4 CPU cores)
Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages git-buildpackage depends on:
ii  devscripts       2.12.6
ii  git              1:1.7.10.4-2
ii  python           2.7.3-4
ii  python-dateutil  1.5+dfsg-0.1

Versions of packages git-buildpackage recommends:
ii  cowbuilder    0.71
ii  pristine-tar  1.26

Versions of packages git-buildpackage suggests:
ii  python-notify  0.1.1-3
ii  unzip          6.0-8

-- no debconf information



Information forwarded to debian-bugs-dist@lists.debian.org, Guido Günther <agx@sigxcpu.org>:
Bug#700411; Package git-buildpackage. (Sun, 05 May 2013 17:45:10 GMT) Full text and rfc822 format available.

Acknowledgement sent to Carlos Alberto Lopez Perez <clopez@igalia.com>:
Extra info received and forwarded to list. Copy sent to Guido Günther <agx@sigxcpu.org>. (Sun, 05 May 2013 17:45:10 GMT) Full text and rfc822 format available.

Message #10 received at 700411@bugs.debian.org (full text, mbox):

From: Carlos Alberto Lopez Perez <clopez@igalia.com>
To: Raphaël Hertzog <hertzog@debian.org>
Cc: 700411@bugs.debian.org, control@bugs.debian.org
Subject: Re: git-buildpackage: git-import-orig should filter the upstream debian directory
Date: Sun, 05 May 2013 19:41:04 +0200
[Message part 1 (text/plain, inline)]
tags 700411 patch
thanks

On 12/02/13 14:37, Raphaël Hertzog wrote:
> 
> git-import-orig will happily import upstream tarballs that contain a
> debian directory and when the upstream debian dir changes, git-import-orig
> will try to merge those changes in the real debian packaging available on
> the master branch.
> 
> In the best case, it fails, in the worst case it leads to indesirable
> changes.
> 
> I tried to work-around this problem with --filter='debian/*' but there's
> no safe pattern to use... because this one will also strip
> "src/plugins/debian" from the upstream tarball and we don't really want
> this by default.
> 
> So I would suggest that git-import-orig should automatically remove the
> top-level "debian" directory when it does its work.
> 
> I'm not sure how git-import-dsc behaves in the same situation but it
> should probably adjust its behaviour depending on the source format.
> With 3.0 (quilt), you want this behaviour since the debian directory
> is always coming from a separate tarball and thus both debian directories
> are entirely unrelated. With "3.0 (native)" you want to keep the debian
> directory because it only exists in the upstream tarball. With "1.0" it
> depends...
> 

Hi!

I have been also affected by this issue.

I have cooked a patch to fix this. It checks if the unpacked upstream
tarball contains a "debian/" directory, then it checks if the current
package is native or not. If it's not native, then it wipes the debian/
directory from the unpacked upstream tarball before doing the import.

The patch attached is against git-buildpackage=0.6.0~git20130414
[filter-upstream-debian-directory.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, attachment)]

Added tag(s) patch. Request was from Carlos Alberto Lopez Perez <clopez@igalia.com> to control@bugs.debian.org. (Sun, 05 May 2013 17:45:13 GMT) Full text and rfc822 format available.

Information forwarded to debian-bugs-dist@lists.debian.org:
Bug#700411; Package git-buildpackage. (Mon, 06 May 2013 16:24:04 GMT) Full text and rfc822 format available.

Acknowledgement sent to Guido Günther <agx@sigxcpu.org>:
Extra info received and forwarded to list. (Mon, 06 May 2013 16:24:04 GMT) Full text and rfc822 format available.

Message #17 received at 700411@bugs.debian.org (full text, mbox):

From: Guido Günther <agx@sigxcpu.org>
To: Carlos Alberto Lopez Perez <clopez@igalia.com>, 700411@bugs.debian.org
Cc: Raphaël Hertzog <hertzog@debian.org>, control@bugs.debian.org
Subject: Re: Bug#700411: git-buildpackage: git-import-orig should filter the upstream debian directory
Date: Mon, 6 May 2013 18:20:08 +0200
tag 700411 -patch
thanks.

Hi Carlos,
On Sun, May 05, 2013 at 07:41:04PM +0200, Carlos Alberto Lopez Perez wrote:
> tags 700411 patch
> thanks
> 
> On 12/02/13 14:37, Raphaël Hertzog wrote:
> > 
> > git-import-orig will happily import upstream tarballs that contain a
> > debian directory and when the upstream debian dir changes, git-import-orig
> > will try to merge those changes in the real debian packaging available on
> > the master branch.
> > 
> > In the best case, it fails, in the worst case it leads to indesirable
> > changes.
> > 
> > I tried to work-around this problem with --filter='debian/*' but there's
> > no safe pattern to use... because this one will also strip
> > "src/plugins/debian" from the upstream tarball and we don't really want
> > this by default.
> > 
> > So I would suggest that git-import-orig should automatically remove the
> > top-level "debian" directory when it does its work.
> > 
> > I'm not sure how git-import-dsc behaves in the same situation but it
> > should probably adjust its behaviour depending on the source format.
> > With 3.0 (quilt), you want this behaviour since the debian directory
> > is always coming from a separate tarball and thus both debian directories
> > are entirely unrelated. With "3.0 (native)" you want to keep the debian
> > directory because it only exists in the upstream tarball. With "1.0" it
> > depends...
> > 
> 
> Hi!
> 
> I have been also affected by this issue.
> 
> I have cooked a patch to fix this. It checks if the unpacked upstream
> tarball contains a "debian/" directory, then it checks if the current
> package is native or not. If it's not native, then it wipes the debian/
> directory from the unpacked upstream tarball before doing the import.

It's a great start but before applying it we should also make sure
git-import-dsc behaves consistently and also not do it for 1.0 packages
since these are expected to have a patch that applies against the
upstream tarball.
Cheers,
 -- Guido

> 
> The patch attached is against git-buildpackage=0.6.0~git20130414

> --- a/gbp/scripts/import_orig.py
> +++ b/gbp/scripts/import_orig.py
> @@ -21,11 +21,13 @@
>  import os
>  import sys
>  import tempfile
> +import shutil
>  import gbp.command_wrappers as gbpc
>  from gbp.deb import (DebianPkgPolicy, parse_changelog_repo)
>  from gbp.deb.uscan import (Uscan, UscanError)
>  from gbp.deb.changelog import ChangeLog, NoChangeLogError
>  from gbp.deb.git import (GitRepositoryError, DebianGitRepository)
> +from gbp.deb.source import DebianSource
>  from gbp.config import GbpOptionParserDebian, GbpOptionGroup, no_upstream_branch_msg
>  from gbp.errors import GbpError
>  import gbp.log
> @@ -283,6 +285,13 @@
>              tmpdir = tempfile.mkdtemp(dir='../')
>              source.unpack(tmpdir, options.filters)
>              gbp.log.debug("Unpacked '%s' to '%s'" % (source.path, source.unpacked))
> +            if os.path.isdir("%s" %(os.path.join(source.unpacked,"debian"))):
> +                try:
> +                    if not DebianSource('.').is_native():
> +                        shutil.rmtree("%s" %(os.path.join(source.unpacked,"debian")))
> +                        gbp.log.info("Deleted 'debian/' directory from unpacked upstream tarball before import")
> +                except Exception as e:
> +                    raise GbpError("Can't determine package type: %s" % e)
>  
>          if source.needs_repack(options):
>              gbp.log.debug("Filter pristine-tar: repacking '%s' from '%s'" % (source.path, source.unpacked))






Removed tag(s) patch. Request was from Guido Günther <agx@sigxcpu.org> to control@bugs.debian.org. (Mon, 06 May 2013 16:24:07 GMT) Full text and rfc822 format available.

Information forwarded to debian-bugs-dist@lists.debian.org, Guido Günther <agx@sigxcpu.org>:
Bug#700411; Package git-buildpackage. (Sat, 15 Feb 2014 18:39:04 GMT) Full text and rfc822 format available.

Acknowledgement sent to Carlos Alberto Lopez Perez <clopez@igalia.com>:
Extra info received and forwarded to list. Copy sent to Guido Günther <agx@sigxcpu.org>. (Sat, 15 Feb 2014 18:39:04 GMT) Full text and rfc822 format available.

Message #24 received at 700411@bugs.debian.org (full text, mbox):

From: Carlos Alberto Lopez Perez <clopez@igalia.com>
To: Guido Günther <agx@sigxcpu.org>, 700411@bugs.debian.org
Cc: Raphaël Hertzog <hertzog@debian.org>, control@bugs.debian.org
Subject: Re: Bug#700411: git-buildpackage: git-import-orig should filter the upstream debian directory
Date: Sat, 15 Feb 2014 19:35:45 +0100
[Message part 1 (text/plain, inline)]
tag 700411 +patch
thanks

On 06/05/13 18:20, Guido Günther wrote:
> 
> It's a great start but before applying it we should also make sure
> git-import-dsc behaves consistently and also not do it for 1.0 packages
> since these are expected to have a patch that applies against the
> upstream tarball.
> Cheers,
>  -- Guido
> 

I'm attaching a rebased patch that checks for the source format of the
package and only wipes the debian/ directory from upstream tarball if
the source format is 3.0 (quilt).


I'm not sure if for the other 3.0 type of packages (git,bzr,custom)
deleting the debian/ directory is expected. If this was the case, then
is a simple matter of changing on the patch:

-                    if source_format.version == "3.0" and source_format.type == "quilt":
+                    if source_format.version == "3.0" and source_format.type != "native":



Regards!
[filter-upstream-debian-directory-on-packages-3.0-qui.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, attachment)]

Added tag(s) patch. Request was from Carlos Alberto Lopez Perez <clopez@igalia.com> to control@bugs.debian.org. (Sat, 15 Feb 2014 18:39:11 GMT) Full text and rfc822 format available.

Information forwarded to debian-bugs-dist@lists.debian.org, Guido Günther <agx@sigxcpu.org>:
Bug#700411; Package git-buildpackage. (Sat, 15 Feb 2014 19:24:09 GMT) Full text and rfc822 format available.

Acknowledgement sent to Raphael Hertzog <hertzog@debian.org>:
Extra info received and forwarded to list. Copy sent to Guido Günther <agx@sigxcpu.org>. (Sat, 15 Feb 2014 19:24:09 GMT) Full text and rfc822 format available.

Message #31 received at 700411@bugs.debian.org (full text, mbox):

From: Raphael Hertzog <hertzog@debian.org>
To: Carlos Alberto Lopez Perez <clopez@igalia.com>
Cc: Guido Günther <agx@sigxcpu.org>, 700411@bugs.debian.org
Subject: Re: Bug#700411: git-buildpackage: git-import-orig should filter the upstream debian directory
Date: Sat, 15 Feb 2014 20:20:03 +0100
Hi,

On Sat, 15 Feb 2014, Carlos Alberto Lopez Perez wrote:
> I'm attaching a rebased patch that checks for the source format of the
> package and only wipes the debian/ directory from upstream tarball if
> the source format is 3.0 (quilt).

Thanks!

> I'm not sure if for the other 3.0 type of packages (git,bzr,custom)
> deleting the debian/ directory is expected.

No, it's not a good idea for the other type of formats.

> +            # wipe upstream "debian/" if package is 3.0 (quilt)
> +            if os.path.isfile(DebianSourceFormat.format_file) and os.path.isdir("%s" %(os.path.join(source.unpacked,"debian"))):

Maybe you should put the result of os.path.join(source.unpacked,"debian")
in a variable since you redo the same operation a few lines later.

And you might want to wrap this on 2 lines since it's very long.

> +                try:
> +                    source_format = DebianSourceFormat.parse_file(DebianSourceFormat.format_file)
> +                    if source_format.version == "3.0" and source_format.type == "quilt":
> +                        shutil.rmtree("%s" %(os.path.join(source.unpacked,"debian")))
> +                        gbp.log.info("Deleted 'debian/' directory from unpacked upstream tarball before import")
> +                except Exception as e:
> +                    raise GbpError("Can't determine package type: %s" % e)

What exception are you trying to catch here? You already checked that we
have a debian/source/format file.

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Discover the Debian Administrator's Handbook:
→ http://debian-handbook.info/get/



Information forwarded to debian-bugs-dist@lists.debian.org, Guido Günther <agx@sigxcpu.org>:
Bug#700411; Package git-buildpackage. (Sat, 15 Feb 2014 19:51:04 GMT) Full text and rfc822 format available.

Acknowledgement sent to Carlos Alberto Lopez Perez <clopez@igalia.com>:
Extra info received and forwarded to list. Copy sent to Guido Günther <agx@sigxcpu.org>. (Sat, 15 Feb 2014 19:51:04 GMT) Full text and rfc822 format available.

Message #36 received at 700411@bugs.debian.org (full text, mbox):

From: Carlos Alberto Lopez Perez <clopez@igalia.com>
To: Raphael Hertzog <hertzog@debian.org>
Cc: Guido Günther <agx@sigxcpu.org>, 700411@bugs.debian.org
Subject: Re: Bug#700411: git-buildpackage: git-import-orig should filter the upstream debian directory
Date: Sat, 15 Feb 2014 20:50:07 +0100
[Message part 1 (text/plain, inline)]
On 15/02/14 20:20, Raphael Hertzog wrote:
> Maybe you should put the result of os.path.join(source.unpacked,"debian")
> in a variable since you redo the same operation a few lines later.
> 
> And you might want to wrap this on 2 lines since it's very long.
> 
>> +                try:
>> +                    source_format = DebianSourceFormat.parse_file(DebianSourceFormat.format_file)
>> +                    if source_format.version == "3.0" and source_format.type == "quilt":
>> +                        shutil.rmtree("%s" %(os.path.join(source.unpacked,"debian")))
>> +                        gbp.log.info("Deleted 'debian/' directory from unpacked upstream tarball before import")
>> +                except Exception as e:
>> +                    raise GbpError("Can't determine package type: %s" % e)
> 
> What exception are you trying to catch here? You already checked that we
> have a debian/source/format file.

Yes, you are right, that try-except don't makes much sense.

Thanks for your comments.


I'm attaching a new version of the patch.
[filter-upstream-debian-directory-on-packages-3.0-quilt-v2.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, attachment)]

Send a report that this bug log contains spam.


Debian bug tracking system administrator <owner@bugs.debian.org>. Last modified: Sun Apr 20 10:52:47 2014; Machine Name: buxtehude.debian.org

Debian Bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.