Opened 3 years ago

Closed 2 years ago

#12905 closed bug (no change required)

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

Reported by: kallisti5 Owned by: nobody
Priority: normal Milestone: R1/beta1
Component: Kits/Package Kit Version: R1/Development
Keywords: package_repo Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description (last modified by kallisti5)

package_repo requires a url in the file. (the 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 url is actually used. (don't get a packages's URL mixed up the internal repo url)



name HaikuPorts
vendor "Haiku Project"
summary "The HaikuPorts repository (for Haiku hrev50482)"
priority 1
architecture x86_64

Change History (16)

comment:1 by kallisti5, 3 years ago

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?

comment:2 by kallisti5, 3 years ago

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.

comment:3 by kallisti5, 3 years ago

Description: modified (diff)

comment:4 by kallisti5, 3 years ago

Description: modified (diff)

comment:5 by kallisti5, 3 years ago

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

  1. 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
  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"


  • url is required in 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.
  • 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
  • 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.,

comment:6 by kallisti5, 3 years ago

Component: - GeneralKits/Package Kit
Milestone: UnscheduledR1/beta1
Owner: changed from nobody to bonefish
Type: enhancementbug

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

comment:7 by pulkomandy, 3 years ago

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).

comment:8 by kallisti5, 3 years ago

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 :-)

comment:9 by kallisti5, 3 years ago

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)

comment:10 by kallisti5, 3 years ago

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

comment:11 by bonefish, 3 years ago

Owner: changed from bonefish to nobody
Status: newassigned

comment:12 by korli, 3 years ago

I suppose this is addressed in hrev50723, or does this hrev address #12917?

Last edited 3 years ago by korli (previous) (diff)

comment:13 by kallisti5, 3 years ago

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

comment:14 by waddlesplash, 2 years ago

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

comment:15 by pulkomandy, 2 years ago

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).

comment:16 by waddlesplash, 2 years ago

Resolution: no change required
Status: assignedclosed
Note: See TracTickets for help on using tickets.