CRUX

Welcome to CRUX bug tracking.
Tasklist

FS#1910 - `prt-get update foo bar` does not check whether foo or bar is installed

Attached to Project: CRUX
Opened by John McQuah (farkuhar) - Monday, 23 May 2022, 19:58 GMT
Task Type Bug Report
Category tools → prt-get
Status New
Assigned To No-one
Operating System CRUX
Severity Medium
Priority Normal
Reported Version 3.6
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

(credit to beerman for discovering this behaviour) When prt-get install (or update) is given an argument list with more than one port, it skips the preliminary check of whether those ports are installed. This useful safeguard is only exercised when exactly one port is given on the command line.

I wrote a patch that reinterprets such a malformed command in the most charitable way: any ports for which install (or update) makes sense are retained; the rest are discarded with a warning. If none of the ports on the command line are compatible with the requested action, then the command fails in the same manner that `prt-get update foo` would fail, when foo is not installed.

TODO: the output would be even nicer if the warnings about ignored (incompatible) arguments could be postponed until the post-transaction summary, rather than getting lost in the scrollback buffer as the usable arguments get processed by pkgmk and pkgadd. But prt-get doesn't provide a straightforward public method for manipulating the contents of m_ignore (populated by argparser.cpp), which would be the most natural place to preserve the information for a post-transaction summary.
   upinst-clean-input.patch (2.6 KiB)
This task depends upon

Comment by Johannes Winkelmann (jw) - Monday, 23 May 2022, 20:58 GMT
I'm too far away to have any string feelings on the patch itself, although I feel it may be more intuitive to simply fail with a warning if you provide a list of ports to update and one isn't previously installed, rather than doing part of the work and failing at the rest.

That said, m_ignore is a direct mapping of a command line argument that lists port the _user_ wants to ignore, and as such should _not_ be used for ports the user explicitly wanted to update but failed at the pkgadd stage. An argument parser is not a natural place to store information about a transaction's success (or lack thereof)
Comment by John McQuah (farkuhar) - Monday, 23 May 2022, 22:09 GMT
I think you and jaeger are right: providing an informative error message and aborting immediately (so that a properly formed command can be issued) is a more intuitive response than carrying out a subset of the tasks requested. You could easily get that behaviour by changing the condition ( compatible_args.size() < 1 ) to ( compatible_args.size() < args.size() ) in my patch.

As for my description of m_ignore as a "natural place" for preserving information, I admit that wasn't the best phrasing. Among the few objects that are accessed during post-transaction reporting, m_ignore stood out as a prime candidate because it gets used in the method that prints the result of an InstallTransaction. Since m_ignore is not meant to be manipulated outside of the argument parser, another object would have to be introduced (and referenced by the evaluateResult method), if we wanted prt-get to have the behaviour of my original patch and still give an informative warning about unused arguments in the post-transaction report.

At the end of the TODO file there's the question "should there be a install/update mixed to mode, to install and update, which ever method is needed?" Implementing this idea would require a heavier refactoring of prtget.cpp than the simple subsetting that my patch performs, but it's another way to address the bug that beerman found.

Loading...