package_repo requires a url in repo-info file which needs eval

[kallisti5]

package_repo requires a url in the repo.info file. (the repo.info file is read in by package_repo tool to build the binary on-disk repo which is then served by http to our package manager)

This really doesn't make sense. You're always going to know the repo's base url... so why would you need to specify it while building the repo?

This makes repo creation a lot more complex than it needs to be... I also haven't been able to find anywhere the repo.info url is actually used. (don't get a packages's URL mixed up the internal repo url)

Example:

URL: ​http://packages.haiku-os.org/haikuports/master/repo/x86_64/c623cf2b4d85bc452d5a21a2586e552fe1b04f6d66eca0663f257461c106db9f/repo.info

name HaikuPorts
vendor "Haiku Project"
summary "The HaikuPorts repository (for Haiku hrev50482)"
priority 1
url http://packages.haiku-os.org/haikuports/master/repo/x86_64/c623cf2b4d85bc452d5a21a2586e552fe1b04f6d66eca0663f257461c106db9f
architecture x86_64

[kallisti5]

kallisti5@ares package :) $ grep -R url * | grep -v Package
RepositoryConfig.cpp:		<< "url=" << fBaseURL << "\n"
RepositoryConfig.cpp:	const char* url = driverSettings.GetParameterValue("url");
RepositoryConfig.cpp:	if (url == NULL || *url == '\0')
RepositoryConfig.cpp:	fBaseURL = url;
RepositoryInfo.cpp:const char* const BRepositoryInfo::kURLField			= "url";
RepositoryInfo.cpp:BRepositoryInfo::SetOriginalBaseURL(const BString& url)
RepositoryInfo.cpp:	fOriginalBaseURL = url;
RepositoryInfo.cpp:	const char* url = get_driver_parameter(settingsHandle, "url", NULL, NULL);
RepositoryInfo.cpp:	if (name == NULL || *name == '\0' || url == NULL || *url == '\0'
RepositoryInfo.cpp:	fOriginalBaseURL = url;

So BRepositoryConfig's fBaseURL, PackagesURL(), and BaseURL() use it...

kallisti5@ares package :( $ egrep -R "PackagesURL\(|BaseURL\(" *
ActivateRepositoryConfigJob.cpp:	fRepositoryBaseURL(repositoryBaseURL),
ActivateRepositoryConfigJob.cpp:	repoConfig.SetBaseURL(fRepositoryBaseURL);
AddRepositoryRequest.cpp:	fRepositoryBaseURL(repositoryBaseURL),
manager/PackageManager.cpp:			BString url = remoteRepository->Config().PackagesURL();
RefreshRepositoryRequest.cpp:		= BString(fRepoConfig.BaseURL()) << "/" << "repo.sha256";
RefreshRepositoryRequest.cpp:		BString("Fetching repository checksum from ") << fRepoConfig.BaseURL(),
RefreshRepositoryRequest.cpp:	BString repoCacheURL = BString(fRepoConfig.BaseURL()) << "/" << "repo";
RefreshRepositoryRequest.cpp:		BString("Fetching repository-cache from ") << fRepoConfig.BaseURL(),
RepositoryConfig.cpp:	fBaseURL(baseURL),
RepositoryConfig.cpp:BRepositoryConfig::BaseURL() const
RepositoryConfig.cpp:BRepositoryConfig::PackagesURL() const
RepositoryConfig.cpp:BRepositoryConfig::SetBaseURL(const BString& baseURL)
RepositoryInfo.cpp:BRepositoryInfo::OriginalBaseURL() const
RepositoryInfo.cpp:BRepositoryInfo::SetOriginalBaseURL(const BString& url)

Since the user provides the repo URL to gain access to the repo, it all seems extremely illogical to then re-read the repo config file and potentially re-adjust where the repo is?

[kallisti5]

It seems like BRepositoryConfig should be constructed with the repo URL from the URL the user or system already has vs needing a second copy from the repo itself.

[kallisti5]

So I did some digging into this... definitely an issue.

1. repo.info is used to generate repo file (binary BRepositoryInfo)
2. url param is required in this file (essentially requiring repo to know where it will live before the repo file is generated) Repo's can't be "moved" to new URL's
   1. ​http://cgit.haiku-os.org/haiku/tree/src/kits/package/RepositoryConfig.cpp#n107
   2. ​http://cgit.haiku-os.org/haiku/tree/src/kits/package/RepositoryInfo.cpp#n260
3. BRepositoryInfo (repo) file is pulled into /boot/system/cache/repository_info and used as the "inventory of repositories" for pkgman. During this pull process ActivateRepositoryConfigJob overwrites url with where the repo "really exists"

So:

• url is required in repo.info for the remote repository when it is constructed
• url is used at various places during the repo addition to pkgman
• ActivateRepositoryConfigJob "rewrites" url to be where the repo was actually pulled from.
  • This is the key, it seems to "ignore" all the url work we've done thus far.
  • ​http://cgit.haiku-os.org/haiku/tree/src/kits/package/ActivateRepositoryConfigJob.cpp#n61
• repo binary descriptor is stored in /boot/system/cache/repository_info

It looks like the strict "url needs to be set" requirement in BRepositoryInfo could be lifted...

• url requirement omitted from repo.info
• proper checks for url put in place based on activity

I've been playing with a repo that omits the url field... however haven't gotten the correct process outlined yet and actually using this repo still fails. ​http://unixzen.com/testrepo, ​http://unixzen.com/testrepo/repo.info

[kallisti5]

Since changing this behavior *after* a release is tricky (if repos no longer contain url, older pkgman's won't work with them) I'm bumping this one to a bug for R1B1

[pulkomandy]

From the commit log (014eed80e289c2bbc187ac91bb7b3a35560a971f):

      The URL in the repository config is usually the same as the in the
      repository info, unless it refers to a mirror site. This allows for
      mirrors to copy the original repository verbatim.

This would allow pkgman add-repo to detect that the repo you are adding is a mirror of an already added one (and possibly balance downloads between the two, for example).

[kallisti5]

ok, that's a good point on the mirror / adding the same repo twice... then again we could assign a random uuid to each repo and use that to track adding the same repo twice from two different locations :-)

[kallisti5]

As for pkgman choosing a mirror, we could add a new optional plain text 'mirrors' file and have pkgman attempt to contact each one of the chosen location is slow. (it could then compare the UUID's to make sure the specified mirror is actually the same repo)

[kallisti5]

*and* each repo file could include a "last updated" timestamp to ensure mirrors are recent within x days.

[korli]

I suppose this is addressed in hrev50723.

[kallisti5]

Not completely, there are some XXX lines that need erased after everyone has some time to upgrade their existing nightly machines.

[waddlesplash]

I think the URL mirroring mechanism is OK for now at least, so can we bump this out of B1?

[pulkomandy]

I would say we can close this as "no change needed". The thing works as designed, and the URI does not even need to be an ​http:// one, so if one wanted to use something else, they could put in an uuid: or similar (we could check that this works, just to be sure).