Page MenuHomeFreeBSD

freebsd-update: Add `updatesready` and `showconfig` commands.
ClosedPublic

Authored by grembo on Aug 30 2019, 2:21 AM.

Details

Summary

This originally contained a change to fix src component detection. This was moved to a separate review now (D21579) to speed up review of the bugfix and allow a separate commit that could be MFCed.

While debugging the issue mentioned above, I added a showconfig command which I found useful enough to leave it in and document it in the man page.

Like discussed somewhere else, I also added an updatesready command that can be used to check if there are any pending fetched updates that can be installed. It exits on status code 2 in case there aren't any pending updates so that if one can tell apart exit 1 (=error) from it (right now there is no code path for that, but if more checks are added in the future this could be useful).

Finally, I changed the exit code of freebsd-update install in case there are no updates pending to be installed and there wasn't a fetch phase to 2 - this should allow future scripts to differentiate between "error" and "nothing to be done" without breaking existing jail managers like ezjail and iocell.

Besides documentation changed, I also added a comma in the manpage to pacify igor.

Happy to accept naming changes (like, if the reviewers happen to not like updatesready and showconfig). Feel free to correct my English.

See also:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=239997
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=240177

Test Plan

Manual testing, mandoc -T lint, igor

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

grembo created this revision.Aug 30 2019, 2:21 AM
grembo edited the summary of this revision. (Show Details)Aug 30 2019, 2:33 AM
grembo added a subscriber: delphij.
yuripv added a subscriber: yuripv.Aug 30 2019, 2:37 AM
yuripv added inline comments.
usr.sbin/freebsd-update/freebsd-update.sh
65 ↗(On Diff #61477)

nit: other commands don't have trailing dot (same for config) except for IDS, doesn't look consistent

3351 ↗(On Diff #61477)

nit: closing single quote seems to be farther that needed, '$0 install'

grembo updated this revision to Diff 61478.Aug 30 2019, 2:43 AM

Fix style issues.

grembo marked 2 inline comments as done.Aug 30 2019, 2:45 AM
grembo added inline comments.
usr.sbin/freebsd-update/freebsd-update.sh
65 ↗(On Diff #61477)

IDS had, which was the one I used as a template as it was last in the list, before moving "updatesready" around where I saw it fit. Fixed and also removed it from IDS. thx

3351 ↗(On Diff #61477)

thx

bcr accepted this revision as: manpages.Aug 30 2019, 7:07 AM
bcr added a subscriber: bcr.

Manpage looks good to me.

grembo retitled this revision from Fix src component detection, add `updatesready` and `config` commands. to freebsd-update: Fix src component detection, add `updatesready` and `config` commands..Aug 31 2019, 12:23 PM
grembo edited reviewers, added: emaste; removed: cperciva.Sep 2 2019, 8:23 PM
grembo marked 2 inline comments as done.
grembo added a subscriber: cperciva.
jilles added a subscriber: jilles.Sep 2 2019, 10:07 PM
jilles added inline comments.
usr.sbin/freebsd-update/freebsd-update.8
158–163 ↗(On Diff #61478)

The usual convention if there is both a "false" status and an error status is that "false" is 1 and errors are 2 or higher. (If there is no "false", errors are often just 1.) This seems to switch that around.

grembo added inline comments.Sep 2 2019, 10:13 PM
usr.sbin/freebsd-update/freebsd-update.8
158–163 ↗(On Diff #61478)

Based on that observation, which exit codes would you suggest?

grembo added inline comments.Sep 2 2019, 10:19 PM
usr.sbin/freebsd-update/freebsd-update.8
158–163 ↗(On Diff #61478)

Like, change all occurrences of "exit 1" to "exit 2" and make those two "exit 1" instead?

grembo updated this revision to Diff 61864.Sep 9 2019, 10:45 PM

Remove "Fix src component detection" part from this review, as it moved to a separate one (D21579)

grembo retitled this revision from freebsd-update: Fix src component detection, add `updatesready` and `config` commands. to freebsd-update: Add `updatesready` and `config` commands..Sep 9 2019, 10:50 PM
grembo edited the summary of this revision. (Show Details)

I'm happy with the config part so far, but probably use showconfig.
I haven't yet reviewed the updatesready.

usr.sbin/freebsd-update/freebsd-update.8
98 ↗(On Diff #61864)

Committed this bit in rS352513

usr.sbin/freebsd-update/freebsd-update.sh
61–65 ↗(On Diff #61864)

Not completely sure about the order of these, but not a new problem in your patch.

68 ↗(On Diff #61864)

Committed this part in rS352514

grembo marked an inline comment as done.Sep 19 2019, 12:33 PM

I'm happy with the config part so far, but probably use showconfig.
I haven't yet reviewed the updatesready.

That was actually what I called it at first (showconfig or dumpconfig), but then I realized that commands provided by freebsd-update prefer brevity over clarity (e.g. cron actually fetches updates from cron, upgrade fetches upgrades, but doesn't install them, IDS has no verb at all). That's why I went with config instead of showconfig. I'm open to changes of course, if you feel the added clarity of showconfig makes more sense.

usr.sbin/freebsd-update/freebsd-update.sh
61–65 ↗(On Diff #61864)

I think they're ordered based on when they're executed - first fetch updates (by using fetch/cron for simple stuff and upgrade for major releases), then check if there is anything to install (updatesready), then install them. If something went wrong, do a rollback. Run IDS regularily and config for debugging.

grembo updated this revision to Diff 62326.Sep 19 2019, 9:28 PM
grembo marked an inline comment as done.

Rebase to rS352542, rename config' to showconfig', update man page date.

grembo marked an inline comment as done.Sep 19 2019, 9:29 PM
grembo marked an inline comment as done.
grembo added inline comments.Sep 19 2019, 9:33 PM
usr.sbin/freebsd-update/freebsd-update.sh
61–65 ↗(On Diff #61864)

Based on my own reasoning one could argue that `showconfig' should be first in the list, as you might want to check what's configured before doing anything else. Then again, as it's *last*, it's really easy to spot on help. So in the end this probably doesn't really matter that much.

That was actually what I called it at first (showconfig or dumpconfig), but then I realized that commands provided by freebsd-update prefer brevity over clarity (e.g. cron actually fetches updates from cron, upgrade fetches upgrades, but doesn't install them, IDS has no verb at all).

Ah, fair enough. Was going to say I'm fine with leaving at as config then, but since you've already changed it let's go with showconfig.

emaste accepted this revision.Tue, Sep 24, 7:08 PM

Fine with me.

This revision is now accepted and ready to land.Tue, Sep 24, 7:08 PM
grembo retitled this revision from freebsd-update: Add `updatesready` and `config` commands. to freebsd-update: Add `updatesready` and `showconfig` commands..Tue, Sep 24, 8:23 PM
grembo edited the summary of this revision. (Show Details)

That was actually what I called it at first (showconfig or dumpconfig), but then I realized that commands provided by freebsd-update prefer brevity over clarity (e.g. cron actually fetches updates from cron, upgrade fetches upgrades, but doesn't install them, IDS has no verb at all).

Ah, fair enough. Was going to say I'm fine with leaving at as config then, but since you've already changed it let's go with showconfig.

Yeah, I also realized that updatesready is actually quite verbose too, so showconfig won't stand out.

Thanks for the review!

This revision was automatically updated to reflect the committed changes.