From debbugs-submit-bounces@debbugs.gnu.org Fri Nov 26 16:09:09 2021 Received: (at 51985) by debbugs.gnu.org; 26 Nov 2021 21:09:10 +0000 Received: from localhost ([127.0.0.1]:60614 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mqiSz-0003gj-IZ for submit@debbugs.gnu.org; Fri, 26 Nov 2021 16:09:09 -0500 Received: from cascadia.aikidev.net ([173.255.214.101]:32854) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mqiSx-0003gW-N1 for 51985@debbugs.gnu.org; Fri, 26 Nov 2021 16:09:08 -0500 Received: from localhost (unknown [IPv6:2600:3c01:e000:21:21:21:0:100e]) (Authenticated sender: vagrant@cascadia.debian.net) by cascadia.aikidev.net (Postfix) with ESMTPSA id 88DE61A982; Fri, 26 Nov 2021 13:09:01 -0800 (PST) From: Vagrant Cascadian To: Ludovic =?utf-8?Q?Court=C3=A8s?= Subject: Re: bug#51985: lint: Adjust patch file length check. In-Reply-To: <8735nke61x.fsf@gnu.org> References: <875ysnsvnt.fsf@yucca> <877dd0ph9g.fsf@gnu.org> <8735nl5jri.fsf@ponder> <8735nke61x.fsf@gnu.org> Date: Fri, 26 Nov 2021 13:08:51 -0800 Message-ID: <87ee72fwv0.fsf@ponder> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="==-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 51985 Cc: 51985@debbugs.gnu.org, philip@philipmcgrath.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --==-=-= Content-Type: multipart/mixed; boundary="=-=-=" --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 2021-11-25, Ludovic Court=C3=A8s wrote: > Vagrant Cascadian skribis: > >> With: >> >> commit bdf5c16ac052af2ca9d5c3acc4acbc08fd9fdbea >> Author: Ludovic Court=C3=A8s >> Date: Tue Nov 23 09:06:49 2021 +0100 >> >> maint: "make dist" builds tarballs in 'ustar' format. >> >> It seems like this actually needs even further updates, as that should >> allow for a much longer file length in general (although a little >> difficult to figure out the exact file length allowed). >> >> And then the corresponding test suite will need changes as well... > > I think independently of the switch to ustar, it=E2=80=99s a good idea fo= r =E2=80=98guix > lint=E2=80=99 to warn about long patch file names, but to warn a bit less > frequently than today. > > In that spirit, your patch is still relevant and worth applying IMO. Sure, although while I'm mucking around... I went ahead and did some real-world testing of file lengths usable by ustar. Using ustar adds a significant buffer, though less than one might think in the case of guix-VERSION/gnu/packages/patches/*.patch (~151 characters vs. ~99). I'm guessing this is plenty buffer though, most existing patches were only a theoretical problem... almost to the point that maybe even removing the check entirely might be fine. Updated patch integrating this and the stronger warning suggested by Philip McGrath. I haven't yet actually *tested* this updated patch yet, but the previous iteration tested just fine. :) live well, vagrant --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-lint-Adjust-patch-file-length-check.patch Content-Transfer-Encoding: quoted-printable From=20c0738574a3571977855d655c157ab0ea0f9be6ef Mon Sep 17 00:00:00 2001 From: Vagrant Cascadian Date: Fri, 26 Nov 2021 12:13:45 -0800 Subject: [PATCH] lint: Adjust patch file length check. With the switch to "ustar" format in commit bdf5c16ac052af2ca9d5c3acc4acbc08fd9fdbea, the maximum file length has increased. * guix/lint.scm (check-patch-file-names): Adjust margin used to check for patch file lengths. Increase allowable patch file length appropriate to n= ew tar format. Extend warning to explain that long files may break 'make dis= t'. * tests/lint.scm: Update tests accordingly. =2D-- guix/lint.scm | 9 ++++++--- tests/lint.scm | 8 ++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/guix/lint.scm b/guix/lint.scm index ac2e7b3841..4a5573e86e 100644 =2D-- a/guix/lint.scm +++ b/guix/lint.scm @@ -957,8 +957,11 @@ patch could not be found." =20 ;; Check whether we're reaching tar's maximum file name length. (let ((prefix (string-length (%distro-directory))) =2D (margin (string-length "guix-2.0.0rc3-10000-1234567890/")) =2D (max 99)) + ;; Margin approximating the largest path that "make dist" might + ;; create, with a release candidate version, 123456 commits, and + ;; git commit hash abcde0. + (margin (string-length "guix-92.0.0rc3-123456-abcde0/")) + (max 151)) (filter-map (match-lambda ((? string? patch) (if (> (+ margin (if (string-prefix? (%distro-direct= ory) @@ -968,7 +971,7 @@ patch could not be found." max) (make-warning package =2D (G_ "~a: file name is too long") + (G_ "~a: file name is too long which may break = 'make dist'") (list (basename patch)) #:field 'patch-file-names) #f)) diff --git a/tests/lint.scm b/tests/lint.scm index 9a91dd5426..8fa0172cf7 100644 =2D-- a/tests/lint.scm +++ b/tests/lint.scm @@ -506,17 +506,17 @@ (file-name "x.patch"))))))))) (check-patch-file-names pkg))) =20 =2D(test-equal "patches: file name too long" +(test-equal "patches: file name too long which may break 'make dist'" (string-append "x-" =2D (make-string 100 #\a) =2D ".patch: file name is too long") + (make-string 152 #\a) + ".patch: file name is too long which may break 'make dist= '") (single-lint-warning-message (let ((pkg (dummy-package "x" (source (dummy-origin (patches (list (string-append "x-" =2D (make-string 100 #\a) + (make-string 152 #\a) ".patch")))))))) (check-patch-file-names pkg)))) =20 =2D-=20 2.30.2 --=-=-=-- --==-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQRlgHNhO/zFx+LkXUXcUY/If5cWqgUCYaFM6AAKCRDcUY/If5cW qrj8AQD02tkebDhmrwJhDJUUy/Ya8U3RnzVIobf7lumScSirtgEAiZCehNi6fGBv IT1c4uugIN52Oh2OqM+EsPJRbAxZOgM= =3hkQ -----END PGP SIGNATURE----- --==-=-=--