git: 2c26d77d989a - main - Remove /boot/efi from mtree, missed in 0b7472b3d8d2.

[Warner Losh]

On Wed, Mar 3, 2021 at 7:09 AM Nathan Whitehorn <nwhitehorn at freebsd.org>
wrote:

>
>
> On 3/2/21 10:19 PM, Warner Losh wrote:
> >
> >
> > On Tue, Mar 2, 2021, 7:01 PM Ed Maste <emaste at freebsd.org
> > <mailto:emaste at freebsd.org>> wrote:
> >
> >     On Tue, 2 Mar 2021 at 19:30, Warner Losh <imp at bsdimp.com
> >     <mailto:imp at bsdimp.com>> wrote:
> >     >
> >     > There has been some talk of moving the mount point to /efi, but
> >     I think that went nowhere...
> >
> >     I thought about /efi based on arguments from the Linux world, more
> >     info at
> >
> https://wiki.archlinux.org/index.php/EFI_system_partition#Typical_mount_points
> >     <
> https://wiki.archlinux.org/index.php/EFI_system_partition#Typical_mount_points
> >.
> >     I did open https://reviews.freebsd.org/D28814
> >     <https://reviews.freebsd.org/D28814> to move the existing
> >     uses to /efi. That said, I'm much more interested in us picking
> >     something and using it in the installer/other tooling than whether
> >     it's /boot/efi or /efi.
> >
> >
> > Me too. I am actually open to either. If I had a do over, I'd pick
> > /efi honestly.
> >
> > But the directory presence or absence shouldn't be used to know if we
> > have an ESP to deal with or not.
> >
> > Warner
> >
>
> It would have been nice to know that when I posted the original patch
> for review that relied on this mechanism two weeks ago and you and Ed
> both signed off. Or when we discussed this on Friday and no one knew why
> it was in mtree -- I still honestly don't know from this thread -- and
> then I proposed removing it and everyone said that was fine. There are
> lots of other directories that we make in the installer that aren't in
> mtree. What's special about this one?
>

You could have asked me directly rather than rely on an IRC conversation I
wasn't part of. You didn't post the review widely enough, nor did you wait
for my comments or seek them out despite knowing I was deeply involved in
this.

To get maximal reuse out of things, we need it in mtree. It's part of the
base system, part of the updates, etc. Arbitrarily removing it without even
asking me for input is what I'm upset about.

The problem here is that the installer needs to know if an ESP has been
> made. If /boot/efi is supposed to be the mountpoint for an ESP, its
> presence seemed like a reasonable way to signal that. Here are some
> options:
> - We could try to see if a partition is actually mounted there. That's
> mildly annoying -- it involves parsing some shell output -- and results
> in making a pointless directory on all non-EFI architectures.
> - We could try to have the installer signal to itself via some temp
> file. This seems fragile and complicated.
> - We could duplicate the logic for whether to make an ESP in the first
> place in several places, which seems like a terrible idea.
>

The installer should just mount it early, then it will know. This will make
the install script reusable on all architectures which will know if it is
mounted to update it. There's no need for anything else to happen and you
don't need to change the wider system.


> But this needs to be settled *now*. It's already very, very late to get
> this into 13.0 and we can't have another cycle of changing our minds
> about reviewed code.
>

No. This was not reviewed code. It was hastily tossed together without
proper follow up. You knew of my interest. You could have seen who
committed the change to mtree in the first place and added them to a
reviewboard review, but you didn't do that. I've hardly been absent, but
I'm not on IRC 24/7. You could have at least asked. But you didn't and now
want to use that fact to prevent me from objecting. That's not a functional
way to run a project.

I'm happy to settle it now and put this aside and get this settled...

Warner