Really compute transitive dependencies in opam_mk_repo (Close: #976).

[gildor478]

No description provided.

[samoht]

It's simpler to write:

OpamPackage.Set.(add nv (union (get_dependencies nv) set))

[samoht]

Woops... Thanks for the fix! Can you just correct that minor remark ?

[samoht]

After some thoughts I'm not totally convinced by your fix. I think it will be safer to simply add nv to the set returned by get_dependencies (not sure why you've added the if .. then .. else construct there: this works only if we are lucky with the order in which we traverse the already computed set).

[gildor478]

If nv is already in the set, it means that it has already been visited, so there is no reason to go through it once again. That is the reason of the if then else.

Adding the element to the get_dependencies, change slightly the meaning of the function since it will return not only the deps but the deps and itself (but that may be fine).

Doing it without the if then else, is fine. BUT not good in term of complexity because the set keep expanding with each recursive call and even if you already have computed deps you will try again to get it again and again...

But this is just my 2 cents and my 2 min hack done done late at night, I may have some bugs here.

[samoht]

If nv is already in the set, it means that it has already been visited, so there is no reason to go through it once again. That is the reason of the if then else.

Well not necessary; if nv is in the set it can also be a dependency of something we already have visited.

[gildor478]

Oh god, you are right in fact. I was mislead but the fact that we have only one set.

We should visited and to_visit set, and the end condition is that to_visit set must be empty.

[gildor478]

will fix that tonight, but the former algo was wrong anyway and the complexity argument stands.

[gildor478]

I think this is fixed, please take another look.

[samoht]

Thanks!