use a common style to show missing or already-installed ports (FS#1843)

be more selective in applying the forceRebuild flag
This commit is contained in:
John McQuah 2023-09-14 17:56:52 -04:00
parent 4e0421bc32
commit dc4b361426
3 changed files with 105 additions and 22 deletions

72
doc/FS1843.md Normal file
View File

@ -0,0 +1,72 @@
# Flyspray 1843 notes
## Was: Re: Please make the output of prt-get --test more common
teodor observed that prt-get --test gives two different styles of output.
The output style was found to be affected by the number of packages requested
on the command line (one versus many), and by whether dependency resolution
was toggled (install versus depinst).
In the case when one package was requested for installation, an early check
of /var/lib/pkg/db for the existence of a previous installation would alert the
user to the malformed command. No such check of /var/lib/pkg/db was
being performed for multiple packages passed on the command line (FS#1910), and
so the program continued all the way through executeTransaction() and
evaluateResult(), which produced the more verbose output that teodor noted.
A similar discrepancy could be observed when requesting an **update** of **one**
package that was not previously installed, versus an update of **multiple**
packages among which there are some not yet installed. You would have seen a
terse warning in the first case, and a more verbose output in the second case.
The softdeps branch in this code base is now more consistent in performing the
scan of /var/lib/pkg/db, no matter how many packages are passed as argument.
But the mixed-upinst branch collapses the distinction between install and
update, always bringing up to date all the ports passed as argument (and their
dependencies, unless --nodeps is given), so it can postpone the scan of
/var/lib/pkg/db until the step of customizing pkgadd flags for each target.
Ports exhibiting no version differences between the repos and /var/lib/pkg/db
would be left alone, unless forceRebuild was requested.
teodor's second observation is more interesting. During dependency resolution,
the invalid arguments (ports not found in the active repos) were silently being
dropped, rather than being kept in memory for a post-transaction report. If
**all** ports passed to a ``depinst`` command are nowhere among the active
repositories, then the empty set is what gets used for executeTransaction(),
with the predictable result "no package specified for install". Why did we see
a different result when running ``install``? Because the calcDependencies()
method was never called, and so executeTransaction() received the entire list
of nowhere-to-be-found ports! Iterating through this list of nonexistent ports
would populate the missingPackages list, which would then be displayed nicely
during the post-transaction summary.
Although we do populate a missingPackages list during calcDependencies(), the
contents of this list were not being included in the post-transaction
summary. As the code was originally conceived, executeTransaction() would get
a filtered list, with missing packages omitted. This side-effect of ``depinst``
violates the principle of least surprise, because users like teodor might
justifiably expect toggling dependency resolution to generate a superset of
**their argument list** $@, not just a superset of the valid ports in $@. To be
more faithful to the principle of least surprise, the softdeps branch now
propagates into executeTransaction() even the ports that are not in the repos,
so that the post-transaction summary can display them.
As for teodor's request to unify into one list the post-transaction summary
of missing ports and already-up-to-date ports, I think the possible benefits
for easier parsing are overstated. Some users might not want the two lists
mixed together, for instance when the dependency tree of a valid target ends
up drowning out all the ports that are not in the repos. Two separate lists
would offer greater readability in most cases.
teodor's proposed heading for a united list, "The following ports were
not found/already installed:", suggests a new way for the softdeps branch to
handle FS#1910. All arguments to ``install`` and ``depinst`` will be tested
against /var/lib/pkg/db to ensure that they're not already installed, and if
all the tests fail then the user is immediately alerted to the malformed
command. This preliminary check is simply jw's original design, but extended
from one port to many. Then, if any of the requested ports is not yet
installed, we can go ahead with initRepo() and attempt a less ambitious
transaction (limited to a subset of the user's argument list). The program
will reach evaluateTransaction() and display what succeeded, as well as what
was wrong with the original command. This approach will give users at
least partial results from a malformed command, while also providing an
informative message so that a revised command can be issued later.

View File

@ -252,7 +252,7 @@ InstallTransaction::installPackage( const Package* package,
commandName = "prt-cache";
}
// - initial information about the package to be build
// - initial information about the package to be built
string message;
message = commandName + ": ";
if (update) {
@ -341,6 +341,12 @@ InstallTransaction::installPackage( const Package* package,
}
string args = "-d " + parser->pkgmkArgs();
// do not force a rebuild unless the port was given on the command line
if ( parser->pkgmkArgs().find(" -f")==string::npos &&
find(parser->otherArgs().begin(), parser->otherArgs().end(),
package->name()) == parser->otherArgs().end() ) {
StringHelper::replaceAll(args," -f", "");
}
Process makeProc( cmd, args, fdlog );
if ( makeProc.executeShell() ) {
result = PKGMK_FAILURE;
@ -667,7 +673,7 @@ InstallTransaction::calcDependencies( )
}
}
if ( !validPackages ) {
return PACKAGE_NOT_FOUND;
return NO_PACKAGE_GIVEN;
}
if ( !calculateDependencies() ) {

View File

@ -614,23 +614,29 @@ void PrtGet::install( bool update, bool dependencies )
}
} else {
for ( ; it != args.end(); ++it ) {
string s = *it;
string s = *it;
if ( m_pkgDB->isInstalled( s ) ) {
// pkgadd will fail on these, since it won't be given the -u flag
invalidArgs.push_back( s );
// pkgadd will fail on these, since it won't be given the -u flag
invalidArgs.push_back( s );
}
}
}
}
if ( invalidArgs.size() > 0 ) {
string attemptedOp = ( update ) ? "update" : "install";
attemptedOp += " the following packages ";
attemptedOp += ( update ) ? "(not yet installed)" : "(already installed)";
cout << "cannot "<< attemptedOp <<endl;
if ( (invalidArgs.size() > 0 && update) ||
(invalidArgs.size() == args.size() && !update) ) {
string invalidHeader = "-- Packages installed before this run (ignored)";
if (update) { invalidHeader = "-- Cannot update the following (not yet installed)"; }
if (m_parser->isTest()) {
cout << "*** prt-get: test mode" << endl << endl;
}
cout << invalidHeader << endl;
list<string>::const_iterator it = invalidArgs.begin();
for ( ; it != invalidArgs.end(); ++it ) {
cout << *it << endl;
}
if (m_parser->isTest()) {
cout << "*** prt-get: test mode end" << endl;
}
m_returnValue = PG_GENERAL_ERROR ;
return;
}
@ -649,18 +655,17 @@ void PrtGet::install( bool update, bool dependencies )
cerr << "prt-get: cyclic dependencies found" << endl;
m_returnValue = PG_GENERAL_ERROR;
return;
} else if ( result == InstallTransaction::PACKAGE_NOT_FOUND ) {
warnPackageNotFound(depTransaction);
m_returnValue = PG_GENERAL_ERROR;
return;
}
const list<string>& depRef = depTransaction.dependencies();
list<string>::const_iterator it = depRef.begin();
list<string> deps = depTransaction.dependencies();
const list< pair<string, string> >& depMissing = depTransaction.missing();
list< pair<string,string> >::const_iterator it = depMissing.begin();
list<string> deps;
for (; it != depRef.end(); ++it) {
if (!m_pkgDB->isInstalled(*it)) {
deps.push_back(*it);
// ensure that missing ports are retained for the post-transaction
// summary (FS#1843). Exempt any ports that were installed manually
// or from a repo that has since been deactivated.
for (; it != depMissing.end(); ++it) {
if (! m_pkgDB->isInstalled(it->first)) {
deps.push_back(it->first);
}
}
@ -1177,7 +1182,7 @@ void PrtGet::evaluateResult( InstallTransaction& transaction,
cout << endl;
if ( errors == 0 && !interrupted ) {
if ( inst.size() && errors == 0 && !interrupted ) {
cout << "prt-get: " << command[1] << " successfully" << endl;
} else {
m_returnValue = PG_PARTIAL_INSTALL_ERROR;