From owner-dev-commits-src-main@freebsd.org Wed Mar 3 15:24:17 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 04E9356D9BE for ; Wed, 3 Mar 2021 15:24:17 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qt1-x82d.google.com (mail-qt1-x82d.google.com [IPv6:2607:f8b0:4864:20::82d]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DrHpm69TRz3vLh for ; Wed, 3 Mar 2021 15:24:16 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qt1-x82d.google.com with SMTP id s15so17790404qtq.0 for ; Wed, 03 Mar 2021 07:24:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Cp6u1g/nLZnivDUEe9PnRNT2IAoeKZ87PnauSyvVsng=; b=RYRVb8VQY3czDV/bT0DJD8of44JyPSzg7JOtTrrqC8cLGaSXWx4322BgcUrdRoO9OL S4yL5ZbsrT3RkBLm+ICWrTPlQ+K+2SBv7NnklxClCGHKzdCh0UMlaPgohFBcn0mF6ITp yq2XTdFEk/gGfD+/HdfSUg3M6GHLfZHlB6OvcAgFCXf9i0MQOBKy9LAK8+Ofmku5lHp0 cQFly6AMtgnpTuI3Lu7q8QvhFdqjHIu6JUxe4/sRsvE12cLkUDhMG2Yg0auDkS/naZIE z/nveMVhL+e9aQxAQhCViWv5i/1hBAym0GrIbz5zf9mWyvf5M62rchiFUD/XWxvun+n1 s24g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Cp6u1g/nLZnivDUEe9PnRNT2IAoeKZ87PnauSyvVsng=; b=BBXn3arnmURTvop8S1tvbrUn4yPMolfAdbjVsBtKNIgYxA2v201BLE0vqFXaEp/KIs Y8zQcFFB2uJXmUdt36HOeUvkxFDcEERcqezSggBBbCgq5W+Yg99lMllJY/6wfTGuQCJN jZo1fwBbkJMwo6/qGJOtlF2y9uchLUHeSi+nVnDwHA7i9gGuPpwZbwxiJlH+gcd27vg/ qkjGT1N92tvRXuO531OpgRObZH8S8kjf/Ybao41yxWuuRAxPRBGq7aPNuw98d/cr35fH bsKeF1kGBSCtFBYztYVI1VtEHSolbj27vv0KzAtbaraUW8Q7JkND0pSl04WbX3AL1fcP d1eQ== X-Gm-Message-State: AOAM532ELyQeuys+BxWURnQqZp3KEdMSsqE0QJqDohxxisGAv4qpYgWo kZKJxKmTuTiFeaUx7Eqq/2Q8b1Z6OVoRMGhBB0t1/w== X-Google-Smtp-Source: ABdhPJw1AFTZKO0Bg36ErqZIBU+byon/yDHvXKlqhgqth2PtJNGl+KlLz/u9iG0wB5P4EpdmFtCNmNpYxFYC/oD7zlQ= X-Received: by 2002:a05:622a:303:: with SMTP id q3mr22377222qtw.235.1614785055948; Wed, 03 Mar 2021 07:24:15 -0800 (PST) MIME-Version: 1.0 References: <202103021856.122IuYgV048086@gndrsh.dnsmgr.net> <3d947e4c-a529-0b27-a8d7-415600783e53@freebsd.org> In-Reply-To: From: Warner Losh Date: Wed, 3 Mar 2021 08:24:05 -0700 Message-ID: Subject: Re: git: 2c26d77d989a - main - Remove /boot/efi from mtree, missed in 0b7472b3d8d2. To: Nathan Whitehorn Cc: Ed Maste , "Rodney W. Grimes" , Brandon Bergren , src-committers , "" , dev-commits-src-main@freebsd.org X-Rspamd-Queue-Id: 4DrHpm69TRz3vLh X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.34 X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Mar 2021 15:24:17 -0000 On Wed, Mar 3, 2021 at 7:09 AM Nathan Whitehorn wrote: > > > On 3/2/21 10:19 PM, Warner Losh wrote: > > > > > > On Tue, Mar 2, 2021, 7:01 PM Ed Maste > > wrote: > > > > On Tue, 2 Mar 2021 at 19:30, Warner Losh > > 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 > > 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