Home My Page Projects Cudf library
Summary Activity Tracker Lists SCM Files

[#19411] Cudf.add_package returns different package uid-s than Cudf.load_universe

Date:
2015-07-12 13:20
Priority:
3
State:
Open
Submitted by:
Vincent Bernardoff (vbmithr)
Assigned to:
Stefano Zacchiroli (zack)
Summary:
Cudf.add_package returns different package uid-s than Cudf.load_universe

Detailed description
Because of this:

let uid = (Hashtbl.length univ.uid2pkgs) + 1 in

Doing a Cudf.add_universe pkg_list is not equivalent to

List.iter Cudf.add_package Cudf.empty_universe

If the second case, the packages are indexed from 1, as opposed to 0 in the first case.

I think the two functions should be semantically equivalent.
This would be useful for OPAM.
Message  ↓
Date: 2015-07-15 09:37
Sender: Vincent Bernardoff

Actually, this is a dose bug.

Dose rely on packages in a universe being indexed starting from zero.
See https://gforge.inria.fr/tracker/index.php?func=detail&aid=19418&group_id=4395&atid=13808

Date: 2015-07-14 16:22
Sender: Vincent Bernardoff

Right, this is an OPAM bug. I'm going to investigate this. Thanks.

Date: 2015-07-14 16:05
Sender: Stefano Zacchiroli

But that doesn't answer my question, does it?
It looks like a bug in opam itself, which is making assumptions that it shouldn't make on how package uid are generated...

Unfortunately, I know nothing about the opam internals, so I can't say if that is because there are no easier alternatives to do what opam is trying to do --- nor I know what actually opam is trying to do.

I'm open to do the change you suggested, but I still want to understand first why it is needed. (And "without it opam crashes" is not a valid answer, IMO.)

Date: 2015-07-14 15:58
Sender: Vincent Bernardoff

https://github.com/vbmithr/opam/commit/34b92b1c1f1acf1f2e207c17b21ccf824bdb654f

This patch makes opam raise Not_found and crash if I don't make the modification I talked about.

Date: 2015-07-14 15:38
Sender: Stefano Zacchiroli

> If the second case, the packages are indexed from 1, as opposed to 0 in the first case.

Thanks for your bug report. I can see why this asymmetry might be inelegant and I don't have strong objections to fixing it.
However, I'm a bit worried by the reliance that client code might start putting in any specific uid allocation scheme. If you want to retrieve the uid for a given package, no matter the allocation scheme, you do have a way: Cudf.uid_by_package.

So, maybe, can you elaborate on this:

> This would be useful for OPAM.

why is it so and on which way would OPAM benefit from a stable allocation scheme?
Also: how *much* would OPAM rely on that. I need to understand whether the change you're asking is meant to become part of the cudf "ABI" (which would definitely give me pause).

Cheers.

Field Old Value Date By
assigned_tonone2015-07-14 15:39zack
summaryCudf.add_package2015-07-14 15:39zack