Vendor variable in lockfile

[Leonidas-from-XIV]

This is a prerequisite PR to the hybrid mode, it sets the vendor variable on all packages that we intend to vendor.

I tried running it on opam-monorepo and while there is a denylist of packages in Config.base_packages it seems to still want to pull packages like ocaml-config. I wonder if this is correct or whether we'd need to add more exceptions to it (like when the package version is "base" for example).

[Leonidas-from-XIV]

@NathanReb The fact that I needed to adjust the parsing part unfortunately means that to accept the new lock files it has to include this code, so it is incompatible with older opam-monorepo versions. Which is a bit sad but given we're going for 0.3.0 anyway it shouldn't be much of an issue.

[NathanReb]

This looks good to me but while trying this out I think our list of base packages is out of date.

I'll open a PR right now with an update so we can rebase this and testing it again!

The code looks good! In the future it might be worth to do the split between opam and duniverse packages elsewhere but I think for now this works well!

[NathanReb]

I took a look at this and I think it's not only about updating the base packages list. This list is complemented by a check that packages are not "virtual" in other places in the code. Virtual packages are packages that have no url.src field, i.e. packages only used for their metadata.

I think we probably want the same filtering here. This is more easily done on Opam.Package_summary.t values though so I'd suggest that Lockfile.Depends.t becomes a record that also embeds the vendor status. What do you think?

[Leonidas-from-XIV]

That's a good point, in the hybrid mode PR I had already converted Depends.t to a record, since I needed to track the vendor information on a per-package base. While going through this I also thought it would be good to convert from string/string to Name.t and Version.t respectively, to prevent accidental swaps (since versions and names are often handled in code in similar places) and make the signatures a bit more understandable.

[NathanReb]

I think the new Depends type make things quite a bit more confusing than they need to be for now.

The previous state of the PR was very clean and straightforward. Do you think you could simplify it as suggested in the comment?
That is keep the previous data type and flow, add for each item in the Depends.t a boolean to determine whether it's vendored or not. The types are a merely a representation of the lockfile content and not meant to be further interpreted. If more is needed, we should use to_duniverse which is a much better structured representation of the vendored dependencies.

The change to use OpamPackage types is welcome but keeping it in a separate PR next time would be best, it really eases the review. A lot of changes here are now unrelated to this feature.

[NathanReb]

I think this should rather be a list of records, each record being a package and a vendor boolean.

This quite complexified the code compared to what it used to be.

Also note that OpamPackage.Name.t * OpamPackage,Version.t is OpamPackage.t

[Leonidas-from-XIV]

You're right, a better representation of vendor removes a lot of complicated logic.

The Name.t * Version.t thing is a good point, I thought there was more to that datatype. I replaced that by using OpamPackage.t directly, at which point the Opam.t type became just an alias for OpamPackage.t with only identity-functions and functions forwarded to OpamPackage.* with the only code being pp (unused) and raw_pp (moved to Opam.Pp).

[NathanReb]

Looks good, thanks!