* [PATCH setup 0/3] Improve the handling of packagesource::sites @ 2018-03-17 15:00 Ken Brown 2018-03-17 15:00 ` [PATCH setup 1/3] Make sure that the IniDBBuilderPackage destructor is called when needed Ken Brown ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Ken Brown @ 2018-03-17 15:00 UTC (permalink / raw) To: cygwin-apps If several setup.ini files contain the same version of a package, we currently create several packageversions in the libsolv pool, each of which knows about one site. This patchset consolidates those packageversions into a single one, whose packagesource::sites vector contains all the sites. In the course of working on this, I found a problem with the way the IniDBBuilderPackage destructor is called. The first patch fixes that problem. Ken Brown (3): Make sure that the IniDBBuilderPackage destructor is called when needed Internalize the libsolv repo attribute data after each setup.ini Keep track of all known sites for a given version of a package IniDBBuilderPackage.cc | 13 ++++++++++++- ini.cc | 8 ++++---- package_db.cc | 2 -- package_meta.cc | 8 ++++++++ 4 files changed, 24 insertions(+), 7 deletions(-) -- 2.16.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH setup 1/3] Make sure that the IniDBBuilderPackage destructor is called when needed 2018-03-17 15:00 [PATCH setup 0/3] Improve the handling of packagesource::sites Ken Brown @ 2018-03-17 15:00 ` Ken Brown 2018-06-10 20:58 ` Jon Turney 2018-03-17 15:00 ` [PATCH setup 3/3] Keep track of all known sites for a given version of a package Ken Brown 2018-03-17 15:00 ` [PATCH setup 2/3] Internalize the libsolv repo attribute data after each setup.ini Ken Brown 2 siblings, 1 reply; 9+ messages in thread From: Ken Brown @ 2018-03-17 15:00 UTC (permalink / raw) To: cygwin-apps The IniDBBuilderPackage destructor contains code that is intended to be run after each setup.ini file is processed. But the IniDBBuilderPackage variables in do_local_ini() and do_remote_ini were declared outside the loop that processed the files. Move the declaration inside the loop so that the destructor is called after each iteration. --- ini.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ini.cc b/ini.cc index d807ed6..7afeba2 100644 --- a/ini.cc +++ b/ini.cc @@ -209,13 +209,13 @@ static bool do_local_ini (HWND owner) { bool ini_error = false; - GuiParseFeedback myFeedback; - IniDBBuilderPackage aBuilder (myFeedback); io_stream *ini_file, *ini_sig_file; // iterate over all setup files found in do_from_local_dir for (IniList::const_iterator n = found_ini_list.begin (); n != found_ini_list.end (); ++n) { + GuiParseFeedback myFeedback; + IniDBBuilderPackage aBuilder (myFeedback); bool sig_fail = false; std::string current_ini_ext, current_ini_name, current_ini_sig_name; @@ -268,8 +268,6 @@ static bool do_remote_ini (HWND owner) { bool ini_error = false; - GuiParseFeedback myFeedback; - IniDBBuilderPackage aBuilder (myFeedback); io_stream *ini_file = NULL, *ini_sig_file; /* FIXME: Get rid of this io_stream pointer travesty. The need to @@ -279,6 +277,8 @@ do_remote_ini (HWND owner) for (SiteList::const_iterator n = site_list.begin (); n != site_list.end (); ++n) { + GuiParseFeedback myFeedback; + IniDBBuilderPackage aBuilder (myFeedback); bool sig_fail = false; std::string current_ini_ext, current_ini_name, current_ini_sig_name; // iterate over known extensions for setup -- 2.16.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH setup 1/3] Make sure that the IniDBBuilderPackage destructor is called when needed 2018-03-17 15:00 ` [PATCH setup 1/3] Make sure that the IniDBBuilderPackage destructor is called when needed Ken Brown @ 2018-06-10 20:58 ` Jon Turney 0 siblings, 0 replies; 9+ messages in thread From: Jon Turney @ 2018-06-10 20:58 UTC (permalink / raw) To: cygwin-apps On 17/03/2018 14:59, Ken Brown wrote: > The IniDBBuilderPackage destructor contains code that is intended to > be run after each setup.ini file is processed. But the > IniDBBuilderPackage variables in do_local_ini() and do_remote_ini were > declared outside the loop that processed the files. Move the > declaration inside the loop so that the destructor is called after > each iteration. Applied, thanks. I need to think a bit more about 2 & 3 in this series and the concerns I had in [1] [1 ]https://sourceware.org/ml/cygwin-apps/2018-03/msg00026.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH setup 3/3] Keep track of all known sites for a given version of a package 2018-03-17 15:00 [PATCH setup 0/3] Improve the handling of packagesource::sites Ken Brown 2018-03-17 15:00 ` [PATCH setup 1/3] Make sure that the IniDBBuilderPackage destructor is called when needed Ken Brown @ 2018-03-17 15:00 ` Ken Brown 2018-07-09 18:17 ` Jon Turney 2018-03-17 15:00 ` [PATCH setup 2/3] Internalize the libsolv repo attribute data after each setup.ini Ken Brown 2 siblings, 1 reply; 9+ messages in thread From: Ken Brown @ 2018-03-17 15:00 UTC (permalink / raw) To: cygwin-apps When adding a packageversion for an entry in setup.ini, make its packagesource::sites vector include the sites from previously processed ini files. Also remove from the libsolv pool the previous packageversions, so that libsolv will always find the one that lists all the sites. --- IniDBBuilderPackage.cc | 11 ++++++++++- package_meta.cc | 8 ++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/IniDBBuilderPackage.cc b/IniDBBuilderPackage.cc index 324f1bf..0b84def 100644 --- a/IniDBBuilderPackage.cc +++ b/IniDBBuilderPackage.cc @@ -148,6 +148,11 @@ IniDBBuilderPackage::buildPackageInstall (const std::string& path, // set archive path, size, mirror, hash cbpv.archive.set_canonical(path.c_str()); cbpv.archive.size = atoi(size.c_str()); + // do we already have some sites from previously read ini files? + packagedb db; + packageversion pv = db.findBinaryVersion(PackageSpecification(name, cbpv.version)); + if (pv) + cbpv.archive.sites = pv.source()->sites; cbpv.archive.sites.push_back(site(parse_mirror)); switch (type) { @@ -190,6 +195,11 @@ IniDBBuilderPackage::buildPackageSource (const std::string& path, cspv.archive = packagesource(); cspv.archive.set_canonical(path.c_str()); cspv.archive.size = atoi(size.c_str()); + // do we already have some sites from previously read ini files? + packagedb db; + packageversion pv = db.findBinaryVersion(PackageSpecification(name, cbpv.version)); + if (pv) + cspv.archive.sites = pv.sourcePackage().source()->sites; cspv.archive.sites.push_back(site(parse_mirror)); switch (type) { @@ -210,7 +220,6 @@ IniDBBuilderPackage::buildPackageSource (const std::string& path, break; } - packagedb db; packageversion spkg_id = db.addSource (name + "-src", cspv); /* create relationship between binary and source packageversions */ diff --git a/package_meta.cc b/package_meta.cc index 7f8110d..89b67bc 100644 --- a/package_meta.cc +++ b/package_meta.cc @@ -140,6 +140,14 @@ packagemeta::add_version (packageversion & thepkg, const SolverPool::addPackageD set <packageversion>::iterator i = versions.find(thepkg); if (i != versions.end()) { + if (pkgdata.reponame != "_installed") + { + if (curr == *i) + curr = thepkg; + if (exp == *i) + exp = thepkg; + i->remove(); + } versions.erase(i); } -- 2.16.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH setup 3/3] Keep track of all known sites for a given version of a package 2018-03-17 15:00 ` [PATCH setup 3/3] Keep track of all known sites for a given version of a package Ken Brown @ 2018-07-09 18:17 ` Jon Turney 2018-07-09 18:19 ` [PATCH setup 1/2] Add Vendor() accessor method to SolvableVersion Jon Turney 2018-07-10 16:12 ` [PATCH setup 3/3] " Ken Brown 0 siblings, 2 replies; 9+ messages in thread From: Jon Turney @ 2018-07-09 18:17 UTC (permalink / raw) To: cygwin-apps On 17/03/2018 14:59, Ken Brown wrote: > When adding a packageversion for an entry in setup.ini, make its > packagesource::sites vector include the sites from previously > processed ini files. > > Also remove from the libsolv pool the previous packageversions, so > that libsolv will always find the one that lists all the sites. Thanks. I think that this might be better done in packagemeta::add_version() as you originally proposed, although that does involve moving more things around. A couple of patches which replace this one to do that follow. > diff --git a/package_meta.cc b/package_meta.cc > index 7f8110d..89b67bc 100644 > --- a/package_meta.cc > +++ b/package_meta.cc > @@ -140,6 +140,14 @@ packagemeta::add_version (packageversion & thepkg, const SolverPool::addPackageD > set <packageversion>::iterator i = versions.find(thepkg); > if (i != versions.end()) > { > + if (pkgdata.reponame != "_installed") > + { > + if (curr == *i) > + curr = thepkg; > + if (exp == *i) > + exp = thepkg; > + i->remove(); > + } A comment would have been useful here. It took a bit of head scratching and testing with this removed to understand what this part is doing. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH setup 1/2] Add Vendor() accessor method to SolvableVersion 2018-07-09 18:17 ` Jon Turney @ 2018-07-09 18:19 ` Jon Turney 2018-07-09 18:19 ` [PATCH setup 2/2] Keep track of all known sites for a given version of a package Jon Turney 2018-07-10 16:12 ` [PATCH setup 3/3] " Ken Brown 1 sibling, 1 reply; 9+ messages in thread From: Jon Turney @ 2018-07-09 18:19 UTC (permalink / raw) To: cygwin-apps; +Cc: Jon Turney --- libsolv.cc | 17 +++++++++++++++++ libsolv.h | 1 + 2 files changed, 18 insertions(+) diff --git a/libsolv.cc b/libsolv.cc index 63942b2..955a1b2 100644 --- a/libsolv.cc +++ b/libsolv.cc @@ -209,6 +209,23 @@ SolvableVersion::sourcePackageName () const return std::string(pool_id2str(pool, spkg)); } +const std::string +SolvableVersion::Vendor () const +{ + if (!id) + return ""; + + // extract vendor + Solvable *solvable = pool_id2solvable(pool, id); + Id vendor = repo_lookup_id(solvable->repo, id, SOLVABLE_VENDOR); + + // has no such attribute + if (!vendor) + return ""; + + return std::string(pool_id2str(pool, vendor)); +} + SolvableVersion SolvableVersion::sourcePackage () const { diff --git a/libsolv.h b/libsolv.h index c218ab8..f394e65 100644 --- a/libsolv.h +++ b/libsolv.h @@ -73,6 +73,7 @@ class SolvableVersion SolvableVersion sourcePackage () const; // where this package archive can be obtained from packagesource *source() const; + const std::string Vendor () const; // fixup spkg_id void fixup_spkg_id(SolvableVersion spkg_id) const; -- 2.17.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH setup 2/2] Keep track of all known sites for a given version of a package 2018-07-09 18:19 ` [PATCH setup 1/2] Add Vendor() accessor method to SolvableVersion Jon Turney @ 2018-07-09 18:19 ` Jon Turney 0 siblings, 0 replies; 9+ messages in thread From: Jon Turney @ 2018-07-09 18:19 UTC (permalink / raw) To: cygwin-apps; +Cc: Jon Turney When adding a packageversion, extend it's packagesource::sites vector to include the sites from any similar packageversions previously processed. Also remove those packageversions from the libsolv pool so that libsolv will always find the one that lists all the sites. This is needed for: - Correct handling of local installs where required packages are spread between cache directories for more than one mirror - Correctly falling back to a second site when multiple mirrors are used and a problem occurs connecting to the first mirror tried. --- package_db.cc | 14 +++------ package_meta.cc | 80 ++++++++++++++++++++++++++++++++++++++----------- package_meta.h | 2 +- 3 files changed, 67 insertions(+), 29 deletions(-) diff --git a/package_db.cc b/package_db.cc index 945b339..b74aafd 100644 --- a/package_db.cc +++ b/package_db.cc @@ -245,11 +245,8 @@ packagedb::addBinary (const std::string &pkgname, packages.insert (packagedb::packagecollection::value_type(pkgname, pkg)); } - /* Create the SolvableVersion */ - SolvableVersion sv = solver.addPackage(pkgname, pkgdata); - - /* Register it in packagemeta */ - pkg->add_version (sv, pkgdata); + /* Create the SolvableVersion and register it in packagemeta */ + pkg->add_version (pkgdata); return pkg; } @@ -266,11 +263,8 @@ packagedb::addSource (const std::string &pkgname, sourcePackages.insert (packagedb::packagecollection::value_type(pkgname, pkg)); } - /* Create the SolvableVersion */ - SolvableVersion sv = solver.addPackage(pkgname, pkgdata); - - /* Register it in packagemeta */ - pkg->add_version (sv, pkgdata); + /* Create the SolvableVersion and register it in packagemeta */ + SolvableVersion sv = pkg->add_version (pkgdata); return sv; } diff --git a/package_meta.cc b/package_meta.cc index a651d28..f765baf 100644 --- a/package_meta.cc +++ b/package_meta.cc @@ -123,11 +123,26 @@ packagemeta::~packagemeta() versions.clear (); } -void -packagemeta::add_version (packageversion & thepkg, const SolverPool::addPackageData &pkgdata) +SolvableVersion +packagemeta::add_version (const SolverPool::addPackageData &inpkgdata) { + SolverPool::addPackageData pkgdata = inpkgdata; + + packageversion *v = NULL; + switch (pkgdata.stability) + { + case TRUST_CURR: + v = &(this->curr); + break; + case TRUST_TEST: + v = &(this->exp); + break; + default: + break; + } + /* - If a packageversion for the same version number is already present,allow + If a packageversion for the same version number is already present, allow this version to replace it. There is a problem where multiple repos provide a package. It's never been @@ -137,12 +152,52 @@ packagemeta::add_version (packageversion & thepkg, const SolverPool::addPackageD We rely on this by adding packages from installed.db last. */ - set <packageversion>::iterator i = versions.find(thepkg); - if (i != versions.end()) + for (set <packageversion>::iterator i = versions.begin(); + i != versions.end(); + i++) { + if (i->Canonical_version() != pkgdata.version) + continue; + + if (pkgdata.vendor == i->Vendor()) + { + /* Merge the site-list from any existing packageversion with the same + repository 'release:' label */ + pkgdata.archive.sites.insert(pkgdata.archive.sites.end(), + i->source()->sites.begin(), + i->source()->sites.end()); + + /* Installed packages do not supersede repo packages */ + if (pkgdata.reponame != "_installed") + { + /* Ensure a stability level doesn't point to a version we're about + to remove */ + if (v && (*v == *i)) + *v = packageversion(); + + i->remove(); + } + } + else + { + /* Otherwise... if we had a way to set repo priorities, that could be + used to control which packageversion the solver picks. For the + moment, just warn that you might not be getting what you think you + should... */ + Log (LOG_PLAIN) << "Version " << pkgdata.version << " of package " << + name << " is present in releases labelled " << pkgdata.vendor << + " and " << i->Vendor() << endLog; + } + versions.erase(i); + + break; } + /* Create the SolvableVersion */ + packagedb db; + SolvableVersion thepkg = db.solver.addPackage(name, pkgdata); + /* Add the version */ std::pair<std::set <packageversion>::iterator, bool> result = versions.insert (thepkg); @@ -154,19 +209,6 @@ packagemeta::add_version (packageversion & thepkg, const SolverPool::addPackageD #endif /* Record the highest version at a given stability level */ - packageversion *v = NULL; - switch (pkgdata.stability) - { - case TRUST_CURR: - v = &(this->curr); - break; - case TRUST_TEST: - v = &(this->exp); - break; - default: - break; - } - if (v) { /* Any version is always greater than no version */ @@ -184,6 +226,8 @@ packagemeta::add_version (packageversion & thepkg, const SolverPool::addPackageD *v = thepkg; } } + + return thepkg; } bool diff --git a/package_meta.h b/package_meta.h index 600a163..8db10e2 100644 --- a/package_meta.h +++ b/package_meta.h @@ -43,7 +43,7 @@ public: ~packagemeta (); - void add_version (packageversion &, const SolverPool::addPackageData &); + SolvableVersion add_version (const SolverPool::addPackageData &); void set_installed_version (const std::string &); void addToCategoryBase(); bool hasNoCategories() const; -- 2.17.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH setup 3/3] Keep track of all known sites for a given version of a package 2018-07-09 18:17 ` Jon Turney 2018-07-09 18:19 ` [PATCH setup 1/2] Add Vendor() accessor method to SolvableVersion Jon Turney @ 2018-07-10 16:12 ` Ken Brown 1 sibling, 0 replies; 9+ messages in thread From: Ken Brown @ 2018-07-10 16:12 UTC (permalink / raw) To: cygwin-apps On 7/9/2018 2:17 PM, Jon Turney wrote: > On 17/03/2018 14:59, Ken Brown wrote: >> When adding a packageversion for an entry in setup.ini, make its >> packagesource::sites vector include the sites from previously >> processed ini files. >> >> Also remove from the libsolv pool the previous packageversions, so >> that libsolv will always find the one that lists all the sites. > > Thanks. > > I think that this might be better done in packagemeta::add_version() as > you originally proposed, although that does involve moving more things > around. > > A couple of patches which replace this one to do that follow. I agree that your approach (or my original approach?) is better. I'm not sure why I gave up on it. >> diff --git a/package_meta.cc b/package_meta.cc >> index 7f8110d..89b67bc 100644 >> --- a/package_meta.cc >> +++ b/package_meta.cc >> @@ -140,6 +140,14 @@ packagemeta::add_version (packageversion & >> thepkg, const SolverPool::addPackageD >>    set <packageversion>::iterator i = versions.find(thepkg); >>    if (i != versions.end()) >>      { >> +     if (pkgdata.reponame != "_installed") >> +   { >> +     if (curr == *i) >> +       curr = thepkg; >> +     if (exp == *i) >> +       exp = thepkg; >> +     i->remove(); >> +   } > > A comment would have been useful here. It took a bit of head scratching > and testing with this removed to understand what this part is doing. Sorry about that. I remember that it took me several tries to get it right, and then I didn't think to document it. Ken ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH setup 2/3] Internalize the libsolv repo attribute data after each setup.ini 2018-03-17 15:00 [PATCH setup 0/3] Improve the handling of packagesource::sites Ken Brown 2018-03-17 15:00 ` [PATCH setup 1/3] Make sure that the IniDBBuilderPackage destructor is called when needed Ken Brown 2018-03-17 15:00 ` [PATCH setup 3/3] Keep track of all known sites for a given version of a package Ken Brown @ 2018-03-17 15:00 ` Ken Brown 2 siblings, 0 replies; 9+ messages in thread From: Ken Brown @ 2018-03-17 15:00 UTC (permalink / raw) To: cygwin-apps Call SolverPool::internalize() in the IniDBBuilderPackage destructor. This makes attribute data from all previously processed setup.ini files available when the next setup.ini is being processed. Remove the now unneeded call to SolverPool::internalize() at the beginning of packagedb::read(). --- IniDBBuilderPackage.cc | 2 ++ package_db.cc | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/IniDBBuilderPackage.cc b/IniDBBuilderPackage.cc index d560cb7..324f1bf 100644 --- a/IniDBBuilderPackage.cc +++ b/IniDBBuilderPackage.cc @@ -37,6 +37,8 @@ currentSpec (0), _feedback (aFeedback){} IniDBBuilderPackage::~IniDBBuilderPackage() { process(); + packagedb db; + db.solver.internalize(); } void diff --git a/package_db.cc b/package_db.cc index 072b419..d12e841 100644 --- a/package_db.cc +++ b/package_db.cc @@ -71,8 +71,6 @@ packagedb::read () { if (!installeddbread) { - solver.internalize(); - /* Read in the local installation database. */ io_stream *db = 0; db = io_stream::open ("cygfile:///etc/setup/installed.db", "rt", 0); -- 2.16.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-07-10 16:12 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-17 15:00 [PATCH setup 0/3] Improve the handling of packagesource::sites Ken Brown 2018-03-17 15:00 ` [PATCH setup 1/3] Make sure that the IniDBBuilderPackage destructor is called when needed Ken Brown 2018-06-10 20:58 ` Jon Turney 2018-03-17 15:00 ` [PATCH setup 3/3] Keep track of all known sites for a given version of a package Ken Brown 2018-07-09 18:17 ` Jon Turney 2018-07-09 18:19 ` [PATCH setup 1/2] Add Vendor() accessor method to SolvableVersion Jon Turney 2018-07-09 18:19 ` [PATCH setup 2/2] Keep track of all known sites for a given version of a package Jon Turney 2018-07-10 16:12 ` [PATCH setup 3/3] " Ken Brown 2018-03-17 15:00 ` [PATCH setup 2/3] Internalize the libsolv repo attribute data after each setup.ini Ken Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).