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:
parent
4e0421bc32
commit
19a3ba19ad
72
doc/FS1843.md
Normal file
72
doc/FS1843.md
Normal 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** @ARGV, not just a superset of the valid ports in @ARGV.
|
||||
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.
|
@ -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() ) {
|
||||
|
@ -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;
|
||||
|
Loading…
Reference in New Issue
Block a user