Paths

Table of Contentst

loader: always install help files
ClosedPublic
Actions

Authored by mhorne on Feb 11 2021, 2:43 PM.

Details

Reviewers

imp
kevans
manu

Group Reviewers

bhyve

Commits

rGac25a113775a: loader: always install help files
rGa2b4abce0e30: loader: always install help files
rG8859960436f5: loader: always install help files

Summary

The existing condition prevents the common help file from being
installed when there are no additional help files defined. Remove this
check and always install the common help file. This benefits platforms
currently missing it: amd64, arm64, and riscv.

Also, install help.fdt with the EFI loader on platforms that include the
fdt command set.

Test Plan

Build and install of stand/ on amd64 includes loader.help.
Build and install of stand/ on riscv includes loader.help with help.fdt.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 48105
Build 44992: arc lint + arc unit

Event Timeline

mhorne created this revision.Feb 11 2021, 2:43 PM

Herald added a subscriber: bcran. · View Herald TranscriptFeb 11 2021, 2:43 PM

mhorne requested review of this revision.Feb 11 2021, 2:43 PM

Harbormaster completed remote builds in B36891: Diff 83690.Feb 11 2021, 2:44 PM

There were some other problems with help, let me go search my inbox...

Ah, ok, here we go: D22951, if you're interested in further molehills to wrangle while you're in the area. :-)

TL;DR being that we only install a single "loader.help", but some platforms have more than one viable loader (e.g., x86 with efiloader and biosloader) -- therefore we'll always lose one of them in the final /boot.

grahamperrin added a subscriber: grahamperrin.Oct 17 2022, 4:26 AM

Herald added a reviewer: manu. · View Herald TranscriptOct 17 2022, 4:26 AM

Herald added a subscriber: dab. · View Herald Transcript

Rework the change. Take inspiration from @sigsys_gmail.com's approach in D22951 and define a different help filename for each consumer of loader.mk.

This does away with the plain loader.help, and instead for an amd64 installation I get the following:

loader.help.bios
loader.help.efi
loader.help.kboot
loader.help.userboot

I'm not tied to the names I've selected here. Notably, different interpreter variations still share the same helpfile.

I believe this approach should adequately address the different issues involved here, albeit with some duplication. Even if it is not perfect, I do think it is important to agree on a solution ahead of the 13.2 and 14.0 release timeframes.

Herald added a reviewer: bhyve. · View Herald TranscriptOct 31 2022, 2:46 PM

Herald added a subscriber: rgrimes. · View Herald Transcript

Harbormaster completed remote builds in B48105: Diff 112439.Oct 31 2022, 2:46 PM

mhorne added inline comments.Oct 31 2022, 2:48 PM

stand/efi/loader/Makefile
71	Removed this hunk from the diff, I'll commit it separately.

Fix an inconsistency with HELP_FILENAME quotation marks.

Also, ping! Would anyone like to weigh in on this change?

Harbormaster completed remote builds in B49173: Diff 115334.Jan 19 2023, 8:09 PM

Last call: I will do some sanity tests, but my plan is to commit this tomorrow. Since this is user facing and has been broken a long time I want to get the fix into 13.2.

I like it in concept. Many of the help.foo files are missing... But that's an orthogonal issue.
Sorry I've not commented sooner... this seems to hit when I'm on my phone and can't reply here.

This revision is now accepted and ready to land.Feb 1 2023, 8:55 PM

In D28591#871049, @imp wrote:

I like it in concept. Many of the help.foo files are missing... But that's an orthogonal issue.
Sorry I've not commented sooner... this seems to hit when I'm on my phone and can't reply here.

That's quite alright. I'm not quite sure what you mean by missing help.foo files... are you saying we should have e.g. a help.userboot file with userboot-specific command descriptions?

In D28591#872316, @mhorne wrote:

In D28591#871049, @imp wrote:

I like it in concept. Many of the help.foo files are missing... But that's an orthogonal issue.
Sorry I've not commented sooner... this seems to hit when I'm on my phone and can't reply here.

That's quite alright. I'm not quite sure what you mean by missing help.foo files... are you saying we should have e.g. a help.userboot file with userboot-specific command descriptions?

Yes. I'm saying the existing help.foo files need some love, and there may be one or two that are missing... To be clear: I like this change as is... It's just one more bit of technical debt that needs to be retired...

In D28591#872317, @imp wrote:

In D28591#872316, @mhorne wrote:

In D28591#871049, @imp wrote:

I like it in concept. Many of the help.foo files are missing... But that's an orthogonal issue.
Sorry I've not commented sooner... this seems to hit when I'm on my phone and can't reply here.

That's quite alright. I'm not quite sure what you mean by missing help.foo files... are you saying we should have e.g. a help.userboot file with userboot-specific command descriptions?

Yes. I'm saying the existing help.foo files need some love, and there may be one or two that are missing... To be clear: I like this change as is... It's just one more bit of technical debt that needs to be retired...

Got you. Even the main help.common is lacking for some more recent commands. I can at least capture this in a PR.

One final note is that this version leaves the old /boot/loader.help intact but unused. I will tweak the change to add that to ObsoleteFiles, unless it seems premature somehow (most recent installs will be missing it entirely).

Closed by commit rG8859960436f5: loader: always install help files (authored by mhorne). · Explain WhyFeb 3 2023, 8:39 PM

This revision was automatically updated to reflect the committed changes.

mhorne added a commit: rG8859960436f5: loader: always install help files.

mhorne added a commit: rGa2b4abce0e30: loader: always install help files.Feb 15 2023, 5:10 PM

mhorne added a commit: rGac25a113775a: loader: always install help files.Feb 23 2023, 6:11 PM