Bug#896084: dose-builddebcheck: Finds invalid solution with versioned provides

[Ralf Treinen]

Hi James,

On Thu, Apr 19, 2018 at 04:25:47PM +0100, James Clarke wrote:
> On Thu, Apr 19, 2018 at 11:40:53AM +0100, James Clarke wrote:

> It turns out this is the same problem as upstream's two failing
> versioned provides tests[1]. The attached patch fixes all of this.

a big thank you for having tracked down this bug, and having written
the patch. I am quite impressed that you dived into the arcanes of
deb to cudf encoding. The solution that you propose, that is creating
separate pseudo-packages for the vesionend and the unversioned virtual
package, is probably the only correct way to work around a bad design
decision in the cudf format (for which I am partly responsable).

I will have a deeper look at what the deb to cudf encoding looks
now over the weekend, and upload the fixed debian package very soon.

Cheers -Ralf.

Bug#896084: dose-builddebcheck: Finds invalid solution with versioned provides

[James Clarke]

On Thu, Apr 19, 2018 at 11:40:53AM +0100, James Clarke wrote:
> Package: dose-builddebcheck
> Version: 5.0.1-9
> Tags: upstream
>
> [It's quite likely this should be against libdose3-ocaml(-dev) as I
> imagine this is a bug in the common library code also used by
> dose-distcheck]
>
> Hi,
> Whilst running a new kfreebsd-{amd64,i386} buildd, I noticed that
> dose-builddebcheck doesn't quite handle versioned provides correctly[1],
> when combined with versioned dependencies. This
> stems from the use of MAX_INT32-1 for an unversioned provides, which is
> of course greater than any version number used in the dependency.

It turns out this is the same problem as upstream's two failing
versioned provides tests[1]. The attached patch fixes all of this.

Regards,
James

[1] 
https://lists.gforge.inria.fr/pipermail/dose-devel/2017-September/000660.html
>From 2b89c68f0711880ce06037d3aa2680215f9a0ab1 Mon Sep 17 00:00:00 2001
From: James Clarke 
Date: Thu, 19 Apr 2018 14:59:49 +0100
Subject: [PATCH] Fix >> and >= constraints against virtual packages

Previously, MAX_INT32-1 was used as the version number for an
unversioned provide (and also generated alongside a versioned provide),
which incorrectly satisfies any >> or >= constraint. Instead,
use --virtual--versioned- as a separate prefix for versioned
provides and constraints, whilst keeping --virtual- for unversioned
provides. This fixes the remaining broken versioned provides test cases.
---
 deb/debcudf.ml | 53 +-
 deb/tests.ml   | 26 -
 2 files changed, 35 insertions(+), 44 deletions(-)

diff --git a/deb/debcudf.ml b/deb/debcudf.ml
index c70ed67..534cb63 100644
--- a/deb/debcudf.ml
+++ b/deb/debcudf.ml
@@ -224,12 +224,16 @@ let get_cudf_version tables (package,version) =
   end
 
 let get_real_name name = 
-  (* Remove --virtual- and architecture encoding *)
+  (* Remove --virtual(--versioned)- and architecture encoding *)
   let dn = (CudfAdd.decode name) in
   let no_virtual =
 if ExtString.String.starts_with dn "--vir"
-then ExtString.String.slice ~first:10 dn
-else dn
+then begin
+  let maybe_versioned = ExtString.String.slice ~first:10 dn in
+  if ExtString.String.starts_with maybe_versioned "-ver"
+  then ExtString.String.slice ~first:11 maybe_versioned
+  else maybe_versioned
+end else dn
   in
   try
 let (n,a) = ExtString.String.split no_virtual ":" in
@@ -260,35 +264,21 @@ let loadl ?native_arch ?package_arch tables l =
   List.flatten (
 List.map (fun ((name,_) as vpkgname,constr) ->
   let encname = add_arch_info ?native_arch ?package_arch vpkgname in
-  match constr with
-  |None ->
+  let (virt_prefix, constr) =
+match constr with
+|None ->
   (* Versioned virtual packages will satisfiy non versioned 
dependencies *)
-  if (OcamlHashtbl.mem tables.virtual_table name) then
-[(encname, None);("--virtual-"^encname,Some(`Eq,Util.max32int - 
1))]
-  else
-[(encname, None)]
-  |Some(op,v) ->
+  ("--virtual-", None)
+|Some(op,v) ->
   (* Non-versioned virtual packages will not satisfy versioned 
dependencies. *)
   let op = Pef.Pefcudf.pefcudf_op op in
-  try
-match SSet.elements !(OcamlHashtbl.find tables.virtual_table name) 
with
-|[] -> assert false
-|l ->
-let dl = 
-  List.filter_map (function
-|(_,None) -> Some 
("--virtual-"^encname,Some(`Eq,Util.max32int))
-|(_,Some _) ->
-let constr = Some(op,get_cudf_version tables (name,v)) 
in
-Some ("--virtual-"^encname,constr)
-  ) l
-in
-if Util.StringHashtbl.mem tables.unit_table name then
-  let constr = Some(op,get_cudf_version tables (name,v)) in
-  (encname,constr)::dl
-else dl
-  with Not_found -> 
-  let constr = Some(op,get_cudf_version tables (name,v)) in
-  [(encname,constr)]
+  let constr = Some(op,get_cudf_version tables (name,v)) in
+  ("--virtual--versioned-",constr)
+  in
+  if (OcamlHashtbl.mem tables.virtual_table name) then
+[(encname, constr);(virt_prefix^encname,constr)]
+  else
+[(encname, constr)]
 ) l
   )
 
@@ -308,12 +298,13 @@ let loadlp ?native_arch ?package_arch tables l =
   List.flatten (
 List.map (fun ((name,_) as vpkgname,constr) ->
 let encname = add_arch_info ?native_arch ?package_arch vpkgname in
-let vencname = "--virtual-"^encname in 
+let vencname = "--virtual-"^encname in
+let vvencname = "--virtual--versioned-"^encname in
 match constr with
 |None -> [(vencname,Some(`Eq,Util.max32int - 1))]
 |Some("=",v) ->
   let 

Bug#896084: dose-builddebcheck: Finds invalid solution with versioned provides

[James Clarke]

Package: dose-builddebcheck
Version: 5.0.1-9
Tags: upstream

[It's quite likely this should be against libdose3-ocaml(-dev) as I
imagine this is a bug in the common library code also used by
dose-distcheck]

Hi,
Whilst running a new kfreebsd-{amd64,i386} buildd, I noticed that
dose-builddebcheck doesn't quite handle versioned provides correctly[1],
when combined with versioned dependencies. This
stems from the use of MAX_INT32-1 for an unversioned provides, which is
of course greater than any version number used in the dependency.

Below is a tiny example illustrating the problem.

Regards,
James

[1] An example is libsigrok, which build depends on libusb-1.0-0-dev (>=
1.0.16), which doesn't exist on kFreeBSD; instead libusb2 provides
libusb-1.0-0-dev (= 1.0.6). Because dose-builddebcheck thinks this
is satisfiable, wanna-build keeps scheduling builds, but apt keeps
failing to find a solution (since there isn't one).

> jrtc27@deb4g:~/tmp/dose-versioned-provides$ cat Packages
> Package: build-essential
> Version: 1
> Architecture: amd64
>
> Package: binary-package
> Version: 1
> Architecture: amd64
> Provides: provided (= 2)
> jrtc27@deb4g:~/tmp/dose-versioned-provides$ cat Sources
> Package: source-package
> Architecture: any
> Version: 1
> Build-Depends: provided (>= 3)
> jrtc27@deb4g:~/tmp/dose-versioned-provides$ dose-builddebcheck 
> --deb-native-arch=amd64 -es --dump=cudf Packages Sources
> output-version: 1.2
> native-architecture: amd64
> report:
>  -
>   package: source-package
>   version: 1
>   architecture: any
>   type: src
>   status: ok
>   installationset:
>-
> package: source-package
> version: 1
> architecture: any
> type: src
>-
> package: binary-package
> version: 1
> architecture: amd64
>-
> package: build-essential
> version: 1
> architecture: amd64
>
> binary-packages: 3
> source-packages: 1
> broken-packages: 0
> jrtc27@deb4g:~/tmp/dose-versioned-provides$ cat cudf
> preamble:
> property: native: int = [0], multiarch: string = [""], installedsize: int = 
> [0], filename: string = [""], essential: bool = [false], sourceversion: int = 
> [1], sourcenumber: string = [""], source: string = [""], priority: string = 
> [""], recommends: vpkgformula = [true!], replaces: vpkglist = [], 
> architecture: string, type: string, number: string, name: string
>
> package: binary-package%3aamd64
> version: 5
> conflicts: binary-package%3aamd64 , binary-package
> provides: binary-package , --virtual-provided%3aamd64 = 3 , 
> --virtual-provided%3aamd64 = 2147483646
> name: binary-package
> architecture: amd64
> number: 1
> source: binary-package
> sourcenumber: 1
> sourceversion: 5
> native: 1
> type: bin
> filename: Packages
>
> package: src%3asource-package
> version: 2
> depends: build-essential%3aamd64 , --virtual-provided%3aamd64 >= 4
> conflicts: src%3asource-package%3aamd64 , src%3asource-package
> provides: src%3asource-package
> name: source-package
> architecture: any
> number: 1
> source: source-package
> sourcenumber: 1
> sourceversion: 2
> type: src
>
> package: build-essential%3aamd64
> version: 2
> conflicts: build-essential%3aamd64 , build-essential
> provides: build-essential
> name: build-essential
> architecture: amd64
> number: 1
> source: build-essential
> sourcenumber: 1
> sourceversion: 2
> native: 1
> type: bin
> filename: Packages