From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS53758 23.128.96.0/24 X-Spam-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id 63A0B1F8C6 for ; Thu, 1 Jul 2021 03:07:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238690AbhGADJd (ORCPT ); Wed, 30 Jun 2021 23:09:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52444 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238682AbhGADJd (ORCPT ); Wed, 30 Jun 2021 23:09:33 -0400 Received: from mail-oi1-x234.google.com (mail-oi1-x234.google.com [IPv6:2607:f8b0:4864:20::234]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 59F26C061756 for ; Wed, 30 Jun 2021 20:07:03 -0700 (PDT) Received: by mail-oi1-x234.google.com with SMTP id a133so5603716oib.13 for ; Wed, 30 Jun 2021 20:07:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:message-id:in-reply-to:references:subject :mime-version:content-transfer-encoding; bh=QMF1l5iTjRedtTVxA64vfiq9pZzpHgmBy+cGHQgEICg=; b=nVm2cs8mBS4Mx1aFAJ/3dtpjWLlxWYiGZ9p/mKv9Y2qZM1O9j8GfI3RryIUfc5u34C Ac4NjyUFXrS10/UPGf5jP0E7wP4+dGxQVqcUeTGv/rzxRyFVdYyTeCKSKFt6LXwYSfyP /iltOutsBPnpswsdXR04BE6lXhQvNSsszWIuDqDHT0r/QKgdTAC/Vjmm0EDcjObaxF6B gmQjJf44tjHDRGpRxQ+25f8GLWOwVmAuwN+//W9jnELeg9z9yP94nIXeDKUvneaX8DQ+ NmzoKRa8qTWJsH/X0+6z0sVibZLdVLJu/AvuyZBef4o82jLpJJdAJRDZcNtln68Utjfc UsIQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:message-id:in-reply-to :references:subject:mime-version:content-transfer-encoding; bh=QMF1l5iTjRedtTVxA64vfiq9pZzpHgmBy+cGHQgEICg=; b=L5kWX1lEctua+H9e0/JJwLAkCtEZvoxJyvd2QHunmgxYwhiO91Siz5LR8zHPXtNTee YHyD8Art97ooHFtB0xCL6GvTUchQ5ZCHg/hW051XvFr1e7ATHBfnQPj2KW2oDN4GoHEP +Z7TCWzLCwgWX+8jijYz6zBQT4BMmUdBbTfIiwNCnA4PgNX6/8ZuH1PpjPx0LctPCKHr nThQQVQK5kD9C7nJOLzw7eJtQmSatUu0qusb1raMk2BsDgw5JOyVyhro6lqGblABTe6B YFi6G4I1IXzJMcP70F50ZjQ1BAYTbrlapL9jx3EdgpLzoWU0Nw06syhAKjTCjGWzEsvt 2CSA== X-Gm-Message-State: AOAM532ShUojh3A6O4u0lBSTFLoZ2yZB392vd/AUiBbHPie64QjkyLr3 eXRIqqkOrFR2QyGorvkgST8= X-Google-Smtp-Source: ABdhPJwsNpHTx+w11d9FbzuEyK8DQGNiyRH20mXU6zb0rBa1CdFK7v7EQefb/XaamXNrEe3CMNZ0lA== X-Received: by 2002:aca:1910:: with SMTP id l16mr16625538oii.12.1625108821708; Wed, 30 Jun 2021 20:07:01 -0700 (PDT) Received: from localhost (fixed-187-189-187-231.totalplay.net. [187.189.187.231]) by smtp.gmail.com with ESMTPSA id s19sm4082368oic.16.2021.06.30.20.07.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Jun 2021 20:07:00 -0700 (PDT) Date: Wed, 30 Jun 2021 22:06:59 -0500 From: Felipe Contreras To: =?UTF-8?B?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= , Felipe Contreras Cc: git@vger.kernel.org, Junio C Hamano , Jeff King Message-ID: <60dd31535310e_174a220828@natae.notmuch> In-Reply-To: <878s2tglzi.fsf@evledraar.gmail.com> References: <60d384ecd5ad3_4290208c@natae.notmuch> <87tulo1hs4.fsf@evledraar.gmail.com> <60d39a71299ef_429020815@natae.notmuch> <878s2tglzi.fsf@evledraar.gmail.com> Subject: Re: [PATCH] Makefile: add and use the ".DELETE_ON_ERROR" flag Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org =C3=86var Arnfj=C3=B6r=C3=B0 Bjarmason wrote: > On Wed, Jun 23 2021, Felipe Contreras wrote: > > =C3=86var Arnfj=C3=B6r=C3=B0 Bjarmason wrote: > >> On Wed, Jun 23 2021, Felipe Contreras wrote: > >> > =C3=86var Arnfj=C3=B6r=C3=B0 Bjarmason wrote: > >> >> As in db10fc6c09f this allows us to remove patterns of removing > >> >> leftover $@ files at the start of rules, since previous failing r= uns > >> >> of the Makefile won't have left those littered around anymore. > >> >> = > >> >> I'm not as confident that we should be replacing the "mv $@+ $@" > >> >> pattern entirely, since that means that external programs or one = of > >> >> our other Makefiles might race and get partial content. > >> > > >> > The reason I did it in db10fc6c09 is because both asciidoctor and > >> > asciidoc should deal with temporary files by themselves (like gcc)= . If > >> > you interrupt the build nothing gets generated. > >> = > >> If you interrupt the build default make behavior without > >> .DELETE_ON_ERROR kicks in. > > > > Generally yes, but it's possible the program traps the interrupt sign= al, > > in which case make never receives it. > = > Okey, so by "should deal with [it]" you meant that would be ideal, not > that it's something they're doing now. I misunderstood you there. It is tricky. For example asciidoctor does trap interrupt signals, but deals with them correctly. On the other hand asciidoc does not, but they clearly did intent to, just did it wrong. I sent a pull request for asciidoc to fix that [1], but so far no response. So it's a mixture of both; ideally they should do it, and they kind of do, but not all of them. Certainly git scripts do not. But they could. > >> My gcc 8.3.0 just does an unlink()/openat(..., O_RDWR|O_CREAT|O_TRUN= C) > >> dance followed by chmod() when I do e.g.: > >> = > >> gcc -o main main.c > >> = > >> So no in-place atomic renaming, does yours do something different? > > > > It doesn't rename the file, but if interrupted the file is unlinked. > = > Right, and with .DELETE_ON_ERROR that "interrupted" is extended to > "interrupted, or errors", but bringing this discussion around that's wh= y > I was confident in replacing the "rm" pattern at the start (which reall= y > is 100% replaced by .DELETE_ON_ERROR), but not the "mv" at the end > (which isn't, and is an orthagonal feature). Depnds on what "the feature" is. If the feature is not having lingering partial files on error, then gcc a= lready deals with that. If the feature is never having partial files at all, then yeah, you need the "mv" at the end, but as Jeff and Junio already pointed out: that feature is of doubtful value. I see value on .DELTE_ON_ERROR, not so much on never having partial files. I have tried to imagine why anybody would want this, and I just can't picture it, though that could be a failure of my imagination. > >> > However, other scripts like build-docdep.perl would indeed generat= e > >> > partial output. > >> > > >> > In my opinion it's the scripts themselves that should be fixed, an= d not > >> > the Makefile, *if* we care about this at all. > >> = > >> I don't think default tool/make/*nix semantics are broken, I just th= ink > >> it's neat to do that rename dance yourself, it's a cheap way to > >> guarantee that we always have working tools for use by other concurr= ent > >> scripts. > > > > It is cheap in the sense that it doesn't cost the computer much, but = it > > makes the code less maintenable and harder to read. > > > > To me it's a layering violation. If the tool is already dealing with > > interrupted builds, and on top of that make is doing the same, not on= ly > > for interrupted builds but also failures, then it makes little sense = to > > add even more safeties on top of that in the Makefile. > = > I agree for interrupted builds, but we're talking about > in-place-renaming, which is orthogonal. In-place-renaming is the means, the end-goal (I presume) is to never have partial files. Yes, it's orthogonal, but also I don't see the point. > > If this was really an important feature, it should be part of make > > itself, or ninja, or whatever. > > > > IMO the whole point of DELETE_ON_ERROR is to avoid everyone doing the= > > exact same dance in their Makefiles. > = > I agree it would be an interesting make feature, but something pretty > far from what it's doing now. > = > In general "make" has been intentionally sloppy about this sort of > thing. When you make a file "foo" it doesn't enforce that you fsync it > either, or that if it's being created the directory it's inserted into > is fsync'd. > = > In a POSIXly-strict sense it can't assume that it can operate properly > without those things happening, but in practice modern OS's deal with i= t > just fine, so "make" leaves that to the rule itself. > = > It would be nice to have a make feature to e.g. have individual rules > say "I emit on stdout, put it into $@ for me", then it could in-place > rename, fsync, display progress through "pv(1)" or whatever. Perhaps. I still don't see why this is something important. Either way a pattern I've seen lately in a lot of software is a reluctance to modernize itself, and that results in other software starting from scracth (GCC vs. LLVM, vim vs. neovim, and make vs. ninja).= If we are reaching the limit to what make can offer us--and plenty of other projects are already using more modern alternatives--does it really make much sense to focus on a small thing make can't offer us natively and work around that? Maybe it would make more sense to stop relying on make so much and attempt to make other tools support this feature natively. Cheers. [1] https://github.com/asciidoc-py/asciidoc-py/pull/195 -- = Felipe Contreras=