unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Vagrant Cascadian <vagrant@debian.org>
To: "Ludovic Courtès" <ludo@gnu.org>
Cc: 51985@debbugs.gnu.org, philip@philipmcgrath.com
Subject: [bug#51985] lint: Adjust patch file length check.
Date: Fri, 26 Nov 2021 13:08:51 -0800	[thread overview]
Message-ID: <87ee72fwv0.fsf@ponder> (raw)
In-Reply-To: <8735nke61x.fsf@gnu.org>


[-- Attachment #1.1: Type: text/plain, Size: 1552 bytes --]

On 2021-11-25, Ludovic Courtès wrote:
> Vagrant Cascadian <vagrant@debian.org> skribis:
>
>> With:
>>
>>   commit bdf5c16ac052af2ca9d5c3acc4acbc08fd9fdbea
>>   Author: Ludovic Courtès <ludo@gnu.org>
>>   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’s a good idea for ‘guix
> lint’ 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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-lint-Adjust-patch-file-length-check.patch --]
[-- Type: text/x-diff, Size: 3123 bytes --]

From c0738574a3571977855d655c157ab0ea0f9be6ef Mon Sep 17 00:00:00 2001
From: Vagrant Cascadian <vagrant@debian.org>
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 new
  tar format. Extend warning to explain that long files may break 'make dist'.
* tests/lint.scm: Update tests accordingly.
---
 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
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -957,8 +957,11 @@ patch could not be found."
 
      ;; Check whether we're reaching tar's maximum file name length.
      (let ((prefix (string-length (%distro-directory)))
-           (margin (string-length "guix-2.0.0rc3-10000-1234567890/"))
-           (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-directory)
@@ -968,7 +971,7 @@ patch could not be found."
                              max)
                           (make-warning
                            package
-                           (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
--- a/tests/lint.scm
+++ b/tests/lint.scm
@@ -506,17 +506,17 @@
                                  (file-name "x.patch")))))))))
     (check-patch-file-names pkg)))
 
-(test-equal "patches: file name too long"
+(test-equal "patches: file name too long which may break 'make dist'"
   (string-append "x-"
-                 (make-string 100 #\a)
-                 ".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-"
-                                               (make-string 100 #\a)
+                                               (make-string 152 #\a)
                                                ".patch"))))))))
      (check-patch-file-names pkg))))
 
-- 
2.30.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

  reply	other threads:[~2021-11-26 21:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-19 21:05 [bug#51985] lint: Adjust patch file length check Vagrant Cascadian
2021-11-22 11:22 ` Ludovic Courtès
2021-11-24 21:25   ` Vagrant Cascadian
2021-11-25 13:08     ` Ludovic Courtès
2021-11-26 21:08       ` Vagrant Cascadian [this message]
2021-11-28 17:02         ` Ludovic Courtès
2021-12-18  8:26           ` bug#51985: " Vagrant Cascadian

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ee72fwv0.fsf@ponder \
    --to=vagrant@debian.org \
    --cc=51985@debbugs.gnu.org \
    --cc=ludo@gnu.org \
    --cc=philip@philipmcgrath.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).