helsing-ai / buffrs

Modern protobuf package management
https://crates.io/crates/buffrs
Apache License 2.0
213 stars 12 forks source link

Big change #221

Closed khanism closed 8 months ago

khanism commented 8 months ago

Enforcing configured registry URL (see https://github.com/helsing-ai/buffrs/issues/102)

mara-schulke commented 8 months ago

Hi, thank you for you contribution!

Did you encounter a situation where this is causing trouble?

I think in the current state of the manifest (e.g. the manifest contains an absolute uri on where to retrieve the buffrs package from) it is dangerous to set the uri to point to self.registry

I think as @asmello pointed out it would be valuable to not implement the registry as a struct containing a url but rather always use the URI from the manifest.

My reasoning behind this:

Imagine two dependencies in two distinct registries which are both proprietary:

If someone wants to install api a now in his project he would declare a dependency on it:

[dependencies]
a = { registry = "https://company-a.jfrog.io/artifactory", version = "1.0.0", repository = "test" }

When buffrs install is being run this would look the following (written in pseudo code):

a = download("https://company-a.jfrog.io/artifactory/test/a-1.0.0.tgz")
manifest_a = unpack(a)

for dependency in manifest_a:
    dep = download("{dependency.registry}/{dependency.repository}-{dependency.version}.tgz")
    dep_manifest = unpack(dep)
    // infinitely traverse the tree

This algorithm makes a call to https://company-b.jfrog.io/artifactory/test/b-1.0.0.tgz to resolve b which is correct.

Your change (as far as I understand) would change the above algorithm to the following:

a = download("https://company-a.jfrog.io/artifactory/test/a-1.0.0.tgz")
manifest_a = unpack(a)

for dependency in manifest_a:
    dep = download("https://company-a.jfrog.io/artifactory/{dependency.repository}-{dependency.version}.tgz")
    dep_manifest = unpack(dep)
    // infinitely traverse the tree

Which would try to resolve the library b in the registry of company-a. This seems incorrect to me.

khanism commented 8 months ago

Hy Mara,

thanks for taking the time to have a look on this. Yes of course, you're right on this. I certainly can imagine a situation (and actually experienced something similar) where one wants to to enforce only downloads from a certain registry, or have something like a priority list (e.g. "first try registry a and then try b" etc.). In this case I just seem to have misinterpret the description of the https://github.com/helsing-ai/buffrs/issues/102, looking for something to start contributing...

Will close this PR

mara-schulke commented 8 months ago

Thank you a lot @khanism! If you need anything, let me know 😊