Bug#730572: reprepro: support for ddebs (debug symbols)

[Simon McVittie]

On Sun, 20 Dec 2015 at 08:53:16 +, Niels Thykier wrote:
> In the actual implementation we got live now, there are a couple of
> changes though.
> 
>  * The dbgsym packages use the .deb extension

For the non-Debian projects for which I developed this patch, we still
need at least basic support for .ddeb files, because Ubuntu's toolchain
still produces those (and as far as I can tell it will continue to do
so indefinitely - they no longer use pkg-create-dbgsym, but they have a
patch in their dh_strip to make it produce .ddeb instead of .deb files).

I wouldn't object to simplifying it by treating .ddeb files as exactly
equivalent to .deb, so they go alongside ordinary debs, instead of
creating a /debug pseudo-component? That's what happens right
now when you import a Debian-built -dbgsym package into an unpatched
reprepro. (You can't currently import an Ubuntu-built -dbgsym package
at all.)

(Cc'ing my colleague Lucas Kanashiro who will be looking into rebasing
this patch for our own use - we need a fork of reprepro that supports
this, even if it can't go upstream, so we might as well provide an
updated patch on this bug too.)

> In DAK / the Debian infrastructure, the dbgsym packages are placed in a
> separate component called "-debug".

Isn't that a suite or a "dist" or something, rather than a component?
The production Debian dbgsym infrastructure seems to have the same
three components (aka archive areas) as the main Debian archive, namely
main, contrib and non-free.

I don't think reprepro can or should mimic dak's output accurately,
because dak creates a separate lookaside apt repository for detached
debug symbols, but the scope of reprepro is that it deals with a single
apt repository.

Because reprepro can already accept *-dbgsym_*.deb and will currently
list them alongside any other .deb, moving Debian-built (.deb) -dbgsym
packages into a separate suite, component or repository would arguably
be an incompatible behaviour change.

smcv

Bug#730572: reprepro: support for ddebs (debug symbols)

[Niels Thykier]

On Thu, 19 Dec 2013 14:45:37 + Simon McVittie
 wrote:
> On 01/12/13 02:34, Bernhard R. Link wrote:
> > * Simon McVittie  [131126 18:03]:
> >> The attached patch is a proof-of-concept implementation
> 
> [...]
> 
> Thanks! I've rebased onto that, and added a regression test (once I
> found where testtool lives - please consider including that in reprepro
> tarballs, if it isn't likely to be packaged separately).
> 
> > I think if those packages will finally come without a Priority or
> > Section in the package, there will be a more complex solution needed,
> > as without no Section at all, reprepro will not accept the package
> > yet (unless there is one in the override file).
> 
> Fixed, I think: I've forced them to Section: debug, Priority: extra.
> 
> [...]

Hi,

Thanks for working on dbgsym support for reprepro.

In the actual implementation we got live now, there are a couple of
changes though.

 * The dbgsym packages use the .deb extension
 * The primary indicator of a dbgsym file is the header
   "Auto-Built-Package", which is set to (exactly) "debug-symbols"
 * Further validation that can/should be done:
   - The package name should end with "-dbgsym"
   - The package should be in section "debug" (possibly in a *non*
 main component)
 * If it sees a package "foo-dbgsym" with the header, then it should
   also assert that a package called "foo" is uploaded.  The latter
   should be listed in the control file.

In DAK / the Debian infrastructure, the dbgsym packages are placed in a
separate component called "-debug".  The current mirror is
available at [1].

Thanks,
~Niels

[1] http://debug.mirrors.debian.org/



signature.asc
Description: OpenPGP digital signature

Bug#730572: reprepro: support for ddebs (debug symbols)

[Simon McVittie]

On 01/12/13 02:34, Bernhard R. Link wrote:
 * Simon McVittie simon.mcvit...@collabora.co.uk [131126 18:03]:
 The attached patch is a proof-of-concept implementation

 -if (entry-type == fe_DEB || entry-type == fe_UDEB)
 +if (FE_BINARY(entry-type))
 
 I've already applied this one. As it makes sense anyway.

Thanks! I've rebased onto that, and added a regression test (once I
found where testtool lives - please consider including that in reprepro
tarballs, if it isn't likely to be packaged separately).

 I think if those packages will finally come without a Priority or
 Section in the package, there will be a more complex solution needed,
 as without no Section at all, reprepro will not accept the package
 yet (unless there is one in the override file).

Fixed, I think: I've forced them to Section: debug, Priority: extra.

 Will there even be Contents files for ddebs?

*shrug* it was easier to implement them than to consider whether to do so...

S

From 9703835e886cce91f647a381ffbe5a6cecec3242 Mon Sep 17 00:00:00 2001
From: Simon McVittie simon.mcvit...@collabora.co.uk
Date: Thu, 19 Dec 2013 14:12:02 +
Subject: [PATCH 2/3] Add basic support for ddebs (Ubuntu-style -dbgsym
 packages)

For the moment they're treated a lot like udebs: they're copied into
the same pool as normal debs and udebs, and their package lists
go into a subdirectory. The pull/updates logic doesn't support
downloading them, since we don't yet know how Debian mirrors that
carry ddebs will be laid out (and Ubuntu uses a separate public-facing
archive for them).

Bug-Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=730572
---
 atoms.c| 12 +---
 atoms.h|  1 +
 changes.c  |  6 --
 changes.h  |  6 +++---
 checkin.c  | 23 +++
 checkindeb.c   | 55 ++-
 contents.c | 48 +++-
 contents.h |  1 +
 copypackages.c |  9 -
 distribution.c | 45 +
 distribution.h |  7 ++-
 incoming.c | 15 +++
 main.c | 28 
 pull.c |  2 ++
 target.c   | 25 +
 target.h   |  1 +
 tool.c | 26 +++---
 updates.c  |  3 +++
 18 files changed, 278 insertions(+), 35 deletions(-)

diff --git a/atoms.c b/atoms.c
index 25214ee..7fa980d 100644
--- a/atoms.c
+++ b/atoms.c
@@ -26,7 +26,7 @@
 
 const char **atoms_architectures;
 const char **atoms_components;
-const char * const packagetypes[4] = { !!NONE!!, dsc, deb, udeb };
+const char * const packagetypes[5] = { !!NONE!!, dsc, deb, udeb, ddeb };
 const char **atoms_packagetypes = (const char **)packagetypes;
 const char **atoms_commands;
 static int command_count;
@@ -159,6 +159,8 @@ packagetype_t packagetype_find(const char *value) {
 		return pt_deb;
 	else if (strcmp(value, udeb) == 0)
 		return pt_udeb;
+	else if (strcmp(value, ddeb) == 0)
+		return pt_ddeb;
 	else
 		return atom_unknown;
 }
@@ -169,8 +171,12 @@ packagetype_t packagetype_find_l(const char *value, size_t len) {
 			return pt_dsc;
 		else if (strncmp(value, deb, 3) == 0)
 			return pt_deb;
-	} else if (len == 4  strncmp(value, udeb, 4) == 0)
-		return pt_udeb;
+	} else if (len == 4) {
+		if (strncmp(value, udeb, 4) == 0)
+			return pt_udeb;
+		else if (strncmp(value, ddeb, 4) == 0)
+			return pt_ddeb;
+	}
 	return atom_unknown;
 }
 
diff --git a/atoms.h b/atoms.h
index be78713..660adb4 100644
--- a/atoms.h
+++ b/atoms.h
@@ -19,6 +19,7 @@ enum atom_type { at_architecture, at_component, at_packagetype, at_command };
 #define pt_dsc ((packagetype_t)1)
 #define pt_deb ((packagetype_t)2)
 #define pt_udeb ((packagetype_t)3)
+#define pt_ddeb ((packagetype_t)4)
 
 #define atom_defined(a) ((a)  (atom_t)0)
 
diff --git a/changes.c b/changes.c
index 5d01754..259162b 100644
--- a/changes.c
+++ b/changes.c
@@ -166,6 +166,8 @@ retvalue changes_parsefileline(const char *fileline, /*@out@*/filetype *result_t
 		type = fe_DEB;
 	else if (l = 5  memcmp(p-5, .udeb, 5) == 0)
 		type = fe_UDEB;
+	else if (l = 5  memcmp(p-5, .ddeb, 5) == 0)
+		type = fe_DDEB;
 	else
 		type = fe_UNKNOWN;
 
@@ -189,7 +191,7 @@ retvalue changes_parsefileline(const char *fileline, /*@out@*/filetype *result_t
 		if (type == fe_DEB)
 			archend = versionstart + l - 4;
 		else {
-			assert (type == fe_UDEB);
+			assert (type == fe_UDEB || type == fe_DDEB);
 			archend = versionstart + l - 5;
 		}
 		if (archend - archstart == 6 
@@ -203,7 +205,7 @@ retvalue changes_parsefileline(const char *fileline, /*@out@*/filetype *result_t
 		const char *eoi;
 
 		/* without those, it gets more complicated.
-		 * It's not .deb or .udeb, so most likely a
+		 * It's not .deb or .udeb or .ddeb, so most likely a
 		 * source file (or perhaps a log (reprepro extension)) */
 
 		/* if it uses a known compression, things are easy,
diff --git a/changes.h 

Bug#730572: reprepro: support for ddebs (debug symbols)

[Bernhard R. Link]

* Simon McVittie simon.mcvit...@collabora.co.uk [131126 18:03]:
 (Deliberately not tagging this +patch right now, since Debian doesn't
 officially support ddebs yet, and the archive layout looks likely to change.)
 
 Ubuntu has a semi-separate archive for separate debug symbols, and
 https://wiki.debian.org/AutomaticDebugPackages is the proposal to
 add the same thing to Debian. It would be great if reprepro could
 support them, too.
 
 The attached patch is a proof-of-concept implementation: it treats them
 as a mixture of .deb and .ddeb. It seems to work OK in conjunction with
 Ubuntu's pkg-create-dbgsym.

Thanks. That fits the coding style of reprepro quite well. If you'd
have claimed I had written it and just forgotten about about, I'd
have had a hard time arguing against that.

 diff --git a/checkin.c b/checkin.c
 index 02ba279..ba5331f 100644
 --- a/checkin.c
 +++ b/checkin.c
 @@ -130,7 +130,7 @@ static void freeentries(/*@only@*/struct fileentry 
 *entry) {
   free(entry-section);
   free(entry-priority);
   free(entry-name);
 - if (entry-type == fe_DEB || entry-type == fe_UDEB)
 + if (FE_BINARY(entry-type))
   deb_free(entry-pkg.deb);
else if (entry-type == fe_DSC) {
strlist_done(entry-needed_filekeys);

I've already applied this one. As it makes sense anyway.


 @@ -1164,8 +1169,7 @@ static retvalue changes_checkpkgs(struct distribution 
 *distribution, struct chan
   e = changes-files;
   while (e != NULL) {
   char *fullfilename;
 - if (e-type != fe_DEB  e-type != fe_DSC
 -  e-type != fe_UDEB) {
 + if (!FE_PACKAGE (e-type)) {
   e = e-next;
   continue;
   }

This, too. But without the space between E and (.

 + /* we use the deb overrides for ddebs too - ddebs aren't
 +  * meant to have overrides so this is probably fine */
   binoverride = distribution-overrides.deb;
   components = distribution-components;

I think if those packages will finally come without a Priority or
Section in the package, there will be a more complex solution needed,
as without no Section at all, reprepro will not accept the package
yet (unless there is one in the override file).


 @@ -173,20 +176,36 @@ static retvalue gentargetcontents(struct target 
 *target, struct release *release
   struct filetorelease *file;
   struct filelist_list *contents;
   struct target_cursor iterator;
 + const char *suffix;
 + const char *symlink_prefix;
  
   if (onlyneeded  target-saved_wasmodified)
   onlyneeded = false;
  
 + switch (target-packagetype) {
 + case pt_ddeb:
 + symlink_prefix = d;
 + suffix = -ddeb;
 + break;
 + case pt_udeb:
 + symlink_prefix = s;
 + suffix = -udeb;
 + break;
 + default:
 + symlink_prefix = ;
 + suffix = ;
 + }
 +
   contentsfilename = mprintf(%s/Contents%s-%s,
   atoms_components[target-component],
 - (target-packagetype == pt_udeb)?-udeb:,
 + suffix,
   atoms_architectures[target-architecture]);

Will there even be Contents files for ddebs?


 +/* This is pretty close to duplicating the enum typedef'd to filetype
 + * in changes.h, but its order corresponds to typesuffix[]. Do not
 + * confuse them. */
  enum filetype { ft_UNKNOWN,


uh, oh. Indeed, I should probably rename one of those two.

Bernhard R. Link
-- 
F8AC 04D5 0B9B 064B 3383  C3DA AFFC 96D1 151D FFDC


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#730572: reprepro: support for ddebs (debug symbols)

[Simon McVittie]

Package: reprepro
Version: 4.13.1-1+b1
Severity: wishlist

(Deliberately not tagging this +patch right now, since Debian doesn't
officially support ddebs yet, and the archive layout looks likely to change.)

Ubuntu has a semi-separate archive for separate debug symbols, and
https://wiki.debian.org/AutomaticDebugPackages is the proposal to
add the same thing to Debian. It would be great if reprepro could
support them, too.

The attached patch is a proof-of-concept implementation: it treats them
as a mixture of .deb and .ddeb. It seems to work OK in conjunction with
Ubuntu's pkg-create-dbgsym.

Known omissions:

* The wiki page says ddeb will have no override entries, so no priority
  or section. In my implementation, they can have override entries
  (sharing an override file with normal debs), although those are not
  mandatory, and they have whatever package and section they are given
  by pkg-create-dbgsym, currently Priority: extra and whatever Section
  was used by the ordinary deb.

* The wiki page says ddeb will go into the component debug, so will
  show up in ./pool/debug/ but in my implementation they go in
  pool/{main,contrib,non-free}. The wiki page lists Do we need to
  split ddeb per main/contrib/non-free? as an open issue, which is partly
  why I chose to sidestep that question for now.

* The wiki page says Standalone Release/Packages in ./debug/dists/
  but in my implementation they behave more like udebs: they go somewhere
  like dists/main/debug/binary-i386/main and are covered by the main
  Release file.

* ddebs seem to end up with a long Description, despite the wiki
  page saying they shouldn't. I think tha might be a pkg-create-dbgsym
  bug, though?

I hope this is a useful starting point anyway.

Regards,
S
From b1d737032fb3949bb018a0799b2ce457499c4c65 Mon Sep 17 00:00:00 2001
From: Simon McVittie simon.mcvit...@collabora.co.uk
Date: Tue, 26 Nov 2013 16:12:22 +
Subject: [PATCH 2/2] Add basic support for ddebs (Ubuntu-style -dbgsym
 packages)

For the moment they're treated a lot like udebs: they're copied into
the same pool as normal debs and udebs, and their package lists
go into a subdirectory. The pull/updates logic doesn't support
downloading them, since we don't yet know how Debian mirrors that
carry ddebs will be laid out (and Ubuntu uses a separate public-facing
archive for them).
---
 atoms.c| 12 +---
 atoms.h|  1 +
 changes.c  |  6 --
 changes.h  |  6 +++---
 checkin.c  | 31 ++-
 checkindeb.c   | 28 +++-
 contents.c | 48 +++-
 contents.h |  1 +
 copypackages.c |  9 -
 distribution.c | 45 +
 distribution.h |  7 ++-
 incoming.c | 15 +++
 main.c | 40 ++--
 pull.c |  2 ++
 target.c   | 25 +
 target.h   |  1 +
 tool.c | 26 +++---
 updates.c  |  3 +++
 18 files changed, 268 insertions(+), 38 deletions(-)

diff --git a/atoms.c b/atoms.c
index 25214ee..7fa980d 100644
--- a/atoms.c
+++ b/atoms.c
@@ -26,7 +26,7 @@
 
 const char **atoms_architectures;
 const char **atoms_components;
-const char * const packagetypes[4] = { !!NONE!!, dsc, deb, udeb };
+const char * const packagetypes[5] = { !!NONE!!, dsc, deb, udeb, ddeb };
 const char **atoms_packagetypes = (const char **)packagetypes;
 const char **atoms_commands;
 static int command_count;
@@ -159,6 +159,8 @@ packagetype_t packagetype_find(const char *value) {
 		return pt_deb;
 	else if (strcmp(value, udeb) == 0)
 		return pt_udeb;
+	else if (strcmp(value, ddeb) == 0)
+		return pt_ddeb;
 	else
 		return atom_unknown;
 }
@@ -169,8 +171,12 @@ packagetype_t packagetype_find_l(const char *value, size_t len) {
 			return pt_dsc;
 		else if (strncmp(value, deb, 3) == 0)
 			return pt_deb;
-	} else if (len == 4  strncmp(value, udeb, 4) == 0)
-		return pt_udeb;
+	} else if (len == 4) {
+		if (strncmp(value, udeb, 4) == 0)
+			return pt_udeb;
+		else if (strncmp(value, ddeb, 4) == 0)
+			return pt_ddeb;
+	}
 	return atom_unknown;
 }
 
diff --git a/atoms.h b/atoms.h
index be78713..660adb4 100644
--- a/atoms.h
+++ b/atoms.h
@@ -19,6 +19,7 @@ enum atom_type { at_architecture, at_component, at_packagetype, at_command };
 #define pt_dsc ((packagetype_t)1)
 #define pt_deb ((packagetype_t)2)
 #define pt_udeb ((packagetype_t)3)
+#define pt_ddeb ((packagetype_t)4)
 
 #define atom_defined(a) ((a)  (atom_t)0)
 
diff --git a/changes.c b/changes.c
index 5d01754..259162b 100644
--- a/changes.c
+++ b/changes.c
@@ -166,6 +166,8 @@ retvalue changes_parsefileline(const char *fileline, /*@out@*/filetype *result_t
 		type = fe_DEB;
 	else if (l = 5  memcmp(p-5, .udeb, 5) == 0)
 		type = fe_UDEB;
+	else if (l = 5  memcmp(p-5, .ddeb, 5) == 0)
+		type = fe_DDEB;
 	else
 		type = fe_UNKNOWN;
 
@@ -189,7 +191,7 @@ retvalue