Versioned links from dependencies and reverse dependencies

[patricoferris]

Currently from any given package page (e.g. https://v3.ocaml.org/p/ipaddr/5.0.0) we link to both the dependencies and the reverse dependencies from the overview page.

The concrete links we use are just to the package name which always redirects to the latest version even when there are version constraints involved. For instance https://v3.ocaml.org/p/ipaddr/5.0.0 has a dependency on macaddr with the version constraint = version clicking on this should go to https://v3.ocaml.org/p/macaddr/5.0.0 but instead we go to https://v3.ocaml.org/p/macaddr (at the time of writing this is 5.2.0).

Other version constraints might be >= "3.1" & < "3.3. In this case we should redirect to the latest version which satisfies the version constraint.

IIRC for reverse dependencies we can add the logic in

https://github.com/ocaml/v3.ocaml.org-server/blob/f9da1e68f6d57bc62eef3837f8f1d30ff6b89753/src/ocamlorg/lib/package.ml#L129

à la opam2web https://github.com/ocaml-opam/opam2web/blob/d1f4296c62da19713d358d1792c7138102678628/src/o2wPackage.ml#L384-L394

[tmattio]

Would it make sense to support version constraints in the router? I.e. handling URLs like https://v3.ocaml.org/p/ocaml-compiler-libs/>=v0.12.0&<v0.13.0

[JiaeK]

Hi, can I work on this issue?

[tmattio]

Sure @JiaeK 🙂

[patricoferris]

Would it make sense to support version constraints in the router? I.e. handling URLs like https://v3.ocaml.org/p/ocaml-compiler-libs/>=v0.12.0&<v0.13.0

Hmm I think it would be easier to do this more statically when compiling the information about the package rather than dynamically through the router. As in the rev-deps example, we already have the complexity of calculating the reverse dependencies so it's just a short opam-library call away whereas moving things into the router would require exposing the server to opam libraries (even indirectly though the Ocamlorg module) right? Plus the versioning can be contextual i.e. where does https://v3.ocaml.org/p/conan/=version go if not contextualise by being on this page https://v3.ocaml.org/p/conan-lwt/0.0.1

[JiaeK]

Just wanna confirm, so the tasks are

• both the dependencies and reverse dependencies' links should redirect to the same version, when the version constraint shows = version
• Any versions which is >= "3.1" & < "3.3", should redirect to the latest version?

[tmattio]

I don't have a good understanding of how to do the reverse deps, so you can start with the dependencies for now (I'll read the code @patricoferris linked to and get back to you with my understanding) unless @patricoferris wants to shed some light on this before.

The >= "3.1" & < "3.3" was just an example of a constraint, but of course, it can be any constraint. The goal is to resolve the constraint and get the latest package version that matches it, then link to this one in the list of dependencies of a package, instead of linking to the last version of the package as we currently do.

[patricoferris]

The >= "3.1" & < "3.3" was just an example of a constraint, but of course, it can be any constraint. The goal is to resolve the constraint and get the latest package version that matches it, then link to this one in the list of dependencies of a package, instead of linking to the last version of the package as we currently do.

Exactly right. Working this into the dependencies is probably more complicated than the reverse dependencies (so a PR just for the rev deps would be fine too). We fold over each package and at that point we have all of the possible versions of the package (as a set):

https://github.com/ocaml/v3.ocaml.org-server/blob/f9da1e68f6d57bc62eef3837f8f1d30ff6b89753/src/ocamlorg/lib/package.ml#L132

At which point we can extract the "maximum element" of the set which is where we should link to. The maximum element as defined by version comparison will always be the right package to link to in this case.

EDIT: so we might have to expand the rev_deps type to contain information about the latest matching version so we can link to it in the package overview page.

https://github.com/ocaml/v3.ocaml.org-server/blob/f9da1e68f6d57bc62eef3837f8f1d30ff6b89753/src/ocamlorg/lib/package.ml#L184

[JiaeK]

I have a question regarding univ.st.packages from https://github.com/ocaml-opam/opam2web/blob/d1f4296c62da19713d358d1792c7138102678628/src/o2wPackage.ml#L388 and pkgs from https://github.com/ocaml/v3.ocaml.org-server/blob/f9da1e68f6d57bc62eef3837f8f1d30ff6b89753/src/ocamlorg/lib/package.ml#L135

Are they supposed to point the same thing? I figured univ stands for universe but may I ask what st stands for? Thanks :)

[JiaeK]

EDIT: so we might have to expand the rev_deps type to contain information about the latest matching version so we can link to it in the package overview page.

https://github.com/ocaml/v3.ocaml.org-server/blob/f9da1e68f6d57bc62eef3837f8f1d30ff6b89753/src/ocamlorg/lib/package.ml#L184

@patricoferris And just a genuine question about this - we also don't need something like let get_rev_deps such as this? https://github.com/ocaml/v3.ocaml.org-server/blob/f9da1e68f6d57bc62eef3837f8f1d30ff6b89753/src/ocamlorg/lib/package.ml#L90

[patricoferris]

Hi @JiaeK,

Indeed, univ is meant as universe. I don't have opam2web checked out and building but a quick guess is st stands for switch state see: https://github.com/ocaml-opam/opam2web/blob/d1f4296c62da19713d358d1792c7138102678628/src/o2wTypes.mli#L20

This is pretty low-level, nuts and bolts of opam and opam2web so I don't want to go too far down that track because v3 doesn't use the same approach, but opam2web bases its data about packages and dependencies off of an opam switch (you probably have a local switch in _opam for this repository). We do not base ours off of a switch but instead reading the package information directly from the opam-repository.

we also don't need something like let get_rev_deps such as this?

That's essentially this function

https://github.com/ocaml/v3.ocaml.org-server/blob/f9da1e68f6d57bc62eef3837f8f1d30ff6b89753/src/ocamlorg/lib/package.ml#L114

Reverse dependencies are just the inverse of dependencies. Not "what do I depend on" but "who depends on me". The dependencies can be read from the opam file and with a list of all the packages and their dependencies we can calculate the reverse dependencies.

With this in hand we can use it in the mk_rev_deps function to calculate the exact information we need for example after doing rev_depends we might know package A.1.1.0, A.1.1.1 and A.1.1.2 are all reverse dependencies of B.0.2.0 we then (in mk_rev_deps) convert this to say something like A >= "1.1.0" but what we also need is the upper bound so when a user clicks on the package they get taken to something that is consistent with the version constraint.

[JiaeK]

Wow, thanks so much @patricoferris 🙌 You scratched my itchy spot, and this explanation is very informative. I'm taking a note!

[JiaeK]

EDIT: so we might have to expand the rev_deps type to contain information about the latest matching version so we can link to it in the package overview page.

https://github.com/ocaml/v3.ocaml.org-server/blob/f9da1e68f6d57bc62eef3837f8f1d30ff6b89753/src/ocamlorg/lib/package.ml#L184

Hi @patricoferris, Unfortunately, I'm still quite stuck with how to expand the ;rev_deps - I'm looking at the type t =, of course line 184, and I'm not really sure but from the line 392. If the last thing is right place to look at, can I ask - what's the difference between all_packages_latest t and get_package_latest t name?

[patricoferris]

Hi @JiaeK,

By "expanding rev_deps" you'll have to change the type of the rev_deps field. Currently it records a list of reverse dependencies (their name and an optional string for the version constraint e.g. ">= 2.1.0") but we also want to record the latest version too. If you add that to the type t the type-checker should lead you around the code to the places you will need to update :))

[JiaeK]

Aha! Thank you @patricoferris 🙂 - so fascinating to know how these are connected to each other..!

[JiaeK]

Hi @patricoferris, When I added 'the latest version' recording to the type t and ran the make test it said either (according to the order of the adding) it has a syntax problem on its own line (inside of type t) or this issue: Could you recommend some material to reference the syntax? I read records and tried to find any similar examples but couldn't find anything.

[patricoferris]

Great, this (I think) is what I meant by the type-checker should lead you around the code to the places you will need to update :))

Suppose I have (I'm using pseudo-repl syntax here similar to UTOP, hence the #).

# type t = {
  name : string;
}
# let alice = { name = "Alice" }
val alice : t = {name = "Alice"}

Now I update the type definition to be a pair of strings for first and last name, and try the same thing:

# type t = {
  name : string * string;
}
# let alice = { name = "Alice" }
Error: This expression has type string but an expression was expected of type
         string * string

The type-checker is telling me the value I tried to set name to is the wrong type so I need:

# let alice = { name = ("Alice", "Smith") }
val alice : t = {name = ("Alice", "Smith")}

Note the rev_deps; is shorthand for rev_deps = rev_deps; in the record construction. So-called Field Punning. So the value the variable called rev_deps (this one https://github.com/ocaml/v3.ocaml.org-server/blob/f9da1e68f6d57bc62eef3837f8f1d30ff6b89753/src/ocamlorg/lib/package.ml#L220) is now the wrong type. I can't actually see what you changed it to but I suspect you just need to actually pass the latest value out so the type is correct :)) Hopefully that helps, let me know if not.

[JiaeK]

Thank you very much for the reply, @patricoferris - now I get the gist of the syntax!

Sorry the full shot was this:

&

Note the rev_deps; is shorthand for rev_deps = rev_deps; in the record construction. So-called Field Punning. So the value the variable called rev_deps (this one

https://github.com/ocaml/v3.ocaml.org-server/blob/f9da1e68f6d57bc62eef3837f8f1d30ff6b89753/src/ocamlorg/lib/package.ml#L220 ) is now the wrong type. I can't actually see what you changed it to but I suspect you just need to actually pass the latest value out so the type is correct :))

So if this is the case, we also need to adjust inside of the line#220? Does this mean 'passing the latest value out'?

[patricoferris]

Cool, that looks about right and sorry I linked the wrong rev_deps I mean this one

https://github.com/ocaml/v3.ocaml.org-server/blob/f9da1e68f6d57bc62eef3837f8f1d30ff6b89753/src/ocamlorg/lib/package.ml#L164

So mk_revdeps currently returns a (Name_map.key * string option) list and needs to return (Name_map.key * string option * Version_map.key) list according to your type

[JiaeK]

Thanks for the response @patricoferris! Still struggling with 'passing the latest value out' inside of rev_deps - could I get a little more light on, please? Is this need something similar style to description or maintainers that has ~default, perhaps?

[patricoferris]

I think that's just me phrasing it poorly. I just mean mk_revdeps needs to also return the latest version as well as what it already returns (i.e. your new type for the rev_deps field).

[JiaeK]

Then I must be missing something 🤔.. Going for checking from the beginning.

[JiaeK]

Hi @patricoferris, Sorry for the late update and thanks for your patience. I'm still stuck at pretty much the same place since, but I don't want to give up so I'm wondering what if I open a PR at this moment and find out/learn what I'm missing? Is this ok for you or in general (in Github culture)?

Also, can I ask a question - what is the difference between OpamPackage.Version.Map and OpamPackage.Version.Set ? Thank you!

[patricoferris]

Hi @JiaeK :))

I'm wondering what if I open a PR at this moment

Definitely that's okay and in general it can be easier for (a) someone to understand the holes in someone's understanding and (b) easier for you to point to bits you're unsure of.

Also, can I ask a question - what is the difference between OpamPackage.Version.Map and OpamPackage.Version.Set ? Thank you!

Questions are always welcome. These two modules are based on the standard library ones:

• Map: https://v3.ocaml.org/p/ocaml-base-compiler/4.13.1/doc/Stdlib/Map/module-type-S/index.html
• Set: https://v3.ocaml.org/p/ocaml-base-compiler/4.13.1/doc/Stdlib/Set/module-type-S/index.html

A set is like the "mathematical set", a collection of elements of the same type. It then has the same properties as a set for example elements that are the same (as defined by some comparison function) are treated as the same element:

(* We use a 'functor' to build a new set that can contain strings *)
module StringSet = Set.Make(String)

let () = 
  let s1 = StringSet.add "hello" StringSet.empty in
  let s2 = StringSet.add "hello" s1 in
    print_int (StringSet.cardinal s2) (* prints 1 *)

A map is a data-structure associating some "key" with some "value":

(* We use a 'functor' again to build a new map that uses strings for keys, the values can be anything
    until we use it for the first time *)
module StringMap = Map.Make(String)

let () = 
  let m = StringMap.add "hello" 42 StringMap.empty in 
    print_int (StringMap.find "hello" m) (* prints 42 *)

So OpamPackage.Version.Map is a Map that uses the OpamPackage.Version.t as keys. OpamPackage.Version.Set is a Set with elements of type OpamPackage.Version.t.

We have a few in-need-of-updating tutorials:

• https://v3.ocaml.org/tutorials/map
• https://v3.ocaml.org/tutorials/sets

[JiaeK]

I appreciate your response and thoughtful answer @patricoferris. That really helped me to understand this more but still stuck at the same place so I'll open a PR soon. Thanks :)

[patricoferris]

Thanks to @JiaeK's work this is now resolved for reverse dependencies, now we will just need the same work for dependencies which will likely be harder because of how they are calculated:

• [x] Versioned links for reverse dependencies #183
• [ ] Versioned links for normal dependencies

[JiaeK]

Thank you for the review @patricoferris. Based on what I've learnt from the PR #183, may I try to fix the dependencies' part as well? If it looks too much for me, please say so, I'll also gladly hand it over :)

[patricoferris]

@JiaeK I don't think anyone else is working on it so go right ahead that would be very much appreciated :)) (feel free to ask question on here/on a PR whenever you need to)

[JiaeK]

Thanks @patricoferris 🙂

Here are the steps I thought these need to be fixed, please correct all the spots I'm missing -

1) To redirect the latest version of dependencies, can I add the logic inside of this?: https://github.com/ocaml/v3.ocaml.org-server/blob/bb31c2835db299c89143b28743a2eddc20c22f0e/src/ocamlorg/lib/package.ml#L55

Or you mentioned that the dependencies can be read from the opam file and with a list of all the packages, so I should fix inside of this?: https://github.com/ocaml/v3.ocaml.org-server/blob/bb31c2835db299c89143b28743a2eddc20c22f0e/src/ocamlorg/lib/package.ml#L199

2) Then I assume we need to expand the type of dependencies here: https://github.com/ocaml/v3.ocaml.org-server/blob/bb31c2835db299c89143b28743a2eddc20c22f0e/src/ocamlorg/lib/package.ml#L25

3) Put the latest value in package_template.eml

4) Matching the type in package.mli

[patricoferris]

Sounds good to me. 2-4 all sound right 👍

For the steps to build up the data you might have to change a bit more as you have eluded to.

https://github.com/ocaml/v3.ocaml.org-server/blob/bb31c2835db299c89143b28743a2eddc20c22f0e/src/ocamlorg/lib/package.ml#L50

At the moment the dependencies are more or less just read from the opam file but we'll need to do something similar to rev_deps in order to calculate the maximum element of all the possible versions of a particular dependency. So I think you can use depends to do this.

[JiaeK]

Thanks for the advice @patricoferris

During the adjustment, I got this error message from the compiler which I don't understand:

Because 'the line 263' points from here: https://github.com/ocaml/v3.ocaml.org-server/blob/bb31c2835db299c89143b28743a2eddc20c22f0e/src/ocamlorg/lib/package.ml#L260 and there is no info.Package.Info.dependencies so I don't know where to adjust. Is this the same with pkg.info?

[patricoferris]

Just before line 263 the compiler also let's you know which file (and in certain terminals you should be able to click it) the error is so in this case:

https://github.com/ocaml/v3.ocaml.org-server/blob/bb31c2835db299c89143b28743a2eddc20c22f0e/src/ocamlorg_web/lib/graphql.ml#L263

[JiaeK]

Oh, right, how stupid that question was.. :sweat_smile: So we need to adjust inside of graphql.ml file as well?

[patricoferris]

Yep :))

[JiaeK]

Hi again @patricoferris 🙂 I'm not sure if this is relevant enough but this bugs me quite a while so I'm raising a question : What are the difference between version, versions, version_set ? Is the set of version_set like the meaning of OpamPackage.Version.Set's set, which you explained it before?

A set is like the "mathematical set", a collection of elements of the same type. It then has the same properties as a set for example elements that are the same (as defined by some comparison function) are treated as the same element

[patricoferris]

Thanks for the PR I've added some comments there.

What are the difference between version, versions, version_set ? Is the set of version_set like the meaning of OpamPackage.Version.Set's set, which you explained it before?

In your PR you have used version_set as a variable name but it is unbound i.e. you have never declared it with a value e.g. let version_set = .... I think the confusion has sprung from the fact that there is also OpamTypes.version_set which is the type of the value we need here, does that make sense ?